From cc13213449fa33b9a812fe5a0f980cd191eadabc Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2020 12:37:13 -0800 Subject: [PATCH 1/7] [google_sign_in_web] Wait for Auth2 to be really ready. The Auth object that `.init()` returns may not be fully ready by the time we start calling methods on it; however it has a `.then()` method that gets called when it is *really* ready to go. * Add a completer to signal when Auth2 is ready. * Move auth2-dependent init to the `then` method. * onSuccess, complete the auth2 ready future. * onError, throw a PlatformException. This is normally triggered when the domain:port of the web app hasn't been whitelisted on the google sign in console, and things like that. Useful for debugging. --- .../lib/google_sign_in_web.dart | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart index 913cd25e6a85..8b5e46c71ebe 100644 --- a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart +++ b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart @@ -5,14 +5,13 @@ import 'dart:async'; import 'dart:html' as html; +import 'package:flutter/services.dart'; import 'package:google_sign_in_platform_interface/google_sign_in_platform_interface.dart'; import 'package:flutter_web_plugins/flutter_web_plugins.dart'; import 'package:js/js.dart'; import 'package:meta/meta.dart'; import 'src/generated/gapiauth2.dart' as auth2; -// TODO: Remove once this lands https://github.com/dart-lang/language/issues/671 -import 'src/generated/gapiauth2.dart' show GoogleAuthExtensions; import 'src/load_gapi.dart' as gapi; import 'src/utils.dart' show gapiUserToPluginUserData; @@ -39,14 +38,15 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { } Future _isGapiInitialized; + Future _isAuthInitialized; /// This is only exposed for testing. It shouldn't be accessed by users of the /// plugin as it could break at any point. @visibleForTesting - Future get initialized => _isGapiInitialized; + Future get initialized => + Future.wait([_isGapiInitialized, _isAuthInitialized]); String _autoDetectedClientId; - FutureOr _lastSeenUser; /// Factory method that initializes the plugin with [GoogleSignInPlatform]. static void registerWith(Registrar registrar) { @@ -72,7 +72,7 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { 'Check https://developers.google.com/identity/protocols/googlescopes ' 'for a list of valid OAuth 2.0 scopes.'); - await initialized; + await _isGapiInitialized; final auth2.GoogleAuth auth = auth2.init(auth2.ClientConfig( hosted_domain: hostedDomain, @@ -81,19 +81,25 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { client_id: appClientId, )); - // Subscribe to changes in the auth instance returned by init, - // and cache the _lastSeenUser as we get notified of new values. - final Completer initUserCompleter = - Completer(); - - auth.currentUser.listen(allowInterop((auth2.GoogleUser nextUser) { - if (!initUserCompleter.isCompleted) { - initUserCompleter.complete(nextUser); - } else { - _lastSeenUser = nextUser; - } + Completer isAuthInitialized = Completer(); + _isAuthInitialized = isAuthInitialized.future; + + auth.then(allowInterop((auth2.GoogleAuth initializedAuth) { + // onSuccess + + // TODO: https://github.com/flutter/flutter/issues/48528 + // This plugin doesn't notify the app of external changes to the + // state of the authentication, i.e: if you logout elsewhere... + + isAuthInitialized.complete(); + }), allowInterop((dynamic reason) { + // onError + throw (PlatformException( + code: 'google_sign_in', + message: reason.error, + details: reason.details, + )); })); - _lastSeenUser = initUserCompleter.future; return null; } @@ -102,7 +108,8 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { Future signInSilently() async { await initialized; - return gapiUserToPluginUserData(await _lastSeenUser); + return gapiUserToPluginUserData( + await auth2.getAuthInstance().currentUser.get()); } @override @@ -154,7 +161,6 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { Future clearAuthCache({String token}) async { await initialized; - _lastSeenUser = null; return auth2.getAuthInstance().disconnect(); } } From 0d4f9da2fd2e446f9151ca20b448d9ab3d06af2a Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2020 12:49:17 -0800 Subject: [PATCH 2/7] [google_sign_in_web] Bump version and update CHANGELOG.md --- packages/google_sign_in/google_sign_in_web/CHANGELOG.md | 5 +++++ packages/google_sign_in/google_sign_in_web/pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/google_sign_in/google_sign_in_web/CHANGELOG.md b/packages/google_sign_in/google_sign_in_web/CHANGELOG.md index a41ce367d9bd..73136410ada3 100644 --- a/packages/google_sign_in/google_sign_in_web/CHANGELOG.md +++ b/packages/google_sign_in/google_sign_in_web/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.8.3 + +* Fix initialization error that causes https://github.com/flutter/flutter/issues/48527 +* Throw a PlatformException when there's an initialization problem (like wrong server-side config). + ## 0.8.2+1 * Add a non-op Android implementation to avoid a flaky Gradle issue. diff --git a/packages/google_sign_in/google_sign_in_web/pubspec.yaml b/packages/google_sign_in/google_sign_in_web/pubspec.yaml index 2790ef8d4b59..a19453ad8ba7 100644 --- a/packages/google_sign_in/google_sign_in_web/pubspec.yaml +++ b/packages/google_sign_in/google_sign_in_web/pubspec.yaml @@ -2,7 +2,7 @@ name: google_sign_in_web description: Flutter plugin for Google Sign-In, a secure authentication system for signing in with a Google account on Android, iOS and Web. homepage: https://github.com/flutter/plugins/tree/master/packages/google_sign_in/google_sign_in_web -version: 0.8.2+1 +version: 0.8.3 flutter: plugin: From 14285fc43dd567b5e3854df258045a123ad75a4f Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2020 13:23:31 -0800 Subject: [PATCH 3/7] [google_sign_in_web] Test the new behavior. --- .../google_sign_in_web/lib/google_sign_in_web.dart | 2 +- .../google_sign_in_web/test/auth2_test.dart | 8 ++++---- .../google_sign_in_web/test/gapi_load_test.dart | 11 +++++++++-- .../test/gapi_mocks/gapi_mocks.dart | 1 - .../test/gapi_mocks/src/auth2_init.dart | 5 +++++ .../test/gapi_mocks/src/gapi_load.dart | 8 -------- 6 files changed, 19 insertions(+), 16 deletions(-) delete mode 100644 packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/gapi_load.dart diff --git a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart index 8b5e46c71ebe..59a77aafb90b 100644 --- a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart +++ b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart @@ -38,7 +38,7 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { } Future _isGapiInitialized; - Future _isAuthInitialized; + Future _isAuthInitialized = Completer().future; /// This is only exposed for testing. It shouldn't be accessed by users of the /// plugin as it could break at any point. diff --git a/packages/google_sign_in/google_sign_in_web/test/auth2_test.dart b/packages/google_sign_in/google_sign_in_web/test/auth2_test.dart index 6eae93a136c8..7a3a01227169 100644 --- a/packages/google_sign_in/google_sign_in_web/test/auth2_test.dart +++ b/packages/google_sign_in/google_sign_in_web/test/auth2_test.dart @@ -27,9 +27,8 @@ void main() { GoogleSignInPlugin plugin; - setUp(() async { + setUp(() { plugin = GoogleSignInPlugin(); - await plugin.initialized; }); test('Init requires clientId', () async { @@ -47,12 +46,13 @@ void main() { }); group('Successful .init, then', () { - setUp(() { - plugin.init( + setUp(() async { + await plugin.init( hostedDomain: 'foo', scopes: ['some', 'scope'], clientId: '1234', ); + await plugin.initialized; }); test('signInSilently', () async { diff --git a/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart b/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart index 815dcdcfc645..7e99a73ae1df 100644 --- a/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart +++ b/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart @@ -7,14 +7,15 @@ import 'dart:html' as html; import 'package:flutter_test/flutter_test.dart'; +import 'package:google_sign_in_platform_interface/google_sign_in_platform_interface.dart'; import 'package:google_sign_in_web/google_sign_in_web.dart'; import 'gapi_mocks/gapi_mocks.dart' as gapi_mocks; import 'utils.dart'; void main() { - gapiUrl = toBase64Url(gapi_mocks.gapiInitSuccess()); + gapiUrl = toBase64Url(gapi_mocks.auth2InitSuccess(GoogleSignInUserData())); - test('Plugin is initialized after GAPI fully loads', () async { + test('Plugin is initialized after GAPI fully loads and init is called', () async { expect( html.querySelector('script[src^="data:"]'), isNull, @@ -26,6 +27,12 @@ void main() { isNotNull, reason: 'Mock script should be injected', ); + expect( + plugin.initialized, + doesNotComplete, + reason: 'The plugin should only complete the future after calling .init', + ); + await plugin.init(hostedDomain: '', clientId: ''); await plugin.initialized; expect( plugin.initialized, diff --git a/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/gapi_mocks.dart b/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/gapi_mocks.dart index d36cd5edf63a..9a7d4f403f97 100644 --- a/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/gapi_mocks.dart +++ b/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/gapi_mocks.dart @@ -10,5 +10,4 @@ import 'src/gapi.dart'; import 'src/google_user.dart'; import 'src/test_iife.dart'; -part 'src/gapi_load.dart'; part 'src/auth2_init.dart'; diff --git a/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/auth2_init.dart b/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/auth2_init.dart index 5484dc5b61bc..1846033151c1 100644 --- a/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/auth2_init.dart +++ b/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/auth2_init.dart @@ -13,6 +13,11 @@ var mockUser = ${googleUser(userData)}; function GapiAuth2() {} GapiAuth2.prototype.init = function (initOptions) { return { + then: (onSuccess, onError) => { + window.setTimeout(() => { + onSuccess(window.gapi.auth2); + }, 30); + }, currentUser: { listen: (cb) => { window.setTimeout(() => { diff --git a/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/gapi_load.dart b/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/gapi_load.dart deleted file mode 100644 index f390500f6f4e..000000000000 --- a/packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/gapi_load.dart +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -part of gapi_mocks; - -// JS mock of a raw GAPI object. -String gapiInitSuccess() => testIife(gapi()); From 520f38bfd3a5916be0b215136df229b4bf4ad4bf Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2020 13:25:15 -0800 Subject: [PATCH 4/7] [google_sign_in_web] dartfmt -w . --- .../google_sign_in/google_sign_in_web/test/gapi_load_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart b/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart index 7e99a73ae1df..2ecf2e2cc4ef 100644 --- a/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart +++ b/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart @@ -15,7 +15,8 @@ import 'utils.dart'; void main() { gapiUrl = toBase64Url(gapi_mocks.auth2InitSuccess(GoogleSignInUserData())); - test('Plugin is initialized after GAPI fully loads and init is called', () async { + test('Plugin is initialized after GAPI fully loads and init is called', + () async { expect( html.querySelector('script[src^="data:"]'), isNull, From 95b46d05ff48624d980d7817ee3844dd65d9ffed Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2020 16:14:41 -0800 Subject: [PATCH 5/7] [google_sign_in_web] Throw StateError synchronously if .initialized is checked before calling .init() --- .../lib/google_sign_in_web.dart | 23 +++++++++++++++---- .../test/gapi_load_test.dart | 10 ++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart index 59a77aafb90b..56635a113e43 100644 --- a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart +++ b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart @@ -38,13 +38,25 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { } Future _isGapiInitialized; - Future _isAuthInitialized = Completer().future; + Future _isAuthInitialized; + bool _isInitCalled = false; + + // This method throws if init hasn't been called at some point in the past. + // It is used by the [initialized] getter to ensure that users can't await + // on a Future that will never resolve. + void _assertIsInitCalled() { + if (!_isInitCalled) { + throw StateError( + 'GoogleSignInPlugin::init() must be called before any other method in this plugin.'); + } + } - /// This is only exposed for testing. It shouldn't be accessed by users of the - /// plugin as it could break at any point. + /// A future that resolves when both GAPI and Auth2 have been correctly initialized. @visibleForTesting - Future get initialized => - Future.wait([_isGapiInitialized, _isAuthInitialized]); + Future get initialized { + _assertIsInitCalled(); + return Future.wait([_isGapiInitialized, _isAuthInitialized]); + } String _autoDetectedClientId; @@ -83,6 +95,7 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { Completer isAuthInitialized = Completer(); _isAuthInitialized = isAuthInitialized.future; + _isInitCalled = true; auth.then(allowInterop((auth2.GoogleAuth initializedAuth) { // onSuccess diff --git a/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart b/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart index 2ecf2e2cc4ef..6703beca2cad 100644 --- a/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart +++ b/packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart @@ -28,11 +28,11 @@ void main() { isNotNull, reason: 'Mock script should be injected', ); - expect( - plugin.initialized, - doesNotComplete, - reason: 'The plugin should only complete the future after calling .init', - ); + expect(() { + plugin.initialized; + }, throwsStateError, + reason: + 'The plugin should throw if checking for `initialized` before calling .init'); await plugin.init(hostedDomain: '', clientId: ''); await plugin.initialized; expect( From 74fb7f9ab250423dfec81ba8d4a992de2922d4f2 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2020 16:18:50 -0800 Subject: [PATCH 6/7] [google_sign_in_web] Update README with latest setup instructions. --- .../google_sign_in_web/CHANGELOG.md | 2 ++ .../google_sign_in_web/README.md | 30 +++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_web/CHANGELOG.md b/packages/google_sign_in/google_sign_in_web/CHANGELOG.md index 73136410ada3..62e3299f0eca 100644 --- a/packages/google_sign_in/google_sign_in_web/CHANGELOG.md +++ b/packages/google_sign_in/google_sign_in_web/CHANGELOG.md @@ -2,6 +2,8 @@ * Fix initialization error that causes https://github.com/flutter/flutter/issues/48527 * Throw a PlatformException when there's an initialization problem (like wrong server-side config). +* Throw a StateError when checking .initialized before calling .init() +* Update setup instructions in the README. ## 0.8.2+1 diff --git a/packages/google_sign_in/google_sign_in_web/README.md b/packages/google_sign_in/google_sign_in_web/README.md index 182b21018aa4..599a49372aa5 100644 --- a/packages/google_sign_in/google_sign_in_web/README.md +++ b/packages/google_sign_in/google_sign_in_web/README.md @@ -5,18 +5,16 @@ The web implementation of [google_sign_in](https://pub.dev/google_sign_in/google ## Usage ### Import the package -To use this plugin, follow the [plugin installation instructions](https://pub.dartlang.org/packages/google_sign_in#pub-pkg-tab-installing). -Remember that for web plugins you need to depend both on the "native" version that provides the Dart interface that you'll use in your app), and the "web" version, that provides the implementation of the plugin for the web platform. +This package is the endorsed implementation of `google_sign_in` for the web platform since version `4.1.0`, so it gets automatically added to your dependencies by depending on `google_sign_in: ^4.1.0`. -This is what the above means to your `pubspec.yaml`: +No modifications to your pubspec.yaml should be required in a recent enough version of Flutter (`>=1.12.13+hotfix.4`): -``` +```yaml ... dependencies: ... - google_sign_in: ^4.0.14 - google_sign_in_web: ^0.8.0 + google_sign_in: ^4.1.0 ... ... ``` @@ -28,8 +26,8 @@ First, go through the instructions [here](https://developers.google.com/identity On your `web/index.html` file, add the following `meta` tag, somewhere in the `head` of the document: -``` - +```html + ``` Read the rest of the instructions if you need to add extra APIs (like Google People API). @@ -74,7 +72,21 @@ Find the example wiring in the [Google sign-in example application](https://gith See the [google_sign_in.dart](https://github.com/flutter/plugins/blob/master/packages/google_sign_in/google_sign_in/lib/google_sign_in.dart) for more API details. +## Contributions and Testing + +Tests are a crucial to contributions to this package. All new contributions should be reasonably tested. + +In order to run tests in this package, do: + +``` +flutter test --platform chrome -j1 +``` + +Contributions to this package are welcome. Read the [Contributing to Flutter Plugins](https://github.com/flutter/plugins/blob/master/CONTRIBUTING.md) guide to get started. + ## Issues and feedback Please file [issues](https://github.com/flutter/flutter/issues/new) -to send feedback or report a bug. Thank you! +to send feedback or report a bug. + +**Thank you!** From f9c0ff33e2fa6058478002c1f4abed36410492ed Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 9 Jan 2020 16:41:34 -0800 Subject: [PATCH 7/7] [google_sign_in_web] Removed unnecessary parentheses. --- .../google_sign_in_web/lib/google_sign_in_web.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart index 56635a113e43..4004d47d5551 100644 --- a/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart +++ b/packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart @@ -107,11 +107,11 @@ class GoogleSignInPlugin extends GoogleSignInPlatform { isAuthInitialized.complete(); }), allowInterop((dynamic reason) { // onError - throw (PlatformException( + throw PlatformException( code: 'google_sign_in', message: reason.error, details: reason.details, - )); + ); })); return null;