mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix(LLM-25012): OAuth2 Authentication for MCP Server Connections (#1059)
This commit is contained in:
42
package-lock.json
generated
42
package-lock.json
generated
@@ -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": [
|
||||
|
||||
201
src/acp-agent.ts
201
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<McpOAuthQuery>).mcpAuthenticate === "function";
|
||||
}
|
||||
|
||||
/** Wait for a polling interval, resolving false when the session is aborted. */
|
||||
function waitUnlessAborted(milliseconds: number, signal: AbortSignal): Promise<boolean> {
|
||||
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<T>(operation: Promise<T>, signal: AbortSignal): Promise<T
|
||||
});
|
||||
}
|
||||
|
||||
type McpAuthenticationHost = {
|
||||
sessions: Record<string, Session>;
|
||||
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<Session, Promise<void>>();
|
||||
|
||||
/** 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<string>,
|
||||
): Promise<void> {
|
||||
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<void> {
|
||||
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<string, Session>,
|
||||
sessionId: string,
|
||||
query: Query,
|
||||
serverName: string,
|
||||
signal: AbortSignal,
|
||||
): Promise<boolean> {
|
||||
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;
|
||||
|
||||
@@ -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<Array<{ name: string; status: string }>>;
|
||||
let mcpAuthenticateImpl: (serverName: string) => Promise<{
|
||||
authUrl?: string;
|
||||
requiresUserAction: boolean;
|
||||
callbackExpected: boolean;
|
||||
}>;
|
||||
vi.mock("@anthropic-ai/claude-agent-sdk", async () => {
|
||||
const actual = await vi.importActual<typeof import("@anthropic-ai/claude-agent-sdk")>(
|
||||
"@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,
|
||||
|
||||
@@ -27,6 +27,7 @@ export function makeMockQuery(overrides: Record<string, unknown> = {}) {
|
||||
setModel: async () => {},
|
||||
setPermissionMode: async () => {},
|
||||
supportedCommands: async () => [],
|
||||
mcpServerStatus: async () => [],
|
||||
getContextUsage: async () => DEFAULT_CONTEXT_USAGE,
|
||||
close: () => {},
|
||||
interrupt: async () => undefined,
|
||||
|
||||
Reference in New Issue
Block a user