From 8ea64a6a8d982cde1c2a51f0fc867e95da7a54f1 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 19 Aug 2026 09:53:26 +0000 Subject: [PATCH] fix(mcp): run oauth discovery on reauth when handshake needs no auth `/mcp reauth ` probed the server with `{ oauth: false }` and treated a successful unauthenticated `initialize` as proof that OAuth was unnecessary, hard-erroring with "Server connection succeeded without OAuth; reauthorization is not required." Per the MCP spec a server may allow unauthenticated `initialize` while requiring a bearer token for `tools/call`, so this left no way to acquire a credential for such servers. When the handshake succeeds without an in-band tool challenge, fall back to `discoverOAuthEndpoints(config.url)` and proceed with the flow if the server advertises OAuth metadata; only refuse when no OAuth endpoint is discoverable. Fixes #8922 --- .../controllers/mcp-command-controller.ts | 17 +++++-- .../test/mcp-command-reauth.test.ts | 47 +++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts index b741c5b29..225d59edf 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -1198,11 +1198,20 @@ export class MCPCommandController { connectionError = error as Error; } - // Server connected fine without auth — reauth is not needed. A tool-level - // challenge overrides this: servers may allow the anonymous handshake yet - // protect individual tool calls with `_meta["mcp/www_authenticate"]`. + // Server connected fine without auth. A tool-level challenge overrides + // this: servers may allow the anonymous handshake yet protect individual + // tool calls with `_meta["mcp/www_authenticate"]`. Even without such a + // challenge, a clean `initialize` is only weak evidence — per the MCP + // spec a server MAY permit unauthenticated `initialize` while requiring a + // bearer token for `tools/call`. The user explicitly asked to reauth, so + // honor it when the server advertises OAuth discovery metadata; only + // refuse when there is genuinely no OAuth endpoint to acquire. if (connectionSucceeded && !authChallenge) { - throw new Error("Server connection succeeded without OAuth; reauthorization is not required."); + const discovered = "url" in config && config.url ? await discoverOAuthEndpoints(config.url) : null; + if (!discovered) { + throw new Error("Server connection succeeded without OAuth; reauthorization is not required."); + } + return discovered; } // Tool calls can carry richer RFC 6750/RFC 9728 hints than the original diff --git a/packages/coding-agent/test/mcp-command-reauth.test.ts b/packages/coding-agent/test/mcp-command-reauth.test.ts index 8f137291a..45c2166cc 100644 --- a/packages/coding-agent/test/mcp-command-reauth.test.ts +++ b/packages/coding-agent/test/mcp-command-reauth.test.ts @@ -353,6 +353,53 @@ describe("/mcp auth commands", () => { expect(showError).not.toHaveBeenCalled(); }); + test("/mcp reauth acquires a credential when the handshake succeeds but tools need OAuth", async () => { + const authStorage = freshAuthStorage(); + await authStorage.reload(); + // The anonymous handshake (`initialize`) succeeds; only `tools/call` is + // gated behind OAuth. The unauthenticated probe must not treat this as + // proof that reauthorization is unnecessary. + vi.spyOn(mcpClient, "connectToServer").mockResolvedValue({} as never); + vi.spyOn(mcpClient, "disconnectServer").mockResolvedValue(undefined as never); + + const fetchMock = Object.assign( + async (input: string | URL | Request): Promise => { + const url = String(input); + if (url === "https://mcp.example.com/.well-known/oauth-authorization-server") { + return new Response( + JSON.stringify({ + issuer: "https://mcp.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + client_id: "advertised-client", + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + return new Response("not found", { status: 404 }); + }, + { preconnect: globalThis.fetch.preconnect }, + ); + vi.spyOn(globalThis, "fetch").mockImplementation(fetchMock); + + vi.spyOn(oauthFlow.MCPOAuthFlow.prototype, "login").mockResolvedValue({ + access: "fresh-access", + refresh: "fresh-refresh", + expires: Date.now() + 3_600_000, + }); + + const { controller, showError } = createController(authStorage); + + await controller.handle("/mcp reauth envserver"); + + expect(showError).not.toHaveBeenCalled(); + expect(authStorage.get(oauthFlow.mcpOAuthCredentialId(EXPANDED_SERVER_URL))).toMatchObject({ + type: "oauth", + access: "fresh-access", + tokenUrl: "https://auth.example.com/token", + }); + }); + test("reuses embedded DCR client secret during reauth token exchange", async () => { const authStorage = freshAuthStorage(); await authStorage.reload();