Skip to content

Commit

Permalink
fix(auth): Remove nonce usage
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
Dillon Nys committed Jun 28, 2023
1 parent 9c67fad commit d0a9345
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ abstract class HostedUiPlatform implements Closeable {
}) async {
final state = generateState();
final codeVerifier = createCodeVerifier();
final nonce = generateState();

await Future.wait<void>(
[
Expand All @@ -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),
);

Expand All @@ -133,7 +128,6 @@ abstract class HostedUiPlatform implements Closeable {
return uri.replace(
queryParameters: <String, String>{
...uri.queryParameters,
'nonce': nonce,
},
);
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object?> toJson() => _$CognitoUserPoolTokensToJson(this);

Expand Down
2 changes: 0 additions & 2 deletions packages/auth/amplify_auth_cognito_test/lib/common/jwt.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ enum TokenType { access, id }
JsonWebToken createJwt({
required TokenType type,
required Duration expiration,
String? nonce,
}) {
return JsonWebToken(
header: const JsonWebHeader(
Expand All @@ -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 [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class _Request {
required this.codeChallenge,
required this.scope,
required this.authCode,
this.nonce,
});

final String clientId;
Expand All @@ -27,7 +26,6 @@ class _Request {
final String codeChallenge;
final String scope;
final String authCode;
final String? nonce;
}

const paramResponseType = 'response_type';
Expand All @@ -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';
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -134,7 +128,6 @@ class MockOAuthServer {
final idToken = createJwt(
type: TokenType.id,
expiration: exp,
nonce: includeNonce ? session.nonce : null,
);
final response = <String, dynamic>{
'token_type': 'bearer',
Expand Down Expand Up @@ -184,7 +177,6 @@ class MockOAuthServer {
if (scope == null) {
return _missingParameter(paramScope, state: state);
}
final nonce = query[paramNonce];

final authCode = generateState();

Expand All @@ -195,7 +187,6 @@ class MockOAuthServer {
state: state,
scope: scope,
authCode: authCode,
nonce: nonce,
);

_pendingRequests[authCode] = session;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,14 @@ void main() {
group('exchange', () {
const state = 'state';
const codeVerifier = 'codeVerifier';
const nonce = 'nonce';

setUp(() {
secureStorage
..write(key: keys[HostedUiKey.state], value: state)
..write(
key: keys[HostedUiKey.codeVerifier],
value: codeVerifier,
)
..write(key: keys[HostedUiKey.nonce], value: nonce);
);
});

tearDown(() {
Expand Down Expand Up @@ -97,27 +95,7 @@ void main() {
);
});

test('missing nonce throws', () async {
server = MockOAuthServer(
tokenHandler: MockOAuthServer.createTokenHandler(
includeNonce: false,
),
);
dependencyManager.addInstance<http.Client>(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),
Expand Down

0 comments on commit d0a9345

Please sign in to comment.