Reimplement Storage emulator /internal/setRules (#6014)

* stash

* commit

* commit

* lint

* fix tests

* const

scripts/storage-emulator-integration/internal/tests.ts

@@ -0,0 +1,168 @@
+import { expect } from "chai";
+import * as supertest from "supertest";
+import { StorageRulesFiles } from "../../../src/test/emulators/fixtures";
+import { TriggerEndToEndTest } from "../../integration-helpers/framework";
+import {
+  EMULATORS_SHUTDOWN_DELAY_MS,
+  getStorageEmulatorHost,
+  readEmulatorConfig,
+  TEST_SETUP_TIMEOUT,
+} from "../utils";
+
+const FIREBASE_PROJECT = process.env.FBTOOLS_TARGET_PROJECT || "fake-project-id";
+const EMULATOR_CONFIG = readEmulatorConfig();
+const STORAGE_EMULATOR_HOST = getStorageEmulatorHost(EMULATOR_CONFIG);
+
+describe("Storage emulator internal endpoints", () => {
+  let test: TriggerEndToEndTest;
+
+  before(async function (this) {
+    this.timeout(TEST_SETUP_TIMEOUT);
+    process.env.STORAGE_EMULATOR_HOST = STORAGE_EMULATOR_HOST;
+    test = new TriggerEndToEndTest(FIREBASE_PROJECT, __dirname, EMULATOR_CONFIG);
+    await test.startEmulators(["--only", "auth,storage"]);
+  });
+
+  beforeEach(async () => {
+    // Reset emulator to default rules.
+    await supertest(STORAGE_EMULATOR_HOST)
+      .put("/internal/setRules")
+      .send({
+        rules: {
+          files: [StorageRulesFiles.readWriteIfAuth],
+        },
+      })
+      .expect(200);
+  });
+
+  after(async function (this) {
+    this.timeout(EMULATORS_SHUTDOWN_DELAY_MS);
+    delete process.env.STORAGE_EMULATOR_HOST;
+    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");
+    });
+  });
+});

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

@@ -1,10 +1,7 @@
 import { expect } from "chai";
 
 import { createTmpDir, StorageRulesFiles } from "../../../src/test/emulators/fixtures";
-import {
-  createStorageRulesManager,
-  StorageRulesManager,
-} from "../../../src/emulator/storage/rules/manager";
+import { createStorageRulesManager } from "../../../src/emulator/storage/rules/manager";
 import { StorageRulesRuntime } from "../../../src/emulator/storage/rules/runtime";
 import * as fs from "fs";
 import { RulesetOperationMethod, SourceFile } from "../../../src/emulator/storage/rules/types";
@@ -14,22 +11,21 @@ import * as path from "path";
 
 const EMULATOR_LOAD_RULESET_DELAY_MS = 20000;
 const SETUP_TIMEOUT = 60000;
+const PROJECT_ID = "demo-project-id";
 
 describe("Storage Rules Manager", () => {
   let rulesRuntime: StorageRulesRuntime;
   const opts = { method: RulesetOperationMethod.GET, file: {}, path: "/b/bucket_0/o/" };
-  const projectId = "demo-project-id";
-  let rulesManager: StorageRulesManager;
 
-  beforeEach(async function (this) {
+  before(async function (this) {
     this.timeout(SETUP_TIMEOUT);
     rulesRuntime = new StorageRulesRuntime();
     await rulesRuntime.start();
   });
 
-  afterEach(async function (this) {
+  after(async function (this) {
     this.timeout(SETUP_TIMEOUT);
-    await rulesManager.stop();
+    await rulesRuntime.stop();
   });
 
   it("should load multiple rulesets on start", async function (this) {
@@ -38,18 +34,30 @@ describe("Storage Rules Manager", () => {
       { resource: "bucket_0", rules: StorageRulesFiles.readWriteIfTrue },
       { resource: "bucket_1", rules: StorageRulesFiles.readWriteIfAuth },
     ];
-    rulesManager = createStorageRulesManager(rules, rulesRuntime);
+    const rulesManager = createStorageRulesManager(rules, rulesRuntime);
     await rulesManager.start();
 
     const bucket0Ruleset = rulesManager.getRuleset("bucket_0");
     expect(
-      await isPermitted({ ...opts, path: "/b/bucket_0/o/", ruleset: bucket0Ruleset!, projectId })
+      await isPermitted({
+        ...opts,
+        path: "/b/bucket_0/o/",
+        ruleset: bucket0Ruleset!,
+        projectId: PROJECT_ID,
+      })
     ).to.be.true;
 
     const bucket1Ruleset = rulesManager.getRuleset("bucket_1");
     expect(
-      await isPermitted({ ...opts, path: "/b/bucket_1/o/", ruleset: bucket1Ruleset!, projectId })
+      await isPermitted({
+        ...opts,
+        path: "/b/bucket_1/o/",
+        ruleset: bucket1Ruleset!,
+        projectId: PROJECT_ID,
+      })
     ).to.be.false;
+
+    await rulesManager.stop();
   });
 
   it("should load single ruleset on start", async function (this) {
@@ -60,11 +68,13 @@ describe("Storage Rules Manager", () => {
     appendBytes(testDir, fileName, Buffer.from(StorageRulesFiles.readWriteIfTrue.content));
 
     const sourceFile = getSourceFile(testDir, fileName);
-    rulesManager = createStorageRulesManager(sourceFile, rulesRuntime);
+    const rulesManager = createStorageRulesManager(sourceFile, rulesRuntime);
     await rulesManager.start();
 
     const ruleset = rulesManager.getRuleset("bucket");
-    expect(await isPermitted({ ...opts, ruleset: ruleset!, projectId })).to.be.true;
+    expect(await isPermitted({ ...opts, ruleset: ruleset!, projectId: PROJECT_ID })).to.be.true;
+
+    await rulesManager.stop();
   });
 
   it("should reload ruleset on changes to source file", async function (this) {
@@ -75,19 +85,31 @@ describe("Storage Rules Manager", () => {
     appendBytes(testDir, fileName, Buffer.from(StorageRulesFiles.readWriteIfTrue.content));
 
     const sourceFile = getSourceFile(testDir, fileName);
-    rulesManager = createStorageRulesManager(sourceFile, rulesRuntime);
+    const rulesManager = createStorageRulesManager(sourceFile, rulesRuntime);
     await rulesManager.start();
 
-    expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket")!, projectId }))
-      .to.be.true;
+    expect(
+      await isPermitted({
+        ...opts,
+        ruleset: rulesManager.getRuleset("bucket")!,
+        projectId: PROJECT_ID,
+      })
+    ).to.be.true;
 
     // Write new rules to file
     deleteFile(testDir, fileName);
     appendBytes(testDir, fileName, Buffer.from(StorageRulesFiles.readWriteIfAuth.content));
 
     await new Promise((resolve) => setTimeout(resolve, EMULATOR_LOAD_RULESET_DELAY_MS));
-    expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket")!, projectId }))
-      .to.be.false;
+    expect(
+      await isPermitted({
+        ...opts,
+        ruleset: rulesManager.getRuleset("bucket")!,
+        projectId: PROJECT_ID,
+      })
+    ).to.be.false;
+
+    await rulesManager.stop();
   });
 });
 

scripts/storage-emulator-integration/run.sh

@@ -10,6 +10,8 @@ source scripts/set-default-credentials.sh
 # Prepare the storage emulator rules runtime
 firebase setup:emulators:storage
 
+mocha scripts/storage-emulator-integration/internal/tests.ts
+
 mocha scripts/storage-emulator-integration/rules/*.test.ts
 
 mocha scripts/storage-emulator-integration/import/tests.ts

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 * as express from "express";
 import {
@@ -118,6 +118,7 @@ export class StorageEmulator implements EmulatorInstance {
 
   async stop(): Promise<void> {
     await this._persistence.deleteAll();
+    await this._rulesRuntime.stop();
     await this._rulesManager.stop();
     return this.destroyServer ? this.destroyServer() : Promise.resolve();
   }
@@ -145,6 +146,12 @@ export class StorageEmulator implements EmulatorInstance {
     return createStorageRulesManager(rules, this._rulesRuntime);
   }
 
+  async replaceRules(rules: SourceFile | RulesConfig[]): Promise<StorageRulesIssues> {
+    await this._rulesManager.stop();
+    this._rulesManager = this.createRulesManager(rules);
+    return this._rulesManager.start();
+  }
+
   private getPersistenceTmpDir(): string {
     return `${tmpdir()}/firebase/storage/blobs`;
   }

src/emulator/storage/rules/manager.ts

@@ -60,7 +60,6 @@ class DefaultStorageRulesManager implements StorageRulesManager {
   }
 
   async start(): Promise<StorageRulesIssues> {
-    this._runtime.start();
     const issues = await this.loadRuleset();
     this.updateWatcher(this._rules.name);
     return issues;
@@ -72,9 +71,6 @@ class DefaultStorageRulesManager implements StorageRulesManager {
 
   async stop(): Promise<void> {
     await this._watcher.close();
-    if (this._runtime.alive) {
-      await this._runtime.stop();
-    }
   }
 
   private updateWatcher(rulesFile: string): void {