Skip to content

Commit df9d6c9

Browse files
7hokerzIzaakGoughjoehan
authored
feat: add apphosting secrets revokeaccess command (#10669)
* feat: add apphosting secrets revokeaccess command * docs: update CHANGELOG.md * chore: apply suggestion --------- Co-authored-by: Izaak Gough <izaak.gough@invertase.io> Co-authored-by: Joe Hanley <joehanley@google.com>
1 parent a16b455 commit df9d6c9

5 files changed

Lines changed: 342 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
- Added `apphosting:secrets:revokeaccess` command. (#10669)
12
- Updated Pub/Sub emulator to version 0.8.33.
23
- Updated Data Connect emulator to version 3.4.12.
34
- Fix Data Connect non-deterministic output order of generated SDK files when compiled from multiple GQL source files.

‎src/apphosting/secrets/index.spec.ts‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,97 @@ describe("secrets", () => {
262262
});
263263
});
264264

265+
describe("revokeSecretAccess", () => {
266+
const secret = {
267+
name: "secret",
268+
projectId: "projectId",
269+
};
270+
271+
it("should revoke access from the appropriate service accounts", async () => {
272+
gcsm.getIamPolicy.resolves({
273+
version: 1,
274+
etag: "tag",
275+
bindings: [
276+
{
277+
role: "roles/viewer",
278+
members: ["serviceAccount:existingSA"],
279+
},
280+
{
281+
role: "roles/secretmanager.secretAccessor",
282+
members: [
283+
"serviceAccount:buildSA",
284+
"serviceAccount:computeSA",
285+
"serviceAccount:otherSA",
286+
],
287+
},
288+
{
289+
role: "roles/secretmanager.secretAccessor",
290+
members: ["serviceAccount:buildSA"],
291+
},
292+
{
293+
role: "roles/secretmanager.viewer",
294+
members: ["serviceAccount:buildSA", "serviceAccount:otherBuildSA"],
295+
},
296+
{
297+
role: "roles/secretmanager.secretVersionManager",
298+
members: [
299+
"serviceAccount:service-12345@gcp-sa-firebaseapphosting.iam.gserviceaccount.com",
300+
],
301+
},
302+
],
303+
});
304+
gcsm.setIamPolicy.resolves();
305+
306+
await secrets.revokeSecretAccess(secret.projectId, secret.name, {
307+
buildServiceAccounts: ["buildSA"],
308+
runServiceAccounts: ["computeSA"],
309+
});
310+
311+
expect(gcsm.getIamPolicy).to.be.calledWithMatch(secret);
312+
expect(gcsm.setIamPolicy).to.be.calledWithMatch(secret, [
313+
{
314+
role: "roles/viewer",
315+
members: ["serviceAccount:existingSA"],
316+
},
317+
{
318+
role: "roles/secretmanager.secretAccessor",
319+
members: ["serviceAccount:otherSA"],
320+
},
321+
{
322+
role: "roles/secretmanager.viewer",
323+
members: ["serviceAccount:otherBuildSA"],
324+
},
325+
{
326+
role: "roles/secretmanager.secretVersionManager",
327+
members: [
328+
"serviceAccount:service-12345@gcp-sa-firebaseapphosting.iam.gserviceaccount.com",
329+
],
330+
},
331+
]);
332+
});
333+
334+
it("should not set IAM policy if no matching bindings exist", async () => {
335+
gcsm.getIamPolicy.resolves({
336+
version: 1,
337+
etag: "tag",
338+
bindings: [
339+
{
340+
role: "roles/secretmanager.secretAccessor",
341+
members: ["serviceAccount:otherSA"],
342+
},
343+
],
344+
});
345+
346+
await secrets.revokeSecretAccess(secret.projectId, secret.name, {
347+
buildServiceAccounts: ["buildSA"],
348+
runServiceAccounts: ["computeSA"],
349+
});
350+
351+
expect(gcsm.getIamPolicy).to.be.calledWithMatch(secret);
352+
expect(gcsm.setIamPolicy).to.not.have.been.called;
353+
});
354+
});
355+
265356
describe("grantEmailsSecretAccess", () => {
266357
const secret = {
267358
projectId: "projectId",
@@ -384,6 +475,53 @@ describe("secrets", () => {
384475
});
385476
});
386477

478+
describe("revokeEmailsSecretAccess", () => {
479+
const secret = {
480+
projectId: "projectId",
481+
name: "secret",
482+
};
483+
484+
it("should revoke user and group access to secrets", async () => {
485+
gcsm.getIamPolicy.resolves({
486+
version: 1,
487+
etag: "tag",
488+
bindings: [
489+
{
490+
role: "roles/viewer",
491+
members: ["serviceAccount:existingSA"],
492+
},
493+
{
494+
role: "roles/secretmanager.secretAccessor",
495+
members: [
496+
"user:user@mydomain.com",
497+
"group:mygroup@mydomain.com",
498+
"serviceAccount:buildSA",
499+
],
500+
},
501+
],
502+
});
503+
gcsm.setIamPolicy.resolves();
504+
505+
await secrets.revokeEmailsSecretAccess(
506+
secret.projectId,
507+
[secret.name],
508+
["user@mydomain.com", "mygroup@mydomain.com"],
509+
);
510+
511+
expect(gcsm.getIamPolicy).to.be.calledWithMatch(secret);
512+
expect(gcsm.setIamPolicy).to.be.calledWithMatch(secret, [
513+
{
514+
role: "roles/viewer",
515+
members: ["serviceAccount:existingSA"],
516+
},
517+
{
518+
role: "roles/secretmanager.secretAccessor",
519+
members: ["serviceAccount:buildSA"],
520+
},
521+
]);
522+
});
523+
});
524+
387525
describe("fetchSecrets", () => {
388526
const projectId = "randomProject";
389527
it("correctly attempts to fetch secret and it's version", async () => {

‎src/apphosting/secrets/index.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,120 @@ export async function grantEmailsSecretAccess(
181181
}
182182
}
183183

184+
/**
185+
* Revokes the backend service accounts' access permissions from the provided secret.
186+
*/
187+
export async function revokeSecretAccess(
188+
projectId: string,
189+
secretName: string,
190+
accounts: MultiServiceAccounts,
191+
): Promise<void> {
192+
const bindingsToRevoke: iam.Binding[] = [
193+
{
194+
role: "roles/secretmanager.secretAccessor",
195+
members: [...accounts.buildServiceAccounts, ...accounts.runServiceAccounts].map(
196+
(sa) => `serviceAccount:${sa}`,
197+
),
198+
},
199+
{
200+
role: "roles/secretmanager.viewer",
201+
members: accounts.buildServiceAccounts.map((sa) => `serviceAccount:${sa}`),
202+
},
203+
];
204+
205+
await revokeSecretBindings(projectId, secretName, bindingsToRevoke);
206+
}
207+
208+
/**
209+
* Revokes the following users or groups access from the provided secrets.
210+
*/
211+
export async function revokeEmailsSecretAccess(
212+
projectId: string,
213+
secretNames: string[],
214+
emails: string[],
215+
): Promise<void> {
216+
const bindingsToRevoke: iam.Binding[] = [
217+
{
218+
role: "roles/secretmanager.secretAccessor",
219+
members: emails.flatMap((email) => [`user:${email}`, `group:${email}`]),
220+
},
221+
];
222+
223+
for (const secretName of secretNames) {
224+
await revokeSecretBindings(projectId, secretName, bindingsToRevoke);
225+
}
226+
}
227+
228+
async function revokeSecretBindings(
229+
projectId: string,
230+
secretName: string,
231+
bindingsToRevoke: iam.Binding[],
232+
): Promise<void> {
233+
let existingBindings: iam.Binding[];
234+
try {
235+
existingBindings = (await gcsm.getIamPolicy({ projectId, name: secretName })).bindings || [];
236+
} catch (err: unknown) {
237+
throw new FirebaseError(
238+
`Failed to get IAM bindings on secret: ${secretName}. Ensure you have the permissions to do so and try again.`,
239+
{ original: getError(err) },
240+
);
241+
}
242+
243+
const removalsByRole = new Map<string, Set<string>>();
244+
245+
for (const binding of bindingsToRevoke) {
246+
let members = removalsByRole.get(binding.role);
247+
if (!members) {
248+
members = new Set<string>();
249+
removalsByRole.set(binding.role, members);
250+
}
251+
252+
for (const member of binding.members) {
253+
members.add(member);
254+
}
255+
}
256+
257+
let updated = false;
258+
const bindings: iam.Binding[] = [];
259+
260+
for (const binding of existingBindings) {
261+
const removals = removalsByRole.get(binding.role);
262+
263+
if (!removals || binding.condition) {
264+
bindings.push(binding);
265+
continue;
266+
}
267+
268+
const members = binding.members.filter((member) => !removals.has(member));
269+
if (members.length !== binding.members.length) {
270+
updated = true;
271+
}
272+
273+
if (members.length) {
274+
bindings.push({ ...binding, members });
275+
} else {
276+
updated = true;
277+
}
278+
}
279+
280+
if (!updated) {
281+
utils.logSuccess(`No matching IAM bindings found on secret ${secretName}.\n`);
282+
return;
283+
}
284+
285+
try {
286+
await gcsm.setIamPolicy({ projectId, name: secretName }, bindings);
287+
} catch (err: unknown) {
288+
throw new FirebaseError(
289+
`Failed to revoke IAM bindings ${JSON.stringify(bindingsToRevoke)} on secret: ${secretName}. Ensure you have the permissions to do so and try again. ` +
290+
"For more information visit https://cloud.google.com/secret-manager/docs/manage-access-to-secrets#required-roles",
291+
{ original: getError(err) },
292+
);
293+
}
294+
295+
utils.logSuccess(`Successfully revoked IAM bindings on secret ${secretName}.\n`);
296+
}
297+
184298
/**
185299
* Ensures a secret exists for use with app hosting, optionally locked to a region.
186300
* If a secret exists, we verify the user is not trying to change the region and verifies a secret

‎src/commands/apphosting-secrets-revokeaccess.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { Command } from "../command";
2+
import { Options } from "../options";
3+
import { needProjectId, needProjectNumber } from "../projectUtils";
4+
import { FirebaseError } from "../error";
5+
import { requireAuth } from "../requireAuth";
6+
import * as secretManager from "../gcp/secretManager";
7+
import { requirePermissions } from "../requirePermissions";
8+
import * as apphosting from "../gcp/apphosting";
9+
import * as secrets from "../apphosting/secrets";
10+
import { getBackendForAmbiguousLocation } from "../apphosting/backend";
11+
12+
export const command = new Command("apphosting:secrets:revokeaccess <secretNames>")
13+
.description(
14+
"Revoke service accounts, users, or groups permissions from the provided secret(s). Can pass one or more secrets, separated by a comma",
15+
)
16+
.option(
17+
"-l, --location <location>",
18+
"the location of the backend to revoke secret access from. Cannot be combined with --emails",
19+
"-",
20+
)
21+
.option(
22+
"-b, --backend <backend>",
23+
"the name of the backend to revoke secret access from. Cannot be combined with --emails",
24+
)
25+
.option(
26+
"-e, --emails <emails>",
27+
"comma delimited list of user or group emails to revoke secret access from. Cannot be combined with --backend",
28+
)
29+
.before(requireAuth)
30+
.before(secretManager.ensureApi)
31+
.before(apphosting.ensureApiEnabled)
32+
.before(requirePermissions, [
33+
"secretmanager.secrets.get",
34+
"secretmanager.secrets.getIamPolicy",
35+
"secretmanager.secrets.setIamPolicy",
36+
])
37+
.action(async (secretNames: string, options: Options) => {
38+
const projectId = needProjectId(options);
39+
40+
if (!options.backend && !options.emails) {
41+
throw new FirebaseError(
42+
"Missing required flag --backend or --emails. See firebase apphosting:secrets:revokeaccess --help for more info",
43+
);
44+
}
45+
if (options.backend && options.emails) {
46+
throw new FirebaseError(
47+
"Cannot specify both --backend and --emails. See firebase apphosting:secrets:revokeaccess --help for more info",
48+
);
49+
}
50+
51+
const secretList = secretNames.split(",");
52+
for (const secretName of secretList) {
53+
const exists = await secretManager.secretExists(projectId, secretName);
54+
if (!exists) {
55+
throw new FirebaseError(`Cannot find secret ${secretName}`);
56+
}
57+
}
58+
59+
if (options.emails) {
60+
return await secrets.revokeEmailsSecretAccess(
61+
projectId,
62+
secretList,
63+
String(options.emails).split(","),
64+
);
65+
}
66+
67+
const projectNumber = await needProjectNumber(options);
68+
const backendId = options.backend as string;
69+
const location = options.location as string;
70+
let backend: apphosting.Backend;
71+
if (location === "" || location === "-") {
72+
backend = await getBackendForAmbiguousLocation(
73+
projectId,
74+
backendId,
75+
"Please select the location of your backend:",
76+
);
77+
} else {
78+
backend = await apphosting.getBackend(projectId, location, backendId);
79+
}
80+
81+
const accounts = secrets.toMulti(
82+
await secrets.serviceAccountsForBackend(projectNumber, backend),
83+
);
84+
85+
await Promise.all(
86+
secretList.map((secretName) => secrets.revokeSecretAccess(projectId, secretName, accounts)),
87+
);
88+
});

‎src/commands/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ export function load(client: CLIClient): CLIClient {
205205
client.apphosting.backends.delete = loadCommand("apphosting-backends-delete");
206206
client.apphosting.secrets = {};
207207
client.apphosting.secrets.set = loadCommand("apphosting-secrets-set");
208+
client.apphosting.secrets.revokeaccess = loadCommand("apphosting-secrets-revokeaccess");
208209
client.apphosting.secrets.grantaccess = loadCommand("apphosting-secrets-grantaccess");
209210
client.apphosting.secrets.describe = loadCommand("apphosting-secrets-describe");
210211
client.apphosting.secrets.access = loadCommand("apphosting-secrets-access");

0 commit comments

Comments
 (0)