diff --git a/client/src/lib/connectionAuthErrors.ts b/client/src/lib/connectionAuthErrors.ts new file mode 100644 index 00000000..b061d37e --- /dev/null +++ b/client/src/lib/connectionAuthErrors.ts @@ -0,0 +1,41 @@ +import { SseError } from "@modelcontextprotocol/sdk/client/sse.js"; +import { StreamableHTTPError } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js"; +import { McpError } from "@modelcontextprotocol/sdk/types.js"; +import { MCP_PROXY_TRANSPORT_ERROR_CODE } from "./constants"; + +function isPlainObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** `McpError.data` from server `mcpProxy` `serializeProxyTransportError`. */ +export function mcpProxyTransportErrorDataIndicatesUnauthorized( + data: Record, +): boolean { + if ("upstream401" in data) { + const snapshot = data.upstream401; + if (snapshot != null) return true; + } + const status = data.httpStatus; + return typeof status === "number" && status === 401; +} + +/** + * Whether `handleAuthError` / OAuth recovery should run for this failure. + */ +export function isConnectionAuthError(error: unknown): boolean { + if (error instanceof SseError && error.code === 401) return true; + if (error instanceof StreamableHTTPError && error.code === 401) return true; + if (error instanceof UnauthorizedError) return true; + + if ( + error instanceof McpError && + error.code === MCP_PROXY_TRANSPORT_ERROR_CODE && + isPlainObject(error.data) && + mcpProxyTransportErrorDataIndicatesUnauthorized(error.data) + ) { + return true; + } + + return false; +} diff --git a/client/src/lib/constants.ts b/client/src/lib/constants.ts index 6cb1a02c..20979d93 100644 --- a/client/src/lib/constants.ts +++ b/client/src/lib/constants.ts @@ -37,6 +37,14 @@ export type ConnectionStatus = export const DEFAULT_MCP_PROXY_LISTEN_PORT = "6277"; +/** + * JSON-RPC error code when the inspector proxy cannot complete a forward to the upstream MCP server. + * **-32099** — high end of JSON-RPC’s -32000..-32099 server-error band to avoid MCP-assigned codes + * (see comment on `server/src/mcpProxy.ts` `MCP_PROXY_TRANSPORT_ERROR_CODE`). Duplicated like + * `DEFAULT_MCP_PROXY_LISTEN_PORT`. + */ +export const MCP_PROXY_TRANSPORT_ERROR_CODE = -32099; + /** * Default configuration for the MCP Inspector, Currently persisted in local_storage in the Browser. * Future plans: Provide json config file + Browser local_storage to override default values diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index 1e5291a0..1d4f4bd0 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -5,12 +5,17 @@ import { ClientRequest, CreateTaskResultSchema, JSONRPCMessage, + McpError, } from "@modelcontextprotocol/sdk/types.js"; import type { AnySchema, SchemaOutput, } from "@modelcontextprotocol/sdk/server/zod-compat.js"; -import { DEFAULT_INSPECTOR_CONFIG, CLIENT_IDENTITY } from "../../constants"; +import { + DEFAULT_INSPECTOR_CONFIG, + CLIENT_IDENTITY, + MCP_PROXY_TRANSPORT_ERROR_CODE, +} from "../../constants"; import { SSEClientTransportOptions, SseError, @@ -94,17 +99,38 @@ jest.mock("@modelcontextprotocol/sdk/client/sse.js", () => { }; }); -jest.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ - StreamableHTTPClientTransport: jest.fn((url, options) => { - mockStreamableHTTPTransport.url = url; - mockStreamableHTTPTransport.options = options; - return mockStreamableHTTPTransport; - }), -})); +jest.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => { + class StreamableHTTPError extends Error { + code: number; + constructor(code: number, message: string) { + super(`Streamable HTTP error: ${message}`); + this.code = code; + } + } -jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ - auth: jest.fn().mockResolvedValue("AUTHORIZED"), -})); + return { + StreamableHTTPError, + StreamableHTTPClientTransport: jest.fn((url, options) => { + mockStreamableHTTPTransport.url = url; + mockStreamableHTTPTransport.options = options; + return mockStreamableHTTPTransport; + }), + }; +}); + +jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => { + class UnauthorizedError extends Error { + constructor(message?: string) { + super(message ?? "Unauthorized"); + this.name = "UnauthorizedError"; + } + } + + return { + UnauthorizedError, + auth: jest.fn().mockResolvedValue("AUTHORIZED"), + }; +}); // Mock the toast hook const mockToast = jest.fn(); @@ -1576,6 +1602,71 @@ describe("useConnection", () => { }); }); + describe("Inspector proxy McpError auth recovery", () => { + beforeEach(() => { + jest.clearAllMocks(); + mockAuth.mockResolvedValue("AUTHORIZED"); + mockDiscoverScopes.mockResolvedValue(undefined); + mockClient.connect.mockResolvedValue(undefined); + }); + + const attemptConnect = async ( + props: Parameters[0] = defaultProps, + ) => { + const { result } = renderHook(() => useConnection(props)); + await act(async () => { + try { + await result.current.connect(); + } catch { + // connect may throw when auth recovery does not retry + } + }); + }; + + it("invokes auth when connect fails with inspector proxy transport McpError and upstream401 data", async () => { + mockClient.connect.mockRejectedValueOnce( + new McpError(MCP_PROXY_TRANSPORT_ERROR_CODE, "proxy transport", { + upstream401: { body: "{}", contentType: "application/json" }, + }), + ); + await attemptConnect(); + expect(mockAuth).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ + serverUrl: defaultProps.sseUrl, + }), + ); + }); + + it("invokes auth when connect fails with inspector proxy transport McpError and httpStatus 401", async () => { + mockClient.connect.mockRejectedValueOnce( + new McpError(MCP_PROXY_TRANSPORT_ERROR_CODE, "proxy transport", { + httpStatus: 401, + }), + ); + await attemptConnect(); + expect(mockAuth).toHaveBeenCalled(); + }); + + it("does not invoke auth for inspector proxy McpError without auth payload", async () => { + mockClient.connect.mockRejectedValueOnce( + new McpError(MCP_PROXY_TRANSPORT_ERROR_CODE, "proxy transport", { + message: "upstream failure", + }), + ); + await attemptConnect(); + expect(mockAuth).not.toHaveBeenCalled(); + }); + + it("does not invoke auth when httpStatus is 401 but JSON-RPC code is not inspector proxy", async () => { + mockClient.connect.mockRejectedValueOnce( + new McpError(-32603, "Internal error", { httpStatus: 401 }), + ); + await attemptConnect(); + expect(mockAuth).not.toHaveBeenCalled(); + }); + }); + describe("MCP_PROXY_FULL_ADDRESS Configuration", () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 3e8e9a19..80739e66 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -1,13 +1,11 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { SSEClientTransport, - SseError, SSEClientTransportOptions, } from "@modelcontextprotocol/sdk/client/sse.js"; import { StreamableHTTPClientTransport, StreamableHTTPClientTransportOptions, - StreamableHTTPError, } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import { ClientNotification, @@ -51,6 +49,7 @@ import { RequestOptions } from "@modelcontextprotocol/sdk/shared/protocol.js"; import { useEffect, useRef, useState } from "react"; import { useToast } from "@/lib/hooks/useToast"; import { ConnectionStatus, CLIENT_IDENTITY } from "../constants"; +import { isConnectionAuthError } from "../connectionAuthErrors"; import { Notification } from "../notificationTypes"; import { auth, @@ -380,16 +379,7 @@ export function useConnection({ } }; - const is401Error = (error: unknown): boolean => { - return ( - (error instanceof SseError && error.code === 401) || - (error instanceof StreamableHTTPError && error.code === 401) || - (error instanceof Error && error.message.includes("401")) || - (error instanceof Error && error.message.includes("Unauthorized")) || - (error instanceof Error && - error.message.includes("Missing Authorization header")) - ); - }; + const is401Error = isConnectionAuthError; const isProxyAuthError = (error: unknown): boolean => { return ( diff --git a/server/src/index.ts b/server/src/index.ts index 1ba615a1..d3214cd4 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -27,7 +27,7 @@ import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import express from "express"; import rateLimit from "express-rate-limit"; import { findActualExecutable } from "spawn-rx"; -import mcpProxy from "./mcpProxy.js"; +import mcpProxy, { type ProxyHeaderHolder } from "./mcpProxy.js"; import { randomUUID, randomBytes, timingSafeEqual } from "node:crypto"; import { fileURLToPath } from "url"; import { dirname, join } from "path"; @@ -72,6 +72,29 @@ const is401Error = (error: unknown): boolean => { return false; }; +/** + * Prefer forwarding the upstream MCP 401 (WWW-Authenticate + body) so the browser + * matches direct-mode OAuth behavior. Falls back to JSON-encoding `error` if unknown. + */ +const sendProxiedUnauthorized = ( + res: express.Response, + error: unknown, + headerHolder?: ProxyHeaderHolder, +) => { + const captured = headerHolder?.lastUpstream401; + if (captured && headerHolder) { + if (captured.wwwAuthenticate) { + res.setHeader("WWW-Authenticate", captured.wwwAuthenticate); + } + res.status(401); + res.setHeader("Content-Type", captured.contentType); + res.send(captured.body); + delete headerHolder.lastUpstream401; + return; + } + res.status(401).json(error); +}; + // Function to get HTTP headers. const getHttpHeaders = (req: express.Request): Record => { const headers: Record = {}; @@ -175,13 +198,16 @@ const updateHeadersInPlace = ( const app = express(); app.use(cors()); app.use((req, res, next) => { - res.header("Access-Control-Expose-Headers", "mcp-session-id"); + res.header( + "Access-Control-Expose-Headers", + "mcp-session-id, WWW-Authenticate", + ); next(); }); const webAppTransports: Map = new Map(); // Web app transports by web app sessionId const serverTransports: Map = new Map(); // Server Transports by web app sessionId -const sessionHeaderHolders: Map = new Map(); // For dynamic header updates +const sessionHeaderHolders: Map = new Map(); // For dynamic header updates // Use provided token from environment or generate a new one const sessionToken = @@ -303,7 +329,7 @@ const createWebReadableStream = (nodeStream: any): ReadableStream => { * `Content-Type` are preserved. For SSE requests, it also converts Node.js * streams to web-compatible streams. */ -const createCustomFetch = (headerHolder: { headers: HeadersInit }) => { +const createCustomFetch = (headerHolder: ProxyHeaderHolder) => { return async ( input: RequestInfo | URL, init?: RequestInit, @@ -334,6 +360,28 @@ const createCustomFetch = (headerHolder: { headers: HeadersInit }) => { { ...init, headers: headersObject } as any, ); + if (response.status === 401) { + const wwwAuthenticate = + response.headers.get("www-authenticate") ?? undefined; + const contentType = + response.headers.get("content-type") ?? "application/json"; + const body = await response.text(); + headerHolder.lastUpstream401 = { + wwwAuthenticate, + body, + contentType, + }; + const responseHeaders: Record = {}; + response.headers.forEach((value: string, key: string) => { + responseHeaders[key] = value; + }); + return new Response(body, { + status: 401, + statusText: response.statusText, + headers: responseHeaders, + }) as Response; + } + // Check if this is an SSE request by looking at the Accept header const acceptHeader = finalHeaders.get("Accept"); const isSSE = acceptHeader?.includes("text/event-stream"); @@ -366,7 +414,7 @@ const createTransport = async ( req: express.Request, ): Promise<{ transport: Transport; - headerHolder?: { headers: HeadersInit }; + headerHolder?: ProxyHeaderHolder; }> => { const query = req.query; console.log("Query parameters:", JSON.stringify(query)); @@ -397,7 +445,7 @@ const createTransport = async ( const headers = getHttpHeaders(req); headers["Accept"] = "text/event-stream"; - const headerHolder = { headers }; + const headerHolder: ProxyHeaderHolder = { headers }; console.log( `SSE transport: url=${url}, headers=${JSON.stringify(headers)}`, @@ -416,7 +464,7 @@ const createTransport = async ( } else if (transportType === "streamable-http") { const headers = getHttpHeaders(req); headers["Accept"] = "text/event-stream, application/json"; - const headerHolder = { headers }; + const headerHolder: ProxyHeaderHolder = { headers }; const transport = new StreamableHTTPClientTransport( new URL(query.url as string), @@ -501,9 +549,11 @@ app.post( } } else { console.log("New StreamableHttp connection request"); + let streamableHeaderHolder: ProxyHeaderHolder | undefined; try { const { transport: serverTransport, headerHolder } = await createTransport(req); + streamableHeaderHolder = headerHolder; const webAppTransport = new StreamableHTTPServerTransport({ sessionIdGenerator: randomUUID, @@ -528,6 +578,7 @@ app.post( mcpProxy({ transportToClient: webAppTransport, transportToServer: serverTransport, + headerHolder, }); await (webAppTransport as StreamableHTTPServerTransport).handleRequest( @@ -541,7 +592,7 @@ app.post( "Received 401 Unauthorized from MCP server:", error instanceof Error ? error.message : error, ); - res.status(401).json(error); + sendProxiedUnauthorized(res, error, streamableHeaderHolder); return; } console.error("Error in /mcp POST route:", error); @@ -681,7 +732,7 @@ app.get( console.error( "Received 401 Unauthorized from MCP server. Authentication failure.", ); - res.status(401).json(error); + sendProxiedUnauthorized(res, error, undefined); return; } console.error("Error in /stdio route:", error); @@ -695,12 +746,14 @@ app.get( originValidationMiddleware, authMiddleware, async (req, res) => { + let sseHeaderHolder: ProxyHeaderHolder | undefined; try { console.log( "New SSE connection request. NOTE: The SSE transport is deprecated and has been replaced by StreamableHttp", ); const { transport: serverTransport, headerHolder } = await createTransport(req); + sseHeaderHolder = headerHolder; const proxyFullAddress = (req.query.proxyFullAddress as string) || ""; const prefix = proxyFullAddress || ""; @@ -721,13 +774,14 @@ app.get( mcpProxy({ transportToClient: webAppTransport, transportToServer: serverTransport, + headerHolder, }); } catch (error) { if (is401Error(error)) { console.error( "Received 401 Unauthorized from MCP server. Authentication failure.", ); - res.status(401).json(error); + sendProxiedUnauthorized(res, error, sseHeaderHolder); return; } else if (error instanceof SseError && error.code === 404) { console.error( diff --git a/server/src/mcpProxy.ts b/server/src/mcpProxy.ts index 174eef0e..bf2d5c87 100644 --- a/server/src/mcpProxy.ts +++ b/server/src/mcpProxy.ts @@ -1,6 +1,30 @@ +import { SseError } from "@modelcontextprotocol/sdk/client/sse.js"; +import { StreamableHTTPError } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import { isJSONRPCRequest } from "@modelcontextprotocol/sdk/types.js"; +/** + * JSON-RPC error code for failed proxy → upstream transport sends. + * + * JSON-RPC 2.0 reserves **-32000 .. -32099** for implementation-defined *server* errors. + * MCP uses that band for protocol/SDK errors (see `@modelcontextprotocol/sdk` `ErrorCode`, e.g. + * `-32000` connection closed, `-32001` request timeout, `-32042` URL elicitation; other codes in + * the band are assigned over time — e.g. **-32002** is used for **ResourceNotFound** in parts of + * the MCP ecosystem). We use **-32099** so this inspector-only bridge error stays clear of those + * registered meanings; the client still keys off `error.data` shape, not the number alone. + */ +export const MCP_PROXY_TRANSPORT_ERROR_CODE = -32099; + +/** Session header bag used with `createCustomFetch`; may hold last upstream 401 snapshot. */ +export type ProxyHeaderHolder = { + headers: HeadersInit; + lastUpstream401?: { + wwwAuthenticate?: string; + body: string; + contentType: string; + }; +}; + function onClientError(error: Error) { console.error("Error from inspector client:", error); } @@ -15,12 +39,49 @@ function onServerError(error: Error) { } } +/** Exported for unit tests; used by the proxy when forwarding transport failures. */ +export function serializeProxyTransportError( + error: Error, + headerHolder?: ProxyHeaderHolder, +): Record { + const data: Record = { + message: error.message, + name: error.name, + }; + if (error.cause !== undefined) { + data.cause = + error.cause instanceof Error ? error.cause.message : String(error.cause); + } + const attachHttpStatusIfValid = (code: number) => { + if (Number.isInteger(code) && code >= 100 && code <= 599) { + data.httpStatus = code; + } + }; + if (error instanceof StreamableHTTPError) { + if (error.code !== undefined) attachHttpStatusIfValid(error.code); + } else if (error instanceof SseError) { + if (error.code !== undefined) attachHttpStatusIfValid(error.code); + } + const u = headerHolder?.lastUpstream401; + if (u && headerHolder) { + data.upstream401 = { + wwwAuthenticate: u.wwwAuthenticate, + body: u.body, + contentType: u.contentType, + }; + delete headerHolder.lastUpstream401; + } + return data; +} + export default function mcpProxy({ transportToClient, transportToServer, + headerHolder, }: { transportToClient: Transport; transportToServer: Transport; + headerHolder?: ProxyHeaderHolder; }) { let transportToClientClosed = false; let transportToServerClosed = false; @@ -28,18 +89,25 @@ export default function mcpProxy({ let reportedServerSession = false; transportToClient.onmessage = (message) => { - transportToServer.send(message).catch((error) => { + transportToServer.send(message).catch((err: unknown) => { + const error = err instanceof Error ? err : new Error(String(err)); // Send error response back to client if it was a request (has id) and connection is still open if (isJSONRPCRequest(message) && !transportToClientClosed) { + const causeStr = + error.cause !== undefined + ? error.cause instanceof Error + ? error.cause.message + : String(error.cause) + : null; const errorResponse = { jsonrpc: "2.0" as const, id: message.id, error: { - code: -32001, - message: error.cause - ? `${error.message} (cause: ${error.cause})` + code: MCP_PROXY_TRANSPORT_ERROR_CODE, + message: causeStr + ? `${error.message} (cause: ${causeStr})` : error.message, - data: error, + data: serializeProxyTransportError(error, headerHolder), }, }; transportToClient.send(errorResponse).catch(onClientError);