-
Notifications
You must be signed in to change notification settings - Fork 375
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
Generic OAuth provider #1372
base: master
Are you sure you want to change the base?
Generic OAuth provider #1372
Conversation
Tested with a local instance of github.com/hitobito/hitobito
143e888
to
ebac0b1
Compare
Wow! Thank you for attempting to solve this issue, this is quite an effort! I will take a deeper look soon, but here's some high-level observations so far:
The team generally wants to keep such large features internal, as there's quite a bit of feedback that's not publicly available and that may not be properly addressed. Please don't get discouraged from contributing further if we decide to put this implementation on hold. |
Thanks for your feedback! I'm glad to hear this is at least getting some attention. It is insignificant to me whether my code or someone else's code will do the job in the end. I just wanted to get the discussion rolling and lower the mental hurdle of "we don't have time to implement this" a little.
Totally agree that this does not scale arbitrarily. I had to do this because all the current providers are configurable via envconfig, and I did not want to introduce a paradigm shift there since this PR is already big enough. I also imagine for most small to medium sized applications, up to 3 (or up to 5 or 10 if you prefer) custom OAuth providers should be enough. But I understand that you want to keep the product flexible to support even the largest of applications. Keep in mind that so far, gotrue only supported one single Keycloak OAuth provider, which in theory there could be multiple. So in this regard having 3 of the custom providers would already represent a step forward in my opinion. But if this were merged, you would of course also have to think about migration later, when you get around to implementing something more scalable.
Well, evidently it's possible, but not with unlimited customizability. Here as well, I did not want to create a paradigm shift by introducing user-defined code which will run inside gotrue (or at least communicates directly with gotrue), especially for security reasons. So as a first step, I reused envconfig's map parsing abilities to implement a simple mapping, as suggested by several community members in the issue. If you find a better way I'm all for it! Please just consider keeping Keycloak support around until some kind of generic OAuth provider is integrated into gotrue. It's currently the only way for someone like me (and all the interested people which I tagged in #451 (comment)) to connect Supabase to smaller, domain-specific identity providers. |
Hi folks, it would be good to revive this. From what I see, it's mainly a refactor of how the various OAuth providers are obtained/organized within the code. The |
Any workaround if I want to use a third party oauth provider? |
Set up a Keycloak instance and configure your third party provider there. Then use the existing supabase keycloak provider. |
Can I use multiple third party providers at the same time when using keycloak? |
Yes, keycloak supports multiple third party providers at the same time. |
What kind of change does this PR introduce?
Fixes most of #451. This PR introduces a new generic OAuth provider, which can be configured to connect to a wide range of OAuth 2.0 identity providers. Thus, most requests in #451 should be solvable using this new provider.
What is the current behavior?
Only explicitly supported providers can be connected to Supabase auth. For applications which need domain-specific or niche providers, it is not practical or even possible to implement a separate provider class and contribute it to supabase auth / gotrue. At the same time, the keycloak provider (which would allow delegation to arbitrary identity providers) might soon be deprecated according to #1108, further closing off Supabase for very domain-specific applications.
What is the new behavior?
Up to three instances of the same new generic OAuth 2.0 provider can now be configured in gotrue. This provider works largely the same as other, hardcoded providers. For mapping the identity provider API payloads to gotrue JWT claims, a mapping can be specified in the config, including resolution of nested fields in the API payload. See test.env for an example mapping.
Currently not supported are custom claims as well as reading claim values from the authorization response (as would be common for OIDC if I understand correctly). The implementation could be extended in the future to support these tasks if needed.
Additional context
I chose not to use reflection for setting the claim values, for security reasons. Also for security reasons, I chose not to use Sprintf for converting claim values to string. Rather, I took a quite manual, explicit approach for both of these tasks. However, I am no Go developer, so please let me know in case I should change something.
I based the provider on the last merged OAuth provider Kakao, in an attempt to have as up-to-date a codestyle as possible.
I have successfully tested the provider with a local instance of https://github.com/hitobito/hitobito/, but that setup is quite involved (requiring to clone at least 3 git repositories and modifying the docker compose setup of gotrue and hitobito). I have called for other people to test with their own identity providers in #451 (comment).