Fix Issues with verifyIdToken (#72)

Author: rrousselGit

.github/workflows/build.yml

@@ -43,6 +43,11 @@ jobs:
       - name: Analyze
         run: dart analyze
 
+      - run: mkdir $HOME/.config/gcloud -p
+      - run: echo $CREDS > $HOME/.config/gcloud/application_default_credentials.json
+        env:
+          CREDS: ${{ secrets.CREDS }}
+
       - name: Run tests
         run: ${{github.workspace}}/scripts/coverage.sh
 

.gitignore

@@ -2,6 +2,9 @@ firebase-debug.log
 ui-debug.log
 firestore-debug.log
 
+node_modules
+packages/dart_firebase_admin/test/client/package-lock.json
+
 build
 coverage
 

packages/dart_firebase_admin/lib/src/auth/token_verifier.dart

@@ -79,9 +79,7 @@ class FirebaseTokenVerifier {
       isEmulator: isEmulator,
     );
 
-    final decodedIdToken = DecodedIdToken.fromMap(decoded.payload);
-    decodedIdToken.uid = decodedIdToken.sub;
-    return decodedIdToken;
+    return DecodedIdToken.fromMap(decoded.payload);
   }
 
   Future<DecodedToken> _decodeAndVerify(
@@ -129,6 +127,13 @@ class FirebaseTokenVerifier {
     required bool isEmulator,
     String? audience,
   }) {
+    Never throws(String message) {
+      throw FirebaseAuthAdminException(
+        AuthClientErrorCode.invalidArgument,
+        message,
+      );
+    }
+
     final header = fullDecodedToken.header ?? <String, dynamic>{};
     final payload = fullDecodedToken.payload as Map;
 
@@ -141,7 +146,6 @@ class FirebaseTokenVerifier {
     late final alg = header['alg'];
     late final sub = payload['sub'];
 
-    String? errorMessage;
     if (!isEmulator && !header.containsKey('kid')) {
       final isCustomToken = (payload['aud'] == _firebaseAudience);
 
@@ -151,53 +155,55 @@ class FirebaseTokenVerifier {
           d is Map &&
           d.containsKey('uid');
 
+      String message;
       if (isCustomToken) {
-        errorMessage = '${tokenInfo.verifyApiName} expects $_shortNameArticle '
+        message = '${tokenInfo.verifyApiName} expects $_shortNameArticle '
             '${tokenInfo.shortName}, but was given a custom token.';
       } else if (isLegacyCustomToken) {
-        errorMessage = '${tokenInfo.verifyApiName} expects $_shortNameArticle '
+        message = '${tokenInfo.verifyApiName} expects $_shortNameArticle '
             '${tokenInfo.shortName}, but was given a legacy custom token.';
       } else {
-        errorMessage = '${tokenInfo.jwtName} has no "kid" claim.';
+        message = '${tokenInfo.jwtName} has no "kid" claim.';
       }
 
-      errorMessage += verifyJwtTokenDocsMessage;
+      throws(message);
     } else if (!isEmulator && alg != _algorithmRS256) {
-      errorMessage = '${tokenInfo.jwtName} has incorrect algorithm. '
+      throws('${tokenInfo.jwtName} has incorrect algorithm. '
           'Expected "$_algorithmRS256" but got "$alg".'
-          '$verifyJwtTokenDocsMessage';
+          '$verifyJwtTokenDocsMessage');
     } else if (audience != null &&
         !(payload['aud'] as String).contains(audience)) {
-      errorMessage =
-          '${tokenInfo.jwtName} has incorrect "aud" (audience) claim. '
-          'Expected "$audience" but got "${payload['aud']}".'
-          '$verifyJwtTokenDocsMessage';
+      throws(
+        '${tokenInfo.jwtName} has incorrect "aud" (audience) claim. '
+        'Expected "$audience" but got "${payload['aud']}".'
+        '$verifyJwtTokenDocsMessage',
+      );
     } else if (audience == null && payload['aud'] != projectId) {
-      errorMessage =
-          '${tokenInfo.jwtName} has incorrect "aud" (audience) claim. '
-          'Expected "$projectId" but got "${payload['aud']}".'
-          '$projectIdMatchMessage$verifyJwtTokenDocsMessage';
+      throws(
+        '${tokenInfo.jwtName} has incorrect "aud" (audience) claim. '
+        'Expected "$projectId" but got "${payload['aud']}".'
+        '$projectIdMatchMessage$verifyJwtTokenDocsMessage',
+      );
     } else if (payload['iss'] != '$issuer$projectId') {
-      errorMessage = '${tokenInfo.jwtName} has incorrect "iss" (issuer) claim. '
-          'Expected "$issuer$projectId" but got "${payload['iss']}".'
-          '$projectIdMatchMessage$verifyJwtTokenDocsMessage';
+      throws(
+        '${tokenInfo.jwtName} has incorrect "iss" (issuer) claim. '
+        'Expected "$issuer$projectId" but got "${payload['iss']}".'
+        '$projectIdMatchMessage$verifyJwtTokenDocsMessage',
+      );
     } else if (sub is! String) {
-      errorMessage = '${tokenInfo.jwtName} has no "sub" (subject) claim.'
-          '$verifyJwtTokenDocsMessage';
+      throws(
+        '${tokenInfo.jwtName} has no "sub" (subject) claim.'
+        '$verifyJwtTokenDocsMessage',
+      );
     } else if (sub.isEmpty) {
-      errorMessage =
-          '${tokenInfo.jwtName} has an empty string "sub" (subject) claim.'
-          '$verifyJwtTokenDocsMessage';
+      throws(
+        '${tokenInfo.jwtName} has an empty string "sub" (subject) claim.'
+        '$verifyJwtTokenDocsMessage',
+      );
     } else if (sub.length > 128) {
-      errorMessage =
-          '${tokenInfo.jwtName} has "sub" (subject) claim longer than 128 characters.'
-          '$verifyJwtTokenDocsMessage';
-    }
-
-    if (errorMessage != null) {
-      throw FirebaseAuthAdminException(
-        AuthClientErrorCode.invalidArgument,
-        errorMessage,
+      throws(
+        '${tokenInfo.jwtName} has "sub" (subject) claim longer than 128 characters.'
+        '$verifyJwtTokenDocsMessage',
       );
     }
   }
@@ -249,6 +255,14 @@ class TokenProvider {
     required this.tenant,
   });
 
+  @internal
+  TokenProvider.fromMap(Map<Object?, Object?> map)
+      : identities = Map.from(map['identities']! as Map),
+        signInProvider = map['sign_in_provider']! as String,
+        signInSecondFactor = map['sign_in_second_factor'] as String?,
+        secondFactorIdentifier = map['second_factor_identifier'] as String?,
+        tenant = map['tenant'] as String?;
+
   /// Provider-specific identity details corresponding
   /// to the provider used to sign in the user.
   Map<String, Object?> identities;
@@ -313,19 +327,13 @@ class DecodedIdToken {
       email: map['email'] as String?,
       emailVerified: map['email_verified'] as bool?,
       exp: map['exp']! as int,
-      firebase: TokenProvider(
-        identities: Map.from(map['firebase']! as Map),
-        signInProvider: map['sign_in_provider']! as String,
-        signInSecondFactor: map['sign_in_second_factor'] as String?,
-        secondFactorIdentifier: map['second_factor_identifier'] as String?,
-        tenant: map['tenant'] as String?,
-      ),
+      firebase: TokenProvider.fromMap(map['firebase']! as Map),
       iat: map['iat']! as int,
       iss: map['iss']! as String,
       phoneNumber: map['phone_number'] as String?,
       picture: map['picture'] as String?,
       sub: map['sub']! as String,
-      uid: map['uid']! as String,
+      uid: map['sub']! as String,
     );
   }
 

packages/dart_firebase_admin/lib/src/google_cloud_firestore/firestore.dart

@@ -191,7 +191,6 @@ class Firestore {
     final reader = _DocumentReader(
       firestore: this,
       documents: documents,
-      transactionId: null,
       fieldMask: fieldMask,
     );
 

packages/dart_firebase_admin/lib/src/google_cloud_firestore/transaction.dart

@@ -97,7 +97,6 @@ class Transaction {
     return _withLazyStartedTransaction<DocumentReference<T>,
         DocumentSnapshot<T>>(
       docRef,
-      fieldMask: null,
       resultFn: _getSingleFn,
     );
   }
@@ -168,8 +167,10 @@ class Transaction {
   /// A [Precondition] restricting this update.
   ///
   void update(
-      DocumentReference<dynamic> documentRef, Map<Object?, Object?> data,
-      {Precondition? precondition}) {
+    DocumentReference<dynamic> documentRef,
+    Map<Object?, Object?> data, {
+    Precondition? precondition,
+  }) {
     if (_writeBatch == null) {
       throw Exception(readOnlyWriteErrorMsg);
     }
@@ -188,8 +189,10 @@ class Transaction {
   ///
   /// A delete for a non-existing document is treated as a success (unless
   /// [precondition] is specified, in which case it throws a [FirebaseFirestoreAdminException] with [FirestoreClientErrorCode.notFound]).
-  void delete(DocumentReference<Map<String, dynamic>> documentRef,
-      {Precondition? precondition}) {
+  void delete(
+    DocumentReference<Map<String, dynamic>> documentRef, {
+    Precondition? precondition,
+  }) {
     if (_writeBatch == null) {
       throw Exception(readOnlyWriteErrorMsg);
     }
@@ -274,17 +277,22 @@ class Transaction {
       // response because we are not starting a new transaction
       return _transactionIdPromise!
           .then(
-            (transactionId) => resultFn(docRef,
-                transactionId: transactionId, fieldMask: fieldMask),
+            (transactionId) => resultFn(
+              docRef,
+              transactionId: transactionId,
+              fieldMask: fieldMask,
+            ),
           )
           .then((r) => r.result);
     } else {
       if (_readOnlyReadTime != null) {
         // We do not start a transaction for read-only transactions
         // do not set _prevTransactionId
-        return resultFn(docRef,
-                readTime: _readOnlyReadTime, fieldMask: fieldMask)
-            .then((r) => r.result);
+        return resultFn(
+          docRef,
+          readTime: _readOnlyReadTime,
+          fieldMask: fieldMask,
+        ).then((r) => r.result);
       } else {
         // This is the first read of the transaction so we create the appropriate
         // options for lazily starting the transaction inside this first read op
@@ -341,7 +349,9 @@ class Transaction {
     );
     final result = await reader._get();
     return _TransactionResult(
-        transaction: result.transaction, result: result.result.single);
+      transaction: result.transaction,
+      result: result.result.single,
+    );
   }
 
   Future<_TransactionResult<List<DocumentSnapshot<T>>>> _getBatchFn<T>(
@@ -362,7 +372,9 @@ class Transaction {
 
     final result = await reader._get();
     return _TransactionResult(
-        transaction: result.transaction, result: result.result);
+      transaction: result.transaction,
+      result: result.result,
+    );
   }
 
   Future<T> _runTransaction<T>(

packages/dart_firebase_admin/lib/src/utils/jwt.dart

@@ -9,12 +9,15 @@ class EmulatorSignatureVerifier implements SignatureVerifier {
   @override
   Future<void> verify(String token) async {
     // Signature checks skipped for emulator; no need to fetch public keys.
+
     try {
       verifyJwtSignature(
         token,
         SecretKey(''),
       );
     } on JWTInvalidException catch (e) {
+      // Emulator tokens have "alg": "none"
+      if (e.message == 'unknown algorithm') return;
       if (e.message == 'invalid signature') return;
       rethrow;
     }
@@ -122,11 +125,23 @@ class PublicKeySignatureVerifier implements SignatureVerifier {
           'no-matching-kid-error',
         );
       }
-      verifyJwtSignature(
-        token,
-        RSAPublicKey.cert(publicKey),
-        issueAt: Duration.zero, // Any past date should be valid
-      );
+
+      try {
+        verifyJwtSignature(
+          token,
+          RSAPublicKey.cert(publicKey),
+          issueAt: Duration.zero, // Any past date should be valid
+        );
+      } catch (e, stackTrace) {
+        Error.throwWithStackTrace(
+          JwtError(
+            JwtErrorCode.invalidSignature,
+            'Error while verifying signature of Firebase ID token: $e',
+          ),
+          stackTrace,
+        );
+      }
+
       // At this point most JWTException's should have been caught in
       // verifyJwtSignature, but we could still get some from JWT.decode above
     } on JWTException catch (e) {
@@ -169,14 +184,6 @@ void verifyJwtSignature(
       ),
       stackTrace,
     );
-  } catch (e, stackTrace) {
-    Error.throwWithStackTrace(
-      JwtError(
-        JwtErrorCode.invalidSignature,
-        'Error while verifying signature of Firebase ID token: $e',
-      ),
-      stackTrace,
-    );
   }
 }
 

packages/dart_firebase_admin/pubspec.yaml

@@ -24,6 +24,7 @@ dev_dependencies:
   file: ^7.0.0
   freezed: ^2.4.2
   mocktail: ^1.0.1
+  path: ^1.9.1
   test: ^1.24.4
   uuid: ^4.0.0