From 6c898234c05dc1fb0ef16440e729c6932b5111fa Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Wed, 12 Aug 2026 14:32:20 -0700 Subject: [PATCH 01/10] feat: enforce and cache roles/mcp.toolUser for remote MCP calls Verifies that the authenticated account has the 'roles/mcp.toolUser' role before making remote calls for 'tools/call'. To avoid ResourceManager API query latency and quota burn, positive validations are cached indefinitely in configstore. If validation fails, it attempts to auto-bind the role using setIamPolicy, falling back to clear error instructions and manual gcloud repair commands on permission failures. - Unit tests for local caching logic, direct API checking (via force flag), prefix resolution, and auto-binding flows. - Unit tests verifying OneMcpServer callTool correctly invokes the validation logic. --- src/ensureRoleBound.spec.ts | 167 +++++++++++++++++++++++++++ src/ensureRoleBound.ts | 105 +++++++++++++++++ src/mcp/onemcp/onemcp_server.spec.ts | 30 +++++ src/mcp/onemcp/onemcp_server.ts | 10 ++ 4 files changed, 312 insertions(+) create mode 100644 src/ensureRoleBound.spec.ts create mode 100644 src/ensureRoleBound.ts diff --git a/src/ensureRoleBound.spec.ts b/src/ensureRoleBound.spec.ts new file mode 100644 index 00000000000..f941c310f26 --- /dev/null +++ b/src/ensureRoleBound.spec.ts @@ -0,0 +1,167 @@ +import { expect } from "chai"; +import * as sinon from "sinon"; +import { FirebaseError } from "./error"; +import { configstore } from "./configstore"; +import * as resourceManager from "./gcp/resourceManager"; +import { ensureRole } from "./ensureRoleBound"; +import * as utils from "./utils"; + +describe("ensureRole", () => { + let sandbox: sinon.SinonSandbox; + let getIamPolicyStub: sinon.SinonStub; + let setIamPolicyStub: sinon.SinonStub; + let cacheStore: Record; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + getIamPolicyStub = sandbox.stub(resourceManager, "getIamPolicy"); + setIamPolicyStub = sandbox.stub(resourceManager, "setIamPolicy"); + sandbox.stub(utils, "sleep").resolves(); + + cacheStore = {}; + sandbox.stub(configstore, "get").callsFake((key: string) => cacheStore[key]); + (sandbox.stub(configstore, "set") as any).callsFake((key: string, val: any) => { + cacheStore[key] = val; + }); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should succeed and cache positive check if role binding exists", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/mcp.toolUser", + members: ["user:test@example.com"], + }, + ], + }); + + await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + + expect(getIamPolicyStub).to.have.been.calledOnceWith("test-project"); + expect(cacheStore["iamRoleCache"]).to.deep.equal({ + "test-project": { + "test@example.com": { + "roles/mcp.toolUser": true, + }, + }, + }); + }); + + it("should skip API call and succeed if role is cached", async () => { + cacheStore["iamRoleCache"] = { + "test-project": { + "test@example.com": { + "roles/mcp.toolUser": true, + }, + }, + }; + + await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + + expect(getIamPolicyStub).to.not.have.been.called; + }); + + it("should query API even if cached when force is true", async () => { + cacheStore["iamRoleCache"] = { + "test-project": { + "test@example.com": { + "roles/mcp.toolUser": true, + }, + }, + }; + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/mcp.toolUser", + members: ["user:test@example.com"], + }, + ], + }); + + await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser", true); + + expect(getIamPolicyStub).to.have.been.calledOnceWith("test-project"); + }); + + it("should attempt to bind role and succeed/cache if setIamPolicy succeeds", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/viewer", + members: ["user:test@example.com"], + }, + ], + }); + setIamPolicyStub.resolves({} as any); + + await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + + expect(setIamPolicyStub).to.have.been.calledOnce; + expect(cacheStore["iamRoleCache"]).to.deep.equal({ + "test-project": { + "test@example.com": { + "roles/mcp.toolUser": true, + }, + }, + }); + }); + + it("should throw FirebaseError with instructions if setIamPolicy fails", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/viewer", + members: ["user:test@example.com"], + }, + ], + }); + setIamPolicyStub.rejects(new Error("Permission denied")); + + await expect( + ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"), + ).to.be.rejectedWith( + FirebaseError, + /Attempted to automatically bind the role but failed[\s\S]*gcloud beta projects add-iam-policy-binding/, + ); + + expect(cacheStore["iamRoleCache"]).to.be.undefined; + }); + + it("should resolve serviceAccount prefix correctly", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/mcp.toolUser", + members: ["serviceAccount:sa@proj.iam.gserviceaccount.com"], + }, + ], + }); + + await ensureRole("test-project", "sa@proj.iam.gserviceaccount.com", "roles/mcp.toolUser"); + + expect(getIamPolicyStub).to.have.been.calledOnce; + }); + + it("should use customLogger for debugging if provided", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/mcp.toolUser", + members: ["user:test@example.com"], + }, + ], + }); + const customLogger = { + debug: sandbox.stub(), + }; + + await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser", false, customLogger); + + expect(customLogger.debug).to.have.been.calledWith(sinon.match(/ensureRole called/)); + expect(customLogger.debug).to.have.been.calledWith(sinon.match(/Caching positive role check/)); + }); +}); diff --git a/src/ensureRoleBound.ts b/src/ensureRoleBound.ts new file mode 100644 index 00000000000..de4b6cd0774 --- /dev/null +++ b/src/ensureRoleBound.ts @@ -0,0 +1,105 @@ +import { bold } from "colorette"; +import { FirebaseError } from "./error"; +import { getIamPolicy, setIamPolicy } from "./gcp/resourceManager"; +import { configstore } from "./configstore"; +import { mergeBindings } from "./gcp/iam"; +import { logger } from "./logger"; +import { sleep } from "./utils"; + +const ROLE_CACHE_KEY = "iamRoleCache"; +const IAM_PROPAGATION_DELAY_MS = 10000; + +function checkRoleCache(projectId: string, email: string, role: string): boolean { + const cache = configstore.get(ROLE_CACHE_KEY) as Record< + string, + Record> + >; + return !!cache?.[projectId]?.[email]?.[role]; +} + +function cacheRole(projectId: string, email: string, role: string): void { + const cache = (configstore.get(ROLE_CACHE_KEY) || {}) as Record< + string, + Record> + >; + if (!cache[projectId]) { + cache[projectId] = {}; + } + if (!cache[projectId][email]) { + cache[projectId][email] = {}; + } + cache[projectId][email][role] = true; + configstore.set(ROLE_CACHE_KEY, cache); +} + +/** + * Assures that the authenticating account holds the specified IAM role on the given project. + * Uses local configstore cache to avoid RM API query latency, unless force is true. + */ +export async function ensureRole( + projectId: string, + accountEmail: string, + role: string, + force = false, + customLogger?: { debug: (message: string) => void }, +): Promise { + const log = customLogger || logger; + log.debug( + `[iam] ensureRole called for project: ${projectId}, account: ${accountEmail}, role: ${role}, force: ${force}`, + ); + if (!force && checkRoleCache(projectId, accountEmail, role)) { + log.debug( + `[iam] ensureRole early out: role ${role} is cached for project ${projectId} and account ${accountEmail}`, + ); + return; + } + + const policy = await getIamPolicy(projectId); + const memberName = accountEmail.includes("gserviceaccount.com") + ? `serviceAccount:${accountEmail}` + : `user:${accountEmail}`; + + const hasRole = + policy.bindings?.some((binding) => { + return binding.role === role && binding.members.includes(memberName); + }) ?? false; + + if (!hasRole) { + policy.bindings = policy.bindings || []; + mergeBindings(policy, [ + { + role, + members: [memberName], + }, + ]); + try { + log.debug( + `[iam] Attempting to setIamPolicy to bind role ${role} for ${memberName} on project ${projectId}`, + ); + await setIamPolicy(projectId, policy, "bindings"); + // It usually takes few seconds to few minutes to propagate. Wait ${IAM_PROPAGATION_DELAY_MS}ms here to be safe. + log.debug( + `[iam] Successfully updated IAM policy. Waiting ${IAM_PROPAGATION_DELAY_MS}ms for propagation...`, + ); + await sleep(IAM_PROPAGATION_DELAY_MS); + } catch (err: any) { + log.debug(`[iam] setIamPolicy failed: ${err.message || err}`); + throw new FirebaseError( + `Authorization failed. Account ${bold(accountEmail)} is missing the required IAM role ${bold( + role, + )} on project ${bold(projectId)}. Attempted to automatically bind the role but failed (error: ${err.message || err}).\n\n` + + `Please ask a project owner to grant you the role. They can do this either in the Google Cloud Console:\n` + + `https://console.cloud.google.com/iam-admin/iam?project=${projectId}\n\n` + + `Or by running the following command:\n` + + `gcloud beta projects add-iam-policy-binding ${projectId} \\\n` + + ` --member="${memberName}" \\\n` + + ` --role="${role}"`, + ); + } + } + + log.debug( + `[iam] Caching positive role check validation for project: ${projectId}, account: ${accountEmail}, role: ${role}`, + ); + cacheRole(projectId, accountEmail, role); +} diff --git a/src/mcp/onemcp/onemcp_server.spec.ts b/src/mcp/onemcp/onemcp_server.spec.ts index e56dee1a4d5..c12c91130af 100644 --- a/src/mcp/onemcp/onemcp_server.spec.ts +++ b/src/mcp/onemcp/onemcp_server.spec.ts @@ -6,11 +6,13 @@ import * as ensureModule from "../../ensureApiEnabled"; import { FirebaseError } from "../../error"; import { ServerFeature } from "../types"; import { LATEST_PROTOCOL_VERSION } from "@modelcontextprotocol/sdk/types.js"; +import * as ensureRoleBound from "../../ensureRoleBound"; describe("OneMcpServer", () => { let sandbox: sinon.SinonSandbox; let clientRequestStub: sinon.SinonStub; let ensureStub: sinon.SinonStub; + let ensureRoleStub: sinon.SinonStub; const feature = "auth" as ServerFeature; const serverUrl = "https://example.com"; @@ -20,6 +22,7 @@ describe("OneMcpServer", () => { sandbox = sinon.createSandbox(); clientRequestStub = sandbox.stub(Client.prototype, "request"); ensureStub = sandbox.stub(ensureModule, "ensure").resolves(); + ensureRoleStub = sandbox.stub(ensureRoleBound, "ensureRole").resolves(); server = new OneMcpServer(feature, serverUrl, { requiresAuth: false }); }); @@ -368,5 +371,32 @@ describe("OneMcpServer", () => { /is not allowed on remote server/, ); }); + + it("should call ensureRole when projectId and accountEmail are present in context", async () => { + const mockMcpTool = { name: "test_tool", inputSchema: { type: "object", properties: {} } }; + clientRequestStub.onFirstCall().resolves({ + body: { result: { tools: [mockMcpTool] } }, + }); + + const tools = await server.listTools(); + const tool = tools[0]; + + clientRequestStub.onSecondCall().resolves({ + body: { result: { content: [] } }, + }); + + const contextWithAuth = { + projectId: "test-project", + accountEmail: "user@example.com", + }; + + await tool.fn({}, contextWithAuth as any); + + expect(ensureRoleStub).to.have.been.calledOnceWith( + "test-project", + "user@example.com", + "roles/mcp.toolUser", + ); + }); }); }); diff --git a/src/mcp/onemcp/onemcp_server.ts b/src/mcp/onemcp/onemcp_server.ts index d9de6718b1f..5219901fc66 100644 --- a/src/mcp/onemcp/onemcp_server.ts +++ b/src/mcp/onemcp/onemcp_server.ts @@ -14,6 +14,7 @@ import { ServerTool, ServerToolMeta } from "../tool"; import { McpContext, ServerFeature } from "../types"; import { FirebaseError } from "../../error"; import { ensure } from "../../ensureApiEnabled"; +import { ensureRole } from "../../ensureRoleBound"; export interface OneMcpServerOptions { /** @@ -160,6 +161,15 @@ export class OneMcpServer { } } + if (ctx.projectId && ctx.accountEmail) { + await ensureRole( + ctx.projectId, + ctx.accountEmail, + "roles/mcp.toolUser", + false, + ctx.host?.logger, + ); + } // TODO: Optimize this to not call ensure on every tool call. if (ctx.projectId) { await ensure(ctx.projectId, this.serverUrl, this.feature, /* silent=*/ true); From e1442500d76ef3c2a1a68dfcedfbad60848ad496 Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Wed, 12 Aug 2026 15:35:24 -0700 Subject: [PATCH 02/10] address review feedback: implement cache TTL, print propagation warning, and make error handling type-safe --- src/ensureRoleBound.spec.ts | 62 ++++++++++++++++++++++++++++++++++--- src/ensureRoleBound.ts | 51 ++++++++++++++++++++---------- 2 files changed, 93 insertions(+), 20 deletions(-) diff --git a/src/ensureRoleBound.spec.ts b/src/ensureRoleBound.spec.ts index f941c310f26..d5048449590 100644 --- a/src/ensureRoleBound.spec.ts +++ b/src/ensureRoleBound.spec.ts @@ -17,6 +17,7 @@ describe("ensureRole", () => { getIamPolicyStub = sandbox.stub(resourceManager, "getIamPolicy"); setIamPolicyStub = sandbox.stub(resourceManager, "setIamPolicy"); sandbox.stub(utils, "sleep").resolves(); + sandbox.stub(Date, "now").returns(1710000000000); cacheStore = {}; sandbox.stub(configstore, "get").callsFake((key: string) => cacheStore[key]); @@ -45,13 +46,33 @@ describe("ensureRole", () => { expect(cacheStore["iamRoleCache"]).to.deep.equal({ "test-project": { "test@example.com": { - "roles/mcp.toolUser": true, + "roles/mcp.toolUser": { + valid: true, + timestamp: 1710000000000, + }, }, }, }); }); - it("should skip API call and succeed if role is cached", async () => { + it("should skip API call and succeed if role is cached and not expired", async () => { + cacheStore["iamRoleCache"] = { + "test-project": { + "test@example.com": { + "roles/mcp.toolUser": { + valid: true, + timestamp: 1710000000000, + }, + }, + }, + }; + + await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + + expect(getIamPolicyStub).to.not.have.been.called; + }); + + it("should skip API call and succeed if legacy boolean role is cached", async () => { cacheStore["iamRoleCache"] = { "test-project": { "test@example.com": { @@ -65,11 +86,41 @@ describe("ensureRole", () => { expect(getIamPolicyStub).to.not.have.been.called; }); + it("should query API if cached role has expired", async () => { + const now = 1710000000000; + const oneDayAgo = now - 24 * 60 * 60 * 1000 - 1000; // expired by 1 second + cacheStore["iamRoleCache"] = { + "test-project": { + "test@example.com": { + "roles/mcp.toolUser": { + valid: true, + timestamp: oneDayAgo, + }, + }, + }, + }; + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/mcp.toolUser", + members: ["user:test@example.com"], + }, + ], + }); + + await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + + expect(getIamPolicyStub).to.have.been.calledOnceWith("test-project"); + }); + it("should query API even if cached when force is true", async () => { cacheStore["iamRoleCache"] = { "test-project": { "test@example.com": { - "roles/mcp.toolUser": true, + "roles/mcp.toolUser": { + valid: true, + timestamp: 1710000000000, + }, }, }, }; @@ -104,7 +155,10 @@ describe("ensureRole", () => { expect(cacheStore["iamRoleCache"]).to.deep.equal({ "test-project": { "test@example.com": { - "roles/mcp.toolUser": true, + "roles/mcp.toolUser": { + valid: true, + timestamp: 1710000000000, + }, }, }, }); diff --git a/src/ensureRoleBound.ts b/src/ensureRoleBound.ts index de4b6cd0774..9ac8c62b751 100644 --- a/src/ensureRoleBound.ts +++ b/src/ensureRoleBound.ts @@ -1,34 +1,46 @@ import { bold } from "colorette"; -import { FirebaseError } from "./error"; +import { FirebaseError, getError } from "./error"; import { getIamPolicy, setIamPolicy } from "./gcp/resourceManager"; import { configstore } from "./configstore"; import { mergeBindings } from "./gcp/iam"; import { logger } from "./logger"; -import { sleep } from "./utils"; +import { sleep, logBullet } from "./utils"; const ROLE_CACHE_KEY = "iamRoleCache"; const IAM_PROPAGATION_DELAY_MS = 10000; +const CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours + +interface CacheEntry { + valid: boolean; + timestamp: number; +} + +type RoleCache = Record>>; function checkRoleCache(projectId: string, email: string, role: string): boolean { - const cache = configstore.get(ROLE_CACHE_KEY) as Record< - string, - Record> - >; - return !!cache?.[projectId]?.[email]?.[role]; + const cache = configstore.get(ROLE_CACHE_KEY) as RoleCache | undefined; + const entry = cache?.[projectId]?.[email]?.[role]; + if (!entry) { + return false; + } + if (typeof entry === "boolean") { + return entry; + } + return Date.now() - entry.timestamp < CACHE_TTL_MS; } function cacheRole(projectId: string, email: string, role: string): void { - const cache = (configstore.get(ROLE_CACHE_KEY) || {}) as Record< - string, - Record> - >; + const cache = (configstore.get(ROLE_CACHE_KEY) || {}) as RoleCache; if (!cache[projectId]) { cache[projectId] = {}; } if (!cache[projectId][email]) { cache[projectId][email] = {}; } - cache[projectId][email][role] = true; + cache[projectId][email][role] = { + valid: true, + timestamp: Date.now(), + }; configstore.set(ROLE_CACHE_KEY, cache); } @@ -45,7 +57,9 @@ export async function ensureRole( ): Promise { const log = customLogger || logger; log.debug( - `[iam] ensureRole called for project: ${projectId}, account: ${accountEmail}, role: ${role}, force: ${force}`, + `[iam] ensureRole called for project: ${projectId}, account: ${accountEmail}, role: ${role}, force: ${String( + force, + )}`, ); if (!force && checkRoleCache(projectId, accountEmail, role)) { log.debug( @@ -81,19 +95,24 @@ export async function ensureRole( log.debug( `[iam] Successfully updated IAM policy. Waiting ${IAM_PROPAGATION_DELAY_MS}ms for propagation...`, ); + logBullet( + `Successfully updated IAM policy. Waiting ${IAM_PROPAGATION_DELAY_MS / 1000}s for propagation...`, + ); await sleep(IAM_PROPAGATION_DELAY_MS); - } catch (err: any) { - log.debug(`[iam] setIamPolicy failed: ${err.message || err}`); + } catch (err: unknown) { + const error = getError(err); + log.debug(`[iam] setIamPolicy failed: ${error.message}`); throw new FirebaseError( `Authorization failed. Account ${bold(accountEmail)} is missing the required IAM role ${bold( role, - )} on project ${bold(projectId)}. Attempted to automatically bind the role but failed (error: ${err.message || err}).\n\n` + + )} on project ${bold(projectId)}. Attempted to automatically bind the role but failed (error: ${error.message}).\n\n` + `Please ask a project owner to grant you the role. They can do this either in the Google Cloud Console:\n` + `https://console.cloud.google.com/iam-admin/iam?project=${projectId}\n\n` + `Or by running the following command:\n` + `gcloud beta projects add-iam-policy-binding ${projectId} \\\n` + ` --member="${memberName}" \\\n` + ` --role="${role}"`, + { original: error }, ); } } From 160806248a12671a75ca7a0bc60a43bad6ef0d99 Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Wed, 12 Aug 2026 15:36:59 -0700 Subject: [PATCH 03/10] address review feedback: refine service account email suffix check --- src/ensureRoleBound.spec.ts | 19 +++++++++++++++++++ src/ensureRoleBound.ts | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/ensureRoleBound.spec.ts b/src/ensureRoleBound.spec.ts index d5048449590..96c05a84f4f 100644 --- a/src/ensureRoleBound.spec.ts +++ b/src/ensureRoleBound.spec.ts @@ -200,6 +200,25 @@ describe("ensureRole", () => { expect(getIamPolicyStub).to.have.been.calledOnce; }); + it("should resolve user prefix correctly even if email contains gserviceaccount.com as a substring", async () => { + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/mcp.toolUser", + members: ["user:sa@proj.iam.gserviceaccount.com.fake.com"], + }, + ], + }); + + await ensureRole( + "test-project", + "sa@proj.iam.gserviceaccount.com.fake.com", + "roles/mcp.toolUser", + ); + + expect(getIamPolicyStub).to.have.been.calledOnce; + }); + it("should use customLogger for debugging if provided", async () => { getIamPolicyStub.resolves({ bindings: [ diff --git a/src/ensureRoleBound.ts b/src/ensureRoleBound.ts index 9ac8c62b751..2daa8e084bc 100644 --- a/src/ensureRoleBound.ts +++ b/src/ensureRoleBound.ts @@ -69,7 +69,7 @@ export async function ensureRole( } const policy = await getIamPolicy(projectId); - const memberName = accountEmail.includes("gserviceaccount.com") + const memberName = accountEmail.endsWith(".gserviceaccount.com") ? `serviceAccount:${accountEmail}` : `user:${accountEmail}`; From 480c484fcbb5680ee1714e2d0c4b7f24e03f4e12 Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Wed, 12 Aug 2026 16:44:27 -0700 Subject: [PATCH 04/10] refactor: check IAM permissions first before setting role --- src/ensureRoleBound.spec.ts | 237 +++++++++++++++++---------- src/ensureRoleBound.ts | 102 ++++++++---- src/mcp/onemcp/onemcp_server.spec.ts | 11 +- src/mcp/onemcp/onemcp_server.ts | 5 +- 4 files changed, 234 insertions(+), 121 deletions(-) diff --git a/src/ensureRoleBound.spec.ts b/src/ensureRoleBound.spec.ts index 96c05a84f4f..a186512351e 100644 --- a/src/ensureRoleBound.spec.ts +++ b/src/ensureRoleBound.spec.ts @@ -3,19 +3,29 @@ import * as sinon from "sinon"; import { FirebaseError } from "./error"; import { configstore } from "./configstore"; import * as resourceManager from "./gcp/resourceManager"; -import { ensureRole } from "./ensureRoleBound"; +import * as iam from "./gcp/iam"; +import { ensurePermissionsThenSetRole } from "./ensureRoleBound"; import * as utils from "./utils"; -describe("ensureRole", () => { +describe("ensurePermissionsThenSetRole", () => { let sandbox: sinon.SinonSandbox; let getIamPolicyStub: sinon.SinonStub; let setIamPolicyStub: sinon.SinonStub; + let testIamPermissionsStub: sinon.SinonStub; let cacheStore: Record; + const mockPermissions = [ + "mcp.tools.call", + "resourcemanager.projects.get", + "resourcemanager.projects.list", + ]; + const mockRole = "roles/mcp.toolUser"; + beforeEach(() => { sandbox = sinon.createSandbox(); getIamPolicyStub = sandbox.stub(resourceManager, "getIamPolicy"); setIamPolicyStub = sandbox.stub(resourceManager, "setIamPolicy"); + testIamPermissionsStub = sandbox.stub(iam, "testIamPermissions"); sandbox.stub(utils, "sleep").resolves(); sandbox.stub(Date, "now").returns(1710000000000); @@ -30,115 +40,137 @@ describe("ensureRole", () => { sandbox.restore(); }); - it("should succeed and cache positive check if role binding exists", async () => { - getIamPolicyStub.resolves({ - bindings: [ - { - role: "roles/mcp.toolUser", - members: ["user:test@example.com"], - }, - ], + it("should succeed and cache positive check if permissions are already held", async () => { + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], }); - await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + await ensurePermissionsThenSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + ); - expect(getIamPolicyStub).to.have.been.calledOnceWith("test-project"); - expect(cacheStore["iamRoleCache"]).to.deep.equal({ + expect(testIamPermissionsStub).to.have.been.calledOnceWith("test-project", mockPermissions); + expect(getIamPolicyStub).to.not.have.been.called; + expect(cacheStore["iamPermissionCache"]).to.deep.equal({ "test-project": { "test@example.com": { - "roles/mcp.toolUser": { - valid: true, - timestamp: 1710000000000, - }, + "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, }, }, }); }); - it("should skip API call and succeed if role is cached and not expired", async () => { - cacheStore["iamRoleCache"] = { + it("should skip API call and succeed if permissions are cached and not expired", async () => { + cacheStore["iamPermissionCache"] = { "test-project": { "test@example.com": { - "roles/mcp.toolUser": { - valid: true, - timestamp: 1710000000000, - }, + "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, }, }, }; - await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + await ensurePermissionsThenSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + ); + expect(testIamPermissionsStub).to.not.have.been.called; expect(getIamPolicyStub).to.not.have.been.called; }); - it("should skip API call and succeed if legacy boolean role is cached", async () => { - cacheStore["iamRoleCache"] = { + it("should skip API call and succeed if legacy boolean permissions are cached", async () => { + cacheStore["iamPermissionCache"] = { "test-project": { "test@example.com": { - "roles/mcp.toolUser": true, + "mcp.tools.call": true, + "resourcemanager.projects.get": true, + "resourcemanager.projects.list": true, }, }, }; - await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + await ensurePermissionsThenSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + ); - expect(getIamPolicyStub).to.not.have.been.called; + expect(testIamPermissionsStub).to.not.have.been.called; }); - it("should query API if cached role has expired", async () => { + it("should query API if cached permissions have expired", async () => { const now = 1710000000000; const oneDayAgo = now - 24 * 60 * 60 * 1000 - 1000; // expired by 1 second - cacheStore["iamRoleCache"] = { + cacheStore["iamPermissionCache"] = { "test-project": { "test@example.com": { - "roles/mcp.toolUser": { - valid: true, - timestamp: oneDayAgo, - }, + "mcp.tools.call": { valid: true, timestamp: oneDayAgo }, + "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, }, }, }; - getIamPolicyStub.resolves({ - bindings: [ - { - role: "roles/mcp.toolUser", - members: ["user:test@example.com"], - }, - ], + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], }); - await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + await ensurePermissionsThenSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + ); - expect(getIamPolicyStub).to.have.been.calledOnceWith("test-project"); + expect(testIamPermissionsStub).to.have.been.calledOnceWith("test-project", mockPermissions); }); it("should query API even if cached when force is true", async () => { - cacheStore["iamRoleCache"] = { + cacheStore["iamPermissionCache"] = { "test-project": { "test@example.com": { - "roles/mcp.toolUser": { - valid: true, - timestamp: 1710000000000, - }, + "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, }, }, }; - getIamPolicyStub.resolves({ - bindings: [ - { - role: "roles/mcp.toolUser", - members: ["user:test@example.com"], - }, - ], + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], }); - await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser", true); + await ensurePermissionsThenSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + true, + ); - expect(getIamPolicyStub).to.have.been.calledOnceWith("test-project"); + expect(testIamPermissionsStub).to.have.been.calledOnceWith("test-project", mockPermissions); }); - it("should attempt to bind role and succeed/cache if setIamPolicy succeeds", async () => { + it("should attempt to bind role and succeed/cache if testIamPermissions fails but setIamPolicy succeeds", async () => { + testIamPermissionsStub.resolves({ + passed: false, + allowed: [], + missing: mockPermissions, + }); getIamPolicyStub.resolves({ bindings: [ { @@ -149,22 +181,31 @@ describe("ensureRole", () => { }); setIamPolicyStub.resolves({} as any); - await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"); + await ensurePermissionsThenSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + ); expect(setIamPolicyStub).to.have.been.calledOnce; - expect(cacheStore["iamRoleCache"]).to.deep.equal({ + expect(cacheStore["iamPermissionCache"]).to.deep.equal({ "test-project": { "test@example.com": { - "roles/mcp.toolUser": { - valid: true, - timestamp: 1710000000000, - }, + "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, + "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, }, }, }); }); - it("should throw FirebaseError with instructions if setIamPolicy fails", async () => { + it("should throw FirebaseError with instructions if testIamPermissions fails and setIamPolicy fails", async () => { + testIamPermissionsStub.resolves({ + passed: false, + allowed: [], + missing: mockPermissions, + }); getIamPolicyStub.resolves({ bindings: [ { @@ -176,65 +217,91 @@ describe("ensureRole", () => { setIamPolicyStub.rejects(new Error("Permission denied")); await expect( - ensureRole("test-project", "test@example.com", "roles/mcp.toolUser"), + ensurePermissionsThenSetRole("test-project", "test@example.com", mockPermissions, mockRole), ).to.be.rejectedWith( FirebaseError, - /Attempted to automatically bind the role but failed[\s\S]*gcloud beta projects add-iam-policy-binding/, + /Attempted to automatically bind the role roles\/mcp\.toolUser but failed/, ); - expect(cacheStore["iamRoleCache"]).to.be.undefined; + expect(cacheStore["iamPermissionCache"]).to.be.undefined; }); - it("should resolve serviceAccount prefix correctly", async () => { + it("should resolve serviceAccount prefix correctly when checking policy bindings", async () => { + testIamPermissionsStub.resolves({ + passed: false, + allowed: [], + missing: mockPermissions, + }); getIamPolicyStub.resolves({ bindings: [ { - role: "roles/mcp.toolUser", + role: mockRole, members: ["serviceAccount:sa@proj.iam.gserviceaccount.com"], }, ], }); - await ensureRole("test-project", "sa@proj.iam.gserviceaccount.com", "roles/mcp.toolUser"); + await ensurePermissionsThenSetRole( + "test-project", + "sa@proj.iam.gserviceaccount.com", + mockPermissions, + mockRole, + ); expect(getIamPolicyStub).to.have.been.calledOnce; + expect(setIamPolicyStub).to.not.have.been.called; }); it("should resolve user prefix correctly even if email contains gserviceaccount.com as a substring", async () => { + testIamPermissionsStub.resolves({ + passed: false, + allowed: [], + missing: mockPermissions, + }); getIamPolicyStub.resolves({ bindings: [ { - role: "roles/mcp.toolUser", + role: mockRole, members: ["user:sa@proj.iam.gserviceaccount.com.fake.com"], }, ], }); - await ensureRole( + await ensurePermissionsThenSetRole( "test-project", "sa@proj.iam.gserviceaccount.com.fake.com", - "roles/mcp.toolUser", + mockPermissions, + mockRole, ); expect(getIamPolicyStub).to.have.been.calledOnce; + expect(setIamPolicyStub).to.not.have.been.called; }); it("should use customLogger for debugging if provided", async () => { - getIamPolicyStub.resolves({ - bindings: [ - { - role: "roles/mcp.toolUser", - members: ["user:test@example.com"], - }, - ], + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], }); const customLogger = { debug: sandbox.stub(), }; - await ensureRole("test-project", "test@example.com", "roles/mcp.toolUser", false, customLogger); + await ensurePermissionsThenSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + false, + customLogger, + ); - expect(customLogger.debug).to.have.been.calledWith(sinon.match(/ensureRole called/)); - expect(customLogger.debug).to.have.been.calledWith(sinon.match(/Caching positive role check/)); + expect(customLogger.debug).to.have.been.calledWith( + sinon.match(/ensurePermissionsThenSetRole called/), + ); + expect(customLogger.debug).to.have.been.calledWith( + sinon.match(/Caching positive permissions check/), + ); }); }); diff --git a/src/ensureRoleBound.ts b/src/ensureRoleBound.ts index 2daa8e084bc..3f6d74b6cc3 100644 --- a/src/ensureRoleBound.ts +++ b/src/ensureRoleBound.ts @@ -2,11 +2,11 @@ import { bold } from "colorette"; import { FirebaseError, getError } from "./error"; import { getIamPolicy, setIamPolicy } from "./gcp/resourceManager"; import { configstore } from "./configstore"; -import { mergeBindings } from "./gcp/iam"; +import { mergeBindings, testIamPermissions } from "./gcp/iam"; import { logger } from "./logger"; import { sleep, logBullet } from "./utils"; -const ROLE_CACHE_KEY = "iamRoleCache"; +const PERMISSION_CACHE_KEY = "iamPermissionCache"; const IAM_PROPAGATION_DELAY_MS = 10000; const CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours @@ -15,59 +15,97 @@ interface CacheEntry { timestamp: number; } -type RoleCache = Record>>; +type PermissionCache = Record>>; -function checkRoleCache(projectId: string, email: string, role: string): boolean { - const cache = configstore.get(ROLE_CACHE_KEY) as RoleCache | undefined; - const entry = cache?.[projectId]?.[email]?.[role]; - if (!entry) { - return false; - } - if (typeof entry === "boolean") { - return entry; +function checkPermissionCache(projectId: string, email: string, permissions: string[]): boolean { + const cache = configstore.get(PERMISSION_CACHE_KEY) as PermissionCache | undefined; + for (const perm of permissions) { + const entry = cache?.[projectId]?.[email]?.[perm]; + if (!entry) { + return false; + } + if (typeof entry === "boolean") { + if (!entry) return false; + continue; + } + if (Date.now() - entry.timestamp >= CACHE_TTL_MS) { + return false; + } } - return Date.now() - entry.timestamp < CACHE_TTL_MS; + return true; } -function cacheRole(projectId: string, email: string, role: string): void { - const cache = (configstore.get(ROLE_CACHE_KEY) || {}) as RoleCache; +function cachePermissions(projectId: string, email: string, permissions: string[]): void { + const cache = (configstore.get(PERMISSION_CACHE_KEY) || {}) as PermissionCache; if (!cache[projectId]) { cache[projectId] = {}; } if (!cache[projectId][email]) { cache[projectId][email] = {}; } - cache[projectId][email][role] = { - valid: true, - timestamp: Date.now(), - }; - configstore.set(ROLE_CACHE_KEY, cache); + for (const perm of permissions) { + cache[projectId][email][perm] = { + valid: true, + timestamp: Date.now(), + }; + } + configstore.set(PERMISSION_CACHE_KEY, cache); } /** - * Assures that the authenticating account holds the specified IAM role on the given project. + * Assures that the authenticating account holds the specified IAM permissions on the given project. + * If not, it attempts to bind the specified IAM role to the user. * Uses local configstore cache to avoid RM API query latency, unless force is true. */ -export async function ensureRole( +export async function ensurePermissionsThenSetRole( projectId: string, accountEmail: string, + permissions: string[], role: string, force = false, customLogger?: { debug: (message: string) => void }, ): Promise { const log = customLogger || logger; log.debug( - `[iam] ensureRole called for project: ${projectId}, account: ${accountEmail}, role: ${role}, force: ${String( - force, - )}`, + `[iam] ensurePermissionsThenSetRole called for project: ${projectId}, account: ${accountEmail}, permissions: ${JSON.stringify( + permissions, + )}, role: ${role}, force: ${String(force)}`, + ); + + if (!force && checkPermissionCache(projectId, accountEmail, permissions)) { + log.debug( + `[iam] ensurePermissionsThenSetRole early out: permissions ${JSON.stringify( + permissions, + )} are cached for project ${projectId} and account ${accountEmail}`, + ); + return; + } + + log.debug( + `[iam] Checking permissions ${JSON.stringify(permissions)} on project ${projectId} for ${accountEmail}`, ); - if (!force && checkRoleCache(projectId, accountEmail, role)) { + const iamResult = await testIamPermissions(projectId, permissions); + if (iamResult.passed) { + log.debug( + `[iam] Account ${accountEmail} already has all required permissions: ${JSON.stringify( + permissions, + )}`, + ); log.debug( - `[iam] ensureRole early out: role ${role} is cached for project ${projectId} and account ${accountEmail}`, + `[iam] Caching positive permissions check validation for project: ${projectId}, account: ${accountEmail}, permissions: ${JSON.stringify( + permissions, + )}`, ); + cachePermissions(projectId, accountEmail, permissions); return; } + log.debug( + `[iam] Account ${accountEmail} is missing permissions: ${JSON.stringify( + iamResult.missing, + )}. Attempting to bind role ${role}`, + ); + const policy = await getIamPolicy(projectId); const memberName = accountEmail.endsWith(".gserviceaccount.com") ? `serviceAccount:${accountEmail}` @@ -103,9 +141,11 @@ export async function ensureRole( const error = getError(err); log.debug(`[iam] setIamPolicy failed: ${error.message}`); throw new FirebaseError( - `Authorization failed. Account ${bold(accountEmail)} is missing the required IAM role ${bold( + `Authorization failed. Account ${bold(accountEmail)} is missing the required IAM permissions on project ${bold( + projectId, + )}. Attempted to automatically bind the role ${bold( role, - )} on project ${bold(projectId)}. Attempted to automatically bind the role but failed (error: ${error.message}).\n\n` + + )} but failed (error: ${error.message}).\n\n` + `Please ask a project owner to grant you the role. They can do this either in the Google Cloud Console:\n` + `https://console.cloud.google.com/iam-admin/iam?project=${projectId}\n\n` + `Or by running the following command:\n` + @@ -118,7 +158,9 @@ export async function ensureRole( } log.debug( - `[iam] Caching positive role check validation for project: ${projectId}, account: ${accountEmail}, role: ${role}`, + `[iam] Caching positive permissions check validation for project: ${projectId}, account: ${accountEmail}, permissions: ${JSON.stringify( + permissions, + )}`, ); - cacheRole(projectId, accountEmail, role); + cachePermissions(projectId, accountEmail, permissions); } diff --git a/src/mcp/onemcp/onemcp_server.spec.ts b/src/mcp/onemcp/onemcp_server.spec.ts index c12c91130af..27d5c88b086 100644 --- a/src/mcp/onemcp/onemcp_server.spec.ts +++ b/src/mcp/onemcp/onemcp_server.spec.ts @@ -12,7 +12,7 @@ describe("OneMcpServer", () => { let sandbox: sinon.SinonSandbox; let clientRequestStub: sinon.SinonStub; let ensureStub: sinon.SinonStub; - let ensureRoleStub: sinon.SinonStub; + let ensurePermissionsThenSetRoleStub: sinon.SinonStub; const feature = "auth" as ServerFeature; const serverUrl = "https://example.com"; @@ -22,7 +22,9 @@ describe("OneMcpServer", () => { sandbox = sinon.createSandbox(); clientRequestStub = sandbox.stub(Client.prototype, "request"); ensureStub = sandbox.stub(ensureModule, "ensure").resolves(); - ensureRoleStub = sandbox.stub(ensureRoleBound, "ensureRole").resolves(); + ensurePermissionsThenSetRoleStub = sandbox + .stub(ensureRoleBound, "ensurePermissionsThenSetRole") + .resolves(); server = new OneMcpServer(feature, serverUrl, { requiresAuth: false }); }); @@ -372,7 +374,7 @@ describe("OneMcpServer", () => { ); }); - it("should call ensureRole when projectId and accountEmail are present in context", async () => { + it("should call ensurePermissionsThenSetRole when projectId and accountEmail are present in context", async () => { const mockMcpTool = { name: "test_tool", inputSchema: { type: "object", properties: {} } }; clientRequestStub.onFirstCall().resolves({ body: { result: { tools: [mockMcpTool] } }, @@ -392,9 +394,10 @@ describe("OneMcpServer", () => { await tool.fn({}, contextWithAuth as any); - expect(ensureRoleStub).to.have.been.calledOnceWith( + expect(ensurePermissionsThenSetRoleStub).to.have.been.calledOnceWith( "test-project", "user@example.com", + ["mcp.tools.call", "resourcemanager.projects.get", "resourcemanager.projects.list"], "roles/mcp.toolUser", ); }); diff --git a/src/mcp/onemcp/onemcp_server.ts b/src/mcp/onemcp/onemcp_server.ts index 5219901fc66..f5abaf6f316 100644 --- a/src/mcp/onemcp/onemcp_server.ts +++ b/src/mcp/onemcp/onemcp_server.ts @@ -14,7 +14,7 @@ import { ServerTool, ServerToolMeta } from "../tool"; import { McpContext, ServerFeature } from "../types"; import { FirebaseError } from "../../error"; import { ensure } from "../../ensureApiEnabled"; -import { ensureRole } from "../../ensureRoleBound"; +import { ensurePermissionsThenSetRole } from "../../ensureRoleBound"; export interface OneMcpServerOptions { /** @@ -162,9 +162,10 @@ export class OneMcpServer { } if (ctx.projectId && ctx.accountEmail) { - await ensureRole( + await ensurePermissionsThenSetRole( ctx.projectId, ctx.accountEmail, + ["mcp.tools.call", "resourcemanager.projects.get", "resourcemanager.projects.list"], "roles/mcp.toolUser", false, ctx.host?.logger, From e35665bc76276f2e52f3934d49d0abb784b721d2 Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Wed, 12 Aug 2026 16:48:06 -0700 Subject: [PATCH 05/10] refactor: rename ensureRoleBound to ensurePermissions and function to ensurePermissionsOrSetRole --- ...ound.spec.ts => ensurePermissions.spec.ts} | 51 +++++-------------- ...nsureRoleBound.ts => ensurePermissions.ts} | 6 +-- src/mcp/onemcp/onemcp_server.spec.ts | 12 ++--- src/mcp/onemcp/onemcp_server.ts | 6 +-- 4 files changed, 25 insertions(+), 50 deletions(-) rename src/{ensureRoleBound.spec.ts => ensurePermissions.spec.ts} (87%) rename src/{ensureRoleBound.ts => ensurePermissions.ts} (95%) diff --git a/src/ensureRoleBound.spec.ts b/src/ensurePermissions.spec.ts similarity index 87% rename from src/ensureRoleBound.spec.ts rename to src/ensurePermissions.spec.ts index a186512351e..2bc059c938d 100644 --- a/src/ensureRoleBound.spec.ts +++ b/src/ensurePermissions.spec.ts @@ -4,10 +4,10 @@ import { FirebaseError } from "./error"; import { configstore } from "./configstore"; import * as resourceManager from "./gcp/resourceManager"; import * as iam from "./gcp/iam"; -import { ensurePermissionsThenSetRole } from "./ensureRoleBound"; +import { ensurePermissionsOrSetRole } from "./ensurePermissions"; import * as utils from "./utils"; -describe("ensurePermissionsThenSetRole", () => { +describe("ensurePermissionsOrSetRole", () => { let sandbox: sinon.SinonSandbox; let getIamPolicyStub: sinon.SinonStub; let setIamPolicyStub: sinon.SinonStub; @@ -47,12 +47,7 @@ describe("ensurePermissionsThenSetRole", () => { missing: [], }); - await ensurePermissionsThenSetRole( - "test-project", - "test@example.com", - mockPermissions, - mockRole, - ); + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); expect(testIamPermissionsStub).to.have.been.calledOnceWith("test-project", mockPermissions); expect(getIamPolicyStub).to.not.have.been.called; @@ -78,12 +73,7 @@ describe("ensurePermissionsThenSetRole", () => { }, }; - await ensurePermissionsThenSetRole( - "test-project", - "test@example.com", - mockPermissions, - mockRole, - ); + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); expect(testIamPermissionsStub).to.not.have.been.called; expect(getIamPolicyStub).to.not.have.been.called; @@ -100,12 +90,7 @@ describe("ensurePermissionsThenSetRole", () => { }, }; - await ensurePermissionsThenSetRole( - "test-project", - "test@example.com", - mockPermissions, - mockRole, - ); + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); expect(testIamPermissionsStub).to.not.have.been.called; }); @@ -128,12 +113,7 @@ describe("ensurePermissionsThenSetRole", () => { missing: [], }); - await ensurePermissionsThenSetRole( - "test-project", - "test@example.com", - mockPermissions, - mockRole, - ); + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); expect(testIamPermissionsStub).to.have.been.calledOnceWith("test-project", mockPermissions); }); @@ -154,7 +134,7 @@ describe("ensurePermissionsThenSetRole", () => { missing: [], }); - await ensurePermissionsThenSetRole( + await ensurePermissionsOrSetRole( "test-project", "test@example.com", mockPermissions, @@ -181,12 +161,7 @@ describe("ensurePermissionsThenSetRole", () => { }); setIamPolicyStub.resolves({} as any); - await ensurePermissionsThenSetRole( - "test-project", - "test@example.com", - mockPermissions, - mockRole, - ); + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); expect(setIamPolicyStub).to.have.been.calledOnce; expect(cacheStore["iamPermissionCache"]).to.deep.equal({ @@ -217,7 +192,7 @@ describe("ensurePermissionsThenSetRole", () => { setIamPolicyStub.rejects(new Error("Permission denied")); await expect( - ensurePermissionsThenSetRole("test-project", "test@example.com", mockPermissions, mockRole), + ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole), ).to.be.rejectedWith( FirebaseError, /Attempted to automatically bind the role roles\/mcp\.toolUser but failed/, @@ -241,7 +216,7 @@ describe("ensurePermissionsThenSetRole", () => { ], }); - await ensurePermissionsThenSetRole( + await ensurePermissionsOrSetRole( "test-project", "sa@proj.iam.gserviceaccount.com", mockPermissions, @@ -267,7 +242,7 @@ describe("ensurePermissionsThenSetRole", () => { ], }); - await ensurePermissionsThenSetRole( + await ensurePermissionsOrSetRole( "test-project", "sa@proj.iam.gserviceaccount.com.fake.com", mockPermissions, @@ -288,7 +263,7 @@ describe("ensurePermissionsThenSetRole", () => { debug: sandbox.stub(), }; - await ensurePermissionsThenSetRole( + await ensurePermissionsOrSetRole( "test-project", "test@example.com", mockPermissions, @@ -298,7 +273,7 @@ describe("ensurePermissionsThenSetRole", () => { ); expect(customLogger.debug).to.have.been.calledWith( - sinon.match(/ensurePermissionsThenSetRole called/), + sinon.match(/ensurePermissionsOrSetRole called/), ); expect(customLogger.debug).to.have.been.calledWith( sinon.match(/Caching positive permissions check/), diff --git a/src/ensureRoleBound.ts b/src/ensurePermissions.ts similarity index 95% rename from src/ensureRoleBound.ts rename to src/ensurePermissions.ts index 3f6d74b6cc3..c457b4d749e 100644 --- a/src/ensureRoleBound.ts +++ b/src/ensurePermissions.ts @@ -57,7 +57,7 @@ function cachePermissions(projectId: string, email: string, permissions: string[ * If not, it attempts to bind the specified IAM role to the user. * Uses local configstore cache to avoid RM API query latency, unless force is true. */ -export async function ensurePermissionsThenSetRole( +export async function ensurePermissionsOrSetRole( projectId: string, accountEmail: string, permissions: string[], @@ -67,14 +67,14 @@ export async function ensurePermissionsThenSetRole( ): Promise { const log = customLogger || logger; log.debug( - `[iam] ensurePermissionsThenSetRole called for project: ${projectId}, account: ${accountEmail}, permissions: ${JSON.stringify( + `[iam] ensurePermissionsOrSetRole called for project: ${projectId}, account: ${accountEmail}, permissions: ${JSON.stringify( permissions, )}, role: ${role}, force: ${String(force)}`, ); if (!force && checkPermissionCache(projectId, accountEmail, permissions)) { log.debug( - `[iam] ensurePermissionsThenSetRole early out: permissions ${JSON.stringify( + `[iam] ensurePermissionsOrSetRole early out: permissions ${JSON.stringify( permissions, )} are cached for project ${projectId} and account ${accountEmail}`, ); diff --git a/src/mcp/onemcp/onemcp_server.spec.ts b/src/mcp/onemcp/onemcp_server.spec.ts index 27d5c88b086..f898eb6506f 100644 --- a/src/mcp/onemcp/onemcp_server.spec.ts +++ b/src/mcp/onemcp/onemcp_server.spec.ts @@ -6,13 +6,13 @@ import * as ensureModule from "../../ensureApiEnabled"; import { FirebaseError } from "../../error"; import { ServerFeature } from "../types"; import { LATEST_PROTOCOL_VERSION } from "@modelcontextprotocol/sdk/types.js"; -import * as ensureRoleBound from "../../ensureRoleBound"; +import * as ensurePermissions from "../../ensurePermissions"; describe("OneMcpServer", () => { let sandbox: sinon.SinonSandbox; let clientRequestStub: sinon.SinonStub; let ensureStub: sinon.SinonStub; - let ensurePermissionsThenSetRoleStub: sinon.SinonStub; + let ensurePermissionsOrSetRoleStub: sinon.SinonStub; const feature = "auth" as ServerFeature; const serverUrl = "https://example.com"; @@ -22,8 +22,8 @@ describe("OneMcpServer", () => { sandbox = sinon.createSandbox(); clientRequestStub = sandbox.stub(Client.prototype, "request"); ensureStub = sandbox.stub(ensureModule, "ensure").resolves(); - ensurePermissionsThenSetRoleStub = sandbox - .stub(ensureRoleBound, "ensurePermissionsThenSetRole") + ensurePermissionsOrSetRoleStub = sandbox + .stub(ensurePermissions, "ensurePermissionsOrSetRole") .resolves(); server = new OneMcpServer(feature, serverUrl, { requiresAuth: false }); }); @@ -374,7 +374,7 @@ describe("OneMcpServer", () => { ); }); - it("should call ensurePermissionsThenSetRole when projectId and accountEmail are present in context", async () => { + it("should call ensurePermissionsOrSetRole when projectId and accountEmail are present in context", async () => { const mockMcpTool = { name: "test_tool", inputSchema: { type: "object", properties: {} } }; clientRequestStub.onFirstCall().resolves({ body: { result: { tools: [mockMcpTool] } }, @@ -394,7 +394,7 @@ describe("OneMcpServer", () => { await tool.fn({}, contextWithAuth as any); - expect(ensurePermissionsThenSetRoleStub).to.have.been.calledOnceWith( + expect(ensurePermissionsOrSetRoleStub).to.have.been.calledOnceWith( "test-project", "user@example.com", ["mcp.tools.call", "resourcemanager.projects.get", "resourcemanager.projects.list"], diff --git a/src/mcp/onemcp/onemcp_server.ts b/src/mcp/onemcp/onemcp_server.ts index f5abaf6f316..c9133fd4ff2 100644 --- a/src/mcp/onemcp/onemcp_server.ts +++ b/src/mcp/onemcp/onemcp_server.ts @@ -14,7 +14,7 @@ import { ServerTool, ServerToolMeta } from "../tool"; import { McpContext, ServerFeature } from "../types"; import { FirebaseError } from "../../error"; import { ensure } from "../../ensureApiEnabled"; -import { ensurePermissionsThenSetRole } from "../../ensureRoleBound"; +import { ensurePermissionsOrSetRole } from "../../ensurePermissions"; export interface OneMcpServerOptions { /** @@ -162,10 +162,10 @@ export class OneMcpServer { } if (ctx.projectId && ctx.accountEmail) { - await ensurePermissionsThenSetRole( + await ensurePermissionsOrSetRole( ctx.projectId, ctx.accountEmail, - ["mcp.tools.call", "resourcemanager.projects.get", "resourcemanager.projects.list"], + ["mcp.tools.call"], "roles/mcp.toolUser", false, ctx.host?.logger, From 436fc26aaa24a477f6cf9e4715b6448da1760b91 Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Wed, 12 Aug 2026 17:12:02 -0700 Subject: [PATCH 06/10] test: update onemcp_server spec permissions to match callTool implementation --- src/mcp/onemcp/onemcp_server.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mcp/onemcp/onemcp_server.spec.ts b/src/mcp/onemcp/onemcp_server.spec.ts index f898eb6506f..94e0273bde8 100644 --- a/src/mcp/onemcp/onemcp_server.spec.ts +++ b/src/mcp/onemcp/onemcp_server.spec.ts @@ -397,7 +397,7 @@ describe("OneMcpServer", () => { expect(ensurePermissionsOrSetRoleStub).to.have.been.calledOnceWith( "test-project", "user@example.com", - ["mcp.tools.call", "resourcemanager.projects.get", "resourcemanager.projects.list"], + ["mcp.tools.call"], "roles/mcp.toolUser", ); }); From 236754b4ec170c1e4d56077cc96d78d2c5affe18 Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Thu, 13 Aug 2026 17:19:19 -0700 Subject: [PATCH 07/10] refactor: simplify permission cache to timestamp only and clean up missing permissions on failure --- src/ensurePermissions.spec.ts | 92 +++++++++++++++++++++++------------ src/ensurePermissions.ts | 40 ++++++++------- 2 files changed, 84 insertions(+), 48 deletions(-) diff --git a/src/ensurePermissions.spec.ts b/src/ensurePermissions.spec.ts index 2bc059c938d..7990a556b60 100644 --- a/src/ensurePermissions.spec.ts +++ b/src/ensurePermissions.spec.ts @@ -54,9 +54,9 @@ describe("ensurePermissionsOrSetRole", () => { expect(cacheStore["iamPermissionCache"]).to.deep.equal({ "test-project": { "test@example.com": { - "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, }, }, }); @@ -66,9 +66,9 @@ describe("ensurePermissionsOrSetRole", () => { cacheStore["iamPermissionCache"] = { "test-project": { "test@example.com": { - "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, }, }, }; @@ -79,31 +79,15 @@ describe("ensurePermissionsOrSetRole", () => { expect(getIamPolicyStub).to.not.have.been.called; }); - it("should skip API call and succeed if legacy boolean permissions are cached", async () => { - cacheStore["iamPermissionCache"] = { - "test-project": { - "test@example.com": { - "mcp.tools.call": true, - "resourcemanager.projects.get": true, - "resourcemanager.projects.list": true, - }, - }, - }; - - await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); - - expect(testIamPermissionsStub).to.not.have.been.called; - }); - it("should query API if cached permissions have expired", async () => { const now = 1710000000000; const oneDayAgo = now - 24 * 60 * 60 * 1000 - 1000; // expired by 1 second cacheStore["iamPermissionCache"] = { "test-project": { "test@example.com": { - "mcp.tools.call": { valid: true, timestamp: oneDayAgo }, - "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, + "mcp.tools.call": oneDayAgo, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, }, }, }; @@ -122,9 +106,9 @@ describe("ensurePermissionsOrSetRole", () => { cacheStore["iamPermissionCache"] = { "test-project": { "test@example.com": { - "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, }, }, }; @@ -167,9 +151,9 @@ describe("ensurePermissionsOrSetRole", () => { expect(cacheStore["iamPermissionCache"]).to.deep.equal({ "test-project": { "test@example.com": { - "mcp.tools.call": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.get": { valid: true, timestamp: 1710000000000 }, - "resourcemanager.projects.list": { valid: true, timestamp: 1710000000000 }, + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, }, }, }); @@ -279,4 +263,50 @@ describe("ensurePermissionsOrSetRole", () => { sinon.match(/Caching positive permissions check/), ); }); + + it("should revoke/remove missing permissions from cache if check fails", async () => { + cacheStore["iamPermissionCache"] = { + "test-project": { + "test@example.com": { + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, + }, + }, + }; + testIamPermissionsStub.resolves({ + passed: false, + allowed: [], + missing: ["mcp.tools.call"], + }); + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/viewer", + members: ["user:test@example.com"], + }, + ], + }); + setIamPolicyStub.rejects(new Error("Permission denied")); + + await expect( + ensurePermissionsOrSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + true, // force to query API even though they are cached + ), + ).to.be.rejectedWith(FirebaseError); + + // mcp.tools.call should be removed from cache since it was reported missing + expect(cacheStore["iamPermissionCache"]["test-project"]["test@example.com"]["mcp.tools.call"]) + .to.be.undefined; + // other permissions that weren't reported missing should remain + expect( + cacheStore["iamPermissionCache"]["test-project"]["test@example.com"][ + "resourcemanager.projects.get" + ], + ).to.equal(1710000000000); + }); }); diff --git a/src/ensurePermissions.ts b/src/ensurePermissions.ts index c457b4d749e..ef64a03da53 100644 --- a/src/ensurePermissions.ts +++ b/src/ensurePermissions.ts @@ -10,25 +10,16 @@ const PERMISSION_CACHE_KEY = "iamPermissionCache"; const IAM_PROPAGATION_DELAY_MS = 10000; const CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours -interface CacheEntry { - valid: boolean; - timestamp: number; -} - -type PermissionCache = Record>>; +type PermissionCache = Record>>; function checkPermissionCache(projectId: string, email: string, permissions: string[]): boolean { const cache = configstore.get(PERMISSION_CACHE_KEY) as PermissionCache | undefined; for (const perm of permissions) { - const entry = cache?.[projectId]?.[email]?.[perm]; - if (!entry) { + const timestamp = cache?.[projectId]?.[email]?.[perm]; + if (!timestamp) { return false; } - if (typeof entry === "boolean") { - if (!entry) return false; - continue; - } - if (Date.now() - entry.timestamp >= CACHE_TTL_MS) { + if (Date.now() - timestamp >= CACHE_TTL_MS) { return false; } } @@ -44,14 +35,28 @@ function cachePermissions(projectId: string, email: string, permissions: string[ cache[projectId][email] = {}; } for (const perm of permissions) { - cache[projectId][email][perm] = { - valid: true, - timestamp: Date.now(), - }; + cache[projectId][email][perm] = Date.now(); } configstore.set(PERMISSION_CACHE_KEY, cache); } +function revokePermissions(projectId: string, email: string, permissions: string[]): void { + const cache = configstore.get(PERMISSION_CACHE_KEY) as PermissionCache | undefined; + if (!cache?.[projectId]?.[email]) { + return; + } + let updated = false; + for (const perm of permissions) { + if (cache[projectId][email][perm] !== undefined) { + delete cache[projectId][email][perm]; + updated = true; + } + } + if (updated) { + configstore.set(PERMISSION_CACHE_KEY, cache); + } +} + /** * Assures that the authenticating account holds the specified IAM permissions on the given project. * If not, it attempts to bind the specified IAM role to the user. @@ -105,6 +110,7 @@ export async function ensurePermissionsOrSetRole( iamResult.missing, )}. Attempting to bind role ${role}`, ); + revokePermissions(projectId, accountEmail, iamResult.missing); const policy = await getIamPolicy(projectId); const memberName = accountEmail.endsWith(".gserviceaccount.com") From 0afc330ee4b880d429cf7691be74bd0cc0c035eb Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Thu, 13 Aug 2026 17:25:44 -0700 Subject: [PATCH 08/10] refactor: cache allowed permissions even when check fails and role needs to be bound --- src/ensurePermissions.spec.ts | 31 +++++++++++++++++++++++++++++++ src/ensurePermissions.ts | 3 +++ 2 files changed, 34 insertions(+) diff --git a/src/ensurePermissions.spec.ts b/src/ensurePermissions.spec.ts index 7990a556b60..740743ddd24 100644 --- a/src/ensurePermissions.spec.ts +++ b/src/ensurePermissions.spec.ts @@ -309,4 +309,35 @@ describe("ensurePermissionsOrSetRole", () => { ], ).to.equal(1710000000000); }); + + it("should cache allowed permissions even if testIamPermissions fails and setIamPolicy fails", async () => { + testIamPermissionsStub.resolves({ + passed: false, + allowed: ["resourcemanager.projects.get"], + missing: ["mcp.tools.call", "resourcemanager.projects.list"], + }); + getIamPolicyStub.resolves({ + bindings: [ + { + role: "roles/viewer", + members: ["user:test@example.com"], + }, + ], + }); + setIamPolicyStub.rejects(new Error("Permission denied")); + + await expect( + ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole), + ).to.be.rejectedWith(FirebaseError); + + // allowed permission should be cached + expect( + cacheStore["iamPermissionCache"]["test-project"]["test@example.com"][ + "resourcemanager.projects.get" + ], + ).to.equal(1710000000000); + // missing permissions should not be cached + expect(cacheStore["iamPermissionCache"]["test-project"]["test@example.com"]["mcp.tools.call"]) + .to.be.undefined; + }); }); diff --git a/src/ensurePermissions.ts b/src/ensurePermissions.ts index ef64a03da53..990d41c94c0 100644 --- a/src/ensurePermissions.ts +++ b/src/ensurePermissions.ts @@ -111,6 +111,9 @@ export async function ensurePermissionsOrSetRole( )}. Attempting to bind role ${role}`, ); revokePermissions(projectId, accountEmail, iamResult.missing); + if (iamResult.allowed && iamResult.allowed.length > 0) { + cachePermissions(projectId, accountEmail, iamResult.allowed); + } const policy = await getIamPolicy(projectId); const memberName = accountEmail.endsWith(".gserviceaccount.com") From aae6a2bf39f7e31a8ee3beffd553fadf5d0b2fcd Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Thu, 13 Aug 2026 17:29:10 -0700 Subject: [PATCH 09/10] remove logBullet --- src/ensurePermissions.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ensurePermissions.ts b/src/ensurePermissions.ts index 990d41c94c0..0aec98b1f0b 100644 --- a/src/ensurePermissions.ts +++ b/src/ensurePermissions.ts @@ -4,7 +4,7 @@ import { getIamPolicy, setIamPolicy } from "./gcp/resourceManager"; import { configstore } from "./configstore"; import { mergeBindings, testIamPermissions } from "./gcp/iam"; import { logger } from "./logger"; -import { sleep, logBullet } from "./utils"; +import { sleep } from "./utils"; const PERMISSION_CACHE_KEY = "iamPermissionCache"; const IAM_PROPAGATION_DELAY_MS = 10000; @@ -142,9 +142,6 @@ export async function ensurePermissionsOrSetRole( log.debug( `[iam] Successfully updated IAM policy. Waiting ${IAM_PROPAGATION_DELAY_MS}ms for propagation...`, ); - logBullet( - `Successfully updated IAM policy. Waiting ${IAM_PROPAGATION_DELAY_MS / 1000}s for propagation...`, - ); await sleep(IAM_PROPAGATION_DELAY_MS); } catch (err: unknown) { const error = getError(err); From 4b88f7bb3f0b4fe4809005b1789a918f58060d93 Mon Sep 17 00:00:00 2001 From: Shawn Kuang Date: Mon, 17 Aug 2026 13:20:18 -0700 Subject: [PATCH 10/10] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 941179d3dc9..49ab1bf2e65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,2 +1,4 @@ - Added extensions replacement registry and scraper tool to track migrations for deprecated extensions ahead of the March 2027 decommission date. - [Added] Loads existing `.env` files and passes environment variables to functions discovery in `runtimeDelegate`. +- Fixed issues in the default TypeScript functions template. +- Esure the user has proper permission and roles to send request to OneMCP servers.