Update /internal/setRules endpoint; fix ruleset file watcher

CHANGELOG.md

@@ -1,3 +1,4 @@
 - Fixes bug where resumable uploads were not setting custom metadata on upload (#3398).
 - Fixes bug where GCS metadataUpdate cloud functions were triggered in incorrect situations (#3398).
 - Fixes bug where quoted escape sequences in .env files were incompletely unescaped. (#4270)
+- Fixes Storage Emulator ruleset file watcher (#4337).

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,
@@ -1141,6 +1141,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"
+        );
+      });
+    });
+  });
+
   /**
    * TODO(abhisun): Add test coverage to validate how many times various cloud functions are triggered.
    */

src/emulator/storage/index.ts

@@ -6,7 +6,7 @@ import { createApp } from "./server";
 import { StorageLayer, StoredFile } 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 {
@@ -42,7 +42,7 @@ export class StorageEmulator implements EmulatorInstance {
 
   private _logger = EmulatorLogger.forEmulator(Emulators.STORAGE);
   private _rulesRuntime: StorageRulesRuntime;
-  private _rulesManager: StorageRulesManager;
+  private _rulesManager!: StorageRulesManager;
   private _files: Map<string, StoredFile> = new Map();
   private _buckets: Map<string, CloudStorageBucketMetadata> = new Map();
   private _cloudFunctions: StorageCloudFunctions;
@@ -54,7 +54,7 @@ export class StorageEmulator implements EmulatorInstance {
 
   constructor(private args: StorageEmulatorArgs) {
     this._rulesRuntime = new StorageRulesRuntime();
-    this._rulesManager = createStorageRulesManager(this.args.rules, this._rulesRuntime);
+    this._rulesManager = this.createRulesManager(this.args.rules);
     this._cloudFunctions = new StorageCloudFunctions(args.projectId);
     this._persistence = new Persistence(this.getPersistenceTmpDir());
     this._uploadService = new UploadService(this._persistence);
@@ -141,6 +141,16 @@ export class StorageEmulator implements EmulatorInstance {
     return this._app!;
   }
 
+  async replaceRules(rules: SourceFile | RulesConfig[]): Promise<StorageRulesIssues> {
+    await this._rulesManager.stop();
+    this._rulesManager = this.createRulesManager(rules);
+    return this._rulesManager.start();
+  }
+
+  private createRulesManager(rules: SourceFile | RulesConfig[]): StorageRulesManager {
+    return createStorageRulesManager(rules, this._rulesRuntime);
+  }
+
   private getPersistenceTmpDir(): string {
     return `${tmpdir()}/firebase/storage/blobs`;
   }