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

fix: Add openid scope by default for Keycloak #1867

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AaronDewes
Copy link

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

Since Keycloak 22, this is required for the userinfo endpoint to work.
@AaronDewes AaronDewes requested a review from a team as a code owner December 10, 2024 18:17
Copy link
Member

@kangmingtay kangmingtay left a 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

@AaronDewes
Copy link
Author

AaronDewes commented Dec 12, 2024

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.

@kangmingtay
Copy link
Member

kangmingtay commented Dec 12, 2024

@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.

you should have working defaults.

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 openid scope.

it would also be great if you can fix the test 🙏

@AaronDewes
Copy link
Author

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.

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

Successfully merging this pull request may close these issues.

2 participants