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. diff --git a/src/ensurePermissions.spec.ts b/src/ensurePermissions.spec.ts new file mode 100644 index 00000000000..740743ddd24 --- /dev/null +++ b/src/ensurePermissions.spec.ts @@ -0,0 +1,343 @@ +import { expect } from "chai"; +import * as sinon from "sinon"; +import { FirebaseError } from "./error"; +import { configstore } from "./configstore"; +import * as resourceManager from "./gcp/resourceManager"; +import * as iam from "./gcp/iam"; +import { ensurePermissionsOrSetRole } from "./ensurePermissions"; +import * as utils from "./utils"; + +describe("ensurePermissionsOrSetRole", () => { + 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); + + 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 permissions are already held", async () => { + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], + }); + + 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; + expect(cacheStore["iamPermissionCache"]).to.deep.equal({ + "test-project": { + "test@example.com": { + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, + }, + }, + }); + }); + + it("should skip API call and succeed if permissions are cached and not expired", async () => { + cacheStore["iamPermissionCache"] = { + "test-project": { + "test@example.com": { + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, + }, + }, + }; + + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); + + expect(testIamPermissionsStub).to.not.have.been.called; + expect(getIamPolicyStub).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": oneDayAgo, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, + }, + }, + }; + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], + }); + + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); + + expect(testIamPermissionsStub).to.have.been.calledOnceWith("test-project", mockPermissions); + }); + + it("should query API even if cached when force is true", async () => { + cacheStore["iamPermissionCache"] = { + "test-project": { + "test@example.com": { + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, + }, + }, + }; + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], + }); + + await ensurePermissionsOrSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + true, + ); + + expect(testIamPermissionsStub).to.have.been.calledOnceWith("test-project", mockPermissions); + }); + + 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: [ + { + role: "roles/viewer", + members: ["user:test@example.com"], + }, + ], + }); + setIamPolicyStub.resolves({} as any); + + await ensurePermissionsOrSetRole("test-project", "test@example.com", mockPermissions, mockRole); + + expect(setIamPolicyStub).to.have.been.calledOnce; + expect(cacheStore["iamPermissionCache"]).to.deep.equal({ + "test-project": { + "test@example.com": { + "mcp.tools.call": 1710000000000, + "resourcemanager.projects.get": 1710000000000, + "resourcemanager.projects.list": 1710000000000, + }, + }, + }); + }); + + it("should throw FirebaseError with instructions if testIamPermissions fails and setIamPolicy fails", async () => { + testIamPermissionsStub.resolves({ + passed: false, + allowed: [], + missing: mockPermissions, + }); + 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, + /Attempted to automatically bind the role roles\/mcp\.toolUser but failed/, + ); + + expect(cacheStore["iamPermissionCache"]).to.be.undefined; + }); + + it("should resolve serviceAccount prefix correctly when checking policy bindings", async () => { + testIamPermissionsStub.resolves({ + passed: false, + allowed: [], + missing: mockPermissions, + }); + getIamPolicyStub.resolves({ + bindings: [ + { + role: mockRole, + members: ["serviceAccount:sa@proj.iam.gserviceaccount.com"], + }, + ], + }); + + await ensurePermissionsOrSetRole( + "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: mockRole, + members: ["user:sa@proj.iam.gserviceaccount.com.fake.com"], + }, + ], + }); + + await ensurePermissionsOrSetRole( + "test-project", + "sa@proj.iam.gserviceaccount.com.fake.com", + mockPermissions, + mockRole, + ); + + expect(getIamPolicyStub).to.have.been.calledOnce; + expect(setIamPolicyStub).to.not.have.been.called; + }); + + it("should use customLogger for debugging if provided", async () => { + testIamPermissionsStub.resolves({ + passed: true, + allowed: mockPermissions, + missing: [], + }); + const customLogger = { + debug: sandbox.stub(), + }; + + await ensurePermissionsOrSetRole( + "test-project", + "test@example.com", + mockPermissions, + mockRole, + false, + customLogger, + ); + + expect(customLogger.debug).to.have.been.calledWith( + sinon.match(/ensurePermissionsOrSetRole called/), + ); + expect(customLogger.debug).to.have.been.calledWith( + 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); + }); + + 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 new file mode 100644 index 00000000000..0aec98b1f0b --- /dev/null +++ b/src/ensurePermissions.ts @@ -0,0 +1,172 @@ +import { bold } from "colorette"; +import { FirebaseError, getError } from "./error"; +import { getIamPolicy, setIamPolicy } from "./gcp/resourceManager"; +import { configstore } from "./configstore"; +import { mergeBindings, testIamPermissions } from "./gcp/iam"; +import { logger } from "./logger"; +import { sleep } from "./utils"; + +const PERMISSION_CACHE_KEY = "iamPermissionCache"; +const IAM_PROPAGATION_DELAY_MS = 10000; +const CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours + +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 timestamp = cache?.[projectId]?.[email]?.[perm]; + if (!timestamp) { + return false; + } + if (Date.now() - timestamp >= CACHE_TTL_MS) { + return false; + } + } + return true; +} + +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] = {}; + } + for (const perm of permissions) { + 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. + * Uses local configstore cache to avoid RM API query latency, unless force is true. + */ +export async function ensurePermissionsOrSetRole( + projectId: string, + accountEmail: string, + permissions: string[], + role: string, + force = false, + customLogger?: { debug: (message: string) => void }, +): Promise { + const log = customLogger || logger; + log.debug( + `[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] ensurePermissionsOrSetRole 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}`, + ); + 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] 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}`, + ); + 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") + ? `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: 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 permissions on project ${bold( + projectId, + )}. Attempted to automatically bind the role ${bold( + 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 }, + ); + } + } + + log.debug( + `[iam] Caching positive permissions check validation for project: ${projectId}, account: ${accountEmail}, permissions: ${JSON.stringify( + permissions, + )}`, + ); + cachePermissions(projectId, accountEmail, permissions); +} diff --git a/src/mcp/onemcp/onemcp_server.spec.ts b/src/mcp/onemcp/onemcp_server.spec.ts index e56dee1a4d5..94e0273bde8 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 ensurePermissions from "../../ensurePermissions"; describe("OneMcpServer", () => { let sandbox: sinon.SinonSandbox; let clientRequestStub: sinon.SinonStub; let ensureStub: sinon.SinonStub; + let ensurePermissionsOrSetRoleStub: sinon.SinonStub; const feature = "auth" as ServerFeature; const serverUrl = "https://example.com"; @@ -20,6 +22,9 @@ describe("OneMcpServer", () => { sandbox = sinon.createSandbox(); clientRequestStub = sandbox.stub(Client.prototype, "request"); ensureStub = sandbox.stub(ensureModule, "ensure").resolves(); + ensurePermissionsOrSetRoleStub = sandbox + .stub(ensurePermissions, "ensurePermissionsOrSetRole") + .resolves(); server = new OneMcpServer(feature, serverUrl, { requiresAuth: false }); }); @@ -368,5 +373,33 @@ describe("OneMcpServer", () => { /is not allowed on remote server/, ); }); + + 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] } }, + }); + + 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(ensurePermissionsOrSetRoleStub).to.have.been.calledOnceWith( + "test-project", + "user@example.com", + ["mcp.tools.call"], + "roles/mcp.toolUser", + ); + }); }); }); diff --git a/src/mcp/onemcp/onemcp_server.ts b/src/mcp/onemcp/onemcp_server.ts index d9de6718b1f..c9133fd4ff2 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 { ensurePermissionsOrSetRole } from "../../ensurePermissions"; export interface OneMcpServerOptions { /** @@ -160,6 +161,16 @@ export class OneMcpServer { } } + if (ctx.projectId && ctx.accountEmail) { + await ensurePermissionsOrSetRole( + ctx.projectId, + ctx.accountEmail, + ["mcp.tools.call"], + "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);