From 9b1a25a133d7e9a15c5558a979046d09ba752c2e Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 24 Jul 2019 16:35:46 -0700 Subject: [PATCH 1/5] Implementing the Firebase Security Rules API --- .../security-rules-api-client.ts | 86 ++++++ src/security-rules/security-rules-utils.ts | 33 +++ src/security-rules/security-rules.ts | 178 +++++++++++++ src/utils/error.ts | 2 +- test/unit/index.spec.ts | 4 + .../security-rules-api-client.spec.ts | 103 ++++++++ .../security-rules/security-rules.spec.ts | 248 ++++++++++++++++++ 7 files changed, 653 insertions(+), 1 deletion(-) create mode 100644 src/security-rules/security-rules-api-client.ts create mode 100644 src/security-rules/security-rules-utils.ts create mode 100644 src/security-rules/security-rules.ts create mode 100644 test/unit/security-rules/security-rules-api-client.spec.ts create mode 100644 test/unit/security-rules/security-rules.spec.ts diff --git a/src/security-rules/security-rules-api-client.ts b/src/security-rules/security-rules-api-client.ts new file mode 100644 index 0000000000..b2f2a2db58 --- /dev/null +++ b/src/security-rules/security-rules-api-client.ts @@ -0,0 +1,86 @@ +/*! + * Copyright 2019 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 { HttpRequestConfig, HttpClient, HttpError } from '../utils/api-request'; +import { PrefixedFirebaseError } from '../utils/error'; +import { FirebaseSecurityRulesError, SecurityRulesErrorCode } from './security-rules-utils'; + +const RULES_API_URL = 'https://firebaserules.googleapis.com/v1/'; + +/** + * Class that facilitates sending requests to the Firebase security rules backend API. + * + * @private + */ +export class SecurityRulesApiClient { + + constructor(private readonly httpClient: HttpClient) { } + + /** + * Gets the specified resource from the rules API. Resource name must be full qualified names with project + * ID prefix (e.g. `projects/project-id/rulesets/ruleset-name`). + * + * @param {string} name Full qualified name of the resource to get. + * @returns {Promise} A promise that fulfills with the resource. + */ + public getResource(name: string): Promise { + const request: HttpRequestConfig = { + method: 'GET', + url: `${RULES_API_URL}${name}`, + }; + return this.httpClient.send(request) + .then((resp) => { + return resp.data as T; + }) + .catch((err) => { + throw this.toFirebaseError(err); + }); + } + + private toFirebaseError(err: HttpError): PrefixedFirebaseError { + if (err instanceof PrefixedFirebaseError) { + return err; + } + + const response = err.response; + if (!response.isJson()) { + return new FirebaseSecurityRulesError( + 'unknown-error', + `Unexpected response with status: ${response.status} and body: ${response.text}`); + } + + const error: Error = (response.data as ErrorResponse).error || {}; + const code = ERROR_CODE_MAPPING[error.status] || 'unknown-error'; + const message = error.message || `Unknown server error: ${response.text}`; + return new FirebaseSecurityRulesError(code, message); + } +} + +interface ErrorResponse { + error?: Error; +} + +interface Error { + code?: number; + message?: string; + status?: string; +} + +const ERROR_CODE_MAPPING: {[key: string]: SecurityRulesErrorCode} = { + NOT_FOUND: 'not-found', + UNAUTHENTICATED: 'authentication-error', + UNKNOWN: 'unknown-error', +}; diff --git a/src/security-rules/security-rules-utils.ts b/src/security-rules/security-rules-utils.ts new file mode 100644 index 0000000000..21f013f311 --- /dev/null +++ b/src/security-rules/security-rules-utils.ts @@ -0,0 +1,33 @@ +/*! + * Copyright 2019 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 { PrefixedFirebaseError } from '../utils/error'; + +export type SecurityRulesErrorCode = + 'already-exists' + | 'authentication-error' + | 'internal-error' + | 'invalid-argument' + | 'invalid-server-response' + | 'not-found' + | 'service-unavailable' + | 'unknown-error'; + +export class FirebaseSecurityRulesError extends PrefixedFirebaseError { + constructor(code: SecurityRulesErrorCode, message: string) { + super('security-rules', code, message); + } +} diff --git a/src/security-rules/security-rules.ts b/src/security-rules/security-rules.ts new file mode 100644 index 0000000000..fc336c9dbb --- /dev/null +++ b/src/security-rules/security-rules.ts @@ -0,0 +1,178 @@ +/*! + * Copyright 2019 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 { FirebaseServiceInterface, FirebaseServiceInternalsInterface } from '../firebase-service'; +import { FirebaseApp } from '../firebase-app'; +import * as utils from '../utils/index'; +import * as validator from '../utils/validator'; +import { SecurityRulesApiClient } from './security-rules-api-client'; +import { AuthorizedHttpClient } from '../utils/api-request'; +import { FirebaseSecurityRulesError } from './security-rules-utils'; + +/** + * A source file containing some Firebase security rules. + */ +export interface RulesFile { + readonly name: string; + readonly content: string; +} + +/** + * Additional metadata associated with a Ruleset. + */ +export interface RulesetMetadata { + readonly name: string; + readonly createTime: string; +} + +interface Release { + readonly name: string; + readonly rulesetName: string; + readonly createTime: string; + readonly updateTime: string; +} + +interface RulesetResponse { + readonly name: string; + readonly createTime: string; + readonly source: { + readonly files: RulesFile[]; + }; +} + +/** + * Representa a set of Firebase security rules. + */ +export class Ruleset implements RulesetMetadata { + + public readonly name: string; + public readonly createTime: string; + public readonly source: RulesFile[]; + + constructor(ruleset: RulesetResponse) { + if (!validator.isNonNullObject(ruleset) || + !validator.isNonEmptyString(ruleset.name) || + !validator.isNonEmptyString(ruleset.createTime) || + !validator.isNonNullObject(ruleset.source)) { + throw new FirebaseSecurityRulesError( + 'invalid-argument', + `Invalid Ruleset response: ${JSON.stringify(ruleset)}`); + } + + this.name = stripProjectIdPrefix(ruleset.name); + this.createTime = new Date(ruleset.createTime).toUTCString(); + this.source = ruleset.source.files || []; + } +} + +/** + * SecurityRules service bound to the provided app. + */ +export class SecurityRules implements FirebaseServiceInterface { + + private static readonly CLOUD_FIRESTORE = 'cloud.firestore'; + + public readonly INTERNAL = new SecurityRulesInternals(); + + private readonly client: SecurityRulesApiClient; + private readonly projectId: string; + + /** + * @param {object} app The app for this SecurityRules service. + * @constructor + */ + constructor(readonly app: FirebaseApp) { + if (!validator.isNonNullObject(app) || !('options' in app)) { + throw new FirebaseSecurityRulesError( + 'invalid-argument', + 'First argument passed to admin.securityRules() must be a valid Firebase app ' + + 'instance.'); + } + + const projectId = utils.getProjectId(app); + if (!validator.isNonEmptyString(projectId)) { + throw new FirebaseSecurityRulesError( + 'invalid-argument', + 'Failed to determine project ID. Initialize the SDK with service account credentials, or ' + + 'set project ID as an app option. Alternatively, set the GOOGLE_CLOUD_PROJECT ' + + 'environment variable.'); + } + + this.projectId = projectId; + this.client = new SecurityRulesApiClient(new AuthorizedHttpClient(app)); + } + + /** + * Gets the Ruleset identified by the given name. The input name should be the short name string without + * the project ID prefix. Rejects with a `not-found` error if the specified Ruleset cannot be found. + * + * @param {string} name Name of the Ruleset to retrieve. + * @returns {Promise} A promise that fulfills with the specified Ruleset. + */ + public getRuleset(name: string): Promise { + if (!validator.isNonEmptyString(name)) { + const err = new FirebaseSecurityRulesError( + 'invalid-argument', 'Ruleset name must be a non-empty string.'); + return Promise.reject(err); + } + + if (name.indexOf('/') !== -1) { + const err = new FirebaseSecurityRulesError( + 'invalid-argument', 'Ruleset name must not contain any "/" characters.'); + return Promise.reject(err); + } + + const resource = `projects/${this.projectId}/rulesets/${name}`; + return this.client.getResource(resource) + .then((rulesetResponse) => { + return new Ruleset(rulesetResponse); + }); + } + + /** + * Gets the Ruleset currently applied to Cloud Firestore. Rejects with a `not-found` error if no Ruleset is + * applied on Firestore. + * + * @returns {Promise} A promise that fulfills with the Firestore Ruleset. + */ + public getFirestoreRuleset(): Promise { + return this.getRulesetForService(SecurityRules.CLOUD_FIRESTORE); + } + + private getRulesetForService(name: string): Promise { + const resource = `projects/${this.projectId}/releases/${name}`; + return this.client.getResource(resource) + .then((release) => { + const rulesetName = release.rulesetName; + if (!validator.isNonEmptyString(rulesetName)) { + throw new FirebaseSecurityRulesError( + 'not-found', `Ruleset name not found for ${name}.`); + } + + return this.getRuleset(stripProjectIdPrefix(rulesetName)); + }); + } +} + +class SecurityRulesInternals implements FirebaseServiceInternalsInterface { + public delete(): Promise { + return Promise.resolve(); + } +} + +function stripProjectIdPrefix(name: string): string { + return name.split('/').pop(); +} diff --git a/src/utils/error.ts b/src/utils/error.ts index 88e5b72335..08b3222bcf 100755 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -80,7 +80,7 @@ export class FirebaseError extends Error { * @param {string} message The error message. * @constructor */ -class PrefixedFirebaseError extends FirebaseError { +export class PrefixedFirebaseError extends FirebaseError { constructor(private codePrefix: string, code: string, message: string) { super({ code: `${codePrefix}/${code}`, diff --git a/test/unit/index.spec.ts b/test/unit/index.spec.ts index e8e73a3aa3..b6b7e48ef1 100755 --- a/test/unit/index.spec.ts +++ b/test/unit/index.spec.ts @@ -58,3 +58,7 @@ import './project-management/project-management.spec'; import './project-management/project-management-api-request.spec'; import './project-management/android-app.spec'; import './project-management/ios-app.spec'; + +// SecurityRules +import './security-rules/security-rules.spec'; +import './security-rules/security-rules-api-client.spec'; diff --git a/test/unit/security-rules/security-rules-api-client.spec.ts b/test/unit/security-rules/security-rules-api-client.spec.ts new file mode 100644 index 0000000000..86c2933db6 --- /dev/null +++ b/test/unit/security-rules/security-rules-api-client.spec.ts @@ -0,0 +1,103 @@ +/*! + * Copyright 2019 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 * as chai from 'chai'; +import * as sinon from 'sinon'; +import { SecurityRulesApiClient } from '../../../src/security-rules/security-rules-api-client'; +import { FirebaseSecurityRulesError } from '../../../src/security-rules/security-rules-utils'; +import { HttpClient } from '../../../src/utils/api-request'; +import * as utils from '../utils'; +import { FirebaseAppError } from '../../../src/utils/error'; + +const expect = chai.expect; + +describe('SecurityRulesApiClient', () => { + + const ERROR_RESPONSE = { + error: { + code: 404, + message: 'Requested entity not found', + status: 'NOT_FOUND', + }, + }; + + const apiClient: SecurityRulesApiClient = new SecurityRulesApiClient(new HttpClient()); + + // Stubs used to simulate underlying api calls. + let stubs: sinon.SinonStub[] = []; + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + }); + + describe('getResource', () => { + it('should resolve with the requested resource on success', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom({foo: 'bar'})); + stubs.push(stub); + return apiClient.getResource<{foo: string}>('foo') + .then((resp) => { + expect(resp.foo).to.equal('bar'); + }); + }); + + it('should throw when a full platform error response is received', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom(ERROR_RESPONSE, 404)); + stubs.push(stub); + const expected = new FirebaseSecurityRulesError('not-found', 'Requested entity not found'); + return apiClient.getResource('foo') + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should throw unknown-error when error code is not present', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom({}, 404)); + stubs.push(stub); + const expected = new FirebaseSecurityRulesError('unknown-error', 'Unknown server error: {}'); + return apiClient.getResource('foo') + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should throw unknown-error for non-json response', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom('not json', 404)); + stubs.push(stub); + const expected = new FirebaseSecurityRulesError( + 'unknown-error', 'Unexpected response with status: 404 and body: not json'); + return apiClient.getResource('foo') + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should throw when rejected with a FirebaseAppError', () => { + const expected = new FirebaseAppError('network-error', 'socket hang up'); + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(expected); + stubs.push(stub); + return apiClient.getResource('foo') + .should.eventually.be.rejected.and.deep.equal(expected); + }); + }); +}); diff --git a/test/unit/security-rules/security-rules.spec.ts b/test/unit/security-rules/security-rules.spec.ts new file mode 100644 index 0000000000..89a6a8d0ca --- /dev/null +++ b/test/unit/security-rules/security-rules.spec.ts @@ -0,0 +1,248 @@ +/*! + * Copyright 2019 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 * as chai from 'chai'; +import * as sinon from 'sinon'; +import { SecurityRules } from '../../../src/security-rules/security-rules'; +import { FirebaseApp } from '../../../src/firebase-app'; +import * as mocks from '../../resources/mocks'; +import { SecurityRulesApiClient } from '../../../src/security-rules/security-rules-api-client'; +import { FirebaseSecurityRulesError } from '../../../src/security-rules/security-rules-utils'; +import { deepCopy } from '../../../src/utils/deep-copy'; + +const expect = chai.expect; + +describe('SecurityRules', () => { + + const NO_PROJECT_ID = 'Failed to determine project ID. Initialize the SDK with service ' + + 'account credentials, or set project ID as an app option. Alternatively, set the ' + + 'GOOGLE_CLOUD_PROJECT environment variable.'; + const EXPECTED_ERROR = new FirebaseSecurityRulesError('internal-error', 'message'); + const FIRESTORE_RULESET_RESPONSE = { + name: 'projects/test-project/rulesets/foo', + createTime: '2019-03-08T23:45:23.288047Z', + source: { + files: [ + { + name: 'firestore.rules', + content: 'service cloud.firestore{\n}\n', + }, + ], + }, + }; + + let securityRules: SecurityRules; + let mockApp: FirebaseApp; + let mockCredentialApp: FirebaseApp; + + // Stubs used to simulate underlying api calls. + let stubs: sinon.SinonStub[] = []; + + before(() => { + mockApp = mocks.app(); + mockCredentialApp = mocks.mockCredentialApp(); + securityRules = new SecurityRules(mockApp); + }); + + after(() => { + return mockApp.delete(); + }); + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + }); + + 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 securityRulesAny: any = SecurityRules; + return new securityRulesAny(invalidApp); + }).to.throw( + 'First argument passed to admin.securityRules() must be a valid Firebase app ' + + 'instance.'); + }); + }); + + it('should throw given no app', () => { + expect(() => { + const securityRulesAny: any = SecurityRules; + return new securityRulesAny(); + }).to.throw( + 'First argument passed to admin.securityRules() must be a valid Firebase app ' + + 'instance.'); + }); + + it('should throw when initialized without project ID', () => { + // Project ID not set in the environment. + delete process.env.GOOGLE_CLOUD_PROJECT; + delete process.env.GCLOUD_PROJECT; + expect(() => { + return new SecurityRules(mockCredentialApp); + }).to.throw(NO_PROJECT_ID); + }); + + it('should not throw given a valid app', () => { + expect(() => { + return new SecurityRules(mockApp); + }).not.to.throw(); + }); + }); + + describe('app', () => { + it('returns the app from the constructor', () => { + // We expect referential equality here + expect(securityRules.app).to.equal(mockApp); + }); + }); + + describe('getRuleset', () => { + const invalidNames: any[] = [null, '', 1, true, {}, []]; + invalidNames.forEach((invalidName) => { + it(`should reject when called with: ${JSON.stringify(invalidName)}`, () => { + return securityRules.getRuleset(invalidName) + .should.eventually.be.rejected.and.have.property( + 'message', 'Ruleset name must be a non-empty string.'); + }); + }); + + it(`should reject when called with prefixed name`, () => { + return securityRules.getRuleset('projects/foo/rulesets/bar') + .should.eventually.be.rejected.and.have.property( + 'message', 'Ruleset name must not contain any "/" characters.'); + }); + + it('should propagate API errors', () => { + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .rejects(EXPECTED_ERROR); + stubs.push(stub); + return securityRules.getRuleset('foo') + .should.eventually.be.rejected.and.deep.equal(EXPECTED_ERROR); + }); + + it('should reject when API response is invalid', () => { + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .resolves(null); + stubs.push(stub); + return securityRules.getRuleset('foo') + .should.eventually.be.rejected.and.have.property( + 'message', 'Invalid Ruleset response: null'); + }); + + it('should reject when API response does not contain a name', () => { + const response = deepCopy(FIRESTORE_RULESET_RESPONSE); + response.name = ''; + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .resolves(response); + stubs.push(stub); + return securityRules.getRuleset('foo') + .should.eventually.be.rejected.and.have.property( + 'message', `Invalid Ruleset response: ${JSON.stringify(response)}`); + }); + + it('should reject when API response does not contain a createTime', () => { + const response = deepCopy(FIRESTORE_RULESET_RESPONSE); + response.createTime = ''; + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .resolves(response); + stubs.push(stub); + return securityRules.getRuleset('foo') + .should.eventually.be.rejected.and.have.property( + 'message', `Invalid Ruleset response: ${JSON.stringify(response)}`); + }); + + it('should reject when API response does not contain a source', () => { + const response = deepCopy(FIRESTORE_RULESET_RESPONSE); + response.source = null; + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .resolves(response); + stubs.push(stub); + return securityRules.getRuleset('foo') + .should.eventually.be.rejected.and.have.property( + 'message', `Invalid Ruleset response: ${JSON.stringify(response)}`); + }); + + it('should resolve with Ruleset on success', () => { + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .resolves(FIRESTORE_RULESET_RESPONSE); + stubs.push(stub); + + return securityRules.getRuleset('foo') + .then((ruleset) => { + expect(ruleset.name).to.equal('foo'); + expect(ruleset.createTime).to.equal('Fri, 08 Mar 2019 23:45:23 GMT'); + expect(ruleset.source.length).to.equal(1); + + const file = ruleset.source[0]; + expect(file.name).equals('firestore.rules'); + expect(file.content).equals('service cloud.firestore{\n}\n'); + }); + }); + }); + + describe('getFirestoreRuleset', () => { + it('should propagate API errors', () => { + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .rejects(EXPECTED_ERROR); + stubs.push(stub); + return securityRules.getFirestoreRuleset() + .should.eventually.be.rejected.and.deep.equal(EXPECTED_ERROR); + }); + + it('should reject when getRelease response is invalid', () => { + const stub = sinon + .stub(SecurityRulesApiClient.prototype, 'getResource') + .resolves({}); + stubs.push(stub); + + return securityRules.getFirestoreRuleset() + .should.eventually.be.rejected.and.have.property( + 'message', 'Ruleset name not found for cloud.firestore.'); + }); + + it('should resolve with Ruleset on success', () => { + const stub = sinon.stub(SecurityRulesApiClient.prototype, 'getResource'); + stub.onCall(0).resolves({ + rulesetName: 'projects/test-project/rulesets/foo', + }); + stub.onCall(1).resolves(FIRESTORE_RULESET_RESPONSE); + stubs.push(stub); + + return securityRules.getFirestoreRuleset() + .then((ruleset) => { + expect(ruleset.name).to.equal('foo'); + expect(ruleset.createTime).to.equal('Fri, 08 Mar 2019 23:45:23 GMT'); + expect(ruleset.source.length).to.equal(1); + + const file = ruleset.source[0]; + expect(file.name).equals('firestore.rules'); + expect(file.content).equals('service cloud.firestore{\n}\n'); + }); + }); + }); +}); From c57c38230047024ccc3865c941c89f93e5a6a9f7 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 24 Jul 2019 16:43:46 -0700 Subject: [PATCH 2/5] More argument validation and assertions --- .../security-rules-api-client.ts | 6 +++++ .../security-rules-api-client.spec.ts | 23 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/security-rules/security-rules-api-client.ts b/src/security-rules/security-rules-api-client.ts index b2f2a2db58..1a944b8f7b 100644 --- a/src/security-rules/security-rules-api-client.ts +++ b/src/security-rules/security-rules-api-client.ts @@ -37,6 +37,12 @@ export class SecurityRulesApiClient { * @returns {Promise} A promise that fulfills with the resource. */ public getResource(name: string): Promise { + if (!name.startsWith('projects/')) { + const err = new FirebaseSecurityRulesError( + 'invalid-argument', 'Resource name must have a project ID prefix.'); + return Promise.reject(err); + } + const request: HttpRequestConfig = { method: 'GET', url: `${RULES_API_URL}${name}`, diff --git a/test/unit/security-rules/security-rules-api-client.spec.ts b/test/unit/security-rules/security-rules-api-client.spec.ts index 86c2933db6..814110d805 100644 --- a/test/unit/security-rules/security-rules-api-client.spec.ts +++ b/test/unit/security-rules/security-rules-api-client.spec.ts @@ -48,24 +48,37 @@ describe('SecurityRulesApiClient', () => { }); describe('getResource', () => { + const RESOURCE_ID = 'projects/test-project/rulesets/ruleset-id'; + it('should resolve with the requested resource on success', () => { const stub = sinon .stub(HttpClient.prototype, 'send') .resolves(utils.responseFrom({foo: 'bar'})); stubs.push(stub); - return apiClient.getResource<{foo: string}>('foo') + return apiClient.getResource<{foo: string}>(RESOURCE_ID) .then((resp) => { expect(resp.foo).to.equal('bar'); + expect(stub).to.have.been.calledOnce.and.calledWith({ + method: 'GET', + url: 'https://firebaserules.googleapis.com/v1/projects/test-project/rulesets/ruleset-id', + }); }); }); + it('should reject when the resource name is unqualified', () => { + const expected = new FirebaseSecurityRulesError( + 'invalid-argument', 'Resource name must have a project ID prefix.'); + return apiClient.getResource('rulesets/ruleset-id') + .should.eventually.be.rejected.and.deep.equal(expected); + }); + it('should throw when a full platform error response is received', () => { const stub = sinon .stub(HttpClient.prototype, 'send') .rejects(utils.errorFrom(ERROR_RESPONSE, 404)); stubs.push(stub); const expected = new FirebaseSecurityRulesError('not-found', 'Requested entity not found'); - return apiClient.getResource('foo') + return apiClient.getResource(RESOURCE_ID) .should.eventually.be.rejected.and.deep.equal(expected); }); @@ -75,7 +88,7 @@ describe('SecurityRulesApiClient', () => { .rejects(utils.errorFrom({}, 404)); stubs.push(stub); const expected = new FirebaseSecurityRulesError('unknown-error', 'Unknown server error: {}'); - return apiClient.getResource('foo') + return apiClient.getResource(RESOURCE_ID) .should.eventually.be.rejected.and.deep.equal(expected); }); @@ -86,7 +99,7 @@ describe('SecurityRulesApiClient', () => { stubs.push(stub); const expected = new FirebaseSecurityRulesError( 'unknown-error', 'Unexpected response with status: 404 and body: not json'); - return apiClient.getResource('foo') + return apiClient.getResource(RESOURCE_ID) .should.eventually.be.rejected.and.deep.equal(expected); }); @@ -96,7 +109,7 @@ describe('SecurityRulesApiClient', () => { .stub(HttpClient.prototype, 'send') .rejects(expected); stubs.push(stub); - return apiClient.getResource('foo') + return apiClient.getResource(RESOURCE_ID) .should.eventually.be.rejected.and.deep.equal(expected); }); }); From 0f6fd730198bcc268a46231a8aaa3044a423bdb0 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Thu, 25 Jul 2019 15:20:13 -0700 Subject: [PATCH 3/5] Cleaning up the rules impl --- .../security-rules-api-client.ts | 29 ++++++++++++------- src/security-rules/security-rules.ts | 23 +++++---------- .../security-rules-api-client.spec.ts | 12 ++------ 3 files changed, 28 insertions(+), 36 deletions(-) diff --git a/src/security-rules/security-rules-api-client.ts b/src/security-rules/security-rules-api-client.ts index 1a944b8f7b..8beb244425 100644 --- a/src/security-rules/security-rules-api-client.ts +++ b/src/security-rules/security-rules-api-client.ts @@ -17,8 +17,9 @@ import { HttpRequestConfig, HttpClient, HttpError } from '../utils/api-request'; import { PrefixedFirebaseError } from '../utils/error'; import { FirebaseSecurityRulesError, SecurityRulesErrorCode } from './security-rules-utils'; +import * as validator from '../utils/validator'; -const RULES_API_URL = 'https://firebaserules.googleapis.com/v1/'; +const RULES_API_URL = 'https://firebaserules.googleapis.com/v1'; /** * Class that facilitates sending requests to the Firebase security rules backend API. @@ -27,25 +28,31 @@ const RULES_API_URL = 'https://firebaserules.googleapis.com/v1/'; */ export class SecurityRulesApiClient { - constructor(private readonly httpClient: HttpClient) { } + private readonly url: string; + + constructor(private readonly httpClient: HttpClient, projectId: string) { + if (!validator.isNonEmptyString(projectId)) { + throw new FirebaseSecurityRulesError( + 'invalid-argument', + 'Failed to determine project ID. Initialize the SDK with service account credentials, or ' + + 'set project ID as an app option. Alternatively, set the GOOGLE_CLOUD_PROJECT ' + + 'environment variable.'); + } + + this.url = `${RULES_API_URL}/projects/${projectId}`; + } /** - * Gets the specified resource from the rules API. Resource name must be full qualified names with project - * ID prefix (e.g. `projects/project-id/rulesets/ruleset-name`). + * Gets the specified resource from the rules API. Resource names must be the short names without project + * ID prefix (e.g. `rulesets/ruleset-name`). * * @param {string} name Full qualified name of the resource to get. * @returns {Promise} A promise that fulfills with the resource. */ public getResource(name: string): Promise { - if (!name.startsWith('projects/')) { - const err = new FirebaseSecurityRulesError( - 'invalid-argument', 'Resource name must have a project ID prefix.'); - return Promise.reject(err); - } - const request: HttpRequestConfig = { method: 'GET', - url: `${RULES_API_URL}${name}`, + url: `${this.url}/${name}`, }; return this.httpClient.send(request) .then((resp) => { diff --git a/src/security-rules/security-rules.ts b/src/security-rules/security-rules.ts index fc336c9dbb..928f455a44 100644 --- a/src/security-rules/security-rules.ts +++ b/src/security-rules/security-rules.ts @@ -88,7 +88,6 @@ export class SecurityRules implements FirebaseServiceInterface { public readonly INTERNAL = new SecurityRulesInternals(); private readonly client: SecurityRulesApiClient; - private readonly projectId: string; /** * @param {object} app The app for this SecurityRules service. @@ -103,21 +102,13 @@ export class SecurityRules implements FirebaseServiceInterface { } const projectId = utils.getProjectId(app); - if (!validator.isNonEmptyString(projectId)) { - throw new FirebaseSecurityRulesError( - 'invalid-argument', - 'Failed to determine project ID. Initialize the SDK with service account credentials, or ' - + 'set project ID as an app option. Alternatively, set the GOOGLE_CLOUD_PROJECT ' - + 'environment variable.'); - } - - this.projectId = projectId; - this.client = new SecurityRulesApiClient(new AuthorizedHttpClient(app)); + this.client = new SecurityRulesApiClient(new AuthorizedHttpClient(app), projectId); } /** * Gets the Ruleset identified by the given name. The input name should be the short name string without - * the project ID prefix. Rejects with a `not-found` error if the specified Ruleset cannot be found. + * the project ID prefix. For example, to retrieve the `projects/project-id/rulesets/my-ruleset`, pass the + * short name "my-ruleset". Rejects with a `not-found` error if the specified Ruleset cannot be found. * * @param {string} name Name of the Ruleset to retrieve. * @returns {Promise} A promise that fulfills with the specified Ruleset. @@ -135,7 +126,7 @@ export class SecurityRules implements FirebaseServiceInterface { return Promise.reject(err); } - const resource = `projects/${this.projectId}/rulesets/${name}`; + const resource = `rulesets/${name}`; return this.client.getResource(resource) .then((rulesetResponse) => { return new Ruleset(rulesetResponse); @@ -152,14 +143,14 @@ export class SecurityRules implements FirebaseServiceInterface { return this.getRulesetForService(SecurityRules.CLOUD_FIRESTORE); } - private getRulesetForService(name: string): Promise { - const resource = `projects/${this.projectId}/releases/${name}`; + private getRulesetForService(serviceName: string): Promise { + const resource = `releases/${serviceName}`; return this.client.getResource(resource) .then((release) => { const rulesetName = release.rulesetName; if (!validator.isNonEmptyString(rulesetName)) { throw new FirebaseSecurityRulesError( - 'not-found', `Ruleset name not found for ${name}.`); + 'not-found', `Ruleset name not found for ${serviceName}.`); } return this.getRuleset(stripProjectIdPrefix(rulesetName)); diff --git a/test/unit/security-rules/security-rules-api-client.spec.ts b/test/unit/security-rules/security-rules-api-client.spec.ts index 814110d805..ba222e0c6f 100644 --- a/test/unit/security-rules/security-rules-api-client.spec.ts +++ b/test/unit/security-rules/security-rules-api-client.spec.ts @@ -37,7 +37,8 @@ describe('SecurityRulesApiClient', () => { }, }; - const apiClient: SecurityRulesApiClient = new SecurityRulesApiClient(new HttpClient()); + const apiClient: SecurityRulesApiClient = new SecurityRulesApiClient( + new HttpClient(), 'test-project'); // Stubs used to simulate underlying api calls. let stubs: sinon.SinonStub[] = []; @@ -48,7 +49,7 @@ describe('SecurityRulesApiClient', () => { }); describe('getResource', () => { - const RESOURCE_ID = 'projects/test-project/rulesets/ruleset-id'; + const RESOURCE_ID = 'rulesets/ruleset-id'; it('should resolve with the requested resource on success', () => { const stub = sinon @@ -65,13 +66,6 @@ describe('SecurityRulesApiClient', () => { }); }); - it('should reject when the resource name is unqualified', () => { - const expected = new FirebaseSecurityRulesError( - 'invalid-argument', 'Resource name must have a project ID prefix.'); - return apiClient.getResource('rulesets/ruleset-id') - .should.eventually.be.rejected.and.deep.equal(expected); - }); - it('should throw when a full platform error response is received', () => { const stub = sinon .stub(HttpClient.prototype, 'send') From d58f0096d43e206ac81747d649fe2ac9b1559592 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Thu, 25 Jul 2019 15:58:13 -0700 Subject: [PATCH 4/5] Internal API renamed --- src/security-rules/security-rules.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/security-rules/security-rules.ts b/src/security-rules/security-rules.ts index 928f455a44..43ddd85044 100644 --- a/src/security-rules/security-rules.ts +++ b/src/security-rules/security-rules.ts @@ -140,17 +140,17 @@ export class SecurityRules implements FirebaseServiceInterface { * @returns {Promise} A promise that fulfills with the Firestore Ruleset. */ public getFirestoreRuleset(): Promise { - return this.getRulesetForService(SecurityRules.CLOUD_FIRESTORE); + return this.getRulesetForRelease(SecurityRules.CLOUD_FIRESTORE); } - private getRulesetForService(serviceName: string): Promise { - const resource = `releases/${serviceName}`; + private getRulesetForRelease(releaseName: string): Promise { + const resource = `releases/${releaseName}`; return this.client.getResource(resource) .then((release) => { const rulesetName = release.rulesetName; if (!validator.isNonEmptyString(rulesetName)) { throw new FirebaseSecurityRulesError( - 'not-found', `Ruleset name not found for ${serviceName}.`); + 'not-found', `Ruleset name not found for ${releaseName}.`); } return this.getRuleset(stripProjectIdPrefix(rulesetName)); From fe16838c6722de023ef4320a79c71b39678af24c Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Mon, 29 Jul 2019 11:05:41 -0700 Subject: [PATCH 5/5] Fixing a typo in a comment --- src/security-rules/security-rules.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security-rules/security-rules.ts b/src/security-rules/security-rules.ts index 43ddd85044..67544e752b 100644 --- a/src/security-rules/security-rules.ts +++ b/src/security-rules/security-rules.ts @@ -54,7 +54,7 @@ interface RulesetResponse { } /** - * Representa a set of Firebase security rules. + * Represents a set of Firebase security rules. */ export class Ruleset implements RulesetMetadata {