From a4fd75fd31bdc0aa97a31eaf08ff41a8ba59f57e Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Wed, 25 Feb 2026 12:04:00 -0800 Subject: [PATCH] [Fix] UI - MCP Servers: Make auth value optional for create flow The backend validator and frontend form both enforced auth_value as required when auth_type is api_key, bearer_token, or basic. Users who want to provide auth dynamically (via per-request headers or OAuth2 flows) could not skip the field. - Remove required validation from auth_value in create_mcp_server.tsx (keep whitespace-only rejection, matching the edit flow) - Remove validate_credentials_requirements in NewMCPServerRequest (all downstream code already treats auth_value as optional) - Add tests for the create MCP server component Co-Authored-By: Claude Opus 4.6 (1M context) --- litellm/proxy/_types.py | 21 +- .../mcp_tools/create_mcp_server.test.tsx | 388 ++++++++++++++++++ .../mcp_tools/create_mcp_server.tsx | 30 +- 3 files changed, 412 insertions(+), 27 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 4053d9d077..8cd2899eab 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -1113,23 +1113,12 @@ class NewMCPServerRequest(LiteLLMPydanticObjectBase): @model_validator(mode="before") @classmethod def validate_credentials_requirements(cls, values): - if not isinstance(values, dict): - return values - - auth_type = values.get("auth_type") - if auth_type in {MCPAuth.api_key, MCPAuth.bearer_token, MCPAuth.basic}: - credentials = values.get("credentials") - auth_value = None - if isinstance(credentials, dict): - auth_value = credentials.get("auth_value") - elif hasattr(credentials, "get"): - auth_value = credentials.get("auth_value") # type: ignore[attr-defined] - - if not auth_value: - raise ValueError( - "auth_value is required when auth_type is api_key, bearer_token, or basic" - ) + """Validate credentials when provided. + auth_value is optional — users may configure it dynamically + (e.g. via per-request headers or OAuth2 flows) instead of + storing a static value at server creation time. + """ return values diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx new file mode 100644 index 0000000000..2b7273c190 --- /dev/null +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx @@ -0,0 +1,388 @@ +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as networking from "../networking"; +import CreateMCPServer from "./create_mcp_server"; + +vi.mock("../networking", () => ({ + createMCPServer: vi.fn(), + testMCPToolsListRequest: vi.fn().mockResolvedValue({ tools: [], error: null }), +})); + +vi.mock("@/hooks/useMcpOAuthFlow", () => ({ + useMcpOAuthFlow: () => ({ + startOAuthFlow: vi.fn(), + status: "idle", + error: null, + tokenResponse: null, + }), +})); + +vi.mock("./mcp_server_cost_config", () => ({ + default: () =>
, +})); + +vi.mock("./MCPPermissionManagement", () => ({ + default: () =>
, +})); + +vi.mock("./mcp_tool_configuration", () => ({ + default: () =>
, +})); + +vi.mock("./mcp_connection_status", () => ({ + default: () =>
, +})); + +vi.mock("./StdioConfiguration", () => ({ + default: () =>
, +})); + +const defaultProps = { + userRole: "Admin", + accessToken: "test-token", + onCreateSuccess: vi.fn(), + isModalVisible: true, + setModalVisible: vi.fn(), + availableAccessGroups: ["group-a", "group-b"], +}; + +/** Helper: get the server_name input by its Ant Form id */ +const getServerNameInput = () => document.getElementById("server_name") as HTMLInputElement; + +/** Helper: select a dropdown option by opening a select near a label and clicking an option */ +async function selectAntOption(labelText: string, optionText: string) { + const label = screen.getByText(labelText); + const formItem = label.closest(".ant-form-item")!; + const select = formItem.querySelector(".ant-select"); + act(() => { + fireEvent.mouseDown(select!.querySelector(".ant-select-selector")!); + }); + + await waitFor(() => { + const options = document.querySelectorAll(".ant-select-item-option"); + expect(options.length).toBeGreaterThan(0); + }); + + const option = Array.from(document.querySelectorAll(".ant-select-item-option")).find((el) => + el.textContent?.includes(optionText), + ); + expect(option).toBeTruthy(); + act(() => { + fireEvent.click(option!); + }); +} + +describe("CreateMCPServer", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("should render the modal with title when visible", () => { + render(); + + expect(screen.getByText("Add New MCP Server")).toBeInTheDocument(); + }); + + it("should not render when user is not an admin", () => { + render(); + + expect(screen.queryByText("Add New MCP Server")).not.toBeInTheDocument(); + }); + + it("should show transport type options", async () => { + render(); + + await selectAntOption("Transport Type", "Streamable HTTP"); + + // Verify the option was applied by checking the URL field appears + await waitFor(() => { + expect(screen.getByPlaceholderText("https://your-mcp-server.com")).toBeInTheDocument(); + }); + }); + + describe("when HTTP transport is selected", () => { + async function selectHttpTransport() { + render(); + await selectAntOption("Transport Type", "Streamable HTTP"); + + // Wait for URL field to appear (confirms transport was set) + await waitFor(() => { + expect(screen.getByPlaceholderText("https://your-mcp-server.com")).toBeInTheDocument(); + }); + } + + it("should show URL field after selecting HTTP transport", async () => { + await selectHttpTransport(); + + expect(screen.getByPlaceholderText("https://your-mcp-server.com")).toBeInTheDocument(); + }); + + it("should show auth type dropdown after selecting HTTP transport", async () => { + await selectHttpTransport(); + + expect(screen.getByText("Authentication")).toBeInTheDocument(); + }); + + it("should show auth value field when API Key auth type is selected", async () => { + await selectHttpTransport(); + + await selectAntOption("Authentication", "API Key"); + + await waitFor(() => { + expect(screen.getByText("Authentication Value")).toBeInTheDocument(); + }); + }); + + it("should not require auth value when creating a server with API Key auth type", async () => { + await selectHttpTransport(); + + const user = userEvent.setup(); + + // Fill in server name (use id to avoid duplicate placeholder) + const nameInput = getServerNameInput(); + await user.type(nameInput, "Test_Server"); + + // Fill in URL + const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com"); + await user.type(urlInput, "https://example.com/mcp"); + + // Select API Key auth type + await selectAntOption("Authentication", "API Key"); + + await waitFor(() => { + expect(screen.getByText("Authentication Value")).toBeInTheDocument(); + }); + + // Leave auth value empty and submit + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "new-server-1", + server_name: "Test_Server", + alias: "Test_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: "api_key", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + const submitButton = screen.getByRole("button", { name: "Add MCP Server" }); + await act(async () => { + fireEvent.click(submitButton); + }); + + // The form should submit without validation error on auth_value + await waitFor(() => { + expect(networking.createMCPServer).toHaveBeenCalledTimes(1); + }); + }); + + it("should not require auth value when creating a server with Bearer Token auth type", async () => { + await selectHttpTransport(); + + const user = userEvent.setup(); + + const nameInput = getServerNameInput(); + await user.type(nameInput, "Test_Server"); + + const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com"); + await user.type(urlInput, "https://example.com/mcp"); + + await selectAntOption("Authentication", "Bearer Token"); + + await waitFor(() => { + expect(screen.getByText("Authentication Value")).toBeInTheDocument(); + }); + + // Leave auth value empty and submit + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "new-server-1", + server_name: "Test_Server", + alias: "Test_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: "bearer_token", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + const submitButton = screen.getByRole("button", { name: "Add MCP Server" }); + await act(async () => { + fireEvent.click(submitButton); + }); + + await waitFor(() => { + expect(networking.createMCPServer).toHaveBeenCalledTimes(1); + }); + }); + + it("should successfully create a server when auth value is provided", async () => { + await selectHttpTransport(); + + const user = userEvent.setup(); + + const nameInput = getServerNameInput(); + await user.type(nameInput, "My_Server"); + + const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com"); + await user.type(urlInput, "https://example.com/mcp"); + + await selectAntOption("Authentication", "API Key"); + + await waitFor(() => { + expect(screen.getByText("Authentication Value")).toBeInTheDocument(); + }); + + // Fill in auth value + const authInput = screen.getByPlaceholderText("Enter token or secret"); + await user.type(authInput, "my-secret-key"); + + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "new-server-1", + server_name: "My_Server", + alias: "My_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: "api_key", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + const submitButton = screen.getByRole("button", { name: "Add MCP Server" }); + await act(async () => { + fireEvent.click(submitButton); + }); + + await waitFor(() => { + expect(networking.createMCPServer).toHaveBeenCalledTimes(1); + }); + + const [token, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + expect(token).toBe("test-token"); + expect(payload.credentials).toEqual({ auth_value: "my-secret-key" }); + }); + + it("should not show auth value field when None auth type is selected", async () => { + await selectHttpTransport(); + + await selectAntOption("Authentication", "None"); + + // Auth value field should not appear for "None" + await waitFor(() => { + expect(screen.queryByText("Authentication Value")).not.toBeInTheDocument(); + }); + }); + + it("should successfully create a server with no auth", async () => { + await selectHttpTransport(); + + const user = userEvent.setup(); + + const nameInput = getServerNameInput(); + await user.type(nameInput, "No_Auth_Server"); + + const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com"); + await user.type(urlInput, "https://example.com/mcp"); + + await selectAntOption("Authentication", "None"); + + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "new-server-1", + server_name: "No_Auth_Server", + alias: "No_Auth_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: "none", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + const submitButton = screen.getByRole("button", { name: "Add MCP Server" }); + await act(async () => { + fireEvent.click(submitButton); + }); + + await waitFor(() => { + expect(networking.createMCPServer).toHaveBeenCalledTimes(1); + }); + + const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + expect(payload.auth_type).toBe("none"); + // No credentials should be sent for "none" auth + expect(payload.credentials).toBeUndefined(); + }); + }); + + describe("when modal is cancelled", () => { + it("should call setModalVisible(false) when cancel is clicked", async () => { + render(); + + const cancelButton = screen.getByRole("button", { name: "Cancel" }); + await act(async () => { + fireEvent.click(cancelButton); + }); + + expect(defaultProps.setModalVisible).toHaveBeenCalledWith(false); + }); + }); + + describe("when stdio transport is selected", () => { + it("should not show auth type or URL fields", async () => { + render(); + + await selectAntOption("Transport Type", "Standard Input/Output"); + + // Auth and URL fields should not be present for stdio + await waitFor(() => { + expect(screen.queryByText("Authentication")).not.toBeInTheDocument(); + expect(screen.queryByPlaceholderText("https://your-mcp-server.com")).not.toBeInTheDocument(); + }); + }); + }); + + describe("when prefillData is provided", () => { + it("should populate form fields from discovery data", async () => { + const prefillData = { + name: "github-mcp", + title: "GitHub MCP", + description: "GitHub integration server", + category: "Development", + transport: "http", + url: "https://github-mcp.example.com", + }; + + render(); + + await waitFor(() => { + // Server name should be sanitized (hyphens replaced with underscores) + const nameInput = getServerNameInput(); + expect(nameInput).toHaveValue("github_mcp"); + }); + }); + }); + + describe("with back to discovery button", () => { + it("should show back button and call onBackToDiscovery when clicked", async () => { + const onBackToDiscovery = vi.fn(); + render(); + + // The back arrow button should be visible + const backButton = screen.getByText("←"); + expect(backButton).toBeInTheDocument(); + + await act(async () => { + fireEvent.click(backButton); + }); + + expect(onBackToDiscovery).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index bd7e3f9034..0062ca5db4 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -47,7 +47,10 @@ const CreateMCPServer: React.FC = ({ const [isLoading, setIsLoading] = useState(false); const [costConfig, setCostConfig] = useState({}); const [formValues, setFormValues] = useState>({}); - const [pendingRestoredValues, setPendingRestoredValues] = useState<{ values: Record; transport?: string } | null>(null); + const [pendingRestoredValues, setPendingRestoredValues] = useState<{ + values: Record; + transport?: string; + } | null>(null); const [aliasManuallyEdited, setAliasManuallyEdited] = useState(false); const [tools, setTools] = useState([]); const [allowedTools, setAllowedTools] = useState([]); @@ -129,7 +132,7 @@ const CreateMCPServer: React.FC = ({ }, onTokenReceived: (token) => { setOauthAccessToken(token?.access_token ?? null); - + if (token?.access_token) { const credentials = { access_token: token.access_token, @@ -137,11 +140,11 @@ const CreateMCPServer: React.FC = ({ ...(token.expires_in && { expires_in: token.expires_in }), ...(token.scope && { scope: token.scope }), }; - + form.setFieldsValue({ credentials }); - + NotificationsManager.success( - "OAuth authorization successful! Please click 'Create MCP Server' to save the configuration." + "OAuth authorization successful! Please click 'Create MCP Server' to save the configuration.", ); } }, @@ -356,7 +359,8 @@ const CreateMCPServer: React.FC = ({ }; payload.static_headers = staticHeaders; - const includeCredentials = restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type); + const includeCredentials = + restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type); if (includeCredentials && credentialsPayload && Object.keys(credentialsPayload).length > 0) { payload.credentials = credentialsPayload; @@ -534,10 +538,7 @@ const CreateMCPServer: React.FC = ({ } name="alias" - rules={[ - { required: false }, - { validator: (_, value) => validateMCPServerName(value) }, - ]} + rules={[{ required: false }, { validator: (_, value) => validateMCPServerName(value) }]} > = ({ } name={["credentials", "auth_value"]} - rules={[{ required: true, message: "Please enter the authentication value" }]} + rules={[ + { + validator: (_, value) => + value && typeof value === "string" && value.trim() === "" + ? Promise.reject(new Error("Authentication value cannot be empty whitespace")) + : Promise.resolve(), + }, + ]} >