Using async project ID discovery API in SecurityRules (#732)

hiranya911 · web-flow · commit 9373d39f890b · 2019-12-20T11:26:37.000-08:00
diff --git a/src/security-rules/security-rules-api-client.ts b/src/security-rules/security-rules-api-client.ts
@@ -14,10 +14,12 @@
  * limitations under the License.
  */
 
-import { HttpRequestConfig, HttpClient, HttpError } from '../utils/api-request';
+import { HttpRequestConfig, HttpClient, HttpError, AuthorizedHttpClient } from '../utils/api-request';
 import { PrefixedFirebaseError } from '../utils/error';
 import { FirebaseSecurityRulesError, SecurityRulesErrorCode } from './security-rules-utils';
+import * as utils from '../utils/index';
 import * as validator from '../utils/validator';
+import { FirebaseApp } from '../firebase-app';
 
 const RULES_V1_API = 'https://firebaserules.googleapis.com/v1';
 const FIREBASE_VERSION_HEADER = {
@@ -54,25 +56,18 @@ export interface ListRulesetsResponse {
  */
 export class SecurityRulesApiClient {
 
-  private readonly projectIdPrefix: string;
-  private readonly url: string;
+  private readonly httpClient: HttpClient;
+  private projectIdPrefix?: string;
 
-  constructor(private readonly httpClient: HttpClient, projectId: string | null) {
-    if (!validator.isNonNullObject(httpClient)) {
-      throw new FirebaseSecurityRulesError(
-        'invalid-argument', 'HttpClient must be a non-null object.');
-    }
-
-    if (!validator.isNonEmptyString(projectId)) {
+  constructor(private readonly app: FirebaseApp) {
+    if (!validator.isNonNullObject(app) || !('options' in app)) {
       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.');
+        'First argument passed to admin.securityRules() must be a valid Firebase app '
+          + 'instance.');
     }
 
-    this.projectIdPrefix = `projects/${projectId}`;
-    this.url = `${RULES_V1_API}/${this.projectIdPrefix}`;
+    this.httpClient = new AuthorizedHttpClient(app);
   }
 
   public getRuleset(name: string): Promise<RulesetResponse> {
@@ -105,23 +100,24 @@ export class SecurityRulesApiClient {
       }
     }
 
-    const request: HttpRequestConfig = {
-      method: 'POST',
-      url: `${this.url}/rulesets`,
-      data: ruleset,
-    };
-    return this.sendRequest<RulesetResponse>(request);
+    return this.getUrl()
+      .then((url) => {
+        const request: HttpRequestConfig = {
+          method: 'POST',
+          url: `${url}/rulesets`,
+          data: ruleset,
+        };
+        return this.sendRequest<RulesetResponse>(request);
+      });
   }
 
   public deleteRuleset(name: string): Promise<void> {
-    return Promise.resolve()
-      .then(() => {
-        return this.getRulesetName(name);
-      })
-      .then((rulesetName) => {
+    return this.getUrl()
+      .then((url) => {
+        const rulesetName = this.getRulesetName(name);
         const request: HttpRequestConfig = {
           method: 'DELETE',
-          url: `${this.url}/${rulesetName}`,
+          url: `${url}/${rulesetName}`,
         };
         return this.sendRequest<void>(request);
       });
@@ -151,28 +147,61 @@ export class SecurityRulesApiClient {
       delete data.pageToken;
     }
 
-    const request: HttpRequestConfig = {
-      method: 'GET',
-      url: `${this.url}/rulesets`,
-      data,
-    };
-    return this.sendRequest<ListRulesetsResponse>(request);
+    return this.getUrl()
+      .then((url) => {
+        const request: HttpRequestConfig = {
+          method: 'GET',
+          url: `${url}/rulesets`,
+          data,
+        };
+        return this.sendRequest<ListRulesetsResponse>(request);
+      });
   }
 
   public getRelease(name: string): Promise<Release> {
     return this.getResource<Release>(`releases/${name}`);
   }
 
   public updateRelease(name: string, rulesetName: string): Promise<Release> {
-    const data = {
-      release: this.getReleaseDescription(name, rulesetName),
-    };
-    const request: HttpRequestConfig = {
-      method: 'PATCH',
-      url: `${this.url}/releases/${name}`,
-      data,
-    };
-    return this.sendRequest<Release>(request);
+    return this.getUrl()
+      .then((url) => {
+        return this.getReleaseDescription(name, rulesetName)
+          .then((release) => {
+            const request: HttpRequestConfig = {
+              method: 'PATCH',
+              url: `${url}/releases/${name}`,
+              data: {release},
+            };
+            return this.sendRequest<Release>(request);
+          });
+      });
+  }
+
+  private getUrl(): Promise<string> {
+    return this.getProjectIdPrefix()
+      .then((projectIdPrefix) => {
+        return `${RULES_V1_API}/${projectIdPrefix}`;
+      });
+  }
+
+  private getProjectIdPrefix(): Promise<string> {
+    if (this.projectIdPrefix) {
+      return Promise.resolve(this.projectIdPrefix);
+    }
+
+    return utils.findProjectId(this.app)
+      .then((projectId) => {
+        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.projectIdPrefix = `projects/${projectId}`;
+        return this.projectIdPrefix;
+      });
   }
 
   /**
@@ -183,18 +212,24 @@ export class SecurityRulesApiClient {
    * @returns {Promise<T>} A promise that fulfills with the resource.
    */
   private getResource<T>(name: string): Promise<T> {
-    const request: HttpRequestConfig = {
-      method: 'GET',
-      url: `${this.url}/${name}`,
-    };
-    return this.sendRequest<T>(request);
+    return this.getUrl()
+      .then((url) => {
+        const request: HttpRequestConfig = {
+          method: 'GET',
+          url: `${url}/${name}`,
+        };
+        return this.sendRequest<T>(request);
+      });
   }
 
-  private getReleaseDescription(name: string, rulesetName: string): Release {
-    return {
-      name: `${this.projectIdPrefix}/releases/${name}`,
-      rulesetName: `${this.projectIdPrefix}/${this.getRulesetName(rulesetName)}`,
-    };
+  private getReleaseDescription(name: string, rulesetName: string): Promise<Release> {
+    return this.getProjectIdPrefix()
+      .then((projectIdPrefix) => {
+        return {
+          name: `${projectIdPrefix}/releases/${name}`,
+          rulesetName: `${projectIdPrefix}/${this.getRulesetName(rulesetName)}`,
+        };
+      });
   }
 
   private getRulesetName(name: string): string {
diff --git a/src/security-rules/security-rules.ts b/src/security-rules/security-rules.ts
@@ -16,12 +16,10 @@
 
 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, RulesetResponse, RulesetContent, ListRulesetsResponse,
 } from './security-rules-api-client';
-import { AuthorizedHttpClient } from '../utils/api-request';
 import { FirebaseSecurityRulesError } from './security-rules-utils';
 
 /**
@@ -115,15 +113,7 @@ export class SecurityRules implements FirebaseServiceInterface {
    * @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);
-    this.client = new SecurityRulesApiClient(new AuthorizedHttpClient(app), projectId);
+    this.client = new SecurityRulesApiClient(app);
   }
 
   /**
diff --git a/test/unit/security-rules/security-rules-api-client.spec.ts b/test/unit/security-rules/security-rules-api-client.spec.ts
@@ -23,7 +23,9 @@ import { SecurityRulesApiClient, RulesetContent } from '../../../src/security-ru
 import { FirebaseSecurityRulesError } from '../../../src/security-rules/security-rules-utils';
 import { HttpClient } from '../../../src/utils/api-request';
 import * as utils from '../utils';
+import * as mocks from '../../resources/mocks';
 import { FirebaseAppError } from '../../../src/utils/error';
+import { FirebaseApp } from '../../../src/firebase-app';
 
 const expect = chai.expect;
 
@@ -39,35 +41,41 @@ describe('SecurityRulesApiClient', () => {
     },
   };
   const EXPECTED_HEADERS = {
+    'Authorization': 'Bearer mock-token',
     'X-Firebase-Client': 'fire-admin-node/<XXX_SDK_VERSION_XXX>',
   };
+  const noProjectId = '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 apiClient: SecurityRulesApiClient = new SecurityRulesApiClient(
-    new HttpClient(), 'test-project');
+  const mockOptions = {
+    credential: new mocks.MockCredential(),
+    projectId: 'test-project',
+  };
+
+  const clientWithoutProjectId = new SecurityRulesApiClient(
+    mocks.mockCredentialApp());
 
   // Stubs used to simulate underlying api calls.
   let stubs: sinon.SinonStub[] = [];
+  let app: FirebaseApp;
+  let apiClient: SecurityRulesApiClient;
+
+  beforeEach(() => {
+    app = mocks.appWithOptions(mockOptions);
+    apiClient = new SecurityRulesApiClient(app);
+  });
 
   afterEach(() => {
     _.forEach(stubs, (stub) => stub.restore());
     stubs = [];
+    return app.delete();
   });
 
   describe('Constructor', () => {
-    it('should throw when the HttpClient is null', () => {
-      expect(() => new SecurityRulesApiClient(null as unknown as HttpClient, 'test'))
-        .to.throw('HttpClient must be a non-null object.');
-    });
-
-    const invalidProjectIds: any[] = [null, undefined, '', {}, [], true, 1];
-    const noProjectId = '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.';
-    invalidProjectIds.forEach((invalidProjectId) => {
-      it(`should throw when the projectId is: ${invalidProjectId}`, () => {
-        expect(() => new SecurityRulesApiClient(new HttpClient(), invalidProjectId))
-          .to.throw(noProjectId);
-      });
+    it('should throw when the app is null', () => {
+      expect(() => new SecurityRulesApiClient(null as unknown as FirebaseApp))
+        .to.throw('First argument passed to admin.securityRules() must be a valid Firebase app');
     });
   });
 
@@ -87,6 +95,11 @@ describe('SecurityRulesApiClient', () => {
           'message', 'Ruleset name must not contain any "/" characters.');
     });
 
+    it(`should reject when project id is not available`, () => {
+      return clientWithoutProjectId.getRuleset(RULESET_NAME)
+        .should.eventually.be.rejectedWith(noProjectId);
+    });
+
     it('should resolve with the requested ruleset on success', () => {
       const stub = sinon
         .stub(HttpClient.prototype, 'send')
@@ -166,6 +179,14 @@ describe('SecurityRulesApiClient', () => {
       });
     });
 
+    it(`should reject when project id is not available`, () => {
+      return clientWithoutProjectId.createRuleset({
+        source: {
+          files: [RULES_FILE],
+        },
+      }).should.eventually.be.rejectedWith(noProjectId);
+    });
+
     const invalidFiles: any[] = [null, undefined, 'test', {}, { name: 'test' }, { content: 'test' }];
     invalidFiles.forEach((file) => {
       it(`should reject when called with: ${JSON.stringify(file)}`, () => {
@@ -306,6 +327,11 @@ describe('SecurityRulesApiClient', () => {
       });
     });
 
+    it(`should reject when project id is not available`, () => {
+      return clientWithoutProjectId.listRulesets()
+        .should.eventually.be.rejectedWith(noProjectId);
+    });
+
     it('should resolve on success when called without any arguments', () => {
       const stub = sinon
         .stub(HttpClient.prototype, 'send')
@@ -400,6 +426,11 @@ describe('SecurityRulesApiClient', () => {
   });
 
   describe('getRelease', () => {
+    it(`should reject when project id is not available`, () => {
+      return clientWithoutProjectId.getRelease(RELEASE_NAME)
+        .should.eventually.be.rejectedWith(noProjectId);
+    });
+
     it('should resolve with the requested release on success', () => {
       const stub = sinon
         .stub(HttpClient.prototype, 'send')
@@ -459,6 +490,11 @@ describe('SecurityRulesApiClient', () => {
   });
 
   describe('updateRelease', () => {
+    it(`should reject when project id is not available`, () => {
+      return clientWithoutProjectId.updateRelease(RELEASE_NAME, RULESET_NAME)
+        .should.eventually.be.rejectedWith(noProjectId);
+    });
+
     it('should resolve with the updated release on success', () => {
       const stub = sinon
         .stub(HttpClient.prototype, 'send')
@@ -539,6 +575,11 @@ describe('SecurityRulesApiClient', () => {
           'message', 'Ruleset name must not contain any "/" characters.');
     });
 
+    it(`should reject when project id is not available`, () => {
+      return clientWithoutProjectId.deleteRuleset(RULESET_NAME)
+        .should.eventually.be.rejectedWith(noProjectId);
+    });
+
     it('should resolve on success', () => {
       const stub = sinon
         .stub(HttpClient.prototype, 'send')
diff --git a/test/unit/security-rules/security-rules.spec.ts b/test/unit/security-rules/security-rules.spec.ts
@@ -128,16 +128,16 @@ describe('SecurityRules', () => {
               + 'instance.');
     });
 
-    it('should throw when initialized without project ID', () => {
+    it('should reject when initialized without project ID', () => {
       // Project ID not set in the environment.
       delete process.env.GOOGLE_CLOUD_PROJECT;
       delete process.env.GCLOUD_PROJECT;
       const noProjectId = '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.';
-      expect(() => {
-        return new SecurityRules(mockCredentialApp);
-      }).to.throw(noProjectId);
+      const rulesWithoutProjectId = new SecurityRules(mockCredentialApp);
+      return rulesWithoutProjectId.getRuleset('test')
+        .should.eventually.rejectedWith(noProjectId);
     });
 
     it('should not throw given a valid app', () => {
