Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit dc284c4

Browse files
authored
[google_sign_in_web] Fix race condition on init. (#2455)
* [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. * Throw StateError synchronously if .initialized is checked before calling .init() * 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. * Update README with latest setup instructions.
1 parent de56da5 commit dc284c4

File tree

9 files changed

+88
-46
lines changed

9 files changed

+88
-46
lines changed

packages/google_sign_in/google_sign_in_web/CHANGELOG.md

+7
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
## 0.8.3
2+
3+
* Fix initialization error that causes https://github.com/flutter/flutter/issues/48527
4+
* Throw a PlatformException when there's an initialization problem (like wrong server-side config).
5+
* Throw a StateError when checking .initialized before calling .init()
6+
* Update setup instructions in the README.
7+
18
## 0.8.2+1
29

310
* Add a non-op Android implementation to avoid a flaky Gradle issue.

packages/google_sign_in/google_sign_in_web/README.md

+21-9
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,16 @@ The web implementation of [google_sign_in](https://pub.dev/google_sign_in/google
55
## Usage
66

77
### Import the package
8-
To use this plugin, follow the [plugin installation instructions](https://pub.dartlang.org/packages/google_sign_in#pub-pkg-tab-installing).
98

10-
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.
9+
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`.
1110

12-
This is what the above means to your `pubspec.yaml`:
11+
No modifications to your pubspec.yaml should be required in a recent enough version of Flutter (`>=1.12.13+hotfix.4`):
1312

14-
```
13+
```yaml
1514
...
1615
dependencies:
1716
...
18-
google_sign_in: ^4.0.14
19-
google_sign_in_web: ^0.8.0
17+
google_sign_in: ^4.1.0
2018
...
2119
...
2220
```
@@ -28,8 +26,8 @@ First, go through the instructions [here](https://developers.google.com/identity
2826
On your `web/index.html` file, add the following `meta` tag, somewhere in the
2927
`head` of the document:
3028

31-
```
32-
<meta name="google-signin-client_id" content="YOUR_GOOGLE_SIGN_IN_OAUTH_CLIENT_ID.apps.googleusercontent.com">
29+
```html
30+
<meta name="google-signin-client_id" content="YOUR_GOOGLE_SIGN_IN_OAUTH_CLIENT_ID.apps.googleusercontent.com">
3331
```
3432

3533
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
7472

7573
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.
7674

75+
## Contributions and Testing
76+
77+
Tests are a crucial to contributions to this package. All new contributions should be reasonably tested.
78+
79+
In order to run tests in this package, do:
80+
81+
```
82+
flutter test --platform chrome -j1
83+
```
84+
85+
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.
86+
7787
## Issues and feedback
7888

7989
Please file [issues](https://github.com/flutter/flutter/issues/new)
80-
to send feedback or report a bug. Thank you!
90+
to send feedback or report a bug.
91+
92+
**Thank you!**

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

+40-21
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@
55
import 'dart:async';
66
import 'dart:html' as html;
77

8+
import 'package:flutter/services.dart';
89
import 'package:google_sign_in_platform_interface/google_sign_in_platform_interface.dart';
910
import 'package:flutter_web_plugins/flutter_web_plugins.dart';
1011
import 'package:js/js.dart';
1112
import 'package:meta/meta.dart';
1213

1314
import 'src/generated/gapiauth2.dart' as auth2;
14-
// TODO: Remove once this lands https://github.com/dart-lang/language/issues/671
15-
import 'src/generated/gapiauth2.dart' show GoogleAuthExtensions;
1615
import 'src/load_gapi.dart' as gapi;
1716
import 'src/utils.dart' show gapiUserToPluginUserData;
1817

@@ -39,14 +38,27 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
3938
}
4039

4140
Future<void> _isGapiInitialized;
41+
Future<void> _isAuthInitialized;
42+
bool _isInitCalled = false;
43+
44+
// This method throws if init hasn't been called at some point in the past.
45+
// It is used by the [initialized] getter to ensure that users can't await
46+
// on a Future that will never resolve.
47+
void _assertIsInitCalled() {
48+
if (!_isInitCalled) {
49+
throw StateError(
50+
'GoogleSignInPlugin::init() must be called before any other method in this plugin.');
51+
}
52+
}
4253

43-
/// This is only exposed for testing. It shouldn't be accessed by users of the
44-
/// plugin as it could break at any point.
54+
/// A future that resolves when both GAPI and Auth2 have been correctly initialized.
4555
@visibleForTesting
46-
Future<void> get initialized => _isGapiInitialized;
56+
Future<void> get initialized {
57+
_assertIsInitCalled();
58+
return Future.wait([_isGapiInitialized, _isAuthInitialized]);
59+
}
4760

4861
String _autoDetectedClientId;
49-
FutureOr<auth2.GoogleUser> _lastSeenUser;
5062

5163
/// Factory method that initializes the plugin with [GoogleSignInPlatform].
5264
static void registerWith(Registrar registrar) {
@@ -72,7 +84,7 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
7284
'Check https://developers.google.com/identity/protocols/googlescopes '
7385
'for a list of valid OAuth 2.0 scopes.');
7486

75-
await initialized;
87+
await _isGapiInitialized;
7688

7789
final auth2.GoogleAuth auth = auth2.init(auth2.ClientConfig(
7890
hosted_domain: hostedDomain,
@@ -81,19 +93,26 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
8193
client_id: appClientId,
8294
));
8395

84-
// Subscribe to changes in the auth instance returned by init,
85-
// and cache the _lastSeenUser as we get notified of new values.
86-
final Completer<auth2.GoogleUser> initUserCompleter =
87-
Completer<auth2.GoogleUser>();
88-
89-
auth.currentUser.listen(allowInterop((auth2.GoogleUser nextUser) {
90-
if (!initUserCompleter.isCompleted) {
91-
initUserCompleter.complete(nextUser);
92-
} else {
93-
_lastSeenUser = nextUser;
94-
}
96+
Completer<void> isAuthInitialized = Completer<void>();
97+
_isAuthInitialized = isAuthInitialized.future;
98+
_isInitCalled = true;
99+
100+
auth.then(allowInterop((auth2.GoogleAuth initializedAuth) {
101+
// onSuccess
102+
103+
// TODO: https://github.com/flutter/flutter/issues/48528
104+
// This plugin doesn't notify the app of external changes to the
105+
// state of the authentication, i.e: if you logout elsewhere...
106+
107+
isAuthInitialized.complete();
108+
}), allowInterop((dynamic reason) {
109+
// onError
110+
throw PlatformException(
111+
code: 'google_sign_in',
112+
message: reason.error,
113+
details: reason.details,
114+
);
95115
}));
96-
_lastSeenUser = initUserCompleter.future;
97116

98117
return null;
99118
}
@@ -102,7 +121,8 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
102121
Future<GoogleSignInUserData> signInSilently() async {
103122
await initialized;
104123

105-
return gapiUserToPluginUserData(await _lastSeenUser);
124+
return gapiUserToPluginUserData(
125+
await auth2.getAuthInstance().currentUser.get());
106126
}
107127

108128
@override
@@ -154,7 +174,6 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
154174
Future<void> clearAuthCache({String token}) async {
155175
await initialized;
156176

157-
_lastSeenUser = null;
158177
return auth2.getAuthInstance().disconnect();
159178
}
160179
}

packages/google_sign_in/google_sign_in_web/pubspec.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: google_sign_in_web
22
description: Flutter plugin for Google Sign-In, a secure authentication system
33
for signing in with a Google account on Android, iOS and Web.
44
homepage: https://github.com/flutter/plugins/tree/master/packages/google_sign_in/google_sign_in_web
5-
version: 0.8.2+1
5+
version: 0.8.3
66

77
flutter:
88
plugin:

packages/google_sign_in/google_sign_in_web/test/auth2_test.dart

+4-4
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ void main() {
2727

2828
GoogleSignInPlugin plugin;
2929

30-
setUp(() async {
30+
setUp(() {
3131
plugin = GoogleSignInPlugin();
32-
await plugin.initialized;
3332
});
3433

3534
test('Init requires clientId', () async {
@@ -47,12 +46,13 @@ void main() {
4746
});
4847

4948
group('Successful .init, then', () {
50-
setUp(() {
51-
plugin.init(
49+
setUp(() async {
50+
await plugin.init(
5251
hostedDomain: 'foo',
5352
scopes: <String>['some', 'scope'],
5453
clientId: '1234',
5554
);
55+
await plugin.initialized;
5656
});
5757

5858
test('signInSilently', () async {

packages/google_sign_in/google_sign_in_web/test/gapi_load_test.dart

+10-2
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,16 @@
77
import 'dart:html' as html;
88

99
import 'package:flutter_test/flutter_test.dart';
10+
import 'package:google_sign_in_platform_interface/google_sign_in_platform_interface.dart';
1011
import 'package:google_sign_in_web/google_sign_in_web.dart';
1112
import 'gapi_mocks/gapi_mocks.dart' as gapi_mocks;
1213
import 'utils.dart';
1314

1415
void main() {
15-
gapiUrl = toBase64Url(gapi_mocks.gapiInitSuccess());
16+
gapiUrl = toBase64Url(gapi_mocks.auth2InitSuccess(GoogleSignInUserData()));
1617

17-
test('Plugin is initialized after GAPI fully loads', () async {
18+
test('Plugin is initialized after GAPI fully loads and init is called',
19+
() async {
1820
expect(
1921
html.querySelector('script[src^="data:"]'),
2022
isNull,
@@ -26,6 +28,12 @@ void main() {
2628
isNotNull,
2729
reason: 'Mock script should be injected',
2830
);
31+
expect(() {
32+
plugin.initialized;
33+
}, throwsStateError,
34+
reason:
35+
'The plugin should throw if checking for `initialized` before calling .init');
36+
await plugin.init(hostedDomain: '', clientId: '');
2937
await plugin.initialized;
3038
expect(
3139
plugin.initialized,

packages/google_sign_in/google_sign_in_web/test/gapi_mocks/gapi_mocks.dart

-1
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,4 @@ import 'src/gapi.dart';
1010
import 'src/google_user.dart';
1111
import 'src/test_iife.dart';
1212

13-
part 'src/gapi_load.dart';
1413
part 'src/auth2_init.dart';

packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/auth2_init.dart

+5
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ var mockUser = ${googleUser(userData)};
1313
function GapiAuth2() {}
1414
GapiAuth2.prototype.init = function (initOptions) {
1515
return {
16+
then: (onSuccess, onError) => {
17+
window.setTimeout(() => {
18+
onSuccess(window.gapi.auth2);
19+
}, 30);
20+
},
1621
currentUser: {
1722
listen: (cb) => {
1823
window.setTimeout(() => {

packages/google_sign_in/google_sign_in_web/test/gapi_mocks/src/gapi_load.dart

-8
This file was deleted.

0 commit comments

Comments
 (0)