mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-18 06:26:16 +00:00
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.
This commit is contained in:
@@ -185,6 +185,8 @@ export const MCPServerView: React.FC<MCPServerViewProps> = ({
|
||||
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}
|
||||
|
||||
@@ -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<string, unknown>) =>
|
||||
render(
|
||||
<QueryClientProvider client={new QueryClient({ defaultOptions: { queries: { retry: false } } })}>
|
||||
<MCPToolsViewer
|
||||
serverId="srv-1"
|
||||
accessToken="litellm-key"
|
||||
userRole="admin"
|
||||
userID="tin@berri.ai"
|
||||
serverAlias="slack"
|
||||
auth_type="oauth2"
|
||||
tokenUrl={TOKEN_URL}
|
||||
{...props}
|
||||
/>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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,26 +31,23 @@ const MCPToolsViewer = ({
|
||||
const [passthroughHeaders, setPassthroughHeaders] = useState<Record<string, string>>({});
|
||||
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<string | null>(() =>
|
||||
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]);
|
||||
}, [serverId, userID, isPassthrough]);
|
||||
|
||||
const {
|
||||
startOAuthFlow,
|
||||
@@ -75,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) {
|
||||
@@ -137,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
|
||||
@@ -289,7 +288,7 @@ const MCPToolsViewer = ({
|
||||
</Text>
|
||||
|
||||
{/* OAuth Auth Gate — shown when token is absent for OAuth servers */}
|
||||
{isOAuth && !oauthToken && (
|
||||
{isPassthrough && !oauthToken && (
|
||||
<div className="p-4 text-center bg-white border border-gray-200 rounded-lg">
|
||||
<LockOutlined className="text-2xl text-gray-400 mb-2" />
|
||||
<p className="text-xs font-medium text-gray-700 mb-1">Authentication required</p>
|
||||
@@ -308,7 +307,7 @@ const MCPToolsViewer = ({
|
||||
)}
|
||||
|
||||
{/* Search Bar — only shown when tools are loaded */}
|
||||
{!isOAuth || oauthToken ? (
|
||||
{!isPassthrough || oauthToken ? (
|
||||
<>
|
||||
{toolsData.length > 0 && (
|
||||
<div className="mb-3">
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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",
|
||||
@@ -164,11 +186,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;
|
||||
@@ -190,6 +215,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;
|
||||
|
||||
Reference in New Issue
Block a user