From 70c4d0f8b37fa740bd02aed9e0e4d2f1ae2ee5bb Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Tue, 21 Jan 2020 10:58:51 -0800 Subject: [PATCH] fix: Differentiating explicitly loaded vs default credentials (#764) * Make it possible to differentiate explicitly loaded vs default credentials * Updated documentation for constructors --- src/auth/credential.ts | 57 ++++++++++++++--- src/firestore/firestore.ts | 4 +- src/storage/storage.ts | 4 +- test/unit/auth/credential.spec.ts | 88 ++++++++++++++++++++++++++- test/unit/firebase.spec.ts | 6 +- test/unit/firestore/firestore.spec.ts | 69 ++++++++++++--------- 6 files changed, 181 insertions(+), 47 deletions(-) diff --git a/src/auth/credential.ts b/src/auth/credential.ts index b74faa2958..cb878ff554 100644 --- a/src/auth/credential.ts +++ b/src/auth/credential.ts @@ -79,11 +79,23 @@ export class ServiceAccountCredential implements Credential { public readonly privateKey: string; public readonly clientEmail: string; - private readonly httpClient: HttpClient; - private readonly httpAgent?: Agent; - constructor(serviceAccountPathOrObject: string | object, httpAgent?: Agent) { + /** + * Creates a new ServiceAccountCredential from the given parameters. + * + * @param serviceAccountPathOrObject Service account json object or path to a service account json file. + * @param httpAgent Optional http.Agent to use when calling the remote token server. + * @param implicit An optinal boolean indicating whether this credential was implicitly discovered from the + * environment, as opposed to being explicitly specified by the developer. + * + * @constructor + */ + constructor( + serviceAccountPathOrObject: string | object, + private readonly httpAgent?: Agent, + readonly implicit: boolean = false) { + const serviceAccount = (typeof serviceAccountPathOrObject === 'string') ? ServiceAccount.fromPath(serviceAccountPathOrObject) : new ServiceAccount(serviceAccountPathOrObject); @@ -91,7 +103,6 @@ export class ServiceAccountCredential implements Credential { this.privateKey = serviceAccount.privateKey; this.clientEmail = serviceAccount.clientEmail; this.httpClient = new HttpClient(); - this.httpAgent = httpAgent; } public getAccessToken(): Promise { @@ -247,14 +258,26 @@ export class RefreshTokenCredential implements Credential { private readonly refreshToken: RefreshToken; private readonly httpClient: HttpClient; - private readonly httpAgent?: Agent; - constructor(refreshTokenPathOrObject: string | object, httpAgent?: Agent) { + /** + * Creates a new RefreshTokenCredential from the given parameters. + * + * @param refreshTokenPathOrObject Refresh token json object or path to a refresh token (user credentials) json file. + * @param httpAgent Optional http.Agent to use when calling the remote token server. + * @param implicit An optinal boolean indicating whether this credential was implicitly discovered from the + * environment, as opposed to being explicitly specified by the developer. + * + * @constructor + */ + constructor( + refreshTokenPathOrObject: string | object, + private readonly httpAgent?: Agent, + readonly implicit: boolean = false) { + this.refreshToken = (typeof refreshTokenPathOrObject === 'string') ? RefreshToken.fromPath(refreshTokenPathOrObject) : new RefreshToken(refreshTokenPathOrObject); this.httpClient = new HttpClient(); - this.httpAgent = httpAgent; } public getAccessToken(): Promise { @@ -331,13 +354,27 @@ export function getApplicationDefault(httpAgent?: Agent): Credential { if (GCLOUD_CREDENTIAL_PATH) { const refreshToken = readCredentialFile(GCLOUD_CREDENTIAL_PATH, true); if (refreshToken) { - return new RefreshTokenCredential(refreshToken, httpAgent); + return new RefreshTokenCredential(refreshToken, httpAgent, true); } } return new ComputeEngineCredential(httpAgent); } +/** + * Checks if the given credential was loaded via the application default credentials mechanism. This + * includes all ComputeEngineCredential instances, and the ServiceAccountCredential and RefreshTokenCredential + * instances that were loaded from well-known files or environment variables, rather than being explicitly + * instantiated. + * + * @param credential The credential instance to check. + */ +export function isApplicationDefault(credential?: Credential): boolean { + return credential instanceof ComputeEngineCredential || + (credential instanceof ServiceAccountCredential && credential.implicit) || + (credential instanceof RefreshTokenCredential && credential.implicit); +} + /** * Copies the specified property from one object to another. * @@ -409,11 +446,11 @@ function credentialFromFile(filePath: string, httpAgent?: Agent): Credential { } if (credentialsFile.type === 'service_account') { - return new ServiceAccountCredential(credentialsFile, httpAgent); + return new ServiceAccountCredential(credentialsFile, httpAgent, true); } if (credentialsFile.type === 'authorized_user') { - return new RefreshTokenCredential(credentialsFile, httpAgent); + return new RefreshTokenCredential(credentialsFile, httpAgent, true); } throw new FirebaseAppError( diff --git a/src/firestore/firestore.ts b/src/firestore/firestore.ts index 827698f29d..c733321da7 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 {ServiceAccountCredential, ComputeEngineCredential} from '../auth/credential'; +import {ServiceAccountCredential, isApplicationDefault} from '../auth/credential'; import {Firestore, Settings} from '@google-cloud/firestore'; import * as validator from '../utils/validator'; @@ -85,7 +85,7 @@ export function getFirestoreOptions(app: FirebaseApp): Settings { projectId: projectId!, firebaseVersion, }; - } else if (app.options.credential instanceof ComputeEngineCredential) { + } else if (isApplicationDefault(app.options.credential)) { // 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 edc0fb44be..6b3ddcaef1 100644 --- a/src/storage/storage.ts +++ b/src/storage/storage.ts @@ -17,7 +17,7 @@ import {FirebaseApp} from '../firebase-app'; import {FirebaseError} from '../utils/error'; import {FirebaseServiceInterface, FirebaseServiceInternalsInterface} from '../firebase-service'; -import {ServiceAccountCredential, ComputeEngineCredential} from '../auth/credential'; +import {ServiceAccountCredential, isApplicationDefault} from '../auth/credential'; import {Bucket, Storage as StorageClient} from '@google-cloud/storage'; import * as utils from '../utils/index'; @@ -83,7 +83,7 @@ export class Storage implements FirebaseServiceInterface { client_email: credential.clientEmail, }, }); - } else if (app.options.credential instanceof ComputeEngineCredential) { + } else if (isApplicationDefault(app.options.credential)) { // Try to use the Google application default credentials. this.storageClient = new storage(); } else { diff --git a/test/unit/auth/credential.spec.ts b/test/unit/auth/credential.spec.ts index 2623134a24..0b9ac61925 100644 --- a/test/unit/auth/credential.spec.ts +++ b/test/unit/auth/credential.spec.ts @@ -32,7 +32,7 @@ import * as mocks from '../../resources/mocks'; import { GoogleOAuthAccessToken, RefreshTokenCredential, ServiceAccountCredential, - ComputeEngineCredential, getApplicationDefault, + ComputeEngineCredential, getApplicationDefault, isApplicationDefault, Credential, } from '../../../src/auth/credential'; import { HttpClient } from '../../../src/utils/api-request'; import {Agent} from 'https'; @@ -183,15 +183,17 @@ describe('Credential', () => { projectId: mockCertificateObject.project_id, clientEmail: mockCertificateObject.client_email, privateKey: mockCertificateObject.private_key, + implicit: false, }); }); - it('should return a certificate', () => { - const c = new ServiceAccountCredential(mockCertificateObject); + it('should return an implicit Credential', () => { + const c = new ServiceAccountCredential(mockCertificateObject, undefined, true); expect(c).to.deep.include({ projectId: mockCertificateObject.project_id, clientEmail: mockCertificateObject.client_email, privateKey: mockCertificateObject.private_key, + implicit: true, }); }); @@ -267,6 +269,20 @@ describe('Credential', () => { .to.throw('Refresh token must contain a "type" property'); }); + it('should return a Credential', () => { + const c = new RefreshTokenCredential(mocks.refreshToken); + expect(c).to.deep.include({ + implicit: false, + }); + }); + + it('should return an implicit Credential', () => { + const c = new RefreshTokenCredential(mocks.refreshToken, undefined, true); + expect(c).to.deep.include({ + implicit: true, + }); + }); + it('should create access tokens', () => { const scope = nock('https://www.googleapis.com') .post('/oauth2/v4/token') @@ -477,6 +493,72 @@ describe('Credential', () => { }); }); + describe('isApplicationDefault()', () => { + let fsStub: sinon.SinonStub; + + afterEach(() => { + if (fsStub) { + fsStub.restore(); + } + }); + + it('should return true for ServiceAccountCredential loaded from GOOGLE_APPLICATION_CREDENTIALS', () => { + process.env.GOOGLE_APPLICATION_CREDENTIALS = path.resolve(__dirname, '../../resources/mock.key.json'); + const c = getApplicationDefault(); + expect(c).to.be.an.instanceof(ServiceAccountCredential); + expect(isApplicationDefault(c)).to.be.true; + }); + + it('should return true for RefreshTokenCredential loaded from GOOGLE_APPLICATION_CREDENTIALS', () => { + process.env.GOOGLE_APPLICATION_CREDENTIALS = GCLOUD_CREDENTIAL_PATH; + fsStub = sinon.stub(fs, 'readFileSync').returns(JSON.stringify(MOCK_REFRESH_TOKEN_CONFIG)); + const c = getApplicationDefault(); + expect(c).is.instanceOf(RefreshTokenCredential); + expect(isApplicationDefault(c)).to.be.true; + }); + + it('should return true for credential loaded from gcloud SDK', () => { + if (!fs.existsSync(GCLOUD_CREDENTIAL_PATH)) { + // tslint:disable-next-line:no-console + console.log( + 'WARNING: Test being skipped because gcloud credentials not found. Run `gcloud beta auth ' + + 'application-default login`.'); + return; + } + delete process.env.GOOGLE_APPLICATION_CREDENTIALS; + const c = getApplicationDefault(); + expect(c).to.be.an.instanceof(RefreshTokenCredential); + expect(isApplicationDefault(c)).to.be.true; + }); + + it('should return true for ComputeEngineCredential', () => { + delete process.env.GOOGLE_APPLICATION_CREDENTIALS; + fsStub = sinon.stub(fs, 'readFileSync').throws(new Error('no gcloud credential file')); + const c = getApplicationDefault(); + expect(c).to.be.an.instanceof(ComputeEngineCredential); + expect(isApplicationDefault(c)).to.be.true; + }); + + it('should return false for explicitly loaded ServiceAccountCredential', () => { + const c = new ServiceAccountCredential(mockCertificateObject); + expect(isApplicationDefault(c)).to.be.false; + }); + + it('should return false for explicitly loaded RefreshTokenCredential', () => { + const c = new RefreshTokenCredential(mocks.refreshToken); + expect(isApplicationDefault(c)).to.be.false; + }); + + it('should return false for custom credential', () => { + const c: Credential = { + getAccessToken: () => { + throw new Error(); + }, + }; + expect(isApplicationDefault(c)).to.be.false; + }); + }); + describe('HTTP Agent', () => { const expectedToken = utils.generateRandomAccessToken(); let stub: sinon.SinonStub; diff --git a/test/unit/firebase.spec.ts b/test/unit/firebase.spec.ts index 1c4786ade8..913f3c5067 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 {RefreshTokenCredential, ServiceAccountCredential} from '../../src/auth/credential'; +import {RefreshTokenCredential, ServiceAccountCredential, isApplicationDefault} from '../../src/auth/credential'; chai.should(); chai.use(chaiAsPromised); @@ -112,6 +112,7 @@ describe('Firebase', () => { credential: firebaseAdmin.credential.cert(mocks.certificateObject), }); + expect(isApplicationDefault(firebaseAdmin.app().options.credential)).to.be.false; return firebaseAdmin.app().INTERNAL.getToken() .should.eventually.have.keys(['accessToken', 'expirationTime']); }); @@ -122,6 +123,7 @@ describe('Firebase', () => { credential: firebaseAdmin.credential.cert(keyPath), }); + expect(isApplicationDefault(firebaseAdmin.app().options.credential)).to.be.false; return firebaseAdmin.app().INTERNAL.getToken() .should.eventually.have.keys(['accessToken', 'expirationTime']); }); @@ -134,6 +136,7 @@ describe('Firebase', () => { credential: firebaseAdmin.credential.applicationDefault(), }); + expect(isApplicationDefault(firebaseAdmin.app().options.credential)).to.be.true; return firebaseAdmin.app().INTERNAL.getToken().then((token) => { if (typeof credPath === 'undefined') { delete process.env.GOOGLE_APPLICATION_CREDENTIALS; @@ -155,6 +158,7 @@ describe('Firebase', () => { credential: firebaseAdmin.credential.refreshToken(mocks.refreshToken), }); + expect(isApplicationDefault(firebaseAdmin.app().options.credential)).to.be.false; return firebaseAdmin.app().INTERNAL.getToken() .should.eventually.have.keys(['accessToken', 'expirationTime']); }); diff --git a/test/unit/firestore/firestore.spec.ts b/test/unit/firestore/firestore.spec.ts index ed3e5b7409..df72549b40 100644 --- a/test/unit/firestore/firestore.spec.ts +++ b/test/unit/firestore/firestore.spec.ts @@ -16,19 +16,17 @@ 'use strict'; -import path = require('path'); import * as _ from 'lodash'; import {expect} from 'chai'; import * as mocks from '../../resources/mocks'; import {FirebaseApp} from '../../../src/firebase-app'; -import { ComputeEngineCredential } from '../../../src/auth/credential'; +import { ComputeEngineCredential, RefreshTokenCredential } from '../../../src/auth/credential'; import {FirestoreService, getFirestoreOptions} from '../../../src/firestore/firestore'; describe('Firestore', () => { let mockApp: FirebaseApp; let mockCredentialApp: FirebaseApp; - let defaultCredentialApp: FirebaseApp; let projectIdApp: FirebaseApp; let firestore: any; @@ -40,8 +38,21 @@ describe('Firestore', () => { + 'credentials. Must initialize the SDK with a certificate credential or application default ' + 'credentials to use Cloud Firestore API.'; - const mockServiceAccount = path.resolve(__dirname, '../../resources/mock.key.json'); const { version: firebaseVersion } = require('../../../package.json'); + const defaultCredentialApps = [ + { + name: 'ComputeEngineCredentials', + app: mocks.appWithOptions({ + credential: new ComputeEngineCredential(), + }), + }, + { + name: 'RefreshTokenCredentials', + app: mocks.appWithOptions({ + credential: new RefreshTokenCredential(mocks.refreshToken, undefined, true), + }), + }, + ]; beforeEach(() => { appCredentials = process.env.GOOGLE_APPLICATION_CREDENTIALS; @@ -51,9 +62,6 @@ describe('Firestore', () => { mockApp = mocks.app(); mockCredentialApp = mocks.mockCredentialApp(); - defaultCredentialApp = mocks.appWithOptions({ - credential: new ComputeEngineCredential(), - }); projectIdApp = mocks.appWithOptions({ credential: mocks.credential, projectId: 'explicit-project-id', @@ -121,14 +129,15 @@ describe('Firestore', () => { }).not.to.throw(); }); - it('should not throw given application default credentials without project ID', () => { - // Project ID not set in the environment. - delete process.env.GOOGLE_CLOUD_PROJECT; - delete process.env.GCLOUD_PROJECT; - process.env.GOOGLE_APPLICATION_CREDENTIALS = mockServiceAccount; - expect(() => { - return new FirestoreService(defaultCredentialApp); - }).not.to.throw(); + defaultCredentialApps.forEach((config) => { + it(`should not throw given default ${config.name} without project ID`, () => { + // Project ID not set in the environment. + delete process.env.GOOGLE_CLOUD_PROJECT; + delete process.env.GCLOUD_PROJECT; + expect(() => { + return new FirestoreService(config.app); + }).not.to.throw(); + }); }); }); @@ -169,18 +178,18 @@ describe('Firestore', () => { expect(options.projectId).to.equal('explicit-project-id'); }); - it('should return a string when GOOGLE_CLOUD_PROJECT is set', () => { - process.env.GOOGLE_CLOUD_PROJECT = 'env-project-id'; - process.env.GOOGLE_APPLICATION_CREDENTIALS = mockServiceAccount; - const options = getFirestoreOptions(defaultCredentialApp); - expect(options.projectId).to.equal('env-project-id'); - }); + defaultCredentialApps.forEach((config) => { + it(`should return a string when GOOGLE_CLOUD_PROJECT is set with ${config.name}`, () => { + process.env.GOOGLE_CLOUD_PROJECT = 'env-project-id'; + const options = getFirestoreOptions(config.app); + expect(options.projectId).to.equal('env-project-id'); + }); - it('should return a string when GCLOUD_PROJECT is set', () => { - process.env.GCLOUD_PROJECT = 'env-project-id'; - process.env.GOOGLE_APPLICATION_CREDENTIALS = mockServiceAccount; - const options = getFirestoreOptions(defaultCredentialApp); - expect(options.projectId).to.equal('env-project-id'); + it(`should return a string when GCLOUD_PROJECT is set with ${config.name}`, () => { + process.env.GCLOUD_PROJECT = 'env-project-id'; + const options = getFirestoreOptions(config.app); + expect(options.projectId).to.equal('env-project-id'); + }); }); }); @@ -190,9 +199,11 @@ describe('Firestore', () => { expect(options.firebaseVersion).to.equal(firebaseVersion); }); - it('should return firebaseVersion when using credential without service account certificate', () => { - const options = getFirestoreOptions(defaultCredentialApp); - expect(options.firebaseVersion).to.equal(firebaseVersion); + defaultCredentialApps.forEach((config) => { + it(`should return firebaseVersion when using default ${config.name}`, () => { + const options = getFirestoreOptions(config.app); + expect(options.firebaseVersion).to.equal(firebaseVersion); + }); }); }); });