diff --git a/clients/web/src/test/integration/mcp/node/transport.test.ts b/clients/web/src/test/integration/mcp/node/transport.test.ts index c813fdf0..95481a30 100644 --- a/clients/web/src/test/integration/mcp/node/transport.test.ts +++ b/clients/web/src/test/integration/mcp/node/transport.test.ts @@ -242,6 +242,85 @@ describe("Transport", () => { } }); + it("records an intercepted 401's status, WWW-Authenticate and body (#2297)", async () => { + const challengeBody = JSON.stringify({ + error: "invalid_token", + error_description: "token expired", + }); + const wwwAuthenticate = + 'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource", scope="files:read"'; + const fetchFn: typeof fetch = async () => + new Response(challengeBody, { + status: 401, + statusText: "Unauthorized", + headers: { + "content-type": "application/json", + "www-authenticate": wwwAuthenticate, + }, + }); + + const fetchRequests: FetchRequestEntryBase[] = []; + const bodies = new Map(); + const result = createTransportNode( + { type: "streamable-http", url: "https://mcp.example/mcp" }, + { + fetchFn, + interceptAuthChallenges: true, + onFetchRequest: (entry) => fetchRequests.push(entry), + onFetchResponseBody: (id, body) => bodies.set(id, body), + }, + ); + + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: {} }, + ); + await expect(client.connect(result.transport)).rejects.toThrow( + "MCP auth challenge (401)", + ); + + expect(fetchRequests).toHaveLength(1); + const [entry] = fetchRequests; + expect(entry.error).toBeUndefined(); + expect(entry.responseStatus).toBe(401); + expect(entry.responseHeaders?.["www-authenticate"]).toBe(wwwAuthenticate); + await vi.waitFor(() => expect(bodies.get(entry.id)).toBe(challengeBody)); + }); + + it("still throws an intercepted 401 whose body never ends (#2297)", async () => { + const fetchFn: typeof fetch = async () => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("partial")); + }, + }), + { + status: 401, + headers: { "www-authenticate": 'Bearer error="invalid_token"' }, + }, + ); + + const fetchRequests: FetchRequestEntryBase[] = []; + const result = createTransportNode( + { type: "streamable-http", url: "https://mcp.example/mcp" }, + { + fetchFn, + interceptAuthChallenges: true, + onFetchRequest: (entry) => fetchRequests.push(entry), + }, + ); + + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: {} }, + ); + await expect(client.connect(result.transport)).rejects.toThrow( + "MCP auth challenge (401)", + ); + expect(fetchRequests[0]?.responseStatus).toBe(401); + }); + it("applies settings.headers to the outgoing streamable-http request", async () => { const server = createTestServerHttp({ serverInfo: createTestServerInfo(), diff --git a/core/mcp/node/authChallengeFetch.ts b/core/mcp/node/authChallengeFetch.ts index 0ce757ba..6bfbf31f 100644 --- a/core/mcp/node/authChallengeFetch.ts +++ b/core/mcp/node/authChallengeFetch.ts @@ -24,8 +24,11 @@ export function createAuthChallengeInterceptFetch( } // Release the connection before throwing so the SDK transport is not left - // with a half-read 401/403 body on streamable HTTP. - await response.body?.cancel().catch(() => {}); + // with a half-read 401/403 body on streamable HTTP. Not awaited: when the + // network tracker sits below this wrapper (#2297) the body is one branch of + // a tee, and cancelling a branch settles only once the tracker's clone has + // read to the end — so a 401 whose body never ends would hang the throw. + void response.body?.cancel().catch(() => {}); throw new AuthChallengeError( challenge, diff --git a/core/mcp/node/transport.ts b/core/mcp/node/transport.ts index dd3c8358..8ed872f4 100644 --- a/core/mcp/node/transport.ts +++ b/core/mcp/node/transport.ts @@ -67,12 +67,27 @@ export function createTransportNode( onAuthChallengeObserved ? createAuthChallengeObserverFetch(inner, onAuthChallengeObserved) : inner; + const withTracking = (inner: typeof fetch): typeof fetch => + onFetchRequest + ? createFetchTracker(inner, { + trackRequest: onFetchRequest, + updateResponseBody: onFetchResponseBody, + }) + : inner; // The observer sits *under* the interceptor so it still reports the // challenge on the path where interception throws — and, more to the point, // on the legacy first-auth path where interception is off entirely. + // + // The network tracker sits under the interceptor too (#2297). Above it, the + // tracker never saw the 401/403 at all: the interceptor cancels the body and + // throws, so the entry was recorded from the thrown error alone — no status, + // no `WWW-Authenticate`, and no response body, which is exactly the detail + // someone debugging an OAuth challenge needs. Below it, the tracker records + // the real response and clones its body before the interceptor cancels it. + const trackedBase = withTracking(withChallengeObserver(baseFetch)); const fetchWithOptionalAuthIntercept = interceptAuthChallenges - ? createAuthChallengeInterceptFetch(withChallengeObserver(baseFetch)) - : withChallengeObserver(baseFetch); + ? createAuthChallengeInterceptFetch(trackedBase) + : trackedBase; if (serverType === "stdio") { const stdioConfig = config as StdioServerConfig; @@ -111,15 +126,9 @@ export function createTransportNode( const configuredSseFetch = sseConfig.eventSourceInit?.fetch as | typeof fetch | undefined; - const sseFetch = configuredSseFetch - ? withChallengeObserver(configuredSseFetch) + const trackedFetch = configuredSseFetch + ? withTracking(withChallengeObserver(configuredSseFetch)) : fetchWithOptionalAuthIntercept; - const trackedFetch = onFetchRequest - ? createFetchTracker(sseFetch, { - trackRequest: onFetchRequest, - updateResponseBody: onFetchResponseBody, - }) - : sseFetch; const headers = headersFromSettings(settings); @@ -134,18 +143,11 @@ export function createTransportNode( ...(headers && { headers }), }; - const postFetch = onFetchRequest - ? createFetchTracker(fetchWithOptionalAuthIntercept, { - trackRequest: onFetchRequest, - updateResponseBody: onFetchResponseBody, - }) - : fetchWithOptionalAuthIntercept; - const transport = new SSEClientTransport(url, { authProvider, eventSourceInit, requestInit, - fetch: postFetch, + fetch: fetchWithOptionalAuthIntercept, }); return { transport }; @@ -161,17 +163,10 @@ export function createTransportNode( ...(headers && { headers }), }; - const transportFetch = onFetchRequest - ? createFetchTracker(fetchWithOptionalAuthIntercept, { - trackRequest: onFetchRequest, - updateResponseBody: onFetchResponseBody, - }) - : fetchWithOptionalAuthIntercept; - const transport = new StreamableHTTPClientTransport(url, { authProvider, requestInit, - fetch: transportFetch, + fetch: fetchWithOptionalAuthIntercept, // SEP-2350: how the transport reacts to a `403 insufficient_scope` // challenge. Defaults to the SDK's `reauthorize` when unset. ...(settings?.oauthOnInsufficientScope && {