From 4ec0d766ffe7d411479b84d40db658ad666989b4 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Fri, 13 Dec 2019 15:44:53 -0800 Subject: [PATCH 1/7] Updated unit tests --- src/auth/credential.ts | 468 ++++++++++++------------- src/auth/token-generator.ts | 34 +- src/firebase-app.ts | 4 +- src/firebase-namespace.ts | 14 +- src/firestore/firestore.ts | 23 +- src/storage/storage.ts | 16 +- src/utils/index.ts | 8 +- test/resources/mocks.ts | 16 +- test/unit/auth/auth.spec.ts | 10 +- test/unit/auth/credential.spec.ts | 300 ++++++---------- test/unit/auth/token-generator.spec.ts | 12 +- test/unit/auth/token-verifier.spec.ts | 4 +- test/unit/firebase-app.spec.ts | 22 +- test/unit/firebase.spec.ts | 6 +- test/unit/firestore/firestore.spec.ts | 4 +- 15 files changed, 404 insertions(+), 537 deletions(-) diff --git a/src/auth/credential.ts b/src/auth/credential.ts index 521f71bedd..b4bf5db303 100644 --- a/src/auth/credential.ts +++ b/src/auth/credential.ts @@ -29,7 +29,8 @@ const GOOGLE_AUTH_TOKEN_PATH = '/o/oauth2/token'; // NOTE: the Google Metadata Service uses HTTP over a vlan const GOOGLE_METADATA_SERVICE_HOST = 'metadata.google.internal'; -const GOOGLE_METADATA_SERVICE_PATH = '/computeMetadata/v1/instance/service-accounts/default/token'; +const GOOGLE_METADATA_SERVICE_TOKEN_PATH = '/computeMetadata/v1/instance/service-accounts/default/token'; +const GOOGLE_METADATA_SERVICE_PROJECT_ID_PATH = '/computeMetadata/v1/project/project-id'; const configDir = (() => { // Windows has a dedicated low-rights location for apps at ~/Application Data @@ -51,91 +52,109 @@ const REFRESH_TOKEN_PATH = '/oauth2/v4/token'; const ONE_HOUR_IN_SECONDS = 60 * 60; const JWT_ALGORITHM = 'RS256'; +/** + * Interface for Google OAuth 2.0 access tokens. + */ +export interface GoogleOAuthAccessToken { + /* tslint:disable:variable-name */ + access_token: string; + expires_in: number; + /* tslint:enable:variable-name */ +} -function copyAttr(to: {[key: string]: any}, from: {[key: string]: any}, key: string, alt: string) { - const tmp = from[key] || from[alt]; - if (typeof tmp !== 'undefined') { - to[key] = tmp; - } +/** + * Interface for things that generate access tokens. + */ +export interface Credential { + getAccessToken(): Promise; } -export class RefreshToken { - public clientId: string; - public clientSecret: string; - public refreshToken: string; - public type: string; +/** + * Implementation of Credential that uses a service account certificate. + */ +export class ServiceAccountCredential implements Credential { - /* - * Tries to load a RefreshToken from a path. If the path is not present, returns null. - * Throws if data at the path is invalid. - */ - public static fromPath(filePath: string): RefreshToken | null { - let jsonString: string; + public readonly projectId: string; + public readonly privateKey: string; + public readonly clientEmail: string; - try { - jsonString = fs.readFileSync(filePath, 'utf8'); - } catch (ignored) { - // Ignore errors if the file is not present, as this is sometimes an expected condition - return null; - } - try { - return new RefreshToken(JSON.parse(jsonString)); - } catch (error) { - // Throw a nicely formed error message if the file contents cannot be parsed - throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse refresh token file: ' + error, - ); - } + private readonly httpClient: HttpClient; + private readonly httpAgent?: Agent; + + constructor(serviceAccountPathOrObject: string | object, httpAgent?: Agent) { + const serviceAccount = (typeof serviceAccountPathOrObject === 'string') ? + ServiceAccount.fromPath(serviceAccountPathOrObject) + : new ServiceAccount(serviceAccountPathOrObject); + this.projectId = serviceAccount.projectId; + this.privateKey = serviceAccount.privateKey; + this.clientEmail = serviceAccount.clientEmail; + this.httpClient = new HttpClient(); + this.httpAgent = httpAgent; } - constructor(json: object) { - copyAttr(this, json, 'clientId', 'client_id'); - copyAttr(this, json, 'clientSecret', 'client_secret'); - copyAttr(this, json, 'refreshToken', 'refresh_token'); - copyAttr(this, json, 'type', 'type'); + public getAccessToken(): Promise { + const token = this.createAuthJwt_(); + const postData = 'grant_type=urn%3Aietf%3Aparams%3Aoauth%3A' + + 'grant-type%3Ajwt-bearer&assertion=' + token; + const request: HttpRequestConfig = { + method: 'POST', + url: `https://${GOOGLE_AUTH_TOKEN_HOST}${GOOGLE_AUTH_TOKEN_PATH}`, + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + data: postData, + httpAgent: this.httpAgent, + }; + return requestAccessToken(this.httpClient, request); + } - let errorMessage; - if (typeof this.clientId !== 'string' || !this.clientId) { - errorMessage = 'Refresh token must contain a "client_id" property.'; - } else if (typeof this.clientSecret !== 'string' || !this.clientSecret) { - errorMessage = 'Refresh token must contain a "client_secret" property.'; - } else if (typeof this.refreshToken !== 'string' || !this.refreshToken) { - errorMessage = 'Refresh token must contain a "refresh_token" property.'; - } else if (typeof this.type !== 'string' || !this.type) { - errorMessage = 'Refresh token must contain a "type" property.'; - } + private createAuthJwt_(): string { + const claims = { + scope: [ + 'https://www.googleapis.com/auth/cloud-platform', + 'https://www.googleapis.com/auth/firebase.database', + 'https://www.googleapis.com/auth/firebase.messaging', + 'https://www.googleapis.com/auth/identitytoolkit', + 'https://www.googleapis.com/auth/userinfo.email', + ].join(' '), + }; - if (typeof errorMessage !== 'undefined') { - throw new FirebaseAppError(AppErrorCodes.INVALID_CREDENTIAL, errorMessage); - } + const jwt = require('jsonwebtoken'); + // This method is actually synchronous so we can capture and return the buffer. + return jwt.sign(claims, this.privateKey, { + audience: GOOGLE_TOKEN_AUDIENCE, + expiresIn: ONE_HOUR_IN_SECONDS, + issuer: this.clientEmail, + algorithm: JWT_ALGORITHM, + }); } } /** * A struct containing the properties necessary to use service-account JSON credentials. */ -export class Certificate { - public projectId: string; - public privateKey: string; - public clientEmail: string; +class ServiceAccount { + + public readonly projectId: string; + public readonly privateKey: string; + public readonly clientEmail: string; - public static fromPath(filePath: string): Certificate { + public static fromPath(filePath: string): ServiceAccount { // Node bug encountered in v6.x. fs.readFileSync hangs when path is a 0 or 1. if (typeof filePath !== 'string') { throw new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse certificate key file: TypeError: path must be a string', + 'Failed to parse service account json file: TypeError: path must be a string', ); } try { - return new Certificate(JSON.parse(fs.readFileSync(filePath, 'utf8'))); + return new ServiceAccount(JSON.parse(fs.readFileSync(filePath, 'utf8'))); } catch (error) { // Throw a nicely formed error message if the file contents cannot be parsed throw new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse certificate key file: ' + error, + 'Failed to parse service account json file: ' + error, ); } } @@ -144,7 +163,7 @@ export class Certificate { if (typeof json !== 'object' || json === null) { throw new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, - 'Certificate object must be an object.', + 'Service account must be an object.', ); } @@ -153,10 +172,12 @@ export class Certificate { copyAttr(this, json, 'clientEmail', 'client_email'); let errorMessage; - if (typeof this.privateKey !== 'string' || !this.privateKey) { - errorMessage = 'Certificate object must contain a string "private_key" property.'; + if (typeof this.projectId !== 'string' || !this.projectId) { + errorMessage = 'Service account object must contain a string "project_id" property.'; + } else if (typeof this.privateKey !== 'string' || !this.privateKey) { + errorMessage = 'Service account object must contain a string "private_key" property.'; } else if (typeof this.clientEmail !== 'string' || !this.clientEmail) { - errorMessage = 'Certificate object must contain a string "client_email" property.'; + errorMessage = 'Service account object must contain a string "client_email" property.'; } if (typeof errorMessage !== 'undefined') { @@ -175,149 +196,44 @@ export class Certificate { } /** - * Interface for Google OAuth 2.0 access tokens. - */ -export interface GoogleOAuthAccessToken { - /* tslint:disable:variable-name */ - access_token: string; - expires_in: number; - /* tslint:enable:variable-name */ -} - -/** - * Obtain a new OAuth2 token by making a remote service call. - */ -function requestAccessToken(client: HttpClient, request: HttpRequestConfig): Promise { - return client.send(request).then((resp) => { - const json = resp.data; - if (!json.access_token || !json.expires_in) { - throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - `Unexpected response while fetching access token: ${ JSON.stringify(json) }`, - ); - } - return json; - }).catch((err) => { - throw new FirebaseAppError(AppErrorCodes.INVALID_CREDENTIAL, getErrorMessage(err)); - }); -} - -/** - * Constructs a human-readable error message from the given Error. - */ -function getErrorMessage(err: Error): string { - const detail: string = (err instanceof HttpError) ? getDetailFromResponse(err.response) : err.message; - return `Error fetching access token: ${detail}`; -} - -/** - * Extracts details from the given HTTP error response, and returns a human-readable description. If - * the response is JSON-formatted, looks up the error and error_description fields sent by the - * Google Auth servers. Otherwise returns the entire response payload as the error detail. - */ -function getDetailFromResponse(response: HttpResponse): string { - if (response.isJson() && response.data.error) { - const json = response.data; - let detail = json.error; - if (json.error_description) { - detail += ' (' + json.error_description + ')'; - } - return detail; - } - return response.text || 'Missing error payload'; -} - -/** - * Implementation of Credential that uses a service account certificate. + * Implementation of Credential that gets access tokens from the metadata service available + * in the Google Cloud Platform. This authenticates the process as the default service account + * of an App Engine instance or Google Compute Engine machine. */ -export class CertCredential implements FirebaseCredential { +export class ComputeEngineCredential implements Credential { - private readonly certificate: Certificate; - private readonly httpClient: HttpClient; + private readonly httpClient = new HttpClient(); private readonly httpAgent?: Agent; - constructor(serviceAccountPathOrObject: string | object, httpAgent?: Agent) { - this.certificate = (typeof serviceAccountPathOrObject === 'string') ? - Certificate.fromPath(serviceAccountPathOrObject) : new Certificate(serviceAccountPathOrObject); - this.httpClient = new HttpClient(); + constructor(httpAgent?: Agent) { this.httpAgent = httpAgent; } public getAccessToken(): Promise { - const token = this.createAuthJwt_(); - const postData = 'grant_type=urn%3Aietf%3Aparams%3Aoauth%3A' + - 'grant-type%3Ajwt-bearer&assertion=' + token; - const request: HttpRequestConfig = { - method: 'POST', - url: `https://${GOOGLE_AUTH_TOKEN_HOST}${GOOGLE_AUTH_TOKEN_PATH}`, - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, - data: postData, - httpAgent: this.httpAgent, - }; + const request = this.buildRequest(GOOGLE_METADATA_SERVICE_TOKEN_PATH); return requestAccessToken(this.httpClient, request); } - public getCertificate(): Certificate { - return this.certificate; + public getProjectId(): Promise { + const request = this.buildRequest(GOOGLE_METADATA_SERVICE_PROJECT_ID_PATH); + return this.httpClient.send(request) + .then((resp) => { + return resp.text!; + }); } - private createAuthJwt_(): string { - const claims = { - scope: [ - 'https://www.googleapis.com/auth/cloud-platform', - 'https://www.googleapis.com/auth/firebase.database', - 'https://www.googleapis.com/auth/firebase.messaging', - 'https://www.googleapis.com/auth/identitytoolkit', - 'https://www.googleapis.com/auth/userinfo.email', - ].join(' '), + private buildRequest(urlPath: string): HttpRequestConfig { + return { + method: 'GET', + url: `http://${GOOGLE_METADATA_SERVICE_HOST}${urlPath}`, + headers: { + 'Metadata-Flavor': 'Google', + }, + httpAgent: this.httpAgent, }; - - const jwt = require('jsonwebtoken'); - // This method is actually synchronous so we can capture and return the buffer. - return jwt.sign(claims, this.certificate.privateKey, { - audience: GOOGLE_TOKEN_AUDIENCE, - expiresIn: ONE_HOUR_IN_SECONDS, - issuer: this.certificate.clientEmail, - algorithm: JWT_ALGORITHM, - }); } } -/** - * Interface for things that generate access tokens. - */ -export interface Credential { - getAccessToken(): Promise; -} - -/** - * Internal interface for credentials that can both generate access tokens and may have a Certificate - * associated with them. - */ -export interface FirebaseCredential extends Credential { - getCertificate(): Certificate | null; -} - -/** - * Attempts to extract a Certificate from the given credential. - * - * @param {Credential} credential A Credential instance. - * @return {Certificate} A Certificate instance or null. - */ -export function tryGetCertificate(credential: Credential | null | undefined): Certificate | null { - if (credential && isFirebaseCredential(credential)) { - return credential.getCertificate(); - } - - return null; -} - -function isFirebaseCredential(credential: Credential): credential is FirebaseCredential { - return 'getCertificate' in credential; -} - /** * Implementation of Credential that gets access tokens from refresh tokens. */ @@ -364,116 +280,174 @@ export class RefreshTokenCredential implements Credential { } } +class RefreshToken { -/** - * Implementation of Credential that gets access tokens from the metadata service available - * in the Google Cloud Platform. This authenticates the process as the default service account - * of an App Engine instance or Google Compute Engine machine. - */ -export class MetadataServiceCredential implements Credential { + public readonly clientId: string; + public readonly clientSecret: string; + public readonly refreshToken: string; + public readonly type: string; - private readonly httpClient = new HttpClient(); - private readonly httpAgent?: Agent; + /* + * Tries to load a RefreshToken from a path. If the path is not present, returns null. + * Throws if data at the path is invalid. + */ + public static fromPath(filePath: string): RefreshToken | null { + let jsonString: string; - constructor(httpAgent?: Agent) { - this.httpAgent = httpAgent; - } + try { + jsonString = fs.readFileSync(filePath, 'utf8'); + } catch (ignored) { + // Ignore errors if the file is not present, as this is sometimes an expected condition + return null; + } - public getAccessToken(): Promise { - const request: HttpRequestConfig = { - method: 'GET', - url: `http://${GOOGLE_METADATA_SERVICE_HOST}${GOOGLE_METADATA_SERVICE_PATH}`, - headers: { - 'Metadata-Flavor': 'Google', - }, - httpAgent: this.httpAgent, - }; - return requestAccessToken(this.httpClient, request); + try { + return new RefreshToken(JSON.parse(jsonString)); + } catch (error) { + // Throw a nicely formed error message if the file contents cannot be parsed + throw new FirebaseAppError( + AppErrorCodes.INVALID_CREDENTIAL, + 'Failed to parse refresh token file: ' + error, + ); + } } -} - -/** - * ApplicationDefaultCredential implements the process for loading credentials as - * described in https://developers.google.com/identity/protocols/application-default-credentials - */ -export class ApplicationDefaultCredential implements FirebaseCredential { - private credential_: Credential; + constructor(json: object) { + copyAttr(this, json, 'clientId', 'client_id'); + copyAttr(this, json, 'clientSecret', 'client_secret'); + copyAttr(this, json, 'refreshToken', 'refresh_token'); + copyAttr(this, json, 'type', 'type'); - constructor(httpAgent?: Agent) { - if (process.env.GOOGLE_APPLICATION_CREDENTIALS) { - this.credential_ = credentialFromFile(process.env.GOOGLE_APPLICATION_CREDENTIALS, httpAgent); - return; + let errorMessage; + if (typeof this.clientId !== 'string' || !this.clientId) { + errorMessage = 'Refresh token must contain a "client_id" property.'; + } else if (typeof this.clientSecret !== 'string' || !this.clientSecret) { + errorMessage = 'Refresh token must contain a "client_secret" property.'; + } else if (typeof this.refreshToken !== 'string' || !this.refreshToken) { + errorMessage = 'Refresh token must contain a "refresh_token" property.'; + } else if (typeof this.type !== 'string' || !this.type) { + errorMessage = 'Refresh token must contain a "type" property.'; } - // It is OK to not have this file. If it is present, it must be valid. - if (GCLOUD_CREDENTIAL_PATH) { - const refreshToken = RefreshToken.fromPath(GCLOUD_CREDENTIAL_PATH); - if (refreshToken) { - this.credential_ = new RefreshTokenCredential(refreshToken, httpAgent); - return; - } + if (typeof errorMessage !== 'undefined') { + throw new FirebaseAppError(AppErrorCodes.INVALID_CREDENTIAL, errorMessage); } + } +} - this.credential_ = new MetadataServiceCredential(httpAgent); +export function getApplicationDefault(httpAgent?: Agent): Credential { + if (process.env.GOOGLE_APPLICATION_CREDENTIALS) { + return credentialFromFile(process.env.GOOGLE_APPLICATION_CREDENTIALS, httpAgent); } - public getAccessToken(): Promise { - return this.credential_.getAccessToken(); + // It is OK to not have this file. If it is present, it must be valid. + if (GCLOUD_CREDENTIAL_PATH) { + const refreshToken = RefreshToken.fromPath(GCLOUD_CREDENTIAL_PATH); + if (refreshToken) { + return new RefreshTokenCredential(refreshToken, httpAgent); + } } - public getCertificate(): Certificate | null { - return tryGetCertificate(this.credential_); + return new ComputeEngineCredential(httpAgent); +} + +function copyAttr(to: {[key: string]: any}, from: {[key: string]: any}, key: string, alt: string) { + const tmp = from[key] || from[alt]; + if (typeof tmp !== 'undefined') { + to[key] = tmp; } +} - // Used in testing to verify we are delegating to the correct implementation. - public getCredential(): Credential { - return this.credential_; +/** + * Obtain a new OAuth2 token by making a remote service call. + */ +function requestAccessToken(client: HttpClient, request: HttpRequestConfig): Promise { + return client.send(request).then((resp) => { + const json = resp.data; + if (!json.access_token || !json.expires_in) { + throw new FirebaseAppError( + AppErrorCodes.INVALID_CREDENTIAL, + `Unexpected response while fetching access token: ${ JSON.stringify(json) }`, + ); + } + return json; + }).catch((err) => { + throw new FirebaseAppError(AppErrorCodes.INVALID_CREDENTIAL, getErrorMessage(err)); + }); +} + +/** + * Constructs a human-readable error message from the given Error. + */ +function getErrorMessage(err: Error): string { + const detail: string = (err instanceof HttpError) ? getDetailFromResponse(err.response) : err.message; + return `Error fetching access token: ${detail}`; +} + +/** + * Extracts details from the given HTTP error response, and returns a human-readable description. If + * the response is JSON-formatted, looks up the error and error_description fields sent by the + * Google Auth servers. Otherwise returns the entire response payload as the error detail. + */ +function getDetailFromResponse(response: HttpResponse): string { + if (response.isJson() && response.data.error) { + const json = response.data; + let detail = json.error; + if (json.error_description) { + detail += ' (' + json.error_description + ')'; + } + return detail; } + return response.text || 'Missing error payload'; } function credentialFromFile(filePath: string, httpAgent?: Agent): Credential { const credentialsFile = readCredentialFile(filePath); if (typeof credentialsFile !== 'object') { throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse contents of the credentials file as an object', + AppErrorCodes.INVALID_CREDENTIAL, + 'Failed to parse contents of the credentials file as an object', ); } + if (credentialsFile.type === 'service_account') { - return new CertCredential(credentialsFile, httpAgent); + return new ServiceAccountCredential(credentialsFile, httpAgent); } + if (credentialsFile.type === 'authorized_user') { return new RefreshTokenCredential(credentialsFile, httpAgent); } + throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Invalid contents in the credentials file', + AppErrorCodes.INVALID_CREDENTIAL, + 'Invalid contents in the credentials file', ); } function readCredentialFile(filePath: string): {[key: string]: any} { if (typeof filePath !== 'string') { throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse credentials file: TypeError: path must be a string', + AppErrorCodes.INVALID_CREDENTIAL, + 'Failed to parse credentials file: TypeError: path must be a string', ); } + let fileText: string; try { fileText = fs.readFileSync(filePath, 'utf8'); } catch (error) { throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - `Failed to read credentials from file ${filePath}: ` + error, + AppErrorCodes.INVALID_CREDENTIAL, + `Failed to read credentials from file ${filePath}: ` + error, ); } + try { return JSON.parse(fileText); } catch (error) { throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse contents of the credentials file as an object: ' + error, + AppErrorCodes.INVALID_CREDENTIAL, + 'Failed to parse contents of the credentials file as an object: ' + error, ); } } diff --git a/src/auth/token-generator.ts b/src/auth/token-generator.ts index b63e5c5186..8e57e3e30b 100644 --- a/src/auth/token-generator.ts +++ b/src/auth/token-generator.ts @@ -15,7 +15,7 @@ */ import { FirebaseApp } from '../firebase-app'; -import {Certificate, tryGetCertificate} from './credential'; +import {ServiceAccountCredential} from './credential'; import {AuthClientErrorCode, FirebaseAuthError } from '../utils/error'; import { AuthorizedHttpClient, HttpError, HttpRequestConfig, HttpClient } from '../utils/api-request'; @@ -81,28 +81,19 @@ interface JWTBody { * sign data. Performs all operations locally, and does not make any RPC calls. */ export class ServiceAccountSigner implements CryptoSigner { - private readonly certificate: Certificate; /** - * Creates a new CryptoSigner instance from the given service account certificate. + * Creates a new CryptoSigner instance from the given service account credential. * - * @param {Certificate} certificate A service account certificate. + * @param {ServiceAccountCredential} credential A service account credential. */ - constructor(certificate: Certificate) { - if (!certificate) { + constructor(private readonly credential: ServiceAccountCredential) { + if (!credential) { throw new FirebaseAuthError( AuthClientErrorCode.INVALID_CREDENTIAL, - 'INTERNAL ASSERT: Must provide a certificate to initialize ServiceAccountSigner.', + 'INTERNAL ASSERT: Must provide a service account credential to initialize ServiceAccountSigner.', ); } - if (!validator.isNonEmptyString(certificate.clientEmail) || !validator.isNonEmptyString(certificate.privateKey)) { - throw new FirebaseAuthError( - AuthClientErrorCode.INVALID_CREDENTIAL, - 'INTERNAL ASSERT: Must provide a certificate with validate clientEmail and privateKey to ' + - 'initialize ServiceAccountSigner.', - ); - } - this.certificate = certificate; } /** @@ -112,14 +103,14 @@ export class ServiceAccountSigner implements CryptoSigner { const crypto = require('crypto'); const sign = crypto.createSign('RSA-SHA256'); sign.update(buffer); - return Promise.resolve(sign.sign(this.certificate.privateKey)); + return Promise.resolve(sign.sign(this.credential.privateKey)); } /** * @inheritDoc */ public getAccountId(): Promise { - return Promise.resolve(this.certificate.clientEmail); + return Promise.resolve(this.credential.clientEmail); } } @@ -231,12 +222,11 @@ export class IAMSigner implements CryptoSigner { * @return {CryptoSigner} A CryptoSigner instance. */ export function cryptoSignerFromApp(app: FirebaseApp): CryptoSigner { - if (app.options.credential) { - const cert = tryGetCertificate(app.options.credential); - if (cert != null && validator.isNonEmptyString(cert.privateKey) && validator.isNonEmptyString(cert.clientEmail)) { - return new ServiceAccountSigner(cert); - } + const credential = app.options.credential; + if (credential instanceof ServiceAccountCredential) { + return new ServiceAccountSigner(credential); } + return new IAMSigner(new AuthorizedHttpClient(app), app.options.serviceAccountId); } diff --git a/src/firebase-app.ts b/src/firebase-app.ts index 8ac3a6ba88..9dccd51f9d 100644 --- a/src/firebase-app.ts +++ b/src/firebase-app.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import {ApplicationDefaultCredential, Credential, GoogleOAuthAccessToken} from './auth/credential'; +import {Credential, GoogleOAuthAccessToken, getApplicationDefault} from './auth/credential'; import * as validator from './utils/validator'; import {deepCopy, deepExtend} from './utils/deep-copy'; import {FirebaseServiceInterface} from './firebase-service'; @@ -264,7 +264,7 @@ export class FirebaseApp { const hasCredential = ('credential' in this.options_); if (!hasCredential) { - this.options_.credential = new ApplicationDefaultCredential(); + this.options_.credential = getApplicationDefault(this.options.httpAgent); } const credential = this.options_.credential; diff --git a/src/firebase-namespace.ts b/src/firebase-namespace.ts index 86ee91c991..738fc0d4d7 100644 --- a/src/firebase-namespace.ts +++ b/src/firebase-namespace.ts @@ -22,9 +22,9 @@ import {AppHook, FirebaseApp, FirebaseAppOptions} from './firebase-app'; import {FirebaseServiceFactory, FirebaseServiceInterface} from './firebase-service'; import { Credential, - CertCredential, RefreshTokenCredential, - ApplicationDefaultCredential, + ServiceAccountCredential, + getApplicationDefault, } from './auth/credential'; import {Auth} from './auth/auth'; @@ -48,8 +48,8 @@ const DEFAULT_APP_NAME = '[DEFAULT]'; export const FIREBASE_CONFIG_VAR: string = 'FIREBASE_CONFIG'; -let globalAppDefaultCred: ApplicationDefaultCredential; -const globalCertCreds: { [key: string]: CertCredential } = {}; +let globalAppDefaultCred: Credential; +const globalCertCreds: { [key: string]: ServiceAccountCredential } = {}; const globalRefreshTokenCreds: { [key: string]: RefreshTokenCredential } = {}; @@ -85,7 +85,7 @@ export class FirebaseNamespaceInternals { public initializeApp(options?: FirebaseAppOptions, appName = DEFAULT_APP_NAME): FirebaseApp { if (typeof options === 'undefined') { options = this.loadOptionsFromEnvVar(); - options.credential = new ApplicationDefaultCredential(); + options.credential = getApplicationDefault(); } if (typeof appName !== 'string' || appName === '') { throw new FirebaseAppError( @@ -275,7 +275,7 @@ const firebaseCredential = { cert: (serviceAccountPathOrObject: string | object, httpAgent?: Agent): Credential => { const stringifiedServiceAccount = JSON.stringify(serviceAccountPathOrObject); if (!(stringifiedServiceAccount in globalCertCreds)) { - globalCertCreds[stringifiedServiceAccount] = new CertCredential(serviceAccountPathOrObject, httpAgent); + globalCertCreds[stringifiedServiceAccount] = new ServiceAccountCredential(serviceAccountPathOrObject, httpAgent); } return globalCertCreds[stringifiedServiceAccount]; }, @@ -291,7 +291,7 @@ const firebaseCredential = { applicationDefault: (httpAgent?: Agent): Credential => { if (typeof globalAppDefaultCred === 'undefined') { - globalAppDefaultCred = new ApplicationDefaultCredential(httpAgent); + globalAppDefaultCred = getApplicationDefault(httpAgent); } return globalAppDefaultCred; }, diff --git a/src/firestore/firestore.ts b/src/firestore/firestore.ts index c7afa87b7c..2364c531c1 100644 --- a/src/firestore/firestore.ts +++ b/src/firestore/firestore.ts @@ -17,7 +17,7 @@ import {FirebaseApp} from '../firebase-app'; import {FirebaseFirestoreError} from '../utils/error'; import {FirebaseServiceInterface, FirebaseServiceInternalsInterface} from '../firebase-service'; -import {ApplicationDefaultCredential, Certificate, tryGetCertificate} from '../auth/credential'; +import {ServiceAccountCredential, ComputeEngineCredential} from '../auth/credential'; import {Firestore, Settings} from '@google-cloud/firestore'; import * as validator from '../utils/validator'; @@ -72,30 +72,21 @@ export function getFirestoreOptions(app: FirebaseApp): Settings { } const projectId: string | null = utils.getProjectId(app); - const cert: Certificate | null = tryGetCertificate(app.options.credential); + const credential = app.options.credential; const { version: firebaseVersion } = require('../../package.json'); - if (cert != null) { + if (credential instanceof ServiceAccountCredential) { // cert is available when the SDK has been initialized with a service account JSON file, // or by setting the GOOGLE_APPLICATION_CREDENTIALS envrionment variable. - if (!validator.isNonEmptyString(projectId)) { - // Assert for an explicit projct ID (either via AppOptions or the cert itself). - throw new FirebaseFirestoreError({ - code: 'no-project-id', - message: 'Failed to determine project ID for Firestore. Initialize the ' - + 'SDK with service account credentials or set project ID as an app option. ' - + 'Alternatively set the GOOGLE_CLOUD_PROJECT environment variable.', - }); - } return { credentials: { - private_key: cert.privateKey, - client_email: cert.clientEmail, + private_key: credential.privateKey, + client_email: credential.clientEmail, }, - projectId, + projectId: projectId!, firebaseVersion, }; - } else if (app.options.credential instanceof ApplicationDefaultCredential) { + } else if (app.options.credential instanceof ComputeEngineCredential) { // Try to use the Google application default credentials. // If an explicit project ID is not available, let Firestore client discover one from the // environment. This prevents the users from having to set GOOGLE_CLOUD_PROJECT in GCP runtimes. diff --git a/src/storage/storage.ts b/src/storage/storage.ts index aa4b3992f9..41b9ff23b5 100644 --- a/src/storage/storage.ts +++ b/src/storage/storage.ts @@ -17,9 +17,10 @@ import {FirebaseApp} from '../firebase-app'; import {FirebaseError} from '../utils/error'; import {FirebaseServiceInterface, FirebaseServiceInternalsInterface} from '../firebase-service'; -import {ApplicationDefaultCredential, Certificate, tryGetCertificate} from '../auth/credential'; +import {ServiceAccountCredential, ComputeEngineCredential} from '../auth/credential'; import {Bucket, Storage as StorageClient} from '@google-cloud/storage'; +import * as utils from '../utils/index'; import * as validator from '../utils/validator'; /** @@ -70,18 +71,19 @@ export class Storage implements FirebaseServiceInterface { }); } - const cert: Certificate | null = tryGetCertificate(app.options.credential); - if (cert != null) { + const projectId: string | null = utils.getProjectId(app); + const credential = app.options.credential; + if (credential instanceof ServiceAccountCredential) { // cert is available when the SDK has been initialized with a service account JSON file, // or by setting the GOOGLE_APPLICATION_CREDENTIALS envrionment variable. this.storageClient = new storage({ - projectId: cert.projectId, + projectId: projectId!, credentials: { - private_key: cert.privateKey, - client_email: cert.clientEmail, + private_key: credential.privateKey, + client_email: credential.clientEmail, }, }); - } else if (app.options.credential instanceof ApplicationDefaultCredential) { + } else if (app.options.credential instanceof ComputeEngineCredential) { // Try to use the Google application default credentials. this.storageClient = new storage(); } else { diff --git a/src/utils/index.ts b/src/utils/index.ts index 214b2b7f10..5fef86c01a 100755 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -15,7 +15,7 @@ */ import {FirebaseApp, FirebaseAppOptions} from '../firebase-app'; -import {Certificate, tryGetCertificate} from '../auth/credential'; +import {ServiceAccountCredential} from '../auth/credential'; import * as validator from './validator'; @@ -69,9 +69,9 @@ export function getProjectId(app: FirebaseApp): string | null { return options.projectId; } - const cert: Certificate | null = tryGetCertificate(options.credential); - if (cert != null && validator.isNonEmptyString(cert.projectId)) { - return cert.projectId; + const credential = app.options.credential; + if (credential instanceof ServiceAccountCredential) { + return credential.projectId; } const projectId = process.env.GOOGLE_CLOUD_PROJECT || process.env.GCLOUD_PROJECT; diff --git a/test/resources/mocks.ts b/test/resources/mocks.ts index 3928141a44..bdc87cb3ab 100644 --- a/test/resources/mocks.ts +++ b/test/resources/mocks.ts @@ -27,7 +27,7 @@ import * as jwt from 'jsonwebtoken'; import {FirebaseNamespace} from '../../src/firebase-namespace'; import {FirebaseServiceInterface} from '../../src/firebase-service'; import {FirebaseApp, FirebaseAppOptions} from '../../src/firebase-app'; -import {Certificate, Credential, CertCredential, GoogleOAuthAccessToken} from '../../src/auth/credential'; +import {Credential, GoogleOAuthAccessToken, ServiceAccountCredential} from '../../src/auth/credential'; const ALGORITHM = 'RS256'; const ONE_HOUR_IN_SECONDS = 60 * 60; @@ -49,7 +49,7 @@ export let databaseAuthVariableOverride = { 'some#string': 'some#val' }; export let storageBucket = 'bucketName.appspot.com'; -export let credential = new CertCredential(path.resolve(__dirname, './mock.key.json')); +export let credential = new ServiceAccountCredential(path.resolve(__dirname, './mock.key.json')); export let appOptions: FirebaseAppOptions = { credential, @@ -85,10 +85,6 @@ export class MockCredential implements Credential { expires_in: 3600, }); } - - public getCertificate(): Certificate | null { - return null; - } } export function app(): FirebaseApp { @@ -111,13 +107,13 @@ export function appWithOptions(options: FirebaseAppOptions): FirebaseApp { } export function appReturningNullAccessToken(): FirebaseApp { - const nullFn: () => Promise|null = () => null; + const nullFn: () => Promise | null = () => null; return new FirebaseApp({ credential: { getAccessToken: nullFn, - getCertificate: () => credential.getCertificate(), } as any, databaseURL, + projectId, }, appName, new FirebaseNamespace().INTERNAL); } @@ -125,9 +121,9 @@ export function appReturningMalformedAccessToken(): FirebaseApp { return new FirebaseApp({ credential: { getAccessToken: () => 5, - getCertificate: () => credential.getCertificate(), } as any, databaseURL, + projectId, }, appName, new FirebaseNamespace().INTERNAL); } @@ -135,9 +131,9 @@ export function appRejectedWhileFetchingAccessToken(): FirebaseApp { return new FirebaseApp({ credential: { getAccessToken: () => Promise.reject(new Error('Promise intentionally rejected.')), - getCertificate: () => credential.getCertificate(), } as any, databaseURL, + projectId, }, appName, new FirebaseNamespace().INTERNAL); } diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index 03bdcd04bc..b6df89a2ae 100755 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -42,6 +42,7 @@ import { } from '../../../src/auth/auth-config'; import {deepCopy} from '../../../src/utils/deep-copy'; import { TenantManager } from '../../../src/auth/tenant-manager'; +import { ServiceAccountCredential } from '../../../src/auth/credential'; chai.should(); chai.use(sinonChai); @@ -377,20 +378,23 @@ AUTH_CONFIGS.forEach((testConfig) => { }); it('should be fulfilled given an app which returns null access tokens', () => { + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').resolves(null); // createCustomToken() does not rely on an access token and therefore works in this scenario. - return nullAccessTokenAuth.createCustomToken(mocks.uid, mocks.developerClaims) + return auth.createCustomToken(mocks.uid, mocks.developerClaims) .should.eventually.be.fulfilled; }); it('should be fulfilled given an app which returns invalid access tokens', () => { + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').resolves('malformed'); // createCustomToken() does not rely on an access token and therefore works in this scenario. - return malformedAccessTokenAuth.createCustomToken(mocks.uid, mocks.developerClaims) + return auth.createCustomToken(mocks.uid, mocks.developerClaims) .should.eventually.be.fulfilled; }); it('should be fulfilled given an app which fails to generate access tokens', () => { + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').rejects('error'); // createCustomToken() does not rely on an access token and therefore works in this scenario. - return rejectedPromiseAccessTokenAuth.createCustomToken(mocks.uid, mocks.developerClaims) + return auth.createCustomToken(mocks.uid, mocks.developerClaims) .should.eventually.be.fulfilled; }); } diff --git a/test/unit/auth/credential.spec.ts b/test/unit/auth/credential.spec.ts index 792db85c20..0fe5c9a189 100644 --- a/test/unit/auth/credential.spec.ts +++ b/test/unit/auth/credential.spec.ts @@ -31,8 +31,8 @@ import * as utils from '../utils'; import * as mocks from '../../resources/mocks'; import { - ApplicationDefaultCredential, CertCredential, Certificate, GoogleOAuthAccessToken, - MetadataServiceCredential, RefreshTokenCredential, tryGetCertificate, + GoogleOAuthAccessToken, RefreshTokenCredential, ServiceAccountCredential, + ComputeEngineCredential, getApplicationDefault, } from '../../../src/auth/credential'; import { HttpClient } from '../../../src/utils/api-request'; import {Agent} from 'https'; @@ -91,167 +91,81 @@ describe('Credential', () => { process.env = oldProcessEnv; }); - - describe('Certificate', () => { - describe('fromPath', () => { - const invalidFilePaths = [null, NaN, 0, 1, true, false, {}, [], { a: 1 }, [1, 'a'], _.noop]; - invalidFilePaths.forEach((invalidFilePath) => { - it('should throw if called with non-string argument: ' + JSON.stringify(invalidFilePath), () => { - expect(() => { - Certificate.fromPath(invalidFilePath as any); - }).to.throw('Failed to parse certificate key file: TypeError: path must be a string'); - }); - }); - - it('should throw if called with no argument', () => { - expect(() => { - (Certificate as any).fromPath(); - }).to.throw('Failed to parse certificate key file: TypeError: path must be a string'); - }); - - it('should throw if called with the path to a non-existent file', () => { - expect(() => Certificate.fromPath('invalid-file')) - .to.throw('Failed to parse certificate key file: Error: ENOENT: no such file or directory'); - }); - - it('should throw if called with the path to an invalid file', () => { - const invalidPath = path.resolve(__dirname, '../../resources/unparesable.json'); - expect(() => Certificate.fromPath(invalidPath)) - .to.throw('Failed to parse certificate key file: Error: ENOENT: no such file or directory'); - }); - - it('should throw if called with an empty string path', () => { - expect(() => Certificate.fromPath('')) - .to.throw('Failed to parse certificate key file: Error: ENOENT: no such file or directory'); - }); - - it('should not throw given a valid path to a key file', () => { - const validPath = path.resolve(__dirname, '../../resources/mock.key.json'); - expect(() => Certificate.fromPath(validPath)).not.to.throw(); - }); - - it('should throw given an object without a "private_key" property', () => { - const invalidCertificate = _.omit(mocks.certificateObject, 'private_key'); - expect(() => { - return new Certificate(invalidCertificate as any); - }).to.throw('Certificate object must contain a string "private_key" property'); - }); - - it('should throw given an object with an empty string "private_key" property', () => { - const invalidCertificate = _.clone(mocks.certificateObject); - invalidCertificate.private_key = ''; - expect(() => { - return new Certificate(invalidCertificate as any); - }).to.throw('Certificate object must contain a string "private_key" property'); - }); - - it('should throw given an object without a "client_email" property', () => { - const invalidCertificate = _.omit(mocks.certificateObject, 'client_email'); - expect(() => { - return new Certificate(invalidCertificate as any); - }).to.throw('Certificate object must contain a string "client_email" property'); - }); - - it('should throw given an object with an empty string "client_email" property', () => { - const invalidCertificate = _.clone(mocks.certificateObject); - invalidCertificate.client_email = ''; - expect(() => { - return new Certificate(invalidCertificate as any); - }).to.throw('Certificate object must contain a string "client_email" property'); - }); - - const invalidCredentials = [null, NaN, 0, 1, true, false, '', 'a', [], {}, { a: 1 }, _.noop]; - invalidCredentials.forEach((invalidCredential) => { - it('should throw given invalid Credential: ' + JSON.stringify(invalidCredential), () => { - expect(() => { - return new Certificate(invalidCredential as any); - }).to.throw(Error); - }); + describe('ServiceAccountCredential', () => { + const invalidFilePaths = [null, NaN, 0, 1, true, false, undefined, _.noop]; + invalidFilePaths.forEach((invalidFilePath) => { + it('should throw if called with non-string argument: ' + JSON.stringify(invalidFilePath), () => { + expect(() => new ServiceAccountCredential(invalidFilePath as any)) + .to.throw('Service account must be an object'); }); }); - describe('constructor', () => { - const invalidCertificateObjects = [null, NaN, 0, 1, true, false, _.noop]; - invalidCertificateObjects.forEach((invalidCertificateObject) => { - it('should throw if called with non-object argument: ' + JSON.stringify(invalidCertificateObject), () => { - expect(() => { - return new Certificate(invalidCertificateObject as any); - }).to.throw('Certificate object must be an object.'); - }); - }); - - it('should throw if called with no argument', () => { - expect(() => { - return new (Certificate as any)(); - }).to.throw('Certificate object must be an object.'); - }); - - it('should throw if certificate object does not contain a valid "client_email"', () => { - mockCertificateObject.client_email = ''; - - expect(() => { - return new Certificate(mockCertificateObject); - }).to.throw('Certificate object must contain a string "client_email" property'); - - delete mockCertificateObject.client_email; - - expect(() => { - return new Certificate(mockCertificateObject); - }).to.throw('Certificate object must contain a string "client_email" property'); - }); + it('should throw if called with the path to a non-existent file', () => { + expect(() => new ServiceAccountCredential('invalid-file')) + .to.throw('Failed to parse service account json file: Error: ENOENT: no such file or directory'); + }); - it('should throw if certificate object does not contain a "private_key"', () => { - mockCertificateObject.private_key = ''; + it('should throw if called with the path to an invalid file', () => { + const invalidPath = path.resolve(__dirname, '../../resources/unparesable.json'); + expect(() => new ServiceAccountCredential(invalidPath)) + .to.throw('Failed to parse service account json file: Error: ENOENT: no such file or directory'); + }); - expect(() => { - return new Certificate(mockCertificateObject); - }).to.throw('Certificate object must contain a string "private_key" property'); + it('should throw if called with an empty string path', () => { + expect(() => new ServiceAccountCredential('')) + .to.throw('Failed to parse service account json file: Error: ENOENT: no such file or directory'); + }); - delete mockCertificateObject.private_key; + it('should throw given an object without a "private_key" property', () => { + const invalidCertificate = _.omit(mocks.certificateObject, 'private_key'); + expect(() => new ServiceAccountCredential(invalidCertificate as any)) + .to.throw('Service account object must contain a string "private_key" property'); + }); - expect(() => { - return new Certificate(mockCertificateObject); - }).to.throw('Certificate object must contain a string "private_key" property'); - }); + it('should throw given an object with an empty string "private_key" property', () => { + const invalidCertificate = _.clone(mocks.certificateObject); + invalidCertificate.private_key = ''; + expect(() => new ServiceAccountCredential(invalidCertificate as any)) + .to.throw('Service account object must contain a string "private_key" property'); + }); - it('should throw if certificate object does not contain a valid "private_key"', () => { - mockCertificateObject.private_key = 'invalid.key'; + it('should throw given an object without a "client_email" property', () => { + const invalidCertificate = _.omit(mocks.certificateObject, 'client_email'); + expect(() => new ServiceAccountCredential(invalidCertificate as any)) + .to.throw('Service account object must contain a string "client_email" property'); + }); - expect(() => { - return new Certificate(mockCertificateObject); - }).to.throw('Failed to parse private key: Error: Invalid PEM formatted message.'); - }); + it('should throw given an object with an empty string "client_email" property', () => { + const invalidCertificate = _.clone(mocks.certificateObject); + invalidCertificate.client_email = ''; + expect(() => new ServiceAccountCredential(invalidCertificate as any)) + .to.throw('Service account object must contain a string "client_email" property'); + }); - it('should not throw given a valid certificate object', () => { - expect(() => { - return new Certificate(mockCertificateObject); - }).not.to.throw(); - }); + it('should not throw given a valid path to a key file', () => { + const validPath = path.resolve(__dirname, '../../resources/mock.key.json'); + expect(() => new ServiceAccountCredential(validPath)).not.to.throw(); + }); - it('should accept "clientEmail" in place of "client_email" for the certificate object', () => { - mockCertificateObject.clientEmail = mockCertificateObject.client_email; - delete mockCertificateObject.client_email; + it('should accept "clientEmail" in place of "client_email" for the certificate object', () => { + mockCertificateObject.clientEmail = mockCertificateObject.client_email; + delete mockCertificateObject.client_email; - expect(() => { - return new Certificate(mockCertificateObject); - }).not.to.throw(); - }); + expect(() => new ServiceAccountCredential(mockCertificateObject)) + .not.to.throw(); + }); - it('should accept "privateKey" in place of "private_key" for the certificate object', () => { - mockCertificateObject.privateKey = mockCertificateObject.private_key; - delete mockCertificateObject.private_key; + it('should accept "privateKey" in place of "private_key" for the certificate object', () => { + mockCertificateObject.privateKey = mockCertificateObject.private_key; + delete mockCertificateObject.private_key; - expect(() => { - return new Certificate(mockCertificateObject); - }).not.to.throw(); - }); + expect(() => new ServiceAccountCredential(mockCertificateObject)) + .not.to.throw(); }); - }); - describe('CertCredential', () => { it('should return a Credential', () => { - const c = new CertCredential(mockCertificateObject); - expect(c.getCertificate()).to.deep.equal({ + const c = new ServiceAccountCredential(mockCertificateObject); + expect(c).to.deep.include({ projectId: mockCertificateObject.project_id, clientEmail: mockCertificateObject.client_email, privateKey: mockCertificateObject.private_key, @@ -259,8 +173,8 @@ describe('Credential', () => { }); it('should return a certificate', () => { - const c = new CertCredential(mockCertificateObject); - expect(tryGetCertificate(c)).to.deep.equal({ + const c = new ServiceAccountCredential(mockCertificateObject); + expect(c).to.deep.include({ projectId: mockCertificateObject.project_id, clientEmail: mockCertificateObject.client_email, privateKey: mockCertificateObject.private_key, @@ -268,7 +182,7 @@ describe('Credential', () => { }); it('should create access tokens', () => { - const c = new CertCredential(mockCertificateObject); + const c = new ServiceAccountCredential(mockCertificateObject); return c.getAccessToken().then((token) => { expect(token.access_token).to.be.a('string').and.to.not.be.empty; expect(token.expires_in).to.equal(ONE_HOUR_IN_SECONDS); @@ -287,14 +201,14 @@ describe('Credential', () => { error: 'invalid_grant', error_description: 'reason', })); - const c = new CertCredential(mockCertificateObject); + const c = new ServiceAccountCredential(mockCertificateObject); return expect(c.getAccessToken()).to.be .rejectedWith('Error fetching access token: invalid_grant (reason)'); }); it('should throw an error including error text payload', () => { httpStub.rejects(utils.errorFrom('not json')); - const c = new CertCredential(mockCertificateObject); + const c = new ServiceAccountCredential(mockCertificateObject); return expect(c.getAccessToken()).to.be .rejectedWith('Error fetching access token: not json'); }); @@ -302,11 +216,6 @@ describe('Credential', () => { }); describe('RefreshTokenCredential', () => { - it('should not return a certificate', () => { - const c = new RefreshTokenCredential(MOCK_REFRESH_TOKEN_CONFIG); - expect(tryGetCertificate(c)).to.be.null; - }); - it('should create access tokens', () => { const scope = nock('https://www.googleapis.com') .post('/oauth2/v4/token') @@ -327,15 +236,10 @@ describe('Credential', () => { }); }); - describe('MetadataServiceCredential', () => { + describe('ComputeEngineCredential', () => { let httpStub: sinon.SinonStub; - before(() => httpStub = sinon.stub(HttpClient.prototype, 'send')); - after(() => httpStub.restore()); - - it('should not return a certificate', () => { - const c = new MetadataServiceCredential(); - expect(tryGetCertificate(c)).to.be.null; - }); + beforeEach(() => httpStub = sinon.stub(HttpClient.prototype, 'send')); + afterEach(() => httpStub.restore()); it('should create access tokens', () => { const expected: GoogleOAuthAccessToken = { @@ -345,7 +249,7 @@ describe('Credential', () => { const response = utils.responseFrom(expected); httpStub.resolves(response); - const c = new MetadataServiceCredential(); + const c = new ComputeEngineCredential(); return c.getAccessToken().then((token) => { expect(token.access_token).to.equal('anAccessToken'); expect(token.expires_in).to.equal(42); @@ -357,9 +261,26 @@ describe('Credential', () => { }); }); }); + + it('should discover project id', () => { + const expected = 'test-project-id'; + const response = utils.responseFrom(expected); + httpStub.resolves(response); + + const c = new ComputeEngineCredential(); + return c.getProjectId().then((projectId) => { + expect(projectId).to.equal(expected); + expect(httpStub).to.have.been.calledOnce.and.calledWith({ + method: 'GET', + url: 'http://metadata.google.internal/computeMetadata/v1/project/project-id', + headers: {'Metadata-Flavor': 'Google'}, + httpAgent: undefined, + }); + }); + }); }); - describe('ApplicationDefaultCredential', () => { + describe('getApplicationDefault()', () => { let fsStub: sinon.SinonStub; afterEach(() => { @@ -370,23 +291,23 @@ describe('Credential', () => { it('should return a CertCredential with GOOGLE_APPLICATION_CREDENTIALS set', () => { process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); - const c = new ApplicationDefaultCredential(); - expect(c.getCredential()).to.be.an.instanceof(CertCredential); + const c = getApplicationDefault(); + expect(c).to.be.an.instanceof(ServiceAccountCredential); }); it('should throw if explicitly pointing to an invalid path', () => { process.env.GOOGLE_APPLICATION_CREDENTIALS = 'invalidpath'; - expect(() => new ApplicationDefaultCredential()).to.throw(Error); + expect(() => getApplicationDefault()).to.throw(Error); }); it('should throw if explicitly pointing to an invalid cert file', () => { fsStub = sinon.stub(fs, 'readFileSync').returns('invalidjson'); - expect(() => new ApplicationDefaultCredential()).to.throw(Error); + expect(() => getApplicationDefault()).to.throw(Error); }); it('should throw error if type not specified on cert file', () => { fsStub = sinon.stub(fs, 'readFileSync').returns(JSON.stringify({})); - expect(() => new ApplicationDefaultCredential()) + expect(() => getApplicationDefault()) .to.throw(Error, 'Invalid contents in the credentials file'); }); @@ -394,7 +315,7 @@ describe('Credential', () => { fsStub = sinon.stub(fs, 'readFileSync').returns(JSON.stringify({ type: 'foo', })); - expect(() => new ApplicationDefaultCredential()).to.throw(Error, 'Invalid contents in the credentials file'); + expect(() => getApplicationDefault()).to.throw(Error, 'Invalid contents in the credentials file'); }); it('should return a RefreshTokenCredential with gcloud login', () => { @@ -406,24 +327,24 @@ describe('Credential', () => { return; } delete process.env.GOOGLE_APPLICATION_CREDENTIALS; - expect((new ApplicationDefaultCredential()).getCredential()).to.be.an.instanceof(RefreshTokenCredential); + expect((getApplicationDefault())).to.be.an.instanceof(RefreshTokenCredential); }); it('should throw if a the gcloud login cache is invalid', () => { delete process.env.GOOGLE_APPLICATION_CREDENTIALS; fsStub = sinon.stub(fs, 'readFileSync').returns('invalidjson'); - expect(() => new ApplicationDefaultCredential()).to.throw(Error); + expect(() => getApplicationDefault()).to.throw(Error); }); it('should return a MetadataServiceCredential as a last resort', () => { delete process.env.GOOGLE_APPLICATION_CREDENTIALS; fsStub = sinon.stub(fs, 'readFileSync').throws(new Error('no gcloud credential file')); - expect((new ApplicationDefaultCredential()).getCredential()).to.be.an.instanceof(MetadataServiceCredential); + expect(getApplicationDefault()).to.be.an.instanceof(ComputeEngineCredential); }); it('should create access tokens', () => { process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); - const c = new ApplicationDefaultCredential(); + const c = getApplicationDefault(); return c.getAccessToken().then((token) => { expect(token.access_token).to.be.a('string').and.to.not.be.empty; expect(token.expires_in).to.equal(ONE_HOUR_IN_SECONDS); @@ -432,18 +353,8 @@ describe('Credential', () => { it('should return a Credential', () => { process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); - const c = new ApplicationDefaultCredential(); - expect(c.getCertificate()).to.deep.equal({ - projectId: mockCertificateObject.project_id, - clientEmail: mockCertificateObject.client_email, - privateKey: mockCertificateObject.private_key, - }); - }); - - it('should return a Certificate', () => { - process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); - const c = new ApplicationDefaultCredential(); - expect(tryGetCertificate(c)).to.deep.equal({ + const c = getApplicationDefault(); + expect(c).to.deep.include({ projectId: mockCertificateObject.project_id, clientEmail: mockCertificateObject.client_email, privateKey: mockCertificateObject.private_key, @@ -456,8 +367,7 @@ describe('Credential', () => { fsStub = sinon.stub(fs, 'readFileSync').returns(JSON.stringify(MOCK_REFRESH_TOKEN_CONFIG)); - const adc = new ApplicationDefaultCredential(); - const c = adc.getCredential(); + const c = getApplicationDefault(); expect(c).is.instanceOf(RefreshTokenCredential); expect(c).to.have.property('refreshToken').that.includes({ clientId: MOCK_REFRESH_TOKEN_CONFIG.client_id, @@ -485,9 +395,9 @@ describe('Credential', () => { stub.restore(); }); - it('CertCredential should use the provided HTTP Agent', () => { + it('ServiceAccountCredential should use the provided HTTP Agent', () => { const agent = new Agent(); - const c = new CertCredential(mockCertificateObject, agent); + const c = new ServiceAccountCredential(mockCertificateObject, agent); return c.getAccessToken().then((token) => { expect(token.access_token).to.equal(expectedToken); expect(stub).to.have.been.calledOnce; @@ -505,9 +415,9 @@ describe('Credential', () => { }); }); - it('MetadataServiceCredential should use the provided HTTP Agent', () => { + it('ComputeEngineCredential should use the provided HTTP Agent', () => { const agent = new Agent(); - const c = new MetadataServiceCredential(agent); + const c = new ComputeEngineCredential(agent); return c.getAccessToken().then((token) => { expect(token.access_token).to.equal(expectedToken); expect(stub).to.have.been.calledOnce; @@ -518,7 +428,7 @@ describe('Credential', () => { it('ApplicationDefaultCredential should use the provided HTTP Agent', () => { process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); const agent = new Agent(); - const c = new ApplicationDefaultCredential(agent); + const c = getApplicationDefault(agent); return c.getAccessToken().then((token) => { expect(token.access_token).to.equal(expectedToken); expect(stub).to.have.been.calledOnce; diff --git a/test/unit/auth/token-generator.spec.ts b/test/unit/auth/token-generator.spec.ts index d36d73d3b6..2814f64db9 100644 --- a/test/unit/auth/token-generator.spec.ts +++ b/test/unit/auth/token-generator.spec.ts @@ -28,7 +28,7 @@ import { BLACKLISTED_CLAIMS, FirebaseTokenGenerator, ServiceAccountSigner, IAMSigner, } from '../../../src/auth/token-generator'; -import {Certificate} from '../../../src/auth/credential'; +import { ServiceAccountCredential } from '../../../src/auth/credential'; import { AuthorizedHttpClient, HttpClient } from '../../../src/utils/api-request'; import { FirebaseApp } from '../../../src/firebase-app'; import * as utils from '../utils'; @@ -70,18 +70,18 @@ describe('CryptoSigner', () => { expect(() => { const anyServiceAccountSigner: any = ServiceAccountSigner; return new anyServiceAccountSigner(); - }).to.throw('Must provide a certificate to initialize ServiceAccountSigner'); + }).to.throw('Must provide a service account credential to initialize ServiceAccountSigner'); }); it('should not throw given a valid certificate', () => { expect(() => { - return new ServiceAccountSigner(new Certificate(mocks.certificateObject)); + return new ServiceAccountSigner(new ServiceAccountCredential(mocks.certificateObject)); }).not.to.throw(); }); it('should sign using the private_key in the certificate', () => { const payload = Buffer.from('test'); - const cert = new Certificate(mocks.certificateObject); + const cert = new ServiceAccountCredential(mocks.certificateObject); const crypto = require('crypto'); const rsa = crypto.createSign('RSA-SHA256'); @@ -95,7 +95,7 @@ describe('CryptoSigner', () => { }); it('should return the client_email from the certificate', () => { - const cert = new Certificate(mocks.certificateObject); + const cert = new ServiceAccountCredential(mocks.certificateObject); const signer = new ServiceAccountSigner(cert); return signer.getAccountId().should.eventually.equal(cert.clientEmail); }); @@ -257,7 +257,7 @@ describe('FirebaseTokenGenerator', () => { let clock: sinon.SinonFakeTimers | undefined; beforeEach(() => { - const cert = new Certificate(mocks.certificateObject); + const cert = new ServiceAccountCredential(mocks.certificateObject); tokenGenerator = new FirebaseTokenGenerator(new ServiceAccountSigner(cert)); }); diff --git a/test/unit/auth/token-verifier.spec.ts b/test/unit/auth/token-verifier.spec.ts index 2fe2183eb5..41dddfe514 100644 --- a/test/unit/auth/token-verifier.spec.ts +++ b/test/unit/auth/token-verifier.spec.ts @@ -31,7 +31,7 @@ import * as mocks from '../../resources/mocks'; import {FirebaseTokenGenerator, ServiceAccountSigner} from '../../../src/auth/token-generator'; import * as verifier from '../../../src/auth/token-verifier'; -import {Certificate} from '../../../src/auth/credential'; +import {ServiceAccountCredential} from '../../../src/auth/credential'; import { AuthClientErrorCode } from '../../../src/utils/error'; import { FirebaseApp } from '../../../src/firebase-app'; @@ -114,7 +114,7 @@ describe('FirebaseTokenVerifier', () => { beforeEach(() => { // Needed to generate custom token for testing. app = mocks.app(); - const cert: Certificate = new Certificate(mocks.certificateObject); + const cert = new ServiceAccountCredential(mocks.certificateObject); tokenGenerator = new FirebaseTokenGenerator(new ServiceAccountSigner(cert)); tokenVerifier = new verifier.FirebaseTokenVerifier( 'https://www.googleapis.com/robot/v1/metadata/x509/securetoken@system.gserviceaccount.com', diff --git a/test/unit/firebase-app.spec.ts b/test/unit/firebase-app.spec.ts index 75262b1456..5d6fc3063c 100644 --- a/test/unit/firebase-app.spec.ts +++ b/test/unit/firebase-app.spec.ts @@ -25,7 +25,7 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as utils from './utils'; import * as mocks from '../resources/mocks'; -import {ApplicationDefaultCredential, CertCredential, GoogleOAuthAccessToken} from '../../src/auth/credential'; +import {GoogleOAuthAccessToken, ServiceAccountCredential} from '../../src/auth/credential'; import {FirebaseServiceInterface} from '../../src/firebase-service'; import {FirebaseApp, FirebaseAccessToken} from '../../src/firebase-app'; import {FirebaseNamespace, FirebaseNamespaceInternals, FIREBASE_CONFIG_VAR} from '../../src/firebase-namespace'; @@ -69,7 +69,7 @@ describe('FirebaseApp', () => { let firebaseConfigVar: string | undefined; beforeEach(() => { - getTokenStub = sinon.stub(CertCredential.prototype, 'getAccessToken').resolves({ + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').resolves({ access_token: 'mock-access-token', expires_in: 3600, }); @@ -243,7 +243,7 @@ describe('FirebaseApp', () => { it('should use explicitly specified options when available and ignore the config file', () => { process.env[FIREBASE_CONFIG_VAR] = './test/resources/firebase_config.json'; const app = firebaseNamespace.initializeApp(mocks.appOptions, mocks.appName); - expect(app.options.credential).to.be.instanceOf(CertCredential); + expect(app.options.credential).to.be.instanceOf(ServiceAccountCredential); expect(app.options.databaseAuthVariableOverride).to.be.undefined; expect(app.options.databaseURL).to.equal('https://databaseName.firebaseio.com'); expect(app.options.projectId).to.be.undefined; @@ -260,7 +260,7 @@ describe('FirebaseApp', () => { it('should not throw when the config environment variable is not set, and some options are present', () => { const app = firebaseNamespace.initializeApp(mocks.appOptionsNoDatabaseUrl, mocks.appName); - expect(app.options.credential).to.be.instanceOf(CertCredential); + expect(app.options.credential).to.be.instanceOf(ServiceAccountCredential); expect(app.options.databaseURL).to.be.undefined; expect(app.options.projectId).to.be.undefined; expect(app.options.storageBucket).to.be.undefined; @@ -268,7 +268,7 @@ describe('FirebaseApp', () => { it('should init with application default creds when no options provided and env variable is not set', () => { const app = firebaseNamespace.initializeApp(); - expect(app.options.credential).to.be.instanceOf(ApplicationDefaultCredential); + expect(app.options.credential).to.not.be.undefined; expect(app.options.databaseURL).to.be.undefined; expect(app.options.projectId).to.be.undefined; expect(app.options.storageBucket).to.be.undefined; @@ -277,7 +277,7 @@ describe('FirebaseApp', () => { it('should init with application default creds when no options provided and env variable is an empty json', () => { process.env[FIREBASE_CONFIG_VAR] = '{}'; const app = firebaseNamespace.initializeApp(); - expect(app.options.credential).to.be.instanceOf(ApplicationDefaultCredential); + expect(app.options.credential).to.not.be.undefined; expect(app.options.databaseURL).to.be.undefined; expect(app.options.projectId).to.be.undefined; expect(app.options.storageBucket).to.be.undefined; @@ -286,7 +286,7 @@ describe('FirebaseApp', () => { it('should init when no init arguments are provided and config var points to a file', () => { process.env[FIREBASE_CONFIG_VAR] = './test/resources/firebase_config.json'; const app = firebaseNamespace.initializeApp(); - expect(app.options.credential).to.be.instanceOf(ApplicationDefaultCredential); + expect(app.options.credential).to.not.be.undefined; expect(app.options.databaseAuthVariableOverride).to.deep.equal({ 'some#key': 'some#val' }); expect(app.options.databaseURL).to.equal('https://hipster-chat.firebaseio.mock'); expect(app.options.projectId).to.equal('hipster-chat-mock'); @@ -301,7 +301,7 @@ describe('FirebaseApp', () => { "storageBucket": "hipster-chat.appspot.mock" }`; const app = firebaseNamespace.initializeApp(); - expect(app.options.credential).to.be.instanceOf(ApplicationDefaultCredential); + expect(app.options.credential).to.not.be.undefined; expect(app.options.databaseAuthVariableOverride).to.deep.equal({ 'some#key': 'some#val' }); expect(app.options.databaseURL).to.equal('https://hipster-chat.firebaseio.mock'); expect(app.options.projectId).to.equal('hipster-chat-mock'); @@ -760,7 +760,7 @@ describe('FirebaseApp', () => { // Restore the stubbed getAccessToken() method. getTokenStub.restore(); - getTokenStub = sinon.stub(CertCredential.prototype, 'getAccessToken').resolves({ + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').resolves({ access_token: 'mock-access-token', expires_in: 3600, }); @@ -944,7 +944,7 @@ describe('FirebaseApp', () => { getTokenStub.restore(); const mockError = new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, 'Something went wrong'); - getTokenStub = sinon.stub(CertCredential.prototype, 'getAccessToken').rejects(mockError); + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').rejects(mockError); const detailedMessage = 'Credential implementation provided to initializeApp() via the "credential" property' + ' failed to fetch a valid Google OAuth2 access token with the following error: "Something went wrong".'; expect(mockApp.INTERNAL.getToken(true)).to.be.rejectedWith(detailedMessage); @@ -954,7 +954,7 @@ describe('FirebaseApp', () => { getTokenStub.restore(); const mockError = new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, 'Failed to get credentials: invalid_grant (reason)'); - getTokenStub = sinon.stub(CertCredential.prototype, 'getAccessToken').rejects(mockError); + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').rejects(mockError); const detailedMessage = 'Credential implementation provided to initializeApp() via the "credential" property' + ' failed to fetch a valid Google OAuth2 access token with the following error: "Failed to get credentials:' + ' invalid_grant (reason)". There are two likely causes: (1) your server time is not properly synced or (2)' diff --git a/test/unit/firebase.spec.ts b/test/unit/firebase.spec.ts index 8d3f0cb1ec..1c4786ade8 100644 --- a/test/unit/firebase.spec.ts +++ b/test/unit/firebase.spec.ts @@ -27,7 +27,7 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as mocks from '../resources/mocks'; import * as firebaseAdmin from '../../src/index'; -import {ApplicationDefaultCredential, CertCredential, RefreshTokenCredential} from '../../src/auth/credential'; +import {RefreshTokenCredential, ServiceAccountCredential} from '../../src/auth/credential'; chai.should(); chai.use(chaiAsPromised); @@ -39,7 +39,7 @@ describe('Firebase', () => { let getTokenStub: sinon.SinonStub; before(() => { - getTokenStub = sinon.stub(CertCredential.prototype, 'getAccessToken').resolves({ + getTokenStub = sinon.stub(ServiceAccountCredential.prototype, 'getAccessToken').resolves({ access_token: 'mock-access-token', expires_in: 3600, }); @@ -71,7 +71,7 @@ describe('Firebase', () => { it('should use application default credentials when no credentials are explicitly specified', () => { const app = firebaseAdmin.initializeApp(mocks.appOptionsNoAuth); expect(app.options).to.have.property('credential'); - expect(app.options.credential).to.be.instanceOf(ApplicationDefaultCredential); + expect(app.options.credential).to.not.be.undefined; }); it('should not modify the provided options object', () => { diff --git a/test/unit/firestore/firestore.spec.ts b/test/unit/firestore/firestore.spec.ts index 9105dab4e7..ed3e5b7409 100644 --- a/test/unit/firestore/firestore.spec.ts +++ b/test/unit/firestore/firestore.spec.ts @@ -22,7 +22,7 @@ import {expect} from 'chai'; import * as mocks from '../../resources/mocks'; import {FirebaseApp} from '../../../src/firebase-app'; -import {ApplicationDefaultCredential} from '../../../src/auth/credential'; +import { ComputeEngineCredential } from '../../../src/auth/credential'; import {FirestoreService, getFirestoreOptions} from '../../../src/firestore/firestore'; describe('Firestore', () => { @@ -52,7 +52,7 @@ describe('Firestore', () => { mockApp = mocks.app(); mockCredentialApp = mocks.mockCredentialApp(); defaultCredentialApp = mocks.appWithOptions({ - credential: new ApplicationDefaultCredential(), + credential: new ComputeEngineCredential(), }); projectIdApp = mocks.appWithOptions({ credential: mocks.credential, From cd2062854aab282442496e5e65a68b382caaa49a Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 16 Dec 2019 13:58:32 -0800 Subject: [PATCH 2/7] Further cleaned up the credential impl --- src/auth/credential.ts | 50 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/auth/credential.ts b/src/auth/credential.ts index b4bf5db303..2a933ba728 100644 --- a/src/auth/credential.ts +++ b/src/auth/credential.ts @@ -19,7 +19,7 @@ import fs = require('fs'); import os = require('os'); import path = require('path'); -import {AppErrorCodes, FirebaseAppError, FirebaseAuthError, AuthClientErrorCode} from '../utils/error'; +import {AppErrorCodes, FirebaseAppError} from '../utils/error'; import {HttpClient, HttpRequestConfig, HttpError, HttpResponse} from '../utils/api-request'; import {Agent} from 'http'; @@ -148,6 +148,7 @@ class ServiceAccount { 'Failed to parse service account json file: TypeError: path must be a string', ); } + try { return new ServiceAccount(JSON.parse(fs.readFileSync(filePath, 'utf8'))); } catch (error) { @@ -244,19 +245,9 @@ export class RefreshTokenCredential implements Credential { private readonly httpAgent?: Agent; constructor(refreshTokenPathOrObject: string | object, httpAgent?: Agent) { - if (typeof refreshTokenPathOrObject === 'string') { - const refreshToken = RefreshToken.fromPath(refreshTokenPathOrObject); - if (!refreshToken) { - throw new FirebaseAuthError( - AuthClientErrorCode.NOT_FOUND, - 'The file refered to by the refreshTokenPathOrObject parameter (' + - refreshTokenPathOrObject + ') was not found.', - ); - } - this.refreshToken = refreshToken; - } else { - this.refreshToken = new RefreshToken(refreshTokenPathOrObject); - } + this.refreshToken = (typeof refreshTokenPathOrObject === 'string') ? + RefreshToken.fromPath(refreshTokenPathOrObject) + : new RefreshToken(refreshTokenPathOrObject); this.httpClient = new HttpClient(); this.httpAgent = httpAgent; } @@ -288,21 +279,20 @@ class RefreshToken { public readonly type: string; /* - * Tries to load a RefreshToken from a path. If the path is not present, returns null. - * Throws if data at the path is invalid. + * Tries to load a RefreshToken from a path. Throws if the path doesn't exist or the + * data at the path is invalid. */ - public static fromPath(filePath: string): RefreshToken | null { - let jsonString: string; - - try { - jsonString = fs.readFileSync(filePath, 'utf8'); - } catch (ignored) { - // Ignore errors if the file is not present, as this is sometimes an expected condition - return null; + public static fromPath(filePath: string): RefreshToken { + // Node bug encountered in v6.x. fs.readFileSync hangs when path is a 0 or 1. + if (typeof filePath !== 'string') { + throw new FirebaseAppError( + AppErrorCodes.INVALID_CREDENTIAL, + 'Failed to parse service account json file: TypeError: path must be a string', + ); } try { - return new RefreshToken(JSON.parse(jsonString)); + return new RefreshToken(JSON.parse(fs.readFileSync(filePath, 'utf8'))); } catch (error) { // Throw a nicely formed error message if the file contents cannot be parsed throw new FirebaseAppError( @@ -342,7 +332,7 @@ export function getApplicationDefault(httpAgent?: Agent): Credential { // It is OK to not have this file. If it is present, it must be valid. if (GCLOUD_CREDENTIAL_PATH) { - const refreshToken = RefreshToken.fromPath(GCLOUD_CREDENTIAL_PATH); + const refreshToken = readCredentialFile(GCLOUD_CREDENTIAL_PATH, true); if (refreshToken) { return new RefreshTokenCredential(refreshToken, httpAgent); } @@ -403,7 +393,7 @@ function getDetailFromResponse(response: HttpResponse): string { function credentialFromFile(filePath: string, httpAgent?: Agent): Credential { const credentialsFile = readCredentialFile(filePath); - if (typeof credentialsFile !== 'object') { + if (typeof credentialsFile !== 'object' || credentialsFile === null) { throw new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, 'Failed to parse contents of the credentials file as an object', @@ -424,7 +414,7 @@ function credentialFromFile(filePath: string, httpAgent?: Agent): Credential { ); } -function readCredentialFile(filePath: string): {[key: string]: any} { +function readCredentialFile(filePath: string, ignoreMissing?: boolean): {[key: string]: any} | null { if (typeof filePath !== 'string') { throw new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, @@ -436,6 +426,10 @@ function readCredentialFile(filePath: string): {[key: string]: any} { try { fileText = fs.readFileSync(filePath, 'utf8'); } catch (error) { + if (ignoreMissing) { + return null; + } + throw new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, `Failed to read credentials from file ${filePath}: ` + error, From 951d5a06a78def1f39911c48456e7cf46cfb5b3f Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 16 Dec 2019 14:28:22 -0800 Subject: [PATCH 3/7] Updated comments --- src/auth/credential.ts | 4 ++-- src/firebase-app.ts | 2 +- src/firestore/firestore.ts | 5 ++--- src/storage/storage.ts | 4 ++-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/auth/credential.ts b/src/auth/credential.ts index 2a933ba728..616a171b73 100644 --- a/src/auth/credential.ts +++ b/src/auth/credential.ts @@ -70,7 +70,7 @@ export interface Credential { } /** - * Implementation of Credential that uses a service account certificate. + * Implementation of Credential that uses a service account. */ export class ServiceAccountCredential implements Credential { @@ -132,7 +132,7 @@ export class ServiceAccountCredential implements Credential { } /** - * A struct containing the properties necessary to use service-account JSON credentials. + * A struct containing the properties necessary to use service account JSON credentials. */ class ServiceAccount { diff --git a/src/firebase-app.ts b/src/firebase-app.ts index 9dccd51f9d..b58f6ac019 100644 --- a/src/firebase-app.ts +++ b/src/firebase-app.ts @@ -264,7 +264,7 @@ export class FirebaseApp { const hasCredential = ('credential' in this.options_); if (!hasCredential) { - this.options_.credential = getApplicationDefault(this.options.httpAgent); + this.options_.credential = getApplicationDefault(this.options_.httpAgent); } const credential = this.options_.credential; diff --git a/src/firestore/firestore.ts b/src/firestore/firestore.ts index 2364c531c1..7a2ad8af81 100644 --- a/src/firestore/firestore.ts +++ b/src/firestore/firestore.ts @@ -75,14 +75,13 @@ export function getFirestoreOptions(app: FirebaseApp): Settings { const credential = app.options.credential; const { version: firebaseVersion } = require('../../package.json'); if (credential instanceof ServiceAccountCredential) { - // cert is available when the SDK has been initialized with a service account JSON file, - // or by setting the GOOGLE_APPLICATION_CREDENTIALS envrionment variable. - return { credentials: { private_key: credential.privateKey, client_email: credential.clientEmail, }, + // When the SDK is initialized with ServiceAccountCredentials projectId is guaranteed to + // be available. projectId: projectId!, firebaseVersion, }; diff --git a/src/storage/storage.ts b/src/storage/storage.ts index 41b9ff23b5..b9cf0e9706 100644 --- a/src/storage/storage.ts +++ b/src/storage/storage.ts @@ -74,9 +74,9 @@ export class Storage implements FirebaseServiceInterface { const projectId: string | null = utils.getProjectId(app); const credential = app.options.credential; if (credential instanceof ServiceAccountCredential) { - // cert is available when the SDK has been initialized with a service account JSON file, - // or by setting the GOOGLE_APPLICATION_CREDENTIALS envrionment variable. this.storageClient = new storage({ + // When the SDK is initialized with ServiceAccountCredentials projectId is guaranteed to + // be available. projectId: projectId!, credentials: { private_key: credential.privateKey, From 7500c6340eada642b6e52e486b556c0f320601bc Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 16 Dec 2019 15:15:14 -0800 Subject: [PATCH 4/7] Added more tests --- src/auth/credential.ts | 23 ------------ test/unit/auth/credential.spec.ts | 60 +++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/auth/credential.ts b/src/auth/credential.ts index 616a171b73..b5c805ac90 100644 --- a/src/auth/credential.ts +++ b/src/auth/credential.ts @@ -141,14 +141,6 @@ class ServiceAccount { public readonly clientEmail: string; public static fromPath(filePath: string): ServiceAccount { - // Node bug encountered in v6.x. fs.readFileSync hangs when path is a 0 or 1. - if (typeof filePath !== 'string') { - throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse service account json file: TypeError: path must be a string', - ); - } - try { return new ServiceAccount(JSON.parse(fs.readFileSync(filePath, 'utf8'))); } catch (error) { @@ -283,14 +275,6 @@ class RefreshToken { * data at the path is invalid. */ public static fromPath(filePath: string): RefreshToken { - // Node bug encountered in v6.x. fs.readFileSync hangs when path is a 0 or 1. - if (typeof filePath !== 'string') { - throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse service account json file: TypeError: path must be a string', - ); - } - try { return new RefreshToken(JSON.parse(fs.readFileSync(filePath, 'utf8'))); } catch (error) { @@ -415,13 +399,6 @@ function credentialFromFile(filePath: string, httpAgent?: Agent): Credential { } function readCredentialFile(filePath: string, ignoreMissing?: boolean): {[key: string]: any} | null { - if (typeof filePath !== 'string') { - throw new FirebaseAppError( - AppErrorCodes.INVALID_CREDENTIAL, - 'Failed to parse credentials file: TypeError: path must be a string', - ); - } - let fileText: string; try { fileText = fs.readFileSync(filePath, 'utf8'); diff --git a/test/unit/auth/credential.spec.ts b/test/unit/auth/credential.spec.ts index 0fe5c9a189..fa9b3dc2f6 100644 --- a/test/unit/auth/credential.spec.ts +++ b/test/unit/auth/credential.spec.ts @@ -106,9 +106,9 @@ describe('Credential', () => { }); it('should throw if called with the path to an invalid file', () => { - const invalidPath = path.resolve(__dirname, '../../resources/unparesable.json'); + const invalidPath = path.resolve(__dirname, '../../resources/unparsable.key.json'); expect(() => new ServiceAccountCredential(invalidPath)) - .to.throw('Failed to parse service account json file: Error: ENOENT: no such file or directory'); + .to.throw('Failed to parse service account json file: SyntaxError'); }); it('should throw if called with an empty string path', () => { @@ -116,6 +116,12 @@ describe('Credential', () => { .to.throw('Failed to parse service account json file: Error: ENOENT: no such file or directory'); }); + it('should throw given an object without a "project_id" property', () => { + const invalidCertificate = _.omit(mocks.certificateObject, 'project_id'); + expect(() => new ServiceAccountCredential(invalidCertificate as any)) + .to.throw('Service account object must contain a string "project_id" property'); + }); + it('should throw given an object without a "private_key" property', () => { const invalidCertificate = _.omit(mocks.certificateObject, 'private_key'); expect(() => new ServiceAccountCredential(invalidCertificate as any)) @@ -142,6 +148,13 @@ describe('Credential', () => { .to.throw('Service account object must contain a string "client_email" property'); }); + it('should throw given an object with a malformed "private_key" property', () => { + const invalidCertificate = _.clone(mocks.certificateObject); + invalidCertificate.private_key = 'malformed'; + expect(() => new ServiceAccountCredential(invalidCertificate as any)) + .to.throw('Failed to parse private key'); + }); + it('should not throw given a valid path to a key file', () => { const validPath = path.resolve(__dirname, '../../resources/mock.key.json'); expect(() => new ServiceAccountCredential(validPath)).not.to.throw(); @@ -212,10 +225,47 @@ describe('Credential', () => { return expect(c.getAccessToken()).to.be .rejectedWith('Error fetching access token: not json'); }); + + it('should throw when the success response is malformed', () => { + httpStub.resolves(utils.responseFrom({})); + const c = new ServiceAccountCredential(mockCertificateObject); + return expect(c.getAccessToken()).to.be + .rejectedWith('Unexpected response while fetching access token'); + }); }); }); describe('RefreshTokenCredential', () => { + it('should throw if called with the path to an invalid file', () => { + const invalidPath = path.resolve(__dirname, '../../resources/unparsable.key.json'); + expect(() => new RefreshTokenCredential(invalidPath)) + .to.throw('Failed to parse refresh token file'); + }); + + it('should throw given an object without a "clientId" property', () => { + const invalidCredential = _.omit(mocks.refreshToken, 'clientId'); + expect(() => new RefreshTokenCredential(invalidCredential as any)) + .to.throw('Refresh token must contain a "client_id" property'); + }); + + it('should throw given an object without a "clientSecret" property', () => { + const invalidCredential = _.omit(mocks.refreshToken, 'clientSecret'); + expect(() => new RefreshTokenCredential(invalidCredential as any)) + .to.throw('Refresh token must contain a "client_secret" property'); + }); + + it('should throw given an object without a "refreshToken" property', () => { + const invalidCredential = _.omit(mocks.refreshToken, 'refreshToken'); + expect(() => new RefreshTokenCredential(invalidCredential as any)) + .to.throw('Refresh token must contain a "refresh_token" property'); + }); + + it('should throw given an object without a "type" property', () => { + const invalidCredential = _.omit(mocks.refreshToken, 'type'); + expect(() => new RefreshTokenCredential(invalidCredential as any)) + .to.throw('Refresh token must contain a "type" property'); + }); + it('should create access tokens', () => { const scope = nock('https://www.googleapis.com') .post('/oauth2/v4/token') @@ -336,6 +386,12 @@ describe('Credential', () => { expect(() => getApplicationDefault()).to.throw(Error); }); + it('should throw if a the credentials file content is not an object', () => { + process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); + fsStub = sinon.stub(fs, 'readFileSync').returns('2'); + expect(() => getApplicationDefault()).to.throw(Error); + }); + it('should return a MetadataServiceCredential as a last resort', () => { delete process.env.GOOGLE_APPLICATION_CREDENTIALS; fsStub = sinon.stub(fs, 'readFileSync').throws(new Error('no gcloud credential file')); From 4abfab789d432c2a3adaa2f19fb377dda7f0a293 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 16 Dec 2019 15:41:05 -0800 Subject: [PATCH 5/7] Fixed test for GCP environment --- test/unit/auth/auth.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index ad98b6f0ed..83f18ece18 100755 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -357,7 +357,7 @@ AUTH_CONFIGS.forEach((testConfig) => { } it('should be eventually rejected if a cert credential is not specified', () => { - const mockCredentialAuth = testConfig.init(mocks.mockCredentialApp()); + const mockCredentialAuth = testConfig.init(mocks.appRejectedWhileFetchingAccessToken()); return mockCredentialAuth.createCustomToken(mocks.uid, mocks.developerClaims) .should.eventually.be.rejected.and.have.property('code', 'auth/invalid-credential'); }); From 0cef44d72341fe6c43985603efc4c59685de2cfc Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 16 Dec 2019 16:00:44 -0800 Subject: [PATCH 6/7] Fixed failing IAMSigner test --- test/unit/auth/auth.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index 83f18ece18..0451be7288 100755 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -43,6 +43,7 @@ import { import {deepCopy} from '../../../src/utils/deep-copy'; import { TenantManager } from '../../../src/auth/tenant-manager'; import { ServiceAccountCredential } from '../../../src/auth/credential'; +import { HttpClient } from '../../../src/utils/api-request'; chai.should(); chai.use(sinonChai); @@ -357,7 +358,9 @@ AUTH_CONFIGS.forEach((testConfig) => { } it('should be eventually rejected if a cert credential is not specified', () => { - const mockCredentialAuth = testConfig.init(mocks.appRejectedWhileFetchingAccessToken()); + const mockCredentialAuth = testConfig.init(mocks.mockCredentialApp()); + // Force the service account ID discovery to fail. + getTokenStub = sinon.stub(HttpClient.prototype, 'send').rejects(utils.errorFrom({})); return mockCredentialAuth.createCustomToken(mocks.uid, mocks.developerClaims) .should.eventually.be.rejected.and.have.property('code', 'auth/invalid-credential'); }); From facbc30d4d375245de133928030ba09406d71ccd Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 6 Jan 2020 14:45:27 -0800 Subject: [PATCH 7/7] Added some docs; Improved validation logic --- src/auth/credential.ts | 28 ++++++++++++++++++++-------- test/unit/auth/credential.spec.ts | 2 +- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/auth/credential.ts b/src/auth/credential.ts index b5c805ac90..895032d352 100644 --- a/src/auth/credential.ts +++ b/src/auth/credential.ts @@ -22,6 +22,7 @@ import path = require('path'); import {AppErrorCodes, FirebaseAppError} from '../utils/error'; import {HttpClient, HttpRequestConfig, HttpError, HttpResponse} from '../utils/api-request'; import {Agent} from 'http'; +import * as util from '../utils/validator'; const GOOGLE_TOKEN_AUDIENCE = 'https://accounts.google.com/o/oauth2/token'; const GOOGLE_AUTH_TOKEN_HOST = 'accounts.google.com'; @@ -153,7 +154,7 @@ class ServiceAccount { } constructor(json: object) { - if (typeof json !== 'object' || json === null) { + if (!util.isNonNullObject(json)) { throw new FirebaseAppError( AppErrorCodes.INVALID_CREDENTIAL, 'Service account must be an object.', @@ -165,11 +166,11 @@ class ServiceAccount { copyAttr(this, json, 'clientEmail', 'client_email'); let errorMessage; - if (typeof this.projectId !== 'string' || !this.projectId) { + if (!util.isNonEmptyString(this.projectId)) { errorMessage = 'Service account object must contain a string "project_id" property.'; - } else if (typeof this.privateKey !== 'string' || !this.privateKey) { + } else if (!util.isNonEmptyString(this.privateKey)) { errorMessage = 'Service account object must contain a string "private_key" property.'; - } else if (typeof this.clientEmail !== 'string' || !this.clientEmail) { + } else if (!util.isNonEmptyString(this.clientEmail)) { errorMessage = 'Service account object must contain a string "client_email" property.'; } @@ -293,13 +294,13 @@ class RefreshToken { copyAttr(this, json, 'type', 'type'); let errorMessage; - if (typeof this.clientId !== 'string' || !this.clientId) { + if (!util.isNonEmptyString(this.clientId)) { errorMessage = 'Refresh token must contain a "client_id" property.'; - } else if (typeof this.clientSecret !== 'string' || !this.clientSecret) { + } else if (!util.isNonEmptyString(this.clientSecret)) { errorMessage = 'Refresh token must contain a "client_secret" property.'; - } else if (typeof this.refreshToken !== 'string' || !this.refreshToken) { + } else if (!util.isNonEmptyString(this.refreshToken)) { errorMessage = 'Refresh token must contain a "refresh_token" property.'; - } else if (typeof this.type !== 'string' || !this.type) { + } else if (!util.isNonEmptyString(this.type)) { errorMessage = 'Refresh token must contain a "type" property.'; } @@ -325,6 +326,17 @@ export function getApplicationDefault(httpAgent?: Agent): Credential { return new ComputeEngineCredential(httpAgent); } +/** + * Copies the specified property from one object to another. + * + * If no property exists by the given "key", looks for a property identified by "alt", and copies it instead. + * This can be used to implement behaviors such as "copy property myKey or my_key". + * + * @param to Target object to copy the property into. + * @param from Source object to copy the property from. + * @param key Name of the property to copy. + * @param alt Alternative name of the property to copy. + */ function copyAttr(to: {[key: string]: any}, from: {[key: string]: any}, key: string, alt: string) { const tmp = from[key] || from[alt]; if (typeof tmp !== 'undefined') { diff --git a/test/unit/auth/credential.spec.ts b/test/unit/auth/credential.spec.ts index fa9b3dc2f6..c44a5a1e0f 100644 --- a/test/unit/auth/credential.spec.ts +++ b/test/unit/auth/credential.spec.ts @@ -386,7 +386,7 @@ describe('Credential', () => { expect(() => getApplicationDefault()).to.throw(Error); }); - it('should throw if a the credentials file content is not an object', () => { + it('should throw if the credentials file content is not an object', () => { process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); fsStub = sinon.stub(fs, 'readFileSync').returns('2'); expect(() => getApplicationDefault()).to.throw(Error);