From d0a9345818951ee74dc21f789eb06b6ced6c4fe1 Mon Sep 17 00:00:00 2001 From: Dillon Nys Date: Wed, 28 Jun 2023 12:24:55 -0700 Subject: [PATCH] fix(auth): Remove `nonce` usage `nonce` was previously used to protect against replay attacks, but these are primarily a concern with the implicit OAuth grant which we do not support. To unblock potential issues with custom OIDC providers, we're removing the nonce checks. --- .../lib/src/credentials/cognito_keys.dart | 5 ++++ .../flows/hosted_ui/hosted_ui_platform.dart | 11 ++------ .../lib/src/jwt/src/cognito.dart | 16 ------------ .../session/cognito_user_pool_tokens.dart | 15 ----------- .../lib/common/jwt.dart | 2 -- .../lib/common/mock_oauth_server.dart | 11 +------- .../hostedui/hosted_ui_platform_test.dart | 26 ++----------------- 7 files changed, 10 insertions(+), 76 deletions(-) diff --git a/packages/auth/amplify_auth_cognito_dart/lib/src/credentials/cognito_keys.dart b/packages/auth/amplify_auth_cognito_dart/lib/src/credentials/cognito_keys.dart index 5779c8afb5..60a5aa1313 100644 --- a/packages/auth/amplify_auth_cognito_dart/lib/src/credentials/cognito_keys.dart +++ b/packages/auth/amplify_auth_cognito_dart/lib/src/credentials/cognito_keys.dart @@ -97,6 +97,11 @@ enum HostedUiKey { codeVerifier, /// The OIDC nonce value. + @Deprecated( + 'This value should no longer be used except for removing existing usages. ' + 'The library used to support nonce verification of the OIDC tokens but no ' + 'does.', + ) nonce, /// The [CognitoSignInWithWebUIPluginOptions] passed to `signInWithWebUI`. diff --git a/packages/auth/amplify_auth_cognito_dart/lib/src/flows/hosted_ui/hosted_ui_platform.dart b/packages/auth/amplify_auth_cognito_dart/lib/src/flows/hosted_ui/hosted_ui_platform.dart index 33d4f56f20..68a49ac0a5 100644 --- a/packages/auth/amplify_auth_cognito_dart/lib/src/flows/hosted_ui/hosted_ui_platform.dart +++ b/packages/auth/amplify_auth_cognito_dart/lib/src/flows/hosted_ui/hosted_ui_platform.dart @@ -99,7 +99,6 @@ abstract class HostedUiPlatform implements Closeable { }) async { final state = generateState(); final codeVerifier = createCodeVerifier(); - final nonce = generateState(); await Future.wait( [ @@ -111,10 +110,6 @@ abstract class HostedUiPlatform implements Closeable { key: _keys[HostedUiKey.codeVerifier], value: codeVerifier, ), - _secureStorage.write( - key: _keys[HostedUiKey.nonce], - value: nonce, - ), ].map(Future.value), ); @@ -133,7 +128,6 @@ abstract class HostedUiPlatform implements Closeable { return uri.replace( queryParameters: { ...uri.queryParameters, - 'nonce': nonce, }, ); } @@ -235,9 +229,7 @@ abstract class HostedUiPlatform implements Closeable { accessToken: JsonWebToken.parse(oAuthCredentials.accessToken), refreshToken: oAuthCredentials.refreshToken!, idToken: JsonWebToken.parse(oAuthCredentials.idToken!), - )..validate( - nonce: await _secureStorage.read(key: _keys[HostedUiKey.nonce]), - ); + ); return tokens; @@ -269,6 +261,7 @@ abstract class HostedUiPlatform implements Closeable { [ _secureStorage.delete(key: _keys[HostedUiKey.state]), _secureStorage.delete(key: _keys[HostedUiKey.codeVerifier]), + // ignore: deprecated_member_use_from_same_package _secureStorage.delete(key: _keys[HostedUiKey.nonce]), _secureStorage.delete(key: _keys[HostedUiKey.options]), ].map(Future.value), diff --git a/packages/auth/amplify_auth_cognito_dart/lib/src/jwt/src/cognito.dart b/packages/auth/amplify_auth_cognito_dart/lib/src/jwt/src/cognito.dart index 5cbe76d599..09eb32c8cb 100644 --- a/packages/auth/amplify_auth_cognito_dart/lib/src/jwt/src/cognito.dart +++ b/packages/auth/amplify_auth_cognito_dart/lib/src/jwt/src/cognito.dart @@ -89,22 +89,6 @@ extension CognitoAccessToken on JsonWebToken { /// - [OpenID Connect Core: ID Token](https://openid.net/specs/openid-connect-core-1_0.html#IDToken) /// - [OpenID Connect Core: Standard Claims](https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims) extension CognitoIdToken on JsonWebToken { - /// The `nonce` claim comes from a parameter of the same name that you can add - /// to requests to your OAuth 2.0 `authorize` endpoint. When you add the - /// parameter, the `nonce` claim is included in the ID token that Amazon - /// Cognito issues, and you can use it to guard against replay attacks. - /// - /// If you do not provide a nonce value in your request, Amazon Cognito - /// automatically generates and validates a nonce when you authenticate - /// through a third-party identity provider, then adds it as a nonce claim to - /// the ID token. - /// - /// The implementation of the nonce claim in Amazon Cognito is based on OIDC - /// standards. - /// - /// [Reference](https://docs.aws.amazon.com/cognito/latest/developerguide/amazon-cognito-user-pools-using-the-id-token.html) - String? get nonce => claims.customClaims['nonce'] as String?; - /// The Cognito username. /// /// [Reference](https://docs.aws.amazon.com/cognito/latest/developerguide/amazon-cognito-user-pools-using-the-id-token.html) diff --git a/packages/auth/amplify_auth_cognito_dart/lib/src/model/session/cognito_user_pool_tokens.dart b/packages/auth/amplify_auth_cognito_dart/lib/src/model/session/cognito_user_pool_tokens.dart index 9623aeae8d..372dd20bc8 100644 --- a/packages/auth/amplify_auth_cognito_dart/lib/src/model/session/cognito_user_pool_tokens.dart +++ b/packages/auth/amplify_auth_cognito_dart/lib/src/model/session/cognito_user_pool_tokens.dart @@ -74,21 +74,6 @@ abstract class CognitoUserPoolTokens /// The Cognito user's username. String get username => CognitoIdToken(idToken).username; - /// Validates the tokens against the client state. - void validate({ - String? nonce, - }) { - // Missing nonce values or nonce mismatches should throw exceptions, - // indicating the tokens cannot be used. - final idTokenNonce = idToken.nonce; - if (nonce == null || idTokenNonce == null) { - throw const InvalidStateException('Missing nonce'); - } - if (nonce != idTokenNonce) { - throw const InvalidStateException('Nonce values do not match'); - } - } - @override Map toJson() => _$CognitoUserPoolTokensToJson(this); diff --git a/packages/auth/amplify_auth_cognito_test/lib/common/jwt.dart b/packages/auth/amplify_auth_cognito_test/lib/common/jwt.dart index a5ada5f948..34f3151e60 100644 --- a/packages/auth/amplify_auth_cognito_test/lib/common/jwt.dart +++ b/packages/auth/amplify_auth_cognito_test/lib/common/jwt.dart @@ -9,7 +9,6 @@ enum TokenType { access, id } JsonWebToken createJwt({ required TokenType type, required Duration expiration, - String? nonce, }) { return JsonWebToken( header: const JsonWebHeader( @@ -21,7 +20,6 @@ JsonWebToken createJwt({ if (type == TokenType.access) 'username': username, if (type == TokenType.id) 'cognito:username': username, 'exp': (DateTime.now().add(expiration)).millisecondsSinceEpoch ~/ 1000, - if (nonce != null) 'nonce': nonce, }, ), signature: const [], diff --git a/packages/auth/amplify_auth_cognito_test/lib/common/mock_oauth_server.dart b/packages/auth/amplify_auth_cognito_test/lib/common/mock_oauth_server.dart index eca994d1ee..f43ea57601 100644 --- a/packages/auth/amplify_auth_cognito_test/lib/common/mock_oauth_server.dart +++ b/packages/auth/amplify_auth_cognito_test/lib/common/mock_oauth_server.dart @@ -18,7 +18,6 @@ class _Request { required this.codeChallenge, required this.scope, required this.authCode, - this.nonce, }); final String clientId; @@ -27,7 +26,6 @@ class _Request { final String codeChallenge; final String scope; final String authCode; - final String? nonce; } const paramResponseType = 'response_type'; @@ -40,7 +38,6 @@ const paramCode = 'code'; const paramCodeChallenge = 'code_challenge'; const paramCodeChallengeMethod = 'code_challenge_method'; const paramCodeVerifier = 'code_verifier'; -const paramNonce = 'nonce'; const paramUsername = 'username'; const paramPassword = 'password'; const paramRefreshToken = 'refresh_token'; @@ -95,10 +92,7 @@ class MockOAuthServer { final MockClientHandler? _tokenHandler; late final MockClientHandler tokenHandler = _tokenHandler ?? createTokenHandler(); - static MockClientHandler createTokenHandler({ - bool includeNonce = true, - }) => - (Request request) async { + static MockClientHandler createTokenHandler() => (Request request) async { final query = request.bodyFields; final code = query[paramCode]; @@ -134,7 +128,6 @@ class MockOAuthServer { final idToken = createJwt( type: TokenType.id, expiration: exp, - nonce: includeNonce ? session.nonce : null, ); final response = { 'token_type': 'bearer', @@ -184,7 +177,6 @@ class MockOAuthServer { if (scope == null) { return _missingParameter(paramScope, state: state); } - final nonce = query[paramNonce]; final authCode = generateState(); @@ -195,7 +187,6 @@ class MockOAuthServer { state: state, scope: scope, authCode: authCode, - nonce: nonce, ); _pendingRequests[authCode] = session; diff --git a/packages/auth/amplify_auth_cognito_test/test/flows/hostedui/hosted_ui_platform_test.dart b/packages/auth/amplify_auth_cognito_test/test/flows/hostedui/hosted_ui_platform_test.dart index c19f547a2b..e5301c1d9a 100644 --- a/packages/auth/amplify_auth_cognito_test/test/flows/hostedui/hosted_ui_platform_test.dart +++ b/packages/auth/amplify_auth_cognito_test/test/flows/hostedui/hosted_ui_platform_test.dart @@ -43,7 +43,6 @@ void main() { group('exchange', () { const state = 'state'; const codeVerifier = 'codeVerifier'; - const nonce = 'nonce'; setUp(() { secureStorage @@ -51,8 +50,7 @@ void main() { ..write( key: keys[HostedUiKey.codeVerifier], value: codeVerifier, - ) - ..write(key: keys[HostedUiKey.nonce], value: nonce); + ); }); tearDown(() { @@ -97,27 +95,7 @@ void main() { ); }); - test('missing nonce throws', () async { - server = MockOAuthServer( - tokenHandler: MockOAuthServer.createTokenHandler( - includeNonce: false, - ), - ); - dependencyManager.addInstance(server.httpClient); - platform = HostedUiPlatform(dependencyManager); - final parameters = await server.authorize( - await platform.getSignInUri( - redirectUri: Uri.parse(redirectUri), - ), - ); - - expect( - platform.exchange(parameters), - throwsInvalidStateException, - ); - }); - - test('succeeds with nonce', () async { + test('succeeds', () async { final parameters = await server.authorize( await platform.getSignInUri( redirectUri: Uri.parse(redirectUri),