diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..b3dcbd9cc17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Added `apphosting:secrets:revokeaccess` command. (#10669) diff --git a/src/apphosting/secrets/index.spec.ts b/src/apphosting/secrets/index.spec.ts index 5dc86eb8253..68a8e40be9e 100644 --- a/src/apphosting/secrets/index.spec.ts +++ b/src/apphosting/secrets/index.spec.ts @@ -262,6 +262,97 @@ describe("secrets", () => { }); }); + describe("revokeSecretAccess", () => { + const secret = { + name: "secret", + projectId: "projectId", + }; + + it("should revoke access from the appropriate service accounts", async () => { + gcsm.getIamPolicy.resolves({ + version: 1, + etag: "tag", + bindings: [ + { + role: "roles/viewer", + members: ["serviceAccount:existingSA"], + }, + { + role: "roles/secretmanager.secretAccessor", + members: [ + "serviceAccount:buildSA", + "serviceAccount:computeSA", + "serviceAccount:otherSA", + ], + }, + { + role: "roles/secretmanager.secretAccessor", + members: ["serviceAccount:buildSA"], + }, + { + role: "roles/secretmanager.viewer", + members: ["serviceAccount:buildSA", "serviceAccount:otherBuildSA"], + }, + { + role: "roles/secretmanager.secretVersionManager", + members: [ + "serviceAccount:service-12345@gcp-sa-firebaseapphosting.iam.gserviceaccount.com", + ], + }, + ], + }); + gcsm.setIamPolicy.resolves(); + + await secrets.revokeSecretAccess(secret.projectId, secret.name, { + buildServiceAccounts: ["buildSA"], + runServiceAccounts: ["computeSA"], + }); + + expect(gcsm.getIamPolicy).to.be.calledWithMatch(secret); + expect(gcsm.setIamPolicy).to.be.calledWithMatch(secret, [ + { + role: "roles/viewer", + members: ["serviceAccount:existingSA"], + }, + { + role: "roles/secretmanager.secretAccessor", + members: ["serviceAccount:otherSA"], + }, + { + role: "roles/secretmanager.viewer", + members: ["serviceAccount:otherBuildSA"], + }, + { + role: "roles/secretmanager.secretVersionManager", + members: [ + "serviceAccount:service-12345@gcp-sa-firebaseapphosting.iam.gserviceaccount.com", + ], + }, + ]); + }); + + it("should not set IAM policy if no matching bindings exist", async () => { + gcsm.getIamPolicy.resolves({ + version: 1, + etag: "tag", + bindings: [ + { + role: "roles/secretmanager.secretAccessor", + members: ["serviceAccount:otherSA"], + }, + ], + }); + + await secrets.revokeSecretAccess(secret.projectId, secret.name, { + buildServiceAccounts: ["buildSA"], + runServiceAccounts: ["computeSA"], + }); + + expect(gcsm.getIamPolicy).to.be.calledWithMatch(secret); + expect(gcsm.setIamPolicy).to.not.have.been.called; + }); + }); + describe("grantEmailsSecretAccess", () => { const secret = { projectId: "projectId", @@ -384,6 +475,53 @@ describe("secrets", () => { }); }); + describe("revokeEmailsSecretAccess", () => { + const secret = { + projectId: "projectId", + name: "secret", + }; + + it("should revoke user and group access to secrets", async () => { + gcsm.getIamPolicy.resolves({ + version: 1, + etag: "tag", + bindings: [ + { + role: "roles/viewer", + members: ["serviceAccount:existingSA"], + }, + { + role: "roles/secretmanager.secretAccessor", + members: [ + "user:user@mydomain.com", + "group:mygroup@mydomain.com", + "serviceAccount:buildSA", + ], + }, + ], + }); + gcsm.setIamPolicy.resolves(); + + await secrets.revokeEmailsSecretAccess( + secret.projectId, + [secret.name], + ["user@mydomain.com", "mygroup@mydomain.com"], + ); + + expect(gcsm.getIamPolicy).to.be.calledWithMatch(secret); + expect(gcsm.setIamPolicy).to.be.calledWithMatch(secret, [ + { + role: "roles/viewer", + members: ["serviceAccount:existingSA"], + }, + { + role: "roles/secretmanager.secretAccessor", + members: ["serviceAccount:buildSA"], + }, + ]); + }); + }); + describe("fetchSecrets", () => { const projectId = "randomProject"; it("correctly attempts to fetch secret and it's version", async () => { diff --git a/src/apphosting/secrets/index.ts b/src/apphosting/secrets/index.ts index c85f0645abc..73ecd2dbd32 100644 --- a/src/apphosting/secrets/index.ts +++ b/src/apphosting/secrets/index.ts @@ -181,6 +181,120 @@ export async function grantEmailsSecretAccess( } } +/** + * Revokes the backend service accounts' access permissions from the provided secret. + */ +export async function revokeSecretAccess( + projectId: string, + secretName: string, + accounts: MultiServiceAccounts, +): Promise { + const bindingsToRevoke: iam.Binding[] = [ + { + role: "roles/secretmanager.secretAccessor", + members: [...accounts.buildServiceAccounts, ...accounts.runServiceAccounts].map( + (sa) => `serviceAccount:${sa}`, + ), + }, + { + role: "roles/secretmanager.viewer", + members: accounts.buildServiceAccounts.map((sa) => `serviceAccount:${sa}`), + }, + ]; + + await revokeSecretBindings(projectId, secretName, bindingsToRevoke); +} + +/** + * Revokes the following users or groups access from the provided secrets. + */ +export async function revokeEmailsSecretAccess( + projectId: string, + secretNames: string[], + emails: string[], +): Promise { + const bindingsToRevoke: iam.Binding[] = [ + { + role: "roles/secretmanager.secretAccessor", + members: emails.flatMap((email) => [`user:${email}`, `group:${email}`]), + }, + ]; + + for (const secretName of secretNames) { + await revokeSecretBindings(projectId, secretName, bindingsToRevoke); + } +} + +async function revokeSecretBindings( + projectId: string, + secretName: string, + bindingsToRevoke: iam.Binding[], +): Promise { + let existingBindings: iam.Binding[]; + try { + existingBindings = (await gcsm.getIamPolicy({ projectId, name: secretName })).bindings || []; + } catch (err: unknown) { + throw new FirebaseError( + `Failed to get IAM bindings on secret: ${secretName}. Ensure you have the permissions to do so and try again.`, + { original: getError(err) }, + ); + } + + const removalsByRole = new Map>(); + + for (const binding of bindingsToRevoke) { + let members = removalsByRole.get(binding.role); + if (!members) { + members = new Set(); + removalsByRole.set(binding.role, members); + } + + for (const member of binding.members) { + members.add(member); + } + } + + let updated = false; + const bindings: iam.Binding[] = []; + + for (const binding of existingBindings) { + const removals = removalsByRole.get(binding.role); + + if (!removals || binding.condition) { + bindings.push(binding); + continue; + } + + const members = binding.members.filter((member) => !removals.has(member)); + if (members.length !== binding.members.length) { + updated = true; + } + + if (members.length) { + bindings.push({ ...binding, members }); + } else { + updated = true; + } + } + + if (!updated) { + utils.logSuccess(`No matching IAM bindings found on secret ${secretName}.\n`); + return; + } + + try { + await gcsm.setIamPolicy({ projectId, name: secretName }, bindings); + } catch (err: unknown) { + throw new FirebaseError( + `Failed to revoke IAM bindings ${JSON.stringify(bindingsToRevoke)} on secret: ${secretName}. Ensure you have the permissions to do so and try again. ` + + "For more information visit https://cloud.google.com/secret-manager/docs/manage-access-to-secrets#required-roles", + { original: getError(err) }, + ); + } + + utils.logSuccess(`Successfully revoked IAM bindings on secret ${secretName}.\n`); +} + /** * Ensures a secret exists for use with app hosting, optionally locked to a region. * If a secret exists, we verify the user is not trying to change the region and verifies a secret diff --git a/src/commands/apphosting-secrets-revokeaccess.ts b/src/commands/apphosting-secrets-revokeaccess.ts new file mode 100644 index 00000000000..dae1f66629c --- /dev/null +++ b/src/commands/apphosting-secrets-revokeaccess.ts @@ -0,0 +1,88 @@ +import { Command } from "../command"; +import { Options } from "../options"; +import { needProjectId, needProjectNumber } from "../projectUtils"; +import { FirebaseError } from "../error"; +import { requireAuth } from "../requireAuth"; +import * as secretManager from "../gcp/secretManager"; +import { requirePermissions } from "../requirePermissions"; +import * as apphosting from "../gcp/apphosting"; +import * as secrets from "../apphosting/secrets"; +import { getBackendForAmbiguousLocation } from "../apphosting/backend"; + +export const command = new Command("apphosting:secrets:revokeaccess ") + .description( + "Revoke service accounts, users, or groups permissions from the provided secret(s). Can pass one or more secrets, separated by a comma", + ) + .option( + "-l, --location ", + "the location of the backend to revoke secret access from. Cannot be combined with --emails", + "-", + ) + .option( + "-b, --backend ", + "the name of the backend to revoke secret access from. Cannot be combined with --emails", + ) + .option( + "-e, --emails ", + "comma delimited list of user or group emails to revoke secret access from. Cannot be combined with --backend", + ) + .before(requireAuth) + .before(secretManager.ensureApi) + .before(apphosting.ensureApiEnabled) + .before(requirePermissions, [ + "secretmanager.secrets.get", + "secretmanager.secrets.getIamPolicy", + "secretmanager.secrets.setIamPolicy", + ]) + .action(async (secretNames: string, options: Options) => { + const projectId = needProjectId(options); + + if (!options.backend && !options.emails) { + throw new FirebaseError( + "Missing required flag --backend or --emails. See firebase apphosting:secrets:revokeaccess --help for more info", + ); + } + if (options.backend && options.emails) { + throw new FirebaseError( + "Cannot specify both --backend and --emails. See firebase apphosting:secrets:revokeaccess --help for more info", + ); + } + + const secretList = secretNames.split(","); + for (const secretName of secretList) { + const exists = await secretManager.secretExists(projectId, secretName); + if (!exists) { + throw new FirebaseError(`Cannot find secret ${secretName}`); + } + } + + if (options.emails) { + return await secrets.revokeEmailsSecretAccess( + projectId, + secretList, + String(options.emails).split(","), + ); + } + + const projectNumber = await needProjectNumber(options); + const backendId = options.backend as string; + const location = options.location as string; + let backend: apphosting.Backend; + if (location === "" || location === "-") { + backend = await getBackendForAmbiguousLocation( + projectId, + backendId, + "Please select the location of your backend:", + ); + } else { + backend = await apphosting.getBackend(projectId, location, backendId); + } + + const accounts = secrets.toMulti( + await secrets.serviceAccountsForBackend(projectNumber, backend), + ); + + await Promise.all( + secretList.map((secretName) => secrets.revokeSecretAccess(projectId, secretName, accounts)), + ); + }); diff --git a/src/commands/index.ts b/src/commands/index.ts index ea736e9144d..2178e4432f9 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -205,6 +205,7 @@ export function load(client: CLIClient): CLIClient { client.apphosting.backends.delete = loadCommand("apphosting-backends-delete"); client.apphosting.secrets = {}; client.apphosting.secrets.set = loadCommand("apphosting-secrets-set"); + client.apphosting.secrets.revokeaccess = loadCommand("apphosting-secrets-revokeaccess"); client.apphosting.secrets.grantaccess = loadCommand("apphosting-secrets-grantaccess"); client.apphosting.secrets.describe = loadCommand("apphosting-secrets-describe"); client.apphosting.secrets.access = loadCommand("apphosting-secrets-access");