Update /internal/setRules endpoint; fix ruleset file watcher

scripts/storage-emulator-integration/rules/manager.test.ts

@@ -15,23 +15,18 @@ import { RulesetOperationMethod, SourceFile } from "../../../src/emulator/storag
 import { isPermitted } from "../../../src/emulator/storage/rules/utils";
 import { readFile } from "../../../src/fsutils";
 
+const EMULATOR_LOAD_RULESET_DELAY_MS = 2000;
+
 describe("Storage Rules Manager", function () {
   const rulesRuntime = new StorageRulesRuntime();
-  const rules = [
-    { resource: "bucket_0", rules: StorageRulesFiles.readWriteIfTrue },
-    { resource: "bucket_1", rules: StorageRulesFiles.readWriteIfAuth },
-  ];
-  const opts = { method: RulesetOperationMethod.GET, file: {}, path: "/b/bucket_2/o/" };
+  const opts = { method: RulesetOperationMethod.GET, file: {}, path: "/b/bucket_0/o/" };
   let rulesManager: StorageRulesManager;
 
   // eslint-disable-next-line @typescript-eslint/no-invalid-this
   this.timeout(TIMEOUT_LONG);
 
   beforeEach(async () => {
     await rulesRuntime.start();
-
-    rulesManager = createStorageRulesManager(rules, rulesRuntime);
-    await rulesManager.start();
   });
 
   afterEach(async () => {
@@ -40,47 +35,28 @@ describe("Storage Rules Manager", function () {
   });
 
   it("should load multiple rulesets on start", async () => {
+    const rules = [
+      { resource: "bucket_0", rules: StorageRulesFiles.readWriteIfTrue },
+      { resource: "bucket_1", rules: StorageRulesFiles.readWriteIfAuth },
+    ];
+    rulesManager = createStorageRulesManager(rules, rulesRuntime);
+    await rulesManager.start();
+
     const bucket0Ruleset = rulesManager.getRuleset("bucket_0");
-    expect(await isPermitted({ ...opts, ruleset: bucket0Ruleset! })).to.be.true;
+    expect(await isPermitted({ ...opts, path: "/b/bucket_0/o/", ruleset: bucket0Ruleset! })).to.be
+      .true;
 
     const bucket1Ruleset = rulesManager.getRuleset("bucket_1");
-    expect(await isPermitted({ ...opts, ruleset: bucket1Ruleset! })).to.be.false;
+    expect(await isPermitted({ ...opts, path: "/b/bucket_1/o/", ruleset: bucket1Ruleset! })).to.be
+      .false;
   });
 
   it("should load single ruleset on start", async () => {
-    const otherRulesManager = createStorageRulesManager(
-      StorageRulesFiles.readWriteIfTrue,
-      rulesRuntime
-    );
-    await otherRulesManager.start();
+    rulesManager = createStorageRulesManager(StorageRulesFiles.readWriteIfTrue, rulesRuntime);
+    await rulesManager.start();
 
-    const ruleset = otherRulesManager.getRuleset("default");
+    const ruleset = rulesManager.getRuleset("bucket");
     expect(await isPermitted({ ...opts, ruleset: ruleset! })).to.be.true;
-
-    await otherRulesManager.stop();
-  });
-
-  it("should load ruleset on update with SourceFile object", async () => {
-    expect(rulesManager.getRuleset("bucket_2")).to.be.undefined;
-    await rulesManager.updateSourceFile(StorageRulesFiles.readWriteIfTrue, "bucket_2");
-    expect(rulesManager.getRuleset("bucket_2")).not.to.be.undefined;
-  });
-
-  it("should set source file", async () => {
-    await rulesManager.updateSourceFile(StorageRulesFiles.readWriteIfTrue, "bucket_2");
-
-    expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket_2")! })).to.be
-      .true;
-
-    const issues = await rulesManager.updateSourceFile(
-      StorageRulesFiles.readWriteIfAuth,
-      "bucket_2"
-    );
-
-    expect(issues.errors.length).to.equal(0);
-    expect(issues.warnings.length).to.equal(0);
-    expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket_2")! })).to.be
-      .false;
   });
 
   it("should reload ruleset on changes to source file", async () => {
@@ -91,15 +67,17 @@ describe("Storage Rules Manager", function () {
     persistence.appendBytes(fileName, Buffer.from(StorageRulesFiles.readWriteIfTrue.content));
 
     const sourceFile = getSourceFile(testDir, fileName);
-    await rulesManager.updateSourceFile(sourceFile, "bucket_2");
-    expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket_2")! })).to.be
-      .true;
+    rulesManager = createStorageRulesManager(sourceFile, rulesRuntime);
+    await rulesManager.start();
+
+    expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket")! })).to.be.true;
 
     // Write new rules to file
     persistence.deleteFile(fileName);
     persistence.appendBytes(fileName, Buffer.from(StorageRulesFiles.readWriteIfAuth.content));
 
-    expect(await isPermitted(opts)).to.be.false;
+    await new Promise((resolve) => setTimeout(resolve, EMULATOR_LOAD_RULESET_DELAY_MS));
+    expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket")! })).to.be.false;
   });
 });
 

scripts/storage-emulator-integration/tests.ts

@@ -9,7 +9,7 @@ import * as puppeteer from "puppeteer";
 import { Bucket, Storage, CopyOptions } from "@google-cloud/storage";
 import supertest = require("supertest");
 
-import { IMAGE_FILE_BASE64 } from "../../src/test/emulators/fixtures";
+import { IMAGE_FILE_BASE64, StorageRulesFiles } from "../../src/test/emulators/fixtures";
 import { TriggerEndToEndTest } from "../integration-helpers/framework";
 import {
   createRandomFile,
@@ -1109,6 +1109,145 @@ describe("Storage emulator", () => {
     });
   });
 
+  emulatorSpecificDescribe("Internal Endpoints", () => {
+    before(async function (this) {
+      this.timeout(TEST_SETUP_TIMEOUT);
+      test = new TriggerEndToEndTest(FIREBASE_PROJECT, __dirname, emulatorConfig);
+      await test.startEmulators(["--only", "storage"]);
+    });
+
+    after(async () => {
+      await test.stopEmulators();
+    });
+
+    describe("setRules", () => {
+      it("should set single ruleset", async () => {
+        await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [StorageRulesFiles.readWriteIfTrue],
+            },
+          })
+          .expect(200);
+      });
+
+      it("should set multiple rules/resource objects", async () => {
+        await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [
+                { resource: "bucket_0", ...StorageRulesFiles.readWriteIfTrue },
+                { resource: "bucket_1", ...StorageRulesFiles.readWriteIfAuth },
+              ],
+            },
+          })
+          .expect(200);
+      });
+
+      it("should overwrite single ruleset with multiple rules/resource objects", async () => {
+        await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [StorageRulesFiles.readWriteIfTrue],
+            },
+          })
+          .expect(200);
+
+        await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [
+                { resource: "bucket_0", ...StorageRulesFiles.readWriteIfTrue },
+                { resource: "bucket_1", ...StorageRulesFiles.readWriteIfAuth },
+              ],
+            },
+          })
+          .expect(200);
+      });
+
+      it("should return 400 if rules.files array is missing", async () => {
+        const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({ rules: {} })
+          .expect(400)
+          .then((res) => res.body.message);
+
+        expect(errorMessage).to.equal("Request body must include 'rules.files' array");
+      });
+
+      it("should return 400 if rules.files array has missing name field", async () => {
+        const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [{ content: StorageRulesFiles.readWriteIfTrue.content }],
+            },
+          })
+          .expect(400)
+          .then((res) => res.body.message);
+
+        expect(errorMessage).to.equal(
+          "Each member of 'rules.files' array must contain 'name' and 'content'"
+        );
+      });
+
+      it("should return 400 if rules.files array has missing content field", async () => {
+        const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [{ name: StorageRulesFiles.readWriteIfTrue.name }],
+            },
+          })
+          .expect(400)
+          .then((res) => res.body.message);
+
+        expect(errorMessage).to.equal(
+          "Each member of 'rules.files' array must contain 'name' and 'content'"
+        );
+      });
+
+      it("should return 400 if rules.files array has missing resource field", async () => {
+        const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [
+                { resource: "bucket_0", ...StorageRulesFiles.readWriteIfTrue },
+                StorageRulesFiles.readWriteIfAuth,
+              ],
+            },
+          })
+          .expect(400)
+          .then((res) => res.body.message);
+
+        expect(errorMessage).to.equal(
+          "Each member of 'rules.files' array must contain 'name', 'content', and 'resource'"
+        );
+      });
+
+      it("should return 400 if rules.files array has invalid content", async () => {
+        const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
+          .put("/internal/setRules")
+          .send({
+            rules: {
+              files: [{ name: StorageRulesFiles.readWriteIfTrue.name, content: "foo" }],
+            },
+          })
+          .expect(400)
+          .then((res) => res.body.message);
+
+        expect(errorMessage).to.equal(
+          "There was an error updating rules, see logs for more details"
+        );
+      });
+    });
+  });
+
   describe("Firebase Endpoints", () => {
     let storage: Storage;
 

src/emulator/storage/index.ts

@@ -6,7 +6,7 @@ import { createApp } from "./server";
 import { StorageLayer } from "./files";
 import { EmulatorLogger } from "../emulatorLogger";
 import { createStorageRulesManager, StorageRulesManager } from "./rules/manager";
-import { StorageRulesRuntime } from "./rules/runtime";
+import { StorageRulesIssues, StorageRulesRuntime } from "./rules/runtime";
 import { SourceFile } from "./rules/types";
 import express = require("express");
 import { getAdminCredentialValidator, getRulesValidator } from "./rules/utils";
@@ -35,14 +35,14 @@ export class StorageEmulator implements EmulatorInstance {
 
   private _logger = EmulatorLogger.forEmulator(Emulators.STORAGE);
   private _rulesRuntime: StorageRulesRuntime;
-  private _rulesManager: StorageRulesManager;
+  private _rulesManager!: StorageRulesManager;
   private _persistence: Persistence;
   private _storageLayer: StorageLayer;
   private _uploadService: UploadService;
 
   constructor(private args: StorageEmulatorArgs) {
     this._rulesRuntime = new StorageRulesRuntime();
-    this._rulesManager = createStorageRulesManager(this.args.rules, this._rulesRuntime);
+    this.createRulesManager(this.args.rules);
     this._persistence = new Persistence(this.getPersistenceTmpDir());
     this._storageLayer = new StorageLayer(
       args.projectId,
@@ -113,6 +113,16 @@ export class StorageEmulator implements EmulatorInstance {
     return this._app!;
   }
 
+  async replaceRules(rules: SourceFile | RulesConfig[]): Promise<StorageRulesIssues> {
+    await this._rulesManager.stop();
+    this.createRulesManager(rules);
+    return this._rulesManager.start();
+  }
+
+  private createRulesManager(rules: SourceFile | RulesConfig[]): void {
+    this._rulesManager = createStorageRulesManager(rules, this._rulesRuntime);
+  }
+
   private getPersistenceTmpDir(): string {
     return `${tmpdir()}/firebase/storage/blobs`;
   }

src/emulator/storage/rules/manager.ts

@@ -4,6 +4,7 @@ import { Emulators } from "../../types";
 import { SourceFile } from "./types";
 import { StorageRulesIssues, StorageRulesRuntime, StorageRulesetInstance } from "./runtime";
 import { RulesConfig } from "..";
+import { readFile } from "../../../fsutils";
 
 /**
  * Keeps track of rules source file(s) and generated ruleset(s), either one for all storage
@@ -14,7 +15,6 @@ import { RulesConfig } from "..";
  * ```
  * const rulesManager = createStorageRulesManager(initialRules);
  * rulesManager.start();
- * rulesManager.updateSourceFile(newRules);
  * rulesManager.stop();
  * ```
  */
@@ -28,12 +28,6 @@ export interface StorageRulesManager {
    */
   getRuleset(resource: string): StorageRulesetInstance | undefined;
 
-  /**
-   * Updates the source file and, correspondingly, the file watcher and ruleset for the resource.
-   * Returns an array of errors and/or warnings that arise from loading the ruleset.
-   */
-  updateSourceFile(rules: SourceFile, resource: string): Promise<StorageRulesIssues>;
-
   /** Removes listeners from all files for all managed resources. */
   stop(): Promise<void>;
 }
@@ -65,22 +59,16 @@ class DefaultStorageRulesManager implements StorageRulesManager {
     this._rules = _rules;
   }
 
-  start(): Promise<StorageRulesIssues> {
-    return this.updateSourceFile(this._rules);
+  async start(): Promise<StorageRulesIssues> {
+    const issues = await this.loadRuleset();
+    this.updateWatcher(this._rules.name);
+    return issues;
   }
 
   getRuleset(): StorageRulesetInstance | undefined {
     return this._ruleset;
   }
 
-  async updateSourceFile(rules: SourceFile): Promise<StorageRulesIssues> {
-    const prevRulesFile = this._rules.name;
-    this._rules = rules;
-    const issues = await this.loadRuleset();
-    this.updateWatcher(rules.name, prevRulesFile);
-    return issues;
-  }
-
   async stop(): Promise<void> {
     await this._watcher.close();
   }
@@ -103,10 +91,15 @@ class DefaultStorageRulesManager implements StorageRulesManager {
           "storage",
           "Change detected, updating rules for Cloud Storage..."
         );
+        this.updateRulesSource(rulesFile);
         await this.loadRuleset();
       });
   }
 
+  private updateRulesSource(rulesFile: string): void {
+    this._rules = { name: rulesFile, content: readFile(rulesFile) };
+  }
+
   private async loadRuleset(): Promise<StorageRulesIssues> {
     const { ruleset, issues } = await this._runtime.loadRuleset({ files: [this._rules] });
 
@@ -157,12 +150,6 @@ class ResourceBasedStorageRulesManager implements StorageRulesManager {
     return this._rulesManagers.get(resource)?.getRuleset();
   }
 
-  updateSourceFile(rules: SourceFile, resource: string): Promise<StorageRulesIssues> {
-    const rulesManager =
-      this._rulesManagers.get(resource) || this.createRulesManager(resource, rules);
-    return rulesManager.updateSourceFile(rules);
-  }
-
   async stop(): Promise<void> {
     await Promise.all(
       Array.from(this._rulesManagers.values(), async (rulesManager) => await rulesManager.stop())