Enable 'strictFunctionTypes' ts compiler option. (#675)

Author: rsgowman

src/auth/token-verifier.ts

@@ -19,6 +19,7 @@ import {AuthClientErrorCode, FirebaseAuthError, ErrorInfo} from '../utils/error'
 import * as validator from '../utils/validator';
 import * as jwt from 'jsonwebtoken';
 import { HttpClient, HttpRequestConfig, HttpError } from '../utils/api-request';
+import { DecodedIdToken } from './auth';
 
 // Audience to use for Firebase Auth Custom tokens
 const FIREBASE_AUDIENCE = 'https://identitytoolkit.googleapis.com/google.identity.identitytoolkit.v1.IdentityToolkit';
@@ -130,10 +131,10 @@ export class FirebaseTokenVerifier {
    * Verifies the format and signature of a Firebase Auth JWT token.
    *
    * @param {string} jwtToken The Firebase Auth JWT token to verify.
-   * @return {Promise<object>} A promise fulfilled with the decoded claims of the Firebase Auth ID
+   * @return {Promise<DecodedIdToken>} A promise fulfilled with the decoded claims of the Firebase Auth ID
    *                           token.
    */
-  public verifyJWT(jwtToken: string): Promise<object> {
+  public verifyJWT(jwtToken: string): Promise<DecodedIdToken> {
     if (!validator.isString(jwtToken)) {
       throw new FirebaseAuthError(
         AuthClientErrorCode.INVALID_ARGUMENT,
@@ -224,16 +225,16 @@ export class FirebaseTokenVerifier {
    * Verifies the JWT signature using the provided public key.
    * @param {string} jwtToken The JWT token to verify.
    * @param {string} publicKey The public key certificate.
-   * @return {Promise<object>} A promise that resolves with the decoded JWT claims on successful
+   * @return {Promise<DecodedIdToken>} A promise that resolves with the decoded JWT claims on successful
    *     verification.
    */
-  private verifyJwtSignatureWithKey(jwtToken: string, publicKey: string): Promise<object> {
+  private verifyJwtSignatureWithKey(jwtToken: string, publicKey: string): Promise<DecodedIdToken> {
     const verifyJwtTokenDocsMessage = ` See ${this.tokenInfo.url} ` +
       `for details on how to retrieve ${this.shortNameArticle} ${this.tokenInfo.shortName}.`;
     return new Promise((resolve, reject) => {
       jwt.verify(jwtToken, publicKey, {
         algorithms: [this.algorithm],
-      }, (error: jwt.VerifyErrors, decodedToken: any) => {
+      }, (error: jwt.VerifyErrors, decodedToken: string | object) => {
         if (error) {
           if (error.name === 'TokenExpiredError') {
             const errorMessage = `${this.tokenInfo.jwtName} has expired. Get a fresh ${this.tokenInfo.shortName}` +
@@ -246,8 +247,19 @@ export class FirebaseTokenVerifier {
           }
           return reject(new FirebaseAuthError(AuthClientErrorCode.INVALID_ARGUMENT, error.message));
         } else {
-          decodedToken.uid = decodedToken.sub;
-          resolve(decodedToken);
+          // TODO(rsgowman): I think the typing on jwt.verify is wrong. It claims that this can be either a string or an
+          // object, but the code always seems to call it as an object. Investigate and upstream typing changes if this
+          // is actually correct.
+          if (typeof decodedToken === 'string') {
+            return reject(new FirebaseAuthError(
+                AuthClientErrorCode.INTERNAL_ERROR,
+                "Unexpected decodedToken. Expected an object but got a string: '" + decodedToken + "'",
+            ));
+          } else {
+            const decodedIdToken = (decodedToken as DecodedIdToken);
+            decodedIdToken.uid = decodedIdToken.sub;
+            resolve(decodedIdToken);
+          }
         }
       });
     });

src/messaging/messaging.ts

@@ -21,7 +21,7 @@ import {
   Message, validateMessage, MessagingDevicesResponse,
   MessagingDeviceGroupResponse, MessagingTopicManagementResponse,
   MessagingPayload, MessagingOptions, MessagingTopicResponse,
-  MessagingConditionResponse, BatchResponse, MulticastMessage,
+  MessagingConditionResponse, BatchResponse, MulticastMessage, DataMessagePayload, NotificationMessagePayload,
 } from './messaging-types';
 import {FirebaseMessagingRequestHandler} from './messaging-api-request';
 import {FirebaseServiceInterface, FirebaseServiceInternalsInterface} from '../firebase-service';
@@ -758,9 +758,7 @@ export class Messaging implements FirebaseServiceInterface {
       );
     }
 
-    payloadKeys.forEach((payloadKey: keyof MessagingPayload) => {
-      const value = payloadCopy[payloadKey];
-
+    const validatePayload = (payloadKey: string, value: DataMessagePayload | NotificationMessagePayload) => {
       // Validate each top-level key in the payload is an object
       if (!validator.isNonNullObject(value)) {
         throw new FirebaseMessagingError(
@@ -786,7 +784,14 @@ export class Messaging implements FirebaseServiceInterface {
           );
         }
       });
-    });
+    };
+
+    if (payloadCopy.data !== undefined) {
+      validatePayload('data', payloadCopy.data);
+    }
+    if (payloadCopy.notification !== undefined) {
+      validatePayload('notification', payloadCopy.notification);
+    }
 
     // Validate the data payload object does not contain blacklisted properties
     if ('data' in payloadCopy) {

test/unit/messaging/messaging.spec.ts

@@ -28,7 +28,7 @@ import * as mocks from '../../resources/mocks';
 
 import {FirebaseApp} from '../../../src/firebase-app';
 import {
-  Message, MessagingOptions, MessagingPayload, MessagingDevicesResponse,
+  Message, MessagingOptions, MessagingPayload, MessagingDevicesResponse, MessagingDeviceGroupResponse,
   MessagingTopicManagementResponse, BatchResponse, SendResponse, MulticastMessage,
 } from '../../../src/messaging/messaging-types';
 import {
@@ -1309,10 +1309,11 @@ describe('Messaging', () => {
           mocks.messaging.registrationToken + '2',
         ],
         mocks.messaging.payload,
-      ).then((response: MessagingDevicesResponse) => {
+      ).then((response: MessagingDevicesResponse | MessagingDeviceGroupResponse) => {
         expect(response).to.have.keys([
           'failureCount', 'successCount', 'canonicalRegistrationTokenCount', 'multicastId', 'results',
         ]);
+        response = response as MessagingDevicesResponse;
         expect(response.failureCount).to.equal(2);
         expect(response.successCount).to.equal(1);
         expect(response.canonicalRegistrationTokenCount).to.equal(1);
@@ -2084,19 +2085,19 @@ describe('Messaging', () => {
     invalidPayloads.forEach((invalidPayload) => {
       it(`should throw given invalid type for payload argument: ${ JSON.stringify(invalidPayload) }`, () => {
         expect(() => {
-          messaging.sendToDevice(mocks.messaging.registrationToken, invalidPayload as MessagingPayload);
+          messaging.sendToDevice(mocks.messaging.registrationToken, invalidPayload as any);
         }).to.throw('Messaging payload must be an object');
 
         expect(() => {
-          messaging.sendToDeviceGroup(mocks.messaging.notificationKey, invalidPayload as MessagingPayload);
+          messaging.sendToDeviceGroup(mocks.messaging.notificationKey, invalidPayload as any);
         }).to.throw('Messaging payload must be an object');
 
         expect(() => {
-          messaging.sendToTopic(mocks.messaging.topic, invalidPayload as MessagingPayload);
+          messaging.sendToTopic(mocks.messaging.topic, invalidPayload as any);
         }).to.throw('Messaging payload must be an object');
 
         expect(() => {
-          messaging.sendToCondition(mocks.messaging.condition, invalidPayload as MessagingPayload);
+          messaging.sendToCondition(mocks.messaging.condition, invalidPayload as any);
         }).to.throw('Messaging payload must be an object');
       });
     });

tsconfig.json

@@ -9,7 +9,7 @@
     "alwaysStrict": true,
     "strictBindCallApply": true,
     //"strictNullChecks": true,
-    //"strictFunctionTypes": true,
+    "strictFunctionTypes": true,
     //"strictPropertyInitialization": true,
     "lib": ["es2015"],
     "outDir": "lib",