diff --git a/src/deploy/functions/release/fabricator.spec.ts b/src/deploy/functions/release/fabricator.spec.ts index 20fda610483..1c0d4250d5f 100644 --- a/src/deploy/functions/release/fabricator.spec.ts +++ b/src/deploy/functions/release/fabricator.spec.ts @@ -14,6 +14,8 @@ import * as runNS from "../../../gcp/run"; import * as runV2NS from "../../../gcp/runv2"; import * as cloudtasksNS from "../../../gcp/cloudtasks"; import * as backend from "../backend"; +import * as utils from "../../../utils"; +import { FIREBASE_FRAMEWORKS_CODEBASE_PREFIX } from "../../../frameworks/constants"; import * as scraper from "./sourceTokenScraper"; import * as planner from "./planner"; import * as v2events from "../../../functions/events/v2"; @@ -76,6 +78,7 @@ describe("Fabricator", () => { run.setIamPolicy.rejects(new Error("unexpected run.setIamPolicy")); run.setInvokerCreate.rejects(new Error("unexpected run.setInvokerCreate")); run.setInvokerUpdate.rejects(new Error("unexpected run.setInvokerUpdate")); + run.ensureInvokerPublic.rejects(new Error("unexpected run.ensureInvokerPublic")); run.replaceService.rejects(new Error("unexpected run.replaceService")); run.updateService.rejects(new Error("Unexpected run.updateService")); runv2.createService.rejects(new Error("unexpected runv2.createService")); @@ -112,6 +115,7 @@ describe("Fabricator", () => { object: "object", generation: 42, }; + const frameworksCodebase = `${FIREBASE_FRAMEWORKS_CODEBASE_PREFIX}site`; const ctorArgs: fabricator.FabricatorArgs = { executor: new executor.InlineExecutor(), functionExecutor: new executor.InlineExecutor(), @@ -121,6 +125,10 @@ describe("Fabricator", () => { sourceUrl: "https://example.com", storage: storage, }, + [frameworksCodebase]: { + sourceUrl: "https://example.com", + storage: storage, + }, }, appEngineLocation: "us-central1", projectNumber: "1234567", @@ -984,6 +992,45 @@ describe("Fabricator", () => { expect(run.setInvokerUpdate).to.not.have.been.called; }); + it("ensures a frameworks SSR function stays publicly invokable when no invoker is declared", async () => { + gcfv2.updateFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.ensureInvokerPublic.resolves(); + const ep = endpoint( + { httpsTrigger: {} }, + { platform: "gcfv2", codebase: frameworksCodebase }, + ); + + await fab.updateV2Function(ep, new scraper.SourceTokenScraper()); + expect(run.ensureInvokerPublic).to.have.been.calledWith("service"); + expect(run.setInvokerUpdate).to.not.have.been.called; + }); + + it("warns but does not fail the deploy when ensuring a frameworks function's invoker fails", async () => { + gcfv2.updateFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.ensureInvokerPublic.rejects(new Error("permission denied")); + const warn = sinon.stub(utils, "logLabeledWarning"); + const ep = endpoint( + { httpsTrigger: {} }, + { platform: "gcfv2", codebase: frameworksCodebase }, + ); + + await expect(fab.updateV2Function(ep, new scraper.SourceTokenScraper())).to.not.be.rejected; + expect(run.ensureInvokerPublic).to.have.been.calledWith("service"); + expect(warn).to.have.been.calledOnce; + expect(warn.firstCall.args[1]).to.contain("publicly invokable"); + }); + + it("does not ensure public invoker for non-frameworks functions", async () => { + gcfv2.updateFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" }); + + await fab.updateV2Function(ep, new scraper.SourceTokenScraper()); + expect(run.ensureInvokerPublic).to.not.have.been.called; + }); + it("updates invoker to public on Node updates when explicitly null", async () => { gcfv2.updateFunction.resolves({ name: "op", done: false }); poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); diff --git a/src/deploy/functions/release/fabricator.ts b/src/deploy/functions/release/fabricator.ts index be42e9a0e3a..1155b1618ef 100644 --- a/src/deploy/functions/release/fabricator.ts +++ b/src/deploy/functions/release/fabricator.ts @@ -27,6 +27,7 @@ import * as scheduler from "../../../gcp/cloudscheduler"; import * as utils from "../../../utils"; import * as services from "../services"; import { getDataConnectP4SA } from "../services/dataconnect"; +import { isFrameworksManagedCodebase } from "../../../frameworks/constants"; import { AUTH_BLOCKING_EVENTS } from "../../../functions/events/v1"; import * as gce from "../../../gcp/computeEngine"; import { getHumanFriendlyPlatformName } from "../functionsDeployHelper"; @@ -626,6 +627,29 @@ export class Fabricator { await this.executor .run(() => run.setInvokerUpdate(endpoint.project, serviceName, invoker!)) .catch(rethrowAs(endpoint, "set invoker")); + } else if ( + backend.isHttpsTriggered(endpoint) && + isFrameworksManagedCodebase(endpoint.codebase) + ) { + // Frameworks-managed SSR functions are fronted by Firebase Hosting, which + // invokes them anonymously, so they must remain publicly invokable. They + // declare no invoker, so the branch above leaves IAM untouched and a binding + // removed out-of-band (e.g. by an org policy) is never restored. Re-assert + // allUsers additively — preserving any members the user added — and warn + // rather than fail the deploy when something blocks it. + // See: https://cloud.google.com/run/docs/authenticating/public + // See: https://github.com/firebase/firebase-tools/issues/10631 + await this.executor + .run(() => run.ensureInvokerPublic(serviceName)) + .catch((err: unknown) => { + utils.logLabeledWarning( + "functions", + `Unable to make the SSR function ${endpoint.id} publicly invokable. Requests served ` + + `by it will return 403 until allUsers is granted the Cloud Run Invoker role. ` + + `See https://cloud.google.com/run/docs/authenticating/public`, + ); + logger.debug(`Failed to ensure ${serviceName} is publicly invokable: ${err}`); + }); } } diff --git a/src/frameworks/constants.ts b/src/frameworks/constants.ts index bcac69e8d1b..0c00218236a 100644 --- a/src/frameworks/constants.ts +++ b/src/frameworks/constants.ts @@ -73,6 +73,17 @@ export const ALLOWED_SSR_REGIONS = [ export const I18N_ROOT = "/"; +/** + * Prefix of the Cloud Functions codebase that the web frameworks integration + * generates for a site's SSR function. See generateSSRCodebaseId. + */ +export const FIREBASE_FRAMEWORKS_CODEBASE_PREFIX = "firebase-frameworks-"; + +/** Whether a codebase is one the web frameworks integration manages for SSR. */ +export function isFrameworksManagedCodebase(codebase: string | undefined): boolean { + return !!codebase?.startsWith(FIREBASE_FRAMEWORKS_CODEBASE_PREFIX); +} + export function GET_DEFAULT_BUILD_TARGETS() { return Promise.resolve(["production", "development"]); } diff --git a/src/frameworks/index.ts b/src/frameworks/index.ts index d9fca0fe123..847a6bdca0d 100644 --- a/src/frameworks/index.ts +++ b/src/frameworks/index.ts @@ -32,6 +32,7 @@ import { DEFAULT_REGION, DEFAULT_SHOULD_USE_DEV_MODE_HANDLE, FIREBASE_ADMIN_VERSION, + FIREBASE_FRAMEWORKS_CODEBASE_PREFIX, FIREBASE_FRAMEWORKS_VERSION, FIREBASE_FUNCTIONS_VERSION, GET_DEFAULT_BUILD_TARGETS, @@ -111,7 +112,7 @@ function memoizeBuild( * during hosting deploy. */ export function generateSSRCodebaseId(site: string) { - return `firebase-frameworks-${site}`; + return `${FIREBASE_FRAMEWORKS_CODEBASE_PREFIX}${site}`; } /** diff --git a/src/gcp/run.spec.ts b/src/gcp/run.spec.ts index 3661401fcdc..0e5d7d39f81 100644 --- a/src/gcp/run.spec.ts +++ b/src/gcp/run.spec.ts @@ -270,6 +270,79 @@ describe("run", () => { }); }); }); + describe("ensureInvokerPublic", () => { + let sandbox: sinon.SinonSandbox; + let getIamPolicyStub: sinon.SinonStub; + let setIamPolicyStub: sinon.SinonStub; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + getIamPolicyStub = sandbox.stub(run, "getIamPolicy"); + setIamPolicyStub = sandbox.stub(run, "setIamPolicy").resolves(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should add allUsers when the service has no policy", async () => { + getIamPolicyStub.resolves({}); + + await run.ensureInvokerPublic("service"); + + expect(setIamPolicyStub).to.be.calledOnceWith("service", { + bindings: [{ role: "roles/run.invoker", members: ["allUsers"] }], + etag: "", + version: 3, + }); + }); + + it("should add allUsers additively, preserving other members and roles", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { role: "random-role", members: ["user:pineapple"] }, + { + role: "roles/run.invoker", + members: ["serviceAccount:sa@project.iam.gserviceaccount.com"], + }, + ], + etag: "1234", + version: 3, + }); + + await run.ensureInvokerPublic("service"); + + expect(setIamPolicyStub).to.be.calledOnceWith("service", { + bindings: [ + { role: "random-role", members: ["user:pineapple"] }, + { + role: "roles/run.invoker", + members: ["serviceAccount:sa@project.iam.gserviceaccount.com", "allUsers"], + }, + ], + etag: "1234", + version: 3, + }); + }); + + it("should be a no-op when allUsers already has the invoker role", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/run.invoker", + members: ["serviceAccount:sa@project.iam.gserviceaccount.com", "allUsers"], + }, + ], + etag: "1234", + version: 3, + }); + + await run.ensureInvokerPublic("service"); + + expect(setIamPolicyStub).to.not.be.called; + }); + }); + describe("updateService", () => { let service: run.Service; let serviceIsResolved: sinon.SinonStub; diff --git a/src/gcp/run.ts b/src/gcp/run.ts index 837e436f946..d3b5127747f 100644 --- a/src/gcp/run.ts +++ b/src/gcp/run.ts @@ -346,6 +346,38 @@ export async function setInvokerUpdate( await setIamPolicy(serviceName, policy, httpClient); } +/** + * Ensures a service is publicly invokable by granting allUsers the invoker role. + * Unlike setInvokerUpdate this is additive: it preserves any invoker members the + * user already added and is a no-op when allUsers already has the role, so it can + * re-assert public access without overwriting deliberate IAM changes. + * @param serviceName Fully qualified name of the Service. + * @throws {@link FirebaseError} when the IAM policy fails to be read or set. + */ +export async function ensureInvokerPublic(serviceName: string): Promise { + const invokerRole = "roles/run.invoker"; + const currentPolicy: IamPolicy = await exports.getIamPolicy(serviceName); + const currentInvokerBinding = currentPolicy.bindings?.find( + (binding) => binding.role === invokerRole, + ); + if (currentInvokerBinding?.members?.includes("allUsers")) { + return; + } + + const bindings = (currentPolicy.bindings || []).filter((binding) => binding.role !== invokerRole); + bindings.push({ + role: invokerRole, + members: [...(currentInvokerBinding?.members || []), "allUsers"], + }); + + const policy: iam.Policy = { + bindings: bindings, + etag: currentPolicy.etag || "", + version: 3, + }; + await exports.setIamPolicy(serviceName, policy); +} + /** * Fetches recent logs for a given Cloud Run service using the Cloud Logging API. * @param projectId The Google Cloud project ID.