From dc13460dda54d6b5bfe0b6d4ae02550c2b39ee9a Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Tue, 27 Feb 2024 13:41:06 -0500 Subject: [PATCH] fix(shorebird_cli): get MS auth token, add token provider env var for CI (#1758) --- packages/shorebird_cli/analysis_options.yaml | 3 + .../shorebird_cli_integration_test.dart | 6 +- packages/shorebird_cli/lib/src/auth/auth.dart | 67 ++++++++++---- .../shorebird_cli/lib/src/auth/ci_token.dart | 37 ++++++++ .../lib/src/auth/ci_token.g.dart | 36 ++++++++ .../lib/src/commands/login_ci_command.dart | 9 +- .../test/src/auth/auth_test.dart | 90 +++++++++++++------ .../src/commands/login_ci_command_test.dart | 14 +-- .../lib/src/auth_functions.dart | 9 +- 9 files changed, 211 insertions(+), 60 deletions(-) create mode 100644 packages/shorebird_cli/lib/src/auth/ci_token.dart create mode 100644 packages/shorebird_cli/lib/src/auth/ci_token.g.dart diff --git a/packages/shorebird_cli/analysis_options.yaml b/packages/shorebird_cli/analysis_options.yaml index fa798a83..78045a57 100644 --- a/packages/shorebird_cli/analysis_options.yaml +++ b/packages/shorebird_cli/analysis_options.yaml @@ -1,4 +1,7 @@ include: package:very_good_analysis/analysis_options.5.1.0.yaml +analyzer: + exclude: + - lib/**.g.dart linter: rules: public_member_api_docs: false diff --git a/packages/shorebird_cli/integration_test/shorebird_cli_integration_test.dart b/packages/shorebird_cli/integration_test/shorebird_cli_integration_test.dart index 9a8d9b7a..8f727653 100644 --- a/packages/shorebird_cli/integration_test/shorebird_cli_integration_test.dart +++ b/packages/shorebird_cli/integration_test/shorebird_cli_integration_test.dart @@ -60,9 +60,11 @@ void main() { test( 'create an app with a release and patch', () async { - final authToken = Platform.environment['SHOREBIRD_TOKEN']; + final authToken = Platform.environment[shorebirdTokenEnvVar]; if (authToken == null || authToken.isEmpty) { - throw Exception('SHOREBIRD_TOKEN environment variable is not set.'); + throw Exception( + '$shorebirdTokenEnvVar environment variable is not set.', + ); } const releaseVersion = '1.0.0+1'; const platform = 'android'; diff --git a/packages/shorebird_cli/lib/src/auth/auth.dart b/packages/shorebird_cli/lib/src/auth/auth.dart index 55ff31b6..85093fc0 100644 --- a/packages/shorebird_cli/lib/src/auth/auth.dart +++ b/packages/shorebird_cli/lib/src/auth/auth.dart @@ -8,13 +8,17 @@ import 'package:http/http.dart' as http; import 'package:jwt/jwt.dart'; import 'package:path/path.dart' as p; import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/auth/ci_token.dart'; import 'package:shorebird_cli/src/auth/endpoints/endpoints.dart'; import 'package:shorebird_cli/src/command.dart'; import 'package:shorebird_cli/src/command_runner.dart'; import 'package:shorebird_cli/src/http_client/http_client.dart'; +import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/platform.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; +export 'ci_token.dart'; + // A reference to a [Auth] instance. final authRef = create(Auth.new); @@ -29,6 +33,9 @@ const googleJwtIssuer = 'https://accounts.google.com'; /// tenant ID, so we just match the prefix. const microsoftJwtIssuerPrefix = 'https://login.microsoftonline.com/'; +/// The environment variable that holds the Shorebird CI token. +const shorebirdTokenEnvVar = 'SHOREBIRD_TOKEN'; + typedef ObtainAccessCredentials = Future Function( oauth2.AuthEndpoints authEndpoints, oauth2.ClientId clientId, @@ -63,7 +70,7 @@ class AuthenticatedClient extends http.BaseClient { AuthenticatedClient.token({ required http.Client httpClient, - required String token, + required CiToken token, OnRefreshCredentials? onRefreshCredentials, RefreshCredentials refreshCredentials = oauth2.refreshCredentials, }) : this._( @@ -77,7 +84,7 @@ class AuthenticatedClient extends http.BaseClient { required http.Client httpClient, OnRefreshCredentials? onRefreshCredentials, oauth2.AccessCredentials? credentials, - String? token, + CiToken? token, RefreshCredentials refreshCredentials = oauth2.refreshCredentials, }) : _baseClient = httpClient, _credentials = credentials, @@ -89,7 +96,7 @@ class AuthenticatedClient extends http.BaseClient { final OnRefreshCredentials? _onRefreshCredentials; final RefreshCredentials _refreshCredentials; oauth2.AccessCredentials? _credentials; - final String? _token; + final CiToken? _token; @override Future send(http.BaseRequest request) async { @@ -97,16 +104,14 @@ class AuthenticatedClient extends http.BaseClient { if (credentials == null) { final token = _token!; - final jwt = Jwt.parse(token); - final authProvider = jwt.authProvider; credentials = _credentials = await _refreshCredentials( - authProvider.authEndpoints, - authProvider.clientId, + token.authProvider.authEndpoints, + token.authProvider.clientId, oauth2.AccessCredentials( // This isn't relevant for a refresh operation. AccessToken('Bearer', '', DateTime.timestamp()), - token, - authProvider.scopes, + token.refreshToken, + token.authProvider.scopes, ), _baseClient, ); @@ -153,7 +158,7 @@ class Auth { final String _credentialsDir; final ObtainAccessCredentials _obtainAccessCredentials; final CodePushClientBuilder _buildCodePushClient; - String? _token; + CiToken? _token; String get credentialsFilePath { return p.join(_credentialsDir, 'credentials.json'); @@ -165,7 +170,10 @@ class Auth { } if (_token != null) { - return AuthenticatedClient.token(token: _token!, httpClient: _httpClient); + return AuthenticatedClient.token( + token: _token!, + httpClient: _httpClient, + ); } return AuthenticatedClient.credentials( @@ -175,7 +183,7 @@ class Auth { ); } - Future loginCI( + Future loginCI( AuthProvider authProvider, { required void Function(String) prompt, }) async { @@ -199,7 +207,14 @@ class Auth { if (user == null) { throw UserNotFoundException(email: credentials.email!); } - return credentials; + if (credentials.refreshToken == null) { + throw Exception('No refresh token found.'); + } + + return CiToken( + refreshToken: credentials.refreshToken!, + authProvider: authProvider, + ); } finally { client.close(); } @@ -248,9 +263,22 @@ class Auth { bool get isAuthenticated => _email != null || _token != null; void _loadCredentials() { - final token = platform.environment['SHOREBIRD_TOKEN']; - if (token != null) { - _token = token; + final envToken = platform.environment[shorebirdTokenEnvVar]; + if (envToken != null) { + try { + _token = CiToken.fromBase64(envToken); + } catch (_) { + // TODO(bryanoltman): Remove this legacy behavior after July 2024 or + // next major release. + logger.warn(''' +The value of $shorebirdTokenEnvVar is not a valid base64-encoded token. This +will become an error in the next major release. Run `shorebird login:ci` before +then to obtain a new token.'''); + _token = CiToken( + refreshToken: envToken, + authProvider: AuthProvider.google, + ); + } return; } @@ -376,6 +404,11 @@ extension OauthValues on AuthProvider { 'openid', 'https://www.googleapis.com/auth/userinfo.email', ], - (AuthProvider.microsoft) => ['openid', 'email'], + (AuthProvider.microsoft) => [ + 'openid', + 'email', + // Required to get refresh tokens. + 'offline_access', + ], }; } diff --git a/packages/shorebird_cli/lib/src/auth/ci_token.dart b/packages/shorebird_cli/lib/src/auth/ci_token.dart new file mode 100644 index 00000000..6b030cd1 --- /dev/null +++ b/packages/shorebird_cli/lib/src/auth/ci_token.dart @@ -0,0 +1,37 @@ +import 'dart:convert'; + +import 'package:json_annotation/json_annotation.dart'; +import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; + +part 'ci_token.g.dart'; + +/// {@template ci_token} +/// A CI token. +/// {@endtemplate} +@JsonSerializable() +class CiToken { + /// {@macro ci_token} + const CiToken({required this.refreshToken, required this.authProvider}); + + /// Creates a [CiToken] from a base64 encoded string. + factory CiToken.fromBase64(String base64) { + return CiToken.fromJson( + jsonDecode(utf8.decode(base64Decode(base64))) as Map, + ); + } + + /// Encodes the [CiToken] to a base64 string. + String toBase64() => base64Encode(utf8.encode(jsonEncode(toJson()))); + + /// Creates a [CiToken] from a JSON object. + static CiToken fromJson(Map json) => _$CiTokenFromJson(json); + + /// Converts the [CiToken] to a JSON object. + Map toJson() => _$CiTokenToJson(this); + + /// The token used to obtain a JWT. + final String refreshToken; + + /// The authentication provider used to obtain the token. + final AuthProvider authProvider; +} diff --git a/packages/shorebird_cli/lib/src/auth/ci_token.g.dart b/packages/shorebird_cli/lib/src/auth/ci_token.g.dart new file mode 100644 index 00000000..849d5f88 --- /dev/null +++ b/packages/shorebird_cli/lib/src/auth/ci_token.g.dart @@ -0,0 +1,36 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: implicit_dynamic_parameter, require_trailing_commas, cast_nullable_to_non_nullable, lines_longer_than_80_chars, strict_raw_type, unnecessary_lambdas + +part of 'ci_token.dart'; + +// ************************************************************************** +// JsonSerializableGenerator +// ************************************************************************** + +CiToken _$CiTokenFromJson(Map json) => $checkedCreate( + 'CiToken', + json, + ($checkedConvert) { + final val = CiToken( + refreshToken: $checkedConvert('refresh_token', (v) => v as String), + authProvider: $checkedConvert( + 'auth_provider', (v) => $enumDecode(_$AuthProviderEnumMap, v)), + ); + return val; + }, + fieldKeyMap: const { + 'refreshToken': 'refresh_token', + 'authProvider': 'auth_provider' + }, + ); + +Map _$CiTokenToJson(CiToken instance) => { + 'refresh_token': instance.refreshToken, + 'auth_provider': _$AuthProviderEnumMap[instance.authProvider]!, + }; + +const _$AuthProviderEnumMap = { + AuthProvider.google: 'google', + AuthProvider.microsoft: 'microsoft', +}; diff --git a/packages/shorebird_cli/lib/src/commands/login_ci_command.dart b/packages/shorebird_cli/lib/src/commands/login_ci_command.dart index bd7ca96f..b00e77ac 100644 --- a/packages/shorebird_cli/lib/src/commands/login_ci_command.dart +++ b/packages/shorebird_cli/lib/src/commands/login_ci_command.dart @@ -1,4 +1,3 @@ -import 'package:googleapis_auth/auth_io.dart'; import 'package:mason_logger/mason_logger.dart'; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/command.dart'; @@ -40,9 +39,9 @@ class LoginCiCommand extends ShorebirdCommand { ); } - final AccessCredentials credentials; + final CiToken ciToken; try { - credentials = await auth.loginCI(provider, prompt: prompt); + ciToken = await auth.loginCI(provider, prompt: prompt); } on UserNotFoundException catch (error) { logger ..err( @@ -62,11 +61,11 @@ We could not find a Shorebird account for ${error.email}.''', 🎉 ${lightGreen.wrap('Success! Use the following token to login on a CI server:')} -${lightCyan.wrap(credentials.refreshToken)} +${lightCyan.wrap(ciToken.toBase64())} Example: -${lightCyan.wrap(r'export SHOREBIRD_TOKEN="$SHOREBIRD_TOKEN" && shorebird patch android')} +${lightCyan.wrap('export $shorebirdTokenEnvVar="\$SHOREBIRD_TOKEN" && shorebird patch android')} '''); return ExitCode.success.code; } diff --git a/packages/shorebird_cli/test/src/auth/auth_test.dart b/packages/shorebird_cli/test/src/auth/auth_test.dart index dca2bda6..bbab282e 100644 --- a/packages/shorebird_cli/test/src/auth/auth_test.dart +++ b/packages/shorebird_cli/test/src/auth/auth_test.dart @@ -119,13 +119,17 @@ void main() { group(Auth, () { const idToken = '''eyJhbGciOiJIUzI1NiIsImtpZCI6IjEyMzQiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20iLCJhenAiOiI1MjMzMDIyMzMyOTMtZWlhNWFudG0wdGd2ZWsyNDB0NDZvcmN0a3RpYWJyZWsuYXBwcy5nb29nbGV1c2VyY29udGVudC5jb20iLCJhdWQiOiI1MjMzMDIyMzMyOTMtZWlhNWFudG0wdGd2ZWsyNDB0NDZvcmN0a3RpYWJyZWsuYXBwcy5nb29nbGV1c2VyY29udGVudC5jb20iLCJzdWIiOiIxMjM0NSIsImhkIjoic2hvcmViaXJkLmRldiIsImVtYWlsIjoidGVzdEBlbWFpbC5jb20iLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaWF0IjoxMjM0LCJleHAiOjY3ODl9.MYbITALvKsGYTYjw1o7AQ0ObkqRWVBSr9cFYJrvA46g'''; + const refreshToken = 'shorebird-token'; + const ciToken = CiToken( + refreshToken: refreshToken, + authProvider: AuthProvider.google, + ); const email = 'test@email.com'; const user = User( id: 42, email: email, jwtIssuer: googleJwtIssuer, ); - const refreshToken = ''; const scopes = []; final accessToken = oauth2.AccessToken( 'Bearer', @@ -133,13 +137,7 @@ void main() { DateTime.now().add(const Duration(minutes: 10)).toUtc(), ); - final accessCredentials = oauth2.AccessCredentials( - accessToken, - refreshToken, - scopes, - idToken: idToken, - ); - + late oauth2.AccessCredentials accessCredentials; late String credentialsDir; late http.Client httpClient; late CodePushClient codePushClient; @@ -185,6 +183,12 @@ void main() { } setUp(() { + accessCredentials = oauth2.AccessCredentials( + accessToken, + refreshToken, + scopes, + idToken: idToken, + ); credentialsDir = Directory.systemTemp.createTempSync().path; httpClient = MockHttpClient(); codePushClient = MockCodePushClient(); @@ -199,13 +203,10 @@ void main() { group('AuthenticatedClient', () { group('token', () { - const token = - '''eyJhbGciOiJIUzI1NiIsImtpZCI6IjEyMzQiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20iLCJhenAiOiI1MjMzMDIyMzMyOTMtZWlhNWFudG0wdGd2ZWsyNDB0NDZvcmN0a3RpYWJyZWsuYXBwcy5nb29nbGV1c2VyY29udGVudC5jb20iLCJhdWQiOiI1MjMzMDIyMzMyOTMtZWlhNWFudG0wdGd2ZWsyNDB0NDZvcmN0a3RpYWJyZWsuYXBwcy5nb29nbGV1c2VyY29udGVudC5jb20iLCJzdWIiOiIxMjM0NSIsImhkIjoic2hvcmViaXJkLmRldiIsImVtYWlsIjoidGVzdEBlbWFpbC5jb20iLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaWF0IjoxMjM0LCJleHAiOjY3ODl9.MYbITALvKsGYTYjw1o7AQ0ObkqRWVBSr9cFYJrvA46g'''; - test('does not require an onRefreshCredentials callback', () { expect( () => AuthenticatedClient.token( - token: token, + token: ciToken, httpClient: httpClient, refreshCredentials: (authEndpoints, clientId, credentials, client) async => @@ -227,7 +228,7 @@ void main() { final onRefreshCredentialsCalls = []; final client = AuthenticatedClient.token( - token: token, + token: ciToken, httpClient: httpClient, onRefreshCredentials: onRefreshCredentialsCalls.add, refreshCredentials: @@ -261,7 +262,7 @@ void main() { ); final onRefreshCredentialsCalls = []; final client = AuthenticatedClient.token( - token: token, + token: ciToken, httpClient: httpClient, onRefreshCredentials: onRefreshCredentialsCalls.add, refreshCredentials: @@ -388,10 +389,28 @@ void main() { expect(request.headers['Authorization'], equals('Bearer $idToken')); }); + group('when token is invalid', () { + setUp(() { + when(() => platform.environment).thenReturn( + {shorebirdTokenEnvVar: 'not a base64 string'}, + ); + }); + + test('prints warning message when token string is not valid base64', + () async { + auth = buildAuth(); + verify( + () => logger.warn(''' +The value of $shorebirdTokenEnvVar is not a valid base64-encoded token. This +will become an error in the next major release. Run `shorebird login:ci` before +then to obtain a new token.'''), + ).called(1); + }); + }); + test( 'returns an authenticated client ' - 'when a token is present.', () async { - const token = 'shorebird-token'; + 'when a token and token provider is present.', () async { when(() => httpClient.send(any())).thenAnswer( (_) async => http.StreamedResponse( const Stream.empty(), @@ -399,7 +418,7 @@ void main() { ), ); when(() => platform.environment).thenReturn( - {'SHOREBIRD_TOKEN': token}, + {shorebirdTokenEnvVar: ciToken.toBase64()}, ); auth = buildAuth(); final client = auth.client; @@ -466,21 +485,18 @@ void main() { }); group('loginCI', () { - const token = 'shorebird-token'; setUp(() { when(() => platform.environment).thenReturn( - {'SHOREBIRD_TOKEN': token}, + {shorebirdTokenEnvVar: ciToken.toBase64()}, ); auth = buildAuth(); }); - test( - 'returns credentials and does not set the email or cache credentials', + test('returns a CI token and does not set the email or cache credentials', () async { - await expectLater( - auth.loginCI(AuthProvider.google, prompt: (_) {}), - completion(equals(accessCredentials)), - ); + final token = await auth.loginCI(AuthProvider.google, prompt: (_) {}); + expect(token.authProvider, ciToken.authProvider); + expect(token.refreshToken, ciToken.refreshToken); expect(auth.email, isNull); expect(auth.isAuthenticated, isTrue); expect(buildAuth().email, isNull); @@ -501,6 +517,30 @@ void main() { expect(auth.email, isNull); }); + + group('when credentials are missing a refresh token', () { + setUp(() { + accessCredentials = oauth2.AccessCredentials( + accessToken, + null, + scopes, + idToken: idToken, + ); + }); + + test('throws if credentials are missing a refresh token', () async { + await expectLater( + auth.loginCI(AuthProvider.google, prompt: (_) {}), + throwsA( + isA().having( + (e) => e.toString(), + 'toString', + 'Exception: No refresh token found.', + ), + ), + ); + }); + }); }); group('logout', () { diff --git a/packages/shorebird_cli/test/src/commands/login_ci_command_test.dart b/packages/shorebird_cli/test/src/commands/login_ci_command_test.dart index ae429528..5c99721d 100644 --- a/packages/shorebird_cli/test/src/commands/login_ci_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/login_ci_command_test.dart @@ -153,15 +153,17 @@ void main() { }); test('exits with code 0 when logged in successfully', () async { - const token = 'shorebird-token'; - final credentials = MockAccessCredentials(); - when(() => credentials.refreshToken).thenReturn(token); + const token = CiToken( + // "shorebird-token" in base64 + refreshToken: 'c2hvcmViaXJkLXRva2Vu', + authProvider: AuthProvider.google, + ); when( () => auth.loginCI( any(), prompt: any(named: 'prompt'), ), - ).thenAnswer((_) async => credentials); + ).thenAnswer((_) async => token); when(() => auth.email).thenReturn(email); final result = await runWithOverrides(command.run); @@ -174,7 +176,9 @@ void main() { ), ).called(1); verify( - () => logger.info(any(that: contains('${lightCyan.wrap(token)}'))), + () => logger.info( + any(that: contains('${lightCyan.wrap(token.toBase64())}')), + ), ).called(1); }); diff --git a/third_party/googleapis_auth/lib/src/auth_functions.dart b/third_party/googleapis_auth/lib/src/auth_functions.dart index d6109296..63d6cc1a 100644 --- a/third_party/googleapis_auth/lib/src/auth_functions.dart +++ b/third_party/googleapis_auth/lib/src/auth_functions.dart @@ -105,11 +105,6 @@ Future refreshCredentials( AccessCredentials credentials, Client client, ) async { - final secret = clientId.secret; - if (secret == null) { - throw ArgumentError('clientId.secret cannot be null.'); - } - final refreshToken = credentials.refreshToken; if (refreshToken == null) { throw ArgumentError('clientId.refreshToken cannot be null.'); @@ -119,7 +114,9 @@ Future refreshCredentials( final jsonMap = await client.oauthTokenRequest( { 'client_id': clientId.identifier, - 'client_secret': secret, + // Not all providers require a client secret, + // e.g. https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow#refresh-the-access-token + if (clientId.secret != null) 'client_secret': clientId.secret!, 'refresh_token': refreshToken, 'grant_type': 'refresh_token', },