-
Notifications
You must be signed in to change notification settings - Fork 450
fix: Add openid scope by default for Keycloak #1867
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
base: master
Are you sure you want to change the base?
Conversation
Since Keycloak 22, this is required for the userinfo endpoint to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @AaronDewes, does this break things for folks using keycloak <= 18? you can actually pass in scopes as a query parameter when you call the authorize endpoint like this: /authorize?provider=keycloak&scopes=openid
Hi! I'm aware of the ability to set scopes manually (That's how I verified adding the scope fixes the problems I was experiencing), but in my opinion, you should have working defaults. According to the docs, the openid scope should work with keycloak 18, but I'll check if I can set up a test instance to try. I'm not sure how many versions you want to be backwards compatible with, so if you have a specific version in mind I can try, please let me know. |
@AaronDewes we don't have a minimum keycloak version enforced unfortunately, so if there's someone out there using this library with an old keycloak version that does not support this scope, then it will break things for them if they are using Supabase.
I do agree with this point, and i think it should be relatively safe for us to add this as long as the last 3 major keycloak versions support the it would also be great if you can fix the test 🙏 |
Great! I'll have a look at the tests. The latest Keycloak major version is 26, so last 3 major versions are definitely not a problem (All of these versions were broken before). Red Hat SSO (which was recently replaced by "Keycloak Red Hat build") also supports this in its last (probably LTS) version. |
@AaronDewes Perhaps we can check the keycloak version somehow to see if the openid scope is needed? I'm a little concerned that going back 3 versions may not be enough for some people using this provider. Some links to documentation or something concrete to help determine if this change is safe would be ideal. |
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
Since Keycloak 19, this is required for the userinfo endpoint to work.
If you try to use Supabase with modern Keycloak, authentication fails with "Error getting user profile from external provider".
What is the new behavior?
Login with Keycloak works.
Additional context