Skip to content

Commit

Permalink
Refactored the credentials implementation (#730)
Browse files Browse the repository at this point in the history
* Updated unit tests

* Further cleaned up the credential impl

* Updated comments

* Added more tests

* Fixed test for GCP environment

* Fixed failing IAMSigner test

* Added some docs; Improved validation logic
  • Loading branch information
hiranya911 committed Jan 6, 2020
1 parent f66c90a commit e953b34
Show file tree
Hide file tree
Showing 15 changed files with 470 additions and 565 deletions.
505 changes: 231 additions & 274 deletions src/auth/credential.ts

Large diffs are not rendered by default.

34 changes: 12 additions & 22 deletions src/auth/token-generator.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -82,28 +82,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;
}

/**
Expand All @@ -113,14 +104,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<string> {
return Promise.resolve(this.certificate.clientEmail);
return Promise.resolve(this.credential.clientEmail);
}
}

Expand Down Expand Up @@ -232,12 +223,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);
}

Expand Down
4 changes: 2 additions & 2 deletions src/firebase-app.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 7 additions & 7 deletions src/firebase-namespace.ts
Expand Up @@ -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';
Expand All @@ -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 } = {};


Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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];
},
Expand All @@ -291,7 +291,7 @@ const firebaseCredential = {

applicationDefault: (httpAgent?: Agent): Credential => {
if (typeof globalAppDefaultCred === 'undefined') {
globalAppDefaultCred = new ApplicationDefaultCredential(httpAgent);
globalAppDefaultCred = getApplicationDefault(httpAgent);
}
return globalAppDefaultCred;
},
Expand Down
28 changes: 9 additions & 19 deletions src/firestore/firestore.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -72,30 +72,20 @@ 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) {
// 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.',
});
}
if (credential instanceof ServiceAccountCredential) {
return {
credentials: {
private_key: cert.privateKey,
client_email: cert.clientEmail,
private_key: credential.privateKey,
client_email: credential.clientEmail,
},
projectId,
// When the SDK is initialized with ServiceAccountCredentials projectId is guaranteed to
// be available.
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.
Expand Down
20 changes: 11 additions & 9 deletions src/storage/storage.ts
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -70,18 +71,19 @@ export class Storage implements FirebaseServiceInterface {
});
}

const cert: Certificate | null = tryGetCertificate(app.options.credential);
if (cert != null) {
// cert is available when the SDK has been initialized with a service account JSON file,
// or by setting the GOOGLE_APPLICATION_CREDENTIALS envrionment variable.
const projectId: string | null = utils.getProjectId(app);
const credential = app.options.credential;
if (credential instanceof ServiceAccountCredential) {
this.storageClient = new storage({
projectId: cert.projectId,
// When the SDK is initialized with ServiceAccountCredentials projectId is guaranteed to
// be available.
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 {
Expand Down
8 changes: 4 additions & 4 deletions src/utils/index.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down
16 changes: 6 additions & 10 deletions test/resources/mocks.ts
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -85,10 +85,6 @@ export class MockCredential implements Credential {
expires_in: 3600,
});
}

public getCertificate(): Certificate | null {
return null;
}
}

export function app(): FirebaseApp {
Expand All @@ -111,33 +107,33 @@ export function appWithOptions(options: FirebaseAppOptions): FirebaseApp {
}

export function appReturningNullAccessToken(): FirebaseApp {
const nullFn: () => Promise<GoogleOAuthAccessToken>|null = () => null;
const nullFn: () => Promise<GoogleOAuthAccessToken> | null = () => null;
return new FirebaseApp({
credential: {
getAccessToken: nullFn,
getCertificate: () => credential.getCertificate(),
} as any,
databaseURL,
projectId,
}, appName, new FirebaseNamespace().INTERNAL);
}

export function appReturningMalformedAccessToken(): FirebaseApp {
return new FirebaseApp({
credential: {
getAccessToken: () => 5,
getCertificate: () => credential.getCertificate(),
} as any,
databaseURL,
projectId,
}, appName, new FirebaseNamespace().INTERNAL);
}

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);
}

Expand Down
10 changes: 7 additions & 3 deletions test/unit/auth/auth.spec.ts
Expand Up @@ -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';
import { HttpClient } from '../../../src/utils/api-request';

chai.should();
Expand Down Expand Up @@ -365,20 +366,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;
});
});
Expand Down

0 comments on commit e953b34

Please sign in to comment.