From bc9a48d407ad974af12ea89cd46af6e6a6ddb29e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 7 Mar 2026 03:37:30 +0000 Subject: [PATCH] fix: MCP OAuth 2.0 UX gaps across admin and internal user flows Fixes found by dogfooding the MCP OAuth 2.0 flow as both an admin and an internal user: 1. Fix [object Object] error when creating MCP server - Use parseErrorMessage() in create_mcp_server.tsx catch handler - Fix deriveErrorMessage() in networking.tsx to always return a string instead of potentially returning an object 2. Pre-configured OAuth servers auto-select auth type - Add auth_type field to DiscoverableMCPServer type - Add auth_type='oauth2' to Atlassian and Linear in mcp_registry.json - Pre-fill auth_type in create form when selecting from catalog - Use requestAnimationFrame to defer setting after transport dropdown mounts 3. Backend: drop tool_choice when MCP resolves no tools - In chat_completions_handler.py, remove tool_choice from kwargs when all_tools is empty after MCP resolution, preventing 400 errors from providers that reject tool_choice without tools 4. Chat UI: OAuth-aware MCP server management - MCPAppsPanel: show lock icon for OAuth servers in grid, show 'Authorization required' banner + 'Authorize' button in detail view, and initiate PKCE OAuth flow via existing proxy endpoints - MCPConnectPicker: show lock icon next to OAuth server names - Store user OAuth tokens in sessionStorage for the session Co-authored-by: Ishaan Jaff --- litellm/proxy/mcp_registry.json | 2 + .../responses/mcp/chat_completions_handler.py | 4 + .../src/components/chat/MCPAppsPanel.tsx | 266 ++++++++++++++++-- .../src/components/chat/MCPConnectPicker.tsx | 21 +- .../mcp_tools/create_mcp_server.tsx | 11 +- .../src/components/mcp_tools/types.tsx | 1 + .../src/components/networking.tsx | 7 +- 7 files changed, 284 insertions(+), 28 deletions(-) diff --git a/litellm/proxy/mcp_registry.json b/litellm/proxy/mcp_registry.json index 2e1e8f64eae..c58ffff3bdb 100644 --- a/litellm/proxy/mcp_registry.json +++ b/litellm/proxy/mcp_registry.json @@ -35,6 +35,7 @@ "registry_url": "https://registry.modelcontextprotocol.io/servers/com.atlassian%2Fatlassian-mcp-server", "transport": "sse", "url": "https://mcp.atlassian.com/v1/sse", + "auth_type": "oauth2", "env_vars": [] }, { @@ -46,6 +47,7 @@ "registry_url": "https://registry.modelcontextprotocol.io/servers/app.linear%2Flinear", "transport": "sse", "url": "https://mcp.linear.app/sse", + "auth_type": "oauth2", "env_vars": [] }, { diff --git a/litellm/responses/mcp/chat_completions_handler.py b/litellm/responses/mcp/chat_completions_handler.py index bacc627cc84..9209cf35e23 100644 --- a/litellm/responses/mcp/chat_completions_handler.py +++ b/litellm/responses/mcp/chat_completions_handler.py @@ -160,6 +160,10 @@ async def acompletion_with_mcp( # noqa: PLR0915 # Remove keys that shouldn't be passed to acompletion clean_kwargs = {k: v for k, v in kwargs.items() if k not in ["acompletion"]} + # If no tools were resolved, also drop tool_choice to avoid provider errors + if not all_tools: + clean_kwargs.pop("tool_choice", None) + base_call_args = { "model": model, "messages": messages, diff --git a/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx b/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx index 926d6697bca..0e0af7de145 100644 --- a/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx +++ b/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx @@ -1,10 +1,10 @@ "use client"; -import React, { useEffect, useState } from "react"; +import React, { useEffect, useState, useCallback } from "react"; import { Switch, Spin, Input, Button } from "antd"; -import { SearchOutlined, ArrowLeftOutlined, RightOutlined } from "@ant-design/icons"; -import { fetchMCPServers, listMCPTools } from "../networking"; -import { MCPServer } from "../mcp_tools/types"; +import { SearchOutlined, ArrowLeftOutlined, RightOutlined, LockOutlined, CheckCircleOutlined } from "@ant-design/icons"; +import { fetchMCPServers, listMCPTools, registerMcpOAuthClient, buildMcpOAuthAuthorizeUrl, exchangeMcpOAuthToken, cacheTemporaryMcpServer } from "../networking"; +import { MCPServer, AUTH_TYPE } from "../mcp_tools/types"; import { message } from "antd"; interface Props { @@ -24,6 +24,38 @@ function getAvatarColor(name: string): string { return AVATAR_COLORS[Math.abs(hash) % AVATAR_COLORS.length]; } +const OAUTH_TOKEN_STORAGE_PREFIX = "litellm-mcp-user-oauth-token-"; +const OAUTH_FLOW_STATE_KEY = "litellm-mcp-chat-oauth-flow-state"; +const OAUTH_RESULT_KEY = "litellm-mcp-oauth-result"; +const OAUTH_RETURN_URL_KEY = "litellm-mcp-oauth-return-url"; +const CHAT_OAUTH_SELECTED_SERVERS_KEY = "litellm-mcp-chat-selected-servers"; + +function getStoredOAuthToken(serverId: string): string | null { + try { + const stored = sessionStorage.getItem(OAUTH_TOKEN_STORAGE_PREFIX + serverId) || + localStorage.getItem(OAUTH_TOKEN_STORAGE_PREFIX + serverId); + return stored || null; + } catch { return null; } +} + +function storeOAuthToken(serverId: string, token: string) { + try { + sessionStorage.setItem(OAUTH_TOKEN_STORAGE_PREFIX + serverId, token); + localStorage.setItem(OAUTH_TOKEN_STORAGE_PREFIX + serverId, token); + } catch { /* ignore */ } +} + +function isOAuthServer(server: MCPServer): boolean { + return server.auth_type === AUTH_TYPE.OAUTH2 || server.auth_type === "oauth2"; +} + +const base64UrlEncode = (buffer: ArrayBuffer) => { + const bytes = new Uint8Array(buffer); + let binary = ""; + bytes.forEach((b) => (binary += String.fromCharCode(b))); + return btoa(binary).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, ""); +}; + type TabKey = "all" | "connected"; const MCPAppsPanel: React.FC = ({ accessToken, selectedServers, onChange }) => { @@ -33,6 +65,8 @@ const MCPAppsPanel: React.FC = ({ accessToken, selectedServers, onChange const [activeTab, setActiveTab] = useState("all"); const [togglingOn, setTogglingOn] = useState>(new Set()); const [detailServer, setDetailServer] = useState(null); + const [oauthAuthorizing, setOauthAuthorizing] = useState(false); + const [oauthTokens, setOauthTokens] = useState>({}); useEffect(() => { let cancelled = false; @@ -42,6 +76,14 @@ const MCPAppsPanel: React.FC = ({ accessToken, selectedServers, onChange if (cancelled) return; const list: MCPServer[] = Array.isArray(data) ? data : (data?.data ?? []); setServers(list); + const tokens: Record = {}; + list.forEach((s) => { + if (isOAuthServer(s)) { + const t = getStoredOAuthToken(s.server_id); + if (t) tokens[s.server_id] = t; + } + }); + setOauthTokens(tokens); }) .catch(() => { if (!cancelled) setServers([]); @@ -52,6 +94,147 @@ const MCPAppsPanel: React.FC = ({ accessToken, selectedServers, onChange return () => { cancelled = true; }; }, [accessToken]); + // Resume OAuth flow after redirect + useEffect(() => { + if (typeof window === "undefined") return; + const storedResult = sessionStorage.getItem(OAUTH_RESULT_KEY) || localStorage.getItem(OAUTH_RESULT_KEY); + const storedFlowState = sessionStorage.getItem(OAUTH_FLOW_STATE_KEY) || localStorage.getItem(OAUTH_FLOW_STATE_KEY); + if (!storedResult || !storedFlowState) return; + + const resumeOAuth = async () => { + try { + const result = JSON.parse(storedResult); + const flowState = JSON.parse(storedFlowState); + // Clean up storage + [sessionStorage, localStorage].forEach((s) => { + try { + s.removeItem(OAUTH_RESULT_KEY); + s.removeItem(OAUTH_FLOW_STATE_KEY); + s.removeItem(OAUTH_RETURN_URL_KEY); + } catch { /* ignore */ } + }); + + if (result.error) { + message.error(result.error_description || result.error); + return; + } + if (!result.code || !flowState.state || result.state !== flowState.state) { + message.error("OAuth state mismatch. Please try again."); + return; + } + + const token = await exchangeMcpOAuthToken({ + serverId: flowState.serverId, + code: result.code, + clientId: flowState.clientId, + clientSecret: flowState.clientSecret, + codeVerifier: flowState.codeVerifier, + redirectUri: flowState.redirectUri, + }); + + if (token?.access_token) { + storeOAuthToken(flowState.originalServerId || flowState.serverId, token.access_token); + setOauthTokens((prev) => ({ ...prev, [flowState.originalServerId || flowState.serverId]: token.access_token })); + message.success("Connected successfully!"); + + // Restore selected servers and auto-connect + try { + const savedServers = sessionStorage.getItem(CHAT_OAUTH_SELECTED_SERVERS_KEY); + if (savedServers) { + const parsed = JSON.parse(savedServers); + sessionStorage.removeItem(CHAT_OAUTH_SELECTED_SERVERS_KEY); + if (Array.isArray(parsed)) { + onChange(parsed); + } + } + } catch { /* ignore */ } + } + } catch (err) { + message.error("Failed to complete OAuth authorization."); + } + }; + + resumeOAuth(); + }, [accessToken, onChange]); + + const startOAuthForServer = useCallback(async (server: MCPServer) => { + if (!accessToken) return; + setOauthAuthorizing(true); + + try { + const serverPayload = { + server_id: server.server_id, + server_name: server.server_name, + alias: server.alias, + description: server.description, + url: server.url, + transport: server.transport || "http", + auth_type: AUTH_TYPE.OAUTH2, + }; + + const cached = await cacheTemporaryMcpServer(accessToken, serverPayload); + const serverId = cached?.server_id?.trim(); + if (!serverId) throw new Error("Failed to prepare OAuth session"); + + let registeredClient: { clientId?: string; clientSecret?: string } = {}; + const registration = await registerMcpOAuthClient(accessToken, serverId, { + client_name: server.alias || server.server_name || serverId, + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }); + registeredClient = { + clientId: registration?.client_id, + clientSecret: registration?.client_secret, + }; + + const verifierArray = new Uint8Array(32); + crypto.getRandomValues(verifierArray); + const verifier = base64UrlEncode(verifierArray.buffer); + const challengeData = new TextEncoder().encode(verifier); + const digest = await crypto.subtle.digest("SHA-256", challengeData); + const challenge = base64UrlEncode(digest); + const state = crypto.randomUUID(); + + const path = window.location.pathname || ""; + const uiIndex = path.indexOf("/ui"); + const uiPrefix = uiIndex >= 0 ? path.slice(0, uiIndex + 3) : ""; + const redirectUri = `${window.location.origin}${uiPrefix}/mcp/oauth/callback`; + + const authorizeUrl = buildMcpOAuthAuthorizeUrl({ + serverId, + clientId: registeredClient.clientId, + redirectUri, + state, + codeChallenge: challenge, + }); + + const flowState = { + state, + codeVerifier: verifier, + clientId: registeredClient.clientId, + clientSecret: registeredClient.clientSecret, + serverId, + originalServerId: server.server_id, + redirectUri, + }; + + // Persist state for after redirect + [sessionStorage, localStorage].forEach((s) => { + try { + s.setItem(OAUTH_FLOW_STATE_KEY, JSON.stringify(flowState)); + s.setItem(OAUTH_RETURN_URL_KEY, window.location.href); + s.setItem(CHAT_OAUTH_SELECTED_SERVERS_KEY, JSON.stringify(selectedServers)); + } catch { /* ignore */ } + }); + + window.location.href = authorizeUrl; + } catch (err) { + message.error(err instanceof Error ? err.message : "Failed to start OAuth flow"); + setOauthAuthorizing(false); + } + }, [accessToken, selectedServers]); + const handleToggle = async (serverName: string, checked: boolean) => { if (!checked) { onChange(selectedServers.filter((s) => s !== serverName)); @@ -95,6 +278,8 @@ const MCPAppsPanel: React.FC = ({ accessToken, selectedServers, onChange const isConnected = selectedServers.includes(name); const isTogglingOn = togglingOn.has(name); const color = getAvatarColor(name); + const isOAuth = isOAuthServer(detailServer); + const hasOAuthToken = Boolean(oauthTokens[detailServer.server_id]); return (
@@ -122,26 +307,68 @@ const MCPAppsPanel: React.FC = ({ accessToken, selectedServers, onChange {name.charAt(0).toUpperCase()}
-

{name}

+
+

{name}

+ {isOAuth && ( + + {hasOAuthToken ? <> Authorized : <> OAuth} + + )} +

{detailServer.description ?? "MCP server"}

- + {isOAuth && !hasOAuthToken ? ( + + ) : ( + + )} + {/* OAuth notice for unauthenticated OAuth servers */} + {isOAuth && !hasOAuthToken && ( +
+ +
+
Authorization required
+ This server uses OAuth 2.0. Click “Authorize” to sign in with the service + provider and grant access to your account. Your credentials are not shared with LiteLLM. +
+
+ )} + {/* Info table */}

Information

{[ ["Server ID", detailServer.server_id], ["Transport", (detailServer as MCPServer & { mcp_info?: { server_url?: string } }).mcp_info?.server_url ? "HTTP" : "stdio"], - ["Status", isConnected ? "Connected" : "Not connected"], + ["Authentication", isOAuth ? "OAuth 2.0" : (detailServer.auth_type || "None")], + ["Status", isConnected ? "Connected" : (isOAuth && !hasOAuthToken ? "Not authorized" : "Not connected")], ].filter(([, v]) => v).map(([label, value], i, arr) => (
= ({ accessToken, selectedServers, onChange const isConnected = selectedServers.includes(name); const color = getAvatarColor(name); const isLeftCol = idx % 2 === 0; + const isOAuth = isOAuthServer(server); + const hasToken = Boolean(oauthTokens[server.server_id]); return (
= ({ accessToken, selectedServers, onChange {name.charAt(0).toUpperCase()}
-
- {name} +
+ + {name} + + {isOAuth && ( + + )}
{server.description ?? "MCP server"} diff --git a/ui/litellm-dashboard/src/components/chat/MCPConnectPicker.tsx b/ui/litellm-dashboard/src/components/chat/MCPConnectPicker.tsx index 234aa8a5281..8e640555e0b 100644 --- a/ui/litellm-dashboard/src/components/chat/MCPConnectPicker.tsx +++ b/ui/litellm-dashboard/src/components/chat/MCPConnectPicker.tsx @@ -1,7 +1,8 @@ import React, { useEffect, useState } from "react"; import { Switch, Spin, message } from "antd"; +import { LockOutlined } from "@ant-design/icons"; import { fetchMCPServers, listMCPTools } from "../networking"; -import { MCPServer } from "../mcp_tools/types"; +import { MCPServer, AUTH_TYPE } from "../mcp_tools/types"; interface Props { accessToken: string; @@ -9,10 +10,13 @@ interface Props { onChange: (servers: string[]) => void; } +function isOAuthServer(server: MCPServer): boolean { + return server.auth_type === AUTH_TYPE.OAUTH2 || server.auth_type === "oauth2"; +} + const MCPConnectPicker: React.FC = ({ accessToken, selectedServers, onChange }) => { const [servers, setServers] = useState([]); const [loadingServers, setLoadingServers] = useState(true); - // Track which individual servers are being toggled on (verifying tools) const [togglingOn, setTogglingOn] = useState>(new Set()); useEffect(() => { @@ -23,7 +27,6 @@ const MCPConnectPicker: React.FC = ({ accessToken, selectedServers, onCha try { const data = await fetchMCPServers(accessToken); if (cancelled) return; - // API returns { data: MCPServer[] } or MCPServer[] const list: MCPServer[] = Array.isArray(data) ? data : (data?.data ?? []); setServers(list); } catch { @@ -46,21 +49,17 @@ const MCPConnectPicker: React.FC = ({ accessToken, selectedServers, onCha const handleToggle = async (serverName: string, checked: boolean) => { if (!checked) { - // Toggle OFF — remove immediately, no tool fetch needed onChange(selectedServers.filter((s) => s !== serverName)); return; } - // Toggle ON — verify tools are reachable first setTogglingOn((prev) => new Set(prev).add(serverName)); try { const result = await listMCPTools(accessToken, serverName); - // listMCPTools never throws; it returns { tools, error, message } on failure if (result?.error) { message.warning( `Could not load tools for ${serverName} — it will be excluded from this message.` ); - // Do not add to selectedServers return; } onChange([...selectedServers, serverName]); @@ -68,7 +67,6 @@ const MCPConnectPicker: React.FC = ({ accessToken, selectedServers, onCha message.warning( `Could not load tools for ${serverName} — it will be excluded from this message.` ); - // Do not add to selectedServers } finally { setTogglingOn((prev) => { const next = new Set(prev); @@ -100,6 +98,7 @@ const MCPConnectPicker: React.FC = ({ accessToken, selectedServers, onCha const name = server.server_name ?? server.alias ?? server.server_id; const isSelected = selectedServers.includes(name); const isTogglingOn = togglingOn.has(name); + const isOAuth = isOAuthServer(server); return (
= ({ accessToken, selectedServers, onCha whiteSpace: "nowrap", overflow: "hidden", textOverflow: "ellipsis", + display: "flex", + alignItems: "center", + gap: 5, }} > {name} + {isOAuth && ( + + )}
{server.description && (
= ({ form.setFieldsValue(prefillValues); setFormValues(prefillValues); setAliasManuallyEdited(false); + + // Auth type must be set after a render cycle so the auth dropdown is mounted + if (prefillData.auth_type && transport !== "stdio") { + requestAnimationFrame(() => { + form.setFieldsValue({ auth_type: prefillData.auth_type }); + setFormValues((prev) => ({ ...prev, auth_type: prefillData.auth_type })); + }); + } }, [isModalVisible, prefillData, form]); const handleCreate = async (values: Record) => { @@ -385,7 +394,7 @@ const CreateMCPServer: React.FC = ({ onCreateSuccess(response); } } catch (error) { - NotificationsManager.fromBackend("Error creating MCP Server: " + error); + NotificationsManager.fromBackend("Error creating MCP Server: " + parseErrorMessage(error)); } finally { setIsLoading(false); } diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx index 6ba25012197..d08a8e90ee9 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx @@ -202,6 +202,7 @@ export interface DiscoverableMCPServer { registry_url?: string | null; transport: string; url?: string | null; + auth_type?: string | null; command?: string | null; args?: string[] | null; env_vars?: Array<{ name: string; description?: string; secret?: boolean }> | null; diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 30c1d3c5b81..0ba8ff62fa3 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -8799,13 +8799,14 @@ export const perUserAnalyticsCall = async ( }; export const deriveErrorMessage = (errorData: any): string => { - return ( + const raw = (errorData?.error && (errorData.error.message || errorData.error)) || errorData?.message || errorData?.detail || errorData?.error || - JSON.stringify(errorData) - ); + errorData; + if (typeof raw === "string") return raw; + return JSON.stringify(raw); }; export interface LoginRequest {