-
-
Notifications
You must be signed in to change notification settings - Fork 193
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
feat: Add third-party auth support #999
Conversation
@@ -75,6 +75,7 @@ class Supabase { | |||
PostgrestClientOptions postgrestOptions = const PostgrestClientOptions(), | |||
StorageClientOptions storageOptions = const StorageClientOptions(), | |||
FlutterAuthClientOptions authOptions = const FlutterAuthClientOptions(), | |||
Future<String> Function()? accessToken, |
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.
Maybe create a typedef for this?
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.
Thanks for the feedback! I know this is highly opinionated, but I really feel that using typedef generally decreases the readability of the code. Especially since a property named accessToken
doesn't sound like a function.
@@ -122,18 +133,30 @@ class SupabaseClient { | |||
}, | |||
_httpClient = httpClient, | |||
_isolate = isolate ?? (YAJsonIsolate()..initialize()) { | |||
auth = _initSupabaseAuthClient( | |||
_authInstance = _initSupabaseAuthClient( |
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.
When a custom access token is passed, no auth instance should be created, right? From what I see, this is still the case, but just because it's marked as late
. I would prefer a clearer structure to ensure no auth instance is created.
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.
We can always come back and change the implementation of it. For now, I'm going to merge this PR as this is not really critical.
What kind of change does this PR introduce?
Adds support for the accessToken option on the Supabase client which can be used to provide a third-party authentication (e.g. Auth0, Clerk, Firebase Auth, ...) access token or ID token to be used instead of Supabase Auth.
When set, supabase.auth.xyz cannot be used and an error will be thrown.
js PR here: supabase/supabase-js#1004