From c5db92fa1a012ffd22991db590f210bd20fbc62b Mon Sep 17 00:00:00 2001 From: Felix Angelov Date: Thu, 19 Dec 2024 11:50:58 -0600 Subject: [PATCH] refactor(jwt): upgrade analysis_options (#2715) --- packages/jwt/analysis_options.yaml | 4 +- packages/jwt/build.yaml | 1 + packages/jwt/example/main.dart | 1 - packages/jwt/lib/src/jwt.dart | 35 ++++---- packages/jwt/lib/src/models/jwk.g.dart | 2 +- packages/jwt/lib/src/models/jwt.dart | 4 +- packages/jwt/lib/src/models/jwt_header.g.dart | 2 +- .../jwt/lib/src/models/jwt_payload.g.dart | 8 +- .../public_key_store/jwk_key_store.g.dart | 2 +- .../public_key_store/key_value_key_store.dart | 10 ++- .../public_key_store/public_key_store.dart | 8 +- packages/jwt/pubspec.yaml | 4 +- packages/jwt/test/src/jwt_test.dart | 87 +++++++++++++++++-- 13 files changed, 127 insertions(+), 41 deletions(-) diff --git a/packages/jwt/analysis_options.yaml b/packages/jwt/analysis_options.yaml index 799268d3..58a08d76 100644 --- a/packages/jwt/analysis_options.yaml +++ b/packages/jwt/analysis_options.yaml @@ -1 +1,3 @@ -include: package:very_good_analysis/analysis_options.5.1.0.yaml +include: package:very_good_analysis/analysis_options.7.0.0.yaml +analyzer: + exclude: ["example/**"] diff --git a/packages/jwt/build.yaml b/packages/jwt/build.yaml index 1160343d..4e02f3c7 100644 --- a/packages/jwt/build.yaml +++ b/packages/jwt/build.yaml @@ -8,6 +8,7 @@ targets: - require_trailing_commas - cast_nullable_to_non_nullable - lines_longer_than_80_chars + - document_ignores json_serializable: options: field_rename: snake diff --git a/packages/jwt/example/main.dart b/packages/jwt/example/main.dart index dc4d5d6c..47fdf345 100644 --- a/packages/jwt/example/main.dart +++ b/packages/jwt/example/main.dart @@ -1,4 +1,3 @@ -// ignore_for_file: avoid_print import 'package:jwt/jwt.dart' as jwt; Future main() async { diff --git a/packages/jwt/lib/src/jwt.dart b/packages/jwt/lib/src/jwt.dart index ad2563cc..0ad2f112 100644 --- a/packages/jwt/lib/src/jwt.dart +++ b/packages/jwt/lib/src/jwt.dart @@ -90,8 +90,13 @@ Future verify( // By using this keystore's key IDs to validate the header above, we've // guaranteed that there is a public key for this key ID. final publicKey = publicKeys.getPublicKey(jwt.header.kid)!; + final bool isValid; + try { + isValid = _verifySignature(encodedJwt, publicKey); + } on Exception { + throw const JwtVerificationFailure('JWT signature is malformed.'); + } - final isValid = _verifySignature(encodedJwt, publicKey); if (!isValid) { throw const JwtVerificationFailure('Invalid signature.'); } @@ -165,22 +170,18 @@ bool _verifySignature(String jwt, String publicKey) { if (pair.public is! rsa.RSAPublicKey) return false; final public = pair.public; - try { - final signer = Signer('SHA-256/RSA'); - final key = RSAPublicKey( - public!.modulus, - BigInt.from(public.publicExponent), - ); - final param = ParametersWithRandom( - PublicKeyParameter(key), - SecureRandom('AES/CTR/PRNG'), - ); - signer.init(false, param); - final rsaSignature = RSASignature(Uint8List.fromList(sign)); - return signer.verifySignature(Uint8List.fromList(body), rsaSignature); - } catch (_) { - return false; - } + final signer = Signer('SHA-256/RSA'); + final key = RSAPublicKey( + public!.modulus, + BigInt.from(public.publicExponent), + ); + final param = ParametersWithRandom( + PublicKeyParameter(key), + SecureRandom('AES/CTR/PRNG'), + ); + signer.init(false, param); + final rsaSignature = RSASignature(Uint8List.fromList(sign)); + return signer.verifySignature(Uint8List.fromList(body), rsaSignature); } /// Visible for testing only diff --git a/packages/jwt/lib/src/models/jwk.g.dart b/packages/jwt/lib/src/models/jwk.g.dart index 087a0add..cb62446d 100644 --- a/packages/jwt/lib/src/models/jwk.g.dart +++ b/packages/jwt/lib/src/models/jwk.g.dart @@ -1,6 +1,6 @@ // 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 +// ignore_for_file: implicit_dynamic_parameter, require_trailing_commas, cast_nullable_to_non_nullable, lines_longer_than_80_chars, document_ignores part of 'jwk.dart'; diff --git a/packages/jwt/lib/src/models/jwt.dart b/packages/jwt/lib/src/models/jwt.dart index 02c8213a..7191a681 100644 --- a/packages/jwt/lib/src/models/jwt.dart +++ b/packages/jwt/lib/src/models/jwt.dart @@ -24,14 +24,14 @@ class Jwt { final JwtHeader header; try { header = JwtHeader.fromJson(decodeJwtPart(parts[0])); - } catch (_) { + } on Exception { throw const FormatException('JWT header is malformed.'); } final JwtPayload payload; try { payload = JwtPayload.fromJson(decodeJwtPart(parts[1])); - } catch (_) { + } on Exception { throw const FormatException('JWT payload is malformed.'); } diff --git a/packages/jwt/lib/src/models/jwt_header.g.dart b/packages/jwt/lib/src/models/jwt_header.g.dart index 04640db4..aafcf295 100644 --- a/packages/jwt/lib/src/models/jwt_header.g.dart +++ b/packages/jwt/lib/src/models/jwt_header.g.dart @@ -1,6 +1,6 @@ // 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 +// ignore_for_file: implicit_dynamic_parameter, require_trailing_commas, cast_nullable_to_non_nullable, lines_longer_than_80_chars, document_ignores part of 'jwt_header.dart'; diff --git a/packages/jwt/lib/src/models/jwt_payload.g.dart b/packages/jwt/lib/src/models/jwt_payload.g.dart index 98dd8ef9..d144aced 100644 --- a/packages/jwt/lib/src/models/jwt_payload.g.dart +++ b/packages/jwt/lib/src/models/jwt_payload.g.dart @@ -1,6 +1,6 @@ // 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 +// ignore_for_file: implicit_dynamic_parameter, require_trailing_commas, cast_nullable_to_non_nullable, lines_longer_than_80_chars, document_ignores part of 'jwt_payload.dart'; @@ -13,12 +13,12 @@ JwtPayload _$JwtPayloadFromJson(Map json) => $checkedCreate( json, ($checkedConvert) { final val = JwtPayload( - exp: $checkedConvert('exp', (v) => v as int), - iat: $checkedConvert('iat', (v) => v as int), + exp: $checkedConvert('exp', (v) => (v as num).toInt()), + iat: $checkedConvert('iat', (v) => (v as num).toInt()), aud: $checkedConvert('aud', (v) => v as String), iss: $checkedConvert('iss', (v) => v as String), sub: $checkedConvert('sub', (v) => v as String), - authTime: $checkedConvert('auth_time', (v) => v as int?), + authTime: $checkedConvert('auth_time', (v) => (v as num?)?.toInt()), ); return val; }, diff --git a/packages/jwt/lib/src/models/public_key_store/jwk_key_store.g.dart b/packages/jwt/lib/src/models/public_key_store/jwk_key_store.g.dart index 1c9c93cf..64d42af3 100644 --- a/packages/jwt/lib/src/models/public_key_store/jwk_key_store.g.dart +++ b/packages/jwt/lib/src/models/public_key_store/jwk_key_store.g.dart @@ -1,6 +1,6 @@ // 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 +// ignore_for_file: implicit_dynamic_parameter, require_trailing_commas, cast_nullable_to_non_nullable, lines_longer_than_80_chars, document_ignores part of 'jwk_key_store.dart'; diff --git a/packages/jwt/lib/src/models/public_key_store/key_value_key_store.dart b/packages/jwt/lib/src/models/public_key_store/key_value_key_store.dart index a24a9171..9d2e7af2 100644 --- a/packages/jwt/lib/src/models/public_key_store/key_value_key_store.dart +++ b/packages/jwt/lib/src/models/public_key_store/key_value_key_store.dart @@ -10,8 +10,14 @@ class KeyValueKeyStore extends PublicKeyStore { const KeyValueKeyStore({required this.keys}); /// Decodes a JSON object into a [KeyValueKeyStore]. - factory KeyValueKeyStore.fromJson(Map json) => - KeyValueKeyStore(keys: json.map((k, v) => MapEntry(k, v as String))); + factory KeyValueKeyStore.fromJson(Map json) { + return KeyValueKeyStore( + keys: json.map((k, v) { + if (v is! String) throw const FormatException('value is not a string.'); + return MapEntry(k, v); + }), + ); + } /// Map of all public key id/value pairs. final Map keys; diff --git a/packages/jwt/lib/src/models/public_key_store/public_key_store.dart b/packages/jwt/lib/src/models/public_key_store/public_key_store.dart index 37358258..94c942c9 100644 --- a/packages/jwt/lib/src/models/public_key_store/public_key_store.dart +++ b/packages/jwt/lib/src/models/public_key_store/public_key_store.dart @@ -15,11 +15,15 @@ abstract class PublicKeyStore { if (json.containsKey('keys')) { try { return JwkKeyStore.fromJson(json); - } catch (_) {} + } on Exception { + // Swallow deserialization exceptions and return null. + } } else { try { return KeyValueKeyStore.fromJson(json); - } catch (_) {} + } on Exception { + // Swallow deserialization exceptions and return null. + } } return null; diff --git a/packages/jwt/pubspec.yaml b/packages/jwt/pubspec.yaml index 7c1839d0..bfb3b780 100644 --- a/packages/jwt/pubspec.yaml +++ b/packages/jwt/pubspec.yaml @@ -10,7 +10,7 @@ dependencies: clock: ^1.1.0 collection: ^1.18.0 http: ^1.0.0 - json_annotation: ^4.4.0 + json_annotation: ^4.9.0 meta: ^1.7.0 pointycastle: ^3.5.0 rsa_pkcs: ^2.0.0 @@ -21,4 +21,4 @@ dev_dependencies: json_serializable: ^6.1.4 path: ^1.9.0 test: ^1.19.2 - very_good_analysis: ^6.0.0 + very_good_analysis: ^7.0.0 diff --git a/packages/jwt/test/src/jwt_test.dart b/packages/jwt/test/src/jwt_test.dart index 4228324c..c8c06203 100644 --- a/packages/jwt/test/src/jwt_test.dart +++ b/packages/jwt/test/src/jwt_test.dart @@ -1,4 +1,3 @@ -// ignore_for_file: prefer_const_constructors import 'dart:io'; import 'package:clock/clock.dart'; @@ -10,10 +9,14 @@ import 'package:test/test.dart'; void main() { const token = // cspell: disable-next-line '''eyJhbGciOiJSUzI1NiIsImtpZCI6ImMxMGM5MGJhNGMzNjYzNTE2ZTA3MDdkMGU5YTg5NDgxMDYyODUxNTgiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJodHRwczovL3NlY3VyZXRva2VuLmdvb2dsZS5jb20vbXktYXBwIiwiYXVkIjoibXktYXBwIiwiYXV0aF90aW1lIjoxNjQzNjg0MjY2LCJ1c2VyX2lkIjoiRzR1MzdXdk90dmVWR0pRb1pCWGpxcHVWazZWMiIsInN1YiI6Ikc0dTM3V3ZPdHZlVkdKUW9aQlhqcXB1Vms2VjIiLCJpYXQiOjE2NDM2ODQyNjYsImV4cCI6MTY0MzY4Nzg2NiwiZW1haWwiOiJ0ZXN0QGdtYWlsLmNvbSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlLCJmaXJlYmFzZSI6eyJpZGVudGl0aWVzIjp7ImVtYWlsIjpbInRlc3RAZ21haWwuY29tIl19LCJzaWduX2luX3Byb3ZpZGVyIjoicGFzc3dvcmQifX0.bUWnX_XmR1d9EmeFeYSsK_CHU1u9NPIHgyaQueZ6urYOtxvuL_QodjPl0c9CBJwctwPnxVyRmkeNCw0oF9xBgph0NApLL4FIG6vpDPZfW9txZBYr8xIvaqvmD0diACENAQdjRT2XmyEdQ2-U7SsTonybHmLoU9FMQTjAgw4NCALQvExfB6rtQ9GDsOBt1xoBkB3Vo7a5OmugZ1aHXF69b8As6137-Dggf5qx5R3oLRFovICMMesQziE3vGi-WKcbQxSeiD-9a6ShPAhk41XiyjFGDEOtUCQo63uwQnMw3g0KVtC6bzIyFq-E91vhxumxXzxPYC-kg7iUYiSZy7Y-Aw'''; + const tokenInvalidPayload = // cspell: disable-next-line + '''eyJhbGciOiJSUzI1NiIsImtpZCI6ImMxMGM5MGJhNGMzNjYzNTE2ZTA3MDdkMGU5YTg5NDgxMDYyODUxNTgiLCJ0eXAiOiJKV1QifQ.invalid.bUWnX_XmR1d9EmeFeYSsK_CHU1u9NPIHgyaQueZ6urYOtxvuL_QodjPl0c9CBJwctwPnxVyRmkeNCw0oF9xBgph0NApLL4FIG6vpDPZfW9txZBYr8xIvaqvmD0diACENAQdjRT2XmyEdQ2-U7SsTonybHmLoU9FMQTjAgw4NCALQvExfB6rtQ9GDsOBt1xoBkB3Vo7a5OmugZ1aHXF69b8As6137-Dggf5qx5R3oLRFovICMMesQziE3vGi-WKcbQxSeiD-9a6ShPAhk41XiyjFGDEOtUCQo63uwQnMw3g0KVtC6bzIyFq-E91vhxumxXzxPYC-kg7iUYiSZy7Y-Aw'''; const tokenNoAuthTime = // cspell: disable-next-line '''eyJhbGciOiJSUzI1NiIsImtpZCI6ImMxMGM5MGJhNGMzNjYzNTE2ZTA3MDdkMGU5YTg5NDgxMDYyODUxNTgiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJodHRwczovL3NlY3VyZXRva2VuLmdvb2dsZS5jb20vbXktYXBwIiwiYXVkIjoibXktYXBwIiwidXNlcl9pZCI6Ikc0dTM3V3ZPdHZlVkdKUW9aQlhqcXB1Vms2VjIiLCJzdWIiOiJHNHUzN1d2T3R2ZVZHSlFvWkJYanFwdVZrNlYyIiwiaWF0IjoxNjQzNjg0MjY2LCJleHAiOjE2NDM2ODc4NjYsImVtYWlsIjoidGVzdEBnbWFpbC5jb20iLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiZmlyZWJhc2UiOnsiaWRlbnRpdGllcyI6eyJlbWFpbCI6WyJ0ZXN0QGdtYWlsLmNvbSJdfSwic2lnbl9pbl9wcm92aWRlciI6InBhc3N3b3JkIn19.ZWCadE43mUk43cPQdNCCi4WhDgB4ZsDT9rhPGQq_1uFPhzkVrCSRcjUhwkzH11VLap_MVurNvI_pGWbu9Z4CRPvGFzXPpuNveWy2qFPEa4jcM-R40vsbrP30vNnrp4PrmqgLar0vWs6FZ2g9fbjU8L1LaU5ik31OKSXufTIKn_hPHhyIC33tYTpWzG3Abq3H9EELHUXKW9nEcN8YYnOHAZ3A6ymb3DyBguhf2O-XAIlrn1WoxRRqlukFGSmprk7heonbVUTzoc3sIDZcC-Cj1U9wTee1NmqmU7v3SvpBRGnuXz-5rzSRHblyVxn_EEfCYwjsDUwetYpyFcCs5dqPlQ'''; const tokenWithNoMatchingKid = // cspell: disable-next-line '''eyJhbGciOiJSUzI1NiIsImtpZCI6IjEyMzQiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJodHRwczovL3NlY3VyZXRva2VuLmdvb2dsZS5jb20vbXktYXBwIiwiYXVkIjoibXktYXBwIiwidXNlcl9pZCI6Ikc0dTM3V3ZPdHZlVkdKUW9aQlhqcXB1Vms2VjIiLCJzdWIiOiJHNHUzN1d2T3R2ZVZHSlFvWkJYanFwdVZrNlYyIiwiaWF0IjoxNjQzNjg0MjY2LCJleHAiOjE2NDM2ODc4NjYsImVtYWlsIjoidGVzdEBnbWFpbC5jb20iLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiZmlyZWJhc2UiOnsiaWRlbnRpdGllcyI6eyJlbWFpbCI6WyJ0ZXN0QGdtYWlsLmNvbSJdfSwic2lnbl9pbl9wcm92aWRlciI6InBhc3N3b3JkIn19.RBAwg-Ttf36aDkVR97Rd50aMp-0yzk8do_4AUPFi9nEhKu0Ye8ox_9hdTBttJYxoOq0NbH2zz0JHSnSHqoTvCleVhoGqg8YghzH0NiqncPfDzi-IcRfy2K8CrOoXuqXaj3YWrbrzNWAYV46eFmvI2TmPO55AyFOjhvLpW-uf96ceOPjueZm8o5K2DZym86BAhSdShknONV2O7b2vW34TXf3UJdISs5p9z6Si4JOWGjbVPY45CO16ODdDxyUGUM2-IVQloB7bg0nMqQbIjKDoO5g9d1nguR2Z-YBiv0BX1MDWDlrBsMAzzRVaFlf6YQS1LKxu2tnMdCrBnK_Wvqt5sQ'''; + const tokenInvalidSignature = // cspell: disable-next-line + '''eyJhbGciOiJSUzI1NiIsImtpZCI6ImMxMGM5MGJhNGMzNjYzNTE2ZTA3MDdkMGU5YTg5NDgxMDYyODUxNTgiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJodHRwczovL3NlY3VyZXRva2VuLmdvb2dsZS5jb20vbXktYXBwIiwiYXVkIjoibXktYXBwIiwiYXV0aF90aW1lIjoxNjQzNjg0MjY2LCJ1c2VyX2lkIjoiRzR1MzdXdk90dmVWR0pRb1pCWGpxcHVWazZWMiIsInN1YiI6Ikc0dTM3V3ZPdHZlVkdKUW9aQlhqcXB1Vms2VjIiLCJpYXQiOjE2NDM2ODQyNjYsImV4cCI6MTY0MzY4Nzg2NiwiZW1haWwiOiJ0ZXN0QGdtYWlsLmNvbSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlLCJmaXJlYmFzZSI6eyJpZGVudGl0aWVzIjp7ImVtYWlsIjpbInRlc3RAZ21haWwuY29tIl19LCJzaWduX2luX3Byb3ZpZGVyIjoicGFzc3dvcmQifX0.invalid-signature'''; const keyValuePublicKeysUrl = 'https://www.googleapis.com/robot/v1/metadata/x509/securetoken@system.gserviceaccount.com'; const jwkPublicKeysUrl = @@ -24,7 +27,7 @@ void main() { File(p.join('test', 'fixtures', 'key_value_key_store.json')) .readAsStringSync(); final expiresAt = DateTime.fromMillisecondsSinceEpoch(1643687866 * 1000); - final validTime = expiresAt.subtract(Duration(minutes: 15)); + final validTime = expiresAt.subtract(const Duration(minutes: 15)); late String keyStoreResponseBody; @@ -85,6 +88,45 @@ void main() { ); }); + test('throws a JwtVerificationFailure if payload is not valid', () async { + await expectLater( + () => verify( + tokenInvalidPayload, + audience: {audience}, + issuer: issuer, + publicKeysUrl: keyValuePublicKeysUrl, + ), + throwsA( + isA().having( + (e) => e.reason, + 'reason', + 'JWT payload is malformed.', + ), + ), + ); + }); + + test('throws a JwtVerificationFailure if signature is not valid', + () async { + await withClock(Clock.fixed(validTime), () async { + await expectLater( + () => verify( + tokenInvalidSignature, + audience: {audience}, + issuer: issuer, + publicKeysUrl: keyValuePublicKeysUrl, + ), + throwsA( + isA().having( + (e) => e.reason, + 'reason', + 'JWT signature is malformed.', + ), + ), + ); + }); + }); + test('throws exception if jwt has no matching public key id', () async { await withClock(Clock.fixed(validTime), () async { await expectLater( @@ -105,8 +147,9 @@ void main() { }); }); - test('throws exception if invalid keys are provided by the publicKeysUrl', - () async { + test( + 'throws exception if invalid keys are provided ' + 'by the publicKeysUrl (KeyValueKeyStore)', () async { getOverride = (Uri uri) async { return Response( '{"123": 456}', @@ -134,6 +177,36 @@ void main() { }); }); + test( + 'throws exception if invalid keys are provided ' + 'by the publicKeysUrl (JwkKeyStore)', () async { + getOverride = (Uri uri) async { + return Response( + '{"keys": 456}', + HttpStatus.ok, + headers: {'cache-control': 'max-age=3600'}, + ); + }; + + await withClock(Clock.fixed(validTime), () async { + await expectLater( + () => verify( + tokenWithNoMatchingKid, + audience: {audience}, + issuer: issuer, + publicKeysUrl: keyValuePublicKeysUrl, + ), + throwsA( + isA().having( + (e) => e.reason, + 'reason', + '''Invalid public keys returned by https://www.googleapis.com/robot/v1/metadata/x509/securetoken@system.gserviceaccount.com.''', + ), + ), + ); + }); + }); + test('can verify an invalid audience', () async { await withClock(Clock.fixed(validTime), () async { try { @@ -144,7 +217,7 @@ void main() { publicKeysUrl: keyValuePublicKeysUrl, ); fail('should throw'); - } catch (error) { + } on Exception catch (error) { expect( error, isA().having( @@ -167,7 +240,7 @@ void main() { publicKeysUrl: keyValuePublicKeysUrl, ); fail('should throw'); - } catch (error) { + } on Exception catch (error) { expect( error, isA().having( @@ -269,7 +342,7 @@ void main() { group('JwtVerificationFailure', () { test('toString is correct', () { const reason = 'reason'; - final failure = JwtVerificationFailure(reason); + const failure = JwtVerificationFailure(reason); expect(failure.toString(), equals('JwtVerificationFailure: $reason')); }); });