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

[google_sign_in_web] Fix race condition on init. #2455

Merged
merged 7 commits into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/google_sign_in/google_sign_in_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 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).
* Throw a StateError when checking .initialized before calling .init()
* Update setup instructions in the README.

## 0.8.2+1

* Add a non-op Android implementation to avoid a flaky Gradle issue.
Expand Down
30 changes: 21 additions & 9 deletions packages/google_sign_in/google_sign_in_web/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
...
...
```
Expand All @@ -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:

```
<meta name="google-signin-client_id" content="YOUR_GOOGLE_SIGN_IN_OAUTH_CLIENT_ID.apps.googleusercontent.com">
```html
<meta name="google-signin-client_id" content="YOUR_GOOGLE_SIGN_IN_OAUTH_CLIENT_ID.apps.googleusercontent.com">
```

Read the rest of the instructions if you need to add extra APIs (like Google People API).
Expand Down Expand Up @@ -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!**
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -39,14 +38,27 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
}

Future<void> _isGapiInitialized;
Future<void> _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<void> get initialized => _isGapiInitialized;
Future<void> get initialized {
_assertIsInitCalled();
return Future.wait([_isGapiInitialized, _isAuthInitialized]);
}

String _autoDetectedClientId;
FutureOr<auth2.GoogleUser> _lastSeenUser;

/// Factory method that initializes the plugin with [GoogleSignInPlatform].
static void registerWith(Registrar registrar) {
Expand All @@ -72,7 +84,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,
Expand All @@ -81,19 +93,26 @@ 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<auth2.GoogleUser> initUserCompleter =
Completer<auth2.GoogleUser>();

auth.currentUser.listen(allowInterop((auth2.GoogleUser nextUser) {
if (!initUserCompleter.isCompleted) {
initUserCompleter.complete(nextUser);
} else {
_lastSeenUser = nextUser;
}
Completer<void> isAuthInitialized = Completer<void>();
_isAuthInitialized = isAuthInitialized.future;
_isInitCalled = true;

auth.then(allowInterop((auth2.GoogleAuth initializedAuth) {
// onSuccess
ditman marked this conversation as resolved.
Show resolved Hide resolved

// 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;
}
Expand All @@ -102,7 +121,8 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
Future<GoogleSignInUserData> signInSilently() async {
await initialized;

return gapiUserToPluginUserData(await _lastSeenUser);
return gapiUserToPluginUserData(
await auth2.getAuthInstance().currentUser.get());
}

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

_lastSeenUser = null;
return auth2.getAuthInstance().disconnect();
}
}
2 changes: 1 addition & 1 deletion packages/google_sign_in/google_sign_in_web/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ void main() {

GoogleSignInPlugin plugin;

setUp(() async {
setUp(() {
plugin = GoogleSignInPlugin();
await plugin.initialized;
});

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

group('Successful .init, then', () {
setUp(() {
plugin.init(
setUp(() async {
await plugin.init(
hostedDomain: 'foo',
scopes: <String>['some', 'scope'],
clientId: '1234',
);
await plugin.initialized;
});

test('signInSilently', () async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@
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,
Expand All @@ -26,6 +28,12 @@ void main() {
isNotNull,
reason: 'Mock script should be injected',
);
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(
plugin.initialized,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down

This file was deleted.