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 3 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
8 changes: 5 additions & 3 deletions 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 Expand Up @@ -116,8 +116,9 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
*/
public verifyIdToken(idToken: string, checkRevoked = false): Promise<DecodedIdToken> {
const isEmulator = useEmulator();
return this.idTokenVerifier.verifyJWT(idToken, isEmulator)
return this.idTokenVerifier.verifyJWT<DecodedIdToken>(idToken, isEmulator)
.then((decodedIdToken: DecodedIdToken) => {
decodedIdToken.uid = decodedIdToken.sub;
// Whether to check if the token was revoked.
if (checkRevoked || isEmulator) {
return this.verifyDecodedJWTNotRevoked(
Expand Down Expand Up @@ -519,8 +520,9 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
public verifySessionCookie(
sessionCookie: string, checkRevoked = false): Promise<DecodedIdToken> {
const isEmulator = useEmulator();
return this.sessionCookieVerifier.verifyJWT(sessionCookie, isEmulator)
return this.sessionCookieVerifier.verifyJWT<DecodedIdToken>(sessionCookie, isEmulator)
.then((decodedIdToken: DecodedIdToken) => {
decodedIdToken.uid = decodedIdToken.sub;
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 code to set the uid alias here. Another way is to add helper functions, verifySessionCookie() and verifyIdToken(), to auth/token-verifier. Let me know your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about extending the util.TokenVerifier as AuthTokenVerifier, and add the logic there?

// Whether to check if the token was revoked.
if (checkRevoked || isEmulator) {
return this.verifyDecodedJWTNotRevoked(
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
54 changes: 28 additions & 26 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 All @@ -20,9 +20,6 @@ import * as jwt from 'jsonwebtoken';
import { HttpClient, HttpRequestConfig, HttpError } from './api-request';
import { FirebaseApp } from '../firebase-app';
import { ErrorCodeConfig, ErrorInfo, PrefixedFirebaseError } from './error';
import { auth } from '../auth/index';

import DecodedIdToken = auth.DecodedIdToken;

// Audience to use for Firebase Auth Custom tokens
const FIREBASE_AUDIENCE = 'https://identitytoolkit.googleapis.com/google.identity.identitytoolkit.v1.IdentityToolkit';
Expand All @@ -37,8 +34,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,24 +68,30 @@ 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';

// For backward compatibility, the project ID is validated in the verification call.
}

/**
* Verifies the format and signature of a Firebase Auth JWT token.
* Verifies the format and signature of a Firebase JWT token.
*
* @param {string} jwtToken The Firebase Auth JWT token to verify.
* @param {boolean=} isEmulator Whether to accept Auth Emulator tokens.
* @return {Promise<DecodedIdToken>} A promise fulfilled with the decoded claims of the Firebase Auth ID
* @template T The returned type of the decoded token.
* @param {string} jwtToken The Firebase JWT token to verify.
* @param {boolean=} isEmulator Whether to accept Emulator tokens.
* @return {Promise<T>} A promise fulfilled with the decoded claims of the Firebase Auth ID
* token.
*/
public verifyJWT(jwtToken: string, isEmulator = false): Promise<DecodedIdToken> {
public verifyJWT<T extends object>(jwtToken: string, isEmulator = false): Promise<T> {
Copy link
Member Author

@lahirumaramba lahirumaramba Mar 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@hiranya911 :
To make the type check tighter we can introduce a base type for returned tokens...

interface FirebaseToken {
  // registered JWT claims
  iss: string
  aud: string
}

interface DecodedIdToken extends FirebaseToken {
...
}

public verifyJWT<T extends FirebaseToken>

Let me know your thoughts. Thanks!

Copy link
Contributor

@hiranya911 hiranya911 Mar 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, this looks good to me. But I don't think we need a generic at all. Just make auth extend the TokenVerifier class, and transform the FirebaseToken into DecodedIdToken there.

// in utils
interface DecodedToken {
  iss: string;
  aud: string;
  sub: string;
}

// in auth
interface DecodedIdToken {
  iss: string;
  aud: string;
  sub: string;
  uid: string;
}

// at callsite
const decodedToken: DecodedToken = await tokenVerifier.verifyToken();
const result: DecodedIdToken = {...decodedToken, uid: decodedToken.sub};

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 breaking the type safety a bit. We should probably introduce a new interface for the return type of this class. And then convert it into an auth specific return type in auth/token-verifier.

if (!validator.isString(jwtToken)) {
throw new this.tokenInfo.errorType(
this.tokenInfo.errorCodeConfig.invalidArg,
Expand All @@ -100,15 +101,15 @@ export class FirebaseTokenVerifier {

return util.findProjectId(this.app)
.then((projectId) => {
return this.verifyJWTWithProjectId(jwtToken, projectId, isEmulator);
return this.verifyJWTWithProjectId<T>(jwtToken, projectId, isEmulator);
});
}

private verifyJWTWithProjectId(
private verifyJWTWithProjectId<T extends object>(
jwtToken: string,
projectId: string | null,
isEmulator: boolean
): Promise<DecodedIdToken> {
): Promise<T> {
if (!validator.isNonEmptyString(projectId)) {
throw new this.tokenInfo.errorType(
this.tokenInfo.errorCodeConfig.invalidCredential,
Expand Down Expand Up @@ -173,7 +174,7 @@ export class FirebaseTokenVerifier {

if (isEmulator) {
// Signature checks skipped for emulator; no need to fetch public keys.
return this.verifyJwtSignatureWithKey(jwtToken, null);
return this.verifyJwtSignatureWithKey<T>(jwtToken, null);
}

return this.fetchPublicKeys().then((publicKeys) => {
Expand All @@ -187,20 +188,23 @@ export class FirebaseTokenVerifier {
),
);
} else {
return this.verifyJwtSignatureWithKey(jwtToken, publicKeys[header.kid]);
return this.verifyJwtSignatureWithKey<T>(jwtToken, publicKeys[header.kid]);
}

});
}

/**
* Verifies the JWT signature using the provided public key.
*
* @template T The returned type of the decoded token.
* @param {string} jwtToken The JWT token to verify.
* @param {string} publicKey The public key certificate.
* @return {Promise<DecodedIdToken>} A promise that resolves with the decoded JWT claims on successful
* @return {Promise<T>} A promise that resolves with the decoded JWT claims on successful
* verification.
*/
private verifyJwtSignatureWithKey(jwtToken: string, publicKey: string | null): Promise<DecodedIdToken> {
private verifyJwtSignatureWithKey<T extends object>(jwtToken: string,
publicKey: string | null): Promise<T> {
const verifyJwtTokenDocsMessage = ` See ${this.tokenInfo.url} ` +
`for details on how to retrieve ${this.shortNameArticle} ${this.tokenInfo.shortName}.`;
return new Promise((resolve, reject) => {
Expand All @@ -213,18 +217,16 @@ 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 (${this.tokenInfo.errorCodeConfig.expiredError.code}).` +
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));
}
return reject(new this.tokenInfo.errorType(this.tokenInfo.errorCodeConfig.invalidArg, error.message));
} else {
const decodedIdToken = (decodedToken as DecodedIdToken);
decodedIdToken.uid = decodedIdToken.sub;
resolve(decodedIdToken);
resolve(decodedToken as T);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not particularly typesafe.

}
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import './utils/index.spec';
import './utils/error.spec';
import './utils/validator.spec';
import './utils/api-request.spec';
import './utils/token-verifier.spec';

// Auth
import './auth/auth.spec';
import './auth/user-record.spec';
import './auth/token-generator.spec';
import './auth/token-verifier.spec';
import './auth/auth-api-request.spec';
import './auth/user-import-builder.spec';
import './auth/action-code-settings-builder.spec';
Expand Down
Loading