Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactored the credentials implementation #730

Merged
merged 11 commits into from Jan 6, 2020
493 changes: 219 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)) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we not need this validation with the new changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These validations happen during the construction of ServiceAccountCredential object. So we can assume, they are all present.

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