Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keycloak 17+ compatibility changes #232

Merged
merged 2 commits into from
Oct 11, 2022
Merged

Conversation

p53
Copy link

@p53 p53 commented Oct 11, 2022

Title

Summary

Type

[] Bug fix
[] Feature request
[] Enhancement
[] Docs

Why?

Requirements

How to try it?

Documentation

Additional Information

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.

@p53 p53 self-assigned this Oct 11, 2022
@p53 p53 added this to In progress in Gatekeeper via automation Oct 11, 2022
@p53 p53 added this to the 2.0.0 milestone Oct 11, 2022
@p53 p53 merged commit c300523 into gogatekeeper:master Oct 11, 2022
Gatekeeper automation moved this from In progress to Done Oct 11, 2022
@brunocribeiro
Copy link

I must say, this is a pretty bad decision, change the hard code path length is clearly not the fix to support deployments that either uses a custom context or not.

Now it doesn't work for deployments that configure a custom context using http-relative-path 😞

@p53
Copy link
Author

p53 commented Oct 17, 2022

@brunocribeiro problem is that Keycloak 17+ has more changes then just realm part, yes i know it doesn't support the old way but i don't want to have two different sets of tests or make them too much complicated just because of realm thing, in tests we are faking keycloak and so /auth thing is used everywhere there, i could change the code but i would logically need run tests against both and there are some additional changes, then i would need to cover all future changes on top of this which i think can be quite bad for maintaining and testing. I would say that relative path should be used just for backward compatibility not to set custom paths, that is even not good for setting some kind of standard

@ChristianCiach
Copy link

ChristianCiach commented Oct 21, 2022

So you are saying that go-gatekeeper doesn't work with Keycloaks that have been configured with --http-relative-path=/auth? That's a bummer. There is just no alternative for us.

@p53
Copy link
Author

p53 commented Oct 21, 2022

@ChristianCiach go-gatekeeper 2.0.0 would not work if released, keycloak has released new version 17 which removes /auth, it can be set with http relative path but i am not sure for how long there will be this option+i would need to adapt also other parts of code and hope they won't change some endpoint in newer versions, then probably adjustments would be hard or not possible, so i am quite afraid of maintaing backward compatibility with <17 releases

@ChristianCiach
Copy link

You don't need to maintain compatibility with keycloak < 17, but I think it would be better to maintain compatibility with supported keycloak versions that do have the --http-relative-path option. Why do you think this option will go away?

@p53
Copy link
Author

p53 commented Oct 21, 2022

@ChristianCiach ok checked documentation https://github.com/keycloak/keycloak/blob/91a58ed1b916f70c1a6de46409c7417a72437de7/docs/guides/src/main/server/reverseproxy.adoc#different-context-path-on-reverse-proxy, seems they introduced it because of proxies, so it should probably stay there, ok so we can make that compromise that /auth will be supported but not backward compatibility with Keycloak <17 will be guaranteed

@p53
Copy link
Author

p53 commented Oct 22, 2022

One more thing only / and /auth will be supported as nerzal gocloak library which is used in gatekeepet supports only those two

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants