diff --git a/.github/workflows/code-qa.yml b/.github/workflows/code-qa.yml index 7c71478266..76754eabee 100644 --- a/.github/workflows/code-qa.yml +++ b/.github/workflows/code-qa.yml @@ -92,6 +92,8 @@ jobs: run: pnpm check-types - name: Model-check concurrent task lifecycle run: pnpm lifecycle:model-check + - name: Validate MCP OAuth integration + run: pnpm mcp:integration-check build-vsix: name: Build test VSIX diff --git a/package.json b/package.json index 1a44a12680..87f110a8b3 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs", "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && pnpm cleanup-protocol:model-check", "cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts", + "mcp:integration-check": "tsx scripts/check-mcp-oauth-integration.ts", "test:coverage": "turbo test:coverage --log-order grouped --output-logs new-only", "format": "turbo format --log-order grouped --output-logs new-only", "build": "turbo build --log-order grouped --output-logs new-only", diff --git a/scripts/check-mcp-oauth-integration.ts b/scripts/check-mcp-oauth-integration.ts new file mode 100644 index 0000000000..bfceec2c1a --- /dev/null +++ b/scripts/check-mcp-oauth-integration.ts @@ -0,0 +1,80 @@ +import assert from "node:assert/strict" + +import { + AUTHORIZATION_CODE_GRANT_TYPE, + buildMcpOAuthClientMetadata, + MCP_OAUTH_GRANT_TYPES, + REFRESH_TOKEN_GRANT_TYPE, + selectMcpOAuthGrantTypes, +} from "../src/services/mcp/oauthMetadata" + +const advertisedGrantTypes = [ + AUTHORIZATION_CODE_GRANT_TYPE, + REFRESH_TOKEN_GRANT_TYPE, + "urn:ietf:params:oauth:grant-type:jwt-bearer", + "urn:example:grant-type:extension", +] as const + +let checkedCases = 0 + +// Repository policy: Zoo Code implements only these two token-endpoint grants. +// Keeping this assertion literal prevents an allowlist expansion from silently +// broadening dynamic registration. +assert.deepEqual(MCP_OAUTH_GRANT_TYPES, ["authorization_code", "refresh_token"]) + +for (let mask = 0; mask < 1 << advertisedGrantTypes.length; mask++) { + const advertised = advertisedGrantTypes.filter((_, index) => mask & (1 << index)) + const selected = selectMcpOAuthGrantTypes(advertised) + const expected = MCP_OAUTH_GRANT_TYPES.filter((grantType) => advertised.includes(grantType)) + + // Normative MUST: RFC 7591 section 2 says grant_types describes grants the + // client can use, and each token-endpoint grant_type must match its registered + // value. https://www.rfc-editor.org/rfc/rfc7591.html#section-2 + // Repository policy: intersect server metadata with Zoo Code's implemented + // grants, canonicalize order, and never propagate unknown extension values. + assert.deepEqual(selected, expected, `unexpected grant selection for ${JSON.stringify(advertised)}`) + assert.equal(new Set(selected).size, selected.length, "registration grant types must be unique") + + const buildMetadata = () => + buildMcpOAuthClientMetadata({ + clientName: "Zoo Code", + redirectUrl: "http://localhost:12345/callback", + grantTypes: selected, + tokenEndpointAuthMethod: "none", + }) + + if (!selected.includes(AUTHORIZATION_CODE_GRANT_TYPE)) { + assert.throws(buildMetadata, /requires authorization_code support/) + checkedCases++ + continue + } + + const metadata = buildMetadata() + + assert.deepEqual(metadata.grant_types, selected) + // Normative SHOULD: RFC 7591 section 2.1 recommends consistent + // authorization_code/code metadata. + // https://www.rfc-editor.org/rfc/rfc7591.html#section-2.1 + assert.deepEqual(metadata.response_types, ["code"]) + // Normative MUST/SHOULD: MCP 2026-07-28 requires DCR clients to declare + // application_type; desktop clients using localhost should identify as native. + // https://modelcontextprotocol.io/specification/2026-07-28/basic/authorization/client-registration#application-type-and-redirect-uri-constraints + assert.equal(metadata.application_type, "native") + assert.match(metadata.redirect_uris[0], /^http:\/\/localhost:/) + + checkedCases++ +} + +// Repository policy: retain both implemented grants when RFC 8414's optional +// grant_types_supported metadata is omitted. +// https://www.rfc-editor.org/rfc/rfc8414.html#section-2 +// Normative SHOULD: MCP clients that use refresh tokens should include +// refresh_token in their grant_types client metadata. +// https://modelcontextprotocol.io/specification/2026-07-28/basic/authorization#refresh-tokens +assert.deepEqual(selectMcpOAuthGrantTypes(), [...MCP_OAUTH_GRANT_TYPES]) +assert.deepEqual( + selectMcpOAuthGrantTypes([REFRESH_TOKEN_GRANT_TYPE, AUTHORIZATION_CODE_GRANT_TYPE, REFRESH_TOKEN_GRANT_TYPE]), + [...MCP_OAUTH_GRANT_TYPES], +) + +console.log(`MCP OAuth integration check passed (${checkedCases} advertised-grant combinations)`) diff --git a/src/services/mcp/McpOAuthClientProvider.ts b/src/services/mcp/McpOAuthClientProvider.ts index 9aa8bad8b1..dba08ef60d 100644 --- a/src/services/mcp/McpOAuthClientProvider.ts +++ b/src/services/mcp/McpOAuthClientProvider.ts @@ -10,6 +10,13 @@ import type { } from "@modelcontextprotocol/sdk/shared/auth.js" import { TOKEN_EXPIRY_BUFFER_MS } from "./constants" +import { + AUTHORIZATION_CODE_GRANT_TYPE, + buildMcpOAuthClientMetadata, + REFRESH_TOKEN_GRANT_TYPE, + selectMcpOAuthGrantTypes, + type McpOAuthGrantType, +} from "./oauthMetadata" import { SecretStorageService } from "./SecretStorageService" import { startCallbackServer, stopCallbackServer } from "./utils/callbackServer" import { fetchOAuthAuthServerMetadata } from "./utils/oauth" @@ -80,7 +87,7 @@ export class McpOAuthClientProvider implements OAuthClientProvider { private _authCodePromise: Promise | null, private _cancelCallbackServer: (() => void) | null, private readonly _tokenEndpointAuthMethod: string, - private readonly _grantTypes: string[], + private readonly _grantTypes: McpOAuthGrantType[], private readonly _scopes: string[], private readonly _state: string, private readonly _authServerMeta: Record | null, @@ -126,7 +133,7 @@ export class McpOAuthClientProvider implements OAuthClientProvider { // Only pick methods we actually implement: "none" or "client_secret_post". const authMethods: string[] = authServerMeta?.token_endpoint_auth_methods_supported ?? [] const tokenEndpointAuthMethod = authMethods.includes("none") ? "none" : "client_secret_post" - const grantTypes: string[] = authServerMeta?.grant_types_supported ?? ["authorization_code", "refresh_token"] + const grantTypes = selectMcpOAuthGrantTypes(authServerMeta?.grant_types_supported) const scopes: string[] = authServerMeta?.scopes_supported ?? [] // Generate a CSRF state token for the OAuth flow. @@ -196,13 +203,12 @@ export class McpOAuthClientProvider implements OAuthClientProvider { } get clientMetadata(): OAuthClientMetadata { - return { - client_name: this._clientName, - redirect_uris: [this.redirectUrl], - grant_types: this._grantTypes, - response_types: ["code"], - token_endpoint_auth_method: this._tokenEndpointAuthMethod, - } + return buildMcpOAuthClientMetadata({ + clientName: this._clientName, + redirectUrl: this.redirectUrl, + grantTypes: this._grantTypes, + tokenEndpointAuthMethod: this._tokenEndpointAuthMethod, + }) } async clientInformation(): Promise { @@ -438,7 +444,7 @@ export class McpOAuthClientProvider implements OAuthClientProvider { // Build the token request body per RFC 6749 §4.1.3 + RFC 7636 §4.5. const params: Record = { - grant_type: "authorization_code", + grant_type: AUTHORIZATION_CODE_GRANT_TYPE, code: authorizationCode, redirect_uri: this.redirectUrl, client_id: this._clientInfo.client_id, @@ -493,7 +499,7 @@ export class McpOAuthClientProvider implements OAuthClientProvider { } const params: Record = { - grant_type: "refresh_token", + grant_type: REFRESH_TOKEN_GRANT_TYPE, refresh_token: refreshToken, client_id: clientId, } diff --git a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts index 19ce121ebe..89ecee198a 100644 --- a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts +++ b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts @@ -193,6 +193,7 @@ describe("McpOAuthClientProvider", () => { expect(metadata.grant_types).toContain("authorization_code") expect(metadata.response_types).toContain("code") expect(metadata.token_endpoint_auth_method).toBe("none") + expect(metadata).toMatchObject({ application_type: "native" }) await provider.close() }) @@ -207,6 +208,61 @@ describe("McpOAuthClientProvider", () => { expect(provider.clientMetadata.client_name).toBe("figma") await provider.close() }) + + it("should exclude jwt-bearer from advertised grant types", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + issuer: "https://auth.example.com", + token_endpoint_auth_methods_supported: ["none"], + grant_types_supported: [ + "authorization_code", + "refresh_token", + "urn:ietf:params:oauth:grant-type:jwt-bearer", + ], + }), + }) + + const provider = await McpOAuthClientProvider.create("https://example.com/mcp", createMockSecretStorage()) + + expect(provider.clientMetadata.grant_types).toEqual(["authorization_code", "refresh_token"]) + await provider.close() + }) + + it("should exclude unknown advertised grant types", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + issuer: "https://auth.example.com", + token_endpoint_auth_methods_supported: ["none"], + grant_types_supported: ["authorization_code", "urn:example:grant-type:foo"], + }), + }) + + const provider = await McpOAuthClientProvider.create("https://example.com/mcp", createMockSecretStorage()) + + expect(provider.clientMetadata.grant_types).toEqual(["authorization_code"]) + await provider.close() + }) + + it("should reject registration metadata when authorization code is unsupported", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + issuer: "https://auth.example.com", + token_endpoint_auth_methods_supported: ["none"], + grant_types_supported: ["refresh_token"], + }), + }) + + const provider = await McpOAuthClientProvider.create("https://example.com/mcp", createMockSecretStorage()) + + expect(() => provider.clientMetadata).toThrow("authorization_code") + await provider.close() + }) }) describe("clientInformation / saveClientInformation", () => { @@ -806,6 +862,47 @@ describe("McpOAuthClientProvider", () => { await provider.close() }) + it("should register when the endpoint rejects unsupported grant types", async () => { + setupCallbackServerMock() + const secretStorage = createMockSecretStorage() + + mockFetch.mockClear() + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + registration_endpoint: "https://auth.example.com/register", + token_endpoint_auth_methods_supported: ["none"], + grant_types_supported: [ + "authorization_code", + "refresh_token", + "urn:ietf:params:oauth:grant-type:jwt-bearer", + ], + }), + }) + mockFetch.mockImplementationOnce((_url, init) => { + const body = JSON.parse(init?.body as string) + const hasUnsupportedGrant = body.grant_types.some( + (grantType: string) => !["authorization_code", "refresh_token"].includes(grantType), + ) + + return Promise.resolve({ + ok: !hasUnsupportedGrant, + status: hasUnsupportedGrant ? 400 : 200, + json: () => Promise.resolve({ client_id: "registered-client-id" }), + }) + }) + + const provider = await McpOAuthClientProvider.create("https://example.com/mcp", secretStorage) + + await expect(provider.registerClientIfNeeded()).resolves.toBeUndefined() + expect((await provider.clientInformation())?.client_id).toBe("registered-client-id") + await provider.close() + }) + it("should use the same redirect URI in DCR and authorization flow", async () => { setupCallbackServerMock() const secretStorage = createMockSecretStorage() diff --git a/src/services/mcp/oauthMetadata.ts b/src/services/mcp/oauthMetadata.ts new file mode 100644 index 0000000000..b0e519ae08 --- /dev/null +++ b/src/services/mcp/oauthMetadata.ts @@ -0,0 +1,36 @@ +import type { OAuthClientMetadata } from "@modelcontextprotocol/sdk/shared/auth.js" + +export const MCP_OAUTH_GRANT_TYPES = ["authorization_code", "refresh_token"] as const +export type McpOAuthGrantType = (typeof MCP_OAUTH_GRANT_TYPES)[number] + +export const AUTHORIZATION_CODE_GRANT_TYPE = MCP_OAUTH_GRANT_TYPES[0] +export const REFRESH_TOKEN_GRANT_TYPE = MCP_OAUTH_GRANT_TYPES[1] + +export interface McpOAuthClientMetadata extends OAuthClientMetadata { + application_type: "native" +} + +export function selectMcpOAuthGrantTypes(supportedGrantTypes?: readonly string[]): McpOAuthGrantType[] { + const supported = new Set(supportedGrantTypes ?? MCP_OAUTH_GRANT_TYPES) + return MCP_OAUTH_GRANT_TYPES.filter((grantType) => supported.has(grantType)) +} + +export function buildMcpOAuthClientMetadata(options: { + clientName: string + redirectUrl: string + grantTypes: readonly McpOAuthGrantType[] + tokenEndpointAuthMethod: string +}): McpOAuthClientMetadata { + if (!options.grantTypes.includes(AUTHORIZATION_CODE_GRANT_TYPE)) { + throw new Error("MCP OAuth registration requires authorization_code support") + } + + return { + application_type: "native", + client_name: options.clientName, + redirect_uris: [options.redirectUrl], + grant_types: [...options.grantTypes], + response_types: ["code"], + token_endpoint_auth_method: options.tokenEndpointAuthMethod, + } +}