supabase / supabase-flutter

Flutter integration for Supabase. This package makes it simple for developers to build secure and scalable products.
https://supabase.com/
MIT License
738 stars 184 forks source link

Full Native Auth support #399

Closed DanMossa closed 1 year ago

DanMossa commented 1 year ago

The criteria for having a successful native authentication process is the ability to do the following:

Sign in on Google working on Android, iOS, and Web

When using sign in with Google, you're currently limited to having sign in with Google on either Android and Web, or iOS. The reason for this is because on GCP, you create credentials and you choose the type of device that the OAuth client is. The web application credentials work for both Android and Web where as for iOS, you're required to use the iOS credentials. Supabase only has a slot for a single client ID/Secret.

This PR is also required for us to pass in a Nonce when using iOS https://github.com/google/GoogleSignIn-iOS/pull/244

Sign in with Apple working on Android, iOS, and Web

I was able to get iOS working on iPhone, but I'm having trouble getting Sign in with Apple working on the web as well. These are the instructions I'm following but I'll update this when I / others have better luck setting this up https://pub.dev/packages/sign_in_with_apple

When creating an account with social auth, you can then link your email with it.

When creating an account with Google the user's email address is registered inside the auth.users table. A user can then link their accounts by following the forgot password route with the same email.

When creating an account with Apple, depending on the user's preferences, either the plain text email or the private email is registered inside auth.users. A user can then link their accounts by following the forgot password route with the same email. This is only possible if the user does not use Apples's private email, but Apple users are aware of this.

When creating an account with email first, you can then link your social auth to it.

This is not possible at the moment. We can an error back stating there's a duplicate of the email already. I've read that toggling isSSO fixes this but I haven't tested it yet.

dshukertjr commented 1 year ago

Mentioned it here as well, but auth team will be working on a solution to fix the Google login platform problem.

We will probably add a clientId parameter on the OIDC endpoint so that when signing in with Android or iOS we can pass it as a parameter.

gregorym commented 1 year ago

Does it mean we will be able to support authentication on web and mobile with different Google OAuth IDs?

dshukertjr commented 1 year ago

@gregorym Yup!

medclans commented 1 year ago

@dshukertjr Waiting for the Google login to work on iOS platform. This issue is blocking the app release for my company. Can you please tell us when can we expect the fix? It helps to plan our release.

elliottetzkorn commented 1 year ago

@dshukertjr Waiting for the Google login to work on iOS platform. This issue is blocking the app release for my company. Can you please tell us when can we expect the fix? It helps to plan our release.

I'm in the same position. I'm sure this work is hard, so if it's something like a 3 month or more timeline that would definitely be helpful to know. I am similarly trying to time a production release around this feature. If it will be a while I will just remove Google sign in from my app for now and launch w/o this.

dshukertjr commented 1 year ago

@medclans @elliottetzkorn We are working on bringing native Apple login on multiple platforms right now. Supporting native Google login will come after that. Can't promise anything definitely, but hopefully we can bring native login with Apple and Google on iOS, and Android very soon. We do not have any timeline for Desktop support for them at the moment.

Follow Launch Week 7 for more updates 😉

elliottetzkorn commented 1 year ago

Amazing thanks!

gregorym commented 1 year ago

@dshukertjr Looks like apple was just announced but Google isn't yet. Any timeline on Google Native Auth?

https://supabase.com/blog/supabase-auth-sso-pkce

dshukertjr commented 1 year ago

@gregorym I should have posted it here as well. https://github.com/supabase/supabase-flutter/issues/5#issuecomment-1507234288

elliottetzkorn commented 1 year ago

Does this mean native sign in with apple is supported on Android now as well?

dshukertjr commented 1 year ago

@elliottetzkorn Apple login on Android is web-based anyway, so you can use the signInWithOAuth() method for it.

mohsin2596 commented 1 year ago

Can anyone share a working way to use Sign in with google just on Android platform?

dshukertjr commented 1 year ago

@mohsin2596 You can click on the Native Google login code at the bottom of the comment here to find a working example.

elliottetzkorn commented 1 year ago

Google folks have responded to the nonce PR, which is very encouraging! https://github.com/google/GoogleSignIn-iOS/pull/244#pullrequestreview-1436796600

elliottetzkorn commented 1 year ago

@dshukertjr could you provide step-by-step instructions for getting Sign in with Apple working on Android? I am really struggling with this part. It is working fine on iOS, and Android I get the error "OAuth state parameter missing".

dshukertjr commented 1 year ago

@elliottetzkorn Apple login on Android is only supported through the web-based login method. You can follow this guide to set it up, and call it like this:

await supabase.auth.signInWithOAuth(Provider.apple);
dshukertjr commented 1 year ago

@elliottetzkorn

will following this guide not invalidate my native Apple sign in on iOS?

For the native Apple sign in, you should enter your bundle ID in your dashboard. As long as you have it there, following the guide will not invalidate your native Apple sign in on iOS.

Screenshot 2023-06-08 at 15 29 09

Also, "Add your domain to the Domains and Subdomains box (do not add https://, just add the domain).", what should this be for flutter?

This is the same for all platforms, and it is the domain of your Supabase URL, e.g. mdembiczgqmbdobqwitc.supabase.co

elliottetzkorn commented 1 year ago

@elliottetzkorn Apple login on Android is only supported through the web-based login method. You can follow this guide to set it up, and call it like this:

await supabase.auth.signInWithOAuth(Provider.apple);

Thank you!

elliottetzkorn commented 1 year ago

@elliottetzkorn

will following this guide not invalidate my native Apple sign in on iOS?

For the native Apple sign in, you should enter your bundle ID in your dashboard. As long as you have it there, following the guide will not invalidate your native Apple sign in on iOS.

Screenshot 2023-06-08 at 15 29 09

Also, "Add your domain to the Domains and Subdomains box (do not add https://, just add the domain).", what should this be for flutter?

This is the same for all platforms, and it is the domain of your Supabase URL, e.g. mdembiczgqmbdobqwitc.supabase.co

Thanks!!

dshukertjr commented 1 year ago

I have locked the conversation here to make it a public discussion board instead of a Q&A issue. If you have any question/ comments about native sign in, please post them here https://github.com/supabase/supabase-flutter/issues/5.

dshukertjr commented 1 year ago

@bdlukaa @DanMossa @Vinzent03 I have a few things that I would like to discuss and love to hear your opinions on.

First of all, a good news 🎉 Supporting multiple Client IDs and multiple bundle IDs is finally landing Supabase, and an official announcement is scheduled to go out Friday morning PST. This means that users can finally add Google login for web, Android, and iOS or add Apple logins on multiple iOS apps. The update to allow users to be able to enter multiple client IDs or bundle IDs as comma separated values in the dashboard is being rolled out soon. With this, we should be able to tick all the check boxes on this issue.

Now with this, I want to discuss how to go about implementing native auth methods on supabase_flutter.

Currently, we have signInWithApple() method, that I personally regret implementing for two reasons

The whole reason why I thought adding signInWithApple() is because the process of signing a user in via signInWithIdToken requires generating a nonce and hashing it, which is not so relevant to the developer trying to implement a simple login button. Luckily, signInWithApple() has an experimental flag, so we have a bit of freedom to edit/ remove it.

Native Apple login code 👇 ```dart import 'package:crypto/crypto.dart'; import 'package:sign_in_with_apple/sign_in_with_apple.dart' as apple; final rawNonce = _generateRandomString(); final hashedNonce = sha256.convert(utf8.encode(rawNonce)).toString(); final credential = await apple.SignInWithApple.getAppleIDCredential( scopes: [ apple.AppleIDAuthorizationScopes.email, apple.AppleIDAuthorizationScopes.fullName, ], nonce: hashedNonce, ); final idToken = credential.identityToken; if (idToken == null) { throw AuthException('Could not find ID Token from generated credential.'); } await signInWithIdToken( provider: Provider.apple, idToken: idToken, nonce: rawNonce, ); ```

However, implementing all native login methods is not very realistic, and I wonder what we should do

I think we have a few choices

  1. Keep signInWithApple(), but do not add any other native methods, and add detailed documentations on README.md and supabase.com for them.
  2. Remove signInWithApple() and add detailed documentations on README.md and supabase.com for all the auth providers.
  3. Create a supabase_native_sign_in_helper library (we can think of a better name), which contains AppAuth, Apple sign in, and any future dependencies that we might need, and provide signInWithXXX methods there that takes care of nonce generations and stuff.

I personally want to go with option 2 here, but the steps to implement Google login involves quite a bit of boiler plate code. Because google_sign_in library does not allow users to specify nonce, we have to use another method, and the method I was able to find was using this one using flutter_appauth.

Native Google login code 👇 ```dart import 'package:crypto/crypto.dart'; import 'package:flutter_appauth/flutter_appauth.dart'; Future signInWithGoogle(String clientId) { // Just a random string final rawNonce = _generateRandomString(); final hashedNonce = sha256.convert(utf8.encode(rawNonce)).toString(); /// bundle ID of the app const bundleId = 'com.supabase.example'; /// fixed for google login const redirectUrl = '$bundleId:/google_auth'; /// fixed for google login const discoveryUrl = 'https://accounts.google.com/.well-known/openid-configuration'; // authorize the user by opening the concent page final result = await appAuth.authorize( AuthorizationRequest( clientId, redirectUrl, discoveryUrl: discoveryUrl, nonce: hashedNonce, scopes: [ 'openid', 'email', ], ), ); if (result == null) { throw 'No result'; } // Request the access and id token to google final tokenResult = await appAuth.token( TokenRequest( clientId, redirectUrl, authorizationCode: result.authorizationCode, discoveryUrl: discoveryUrl, codeVerifier: result.codeVerifier, nonce: result.nonce, scopes: [ 'openid', 'email', ], ), ); final idToken = tokenResult?.idToken; if (idToken == null) { throw 'No idToken'; } return supabase.auth.signInWithIdToken( provider: Provider.google, idToken: idToken, nonce: rawNonce, ); } ```

Do you think as long as we have detailed instruction on how to implement it, it would be okay to have some long boiler plate code, or should we provide something that makes it easier to implement it?

DanMossa commented 1 year ago

@dshukertjr

I'm a big fan of setting up a simple method with documentation on how to use Apple/Google/etc logins.

Like I think we should just have a basic signInWithIdToken with documentation explaining the different fields with examples of how to use Google and Apple.

dshukertjr commented 1 year ago

@DanMossa So that would be option 2, correct? Right there with you!

While we wait for Bruno and Vinzent to drop what they think, I will prepare a PR to update the README.md to include detailed instructions on how to setup Apple login and Google login with signInWithIdToken method.

Vinzent03 commented 1 year ago

I kinda like to keep the signInWithApple() method, because if we document on how to implement that with signInWithIdToken and it's just copy pasting from the website, I think we can just keep it. The same goes for google and other providers, right? If we document what the dev has to copy paste, it's not a great dx. But I understand the dependency issue, so I would go with the third option. I think that makes the best experience for the dev. If we already document on how to use native auth for these providers, we can provide a ready to go package as well.

dshukertjr commented 1 year ago

@Vinzent03 Nice! Yeah, it is some unnecessary long code for every developer to copy and paste every single time. Option 3 does take the best of both world off loading the dependency issue and providing better developer experience!

Let's also wait for @bdlukaa to hear what he thinks!

bdlukaa commented 1 year ago

From a maintainer perspective, keeping the signInWithApple is a dead-hell. Depending on third-party libraries is not an easy task, but it is feasible. From a dev perspective, I would LOVE to have such method built-in, with no extra code.

But keeping it assembles the question: do we need to create a sign in method for every supported third-party authentication method? Keeping the method would also imply in large bundle size on apps that don't make usage of social login.

With that said, I like the second option tho. It should be up to the dev to handle these stuff (like scopes), I believe. There is already parts that the dev will need to do regardless of the solution we adopt, such as configuring intents on Android, so adding a dependency surely will not be an issue.

Some methods, such as generating the nonce, could be built-in tho.

dshukertjr commented 1 year ago

Thanks everyone! It's so nice to discuss this with all of you.

Why don't we do this. Let's start out with option 2, where we remove the signInWithApple() method, and add an instruction in the docs + README.md. Let's then see how the community reacts to it and if there is a strong push for an simpler way to implement things, we can consider option 3 to create a separate library. This buys us some time to think about how to go about implementing the library, and maybe we can come up with a better way to do it too!

DanMossa commented 1 year ago

@dshukertjr We can also do something where signInWithIdToken can take in a dependency and depending on what that dependency is we can setup auth for the user.

Example: A user do this

import 'package:sign_in_with_apple/sign_in_with_apple.dart' as apple;

await signInWithIdToken(
  provider: Provider.apple,
  idToken: idToken,
  nonce: rawNonce
  dependency: apple,
);

signInWithIdToken

import 'package:crypto/crypto.dart';

signInWithIdToken(provider, idToken, nonce, dependency){
  final rawNonce = _generateRandomString();
  final hashedNonce = sha256.convert(utf8.encode(rawNonce)).toString();

  final credential = await apple.SignInWithApple.getAppleIDCredential(
    scopes: [
      apple.AppleIDAuthorizationScopes.email,
      apple.AppleIDAuthorizationScopes.fullName,
    ],
    nonce: hashedNonce,
  );

  final idToken = credential.identityToken;
  if (idToken == null) {
    throw AuthException('Could not find ID Token from generated credential.');
  }

return idToken;
}

I kind of hate this though and would rather just have Option 2.

dshukertjr commented 1 year ago

@DanMossa Thanks for the suggestion. In that case would the dependency parameter be a type dynamic? If we do that, is there a way for supabase_flutter to tell what type dependency is?

bdlukaa commented 1 year ago

Well, for sign_in_with_apple package, there is sign_in_with_apple_platform_interface, which doesn't add any extra platform code. This way we would be able to have the dev to add the sign_in_with_apple, if wanted, and keep the helper method. The same implies for google with google_sign_in_platform_interface

bdlukaa commented 1 year ago

If we do that, is there a way for supabase_flutter to tell what type dependency is?

Currently in dart these is no union types (see https://github.com/dart-lang/language/issues/83), but it is possible to make it dynamic and check if the type matches the provider type.

dshukertjr commented 1 year ago

@bdlukaa

Currently in dart these is no union types (see https://github.com/dart-lang/language/issues/83), but it is possible to make it dynamic and check if the type matches the provider type.

But in order to check the type, we would do something like dependency is AppleSignIn right? In order to do that, we would need the AppleSignIn class in the code, meaning we would have to add apple_sign_in as the dependencies, don't we?

Vinzent03 commented 1 year ago

I think you are right, because the sign_in_with_apple_platform_interface does only provide the class you get by calling getAppleIDCredential so the platform interface dependency wouldn't work.

bdlukaa commented 1 year ago

It'd actually look something like the following:

just a POC tho

// the implementation
import 'package:crypto/crypto.dart';

String getAppleNonce() {
  final rawNonce = _generateRandomString();
  final hashedNonce = sha256.convert(utf8.encode(rawNonce)).toString();

  return hashedNonce;
}

// idToken optional maybe?
void signInWithIdToken(provider, idToken, dynamic credential){
  if (credential is AuthorizationCredentialAppleID) {
    final idToken = credential.identityToken;
    if (idToken == null) {
      throw AuthException('Could not find ID Token from generated credential.');
    }
  } else if (credential is GoogleSignInAccount) {
    // impl with the provided info
  } 

  return idToken;
}
// the usage

// apple
await signInWithIdToken(
  provider: Provider.apple,
  idToken: idToken,
  dependency: await apple.SignInWithApple.getAppleIDCredential(
    scopes: [
      apple.AppleIDAuthorizationScopes.email,
      apple.AppleIDAuthorizationScopes.fullName,
    ],
    nonce: getAppleNonce(),
  ),
);

// google
await signInWithIdToken(
  provider: Provider.google,
  idToken: idToken,
  dependency: await GoogleSignIn().signIn(),
);

The platform interface for these would work because we only need the data from them. The one responsible for calling the actual platform method (and adding the dependency) is the developer.

dshukertjr commented 1 year ago

@bdlukaa In order to have credential is AuthorizationCredentialAppleID through, we would need to add apple_sign_in as the dependencies of supabase_flutter, which defeats the original purpose of getting rid of dependencies.

bdlukaa commented 1 year ago

Not really. AuthorizationCredentialAppleID comes from sign_in_with_apple_platform_interface, which itself doesn't add any native implementation

See https://pub.dev/documentation/sign_in_with_apple_platform_interface/latest/authorization_credential/AuthorizationCredentialAppleID-class.html

This would make the sign in with apple feature optional to the developer

dshukertjr commented 1 year ago

@bdlukaa I see. Yeah, sounds like an idea that is worth exploring!

Vinzent03 commented 1 year ago

To be honest, I would wait with the removal of the signInWithApple method until we have a solution. I find the raw removal and replacement with documentation on how to do it yourself until we are done discussing strange.

I don't think the semi integration proposed here is future-proof, because that way may not always work in the future or for every other provider. Maybe other providers don't even provide a platform interface package. I would rather make it completely inclusive, like creating a new package that holds these native helpers (solution 3), or doing only documentation (solution 2).

dshukertjr commented 1 year ago

Yeah, we are in a weird position especially for Google login, because the official Google login package isn't a viable option for us, and we have to go with flutter_appauth package. There are a lot of moving parts here. I said it might be worth exploring, because we are just brain storming possible ideas here and I wanted here to be a safe place to talk about whatever ideas that come to our minds.

With the current situations though, the proposal of adding a dependency parameter seems a bit risky as @Vinzent03 has said, and might not be an direction that I would pursue personally. However, anyone is welcome to explore the possibilities, and maybe someone can come up with a cleaver and safe implementation down the line.

For the time being though, I've proposed to go with option 2 for now, and see how the community reacts and if there seems to be a huge demand for a simpler way of implementing native auth, we can consider option 3 here, but do we have a strong objection against this route?

Also maybe instead of getting rid of signInWithApple() immediately, we can mark it as deprecated and leave it for a while.

dshukertjr commented 1 year ago

I am going to close this issue as all the tasks on this issue is complete.