HTTP Invoker for v2 functions (#3683)

* adding invoker to replace allUsers

* fixing build issues

* adding tests

* update function names, clean up comments

* add invoker option when parsing function from SDK

* fix service account format for IAM API

* fixing ts issues and added check for full service account

* cleaning up code for readability

* merged iam policy creation and set policy into setInvoker

* cleaning up old function and adding comments

* added unit tests

* fixing api version & removing import

* adding setInvoker to cloud run

* cleaning up

* lint fix

* fix reference

* added v2 invoker functions

* fixing pr comments

src/deploy/functions/tasks.ts

@@ -128,7 +128,7 @@ export function createFunctionTask(
             await gcf.setInvokerCreate(params.projectId, fnName, invoker);
           } else {
             const serviceName = (cloudFunction as gcfV2.CloudFunction).serviceConfig.service!;
-            cloudrun.setIamPolicy(serviceName, cloudrun.DEFAULT_PUBLIC_POLICY);
+            cloudrun.setInvokerCreate(params.projectId, serviceName, invoker);
           }
         } catch (err) {
           params.errorHandler.record("error", fnName, "set invoker", err.message);
@@ -197,7 +197,8 @@ export function updateFunctionTask(
         if (fn.platform === "gcfv1") {
           await gcf.setInvokerUpdate(params.projectId, fnName, fn.invoker);
         } else {
-          // TODO: gcfv2
+          const serviceName = (cloudFunction as gcfV2.CloudFunction).serviceConfig.service!;
+          cloudrun.setInvokerUpdate(params.projectId, serviceName, fn.invoker);
         }
       } catch (err) {
         params.errorHandler.record("error", fnName, "set invoker", err.message);

src/gcp/cloudfunctions.ts

@@ -331,7 +331,7 @@ export async function setInvokerUpdate(
   );
   if (
     currentInvokerBinding &&
-    _.isEqual(new Set(currentInvokerBinding.members), new Set(invokerMembers))
+    JSON.stringify(currentInvokerBinding.members.sort()) === JSON.stringify(invokerMembers.sort())
   ) {
     return;
   }

src/gcp/run.ts

@@ -2,6 +2,8 @@ import { Client } from "../apiv2";
 import { FirebaseError } from "../error";
 import { runOrigin } from "../api";
 import * as proto from "./proto";
+import * as iam from "./iam";
+import * as _ from "lodash";
 
 const API_VERSION = "v1";
 
@@ -105,22 +107,12 @@ export interface TrafficTarget {
 }
 
 export interface IamPolicy {
-  version: number;
+  version?: number;
-  bindings: Record<string, unknown>[];
+  bindings?: iam.Binding[];
   auditConfigs?: Record<string, unknown>[];
   etag?: string;
 }
 
-export const DEFAULT_PUBLIC_POLICY = {
-  version: 3,
-  bindings: [
-    {
-      role: "roles/run.invoker",
-      members: ["allUsers"],
-    },
-  ],
-};
-
 export async function getService(name: string): Promise<Service> {
   try {
     const response = await client.get<Service>(name);
@@ -148,16 +140,20 @@ export async function replaceService(name: string, service: Service): Promise<Se
  * @param name Fully qualified name of the Service.
  * @param policy The [policy](https://cloud.google.com/run/docs/reference/rest/v1/projects.locations.services/setIamPolicy) to set.
  */
-export async function setIamPolicy(name: string, policy: IamPolicy): Promise<void> {
+export async function setIamPolicy(
+  name: string,
+  policy: iam.Policy,
+  httpClient: Client = client
+): Promise<void> {
   // Cloud Run has an atypical REST binding for SetIamPolicy. Instead of making the body a policy and
   // the update mask a query parameter (e.g. Cloud Functions v1) the request body is the literal
   // proto.
   interface Request {
-    policy: IamPolicy;
+    policy: iam.Policy;
     updateMask: string;
   }
   try {
-    await client.post<Request, IamPolicy>(`${name}:setIamPolicy`, {
+    await httpClient.post<Request, IamPolicy>(`${name}:setIamPolicy`, {
       policy,
       updateMask: proto.fieldMasks(policy).join(","),
     });
@@ -167,3 +163,91 @@ export async function setIamPolicy(name: string, policy: IamPolicy): Promise<voi
     });
   }
 }
+
+export async function getIamPolicy(
+  serviceName: string,
+  httpClient: Client = client
+): Promise<IamPolicy> {
+  try {
+    const response = await httpClient.get<IamPolicy>(`${serviceName}:getIamPolicy`);
+    return response.body;
+  } catch (err) {
+    throw new FirebaseError(`Failed to get the IAM Policy on the Service ${serviceName}`, {
+      original: err,
+    });
+  }
+}
+
+/**
+ * Gets the current IAM policy for the run service and overrides the invoker role with the supplied invoker members
+ * @param projectId id of the project
+ * @param serviceName cloud run service
+ * @param invoker an array of invoker strings
+ *
+ * @throws {@link FirebaseError} on an empty invoker, when the IAM Polciy fails to be grabbed or set
+ */
+export async function setInvokerCreate(
+  projectId: string,
+  serviceName: string,
+  invoker: string[],
+  httpClient: Client = client // for unit testing
+) {
+  if (invoker.length == 0) {
+    throw new FirebaseError("Invoker cannot be an empty array");
+  }
+  const invokerMembers = proto.getInvokerMembers(invoker, projectId);
+  const invokerRole = "roles/run.invoker";
+  const bindings = [{ role: invokerRole, members: invokerMembers }];
+
+  const policy: iam.Policy = {
+    bindings: bindings,
+    etag: "",
+    version: 3,
+  };
+
+  await setIamPolicy(serviceName, policy, httpClient);
+}
+
+/**
+ * Gets the current IAM policy for the run service and overrides the invoker role with the supplied invoker members
+ * @param projectId id of the project
+ * @param serviceName cloud run service
+ * @param invoker an array of invoker strings
+ *
+ * @throws {@link FirebaseError} on an empty invoker, when the IAM Polciy fails to be grabbed or set
+ */
+export async function setInvokerUpdate(
+  projectId: string,
+  serviceName: string,
+  invoker: string[],
+  httpClient: Client = client // for unit testing
+) {
+  if (invoker.length == 0) {
+    throw new FirebaseError("Invoker cannot be an empty array");
+  }
+  const invokerMembers = proto.getInvokerMembers(invoker, projectId);
+  const invokerRole = "roles/run.invoker";
+  const currentPolicy = await getIamPolicy(serviceName, httpClient);
+  const currentInvokerBinding = currentPolicy.bindings?.find(
+    (binding) => binding.role === invokerRole
+  );
+  if (
+    currentInvokerBinding &&
+    JSON.stringify(currentInvokerBinding.members.sort()) === JSON.stringify(invokerMembers.sort())
+  ) {
+    return;
+  }
+
+  const bindings = (currentPolicy.bindings || []).filter((binding) => binding.role !== invokerRole);
+  bindings.push({
+    role: invokerRole,
+    members: invokerMembers,
+  });
+
+  const policy: iam.Policy = {
+    bindings: bindings,
+    etag: currentPolicy.etag || "",
+    version: 3,
+  };
+  await setIamPolicy(serviceName, policy, httpClient);
+}

src/test/gcp/run.spec.ts

@@ -0,0 +1,273 @@
+import { expect } from "chai";
+import * as sinon from "sinon";
+import * as run from "../../gcp/run";
+import { Client } from "../../apiv2";
+
+describe("run", () => {
+  describe("setInvokerCreate", () => {
+    let sandbox: sinon.SinonSandbox;
+    let apiRequestStub: sinon.SinonStub;
+    let client: Client;
+
+    beforeEach(() => {
+      client = new Client({
+        urlPrefix: "origin",
+        auth: true,
+        apiVersion: "v1",
+      });
+      sandbox = sinon.createSandbox();
+      apiRequestStub = sandbox.stub(client, "post").throws("Unexpected API post call");
+    });
+
+    afterEach(() => {
+      sandbox.restore();
+    });
+
+    it("should reject on emtpy invoker array", async () => {
+      await expect(run.setInvokerCreate("project", "service", [], client)).to.be.rejected;
+    });
+
+    it("should reject if the setting the IAM policy fails", async () => {
+      apiRequestStub.onFirstCall().throws("Error calling set api.");
+
+      await expect(
+        run.setInvokerCreate("project", "service", ["public"], client)
+      ).to.be.rejectedWith("Failed to set the IAM Policy on the Service service");
+      expect(apiRequestStub).to.be.calledOnce;
+    });
+
+    it("should set a private policy on a function", async () => {
+      apiRequestStub.onFirstCall().callsFake((path: string, json: any) => {
+        expect(json.policy).to.deep.eq({
+          bindings: [
+            {
+              role: "roles/run.invoker",
+              members: [],
+            },
+          ],
+          etag: "",
+          version: 3,
+        });
+
+        return Promise.resolve();
+      });
+
+      await expect(run.setInvokerCreate("project", "service", ["private"], client)).to.not.be
+        .rejected;
+      expect(apiRequestStub).to.be.calledOnce;
+    });
+
+    it("should set a public policy on a function", async () => {
+      apiRequestStub.onFirstCall().callsFake((path: string, json: any) => {
+        expect(json.policy).to.deep.eq({
+          bindings: [
+            {
+              role: "roles/run.invoker",
+              members: ["allUsers"],
+            },
+          ],
+          etag: "",
+          version: 3,
+        });
+
+        return Promise.resolve();
+      });
+
+      await expect(run.setInvokerCreate("project", "service", ["public"], client)).to.not.be
+        .rejected;
+      expect(apiRequestStub).to.be.calledOnce;
+    });
+
+    it("should set the policy with a set of invokers with active policies", async () => {
+      apiRequestStub.onFirstCall().callsFake((path: string, json: any) => {
+        json.policy.bindings[0].members.sort();
+        expect(json.policy.bindings[0].members).to.deep.eq([
+          "serviceAccount:service-account1@project.iam.gserviceaccount.com",
+          "serviceAccount:service-account2@project.iam.gserviceaccount.com",
+          "serviceAccount:service-account3@project.iam.gserviceaccount.com",
+        ]);
+
+        return Promise.resolve();
+      });
+
+      await expect(
+        run.setInvokerCreate(
+          "project",
+          "service",
+          [
+            "service-account1@",
+            "service-account2@project.iam.gserviceaccount.com",
+            "service-account3@",
+          ],
+          client
+        )
+      ).to.not.be.rejected;
+      expect(apiRequestStub).to.be.calledOnce;
+    });
+  });
+
+  describe("setInvokerUpdate", () => {
+    describe("setInvokerCreate", () => {
+      let sandbox: sinon.SinonSandbox;
+      let apiPostStub: sinon.SinonStub;
+      let apiGetStub: sinon.SinonStub;
+      let client: Client;
+
+      beforeEach(() => {
+        client = new Client({
+          urlPrefix: "origin",
+          auth: true,
+          apiVersion: "v1",
+        });
+        sandbox = sinon.createSandbox();
+        apiPostStub = sandbox.stub(client, "post").throws("Unexpected API post call");
+        apiGetStub = sandbox.stub(client, "get").throws("Unexpected API get call");
+      });
+
+      afterEach(() => {
+        sandbox.restore();
+      });
+
+      it("should reject on emtpy invoker array", async () => {
+        await expect(run.setInvokerUpdate("project", "service", [])).to.be.rejected;
+      });
+
+      it("should reject if the getting the IAM policy fails", async () => {
+        apiGetStub.onFirstCall().throws("Error calling get api.");
+
+        await expect(
+          run.setInvokerUpdate("project", "service", ["public"], client)
+        ).to.be.rejectedWith("Failed to get the IAM Policy on the Service service");
+
+        expect(apiGetStub).to.be.called;
+      });
+
+      it("should reject if the setting the IAM policy fails", async () => {
+        apiGetStub.resolves({ body: {} });
+        apiPostStub.throws("Error calling set api.");
+
+        await expect(
+          run.setInvokerUpdate("project", "service", ["public"], client)
+        ).to.be.rejectedWith("Failed to set the IAM Policy on the Service service");
+        expect(apiGetStub).to.be.calledOnce;
+        expect(apiPostStub).to.be.calledOnce;
+      });
+
+      it("should set a basic policy on a function without any polices", async () => {
+        apiGetStub.onFirstCall().resolves({ body: {} });
+        apiPostStub.onFirstCall().callsFake((path: string, json: any) => {
+          expect(json.policy).to.deep.eq({
+            bindings: [
+              {
+                role: "roles/run.invoker",
+                members: ["allUsers"],
+              },
+            ],
+            etag: "",
+            version: 3,
+          });
+
+          return Promise.resolve();
+        });
+
+        await expect(run.setInvokerUpdate("project", "service", ["public"], client)).to.not.be
+          .rejected;
+        expect(apiGetStub).to.be.calledOnce;
+        expect(apiPostStub).to.be.calledOnce;
+      });
+
+      it("should set the policy with private invoker with active policies", async () => {
+        apiGetStub.onFirstCall().resolves({
+          body: {
+            bindings: [
+              { role: "random-role", members: ["user:pineapple"] },
+              { role: "roles/run.invoker", members: ["some-service-account"] },
+            ],
+            etag: "1234",
+            version: 3,
+          },
+        });
+        apiPostStub.onFirstCall().callsFake((path: string, json: any) => {
+          expect(json.policy).to.deep.eq({
+            bindings: [
+              { role: "random-role", members: ["user:pineapple"] },
+              { role: "roles/run.invoker", members: [] },
+            ],
+            etag: "1234",
+            version: 3,
+          });
+
+          return Promise.resolve();
+        });
+
+        await expect(run.setInvokerUpdate("project", "service", ["private"], client)).to.not.be
+          .rejected;
+        expect(apiGetStub).to.be.calledOnce;
+        expect(apiPostStub).to.be.calledOnce;
+      });
+
+      it("should set the policy with a set of invokers with active policies", async () => {
+        apiGetStub.onFirstCall().resolves({ body: {} });
+        apiPostStub.onFirstCall().callsFake((path: string, json: any) => {
+          json.policy.bindings[0].members.sort();
+          expect(json.policy.bindings[0].members).to.deep.eq([
+            "serviceAccount:service-account1@project.iam.gserviceaccount.com",
+            "serviceAccount:service-account2@project.iam.gserviceaccount.com",
+            "serviceAccount:service-account3@project.iam.gserviceaccount.com",
+          ]);
+
+          return Promise.resolve();
+        });
+
+        await expect(
+          run.setInvokerUpdate(
+            "project",
+            "service",
+            [
+              "service-account1@",
+              "service-account2@project.iam.gserviceaccount.com",
+              "service-account3@",
+            ],
+            client
+          )
+        ).to.not.be.rejected;
+        expect(apiGetStub).to.be.calledOnce;
+        expect(apiPostStub).to.be.calledOnce;
+      });
+
+      it("should not set the policy if the set of invokers is the same as the current invokers", async () => {
+        apiGetStub.onFirstCall().resolves({
+          body: {
+            bindings: [
+              {
+                role: "roles/run.invoker",
+                members: [
+                  "serviceAccount:service-account1@project.iam.gserviceaccount.com",
+                  "serviceAccount:service-account3@project.iam.gserviceaccount.com",
+                  "serviceAccount:service-account2@project.iam.gserviceaccount.com",
+                ],
+              },
+            ],
+            etag: "1234",
+            version: 3,
+          },
+        });
+
+        await expect(
+          run.setInvokerUpdate(
+            "project",
+            "service",
+            [
+              "service-account2@project.iam.gserviceaccount.com",
+              "service-account3@",
+              "service-account1@",
+            ],
+            client
+          )
+        ).to.not.be.rejected;
+        expect(apiGetStub).to.be.calledOnce;
+        expect(apiPostStub).to.not.be.called;
+      });
+    });
+  });
+});