From 0cbbaf33d90471acdfb4d7d5288192f53cdd36fc Mon Sep 17 00:00:00 2001 From: Briliantov Vadim Date: Tue, 1 Sep 2026 19:31:21 +0200 Subject: [PATCH] fix(LLM-25012): OAuth2 Authentication for MCP Server Connections (#1059) --- package-lock.json | 42 ----- src/acp-agent.ts | 201 +++++++++++++++++++++++ src/tests/create-session-options.test.ts | 70 +++++++- src/tests/helpers.ts | 1 + 4 files changed, 271 insertions(+), 43 deletions(-) diff --git a/package-lock.json b/package-lock.json index 892b5064..61332e85 100644 --- a/package-lock.json +++ b/package-lock.json @@ -101,9 +101,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -117,9 +114,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -133,9 +127,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -149,9 +140,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -610,9 +598,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -630,9 +615,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -650,9 +632,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -670,9 +649,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -690,9 +666,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -710,9 +683,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2654,9 +2624,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2678,9 +2645,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2702,9 +2666,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2726,9 +2687,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/src/acp-agent.ts b/src/acp-agent.ts index cd2fff2f..806f4c5a 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -64,6 +64,7 @@ import { getSessionMessages, listSessions, McpServerConfig, + McpServerStatus, ModelInfo, ModelUsage, OnElicitation, @@ -281,6 +282,50 @@ const DEFAULT_CONTEXT_WINDOW = 200000; * pre-empt a slow-but-healthy interrupt. */ const DEFAULT_FORCE_CANCEL_GRACE_MS = 30_000; +/** Claude Code keeps the OAuth callback listener open in the background after + * `mcpAuthenticate` returns the authorization URL. The SDK does not expose + * that listener's completion promise, so watch the server status while the + * ACP client has the URL elicitation open. */ +const MCP_OAUTH_STATUS_POLL_MS = 1_000; +const MCP_OAUTH_TIMEOUT_MS = 10 * 60_000; + +/** Runtime MCP OAuth control exposed by the pinned Agent SDK. It is not yet in + * the public `Query` declaration, even though the method is present on the + * SDK query object and backed by Claude Code's `mcp_authenticate` control. */ +type McpOAuthQuery = Query & { + mcpAuthenticate( + serverName: string, + redirectUri?: string, + ): Promise<{ + authUrl?: string; + requiresUserAction: boolean; + callbackExpected: boolean; + redirectScheme?: "localhost" | "custom"; + callbackPort?: number; + state?: string; + }>; +}; + +function supportsMcpOAuth(query: Query): query is McpOAuthQuery { + return typeof (query as Partial).mcpAuthenticate === "function"; +} + +/** Wait for a polling interval, resolving false when the session is aborted. */ +function waitUnlessAborted(milliseconds: number, signal: AbortSignal): Promise { + if (signal.aborted) return Promise.resolve(false); + return new Promise((resolve) => { + const timer = setTimeout(() => { + signal.removeEventListener("abort", onAbort); + resolve(true); + }, milliseconds); + const onAbort = () => { + clearTimeout(timer); + resolve(false); + }; + signal.addEventListener("abort", onAbort, { once: true }); + }); +} + /** Error surfaced when the SDK declares a turn over (`session_state_changed: * idle`, its authoritative turn-over signal) without ever emitting the turn's * `result` — a model stream that dropped mid-turn, or an async agent that @@ -1511,6 +1556,159 @@ function raceWithAbort(operation: Promise, signal: AbortSignal): Promise; + client: AcpClient; + clientCapabilities?: ClientCapabilities; + logger: Logger; +}; + +/** Background OAuth work is implementation state, not part of the public + * `Session` shape exposed through `ClaudeAcpAgent.sessions`. */ +const mcpAuthentications = new WeakMap>(); + +/** Start OAuth for ACP-provided MCP servers that Claude reported as needing + * authentication. This runs after session creation has returned so an + * interactive browser flow never delays `session/new`. */ +function startMcpAuthentication( + host: McpAuthenticationHost, + sessionId: string, + mcpServers: NewSessionRequest["mcpServers"], +): void { + if (!host.clientCapabilities?.elicitation?.url || mcpServers.length === 0) return; + + const session = host.sessions[sessionId]; + if (!session || mcpAuthentications.has(session)) return; + + const requestedServers = new Set(mcpServers.map((server) => server.name)); + const authentication = authenticateMcpServers(host, sessionId, session.query, requestedServers) + .catch((error) => { + if (!session.abortController.signal.aborted) { + host.logger.error(`Failed to inspect MCP servers for OAuth: ${error}`); + } + }) + .finally(() => { + if (mcpAuthentications.get(session) === authentication) { + mcpAuthentications.delete(session); + } + }); + mcpAuthentications.set(session, authentication); +} + +async function authenticateMcpServers( + host: McpAuthenticationHost, + sessionId: string, + query: Query, + requestedServers: Set, +): Promise { + if (!supportsMcpOAuth(query)) { + host.logger.error("The Claude Agent SDK does not expose MCP OAuth authentication."); + return; + } + + const statuses = await query.mcpServerStatus(); + for (const status of statuses) { + if (status.status !== "needs-auth" || !requestedServers.has(status.name)) continue; + try { + await authenticateMcpServer(host, sessionId, query, status.name); + } catch (error) { + const session = host.sessions[sessionId]; + if (session && !session.abortController.signal.aborted) { + host.logger.error(`Failed to authenticate MCP server ${status.name}: ${error}`); + } + } + } +} + +/** Bridge Claude Code's startup MCP OAuth control to ACP URL elicitation. + * Claude opens and owns the localhost callback listener; the ACP client only + * needs to present the returned authorization URL. */ +async function authenticateMcpServer( + host: McpAuthenticationHost, + sessionId: string, + query: McpOAuthQuery, + serverName: string, +): Promise { + const session = host.sessions[sessionId]; + if (!session) return; + + const login = await query.mcpAuthenticate(serverName); + if (!login.requiresUserAction) return; + if (!login.authUrl) { + throw new Error("Claude Code requested user action without returning an authorization URL"); + } + + const elicitationId = `mcp-oauth-${randomUUID()}`; + const flowAbort = new AbortController(); + const abortFlow = () => flowAbort.abort(session.abortController.signal.reason); + session.abortController.signal.addEventListener("abort", abortFlow, { once: true }); + if (session.abortController.signal.aborted) abortFlow(); + + try { + const completed = waitForMcpAuthentication( + host.sessions, + sessionId, + query, + serverName, + flowAbort.signal, + ); + const elicitation = host.client.unstable_createElicitation( + { + mode: "url", + sessionId, + message: `Authenticate with MCP server ${serverName}`, + url: login.authUrl, + elicitationId, + }, + flowAbort.signal, + ); + const first = await Promise.race([ + completed.then((authenticated) => ({ type: "completed" as const, authenticated })), + elicitation.then((response) => ({ type: "elicitation" as const, response })), + ]); + + if (first.type === "elicitation" && !CreateElicitationResponse.isAccept(first.response)) { + return; + } + + if (first.type === "elicitation") { + await completed; + } + try { + await host.client.unstable_completeElicitation({ elicitationId }); + } catch (error) { + if (!flowAbort.signal.aborted) { + host.logger.error(`Failed to complete MCP OAuth elicitation: ${error}`); + } + } + } finally { + flowAbort.abort(); + session.abortController.signal.removeEventListener("abort", abortFlow); + } +} + +async function waitForMcpAuthentication( + sessions: Record, + sessionId: string, + query: Query, + serverName: string, + signal: AbortSignal, +): Promise { + const deadline = Date.now() + MCP_OAUTH_TIMEOUT_MS; + while (!signal.aborted && Date.now() < deadline) { + const session = sessions[sessionId]; + if (!session || session.query !== query) return false; + + const status: McpServerStatus | undefined = (await query.mcpServerStatus()).find( + (server) => server.name === serverName, + ); + if (!status || status.status === "failed" || status.status === "disabled") return false; + if (status.status === "connected") return true; + if (!(await waitUnlessAborted(MCP_OAUTH_STATUS_POLL_MS, signal))) return false; + } + return false; +} + export class ClaudeAcpAgent { sessions: { [key: string]: Session; @@ -1766,6 +1964,7 @@ export class ClaudeAcpAgent { // Needs to happen after we return the session setTimeout(() => { this.sendAvailableCommandsUpdate(response.sessionId); + startMcpAuthentication(this, response.sessionId, params.mcpServers); }, 0); return response; } @@ -1785,6 +1984,7 @@ export class ClaudeAcpAgent { // Needs to happen after we return the session setTimeout(() => { this.sendAvailableCommandsUpdate(params.sessionId); + startMcpAuthentication(this, params.sessionId, params.mcpServers ?? []); }, 0); return result; } @@ -1798,6 +1998,7 @@ export class ClaudeAcpAgent { // Send available commands after replay so it doesn't interleave with history setTimeout(() => { this.sendAvailableCommandsUpdate(params.sessionId); + startMcpAuthentication(this, params.sessionId, params.mcpServers ?? []); }, 0); return result; diff --git a/src/tests/create-session-options.test.ts b/src/tests/create-session-options.test.ts index 50ae4c8b..ee65599b 100644 --- a/src/tests/create-session-options.test.ts +++ b/src/tests/create-session-options.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { RequestError, SessionNotification } from "@agentclientprotocol/sdk"; +import { + CreateElicitationRequest, + RequestError, + SessionNotification, +} from "@agentclientprotocol/sdk"; import type { ClientCapabilities } from "@agentclientprotocol/sdk"; import type { Options } from "@anthropic-ai/claude-agent-sdk"; import type { AcpClient, ClaudeAcpAgent as ClaudeAcpAgentType } from "../acp-agent.js"; @@ -9,6 +13,12 @@ import * as path from "node:path"; let capturedOptions: Options | undefined; let contextUsageResult: (() => Promise<{ rawMaxTokens: number; model?: string }>) | undefined; +let mcpServerStatusResult: () => Promise>; +let mcpAuthenticateImpl: (serverName: string) => Promise<{ + authUrl?: string; + requiresUserAction: boolean; + callbackExpected: boolean; +}>; vi.mock("@anthropic-ai/claude-agent-sdk", async () => { const actual = await vi.importActual( "@anthropic-ai/claude-agent-sdk", @@ -31,6 +41,8 @@ vi.mock("@anthropic-ai/claude-agent-sdk", async () => { }), getContextUsage: () => contextUsageResult ? contextUsageResult() : Promise.resolve(DEFAULT_CONTEXT_USAGE), + mcpServerStatus: () => mcpServerStatusResult(), + mcpAuthenticate: (serverName: string) => mcpAuthenticateImpl(serverName), }); }, }; @@ -60,6 +72,11 @@ describe("createSession options merging", () => { beforeEach(async () => { capturedOptions = undefined; contextUsageResult = undefined; + mcpServerStatusResult = async () => []; + mcpAuthenticateImpl = async () => ({ + requiresUserAction: false, + callbackExpected: false, + }); vi.resetModules(); const acpAgent = await import("../acp-agent.js"); @@ -651,6 +668,57 @@ describe("createSession options merging", () => { expect(typeof capturedOptions!.onElicitation).toBe("function"); }); + it("starts MCP OAuth as an ACP URL elicitation and completes it after reconnect", async () => { + let statusCall = 0; + mcpServerStatusResult = async () => + statusCall++ === 0 + ? [{ name: "linear", status: "needs-auth" }] + : [{ name: "linear", status: "connected" }]; + const authenticate = vi.fn(async () => ({ + authUrl: "https://example.com/oauth/authorize", + requiresUserAction: true, + callbackExpected: true, + })); + mcpAuthenticateImpl = authenticate; + const createElicitation = vi.fn(async (_request: CreateElicitationRequest) => ({ + action: "accept" as const, + })); + const completeElicitation = vi.fn(async () => {}); + Object.assign((agent as unknown as { client: object }).client, { + unstable_createElicitation: createElicitation, + unstable_completeElicitation: completeElicitation, + }); + + await agent.initialize({ + protocolVersion: 1, + clientCapabilities: { elicitation: { url: {} } }, + }); + const { sessionId } = await agent.newSession({ + cwd: process.cwd(), + mcpServers: [ + { name: "linear", type: "http", url: "https://mcp.linear.app/mcp", headers: [] }, + ], + }); + + await vi.waitFor(() => expect(completeElicitation).toHaveBeenCalledOnce()); + + expect(authenticate).toHaveBeenCalledWith("linear"); + expect(createElicitation).toHaveBeenCalledOnce(); + const request = createElicitation.mock.calls[0]![0]; + expect(request.mode).toBe("url"); + if (request.mode !== "url") throw new Error("Expected a URL elicitation"); + expect(request).toMatchObject({ + mode: "url", + sessionId, + message: "Authenticate with MCP server linear", + url: "https://example.com/oauth/authorize", + elicitationId: expect.stringMatching(/^mcp-oauth-/), + }); + expect(completeElicitation).toHaveBeenCalledWith({ + elicitationId: request.elicitationId, + }); + }); + it("still merges user-provided disallowedTools when AskUserQuestion is enabled", async () => { await agent.initialize({ protocolVersion: 1, diff --git a/src/tests/helpers.ts b/src/tests/helpers.ts index 56510451..85eb8395 100644 --- a/src/tests/helpers.ts +++ b/src/tests/helpers.ts @@ -27,6 +27,7 @@ export function makeMockQuery(overrides: Record = {}) { setModel: async () => {}, setPermissionMode: async () => {}, supportedCommands: async () => [], + mcpServerStatus: async () => [], getContextUsage: async () => DEFAULT_CONTEXT_USAGE, close: () => {}, interrupt: async () => undefined,