feat(ui_oauth_google)!: migrate to google_sign_in 7 #689
+170
−90
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,14 @@ class GoogleProvider extends OAuthProvider { | |
| /// Ignored on Android and iOS (if `iOSPreferPlist` is true). | ||
| final String clientId; | ||
|
|
||
| /// The client ID of the web OAuth client associated with the app's | ||
| /// server-side component, if any. | ||
| /// | ||
| /// On Android this is required to receive an ID token, unless the app uses | ||
| /// `google-services.json` and it contains a web OAuth client entry, in | ||
| /// which case the plugin reads the value from there. | ||
| final String? serverClientId; | ||
|
|
||
| /// When true, the Google Sign In plugin will use the GoogleService-Info.plist | ||
| /// for configuration instead of the `clientId` parameter. | ||
| final bool iOSPreferPlist; | ||
|
|
@@ -28,7 +36,23 @@ class GoogleProvider extends OAuthProvider { | |
| /// The list of requested authorization scopes requested when signing in. | ||
| final List<String>? scopes; | ||
|
|
||
| /// The plugin instance. google_sign_in 7 exposes a single shared instance. | ||
| /// Assignable so that tests can inject a mock. | ||
| GoogleSignIn provider = GoogleSignIn.instance; | ||
|
|
||
| // google_sign_in 7 requires initialize to be called exactly once, while | ||
| // multiple GoogleProvider instances may be created (for example one per | ||
| // GoogleSignInButton). The first instance to sign in configures the plugin. | ||
| static Future<void>? _initialization; | ||
|
|
||
| /// Resets the one-time initialization state. | ||
| /// | ||
| /// The plugin is initialized exactly once per process, so tests that inject | ||
| /// a fresh mock in `setUp` must clear the cached future to stay isolated. | ||
| @visibleForTesting | ||
| static void debugReset() { | ||
| _initialization = null; | ||
| } | ||
|
|
||
| @override | ||
| final fba.GoogleAuthProvider firebaseAuthProvider = fba.GoogleAuthProvider(); | ||
|
|
@@ -44,19 +68,14 @@ class GoogleProvider extends OAuthProvider { | |
|
|
||
| GoogleProvider({ | ||
| required this.clientId, | ||
| this.serverClientId, | ||
| this.redirectUri, | ||
| this.scopes, | ||
| this.iOSPreferPlist = false, | ||
| }) { | ||
| firebaseAuthProvider.setCustomParameters(const { | ||
| 'prompt': 'select_account', | ||
| }); | ||
| } | ||
|
|
||
| bool _ignoreClientId() { | ||
|
|
@@ -68,25 +87,55 @@ class GoogleProvider extends OAuthProvider { | |
| return false; | ||
| } | ||
|
|
||
| Future<void> _ensureInitialized() { | ||
| return _initialization ??= provider.initialize( | ||
| clientId: _ignoreClientId() ? null : clientId, | ||
| serverClientId: serverClientId, | ||
| ).catchError((Object err) { | ||
| // Allow a later sign-in attempt to retry initialization instead of | ||
| // rethrowing the same stale error forever. | ||
| _initialization = null; | ||
| throw err; | ||
| }); | ||
| } | ||
|
Comment on lines
+90
to
+100
There was a problem hiding this comment. The Future<void> _ensureInitialized() {
return _initialization ??= provider.initialize(
clientId: _ignoreClientId() ? null : clientId,
serverClientId: serverClientId,
).catchError((Object err) {
// Allow a later sign-in attempt to retry initialization instead of
// rethrowing the same stale error forever.
_initialization = null;
throw err;
});
}
Author
There was a problem hiding this comment. Done in 160260b — chained |
||
|
|
||
| @override | ||
| void mobileSignIn(AuthAction action) async { | ||
| final requestedScopes = scopes ?? const <String>[]; | ||
|
|
||
| try { | ||
| await _ensureInitialized(); | ||
|
|
||
| final account = await provider.authenticate(scopeHint: requestedScopes); | ||
|
|
||
| // Authentication and authorization are separate steps in | ||
| // google_sign_in 7. Reuse an existing authorization when one is | ||
| // available, otherwise prompt for the requested scopes so that the | ||
| // credential carries an access token, matching the previous behavior. | ||
| String? accessToken; | ||
| if (requestedScopes.isNotEmpty) { | ||
| final client = account.authorizationClient; | ||
| final authorization = | ||
| await client.authorizationForScopes(requestedScopes) ?? | ||
| await client.authorizeScopes(requestedScopes); | ||
| accessToken = authorization.accessToken; | ||
| } | ||
|
|
||
| final credential = fba.GoogleAuthProvider.credential( | ||
| accessToken: accessToken, | ||
| idToken: account.authentication.idToken, | ||
| ); | ||
|
|
||
| onCredentialReceived(credential, action); | ||
| } on GoogleSignInException catch (err) { | ||
| if (err.code == GoogleSignInExceptionCode.canceled) { | ||
| authListener.onError(AuthCancelledException()); | ||
| } else { | ||
| authListener.onError(err); | ||
| } | ||
| } catch (err) { | ||
| authListener.onError(err); | ||
| } | ||
| } | ||
|
|
||
| @override | ||
|
|
@@ -112,6 +161,7 @@ class GoogleProvider extends OAuthProvider { | |
| if (defaultTargetPlatform == TargetPlatform.android || | ||
| defaultTargetPlatform == TargetPlatform.iOS || | ||
| defaultTargetPlatform == TargetPlatform.macOS) { | ||
| await _ensureInitialized(); | ||
| await provider.signOut(); | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Because
_initializationis a static variable, its state will persist across different tests in the same test suite. Since each test ingoogle_sign_in_test.dartinjects a newMockGoogleSignIninstance insetUp, subsequent tests will reuse the cached_initializationfuture from the first test and will not callinitializeon their respective mock providers. To ensure proper test isolation and prevent potential flakiness, consider exposing a@visibleForTestingstatic method to reset_initializationand calling it in the test'ssetUpblock.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.
Done in 160260b — added a
@visibleForTesting debugReset()that clears_initialization, and call it at the top of the testsetUpso each test re-initializes on its freshly injected mock.