diff --git a/app_flutter/lib/build_dashboard.dart b/app_flutter/lib/build_dashboard.dart index e797f8d6cb..fb2ff12d36 100644 --- a/app_flutter/lib/build_dashboard.dart +++ b/app_flutter/lib/build_dashboard.dart @@ -5,7 +5,7 @@ import 'package:flutter/material.dart'; import 'package:provider/provider.dart'; -import 'service/google_authentication.dart'; +import 'sign_in_button.dart'; import 'state/flutter_build.dart'; import 'status_grid.dart'; @@ -54,7 +54,7 @@ class BuildDashboard extends StatelessWidget { ? theme.primaryColor : theme.errorColor, actions: [ - UserAvatar(buildState: buildState), + SignInButton(authService: buildState.authService), ], ), body: Column( @@ -66,28 +66,3 @@ class BuildDashboard extends StatelessWidget { ); } } - -/// Widget for displaying sign in information for the current user. -/// -/// If logged in, it will display the user's avatar. Otherwise, it will show -/// a button for sign in. -class UserAvatar extends StatelessWidget { - const UserAvatar({@required this.buildState, Key key}) : super(key: key); - - final FlutterBuildState buildState; - - @override - Widget build(BuildContext context) { - final GoogleSignInService authService = buildState.authService; - - if (authService.isAuthenticated) { - /// Size needs to be specified - return Image.network(authService.avatarUrl + '=s100'); - } - - return FlatButton( - child: const Text('Sign in'), - onPressed: () => buildState.signIn(), - ); - } -} diff --git a/app_flutter/lib/service/appengine_cocoon.dart b/app_flutter/lib/service/appengine_cocoon.dart index fe356df117..788a9fc55e 100644 --- a/app_flutter/lib/service/appengine_cocoon.dart +++ b/app_flutter/lib/service/appengine_cocoon.dart @@ -84,14 +84,14 @@ class AppEngineCocoonService implements CocoonService { } @override - Future rerunTask(Task task, String accessToken) async { - assert(accessToken != null); + Future rerunTask(Task task, String idToken) async { + assert(idToken != null); final String postResetTaskUrl = _apiEndpoint('/api/reset-devicelab-task'); /// This endpoint only returns a status code. final http.Response response = await _client.post(postResetTaskUrl, headers: { - 'X-Flutter-AccessToken': accessToken, + 'X-Flutter-IdToken': idToken, }, body: jsonEncode({ 'Key': task.key.child.name, diff --git a/app_flutter/lib/service/google_authentication.dart b/app_flutter/lib/service/google_authentication.dart index f77c6884bf..0454d069f5 100644 --- a/app_flutter/lib/service/google_authentication.dart +++ b/app_flutter/lib/service/google_authentication.dart @@ -2,18 +2,27 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:google_sign_in_all/google_sign_in_all.dart'; +import 'package:flutter/rendering.dart'; +import 'package:google_sign_in/google_sign_in.dart'; /// Service class for interacting with Google Sign In authentication for Cocoon backend. class GoogleSignInService { /// Creates a new [GoogleSignIn]. - GoogleSignInService({GoogleSignIn googleSignIn}) + GoogleSignInService({GoogleSignIn googleSignIn, this.notifyListeners}) : _googleSignIn = googleSignIn ?? - setupGoogleSignIn( + GoogleSignIn( scopes: _googleScopes, - webClientId: - '308150028417-vlj9mqlm3gk1d03fb0efif1fu5nagdtt.apps.googleusercontent.com', - ); + ) { + _googleSignIn.onCurrentUserChanged + .listen((GoogleSignInAccount accountValue) { + user = accountValue; + notifyListeners(); + }); + _googleSignIn.signInSilently(); + } + + /// A callback for notifying listeners there has been an update. + final VoidCallback notifyListeners; /// A list of Google API OAuth Scopes this project needs access to. /// @@ -24,30 +33,29 @@ class GoogleSignInService { static const List _googleScopes = [ 'https://www.googleapis.com/auth/userinfo.email', 'https://www.googleapis.com/auth/userinfo.profile', + 'openid', ]; - // TODO(chillers): Switch to official Flutter plugin when it supports web. final GoogleSignIn _googleSignIn; - AuthCredentials _credentials; - - GoogleAccount _user; - /// Whether or not the application has been signed in to. - bool get isAuthenticated => _credentials?.accessToken != null; + Future get isAuthenticated => _googleSignIn.isSignedIn(); - /// The profile photo url of the current user signed in. - String get avatarUrl => _user?.photoUrl; - - /// The email of the current user signed in. - String get email => _user?.email; + /// The Google Account for the signed in user, null if no user is signed in. + /// + /// Read only object with only access to clear client auth tokens. + GoogleSignInAccount user; /// Authentication token to be sent to Cocoon Backend to verify API calls. - String get accessToken => _credentials?.accessToken; + Future get idToken => user?.authentication + ?.then((GoogleSignInAuthentication key) => key.idToken); /// Initiate the Google Sign In process. Future signIn() async { - _credentials = await _googleSignIn.signIn(); - _user = await _googleSignIn.getCurrentUser(); + user = await _googleSignIn.signIn(); + } + + Future signOut() async { + await _googleSignIn.signOut(); } } diff --git a/app_flutter/lib/sign_in_button.dart b/app_flutter/lib/sign_in_button.dart new file mode 100644 index 0000000000..1f718352ea --- /dev/null +++ b/app_flutter/lib/sign_in_button.dart @@ -0,0 +1,53 @@ +// Copyright (c) 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. + +import 'package:flutter/material.dart'; +import 'package:flutter/widgets.dart'; + +import 'service/google_authentication.dart'; + +/// Widget for displaying sign in information for the current user. +/// +/// If logged in, it will display the user's avatar. Clicking it opens a dropdown for logging out. +/// Otherwise, a sign in button will show. +class SignInButton extends StatelessWidget { + const SignInButton({@required this.authService, Key key}) : super(key: key); + + final GoogleSignInService authService; + + @override + Widget build(BuildContext context) { + return FutureBuilder( + future: authService.isAuthenticated, + builder: (_, AsyncSnapshot isAuthenticated) { + /// On sign out, there's a second where the user is null before isAuthenticated catches up. + if (isAuthenticated.data == true && authService.user != null) { + return PopupMenuButton( + // TODO(chillers): Switch to use avatar widget provided by google_sign_in plugin + child: Image.network(authService.user?.photoUrl), + offset: const Offset(0, 50), + itemBuilder: (BuildContext context) => >[ + const PopupMenuItem( + value: 'logout', + child: Text('Log out'), + ), + ], + onSelected: (String value) { + if (value == 'logout') { + authService.signOut(); + } + }, + ); + } + return FlatButton( + child: const Text( + 'Sign in', + style: TextStyle(color: Colors.white), + ), + onPressed: () => authService.signIn(), + ); + }, + ); + } +} diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 97e56219b8..384aa64d76 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -17,16 +17,18 @@ class FlutterBuildState extends ChangeNotifier { /// /// If [CocoonService] is not specified, a new [CocoonService] instance is created. FlutterBuildState({ - CocoonService cocoonService, - GoogleSignInService authService, - }) : authService = authService ?? GoogleSignInService(), - _cocoonService = cocoonService ?? CocoonService(); + CocoonService cocoonServiceValue, + GoogleSignInService authServiceValue, + }) : _cocoonService = cocoonServiceValue ?? CocoonService() { + authService = authServiceValue ?? + GoogleSignInService(notifyListeners: notifyListeners); + } /// Cocoon backend service that retrieves the data needed for this state. final CocoonService _cocoonService; /// Authentication service for managing Google Sign In. - final GoogleSignInService authService; + GoogleSignInService authService; /// How often to query the Cocoon backend for the current build state. @visibleForTesting @@ -88,13 +90,11 @@ class FlutterBuildState extends ChangeNotifier { notifyListeners(); } - Future signIn() async { - await authService.signIn(); - notifyListeners(); - } + Future signIn() => authService.signIn(); + Future signOut() => authService.signOut(); - Future rerunTask(Task task) { - return _cocoonService.rerunTask(task, authService.accessToken); + Future rerunTask(Task task) async { + return _cocoonService.rerunTask(task, await authService.idToken); } @override diff --git a/app_flutter/lib/task_box.dart b/app_flutter/lib/task_box.dart index 4819e9c2f0..6bdd5830bc 100644 --- a/app_flutter/lib/task_box.dart +++ b/app_flutter/lib/task_box.dart @@ -341,7 +341,7 @@ class TaskOverlayContents extends StatelessWidget { // Only send access token for devicelab tasks since they require authentication final Map headers = isDevicelab(task) ? { - 'X-Flutter-AccessToken': buildState.authService.accessToken, + 'X-Flutter-IdToken': await buildState.authService.idToken, } : null; launch(logUrl(task), headers: headers); diff --git a/app_flutter/pubspec.yaml b/app_flutter/pubspec.yaml index fe7186e6f8..cfe3abbaa4 100644 --- a/app_flutter/pubspec.yaml +++ b/app_flutter/pubspec.yaml @@ -18,7 +18,7 @@ description: A new Flutter project. version: 1.0.0+1 environment: - sdk: ">=2.3.0 <3.0.0" + sdk: ">=2.6.0 <3.0.0" dependencies: flutter: @@ -31,7 +31,14 @@ dependencies: cocoon_service: path: ../app_dart flutter_progress_button: ^1.0.0 - google_sign_in_all: ^0.0.3 + google_sign_in: ^4.0.14 + google_sign_in_web: + git: + # TODO(chillers): https://github.com/flutter/plugins/pull/2280 and the Google Sign In plugin + # publishes the update switch to the official version. + url: git://github.com/ditman/plugins.git + ref: federated_google_sign_in_web + path: packages/google_sign_in/google_sign_in_web provider: ^3.0.0 url_launcher: 5.2.4 url_launcher_web: diff --git a/app_flutter/test/build_dashboard_test.dart b/app_flutter/test/build_dashboard_test.dart index 6e3ae9c72b..f3afc1114c 100644 --- a/app_flutter/test/build_dashboard_test.dart +++ b/app_flutter/test/build_dashboard_test.dart @@ -4,72 +4,22 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:mockito/mockito.dart'; -import 'package:test/test.dart' as test; +import 'package:provider/provider.dart'; import 'package:app_flutter/build_dashboard.dart'; -import 'package:app_flutter/service/google_authentication.dart'; -import 'package:app_flutter/service/fake_cocoon.dart'; +import 'package:app_flutter/sign_in_button.dart'; import 'package:app_flutter/state/flutter_build.dart'; void main() { - group('UserAvatar', () { - GoogleSignInService authService; + testWidgets('shows sign in button', (WidgetTester tester) async { + final FlutterBuildState buildState = FlutterBuildState(); - setUp(() { - authService = MockGoogleSignInService(); - }); + await tester.pumpWidget(MaterialApp( + home: ChangeNotifierProvider( + builder: (_) => buildState, + child: BuildDashboard(), + ))); - testWidgets('shows sign in button when not signed in', - (WidgetTester tester) async { - when(authService.isAuthenticated).thenReturn(false); - final FlutterBuildState buildState = FlutterBuildState( - authService: authService, cocoonService: FakeCocoonService()); - await tester.pumpWidget(MaterialApp( - home: UserAvatar( - buildState: buildState, - ), - )); - - expect(find.text('Sign in'), findsOneWidget); - }); - - testWidgets('sign in button activates google sign in when pressed', - (WidgetTester tester) async { - when(authService.isAuthenticated).thenReturn(false); - final FlutterBuildState buildState = FlutterBuildState( - authService: authService, cocoonService: FakeCocoonService()); - await tester.pumpWidget(MaterialApp( - home: UserAvatar( - buildState: buildState, - ), - )); - - verifyNever(authService.signIn()); - - await tester.tap(find.byType(UserAvatar)); - - verify(authService.signIn()).called(1); - }); - - testWidgets('shows user avatar when signed in', - (WidgetTester tester) async { - when(authService.isAuthenticated).thenReturn(true); - when(authService.avatarUrl).thenReturn('https://flutter.dev'); - final FlutterBuildState buildState = FlutterBuildState( - authService: authService, cocoonService: FakeCocoonService()); - await tester.pumpWidget(MaterialApp( - home: UserAvatar( - buildState: buildState, - ), - )); - - expect(tester.takeException(), - const test.TypeMatcher()); - expect(find.byType(Image), findsOneWidget); - }); + expect(find.byType(SignInButton), findsOneWidget); }); } - -/// Mock [GoogleSignInService] for testing interactions. -class MockGoogleSignInService extends Mock implements GoogleSignInService {} diff --git a/app_flutter/test/service/google_authentication_test.dart b/app_flutter/test/service/google_authentication_test.dart index 529b500508..5961b2a155 100644 --- a/app_flutter/test/service/google_authentication_test.dart +++ b/app_flutter/test/service/google_authentication_test.dart @@ -2,28 +2,35 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:google_sign_in_all/google_sign_in_all.dart'; +import 'package:google_sign_in/google_sign_in.dart'; import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; import 'package:app_flutter/service/google_authentication.dart'; +import '../utils/fake_google_account.dart'; + void main() { group('GoogleSignInService not signed in', () { GoogleSignInService authService; + GoogleSignIn mockSignIn; setUp(() { - authService = GoogleSignInService(googleSignIn: MockGoogleSignIn()); + mockSignIn = MockGoogleSignIn(); + when(mockSignIn.onCurrentUserChanged) + .thenAnswer((_) => const Stream.empty()); + when(mockSignIn.isSignedIn()) + .thenAnswer((_) => Future.value(false)); + authService = GoogleSignInService(googleSignIn: mockSignIn); }); - test('not authenticated', () { - expect(authService.isAuthenticated, false); + test('not authenticated', () async { + expect(await authService.isAuthenticated, false); }); test('no user information', () { - expect(authService.avatarUrl, null); - expect(authService.email, null); - expect(authService.accessToken, null); + expect(authService.user, null); + expect(authService.idToken, null); }); }); @@ -31,14 +38,16 @@ void main() { GoogleSignInService authService; GoogleSignIn mockSignIn; + final GoogleSignInAccount testAccount = FakeGoogleSignInAccount(); + setUp(() { mockSignIn = MockGoogleSignIn(); - final AuthCredentials fakeCredentials = FakeAuthCredentials(); when(mockSignIn.signIn()) - .thenAnswer((_) => Future.value(fakeCredentials)); - when(mockSignIn.getCurrentUser()).thenAnswer((_) => - Future.value(GoogleAccount( - email: 'fake@fake.com', photoUrl: 'fake://fake.png'))); + .thenAnswer((_) => Future.value(testAccount)); + when(mockSignIn.currentUser).thenReturn(testAccount); + when(mockSignIn.isSignedIn()).thenAnswer((_) => Future.value(true)); + when(mockSignIn.onCurrentUserChanged) + .thenAnswer((_) => const Stream.empty()); authService = GoogleSignInService(googleSignIn: mockSignIn); }); @@ -46,29 +55,40 @@ void main() { test('is authenticated after successful sign in', () async { await authService.signIn(); - expect(authService.isAuthenticated, true); + expect(await authService.isAuthenticated, true); + expect(authService.user, testAccount); }); test('there is user information after successful sign in', () async { await authService.signIn(); - expect(authService.email, 'fake@fake.com'); - expect(authService.avatarUrl, 'fake://fake.png'); - expect(authService.accessToken, 'fake'); + expect(authService.user.displayName, 'Dr. Test'); + expect(authService.user.email, 'test@flutter.dev'); + expect(authService.user.id, 'test123'); + expect(authService.user.photoUrl, + 'https://lh3.googleusercontent.com/-ukEAtRyRhw8/AAAAAAAAAAI/AAAAAAAAAAA/ACHi3rfhID9XACtdb9q_xK43VSXQvBV11Q.CMID'); + }); + + test('id token available with logged in user', () async { + final GoogleSignInAccount testAccountWithAuthentication = + FakeGoogleSignInAccount() + ..authentication = Future.value( + FakeGoogleSignInAuthentication()); + authService.user = testAccountWithAuthentication; + + expect(await authService.idToken, 'id123'); }); test('is not authenticated after failure in sign in', () async { + when(mockSignIn.signInSilently()) + .thenAnswer((_) => Future.value(null)); when(mockSignIn.signIn()) - .thenAnswer((_) => Future.value(null)); - when(mockSignIn.getCurrentUser()) - .thenAnswer((_) => Future.value(null)); + .thenAnswer((_) => Future.value(null)); await authService.signIn(); - expect(authService.isAuthenticated, false); - expect(authService.email, null); - expect(authService.avatarUrl, null); - expect(authService.accessToken, null); + expect(authService.user, null); + expect(authService.idToken, null); }); }); } @@ -76,11 +96,10 @@ void main() { /// Mock [GoogleSignIn] for testing interactions. class MockGoogleSignIn extends Mock implements GoogleSignIn {} -/// Fake [AuthCredentials] for [MockGoogleSignIn]. -class FakeAuthCredentials implements AuthCredentials { +class FakeGoogleSignInAuthentication implements GoogleSignInAuthentication { @override - final String accessToken = 'fake'; + String get accessToken => 'access123'; @override - final String idToken = 'faker'; + String get idToken => 'id123'; } diff --git a/app_flutter/test/sign_in_button_test.dart b/app_flutter/test/sign_in_button_test.dart new file mode 100644 index 0000000000..57f75367ba --- /dev/null +++ b/app_flutter/test/sign_in_button_test.dart @@ -0,0 +1,127 @@ +// Copyright (c) 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. + +import 'package:flutter/material.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:google_sign_in/google_sign_in.dart'; +import 'package:mockito/mockito.dart'; +import 'package:test/test.dart' as test; + +import 'package:app_flutter/service/google_authentication.dart'; +import 'package:app_flutter/sign_in_button.dart'; +import 'package:app_flutter/state/flutter_build.dart'; + +import 'utils/fake_google_account.dart'; + +void main() { + GoogleSignInService mockAuthService; + + setUp(() { + mockAuthService = MockGoogleSignInService(); + }); + + tearDown(() { + clearInteractions(mockAuthService); + }); + + testWidgets('shows sign in when not authenticated', + (WidgetTester tester) async { + when(mockAuthService.isAuthenticated) + .thenAnswer((_) async => Future.value(false)); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: SignInButton( + authService: mockAuthService, + ), + ), + ); + await tester.pump(); + + expect(find.byType(GoogleUserCircleAvatar), findsNothing); + expect(find.text('Sign in'), findsOneWidget); + }); + + testWidgets('calls sign in on tap when not authenticated', + (WidgetTester tester) async { + when(mockAuthService.isAuthenticated) + .thenAnswer((_) async => Future.value(false)); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: SignInButton( + authService: mockAuthService, + ), + ), + ); + await tester.pump(); + + verifyNever(mockAuthService.signIn()); + + await tester.tap(find.text('Sign in')); + await tester.pump(); + + verify(mockAuthService.signIn()).called(1); + }); + + testWidgets('shows avatar when authenticated', (WidgetTester tester) async { + when(mockAuthService.isAuthenticated) + .thenAnswer((_) async => Future.value(true)); + + final GoogleSignInAccount user = FakeGoogleSignInAccount(); + when(mockAuthService.user).thenReturn(user); + + await tester.pumpWidget( + MaterialApp( + home: AppBar( + leading: SignInButton( + authService: mockAuthService, + ), + ), + ), + ); + await tester.pump(); + expect(tester.takeException(), + const test.TypeMatcher()); + + expect(find.text('Sign in'), findsNothing); + expect(find.byType(Image), findsOneWidget); + }); + + testWidgets('calls sign out on tap when authenticated', + (WidgetTester tester) async { + when(mockAuthService.isAuthenticated) + .thenAnswer((_) async => Future.value(true)); + + final GoogleSignInAccount user = FakeGoogleSignInAccount(); + when(mockAuthService.user).thenReturn(user); + + await tester.pumpWidget( + MaterialApp( + home: AppBar( + leading: SignInButton( + authService: mockAuthService, + ), + ), + ), + ); + await tester.pump(); + + await tester.tap(find.byType(Image)); + await tester.pumpAndSettle(); + + verifyNever(mockAuthService.signOut()); + + await tester.tap(find.text('Log out')); + + verify(mockAuthService.signOut()).called(1); + }); +} + +class MockFlutterBuildState extends Mock implements FlutterBuildState {} + +class MockGoogleSignInService extends Mock implements GoogleSignInService {} diff --git a/app_flutter/test/state/flutter_build_test.dart b/app_flutter/test/state/flutter_build_test.dart index 4a806b603d..1d963d4aea 100644 --- a/app_flutter/test/state/flutter_build_test.dart +++ b/app_flutter/test/state/flutter_build_test.dart @@ -20,7 +20,7 @@ void main() { setUp(() { mockService = MockCocoonService(); - buildState = FlutterBuildState(cocoonService: mockService); + buildState = FlutterBuildState(cocoonServiceValue: mockService); when(mockService.fetchCommitStatuses()).thenAnswer((_) => Future>>.value( diff --git a/app_flutter/test/task_box_test.dart b/app_flutter/test/task_box_test.dart index 983dd77c28..8069462cec 100644 --- a/app_flutter/test/task_box_test.dart +++ b/app_flutter/test/task_box_test.dart @@ -313,7 +313,7 @@ void main() { }); final GoogleSignInService mockAuth = MockGoogleSignInService(); - when(mockAuth.accessToken).thenReturn('abc123'); + when(mockAuth.idToken).thenAnswer((_) => Future.value('abc123')); when(buildState.authService).thenReturn(mockAuth); await tester.pumpWidget( MaterialApp( @@ -345,7 +345,7 @@ void main() { 'enableDomStorage': false, 'universalLinksOnly': false, 'headers': { - 'X-Flutter-AccessToken': 'abc123', + 'X-Flutter-IdToken': 'abc123', } }) ], diff --git a/app_flutter/test/utils/fake_google_account.dart b/app_flutter/test/utils/fake_google_account.dart new file mode 100644 index 0000000000..f289555b83 --- /dev/null +++ b/app_flutter/test/utils/fake_google_account.dart @@ -0,0 +1,29 @@ +// Copyright (c) 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. + +import 'package:google_sign_in/google_sign_in.dart'; + +class FakeGoogleSignInAccount implements GoogleSignInAccount { + @override + String get displayName => 'Dr. Test'; + + @override + String get email => 'test@flutter.dev'; + + @override + String get id => 'test123'; + + @override + String get photoUrl => + 'https://lh3.googleusercontent.com/-ukEAtRyRhw8/AAAAAAAAAAI/AAAAAAAAAAA/ACHi3rfhID9XACtdb9q_xK43VSXQvBV11Q.CMID'; + + @override + Future> get authHeaders => null; + + @override + Future authentication; + + @override + Future clearAuthCache() => null; +} diff --git a/app_flutter/web/index.html b/app_flutter/web/index.html index 078c6201c1..5568d35d2d 100644 --- a/app_flutter/web/index.html +++ b/app_flutter/web/index.html @@ -5,6 +5,7 @@ + app_flutter