From bbc810a73b8910a3539c6f65a869269acd82709a Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 20 Nov 2017 19:05:51 -0800 Subject: [PATCH 01/10] Implemented instance ID delete API --- src/auth/credential.ts | 1 + src/firebase-app.ts | 12 ++ src/firebase-namespace.ts | 13 ++ src/index.d.ts | 10 ++ src/instance-id/instance-id-request.ts | 103 ++++++++++++ src/instance-id/instance-id.ts | 96 +++++++++++ src/utils/api-request.ts | 15 +- src/utils/error.ts | 34 ++++ test/integration/index.js | 2 + test/integration/instance-id.js | 41 +++++ test/unit/firebase-app.spec.ts | 27 +++ test/unit/firebase-namespace.spec.ts | 32 ++++ test/unit/index.spec.ts | 3 + test/unit/instance-id/instance-id.spec.ts | 193 ++++++++++++++++++++++ test/unit/utils/api-request.spec.ts | 36 ++++ 15 files changed, 613 insertions(+), 5 deletions(-) create mode 100644 src/instance-id/instance-id-request.ts create mode 100644 src/instance-id/instance-id.ts create mode 100644 test/integration/instance-id.js create mode 100644 test/unit/instance-id/instance-id.spec.ts diff --git a/src/auth/credential.ts b/src/auth/credential.ts index e63c6454b6..3d89e2d387 100644 --- a/src/auth/credential.ts +++ b/src/auth/credential.ts @@ -270,6 +270,7 @@ export class CertCredential implements Credential { 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', diff --git a/src/firebase-app.ts b/src/firebase-app.ts index e4fbb5c40e..588faafbc4 100644 --- a/src/firebase-app.ts +++ b/src/firebase-app.ts @@ -29,6 +29,7 @@ import {Database} from '@firebase/database'; import {DatabaseService} from './database/database'; import {Firestore} from '@google-cloud/firestore'; import {FirestoreService} from './firestore/firestore'; +import {InstanceId} from './instance-id/instance-id'; /** * Type representing a callback which is called every time an app lifecycle event occurs. @@ -333,6 +334,17 @@ export class FirebaseApp { return service.client; } + /** + * Returns the InstanceId service instance associated with this app. + * + * @return {InstanceId} The InstanceId service instance of this app. + */ + public instanceId(): InstanceId { + return this.ensureService_('iid', () => { + return new InstanceId(this); + }); + } + /** * Returns the name of the FirebaseApp instance. * diff --git a/src/firebase-namespace.ts b/src/firebase-namespace.ts index de7455c6dc..690c8aaa80 100644 --- a/src/firebase-namespace.ts +++ b/src/firebase-namespace.ts @@ -30,6 +30,7 @@ import {Messaging} from './messaging/messaging'; import {Storage} from './storage/storage'; import {Database} from '@firebase/database'; import {Firestore} from '@google-cloud/firestore'; +import {InstanceId} from './instance-id/instance-id'; const DEFAULT_APP_NAME = '[DEFAULT]'; @@ -338,6 +339,18 @@ export class FirebaseNamespace { return Object.assign(fn, require('@google-cloud/firestore')); } + /** + * Gets the `InstanceId` service namespace. The returned namespace can be used to get the + * `Instance` service for the default app or an explicitly specified app. + */ + get instanceId(): FirebaseServiceNamespace { + const ns: FirebaseNamespace = this; + let fn: FirebaseServiceNamespace = (app?: FirebaseApp) => { + return ns.ensureApp(app).instanceId(); + }; + return Object.assign(fn, {InstanceId}); + } + /** * Initializes the FirebaseApp instance. * diff --git a/src/index.d.ts b/src/index.d.ts index 77a88e03cf..1a0d02fb74 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -58,6 +58,7 @@ declare namespace admin { function messaging(app?: admin.app.App): admin.messaging.Messaging; function storage(app?: admin.app.App): admin.storage.Storage; function firestore(app?: admin.app.App): admin.firestore.Firestore; + function instanceId(app?: admin.app.App): admin.instanceId.InstanceId; function initializeApp(options: admin.AppOptions, name?: string): admin.app.App; } @@ -69,6 +70,7 @@ declare namespace admin.app { auth(): admin.auth.Auth; database(url?: string): admin.database.Database; firestore(): admin.firestore.Firestore; + instanceId(): admin.instanceId.InstanceId; messaging(): admin.messaging.Messaging; storage(): admin.storage.Storage; delete(): Promise; @@ -415,6 +417,14 @@ declare namespace admin.firestore { export import setLogFunction = _firestore.setLogFunction; } +declare namespace admin.instanceId { + interface InstanceId { + app: admin.app.App; + + deleteInstanceId(instanceId: string): Promise; + } +} + declare module 'firebase-admin' { } diff --git a/src/instance-id/instance-id-request.ts b/src/instance-id/instance-id-request.ts new file mode 100644 index 0000000000..b8141efb6f --- /dev/null +++ b/src/instance-id/instance-id-request.ts @@ -0,0 +1,103 @@ +/*! + * Copyright 2017 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {FirebaseApp} from '../firebase-app'; +import {FirebaseError, FirebaseInstanceIdError, InstanceIdClientErrorCode} from '../utils/error'; +import { + HttpMethod, SignedApiRequestHandler, ApiSettings, +} from '../utils/api-request'; + +import * as validator from '../utils/validator'; + +/** Firebase IID backend host. */ +const FIREBASE_IID_HOST = 'console.firebase.google.com'; +/** Firebase IID backend port number. */ +const FIREBASE_IID_PORT = 443; +/** Firebase IID backend path. */ +const FIREBASE_IID_PATH = '/v1/'; +/** Firebase IID request timeout duration in milliseconds. */ +const FIREBASE_IID_TIMEOUT = 10000; + +/** + * Class that provides mechanism to send requests to the Firebase Auth backend endpoints. + */ +export class FirebaseInstanceIdRequestHandler { + + private host: string = FIREBASE_IID_HOST; + private port: number = FIREBASE_IID_PORT; + private timeout: number = FIREBASE_IID_TIMEOUT; + private signedApiRequestHandler: SignedApiRequestHandler; + private path: string; + + /** + * @param {FirebaseApp} app The app used to fetch access tokens to sign API requests. + * @param {string} projectId A Firebase project ID string. + * + * @constructor + */ + constructor(app: FirebaseApp, projectId: string) { + this.signedApiRequestHandler = new SignedApiRequestHandler(app); + this.path = FIREBASE_IID_PATH + `project/${projectId}/instanceId/`; + } + + public deleteInstanceId(instanceId: string): Promise { + if (!validator.isNonEmptyString(instanceId)) { + return Promise.reject(new FirebaseInstanceIdError( + InstanceIdClientErrorCode.INVALID_INSTANCE_ID, + 'Instance ID must be a non-empty string.' + )); + } + return this.invokeRequestHandler(new ApiSettings(instanceId, 'DELETE')); + } + + /** + * Invokes the request handler based on the API settings object passed. + * + * @param {ApiSettings} apiSettings The API endpoint settings to apply to request and response. + * @param {Object} requestData The request data. + * @return {Promise} A promise that resolves with the response. + */ + private invokeRequestHandler(apiSettings: ApiSettings): Promise { + let path: string = this.path + apiSettings.getEndpoint(); + let httpMethod: HttpMethod = apiSettings.getHttpMethod(); + return Promise.resolve() + .then(() => { + return this.signedApiRequestHandler.sendRequest( + this.host, this.port, path, httpMethod, undefined, undefined, this.timeout); + }) + .then((response) => { + return response; + }) + .catch((response) => { + let error; + if (typeof response === 'object' && 'statusCode' in response) { + // response came directly from a non-200 response. + error = response.error; + } else { + // response came from a 200 response. + error = response; + } + + if (error instanceof FirebaseError) { + throw error; + } + throw new FirebaseInstanceIdError( + InstanceIdClientErrorCode.API_ERROR, + JSON.stringify(error) + ); + }); + } +} diff --git a/src/instance-id/instance-id.ts b/src/instance-id/instance-id.ts new file mode 100644 index 0000000000..36fc0ab80a --- /dev/null +++ b/src/instance-id/instance-id.ts @@ -0,0 +1,96 @@ +/*! + * Copyright 2017 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {FirebaseApp} from '../firebase-app'; +import {FirebaseInstanceIdError, InstanceIdClientErrorCode} from '../utils/error'; +import {FirebaseServiceInterface, FirebaseServiceInternalsInterface} from '../firebase-service'; +import {FirebaseInstanceIdRequestHandler} from './instance-id-request'; + +import * as utils from '../utils/index'; +import * as validator from '../utils/validator'; + +/** + * Internals of an InstanceId service instance. + */ +class InstanceIdInternals implements FirebaseServiceInternalsInterface { + /** + * Deletes the service and its associated resources. + * + * @return {Promise<()>} An empty Promise that will be fulfilled when the service is deleted. + */ + public delete(): Promise { + // There are no resources to clean up + return Promise.resolve(undefined); + } +} + +export class InstanceId implements FirebaseServiceInterface { + public INTERNAL: InstanceIdInternals = new InstanceIdInternals(); + + private app_: FirebaseApp; + private requestHandler: FirebaseInstanceIdRequestHandler; + + /** + * @param {Object} app The app for this Auth service. + * @constructor + */ + constructor(app: FirebaseApp) { + if (!validator.isNonNullObject(app) || !('options' in app)) { + throw new FirebaseInstanceIdError( + InstanceIdClientErrorCode.INVALID_ARGUMENT, + 'First argument passed to admin.instanceId() must be a valid Firebase app instance.' + ); + } + + const projectId: string = utils.getProjectId(app); + if (!validator.isNonEmptyString(projectId)) { + // Assert for an explicit projct ID (either via AppOptions or the cert itself). + throw new FirebaseInstanceIdError( + InstanceIdClientErrorCode.INVALID_PROJECT_ID, + 'Failed to determine project ID for InstanceId. Initialize the ' + + 'SDK with service account credentials or set project ID as an app option. ' + + 'Alternatively set the GCLOUD_PROJECT environment variable.', + ); + } + + this.app_ = app; + this.requestHandler = new FirebaseInstanceIdRequestHandler(app, projectId); + } + + /** + * Deletes the specified instance ID from Firebase. This can be used to delete an instance ID + * and associated user data from a Firebase project, pursuant to the General Data protection + * Regulation (GDPR). + * + * @param {string} instanceId The instance ID to be deleted + * @return {Promise} A promise that resolves when the instance ID is successfully deleted. + */ + public deleteInstanceId(instanceId: string): Promise { + return this.requestHandler.deleteInstanceId(instanceId) + .then((result) => { + // Return nothing on success + }); + } + + /** + * Returns the app associated with this InstanceId instance. + * + * @return {FirebaseApp} The app associated with this InstanceId instance. + */ + get app(): FirebaseApp { + return this.app_; + } +} diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index b827b05b50..7eb1bc4abf 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -22,7 +22,7 @@ import {OutgoingHttpHeaders} from 'http'; import https = require('https'); /** Http method type definition. */ -export type HttpMethod = 'GET' | 'POST'; +export type HttpMethod = 'GET' | 'POST' | 'DELETE'; /** API callback function type definition. */ export type ApiCallbackFunction = (data: Object) => void; @@ -224,11 +224,16 @@ export class SignedApiRequestHandler extends HttpRequestHandler { port: number, path: string, httpMethod: HttpMethod, - data: Object, - headers: Object, - timeout: number): Promise { + data?: Object, + headers?: Object, + timeout?: number): Promise { return this.app_.INTERNAL.getToken().then((accessTokenObj) => { - let headersCopy: Object = deepCopy(headers); + let headersCopy: Object; + if (typeof headers !== 'undefined') { + headersCopy = deepCopy(headers); + } else { + headersCopy = {}; + } let authorizationHeaderKey = 'Authorization'; headersCopy[authorizationHeaderKey] = 'Bearer ' + accessTokenObj.accessToken; return super.sendRequest(host, port, path, httpMethod, data, headersCopy, timeout); diff --git a/src/utils/error.ts b/src/utils/error.ts index dfc2829083..29ae5633e6 100644 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -206,6 +206,21 @@ export class FirebaseFirestoreError extends FirebaseError { } } +/** + * Firebase instance ID error code structure. This extends FirebaseError. + * + * @param {ErrorInfo} info The error code info. + * @param {string} [message] The error message. This will override the default + * message if provided. + * @constructor + */ +export class FirebaseInstanceIdError extends FirebaseError { + constructor(info: ErrorInfo, message?: string) { + // Override default message if custom message provided. + super({code: 'instance-id/' + info.code, message: message || info.message}); + } +} + /** * Firebase Messaging error code structure. This extends PrefixedFirebaseError. @@ -472,6 +487,25 @@ export class MessagingClientErrorCode { }; }; +export class InstanceIdClientErrorCode { + public static INVALID_ARGUMENT = { + code: 'invalid-argument', + message: 'Invalid argument provided.', + }; + public static INVALID_PROJECT_ID = { + code: 'invalid-project-id', + message: 'Invalid project ID provided.', + }; + public static INVALID_INSTANCE_ID = { + code: 'invalid-instance-id', + message: 'Invalid instance ID provided.', + }; + public static API_ERROR = { + code: 'api-error', + message: 'Instance ID API call failed.', + }; +} + /** @const {ServerToClientCode} Auth server to client enum error codes. */ const AUTH_SERVER_TO_CLIENT_CODE: ServerToClientCode = { // Claims payload is too large. diff --git a/test/integration/index.js b/test/integration/index.js index f7e08e64ae..f631201c84 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -38,6 +38,7 @@ var utils = require('./utils'); var app = require('./app'); var auth = require('./auth'); var database = require('./database'); +var instanceId = require('./instance-id'); var messaging = require('./messaging'); var storage = require('./storage'); var firestore = require('./firestore'); @@ -239,6 +240,7 @@ return promptForUpdateRules(flags['overwrite']) .then(_.partial(app.test, utils)) .then(_.partial(auth.test, utils)) .then(_.partial(database.test, utils)) + .then(_.partial(instanceId.test, utils)) .then(_.partial(messaging.test, utils)) .then(_.partial(storage.test, utils)) .then(_.partial(firestore.test, utils)) diff --git a/test/integration/instance-id.js b/test/integration/instance-id.js new file mode 100644 index 0000000000..52088f270b --- /dev/null +++ b/test/integration/instance-id.js @@ -0,0 +1,41 @@ +/*! + * Copyright 2017 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +var admin = require('../../lib/index'); + +function test(utils) { + console.log('\nInstanceId:'); + + function testNonExistingInstanceId() { + return admin.instanceId().deleteInstanceId('non-existing') + .then(result => { + utils.logFailure('instanceId()', 'No error thrown for non-existing instance ID'); + }) + .catch(err => { + utils.assert( + err.code == 'instance-id/api-error', + 'admin.instanceId().deleteInstanceId(non-existing)', + 'Invalid error for non-existing instance ID: ' + err); + }); + } + + return Promise.resolve() + .then(testNonExistingInstanceId); +} + +module.exports = { + test: test +} \ No newline at end of file diff --git a/test/unit/firebase-app.spec.ts b/test/unit/firebase-app.spec.ts index 85359a2e1e..eb3db064e7 100644 --- a/test/unit/firebase-app.spec.ts +++ b/test/unit/firebase-app.spec.ts @@ -40,6 +40,7 @@ import {Messaging} from '../../src/messaging/messaging'; import {Storage} from '../../src/storage/storage'; import {Firestore} from '@google-cloud/firestore'; import {Database} from '@firebase/database'; +import {InstanceId} from '../../src/instance-id/instance-id'; chai.should(); chai.use(sinonChai); @@ -370,6 +371,32 @@ describe('FirebaseApp', () => { }); }); + describe('instanceId()', () => { + it('should throw if the app has already been deleted', () => { + const app = firebaseNamespace.initializeApp(mocks.appOptions, mocks.appName); + + return app.delete().then(() => { + expect(() => { + return app.instanceId(); + }).to.throw(`Firebase app named "${mocks.appName}" has already been deleted.`); + }); + }); + + it('should return the InstanceId client', () => { + const app = firebaseNamespace.initializeApp(mocks.appOptions, mocks.appName); + + const iid: InstanceId = app.instanceId(); + expect(iid).not.be.null; + }); + + it('should return a cached version of InstanceId on subsequent calls', () => { + const app = firebaseNamespace.initializeApp(mocks.appOptions, mocks.appName); + const service1: InstanceId = app.instanceId(); + const service2: InstanceId = app.instanceId(); + expect(service1).to.equal(service2); + }); + }); + describe('#[service]()', () => { it('should throw if the app has already been deleted', () => { firebaseNamespace.INTERNAL.registerService(mocks.serviceName, mockServiceFactory); diff --git a/test/unit/firebase-namespace.spec.ts b/test/unit/firebase-namespace.spec.ts index 1a06c60b9a..3415885f44 100644 --- a/test/unit/firebase-namespace.spec.ts +++ b/test/unit/firebase-namespace.spec.ts @@ -48,6 +48,7 @@ import { GeoPoint, setLogFunction, } from '@google-cloud/firestore'; +import {InstanceId} from '../../src/instance-id/instance-id'; chai.should(); chai.use(sinonChai); @@ -550,4 +551,35 @@ describe('FirebaseNamespace', () => { expect(firebaseNamespace.firestore.setLogFunction).to.be.deep.equal(setLogFunction); }); }); + + describe('#instanceId()', () => { + it('should throw when called before initializing an app', () => { + expect(() => { + firebaseNamespace.instanceId(); + }).to.throw(DEFAULT_APP_NOT_FOUND); + }); + + it('should throw when default app is not initialized', () => { + firebaseNamespace.initializeApp(mocks.appOptions, 'testApp'); + expect(() => { + firebaseNamespace.instanceId(); + }).to.throw(DEFAULT_APP_NOT_FOUND); + }); + + it('should return a valid namespace when the default app is initialized', () => { + let app: FirebaseApp = firebaseNamespace.initializeApp(mocks.appOptions); + let iid: InstanceId = firebaseNamespace.instanceId(); + expect(iid).to.not.be.null; + }); + + it('should return a valid namespace when the named app is initialized', () => { + let app: FirebaseApp = firebaseNamespace.initializeApp(mocks.appOptions, 'testApp'); + let iid: InstanceId = firebaseNamespace.instanceId(app); + expect(iid).to.not.be.null; + }); + + it('should return a reference to InstanceId type', () => { + expect(firebaseNamespace.instanceId.InstanceId).to.be.deep.equal(InstanceId); + }); + }); }); diff --git a/test/unit/index.spec.ts b/test/unit/index.spec.ts index de6ede2898..31a375f27a 100644 --- a/test/unit/index.spec.ts +++ b/test/unit/index.spec.ts @@ -43,3 +43,6 @@ import './storage/storage.spec'; // Firestore import './firestore/firestore.spec'; + +// InstanceId +import './instance-id/instance-id.spec'; diff --git a/test/unit/instance-id/instance-id.spec.ts b/test/unit/instance-id/instance-id.spec.ts new file mode 100644 index 0000000000..dbbbd267af --- /dev/null +++ b/test/unit/instance-id/instance-id.spec.ts @@ -0,0 +1,193 @@ +/*! + * Copyright 2017 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +import * as _ from 'lodash'; +import {expect} from 'chai'; +import * as chai from 'chai'; +import * as nock from 'nock'; +import * as sinon from 'sinon'; +import * as sinonChai from 'sinon-chai'; +import * as chaiAsPromised from 'chai-as-promised'; + +import * as utils from '../utils'; +import * as mocks from '../../resources/mocks'; + +import {InstanceId} from '../../../src/instance-id/instance-id'; +import {FirebaseInstanceIdRequestHandler} from '../../../src/instance-id/instance-id-request'; +import {FirebaseApp} from '../../../src/firebase-app'; +import {FirebaseInstanceIdError, InstanceIdClientErrorCode} from '../../../src/utils/error'; + +import * as validator from '../../../src/utils/validator'; + +chai.should(); +chai.use(sinonChai); +chai.use(chaiAsPromised); + +describe('InstanceId', () => { + let iid: InstanceId; + let mockApp: FirebaseApp; + let mockCredentialApp: FirebaseApp; + + let nullAccessTokenClient: InstanceId; + let malformedAccessTokenClient: InstanceId; + let rejectedPromiseAccessTokenClient: InstanceId; + + let gcloudProject: string; + + const noProjectIdError = 'Failed to determine project ID for InstanceId. Initialize the SDK ' + + 'with service account credentials or set project ID as an app option. Alternatively set the ' + + 'GCLOUD_PROJECT environment variable.'; + + before(() => utils.mockFetchAccessTokenRequests()); + + after(() => nock.cleanAll()); + + beforeEach(() => { + mockApp = mocks.app(); + mockCredentialApp = mocks.mockCredentialApp(); + iid = new InstanceId(mockApp); + + gcloudProject = process.env.GCLOUD_PROJECT; + + nullAccessTokenClient = new InstanceId(mocks.appReturningNullAccessToken()); + malformedAccessTokenClient = new InstanceId(mocks.appReturningMalformedAccessToken()); + rejectedPromiseAccessTokenClient = new InstanceId(mocks.appRejectedWhileFetchingAccessToken()); + }); + + afterEach(() => { + process.env.GCLOUD_PROJECT = gcloudProject; + return mockApp.delete(); + }); + + + describe('Constructor', () => { + const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; + invalidApps.forEach((invalidApp) => { + it('should throw given invalid app: ' + JSON.stringify(invalidApp), () => { + expect(() => { + const iidAny: any = InstanceId; + return new iidAny(invalidApp); + }).to.throw('First argument passed to admin.instanceId() must be a valid Firebase app instance.'); + }); + }); + + it('should throw given no app', () => { + expect(() => { + const iidAny: any = InstanceId; + return new iidAny(); + }).to.throw('First argument passed to admin.instanceId() must be a valid Firebase app instance.'); + }); + + it('should throw given an invalid credential without project ID', () => { + // Project ID not set in the environment. + delete process.env.GCLOUD_PROJECT; + expect(() => { + return new InstanceId(mockCredentialApp); + }).to.throw(noProjectIdError); + }); + + it('should not throw given a valid app', () => { + expect(() => { + return new InstanceId(mockApp); + }).not.to.throw(); + }); + }); + + describe('app', () => { + it('returns the app from the constructor', () => { + // We expect referential equality here + expect(iid.app).to.equal(mockApp); + }); + + it('is read-only', () => { + expect(() => { + (iid as any).app = mockApp; + }).to.throw('Cannot set property app of # which has only a getter'); + }); + }); + + describe('deleteInstanceId()', () => { + + // Stubs used to simulate underlying api calls. + let stubs: sinon.SinonStub[] = []; + const expectedError = new FirebaseInstanceIdError(InstanceIdClientErrorCode.API_ERROR); + const testInstanceId = 'test-iid'; + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + }); + + it('should be rejected given no instance ID', () => { + return (iid as any).deleteInstanceId() + .should.eventually.be.rejected.and.have.property('code', 'instance-id/invalid-instance-id'); + }); + + it('should be rejected given an invalid instance ID', () => { + return iid.deleteInstanceId('') + .then(() => { + throw new Error('Unexpected success'); + }) + .catch((error) => { + expect(error).to.have.property('code', 'instance-id/invalid-instance-id'); + }); + }); + + it('should be rejected given an app which returns null access tokens', () => { + return nullAccessTokenClient.deleteInstanceId(testInstanceId) + .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); + }); + + it('should be rejected given an app which returns invalid access tokens', () => { + return malformedAccessTokenClient.deleteInstanceId(testInstanceId) + .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); + }); + + it('should be rejected given an app which fails to generate access tokens', () => { + return rejectedPromiseAccessTokenClient.deleteInstanceId(testInstanceId) + .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); + }); + + it('should resolve without errors on success', () => { + let stub = sinon.stub(FirebaseInstanceIdRequestHandler.prototype, 'deleteInstanceId') + .returns(Promise.resolve(null)); + stubs.push(stub); + return iid.deleteInstanceId(testInstanceId) + .then((result) => { + // Confirm underlying API called with expected parameters. + expect(stub).to.have.been.calledOnce.and.calledWith(testInstanceId); + }); + }); + + it('should throw an error when the backend returns an error', () => { + // Stub getAccountInfoByUid to throw a backend error. + let stub = sinon.stub(FirebaseInstanceIdRequestHandler.prototype, 'deleteInstanceId') + .returns(Promise.reject(expectedError)); + stubs.push(stub); + return iid.deleteInstanceId(testInstanceId) + .then((result) => { + throw new Error('Unexpected success'); + }, (error) => { + // Confirm underlying API called with expected parameters. + expect(stub).to.have.been.calledOnce.and.calledWith(testInstanceId); + // Confirm expected error returned. + expect(error).to.equal(expectedError); + }); + }); + }); +}); \ No newline at end of file diff --git a/test/unit/utils/api-request.spec.ts b/test/unit/utils/api-request.spec.ts index 0597abf24f..b028fbf49b 100644 --- a/test/unit/utils/api-request.spec.ts +++ b/test/unit/utils/api-request.spec.ts @@ -397,6 +397,42 @@ describe('SignedApiRequestHandler', () => { }); }); }); + + describe('sendDeleteRequest', () => { + let mockedRequests: nock.Scope[] = []; + afterEach(() => { + _.forEach(mockedRequests, (mockedRequest) => mockedRequest.done()); + mockedRequests = []; + }); + + const expectedResult = { + users : [ + {localId: 'uid'}, + ], + }; + let stub: sinon.SinonStub; + beforeEach(() => stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest') + .returns(Promise.resolve(expectedResult))); + afterEach(() => stub.restore()); + const headers = { + Authorization: 'Bearer ' + mockAccessToken, + }; + const httpMethod: any = 'DELETE'; + const host = 'www.googleapis.com'; + const port = 443; + const path = '/identitytoolkit/v3/relyingparty/getAccountInfo'; + const timeout = 10000; + it('should resolve successfully with a valid request', () => { + const requestHandler = new SignedApiRequestHandler(mockApp); + return requestHandler.sendRequest( + host, port, path, httpMethod, undefined, undefined, timeout) + .then((result) => { + expect(result).to.deep.equal(expectedResult); + expect(stub).to.have.been.calledOnce.and.calledWith( + host, port, path, httpMethod, undefined, headers, timeout); + }); + }); + }); }); describe('ApiSettings', () => { From 8c2ce2785b22d7eb81f97b8af44de294ed0ae75c Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 22 Nov 2017 13:30:55 -0800 Subject: [PATCH 02/10] Adding a couple of missing new lines --- test/integration/instance-id.js | 2 +- test/unit/instance-id/instance-id.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/instance-id.js b/test/integration/instance-id.js index 52088f270b..b2e496084e 100644 --- a/test/integration/instance-id.js +++ b/test/integration/instance-id.js @@ -38,4 +38,4 @@ function test(utils) { module.exports = { test: test -} \ No newline at end of file +} diff --git a/test/unit/instance-id/instance-id.spec.ts b/test/unit/instance-id/instance-id.spec.ts index dbbbd267af..ebf6e72666 100644 --- a/test/unit/instance-id/instance-id.spec.ts +++ b/test/unit/instance-id/instance-id.spec.ts @@ -190,4 +190,4 @@ describe('InstanceId', () => { }); }); }); -}); \ No newline at end of file +}); From 0e54c792ecd27fa54af9cd88ad711849b1ee5529 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 29 Nov 2017 11:43:39 -0800 Subject: [PATCH 03/10] Improved error handling --- src/instance-id/instance-id-request.ts | 22 +++++++++++++++------- test/integration/instance-id.js | 11 ++++++++--- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/instance-id/instance-id-request.ts b/src/instance-id/instance-id-request.ts index b8141efb6f..e4381ccb16 100644 --- a/src/instance-id/instance-id-request.ts +++ b/src/instance-id/instance-id-request.ts @@ -83,21 +83,29 @@ export class FirebaseInstanceIdRequestHandler { }) .catch((response) => { let error; - if (typeof response === 'object' && 'statusCode' in response) { - // response came directly from a non-200 response. + if (typeof response === 'object' && 'error' in response) { error = response.error; } else { - // response came from a 200 response. error = response; } if (error instanceof FirebaseError) { + // In case of timeouts and other network errors, the API request handler returns a + // FirebaseError wrapped in the response. Simply throw it here. throw error; } - throw new FirebaseInstanceIdError( - InstanceIdClientErrorCode.API_ERROR, - JSON.stringify(error) - ); + + let message: string; + if (response.statusCode === 404) { + message = 'Failed to find the specified instance ID'; + } else if (response.statusCode === 429) { + message = 'Request throttled out by the backend server'; + } else if (response.statusCode === 500) { + message = 'Internal server error'; + } else { + message = JSON.stringify(error); + } + throw new FirebaseInstanceIdError(InstanceIdClientErrorCode.API_ERROR, message); }); } } diff --git a/test/integration/instance-id.js b/test/integration/instance-id.js index b2e496084e..a7f48853b1 100644 --- a/test/integration/instance-id.js +++ b/test/integration/instance-id.js @@ -26,9 +26,14 @@ function test(utils) { }) .catch(err => { utils.assert( - err.code == 'instance-id/api-error', - 'admin.instanceId().deleteInstanceId(non-existing)', - 'Invalid error for non-existing instance ID: ' + err); + err.code == 'instance-id/api-error', + 'admin.instanceId().deleteInstanceId(non-existing)', + 'Invalid error for non-existing instance ID: ' + err); + // TODO: Update error message when backend is fixed. + utils.assert( + err.message == 'Request throttled out by the backend server', + 'admin.instanceId().deleteInstanceId(non-existing)', + 'Invalid error for non-existing instance ID: ' + err) }); } From 29600398c1b25f2620ef120d5ff319a1b2d877bc Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 29 Nov 2017 12:09:49 -0800 Subject: [PATCH 04/10] Improved error handling --- src/instance-id/instance-id-request.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/instance-id/instance-id-request.ts b/src/instance-id/instance-id-request.ts index e4381ccb16..427299a076 100644 --- a/src/instance-id/instance-id-request.ts +++ b/src/instance-id/instance-id-request.ts @@ -97,7 +97,7 @@ export class FirebaseInstanceIdRequestHandler { let message: string; if (response.statusCode === 404) { - message = 'Failed to find the specified instance ID'; + message = 'Failed to find the instance ID: ' + apiSettings.getEndpoint(); } else if (response.statusCode === 429) { message = 'Request throttled out by the backend server'; } else if (response.statusCode === 500) { From e242da6b4a7d920b376f1797e0861ea05068cdea Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Tue, 5 Dec 2017 14:35:06 -0800 Subject: [PATCH 05/10] Updated test --- src/instance-id/instance-id.ts | 2 +- test/integration/instance-id.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/instance-id/instance-id.ts b/src/instance-id/instance-id.ts index 36fc0ab80a..6e00c5d3ab 100644 --- a/src/instance-id/instance-id.ts +++ b/src/instance-id/instance-id.ts @@ -72,7 +72,7 @@ export class InstanceId implements FirebaseServiceInterface { /** * Deletes the specified instance ID from Firebase. This can be used to delete an instance ID - * and associated user data from a Firebase project, pursuant to the General Data protection + * and associated user data from a Firebase project, pursuant to the General Data Protection * Regulation (GDPR). * * @param {string} instanceId The instance ID to be deleted diff --git a/test/integration/instance-id.js b/test/integration/instance-id.js index a7f48853b1..34960835a5 100644 --- a/test/integration/instance-id.js +++ b/test/integration/instance-id.js @@ -29,9 +29,8 @@ function test(utils) { err.code == 'instance-id/api-error', 'admin.instanceId().deleteInstanceId(non-existing)', 'Invalid error for non-existing instance ID: ' + err); - // TODO: Update error message when backend is fixed. utils.assert( - err.message == 'Request throttled out by the backend server', + err.message == 'Failed to find the instance ID: non-existing', 'admin.instanceId().deleteInstanceId(non-existing)', 'Invalid error for non-existing instance ID: ' + err) }); From 7d99cdd44ea28137f3d91a57f0f926453e276e39 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Fri, 8 Dec 2017 14:08:20 -0800 Subject: [PATCH 06/10] Fixed indentation; Updated tests --- src/instance-id/instance-id-request.ts | 2 +- src/instance-id/instance-id.ts | 116 +++++----- src/utils/api-request.ts | 7 +- test/unit/firebase-namespace.spec.ts | 2 + test/unit/instance-id/instance-id.spec.ts | 253 +++++++++++----------- 5 files changed, 186 insertions(+), 194 deletions(-) diff --git a/src/instance-id/instance-id-request.ts b/src/instance-id/instance-id-request.ts index 427299a076..84006dbeaa 100644 --- a/src/instance-id/instance-id-request.ts +++ b/src/instance-id/instance-id-request.ts @@ -32,7 +32,7 @@ const FIREBASE_IID_PATH = '/v1/'; const FIREBASE_IID_TIMEOUT = 10000; /** - * Class that provides mechanism to send requests to the Firebase Auth backend endpoints. + * Class that provides mechanism to send requests to the Firebase Instance ID backend endpoints. */ export class FirebaseInstanceIdRequestHandler { diff --git a/src/instance-id/instance-id.ts b/src/instance-id/instance-id.ts index 6e00c5d3ab..fcc72a613b 100644 --- a/src/instance-id/instance-id.ts +++ b/src/instance-id/instance-id.ts @@ -26,71 +26,71 @@ import * as validator from '../utils/validator'; * Internals of an InstanceId service instance. */ class InstanceIdInternals implements FirebaseServiceInternalsInterface { - /** - * Deletes the service and its associated resources. - * - * @return {Promise<()>} An empty Promise that will be fulfilled when the service is deleted. - */ - public delete(): Promise { - // There are no resources to clean up - return Promise.resolve(undefined); - } + /** + * Deletes the service and its associated resources. + * + * @return {Promise<()>} An empty Promise that will be fulfilled when the service is deleted. + */ + public delete(): Promise { + // There are no resources to clean up + return Promise.resolve(undefined); + } } export class InstanceId implements FirebaseServiceInterface { - public INTERNAL: InstanceIdInternals = new InstanceIdInternals(); - - private app_: FirebaseApp; - private requestHandler: FirebaseInstanceIdRequestHandler; - - /** - * @param {Object} app The app for this Auth service. - * @constructor - */ - constructor(app: FirebaseApp) { - if (!validator.isNonNullObject(app) || !('options' in app)) { - throw new FirebaseInstanceIdError( - InstanceIdClientErrorCode.INVALID_ARGUMENT, - 'First argument passed to admin.instanceId() must be a valid Firebase app instance.' - ); - } + public INTERNAL: InstanceIdInternals = new InstanceIdInternals(); - const projectId: string = utils.getProjectId(app); - if (!validator.isNonEmptyString(projectId)) { - // Assert for an explicit projct ID (either via AppOptions or the cert itself). - throw new FirebaseInstanceIdError( - InstanceIdClientErrorCode.INVALID_PROJECT_ID, - 'Failed to determine project ID for InstanceId. Initialize the ' - + 'SDK with service account credentials or set project ID as an app option. ' - + 'Alternatively set the GCLOUD_PROJECT environment variable.', - ); - } + private app_: FirebaseApp; + private requestHandler: FirebaseInstanceIdRequestHandler; - this.app_ = app; - this.requestHandler = new FirebaseInstanceIdRequestHandler(app, projectId); + /** + * @param {Object} app The app for this InstanceId service. + * @constructor + */ + constructor(app: FirebaseApp) { + if (!validator.isNonNullObject(app) || !('options' in app)) { + throw new FirebaseInstanceIdError( + InstanceIdClientErrorCode.INVALID_ARGUMENT, + 'First argument passed to admin.instanceId() must be a valid Firebase app instance.' + ); } - /** - * Deletes the specified instance ID from Firebase. This can be used to delete an instance ID - * and associated user data from a Firebase project, pursuant to the General Data Protection - * Regulation (GDPR). - * - * @param {string} instanceId The instance ID to be deleted - * @return {Promise} A promise that resolves when the instance ID is successfully deleted. - */ - public deleteInstanceId(instanceId: string): Promise { - return this.requestHandler.deleteInstanceId(instanceId) - .then((result) => { - // Return nothing on success - }); + const projectId: string = utils.getProjectId(app); + if (!validator.isNonEmptyString(projectId)) { + // Assert for an explicit projct ID (either via AppOptions or the cert itself). + throw new FirebaseInstanceIdError( + InstanceIdClientErrorCode.INVALID_PROJECT_ID, + 'Failed to determine project ID for InstanceId. Initialize the ' + + 'SDK with service account credentials or set project ID as an app option. ' + + 'Alternatively set the GCLOUD_PROJECT environment variable.', + ); } - /** - * Returns the app associated with this InstanceId instance. - * - * @return {FirebaseApp} The app associated with this InstanceId instance. - */ - get app(): FirebaseApp { - return this.app_; - } + this.app_ = app; + this.requestHandler = new FirebaseInstanceIdRequestHandler(app, projectId); + } + + /** + * Deletes the specified instance ID from Firebase. This can be used to delete an instance ID + * and associated user data from a Firebase project, pursuant to the General Data Protection + * Regulation (GDPR). + * + * @param {string} instanceId The instance ID to be deleted + * @return {Promise} A promise that resolves when the instance ID is successfully deleted. + */ + public deleteInstanceId(instanceId: string): Promise { + return this.requestHandler.deleteInstanceId(instanceId) + .then((result) => { + // Return nothing on success + }); + } + + /** + * Returns the app associated with this InstanceId instance. + * + * @return {FirebaseApp} The app associated with this InstanceId instance. + */ + get app(): FirebaseApp { + return this.app_; + } } diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index 7eb1bc4abf..f8f5d916fb 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -228,12 +228,7 @@ export class SignedApiRequestHandler extends HttpRequestHandler { headers?: Object, timeout?: number): Promise { return this.app_.INTERNAL.getToken().then((accessTokenObj) => { - let headersCopy: Object; - if (typeof headers !== 'undefined') { - headersCopy = deepCopy(headers); - } else { - headersCopy = {}; - } + let headersCopy: Object = (headers && deepCopy(headers)) || {}; let authorizationHeaderKey = 'Authorization'; headersCopy[authorizationHeaderKey] = 'Bearer ' + accessTokenObj.accessToken; return super.sendRequest(host, port, path, httpMethod, data, headersCopy, timeout); diff --git a/test/unit/firebase-namespace.spec.ts b/test/unit/firebase-namespace.spec.ts index 3415885f44..8f173faf9a 100644 --- a/test/unit/firebase-namespace.spec.ts +++ b/test/unit/firebase-namespace.spec.ts @@ -570,12 +570,14 @@ describe('FirebaseNamespace', () => { let app: FirebaseApp = firebaseNamespace.initializeApp(mocks.appOptions); let iid: InstanceId = firebaseNamespace.instanceId(); expect(iid).to.not.be.null; + expect(iid.app).to.be.deep.equal(app); }); it('should return a valid namespace when the named app is initialized', () => { let app: FirebaseApp = firebaseNamespace.initializeApp(mocks.appOptions, 'testApp'); let iid: InstanceId = firebaseNamespace.instanceId(app); expect(iid).to.not.be.null; + expect(iid.app).to.be.deep.equal(app); }); it('should return a reference to InstanceId type', () => { diff --git a/test/unit/instance-id/instance-id.spec.ts b/test/unit/instance-id/instance-id.spec.ts index ebf6e72666..20406cc30b 100644 --- a/test/unit/instance-id/instance-id.spec.ts +++ b/test/unit/instance-id/instance-id.spec.ts @@ -39,155 +39,150 @@ chai.use(sinonChai); chai.use(chaiAsPromised); describe('InstanceId', () => { - let iid: InstanceId; - let mockApp: FirebaseApp; - let mockCredentialApp: FirebaseApp; - - let nullAccessTokenClient: InstanceId; - let malformedAccessTokenClient: InstanceId; - let rejectedPromiseAccessTokenClient: InstanceId; - - let gcloudProject: string; - - const noProjectIdError = 'Failed to determine project ID for InstanceId. Initialize the SDK ' - + 'with service account credentials or set project ID as an app option. Alternatively set the ' - + 'GCLOUD_PROJECT environment variable.'; - - before(() => utils.mockFetchAccessTokenRequests()); - - after(() => nock.cleanAll()); - - beforeEach(() => { - mockApp = mocks.app(); - mockCredentialApp = mocks.mockCredentialApp(); - iid = new InstanceId(mockApp); - - gcloudProject = process.env.GCLOUD_PROJECT; - - nullAccessTokenClient = new InstanceId(mocks.appReturningNullAccessToken()); - malformedAccessTokenClient = new InstanceId(mocks.appReturningMalformedAccessToken()); - rejectedPromiseAccessTokenClient = new InstanceId(mocks.appRejectedWhileFetchingAccessToken()); - }); - - afterEach(() => { - process.env.GCLOUD_PROJECT = gcloudProject; - return mockApp.delete(); - }); - - - describe('Constructor', () => { - const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; - invalidApps.forEach((invalidApp) => { - it('should throw given invalid app: ' + JSON.stringify(invalidApp), () => { - expect(() => { - const iidAny: any = InstanceId; - return new iidAny(invalidApp); - }).to.throw('First argument passed to admin.instanceId() must be a valid Firebase app instance.'); - }); - }); - - it('should throw given no app', () => { + let iid: InstanceId; + let mockApp: FirebaseApp; + let mockCredentialApp: FirebaseApp; + + let nullAccessTokenClient: InstanceId; + let malformedAccessTokenClient: InstanceId; + let rejectedPromiseAccessTokenClient: InstanceId; + + let gcloudProject: string; + + const noProjectIdError = 'Failed to determine project ID for InstanceId. Initialize the SDK ' + + 'with service account credentials or set project ID as an app option. Alternatively set the ' + + 'GCLOUD_PROJECT environment variable.'; + + before(() => utils.mockFetchAccessTokenRequests()); + + after(() => nock.cleanAll()); + + beforeEach(() => { + mockApp = mocks.app(); + mockCredentialApp = mocks.mockCredentialApp(); + iid = new InstanceId(mockApp); + + gcloudProject = process.env.GCLOUD_PROJECT; + + nullAccessTokenClient = new InstanceId(mocks.appReturningNullAccessToken()); + malformedAccessTokenClient = new InstanceId(mocks.appReturningMalformedAccessToken()); + rejectedPromiseAccessTokenClient = new InstanceId(mocks.appRejectedWhileFetchingAccessToken()); + }); + + afterEach(() => { + process.env.GCLOUD_PROJECT = gcloudProject; + return mockApp.delete(); + }); + + + describe('Constructor', () => { + const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; + invalidApps.forEach((invalidApp) => { + it('should throw given invalid app: ' + JSON.stringify(invalidApp), () => { expect(() => { const iidAny: any = InstanceId; - return new iidAny(); + return new iidAny(invalidApp); }).to.throw('First argument passed to admin.instanceId() must be a valid Firebase app instance.'); }); + }); - it('should throw given an invalid credential without project ID', () => { - // Project ID not set in the environment. - delete process.env.GCLOUD_PROJECT; - expect(() => { - return new InstanceId(mockCredentialApp); - }).to.throw(noProjectIdError); - }); - - it('should not throw given a valid app', () => { - expect(() => { - return new InstanceId(mockApp); - }).not.to.throw(); - }); + it('should throw given no app', () => { + expect(() => { + const iidAny: any = InstanceId; + return new iidAny(); + }).to.throw('First argument passed to admin.instanceId() must be a valid Firebase app instance.'); }); - - describe('app', () => { - it('returns the app from the constructor', () => { - // We expect referential equality here - expect(iid.app).to.equal(mockApp); - }); - - it('is read-only', () => { - expect(() => { - (iid as any).app = mockApp; - }).to.throw('Cannot set property app of # which has only a getter'); - }); + + it('should throw given an invalid credential without project ID', () => { + // Project ID not set in the environment. + delete process.env.GCLOUD_PROJECT; + expect(() => { + return new InstanceId(mockCredentialApp); + }).to.throw(noProjectIdError); }); - describe('deleteInstanceId()', () => { + it('should not throw given a valid app', () => { + expect(() => { + return new InstanceId(mockApp); + }).not.to.throw(); + }); + }); - // Stubs used to simulate underlying api calls. - let stubs: sinon.SinonStub[] = []; - const expectedError = new FirebaseInstanceIdError(InstanceIdClientErrorCode.API_ERROR); - const testInstanceId = 'test-iid'; + describe('app', () => { + it('returns the app from the constructor', () => { + // We expect referential equality here + expect(iid.app).to.equal(mockApp); + }); - afterEach(() => { - _.forEach(stubs, (stub) => stub.restore()); - stubs = []; - }); + it('is read-only', () => { + expect(() => { + (iid as any).app = mockApp; + }).to.throw('Cannot set property app of # which has only a getter'); + }); + }); + + describe('deleteInstanceId()', () => { - it('should be rejected given no instance ID', () => { - return (iid as any).deleteInstanceId() + // Stubs used to simulate underlying api calls. + let stubs: sinon.SinonStub[] = []; + const expectedError = new FirebaseInstanceIdError(InstanceIdClientErrorCode.API_ERROR); + const testInstanceId = 'test-iid'; + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + }); + + it('should be rejected given no instance ID', () => { + return (iid as any).deleteInstanceId() .should.eventually.be.rejected.and.have.property('code', 'instance-id/invalid-instance-id'); - }); + }); - it('should be rejected given an invalid instance ID', () => { - return iid.deleteInstanceId('') - .then(() => { - throw new Error('Unexpected success'); - }) - .catch((error) => { - expect(error).to.have.property('code', 'instance-id/invalid-instance-id'); - }); - }); + it('should be rejected given an invalid instance ID', () => { + return iid.deleteInstanceId('') + .should.eventually.be.rejected.and.have.property('code', 'instance-id/invalid-instance-id'); + }); - it('should be rejected given an app which returns null access tokens', () => { - return nullAccessTokenClient.deleteInstanceId(testInstanceId) + it('should be rejected given an app which returns null access tokens', () => { + return nullAccessTokenClient.deleteInstanceId(testInstanceId) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); - }); + }); - it('should be rejected given an app which returns invalid access tokens', () => { - return malformedAccessTokenClient.deleteInstanceId(testInstanceId) + it('should be rejected given an app which returns invalid access tokens', () => { + return malformedAccessTokenClient.deleteInstanceId(testInstanceId) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); - }); + }); - it('should be rejected given an app which fails to generate access tokens', () => { - return rejectedPromiseAccessTokenClient.deleteInstanceId(testInstanceId) + it('should be rejected given an app which fails to generate access tokens', () => { + return rejectedPromiseAccessTokenClient.deleteInstanceId(testInstanceId) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); - }); + }); - it('should resolve without errors on success', () => { - let stub = sinon.stub(FirebaseInstanceIdRequestHandler.prototype, 'deleteInstanceId') - .returns(Promise.resolve(null)); - stubs.push(stub); - return iid.deleteInstanceId(testInstanceId) - .then((result) => { - // Confirm underlying API called with expected parameters. - expect(stub).to.have.been.calledOnce.and.calledWith(testInstanceId); - }); - }); + it('should resolve without errors on success', () => { + let stub = sinon.stub(FirebaseInstanceIdRequestHandler.prototype, 'deleteInstanceId') + .returns(Promise.resolve(null)); + stubs.push(stub); + return iid.deleteInstanceId(testInstanceId) + .then((result) => { + // Confirm underlying API called with expected parameters. + expect(stub).to.have.been.calledOnce.and.calledWith(testInstanceId); + }); + }); - it('should throw an error when the backend returns an error', () => { - // Stub getAccountInfoByUid to throw a backend error. - let stub = sinon.stub(FirebaseInstanceIdRequestHandler.prototype, 'deleteInstanceId') - .returns(Promise.reject(expectedError)); - stubs.push(stub); - return iid.deleteInstanceId(testInstanceId) - .then((result) => { - throw new Error('Unexpected success'); - }, (error) => { - // Confirm underlying API called with expected parameters. - expect(stub).to.have.been.calledOnce.and.calledWith(testInstanceId); - // Confirm expected error returned. - expect(error).to.equal(expectedError); - }); - }); + it('should throw an error when the backend returns an error', () => { + // Stub deleteInstanceId to throw a backend error. + let stub = sinon.stub(FirebaseInstanceIdRequestHandler.prototype, 'deleteInstanceId') + .returns(Promise.reject(expectedError)); + stubs.push(stub); + return iid.deleteInstanceId(testInstanceId) + .then((result) => { + throw new Error('Unexpected success'); + }, (error) => { + // Confirm underlying API called with expected parameters. + expect(stub).to.have.been.calledOnce.and.calledWith(testInstanceId); + // Confirm expected error returned. + expect(error).to.equal(expectedError); + }); }); + }); }); From 283d2087971f579146b8bbb631edc080b604f5c1 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 13 Dec 2017 14:39:19 -0800 Subject: [PATCH 07/10] Improved error handling and more unit tests --- src/instance-id/instance-id-request.ts | 24 ++- test/unit/index.spec.ts | 1 + .../instance-id/instance-id-request.spec.ts | 137 ++++++++++++++++++ 3 files changed, 155 insertions(+), 7 deletions(-) create mode 100644 test/unit/instance-id/instance-id-request.spec.ts diff --git a/src/instance-id/instance-id-request.ts b/src/instance-id/instance-id-request.ts index 84006dbeaa..ae5f58bebe 100644 --- a/src/instance-id/instance-id-request.ts +++ b/src/instance-id/instance-id-request.ts @@ -20,6 +20,7 @@ import { HttpMethod, SignedApiRequestHandler, ApiSettings, } from '../utils/api-request'; +import * as util from 'util'; import * as validator from '../utils/validator'; /** Firebase IID backend host. */ @@ -31,6 +32,19 @@ const FIREBASE_IID_PATH = '/v1/'; /** Firebase IID request timeout duration in milliseconds. */ const FIREBASE_IID_TIMEOUT = 10000; +/** HTTP error codes raised by the backend server. */ +const ERROR_CODES = { + 400: 'Invalid argument. Instance ID "%s" is malformed.', + 401: 'Request not authorized.', + 403: 'Permission denied. Project does not match instance ID or the client does not have ' + + 'sufficient privileges.', + 404: 'Failed to find the instance ID: "%s".', + 409: 'Instance ID "%s" is already deleted.', + 429: 'Request throttled out by the backend server.', + 500: 'Internal server error.', + 503: 'Backend servers are over capacity. Try again later.', +}; + /** * Class that provides mechanism to send requests to the Firebase Instance ID backend endpoints. */ @@ -95,13 +109,9 @@ export class FirebaseInstanceIdRequestHandler { throw error; } - let message: string; - if (response.statusCode === 404) { - message = 'Failed to find the instance ID: ' + apiSettings.getEndpoint(); - } else if (response.statusCode === 429) { - message = 'Request throttled out by the backend server'; - } else if (response.statusCode === 500) { - message = 'Internal server error'; + let message: string = ERROR_CODES[response.statusCode]; + if (message) { + message = util.format(message, apiSettings.getEndpoint()); } else { message = JSON.stringify(error); } diff --git a/test/unit/index.spec.ts b/test/unit/index.spec.ts index 31a375f27a..5307702df4 100644 --- a/test/unit/index.spec.ts +++ b/test/unit/index.spec.ts @@ -46,3 +46,4 @@ import './firestore/firestore.spec'; // InstanceId import './instance-id/instance-id.spec'; +import './instance-id/instance-id-request.spec'; diff --git a/test/unit/instance-id/instance-id-request.spec.ts b/test/unit/instance-id/instance-id-request.spec.ts new file mode 100644 index 0000000000..f023c62c3d --- /dev/null +++ b/test/unit/instance-id/instance-id-request.spec.ts @@ -0,0 +1,137 @@ +/*! + * Copyright 2017 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +import * as _ from 'lodash'; +import {expect} from 'chai'; +import * as chai from 'chai'; +import * as nock from 'nock'; +import * as sinon from 'sinon'; +import * as sinonChai from 'sinon-chai'; +import * as chaiAsPromised from 'chai-as-promised'; + +import * as utils from '../utils'; +import * as mocks from '../../resources/mocks'; + +import {FirebaseApp} from '../../../src/firebase-app'; +import {HttpRequestHandler} from '../../../src/utils/api-request'; +import { FirebaseInstanceIdRequestHandler } from '../../../src/instance-id/instance-id-request'; +import { FirebaseInstanceIdError, InstanceIdClientErrorCode } from '../../../src/utils/error'; + +chai.should(); +chai.use(sinonChai); +chai.use(chaiAsPromised); + +describe('FirebaseInstanceIdRequestHandler', () => { + let mockApp: FirebaseApp; + let projectId: string = 'test-project-id'; + let mockedRequests: nock.Scope[] = []; + let stubs: sinon.SinonStub[] = []; + let mockAccessToken: string = utils.generateRandomAccessToken(); + let expectedHeaders: Object; + + before(() => utils.mockFetchAccessTokenRequests(mockAccessToken)); + + after(() => { + stubs = []; + nock.cleanAll(); + }); + + beforeEach(() => { + mockApp = mocks.app(); + expectedHeaders = { + Authorization: 'Bearer ' + mockAccessToken, + }; + }); + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + _.forEach(mockedRequests, (mockedRequest) => mockedRequest.done()); + return mockApp.delete(); + }); + + describe('Constructor', () => { + it('should succeed with a FirebaseApp instance', () => { + expect(() => { + return new FirebaseInstanceIdRequestHandler(mockApp, projectId); + }).not.to.throw(Error); + }); + }); + + describe('deleteInstanceId', () => { + const httpMethod = 'DELETE'; + const host = 'console.firebase.google.com'; + const port = 443; + const path = `/v1/project/${projectId}/instanceId/test-iid`; + const timeout = 10000; + + it('should be fulfilled given a valid instance ID', () => { + const expectedResult = {}; + + let stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest') + .returns(Promise.resolve(expectedResult)); + stubs.push(stub); + + const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); + return requestHandler.deleteInstanceId('test-iid') + .then((result) => { + expect(result).to.deep.equal(expectedResult); + expect(stub).to.have.been.calledOnce.and.calledWith( + host, port, path, httpMethod, undefined, expectedHeaders, timeout); + }); + }); + + it('should throw for HTTP 404 errors', () => { + const expectedResult = {'statusCode': 404}; + + let stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest') + .returns(Promise.reject(expectedResult)); + stubs.push(stub); + + const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); + return requestHandler.deleteInstanceId('test-iid') + .should.eventually.be.rejected.and.have.property( + 'message', 'Failed to find the instance ID: "test-iid".'); + }); + + it('should throw for HTTP 409 errors', () => { + const expectedResult = {'statusCode': 409}; + + let stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest') + .returns(Promise.reject(expectedResult)); + stubs.push(stub); + + const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); + return requestHandler.deleteInstanceId('test-iid') + .should.eventually.be.rejected.and.have.property( + 'message', 'Instance ID "test-iid" is already deleted.'); + }); + + it('should throw for unexpected HTTP errors', () => { + const expectedResult = {'statusCode': 511}; + + let stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest') + .returns(Promise.reject(expectedResult)); + stubs.push(stub); + + const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); + return requestHandler.deleteInstanceId('test-iid') + .should.eventually.be.rejected.and.have.property( + 'message', JSON.stringify(expectedResult)); + }); + }); +}); From 79afba33160bec5e0b7fb294dcd51b199c92033a Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 13 Dec 2017 15:01:04 -0800 Subject: [PATCH 08/10] Cleaned up tests --- .../unit/instance-id/instance-id-request.spec.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/test/unit/instance-id/instance-id-request.spec.ts b/test/unit/instance-id/instance-id-request.spec.ts index f023c62c3d..29b7f92d74 100644 --- a/test/unit/instance-id/instance-id-request.spec.ts +++ b/test/unit/instance-id/instance-id-request.spec.ts @@ -30,7 +30,6 @@ import * as mocks from '../../resources/mocks'; import {FirebaseApp} from '../../../src/firebase-app'; import {HttpRequestHandler} from '../../../src/utils/api-request'; import { FirebaseInstanceIdRequestHandler } from '../../../src/instance-id/instance-id-request'; -import { FirebaseInstanceIdError, InstanceIdClientErrorCode } from '../../../src/utils/error'; chai.should(); chai.use(sinonChai); @@ -103,9 +102,8 @@ describe('FirebaseInstanceIdRequestHandler', () => { stubs.push(stub); const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); - return requestHandler.deleteInstanceId('test-iid') - .should.eventually.be.rejected.and.have.property( - 'message', 'Failed to find the instance ID: "test-iid".'); + return expect(requestHandler.deleteInstanceId('test-iid')) + .to.be.rejectedWith('Failed to find the instance ID: "test-iid".'); }); it('should throw for HTTP 409 errors', () => { @@ -116,9 +114,8 @@ describe('FirebaseInstanceIdRequestHandler', () => { stubs.push(stub); const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); - return requestHandler.deleteInstanceId('test-iid') - .should.eventually.be.rejected.and.have.property( - 'message', 'Instance ID "test-iid" is already deleted.'); + return expect(requestHandler.deleteInstanceId('test-iid')) + .to.be.rejectedWith('Instance ID "test-iid" is already deleted.'); }); it('should throw for unexpected HTTP errors', () => { @@ -129,9 +126,8 @@ describe('FirebaseInstanceIdRequestHandler', () => { stubs.push(stub); const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); - return requestHandler.deleteInstanceId('test-iid') - .should.eventually.be.rejected.and.have.property( - 'message', JSON.stringify(expectedResult)); + return expect(requestHandler.deleteInstanceId('test-iid')) + .to.be.rejectedWith(JSON.stringify(expectedResult)); }); }); }); From 1ae5ea1598a4102d03bc6c5e6c0d1369a805bd5e Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 13 Dec 2017 15:25:48 -0800 Subject: [PATCH 09/10] Using simple string replacement instead of format due to the weird semantics of util.format() --- src/instance-id/instance-id-request.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/instance-id/instance-id-request.ts b/src/instance-id/instance-id-request.ts index ae5f58bebe..67da9be887 100644 --- a/src/instance-id/instance-id-request.ts +++ b/src/instance-id/instance-id-request.ts @@ -20,7 +20,6 @@ import { HttpMethod, SignedApiRequestHandler, ApiSettings, } from '../utils/api-request'; -import * as util from 'util'; import * as validator from '../utils/validator'; /** Firebase IID backend host. */ @@ -111,7 +110,7 @@ export class FirebaseInstanceIdRequestHandler { let message: string = ERROR_CODES[response.statusCode]; if (message) { - message = util.format(message, apiSettings.getEndpoint()); + message = message.replace('%s', apiSettings.getEndpoint()); } else { message = JSON.stringify(error); } From f46a53fffc8d4ace7b7871f6832bc74c0b8c036b Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 13 Dec 2017 17:05:56 -0800 Subject: [PATCH 10/10] Checking for error code in unit tests; Updated error message format to be consistent across all SDKs. --- src/instance-id/instance-id-request.ts | 16 +++++----- test/integration/instance-id.js | 2 +- .../instance-id/instance-id-request.spec.ts | 32 +++++++++++++++---- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/instance-id/instance-id-request.ts b/src/instance-id/instance-id-request.ts index 67da9be887..081938577c 100644 --- a/src/instance-id/instance-id-request.ts +++ b/src/instance-id/instance-id-request.ts @@ -33,12 +33,11 @@ const FIREBASE_IID_TIMEOUT = 10000; /** HTTP error codes raised by the backend server. */ const ERROR_CODES = { - 400: 'Invalid argument. Instance ID "%s" is malformed.', + 400: 'Malformed instance ID argument.', 401: 'Request not authorized.', - 403: 'Permission denied. Project does not match instance ID or the client does not have ' + - 'sufficient privileges.', - 404: 'Failed to find the instance ID: "%s".', - 409: 'Instance ID "%s" is already deleted.', + 403: 'Project does not match instance ID or the client does not have sufficient privileges.', + 404: 'Failed to find the instance ID.', + 409: 'Already deleted.', 429: 'Request throttled out by the backend server.', 500: 'Internal server error.', 503: 'Backend servers are over capacity. Try again later.', @@ -108,9 +107,10 @@ export class FirebaseInstanceIdRequestHandler { throw error; } - let message: string = ERROR_CODES[response.statusCode]; - if (message) { - message = message.replace('%s', apiSettings.getEndpoint()); + let template: string = ERROR_CODES[response.statusCode]; + let message: string; + if (template) { + message = `Instance ID "${apiSettings.getEndpoint()}": ${template}`; } else { message = JSON.stringify(error); } diff --git a/test/integration/instance-id.js b/test/integration/instance-id.js index 34960835a5..fbe26139cb 100644 --- a/test/integration/instance-id.js +++ b/test/integration/instance-id.js @@ -30,7 +30,7 @@ function test(utils) { 'admin.instanceId().deleteInstanceId(non-existing)', 'Invalid error for non-existing instance ID: ' + err); utils.assert( - err.message == 'Failed to find the instance ID: non-existing', + err.message == 'Instance ID "non-existing": Failed to find the instance ID.', 'admin.instanceId().deleteInstanceId(non-existing)', 'Invalid error for non-existing instance ID: ' + err) }); diff --git a/test/unit/instance-id/instance-id-request.spec.ts b/test/unit/instance-id/instance-id-request.spec.ts index 29b7f92d74..c1caba6fd8 100644 --- a/test/unit/instance-id/instance-id-request.spec.ts +++ b/test/unit/instance-id/instance-id-request.spec.ts @@ -29,7 +29,7 @@ import * as mocks from '../../resources/mocks'; import {FirebaseApp} from '../../../src/firebase-app'; import {HttpRequestHandler} from '../../../src/utils/api-request'; -import { FirebaseInstanceIdRequestHandler } from '../../../src/instance-id/instance-id-request'; +import {FirebaseInstanceIdRequestHandler} from '../../../src/instance-id/instance-id-request'; chai.should(); chai.use(sinonChai); @@ -102,8 +102,14 @@ describe('FirebaseInstanceIdRequestHandler', () => { stubs.push(stub); const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); - return expect(requestHandler.deleteInstanceId('test-iid')) - .to.be.rejectedWith('Failed to find the instance ID: "test-iid".'); + return requestHandler.deleteInstanceId('test-iid') + .then(() => { + throw new Error('Unexpected success'); + }) + .catch((error) => { + expect(error.code).to.equal('instance-id/api-error'); + expect(error.message).to.equal('Instance ID "test-iid": Failed to find the instance ID.'); + }); }); it('should throw for HTTP 409 errors', () => { @@ -114,8 +120,14 @@ describe('FirebaseInstanceIdRequestHandler', () => { stubs.push(stub); const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); - return expect(requestHandler.deleteInstanceId('test-iid')) - .to.be.rejectedWith('Instance ID "test-iid" is already deleted.'); + return requestHandler.deleteInstanceId('test-iid') + .then(() => { + throw new Error('Unexpected success'); + }) + .catch((error) => { + expect(error.code).to.equal('instance-id/api-error'); + expect(error.message).to.equal('Instance ID "test-iid": Already deleted.'); + }); }); it('should throw for unexpected HTTP errors', () => { @@ -126,8 +138,14 @@ describe('FirebaseInstanceIdRequestHandler', () => { stubs.push(stub); const requestHandler = new FirebaseInstanceIdRequestHandler(mockApp, projectId); - return expect(requestHandler.deleteInstanceId('test-iid')) - .to.be.rejectedWith(JSON.stringify(expectedResult)); + return requestHandler.deleteInstanceId('test-iid') + .then(() => { + throw new Error('Unexpected success'); + }) + .catch((error) => { + expect(error.code).to.equal('instance-id/api-error'); + expect(error.message).to.equal(JSON.stringify(expectedResult)); + }); }); }); });