Skip to content

chore: Move token verification logic to util #1189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

chore: Move token verification logic to util #1189

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8044f7d
chore: Move Token verification logic to util
lahirumaramba Mar 8, 2021
6719cfa
Moved Auth related items to token-verifier-util
lahirumaramba Mar 9, 2021
913ce37
Change the value errors in FirebaseTokenVerifier constructor to Error…
lahirumaramba Mar 9, 2021
bba0b02
Move the error type test into verifyJWT() tests
lahirumaramba Mar 9, 2021
27e0e6f
Merge expired error code to erro code config
lahirumaramba Mar 10, 2021
5d3597a
Fixed copyright years
lahirumaramba Mar 10, 2021
ac28a55
Remove all Auth dependencies from the token verifier
lahirumaramba Mar 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/auth/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import * as utils from '../utils/index';
import * as validator from '../utils/validator';
import { auth } from './index';
import { FirebaseTokenVerifier } from '../utils/token-verifier';
import { createSessionCookieVerifier, createIdTokenVerifier } from './token-verifier-util';
import { createSessionCookieVerifier, createIdTokenVerifier } from './token-verifier';
import {
SAMLConfig, OIDCConfig, OIDCConfigServerResponse, SAMLConfigServerResponse,
} from './auth-config';
Expand Down
21 changes: 14 additions & 7 deletions src/auth/token-verifier-util.ts → src/auth/token-verifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*!
* Copyright 2021 Google Inc.
* Copyright 2018 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -27,11 +27,20 @@ const CLIENT_CERT_URL = 'https://www.googleapis.com/robot/v1/metadata/x509/secur
// URL containing the public keys for Firebase session cookies. This will be updated to a different URL soon.
const SESSION_COOKIE_CERT_URL = 'https://www.googleapis.com/identitytoolkit/v3/relyingparty/publicKeys';

/** Error codes that matches the FirebaseAuthError type */
const AUTH_ERROR_CODE_CONFIG: ErrorCodeConfig = {
/** Matching Auth error code config for ID token */
export const ID_TOKEN_ERROR_CODE_CONFIG: ErrorCodeConfig = {
invalidArg: AuthClientErrorCode.INVALID_ARGUMENT,
invalidCredential: AuthClientErrorCode.INVALID_CREDENTIAL,
internalError: AuthClientErrorCode.INTERNAL_ERROR,
expiredError: AuthClientErrorCode.ID_TOKEN_EXPIRED,
}

/** Matching Auth error code config for session cookie */
export const SESSION_COOKIE_ERROR_CODE_CONFIG: ErrorCodeConfig = {
invalidArg: AuthClientErrorCode.INVALID_ARGUMENT,
invalidCredential: AuthClientErrorCode.INVALID_CREDENTIAL,
internalError: AuthClientErrorCode.INTERNAL_ERROR,
expiredError: AuthClientErrorCode.SESSION_COOKIE_EXPIRED,
}

/** User facing token information related to the Firebase ID token. */
Expand All @@ -40,8 +49,7 @@ export const ID_TOKEN_INFO: FirebaseTokenInfo = {
verifyApiName: 'verifyIdToken()',
jwtName: 'Firebase ID token',
shortName: 'ID token',
expiredErrorCode: AuthClientErrorCode.ID_TOKEN_EXPIRED,
errorCodeConfig: AUTH_ERROR_CODE_CONFIG,
errorCodeConfig: ID_TOKEN_ERROR_CODE_CONFIG,
errorType: FirebaseAuthError,
};

Expand All @@ -51,8 +59,7 @@ export const SESSION_COOKIE_INFO: FirebaseTokenInfo = {
verifyApiName: 'verifySessionCookie()',
jwtName: 'Firebase session cookie',
shortName: 'session cookie',
expiredErrorCode: AuthClientErrorCode.SESSION_COOKIE_EXPIRED,
errorCodeConfig: AUTH_ERROR_CODE_CONFIG,
errorCodeConfig: SESSION_COOKIE_ERROR_CODE_CONFIG,
errorType: FirebaseAuthError,
};

Expand Down
1 change: 1 addition & 0 deletions src/utils/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface ErrorCodeConfig {
invalidArg: ErrorInfo;
invalidCredential: ErrorInfo;
internalError: ErrorInfo;
expiredError: ErrorInfo;
}

/**
Expand Down
19 changes: 11 additions & 8 deletions src/utils/token-verifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*!
* Copyright 2018 Google Inc.
* Copyright 2021 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -37,8 +37,6 @@ export interface FirebaseTokenInfo {
jwtName: string;
/** The JWT short name. */
shortName: string;
/** JWT Expiration error code. */
expiredErrorCode: ErrorInfo;
/** Error code config of the public error type. */
errorCodeConfig: ErrorCodeConfig;
/** Public error type. */
Expand Down Expand Up @@ -73,9 +71,14 @@ export class FirebaseTokenVerifier {
throw new Error('The JWT public full name must be a non-empty string.');
} else if (!validator.isNonEmptyString(tokenInfo.shortName)) {
throw new Error('The JWT public short name must be a non-empty string.');
} else if (!validator.isNonNullObject(tokenInfo.expiredErrorCode) ||
!('code' in tokenInfo.expiredErrorCode)) {
throw new Error('The JWT expiration error code must be a non-null ErrorInfo object.');
} else if (!(typeof tokenInfo.errorType === 'function' && tokenInfo.errorType !== null)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typeof errorType === 'function' also asserts that errorType !== null. So the 2nd part of the conjunction is redundant.

throw new Error('The provided error type must be a non-null PrefixedFirebaseError type.');
} else if (!validator.isNonNullObject(tokenInfo.errorCodeConfig) ||
!('invalidArg' in tokenInfo.errorCodeConfig ||
'invalidCredential' in tokenInfo.errorCodeConfig ||
'internalError' in tokenInfo.errorCodeConfig ||
'expiredError' in tokenInfo.errorCodeConfig)) {
throw new Error('The provided error code config must be a non-null ErrorCodeInfo object.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ErrorCodeConfig?

}
this.shortNameArticle = tokenInfo.shortName.charAt(0).match(/[aeiou]/i) ? 'an' : 'a';

Expand Down Expand Up @@ -213,9 +216,9 @@ export class FirebaseTokenVerifier {
if (error) {
if (error.name === 'TokenExpiredError') {
const errorMessage = `${this.tokenInfo.jwtName} has expired. Get a fresh ${this.tokenInfo.shortName}` +
` from your client app and try again (auth/${this.tokenInfo.expiredErrorCode.code}).` +
` from your client app and try again (auth/${this.tokenInfo.errorCodeConfig.expiredError.code}).` +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a reference to auth here. Should probably get rid of it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good catch! I actually ended up finding more references to Auth. Ended up using generics in the verify token functions and moved the code to set the user id alias to one level up to auth.ts.
Let me know what you think. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: the error code prefix. I just removed the auth prefix, but the error code feels a bit redundant here as the error code will also be included in the public error type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something that will get cleaned up once we fix error handling for this SDK. For now this solution is adequate.

verifyJwtTokenDocsMessage;
return reject(new this.tokenInfo.errorType(this.tokenInfo.expiredErrorCode, errorMessage));
return reject(new this.tokenInfo.errorType(this.tokenInfo.errorCodeConfig.expiredError, errorMessage));
} else if (error.name === 'JsonWebTokenError') {
const errorMessage = `${this.tokenInfo.jwtName} has invalid signature.` + verifyJwtTokenDocsMessage;
return reject(new this.tokenInfo.errorType(this.tokenInfo.errorCodeConfig.invalidArg, errorMessage));
Expand Down
97 changes: 51 additions & 46 deletions test/unit/auth/token-verifier.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ import LegacyFirebaseTokenGenerator = require('firebase-token-generator');
import * as mocks from '../../resources/mocks';
import { FirebaseTokenGenerator, ServiceAccountSigner } from '../../../src/auth/token-generator';
import * as verifier from '../../../src/utils/token-verifier';
import * as verifierUtil from '../../../src/auth/token-verifier-util';
import * as verifierAuth from '../../../src/auth/token-verifier';

import { ServiceAccountCredential } from '../../../src/credential/credential-internal';
import { AuthClientErrorCode, ErrorCodeConfig, FirebaseAuthError } from '../../../src/utils/error';
import { FirebaseAuthError } from '../../../src/utils/error';
import { FirebaseApp } from '../../../src/firebase-app';
import { Algorithm } from 'jsonwebtoken';

Expand All @@ -46,12 +46,6 @@ const expect = chai.expect;
const ONE_HOUR_IN_SECONDS = 60 * 60;
const idTokenPublicCertPath = '/robot/v1/metadata/x509/[email protected]';

const AUTH_ERROR_CODE_CONFIG: ErrorCodeConfig = {
invalidArg: AuthClientErrorCode.INVALID_ARGUMENT,
invalidCredential: AuthClientErrorCode.INVALID_CREDENTIAL,
internalError: AuthClientErrorCode.INTERNAL_ERROR,
}

/**
* Returns a mocked out success response from the URL containing the public keys for the Google certs.
*
Expand Down Expand Up @@ -121,7 +115,7 @@ function createTokenVerifier(
'https://www.googleapis.com/robot/v1/metadata/x509/[email protected]',
algorithm,
'https://securetoken.google.com/',
verifierUtil.ID_TOKEN_INFO,
verifierAuth.ID_TOKEN_INFO,
app
);
}
Expand Down Expand Up @@ -166,8 +160,7 @@ describe('FirebaseTokenVerifier', () => {
verifyApiName: 'verifyToken()',
jwtName: 'Important Token',
shortName: 'token',
expiredErrorCode: AuthClientErrorCode.INVALID_ARGUMENT,
errorCodeConfig: AUTH_ERROR_CODE_CONFIG,
errorCodeConfig: verifierAuth.ID_TOKEN_ERROR_CODE_CONFIG,
errorType: FirebaseAuthError,
},
app,
Expand All @@ -183,7 +176,7 @@ describe('FirebaseTokenVerifier', () => {
invalidCertUrl as any,
'RS256',
'https://www.example.com/issuer/',
verifierUtil.ID_TOKEN_INFO,
verifierAuth.ID_TOKEN_INFO,
app,
);
}).to.throw('The provided public client certificate URL is an invalid URL.');
Expand All @@ -198,7 +191,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.example.com/publicKeys',
invalidAlgorithm as any,
'https://www.example.com/issuer/',
verifierUtil.ID_TOKEN_INFO,
verifierAuth.ID_TOKEN_INFO,
app);
}).to.throw('The provided JWT algorithm is an empty string.');
});
Expand All @@ -212,7 +205,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.example.com/publicKeys',
'RS256',
invalidIssuer as any,
verifierUtil.ID_TOKEN_INFO,
verifierAuth.ID_TOKEN_INFO,
app,
);
}).to.throw('The provided JWT issuer is an invalid URL.');
Expand All @@ -232,8 +225,7 @@ describe('FirebaseTokenVerifier', () => {
verifyApiName: invalidVerifyApiName as any,
jwtName: 'Important Token',
shortName: 'token',
expiredErrorCode: AuthClientErrorCode.INVALID_ARGUMENT,
errorCodeConfig: AUTH_ERROR_CODE_CONFIG,
errorCodeConfig: verifierAuth.ID_TOKEN_ERROR_CODE_CONFIG,
errorType: FirebaseAuthError,
},
app,
Expand All @@ -255,8 +247,7 @@ describe('FirebaseTokenVerifier', () => {
verifyApiName: 'verifyToken()',
jwtName: invalidJwtName as any,
shortName: 'token',
expiredErrorCode: AuthClientErrorCode.INVALID_ARGUMENT,
errorCodeConfig: AUTH_ERROR_CODE_CONFIG,
errorCodeConfig: verifierAuth.ID_TOKEN_ERROR_CODE_CONFIG,
errorType: FirebaseAuthError,
},
app,
Expand All @@ -278,8 +269,7 @@ describe('FirebaseTokenVerifier', () => {
verifyApiName: 'verifyToken()',
jwtName: 'Important Token',
shortName: invalidShortName as any,
expiredErrorCode: AuthClientErrorCode.INVALID_ARGUMENT,
errorCodeConfig: AUTH_ERROR_CODE_CONFIG,
errorCodeConfig: verifierAuth.ID_TOKEN_ERROR_CODE_CONFIG,
errorType: FirebaseAuthError,
},
app,
Expand All @@ -288,9 +278,9 @@ describe('FirebaseTokenVerifier', () => {
});
});

const invalidExpiredErrorCodes = [null, NaN, 0, 1, true, false, [], {}, { a: 1 }, _.noop, '', 'test'];
invalidExpiredErrorCodes.forEach((invalidExpiredErrorCode) => {
it('should throw given an invalid expiration error code: ' + JSON.stringify(invalidExpiredErrorCode), () => {
const invalidErrorCodeTypes = [null, NaN, 0, 1, true, false, [], {}, { a: 1 }, _.noop, '', 'test'];
invalidErrorCodeTypes.forEach((invalidErrorCodeType) => {
it('should throw given an invalid error code config: ' + JSON.stringify(invalidErrorCodeType), () => {
expect(() => {
new verifier.FirebaseTokenVerifier(
'https://www.example.com/publicKeys',
Expand All @@ -301,13 +291,34 @@ describe('FirebaseTokenVerifier', () => {
verifyApiName: 'verifyToken()',
jwtName: 'Important Token',
shortName: 'token',
expiredErrorCode: invalidExpiredErrorCode as any,
errorCodeConfig: AUTH_ERROR_CODE_CONFIG,
errorCodeConfig: verifierAuth.ID_TOKEN_ERROR_CODE_CONFIG,
errorType: invalidErrorCodeTypes as any,
},
app,
);
}).to.throw('The provided error type must be a non-null PrefixedFirebaseError type.');
});
});

const invalidErrorCodeConfigs = [null, NaN, 0, 1, true, false, [], {}, { a: 1 }, _.noop, '', 'test'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests now belong in utils.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the tests to test/utils

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not move the whole file. It still contains many auth-specific tests.

invalidErrorCodeConfigs.forEach((invalidErrorCodeConfig) => {
it('should throw given an invalid error code config: ' + JSON.stringify(invalidErrorCodeConfig), () => {
expect(() => {
new verifier.FirebaseTokenVerifier(
'https://www.example.com/publicKeys',
'RS256',
'https://www.example.com/issuer/',
{
url: 'https://docs.example.com/verify-tokens',
verifyApiName: 'verifyToken()',
jwtName: 'Important Token',
shortName: 'token',
errorCodeConfig: invalidErrorCodeConfig as any,
errorType: FirebaseAuthError,
},
app,
);
}).to.throw('The JWT expiration error code must be a non-null ErrorInfo object.');
}).to.throw('The provided error code config must be a non-null ErrorCodeInfo object.');
});
});
});
Expand All @@ -326,10 +337,9 @@ describe('FirebaseTokenVerifier', () => {
}).to.throw('First argument to verifyIdToken() must be a Firebase ID token');
});

const errorTypes = [
{ type: FirebaseAuthError, config: AUTH_ERROR_CODE_CONFIG },
];
errorTypes.forEach((errorType) => {
it('should throw with the correct error type and code set in token info', () => {
const errorType = FirebaseAuthError;
const errorCodeConfig = verifierAuth.ID_TOKEN_ERROR_CODE_CONFIG;
const tokenVerifier = new verifier.FirebaseTokenVerifier(
'https://www.example.com/publicKeys',
'RS256',
Expand All @@ -339,19 +349,14 @@ describe('FirebaseTokenVerifier', () => {
verifyApiName: 'verifyToken()',
jwtName: 'Important Token',
shortName: 'token',
expiredErrorCode: errorType.config.invalidArg,
errorCodeConfig: errorType.config,
errorType: errorType.type,
errorCodeConfig: errorCodeConfig,
errorType,
},
app,
);
it('should throw with the correct error type and code set in token info', () => {
expect(() => {
(tokenVerifier as any).verifyJWT();
}).to.throw(errorType.type).with.property('code').match(
new RegExp(`(.*)/${errorType.config.invalidArg.code}`)
);
});
expect(() => {
(tokenVerifier as any).verifyJWT();
}).to.throw(errorType).with.property('code').equal(`auth/${errorCodeConfig.invalidArg.code}`);
});

const invalidIdTokens = [null, NaN, 0, 1, true, false, [], {}, { a: 1 }, _.noop];
Expand All @@ -378,7 +383,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.googleapis.com/robot/v1/metadata/x509/[email protected]',
'RS256',
'https://securetoken.google.com/',
verifierUtil.ID_TOKEN_INFO,
verifierAuth.ID_TOKEN_INFO,
mocks.mockCredentialApp(),
);
const mockIdToken = mocks.generateIdToken();
Expand Down Expand Up @@ -485,7 +490,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.googleapis.com/identitytoolkit/v3/relyingparty/publicKeys',
'RS256',
'https://session.firebase.google.com/',
verifierUtil.SESSION_COOKIE_INFO,
verifierAuth.SESSION_COOKIE_INFO,
app,
);
mockedRequests.push(mockFetchPublicKeys('/identitytoolkit/v3/relyingparty/publicKeys'));
Expand Down Expand Up @@ -530,7 +535,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.googleapis.com/identitytoolkit/v3/relyingparty/publicKeys',
'RS256',
'https://session.firebase.google.com/',
verifierUtil.SESSION_COOKIE_INFO,
verifierAuth.SESSION_COOKIE_INFO,
app,
);
return tokenGenerator.createCustomToken(mocks.uid)
Expand All @@ -556,7 +561,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.googleapis.com/identitytoolkit/v3/relyingparty/publicKeys',
'RS256',
'https://session.firebase.google.com/',
verifierUtil.SESSION_COOKIE_INFO,
verifierAuth.SESSION_COOKIE_INFO,
app,
);
const legacyTokenGenerator = new LegacyFirebaseTokenGenerator('foo');
Expand Down Expand Up @@ -649,7 +654,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.googleapis.com/robot/v1/metadata/x509/[email protected]',
'RS256',
'https://securetoken.google.com/',
verifierUtil.ID_TOKEN_INFO,
verifierAuth.ID_TOKEN_INFO,
appWithAgent,
);
mockedRequests.push(mockFetchPublicKeys());
Expand All @@ -672,7 +677,7 @@ describe('FirebaseTokenVerifier', () => {
'https://www.googleapis.com/robot/v1/metadata/x509/[email protected]',
'RS256',
'https://securetoken.google.com/',
verifierUtil.ID_TOKEN_INFO,
verifierAuth.ID_TOKEN_INFO,
app,
);
expect(https.request).not.to.have.been.called;
Expand Down