From 83d33800bb9c0267f4ed41979f6d615b94478fdc Mon Sep 17 00:00:00 2001 From: tin-berri Date: Fri, 5 Jun 2026 10:51:46 -0700 Subject: [PATCH] fix(ui): route MCP playground auth by oauth2 mode instead of token_url (#29714) Interactive PKCE and OBO servers were mislabeled as M2M, so passthrough never showed the Authorize gate; classify by oauth2_flow + delegate_auth_to_upstream instead. --- .../components/mcp_tools/mcp_server_view.tsx | 2 + .../components/mcp_tools/mcp_tools.test.tsx | 91 +++++ .../src/components/mcp_tools/mcp_tools.tsx | 334 +++++++++--------- .../src/components/mcp_tools/types.test.tsx | 71 +++- .../src/components/mcp_tools/types.tsx | 34 +- 5 files changed, 357 insertions(+), 175 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_view.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_view.tsx index 5a8035d4e0b..416f0080d0f 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_view.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_view.tsx @@ -174,6 +174,8 @@ export const MCPServerView: React.FC = ({ serverId={mcpServer.server_id} accessToken={accessToken} auth_type={mcpServer.auth_type} + oauth2_flow={mcpServer.oauth2_flow} + delegate_auth_to_upstream={mcpServer.delegate_auth_to_upstream} tokenUrl={mcpServer.token_url} userRole={userRole} userID={userID} diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx new file mode 100644 index 00000000000..4917f1fcf1d --- /dev/null +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx @@ -0,0 +1,91 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { describe, expect, it, vi, beforeEach } from "vitest"; +import MCPToolsViewer from "./mcp_tools"; +import { listMCPTools } from "../networking"; +import { isTokenValid, getToken } from "@/utils/mcpTokenStore"; + +vi.mock("../networking", () => ({ + listMCPTools: vi.fn(), + callMCPTool: vi.fn(), +})); + +vi.mock("@/utils/mcpTokenStore", () => ({ + isTokenValid: vi.fn(), + getToken: vi.fn(), + removeToken: vi.fn(), +})); + +vi.mock("@/hooks/useToolsOAuthFlow", () => ({ + useToolsOAuthFlow: () => ({ startOAuthFlow: vi.fn(), status: "idle", error: null }), +})); + +const GATE_TEXT = "Authentication required"; +// Realistic interactive servers carry a token endpoint; the old heuristic +// (`oauth2 && !tokenUrl`) mislabeled exactly these as M2M. Setting it here is +// what makes the passthrough cases fail on the pre-fix code. +const TOKEN_URL = "https://slack.com/api/oauth.v2.user.access"; + +const renderViewer = (props: Record) => + render( + + + , + ); + +describe("MCPToolsViewer auth gate routing", () => { + beforeEach(() => { + vi.mocked(listMCPTools).mockReset().mockResolvedValue({ tools: [], error: null }); + vi.mocked(isTokenValid).mockReset().mockReturnValue(false); + vi.mocked(getToken) + .mockReset() + .mockReturnValue(undefined as any); + }); + + it("shows the Authorize gate for a passthrough server with a token endpoint and does not list tools", async () => { + renderViewer({ oauth2_flow: null, delegate_auth_to_upstream: true }); + + expect(await screen.findByText(GATE_TEXT)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Authorize" })).toBeInTheDocument(); + expect(vi.mocked(listMCPTools)).not.toHaveBeenCalled(); + }); + + it("forwards the session token via the x-mcp header for a passthrough server that has one", async () => { + vi.mocked(isTokenValid).mockReturnValue(true); + vi.mocked(getToken).mockReturnValue({ access_token: "slack-tok" } as any); + + renderViewer({ oauth2_flow: null, delegate_auth_to_upstream: true }); + + await waitFor(() => + expect(vi.mocked(listMCPTools)).toHaveBeenCalledWith( + "litellm-key", + "srv-1", + expect.objectContaining({ "x-mcp-slack-authorization": "Bearer slack-tok" }), + ), + ); + expect(screen.queryByText(GATE_TEXT)).not.toBeInTheDocument(); + }); + + it("does not gate an OBO server with a token endpoint; lists with the LiteLLM key and no x-mcp header", async () => { + renderViewer({ oauth2_flow: null, delegate_auth_to_upstream: false }); + + await waitFor(() => expect(vi.mocked(listMCPTools)).toHaveBeenCalledWith("litellm-key", "srv-1", undefined)); + expect(screen.queryByText(GATE_TEXT)).not.toBeInTheDocument(); + }); + + it("does not gate an M2M server; lists with the LiteLLM key", async () => { + renderViewer({ oauth2_flow: "client_credentials", delegate_auth_to_upstream: false }); + + await waitFor(() => expect(vi.mocked(listMCPTools)).toHaveBeenCalledWith("litellm-key", "srv-1", undefined)); + expect(screen.queryByText(GATE_TEXT)).not.toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx index 888316f0c3f..5ec9a3d683a 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx @@ -1,7 +1,7 @@ import React, { useEffect, useState } from "react"; import { useQuery, useMutation } from "@tanstack/react-query"; import { ToolTestPanel } from "./ToolTestPanel"; -import { MCPTool, MCPToolsViewerProps, MCPContent, CallMCPToolResponse } from "./types"; +import { MCPTool, MCPToolsViewerProps, MCPContent, CallMCPToolResponse, getMcpOAuthMode } from "./types"; import { listMCPTools, callMCPTool } from "../networking"; import { isTokenValid, getToken, removeToken } from "@/utils/mcpTokenStore"; import { sanitizeMcpAliasForHeader } from "@/utils/mcpHeaderUtils"; @@ -15,7 +15,8 @@ const MCPToolsViewer = ({ serverId, accessToken, auth_type, - tokenUrl, + oauth2_flow, + delegate_auth_to_upstream, userRole, userID, serverAlias, @@ -30,34 +31,29 @@ const MCPToolsViewer = ({ const [passthroughHeaders, setPassthroughHeaders] = useState>({}); const [showHeaderInput, setShowHeaderInput] = useState(false); - // OAuth session token (sessionStorage-backed, cleared on tab/browser close). - // Only the interactive (authorization_code/PKCE) flow needs a user-facing - // auth gate. M2M (client_credentials) servers are also `auth_type === "oauth2"`, - // but the backend fetches their token internally — gating tool listing on - // them would force users through a non-existent authorization endpoint. - // We detect M2M via the presence of `tokenUrl`, matching the heuristic in - // `mcp_server_edit.tsx`. - const isOAuth = auth_type === "oauth2" && !tokenUrl; + // Only PKCE passthrough uses a browser-held session token (sessionStorage, + // cleared on tab/browser close) and a user-facing auth gate. OBO uses the + // backend-stored per-user token and M2M uses the backend's own service token, + // so neither needs a gate — they list tools with just the LiteLLM key. + const isPassthrough = getMcpOAuthMode({ auth_type, oauth2_flow, delegate_auth_to_upstream }) === "passthrough"; const [oauthToken, setOauthToken] = useState(() => - isOAuth && isTokenValid(serverId, userID) - ? (getToken(serverId, userID)?.access_token ?? null) - : null + isPassthrough && isTokenValid(serverId, userID) ? getToken(serverId, userID)?.access_token ?? null : null, ); // Re-sync token when serverId/userID changes (useState initializer only runs on mount). useEffect(() => { - if (!isOAuth) { + if (!isPassthrough) { setOauthToken(null); return; } - setOauthToken( - isTokenValid(serverId, userID) - ? (getToken(serverId, userID)?.access_token ?? null) - : null - ); - }, [serverId, userID, isOAuth]); + setOauthToken(isTokenValid(serverId, userID) ? getToken(serverId, userID)?.access_token ?? null : null); + }, [serverId, userID, isPassthrough]); - const { startOAuthFlow, status: oauthStatus, error: oauthError } = useToolsOAuthFlow({ + const { + startOAuthFlow, + status: oauthStatus, + error: oauthError, + } = useToolsOAuthFlow({ accessToken: accessToken ?? "", serverId, serverAlias, @@ -77,7 +73,8 @@ const MCPToolsViewer = ({ // The backend's _get_mcp_server_auth_headers_from_headers() picks up the // x-mcp-{alias}-{header} pattern and forwards it to the upstream MCP server. // When no alias is available, fall back to x-mcp-auth (legacy but still supported). - if (oauthToken) { + // Passthrough only: OBO/M2M tokens are attached server-side, not from the browser. + if (isPassthrough && oauthToken) { if (serverAlias) { const safeAlias = sanitizeMcpAliasForHeader(serverAlias); if (safeAlias) { @@ -126,9 +123,7 @@ const MCPToolsViewer = ({ if (status === 401) { removeToken(serverId, userID); } - const enhancedError = new Error( - result.message || result.error || "Failed to fetch MCP tools", - ) as Error & { + const enhancedError = new Error(result.message || result.error || "Failed to fetch MCP tools") as Error & { status?: number; statusText?: string; details?: any; @@ -141,7 +136,7 @@ const MCPToolsViewer = ({ return result; }, // For OAuth servers, block the query until a session token is available - enabled: !!accessToken && (!isOAuth || oauthToken !== null), + enabled: !!accessToken && (!isPassthrough || oauthToken !== null), staleTime: 30000, // Consider data fresh for 30 seconds retry: (failureCount, error: any) => { // Don't retry on 401 — token is invalid, user must re-authenticate @@ -153,9 +148,7 @@ const MCPToolsViewer = ({ // If the tools query fails with 401, the cached OAuth token is invalid — // clear it so the auth gate is shown again and the user can re-authenticate. useEffect(() => { - const err = mcpToolsError as - | (Error & { status?: number; response?: { status?: number } }) - | null; + const err = mcpToolsError as (Error & { status?: number; response?: { status?: number } }) | null; const status = err?.status ?? err?.response?.status; if (status === 401) { removeToken(serverId, userID); @@ -169,13 +162,9 @@ const MCPToolsViewer = ({ if (!accessToken) throw new Error("Access Token required"); try { - const result: CallMCPToolResponse = await callMCPTool( - accessToken, - serverId, - args.tool.name, - args.arguments, - { customHeaders: buildCustomHeaders() } - ); + const result: CallMCPToolResponse = await callMCPTool(accessToken, serverId, args.tool.name, args.arguments, { + customHeaders: buildCustomHeaders(), + }); return result; } catch (error) { throw error; @@ -223,9 +212,7 @@ const MCPToolsViewer = ({
- - Additional Headers - + Additional Headers
- + {!showHeaderInput && Object.keys(passthroughHeaders).length === 0 && ( This server requires additional headers. Click "Configure" to provide values. )} - + {showHeaderInput && (
{extraHeaders?.map((headerName) => (
- + !v || !v.trim())} + disabled={Object.values(passthroughHeaders).every((v) => !v || !v.trim())} className="w-full mt-2" > Load Tools
)} - + {!showHeaderInput && Object.keys(passthroughHeaders).length > 0 && (
@@ -303,13 +288,11 @@ const MCPToolsViewer = ({ {/* OAuth Auth Gate — shown when token is absent for OAuth servers */} - {isOAuth && !oauthToken && ( + {isPassthrough && !oauthToken && (

Authentication required

-

- Authenticate to view available tools -

+

Authenticate to view available tools

Authorize - {oauthError && ( -

{oauthError}

- )} + {oauthError &&

{oauthError}

}
)} {/* Search Bar — only shown when tools are loaded */} - {!isOAuth || oauthToken ? <> - {toolsData.length > 0 && ( -
- } - value={toolSearchTerm} - onChange={(e) => setToolSearchTerm(e.target.value)} - allowClear - className="rounded-lg" - size="middle" - /> -
- )} - - {/* Loading State */} - {isLoadingTools && ( -
-
-
-
-
-

Loading tools...

-
- )} - - {/* Error State */} - {(mcpToolsResponse?.error || mcpToolsError) && !isLoadingTools && !toolsData.length && ( -
-

- Error: {mcpToolsResponse?.message || (mcpToolsError as Error)?.message} -

-
- )} - - {/* No Tools State */} - {!isLoadingTools && !mcpToolsResponse?.error && !mcpToolsError && (!toolsData || toolsData.length === 0) && ( -
-
- - - -
-

No tools available

-

No tools found for this server

-
- )} - - {/* Tools List */} - {!isLoadingTools && !mcpToolsResponse?.error && toolsData.length > 0 && ( + {!isPassthrough || oauthToken ? ( <> - {filteredTools.length === 0 ? ( -
- -

No tools found

-

No tools match "{toolSearchTerm}"

-
- ) : ( -
- {filteredTools.map((tool: MCPTool) => ( -
{ - setSelectedTool(tool); - setToolResult(null); - setToolError(null); - }} - > -
- {tool.mcp_info.logo_url && ( - {`${tool.mcp_info.server_name} - )} -
-

{tool.name}

-

{tool.mcp_info.server_name}

-

- {tool.description} -

-
-
- {selectedTool?.name === tool.name && ( -
-
- - - - Selected -
-
- )} -
- ))} + {toolsData.length > 0 && ( +
+ } + value={toolSearchTerm} + onChange={(e) => setToolSearchTerm(e.target.value)} + allowClear + className="rounded-lg" + size="middle" + />
)} + + {/* Loading State */} + {isLoadingTools && ( +
+
+
+
+
+

Loading tools...

+
+ )} + + {/* Error State */} + {(mcpToolsResponse?.error || mcpToolsError) && !isLoadingTools && !toolsData.length && ( +
+

+ Error: {mcpToolsResponse?.message || (mcpToolsError as Error)?.message} +

+
+ )} + + {/* No Tools State */} + {!isLoadingTools && + !mcpToolsResponse?.error && + !mcpToolsError && + (!toolsData || toolsData.length === 0) && ( +
+
+ + + +
+

No tools available

+

No tools found for this server

+
+ )} + + {/* Tools List */} + {!isLoadingTools && !mcpToolsResponse?.error && toolsData.length > 0 && ( + <> + {filteredTools.length === 0 ? ( +
+ +

No tools found

+

No tools match "{toolSearchTerm}"

+
+ ) : ( +
+ {filteredTools.map((tool: MCPTool) => ( +
{ + setSelectedTool(tool); + setToolResult(null); + setToolError(null); + }} + > +
+ {tool.mcp_info.logo_url && ( + {`${tool.mcp_info.server_name} + )} +
+

+ {tool.name} +

+

{tool.mcp_info.server_name}

+

+ {tool.description} +

+
+
+ {selectedTool?.name === tool.name && ( +
+
+ + + + Selected +
+
+ )} +
+ ))} +
+ )} + + )} - )} - : null} + ) : null}
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx index 155abe27ae2..6f050d22fb4 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx @@ -1,5 +1,13 @@ import { describe, it, expect } from "vitest"; -import { AUTH_TYPE, OAUTH_FLOW, TRANSPORT, handleTransport, handleAuth } from "./types"; +import { + AUTH_TYPE, + OAUTH_FLOW, + MCP_OAUTH2_FLOW_M2M, + TRANSPORT, + handleTransport, + handleAuth, + getMcpOAuthMode, +} from "./types"; describe("handleTransport", () => { it("should default to SSE when transport is null", () => { @@ -56,4 +64,65 @@ describe("constants", () => { expect(OAUTH_FLOW.INTERACTIVE).toBe("interactive"); expect(OAUTH_FLOW.M2M).toBe("m2m"); }); + + it("should define the backend M2M flow value", () => { + expect(MCP_OAUTH2_FLOW_M2M).toBe("client_credentials"); + }); +}); + +describe("getMcpOAuthMode", () => { + it("returns null for non-OAuth2 servers", () => { + expect(getMcpOAuthMode({ auth_type: AUTH_TYPE.API_KEY })).toBeNull(); + expect(getMcpOAuthMode({ auth_type: AUTH_TYPE.NONE })).toBeNull(); + expect(getMcpOAuthMode({})).toBeNull(); + }); + + it("classifies client_credentials as m2m", () => { + expect(getMcpOAuthMode({ auth_type: AUTH_TYPE.OAUTH2, oauth2_flow: MCP_OAUTH2_FLOW_M2M })).toBe("m2m"); + }); + + it("treats m2m as m2m even when delegate_auth_to_upstream is true", () => { + expect( + getMcpOAuthMode({ + auth_type: AUTH_TYPE.OAUTH2, + oauth2_flow: MCP_OAUTH2_FLOW_M2M, + delegate_auth_to_upstream: true, + }), + ).toBe("m2m"); + }); + + it("classifies an interactive server with delegate_auth_to_upstream as passthrough", () => { + expect(getMcpOAuthMode({ auth_type: AUTH_TYPE.OAUTH2, oauth2_flow: null, delegate_auth_to_upstream: true })).toBe( + "passthrough", + ); + }); + + it("classifies an interactive server without delegation as obo", () => { + expect(getMcpOAuthMode({ auth_type: AUTH_TYPE.OAUTH2, oauth2_flow: null, delegate_auth_to_upstream: false })).toBe( + "obo", + ); + }); + + it("defaults to obo when delegate_auth_to_upstream is undefined", () => { + expect(getMcpOAuthMode({ auth_type: AUTH_TYPE.OAUTH2 })).toBe("obo"); + }); + + it("treats explicit authorization_code as interactive, not m2m", () => { + expect( + getMcpOAuthMode({ + auth_type: AUTH_TYPE.OAUTH2, + oauth2_flow: "authorization_code", + delegate_auth_to_upstream: false, + }), + ).toBe("obo"); + }); + + // Regression: the old heuristic labeled any OAuth2 server with a token endpoint + // as M2M. getMcpOAuthMode ignores token_url, so an interactive server that + // legitimately carries one is classified by oauth2_flow + delegate, never M2M. + it("does not treat an interactive server with a token endpoint as m2m", () => { + expect(getMcpOAuthMode({ auth_type: AUTH_TYPE.OAUTH2, oauth2_flow: null, delegate_auth_to_upstream: false })).toBe( + "obo", + ); + }); }); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx index 9a8f2e8f514..d34887bc026 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx @@ -47,6 +47,28 @@ export const OAUTH_FLOW = { M2M: "m2m", }; +// Backend value of `oauth2_flow` that marks a machine-to-machine server. Distinct +// from the UI-local OAUTH_FLOW.M2M ("m2m"); this is what the API actually returns. +export const MCP_OAUTH2_FLOW_M2M = "client_credentials"; + +export type McpOAuthMode = "m2m" | "passthrough" | "obo"; + +// Classify an OAuth2 MCP server into the mode that decides how the tool list is +// authenticated: M2M (backend service token), PKCE passthrough (browser-held +// session token), or OBO (backend-stored per-user token). `token_url` is +// intentionally not consulted: every OAuth2 grant that exchanges for a token +// carries one (interactive PKCE and client_credentials alike), so it cannot +// distinguish the modes; `oauth2_flow` is the authoritative M2M signal. +export function getMcpOAuthMode(s: { + auth_type?: string | null; + oauth2_flow?: string | null; + delegate_auth_to_upstream?: boolean | null; +}): McpOAuthMode | null { + if (s.auth_type !== AUTH_TYPE.OAUTH2) return null; + if (s.oauth2_flow === MCP_OAUTH2_FLOW_M2M) return "m2m"; + return s.delegate_auth_to_upstream ? "passthrough" : "obo"; +} + export const TRANSPORT = { SSE: "sse", HTTP: "http", @@ -163,11 +185,14 @@ export interface MCPToolsViewerProps { serverId: string; accessToken: string | null; auth_type?: string | null; + /** Backend OAuth2 grant; `client_credentials` marks an M2M server. */ + oauth2_flow?: string | null; + /** When true (interactive OAuth2), the server uses PKCE passthrough. */ + delegate_auth_to_upstream?: boolean | null; /** - * When set, indicates the server uses the OAuth2 M2M (client_credentials) - * flow — the backend handles token acquisition internally, so the UI must - * not gate tool listing behind an interactive PKCE authorization. Mirrors - * the heuristic used in `mcp_server_edit.tsx` (`token_url` set => M2M). + * Connection field present on every OAuth2 flow (interactive and M2M alike), + * so it does not indicate the mode. Retained for callers/other uses; not read + * for mode detection — see getMcpOAuthMode. */ tokenUrl?: string | null; userRole: string | null; @@ -189,6 +214,7 @@ export interface MCPServer { spec_path?: string | null; transport?: string | null; auth_type?: string | null; + oauth2_flow?: string | null; authorization_url?: string | null; token_url?: string | null; registration_url?: string | null;