diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/AwsSigV4Fields.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/AwsSigV4Fields.tsx index d4ae537bffa..09016bda266 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/AwsSigV4Fields.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/AwsSigV4Fields.tsx @@ -1,7 +1,25 @@ import React from "react"; -import { Form, Input, Tooltip } from "antd"; +import { Input, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; +import { requiredWhenSiblingSet, textControl } from "./mcpFieldRules"; + +const fieldClassName = "rounded-lg border-gray-300 focus:border-blue-500 focus:ring-blue-500"; + +const FieldLabel: React.FC<{ label: string; tooltip: string }> = ({ label, tooltip }) => ( + + {label} + + + + +); + +const ACCESS_KEY_PATH = ["credentials", "aws_access_key_id"] as const; +const SECRET_KEY_PATH = ["credentials", "aws_secret_access_key"] as const; + const AwsSigV4Fields: React.FC = () => ( <>

@@ -15,140 +33,120 @@ const AwsSigV4Fields: React.FC = () => ( View docs →

- - AWS Region - - - - - } + } name={["credentials", "aws_region_name"]} - rules={[{ required: true, message: "AWS region is required for SigV4 auth" }]} + required + rules={{ validate: { required: antdRequired("AWS region is required for SigV4 auth") } }} > - - - } + + - AWS Service Name - - - - + } name={["credentials", "aws_service_name"]} > - - - } + + - AWS Access Key ID - - - - + } - name={["credentials", "aws_access_key_id"]} - dependencies={[["credentials", "aws_secret_access_key"]]} - rules={[ - ({ getFieldValue }) => ({ - validator(_, value) { - const secretKey = getFieldValue(["credentials", "aws_secret_access_key"]); - if (secretKey && !value) { - return Promise.reject(new Error("Access Key ID is required when Secret Access Key is provided")); - } - return Promise.resolve(); - }, - }), - ]} + name={ACCESS_KEY_PATH} + rules={{ + deps: ["credentials.aws_secret_access_key"], + validate: { + pairedWithSecret: requiredWhenSiblingSet( + SECRET_KEY_PATH, + "Access Key ID is required when Secret Access Key is provided", + ), + }, + }} > - - - ( + + )} + + - AWS Secret Access Key - - - - + } - name={["credentials", "aws_secret_access_key"]} - dependencies={[["credentials", "aws_access_key_id"]]} - rules={[ - ({ getFieldValue }) => ({ - validator(_, value) { - const accessKeyId = getFieldValue(["credentials", "aws_access_key_id"]); - if (accessKeyId && !value) { - return Promise.reject(new Error("Secret Access Key is required when Access Key ID is provided")); - } - return Promise.resolve(); - }, - }), - ]} + name={SECRET_KEY_PATH} + rules={{ + deps: ["credentials.aws_access_key_id"], + validate: { + pairedWithAccessKey: requiredWhenSiblingSet( + ACCESS_KEY_PATH, + "Secret Access Key is required when Access Key ID is provided", + ), + }, + }} > - - - - AWS Session Token - - - - - } + {(control) => ( + + )} + + } name={["credentials", "aws_session_token"]} > - - - ( + + )} + + - AWS Role ARN - - - - + } name={["credentials", "aws_role_name"]} > - - - ( + + )} + + - AWS Session Name - - - - + } name={["credentials", "aws_session_name"]} > - - + {(control) => ( + + )} + ); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.integration.test.tsx index f08aacb5522..78d89b7e19e 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.integration.test.tsx @@ -2147,7 +2147,7 @@ describe("CreateMCPServer dcr_bridge toggle", () => { }); // Forcing dcr_bridge false for every non-client-forwarded auth type is covered in - // createServerPayload.test.ts. The two form-state cases below stay: they prove the Form.Item + // createServerPayload.test.ts. The two form-state cases below stay: they prove the field // unmounts on a switch away, and that the live value survives a client-forwarded swap. it("forces dcr_bridge: false when the auth type is switched away after toggling", async () => { @@ -2179,7 +2179,7 @@ describe("CreateMCPServer dcr_bridge toggle", () => { }); expect(getDcrToggle()).toHaveAttribute("aria-checked", "true"); - // The Form.Item is mounted in both client-forwarded modes, so switching between them keeps the + // The field is mounted in both client-forwarded modes, so switching between them keeps the // live toggle value rather than forcing it back to the default or to false. await selectAntOption("Authentication", "OAuth Delegate (client-supplied upstream token)"); await waitFor(() => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.permissions.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.permissions.integration.test.tsx new file mode 100644 index 00000000000..788be106d8a --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.permissions.integration.test.tsx @@ -0,0 +1,160 @@ +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 "@/components/networking"; +import CreateMCPServer from "./CreateMCPServer"; +import { selectAntOption } from "./testUtils"; + +vi.mock("@/components/networking", () => ({ + createMCPServer: vi.fn(), + fetchOpenAPIRegistry: vi.fn().mockResolvedValue({ apis: [] }), + registerMCPServer: vi.fn(), + storeMCPOAuthUserCredential: vi.fn().mockResolvedValue({}), + testMCPToolsListRequest: vi.fn().mockResolvedValue({ tools: [], error: null }), +})); + +vi.mock("@/utils/mcpTokenStore", () => ({ + setToken: vi.fn(), +})); + +vi.mock("./OpenAPIQuickPicker", () => ({ + default: () => null, +})); + +vi.mock("@/hooks/useMcpOAuthFlow", () => ({ + useMcpOAuthFlow: () => ({ + startOAuthFlow: vi.fn(), + status: "idle", + error: null, + tokenResponse: null, + reset: vi.fn(), + }), +})); + +vi.mock("./mcp_server_cost_config", () => ({ + 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"], +}; + +const getServerNameInput = () => document.getElementById("server_name") as HTMLInputElement; + +const switchFor = (labelText: string): HTMLElement => { + const label = screen.getByText(labelText); + const row = label.closest(".flex.items-start.justify-between"); + const control = row?.querySelector("button[role='switch']"); + if (control === null || control === undefined) { + throw new Error(`no switch found for "${labelText}"`); + } + return control as HTMLElement; +}; + +const fillMinimalHttpServer = async (name: string) => { + await selectAntOption("Transport Type", "Streamable HTTP"); + await waitFor(() => { + expect(screen.getByPlaceholderText("https://your-mcp-server.com")).toBeInTheDocument(); + }); + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), name); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + await selectAntOption("Authentication", "None"); +}; + +const submitAndReadPayload = async () => { + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Add MCP Server" })); + }); + await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1)); + return vi.mocked(networking.createMCPServer).mock.calls[0][1]; +}; + +const createdServer = { + server_id: "new-server-1", + server_name: "Perm_Server", + alias: "Perm_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", +}; + +describe("CreateMCPServer permission toggles reaching the payload", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(networking.createMCPServer).mockResolvedValue(createdServer); + }); + + it("sends the panel's untouched defaults rather than dropping the keys the panel owns", async () => { + render(); + await fillMinimalHttpServer("Perm_Server"); + + const payload = await submitAndReadPayload(); + + expect(payload.allow_all_keys).toBe(false); + expect(payload.available_on_public_internet).toBe(true); + }); + + it("sends allow_all_keys true once the operator turns the public-to-all-keys switch on", async () => { + render(); + await fillMinimalHttpServer("Perm_Server"); + + await act(async () => { + fireEvent.click(switchFor("Allow All LiteLLM Keys")); + }); + + const payload = await submitAndReadPayload(); + + expect(payload.allow_all_keys).toBe(true); + }); + + it("sends available_on_public_internet false when the operator restricts the server to the internal network", async () => { + render(); + await fillMinimalHttpServer("Perm_Server"); + + const internalOnly = switchFor("Internal network only"); + expect(internalOnly).toHaveAttribute("aria-checked", "false"); + + await act(async () => { + fireEvent.click(internalOnly); + }); + expect(internalOnly).toHaveAttribute("aria-checked", "true"); + + const payload = await submitAndReadPayload(); + + expect(payload.available_on_public_internet).toBe(false); + }); + + it("omits delegate_auth_to_upstream's true value on a none-auth server, whose gate never mounts that switch", async () => { + render(); + await fillMinimalHttpServer("Perm_Server"); + + expect(screen.getByText("Allow All LiteLLM Keys")).toBeInTheDocument(); + expect(screen.queryByText("Delegate auth to upstream (PKCE passthrough)")).not.toBeInTheDocument(); + + const payload = await submitAndReadPayload(); + + expect(payload.delegate_auth_to_upstream).toBe(false); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx index ed8d3bddc98..92bdd14754c 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx @@ -1,5 +1,6 @@ import React, { useState } from "react"; -import { Modal, Tooltip, Form, Select, Input as AntdInput, InputNumber, Collapse } from "antd"; +import { Modal, Tooltip, Select, Input as AntdInput, InputNumber, Collapse } from "antd"; +import { FormProvider, useForm, useWatch } from "react-hook-form"; import { InfoCircleOutlined } from "@ant-design/icons"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; @@ -49,6 +50,16 @@ import { validateMCPServerUrl, validateMCPServerName } from "./utils"; import { toast } from "@/lib/toast"; import { useMcpOAuthFlow } from "@/hooks/useMcpOAuthFlow"; import { useTestMCPConnection } from "@/hooks/useTestMCPConnection"; +import { + MountedFormField, + MountedFormProvider, + projectMountedValues, + useMountRegistry, + type MountedFormValues, +} from "@/components/common_components/MountedFormField"; +import { antdRequired, antdRules } from "@/components/common_components/antdFormRules"; +import { allFieldsValue, mountedPaths, resetFields, setFieldsValue, singleBranchChange } from "./mcpFormStore"; +import { numberControl, notOnlyWhitespace, selectControl, textControl } from "./mcpFieldRules"; import mcpLogo from "../../../../../public/assets/logos/mcp_logo.png"; export const mcpLogoImg = mcpLogo.src; @@ -76,6 +87,13 @@ const payloadErrorMessage = (result: Exclude = ({ userID, userRole, @@ -87,7 +105,8 @@ const CreateMCPServer: React.FC = ({ prefillData, onBackToDiscovery, }) => { - const [form] = Form.useForm(); + const form = useForm({ mode: "onChange", defaultValues: CREATE_DEFAULTS }); + const registry = useMountRegistry(); const [isLoading, setIsLoading] = useState(false); const [costConfig, setCostConfig] = useState({}); const [formValues, setFormValues] = useState>({}); @@ -136,6 +155,8 @@ const CreateMCPServer: React.FC = ({ enabled: true, }); + const authSectionMounted = transportType !== "stdio" && transportType !== ""; + const watchedAuthType = useWatch({ control: form.control, name: "auth_type" }) as string | undefined; const authType = formValues.auth_type as string | undefined; const shouldShowAuthValueField = authType ? AUTH_TYPES_REQUIRING_AUTH_VALUE.includes(authType) : false; const isOAuthAuthType = authType === AUTH_TYPE.OAUTH2; @@ -147,7 +168,7 @@ const CreateMCPServer: React.FC = ({ const persistCreateUiState = () => { writeCreateUiSnapshot({ modalVisible: isModalVisible, - formValues: form.getFieldsValue(true), + formValues: allFieldsValue(form), transportType, costConfig, allowedTools, @@ -170,11 +191,11 @@ const CreateMCPServer: React.FC = ({ // Merge the ref-held DCR client so a re-authorize reuses the registered client instead of // re-registering; the form store itself never holds the DCR client (see onTokenReceived). getCredentials: () => ({ - ...((form.getFieldValue("credentials") as Record | undefined) ?? {}), + ...((allFieldsValue(form).credentials as Record | undefined) ?? {}), ...(dcrClientRef.current ?? {}), }), getTemporaryPayload: () => { - const values = form.getFieldsValue(true); + const values = allFieldsValue(form); const transport = values.transport || transportType; // For OpenAPI transport the form has spec_path instead of url. // We pass the spec_path as url so the temp-session endpoint has something @@ -218,12 +239,12 @@ const CreateMCPServer: React.FC = ({ return; } - if (isClientForwardedTokenMode(form.getFieldValue("auth_type"))) { + if (isClientForwardedTokenMode(allFieldsValue(form).auth_type)) { // Browser-only modes: the token is held in local state (oauthAccessToken) for tool preview // and committed to sessionStorage on submit; it must never be written into form.credentials, // which would persist it as server-level credentials on the created server row. Mirrors the // edit form's onTokenReceived early return. - setAuthorizedIdentity(getOAuthAuthorizationIdentity(form.getFieldsValue(true))); + setAuthorizedIdentity(getOAuthAuthorizationIdentity(allFieldsValue(form))); toast.success( "Token held for this browser session. Tools can now be previewed and configured; the token is not saved to LiteLLM.", ); @@ -240,7 +261,7 @@ const CreateMCPServer: React.FC = ({ } : null; - const current = (form.getFieldValue("credentials") as Record | undefined) ?? {}; + const current = (allFieldsValue(form).credentials as Record | undefined) ?? {}; const nextCredentials = { ...(preservedAdminCredentials(current) ?? {}), ...(current.scopes !== undefined && { scopes: current.scopes }), @@ -252,10 +273,10 @@ const CreateMCPServer: React.FC = ({ // Path-replace (not deep-merge) so a re-authorize with fewer token fields does not leave stale // siblings from the previous token behind; the admin-typed client keys and scopes are carried // explicitly above. - form.setFieldValue("credentials", nextCredentials); + form.setValue("credentials", nextCredentials); // Capture the identity AFTER writing the token so the held token is not spuriously invalidated by // its own credential write. - setAuthorizedIdentity(getOAuthAuthorizationIdentity(form.getFieldsValue(true))); + setAuthorizedIdentity(getOAuthAuthorizationIdentity(allFieldsValue(form))); toast.success("OAuth authorization successful! Please click 'Create MCP Server' to save the configuration."); }, @@ -277,10 +298,10 @@ const CreateMCPServer: React.FC = ({ // Capture the admin-typed app before resetFields destroys it, then re-apply it: the app is // upstream-scoped config, not minted material, so it survives every invalidation (the token is // what gets discarded). Token-shaped keys are excluded by the helper's key filter. - const keptAdminCredentials = preservedAdminCredentials(form.getFieldValue("credentials")); - form.resetFields([...CLEARED_ON_INVALIDATION]); + const keptAdminCredentials = preservedAdminCredentials(allFieldsValue(form).credentials); + resetFields(form, [...CLEARED_ON_INVALIDATION]); if (keptAdminCredentials) { - form.setFieldsValue({ credentials: keptAdminCredentials }); + setFieldsValue(form, { credentials: keptAdminCredentials }); } // Re-apply the in-flight edit last; rc-field-form deep-merges nested objects, so a changed // credentials sub-field composes with the preserved sibling instead of replacing the object. @@ -288,7 +309,7 @@ const CreateMCPServer: React.FC = ({ CLEARED_ON_INVALIDATION.filter((key) => key in changedValues).map((key) => [key, changedValues[key]]), ); if (Object.keys(preserved).length > 0) { - form.setFieldsValue(preserved); + setFieldsValue(form, preserved); } }; @@ -337,7 +358,7 @@ const CreateMCPServer: React.FC = ({ // wait until transportType state catches up so the URL field is mounted return; } - form.setFieldsValue(pendingRestoredValues.values); + setFieldsValue(form, pendingRestoredValues.values); setFormValues(pendingRestoredValues.values); setPendingRestoredValues(null); }, [pendingRestoredValues, form, transportType]); @@ -381,11 +402,20 @@ const CreateMCPServer: React.FC = ({ prefillValues.url = prefillData.url; } - form.setFieldsValue(prefillValues); + setFieldsValue(form, prefillValues); setFormValues(prefillValues); setAliasManuallyEdited(false); }, [isModalVisible, prefillData, form]); + const handleSubmit = async (event: React.FormEvent) => { + event.preventDefault(); + const isValid = await form.trigger(mountedPaths(registry) as string[]); + if (!isValid) { + return; + } + await handleCreate(projectMountedValues(registry, form.getValues)); + }; + const handleCreate = async (values: Record) => { const built = buildCreateServerPayload(values, { transportType, @@ -446,7 +476,7 @@ const CreateMCPServer: React.FC = ({ description: "Once an admin approves it, the server will appear in your MCP Servers list.", }); } - form.resetFields(); + form.reset(CREATE_DEFAULTS); setCostConfig({}); clearTools(); setAllowedTools([]); @@ -466,7 +496,7 @@ const CreateMCPServer: React.FC = ({ // state const handleCancel = () => { - form.resetFields(); + form.reset(CREATE_DEFAULTS); setCostConfig({}); clearTools(); setAllowedTools([]); @@ -489,11 +519,11 @@ const CreateMCPServer: React.FC = ({ ? { url: undefined, command: undefined, args: undefined, env: undefined } : { spec_path: undefined, command: undefined, args: undefined, env: undefined }; - form.setFieldsValue(transportValues); - if (isHeldOAuthTokenStale(form.getFieldsValue(true), authorizedIdentity)) { + setFieldsValue(form, transportValues); + if (isHeldOAuthTokenStale(allFieldsValue(form), authorizedIdentity)) { clearHeldOAuthToken(); } - setFormValues(form.getFieldsValue(true)); + setFormValues(allFieldsValue(form)); }; // Generate options with existing groups and potential new group @@ -532,7 +562,7 @@ const CreateMCPServer: React.FC = ({ React.useEffect(() => { if (!aliasManuallyEdited && formValues.server_name) { const normalized = formValues.server_name.replace(/\s+/g, "_"); - form.setFieldsValue({ alias: normalized }); + setFieldsValue(form, { alias: normalized }); setFormValues((prev) => ({ ...prev, alias: normalized })); } }, [formValues.server_name]); @@ -549,7 +579,7 @@ const CreateMCPServer: React.FC = ({ const wasVisible = wasModalVisibleRef.current; wasModalVisibleRef.current = isModalVisible; if (!isModalVisible && wasVisible) { - form.resetFields(); + form.reset(CREATE_DEFAULTS); setFormValues({}); setOauthAccessToken(null); clearTools(); @@ -582,19 +612,35 @@ const CreateMCPServer: React.FC = ({ const upstreamChanged = ["url", "spec_path", "issuer", "authorization_url", "token_url", "registration_url"].some( (key) => key in changedValues, ); - const hasDeclaredApp = preservedDeclaredAppCredentials(form.getFieldValue("credentials")) !== undefined; + const hasDeclaredApp = preservedDeclaredAppCredentials(allFieldsValue(form).credentials) !== undefined; if (upstreamChanged && hasDeclaredApp) { setAppMayNotMatchUpstream(true); } } - if (isHeldOAuthTokenStale(form.getFieldsValue(true), authorizedIdentity)) { + if (isHeldOAuthTokenStale(allFieldsValue(form), authorizedIdentity)) { clearHeldOAuthToken(changedValues); - setFormValues(form.getFieldsValue(true)); + setFormValues(allFieldsValue(form)); return; } setFormValues(allValues); }; + const valuesChangeRef = React.useRef(handleFormValuesChange); + valuesChangeRef.current = handleFormValuesChange; + + React.useEffect(() => { + const subscription = form.watch((values, { name, type }) => { + if (type !== "change" || name === undefined) { + return; + } + valuesChangeRef.current( + singleBranchChange(name, values as MountedFormValues), + projectMountedValues(registry, form.getValues), + ); + }); + return () => subscription.unsubscribe(); + }, [form, registry]); + // rendering return ( = ({ }} >
-
- {!isAdmin && ( -
- Your submission will be sent for admin review. Once approved, the server will appear in your MCP Servers - list. The request must be made with a team-scoped API key. -
- )} -
- - MCP Server Name - - - - - } - name="server_name" - rules={[ - { required: false, message: "Please enter a server name" }, - { validator: (_, value) => validateMCPServerName(value) }, - ]} - > - - + + + + {!isAdmin && ( +
+ Your submission will be sent for admin review. Once approved, the server will appear in your MCP + Servers list. The request must be made with a team-scoped API key. +
+ )} +
+ + MCP Server Name + + + + + } + name="server_name" + rules={{ validate: antdRules({ validator: (_, value) => validateMCPServerName(value) }) }} + > + {(control) => ( + + )} + - - Alias - - - - - } - name="alias" - rules={[{ required: false }, { validator: (_, value) => validateMCPServerName(value) }]} - > - setAliasManuallyEdited(true)} - /> - + + Alias + + + + + } + name="alias" + rules={{ validate: antdRules({ validator: (_, value) => validateMCPServerName(value) }) }} + > + {(control) => ( + { + control.onChange(event); + setAliasManuallyEdited(true); + }} + /> + )} + - Description} - name="description" - rules={[ - { - required: false, - message: "Please enter a server description", - }, - ]} - > - - + Description} + name="description" + > + {(control) => ( + + )} + - + - GitHub / Source URL} - name="source_url" - > - - + GitHub / Source URL} + name="source_url" + > + {(control) => ( + + )} + - Transport Type} - name="transport" - rules={[{ required: true, message: "Please select a transport type" }]} - > - - + Transport Type} + name="transport" + required + rules={{ validate: { required: antdRequired("Please select a transport type") } }} + > + {(control) => ( + + )} + - {/* URL field - only show for HTTP and SSE */} - {(transportType === "http" || transportType === "sse") && ( - MCP Server URL} - name="url" - rules={[ - { required: true, message: "Please enter a server URL" }, - { validator: (_, value) => validateMCPServerUrl(value) }, - ]} - > - - - )} + {/* URL field - only show for HTTP and SSE */} + {(transportType === "http" || transportType === "sse") && ( + MCP Server URL} + name="url" + required + rules={{ + validate: { + required: antdRequired("Please enter a server URL"), + ...antdRules({ validator: (_, value) => validateMCPServerUrl(value) }), + }, + }} + > + {(control) => ( + + )} + + )} - {/* OpenAPI: logo picker + spec URL input */} - {transportType === TRANSPORT.OPENAPI && ( - - handleFormValuesChange(updates, { ...form.getFieldsValue(true), ...updates }) - } - onKeyToolsChange={setKeyTools} - onLogoUrlChange={setLogoUrl} - onOAuthDocsUrlChange={setOauthDocsUrl} - /> - )} + {/* OpenAPI: logo picker + spec URL input */} + {transportType === TRANSPORT.OPENAPI && ( + + handleFormValuesChange(updates, { ...allFieldsValue(form), ...updates }) + } + onKeyToolsChange={setKeyTools} + onLogoUrlChange={setLogoUrl} + onOAuthDocsUrlChange={setOauthDocsUrl} + /> + )} - {/* BYOK toggle - only for OpenAPI */} - {transportType === TRANSPORT.OPENAPI && } + {/* BYOK toggle - only for OpenAPI */} + {transportType === TRANSPORT.OPENAPI && } - - Max Concurrent Requests (optional) - - - - - } - name="max_concurrent_requests" - > - - + + Max Concurrent Requests (optional) + + + + + } + name="max_concurrent_requests" + > + {(control) => ( + + )} + - {/* Authentication - show for HTTP, SSE, and OpenAPI */} - {transportType !== "stdio" && transportType !== "" && ( - Authentication, - children: ( - <> - - - + {/* Authentication - show for HTTP, SSE, and OpenAPI */} + {transportType !== "stdio" && transportType !== "" && ( + Authentication, + children: ( + <> + + {(control) => ( + + )} + - + - - - {shouldShowAuthValueField && ( - - Authentication Value - - - - - } - name={["credentials", "auth_value"]} - rules={[ - { - validator: (_, value) => - value && typeof value === "string" && value.trim() === "" - ? Promise.reject(new Error("Authentication value cannot be empty whitespace")) - : Promise.resolve(), - }, - ]} - > - - - )} - {isOAuthAuthType && ( - - )} + {shouldShowAuthValueField && ( + + Authentication Value + + + + + } + name={["credentials", "auth_value"]} + rules={{ + validate: { + notWhitespace: notOnlyWhitespace("Authentication value cannot be empty whitespace"), + }, + }} + > + {(control) => ( + + )} + + )} - {isTokenExchangeAuthType && } + {isOAuthAuthType && ( + + )} - {isIdJagAuthType && } - - ), - }, - ]} - /> - )} + {isTokenExchangeAuthType && } - {transportType !== "stdio" && transportType !== "" && isAwsSigV4AuthType && } + {isIdJagAuthType && } + + ), + }, + ]} + /> + )} - {/* Stdio Configuration - only show for stdio transport */} - -
+ {transportType !== "stdio" && transportType !== "" && isAwsSigV4AuthType && } - {/* Environment Variables Section */} -
- -
+ {/* Stdio Configuration - only show for stdio transport */} + +
- {/* Permission Management / Access Control Section */} -
- -
+ {/* Environment Variables Section */} +
+ +
- {/* Connection Status Section */} -
- -
+ {/* Permission Management / Access Control Section */} +
+ +
- {/* Tool Configuration Section */} -
- setHasToolAllowlistInteraction(true)} - toolNameToDisplayName={toolNameToDisplayName} - toolNameToDescription={toolNameToDescription} - onToolNameToDisplayNameChange={setToolNameToDisplayName} - onToolNameToDescriptionChange={setToolNameToDescription} - keyTools={keyTools} - externalTools={tools} - externalIsLoading={isLoadingTools} - externalError={toolsError} - externalErrorStatus={toolsErrorStatus} - externalCanFetch={canFetchTools} - /> -
+ {/* Connection Status Section */} +
+ +
- {/* Cost Configuration Section */} -
- allowedTools.includes(tool.name))} - disabled={false} - /> -
+ {/* Tool Configuration Section */} +
+ setHasToolAllowlistInteraction(true)} + toolNameToDisplayName={toolNameToDisplayName} + toolNameToDescription={toolNameToDescription} + onToolNameToDisplayNameChange={setToolNameToDisplayName} + onToolNameToDescriptionChange={setToolNameToDescription} + keyTools={keyTools} + externalTools={tools} + externalIsLoading={isLoadingTools} + externalError={toolsError} + externalErrorStatus={toolsErrorStatus} + externalCanFetch={canFetchTools} + /> +
-
- - -
-
+ {/* Cost Configuration Section */} +
+ allowedTools.includes(tool.name))} + disabled={false} + /> +
+ +
+ + +
+ + +
); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/DcrBridgeToggle.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/DcrBridgeToggle.tsx index 49c182aa6be..35b23f9873c 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/DcrBridgeToggle.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/DcrBridgeToggle.tsx @@ -1,16 +1,19 @@ import React from "react"; -import { Form, Switch, Tooltip } from "antd"; +import { Switch, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; + +import { MountedFormField } from "@/components/common_components/MountedFormField"; import { isClientForwardedTokenMode } from "@/components/mcp_tools/types"; +import { switchControl } from "./mcpFieldRules"; /** * DCR-bridge toggle for the client-forwarded token modes (true_passthrough / * oauth_delegate); self-gates to those two auth types and renders nothing * otherwise. When on, OAuth-only clients like Claude Desktop can register and * sign in through the gateway; when off, the gateway relays the upstream - * server's own OAuth metadata instead. `initialChecked` seeds the antd - * Form.Item `initialValue` (not the Switch's DOM defaultChecked): the create - * form defaults it on, the edit form seeds it from the stored value. + * server's own OAuth metadata instead. `initialChecked` seeds the field's + * default value (not the Switch's DOM defaultChecked): the create form defaults + * it on, the edit form seeds it from the stored value. */ export default function DcrBridgeToggle({ authType, @@ -21,7 +24,7 @@ export default function DcrBridgeToggle({ }) { if (!isClientForwardedTokenMode(authType)) return null; return ( - Gateway-hosted sign-in (DCR bridge) @@ -31,10 +34,9 @@ export default function DcrBridgeToggle({ } name="dcr_bridge" - valuePropName="checked" - initialValue={initialChecked} + defaultValue={initialChecked} > - - + {(control) => } + ); } diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/EnvVarsSection.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/EnvVarsSection.test.tsx new file mode 100644 index 00000000000..a57bde7eb9d --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/EnvVarsSection.test.tsx @@ -0,0 +1,86 @@ +import React from "react"; +import { describe, it, expect, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { FormProvider, useForm } from "react-hook-form"; + +import { + MountedFormProvider, + projectMountedValues, + useMountRegistry, + type MountedFormValues, +} from "@/components/common_components/MountedFormField"; +import EnvVarsSection from "./EnvVarsSection"; + +const renderSection = (defaultValues: MountedFormValues) => { + const onFinish = vi.fn(); + const Harness: React.FC = () => { + const form = useForm({ mode: "onChange", defaultValues }); + const registry = useMountRegistry(); + return ( + + +
{ + event.preventDefault(); + onFinish(projectMountedValues(registry, form.getValues)); + }} + > + + + +
+ + ); + }; + render(); + return onFinish; +}; + +describe("EnvVarsSection", () => { + it("submits a per-user row whole, keeping the value key whose input the scope hides", async () => { + const onFinish = renderSection({ + env_vars: [{ name: "DB_USER", value: "admin", scope: "user", description: "Your DB username" }], + }); + + expect(screen.queryByPlaceholderText("e.g. postgresql")).not.toBeInTheDocument(); + await userEvent.click(screen.getByText("Submit")); + + expect(onFinish).toHaveBeenCalledWith( + expect.objectContaining({ + env_vars: [{ name: "DB_USER", value: "admin", scope: "user", description: "Your DB username" }], + }), + ); + }); + + it("submits an empty env_vars key when the list has no rows, rather than dropping the key", async () => { + const onFinish = renderSection({ env_vars: [] }); + + await userEvent.click(screen.getByText("Submit")); + + expect(onFinish.mock.calls[0][0]).toHaveProperty("env_vars", []); + }); + + it("carries a row added after mount into the submitted list, scoped global without the user picking one", async () => { + const onFinish = renderSection({ env_vars: [] }); + + await userEvent.click(screen.getByText("Add Variable")); + await userEvent.type(screen.getByPlaceholderText("e.g. DB_PROTOCOL"), "DB_PROTOCOL"); + await userEvent.type(screen.getByPlaceholderText("e.g. postgresql"), "postgresql"); + await userEvent.click(screen.getByText("Submit")); + + expect(onFinish).toHaveBeenCalledWith( + expect.objectContaining({ + env_vars: [expect.objectContaining({ name: "DB_PROTOCOL", value: "postgresql", scope: "global" })], + }), + ); + }); + + it("rejects a variable name that starts with a digit", async () => { + renderSection({ env_vars: [{ name: "", value: "", scope: "global", description: "" }] }); + + await userEvent.type(screen.getByPlaceholderText("e.g. DB_PROTOCOL"), "9LIVES"); + + expect(await screen.findByText("Use letters, digits, underscores; cannot start with a digit.")).toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/EnvVarsSection.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/EnvVarsSection.tsx index fbaacc40263..539a06910c1 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/EnvVarsSection.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/EnvVarsSection.tsx @@ -1,6 +1,16 @@ import React from "react"; -import { Form, Input, Select, Button, Tooltip, Typography } from "antd"; +import { Input, Select, Button, Tooltip, Typography } from "antd"; import { InfoCircleOutlined, MinusCircleOutlined, PlusOutlined } from "@ant-design/icons"; +import { useFieldArray, useFormContext, useWatch } from "react-hook-form"; + +import { + MountedFormField, + useMountedName, + type MountedFormValues, +} from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; +import { matchesPattern, selectControl, textControl } from "./mcpFieldRules"; +import { listControl } from "./mcpFormStore"; const { Text } = Typography; @@ -9,6 +19,8 @@ const SCOPE_OPTIONS = [ { value: "user", label: "Per-user" }, ]; +const VARIABLE_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/; + /** * Form section for admin-configured MCP environment variables. * @@ -20,6 +32,10 @@ const SCOPE_OPTIONS = [ * The parent form reads the ``env_vars`` field from the form values. */ const EnvVarsSection: React.FC = () => { + const { control } = useFormContext(); + const { fields, append, remove } = useFieldArray({ control: listControl(control), name: "env_vars" }); + useMountedName("env_vars"); + return (
@@ -48,60 +64,52 @@ const EnvVarsSection: React.FC = () => { - - {(fields, { add, remove }) => ( -
- {fields.length > 0 && ( -
-
Variable Name
-
Value / Description
-
Scope
-
-
- )} - {fields.map(({ key, name, ...restField }) => ( -
- - - -
- -
- - + )} + +
+ +
+ + {(control) => - - - Hint - - - } - placeholder="e.g. Your DB username" - styles={{ input: { color: "#9ca3af" } }} - /> - + + {(control) => ( + + + + Hint + + + } + placeholder="e.g. Your DB username" + styles={{ input: { color: "#9ca3af" } }} + /> + )} + ); } return ( - - - + + {(control) => } + ); }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/IdJagFormFields.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/IdJagFormFields.tsx index e8730a5b974..9a6f4ab571f 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/IdJagFormFields.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/IdJagFormFields.tsx @@ -1,7 +1,11 @@ import React from "react"; -import { Form, Input, Select, Tooltip } from "antd"; +import { Input, Select, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; +import { requiredUnlessSiblingSet, selectControl, textControl } from "./mcpFieldRules"; + interface IdJagFormFieldsProps { isEditing?: boolean; } @@ -17,12 +21,16 @@ const FieldLabel: React.FC<{ label: string; tooltip: string }> = ({ label, toolt ); +const PRIVATE_KEY_PATH = ["credentials", "client_private_key"] as const; + const IdJagFormFields: React.FC = ({ isEditing = false }) => { const placeholderSuffix = isEditing ? " (leave blank to keep existing)" : ""; + const requiredWhenCreating = (message: string) => + isEditing ? undefined : { validate: { required: antdRequired(message) } }; return ( <> - = ({ isEditing = false }) /> } name="token_exchange_endpoint" - rules={[{ required: !isEditing, message: "The org token endpoint is required for ID-JAG" }]} + required={!isEditing} + rules={requiredWhenCreating("The org token endpoint is required for ID-JAG")} > - - - ( + + )} + + = ({ isEditing = false }) /> } name={["credentials", "id_jag_resource_token_endpoint"]} - rules={[{ required: !isEditing, message: "The resource token endpoint is required for ID-JAG" }]} + required={!isEditing} + rules={requiredWhenCreating("The resource token endpoint is required for ID-JAG")} > - - - ( + + )} + + } name={["credentials", "client_id"]} - rules={[{ required: !isEditing, message: "Client ID is required for ID-JAG" }]} + required={!isEditing} + rules={requiredWhenCreating("Client ID is required for ID-JAG")} > - - - ( + + )} + + = ({ isEditing = false }) /> } name={["credentials", "client_secret"]} - dependencies={[["credentials", "client_private_key"]]} - rules={[ - ({ getFieldValue }) => ({ - validator: (_, value) => { - if (isEditing || value || getFieldValue(["credentials", "client_private_key"])) { - return Promise.resolve(); + rules={ + isEditing + ? undefined + : { + deps: ["credentials.client_private_key"], + validate: { + secretOrPrivateKey: requiredUnlessSiblingSet( + PRIVATE_KEY_PATH, + "Provide either a client secret or a client private key", + ), + }, } - return Promise.reject(new Error("Provide either a client secret or a client private key")); - }, - }), - ]} + } > - - - ( + + )} + + } - name={["credentials", "client_private_key"]} + name={PRIVATE_KEY_PATH} > - - - ( + + )} + + = ({ isEditing = false }) } name={["credentials", "client_private_key_id"]} > - - - } + + = ({ isEditing = false }) } name={["credentials", "client_assertion_signing_alg"]} > - - - } + + = ({ isEditing = false }) } name="audience" > - - - ( + + )} + + = ({ isEditing = false }) } name={["credentials", "id_jag_resource"]} > - - - ( + + )} + + = ({ isEditing = false }) } name="subject_token_type" > - - - ( + + )} + + } name={["credentials", "scopes"]} > - + )} + ); }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.test.tsx index ee6aee86a4d..7d36872cee8 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.test.tsx @@ -1,10 +1,10 @@ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect } from "vitest"; -import { Form } from "antd"; import MCPPermissionManagement from "./MCPPermissionManagement"; +import { renderInMcpForm } from "./McpFormTestHarness"; const defaultProps = { availableAccessGroups: [], @@ -12,6 +12,7 @@ const defaultProps = { searchValue: "", setSearchValue: () => {}, getAccessGroupOptions: () => [], + mountedAuthType: undefined, }; describe("MCPPermissionManagement", () => { @@ -24,22 +25,8 @@ describe("MCPPermissionManagement", () => { return user; }; - const renderWithForm = (props = {}) => { - const Wrapper: React.FC = ({ children }) => { - const [form] = Form.useForm(); - return ( -
- {children} -
- ); - }; - - return render( - - - , - ); - }; + const renderWithForm = (props = {}) => + renderInMcpForm(, { allow_all_keys: false }); it("should default allow_all_keys switch to unchecked for new servers", async () => { renderWithForm(); @@ -51,27 +38,15 @@ describe("MCPPermissionManagement", () => { expect(toggle).not.toBeChecked(); }); - const renderWithInitialValues = (initialValues: Record, props = {}) => { - const Wrapper: React.FC = ({ children }) => { - const [form] = Form.useForm(); - return ( -
- {/* In the real app auth_type is registered by the parent form; the - component only watches it. Register a hidden field here so - Form.useWatch("auth_type") resolves the initial value. */} - - {children} -
- ); - }; - return render( - - - , + const renderWithInitialValues = (initialValues: Record, props = {}) => + renderInMcpForm( + , + initialValues, ); - }; it("shows only the oauth2 PKCE-delegation toggle for oauth2 servers", async () => { renderWithInitialValues({ allow_all_keys: false, auth_type: "oauth2" }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx index aae13d4b467..3711140562f 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx @@ -1,7 +1,17 @@ import React, { useEffect } from "react"; -import { Alert, Form, Select, Tooltip, Collapse, Input, Space, Button, Switch } from "antd"; +import { Alert, Select, Tooltip, Collapse, Input, Space, Button, Switch } from "antd"; import { InfoCircleOutlined, MinusCircleOutlined, PlusOutlined } from "@ant-design/icons"; +import { useFieldArray, useFormContext, useWatch } from "react-hook-form"; import { MCPServer, AUTH_TYPE } from "@/components/mcp_tools/types"; +import { + MountedFormField, + useMountedName, + type MountedFormValues, +} from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; +import { Field, FieldLabel } from "@/components/shared/form/field"; +import { invertedSwitchControl, selectControl, switchControl, textControl } from "./mcpFieldRules"; +import { listControl } from "./mcpFormStore"; const { Panel } = Collapse; interface MCPPermissionManagementProps { @@ -13,20 +23,79 @@ interface MCPPermissionManagementProps { value: string; label: React.ReactNode; }>; + /** + * The auth type as seen through the gate that mounts the auth_type field. + * Callers pass undefined whenever that field is unmounted, because both + * toggles below are mounted from this value and the payload only carries + * what is mounted. + */ + mountedAuthType: string | null | undefined; } +const StaticHeadersFieldArray: React.FC = () => { + const { control } = useFormContext(); + const { fields, append, remove } = useFieldArray({ control: listControl(control), name: "static_headers" }); + useMountedName("static_headers"); + + return ( +
+ {fields.map((item, index) => ( + + + {(headerControl) => ( + + )} + + + {(valueControl) => ( + + )} + + remove(index)} + className="text-gray-500 hover:text-red-500 cursor-pointer" + /> + + ))} + +
+ ); +}; + const MCPPermissionManagement: React.FC = ({ availableAccessGroups, mcpServer, searchValue, setSearchValue, getAccessGroupOptions, + mountedAuthType, }) => { - const form = Form.useFormInstance(); - const watchedAuthType = Form.useWatch("auth_type", form); - const isOAuth2 = watchedAuthType === AUTH_TYPE.OAUTH2; - const isNoneAuth = watchedAuthType === AUTH_TYPE.NONE || watchedAuthType == null; - const watchedExtraHeaders = Form.useWatch("extra_headers", form); + const { setValue } = useFormContext(); + const isOAuth2 = mountedAuthType === AUTH_TYPE.OAUTH2; + const isNoneAuth = mountedAuthType === AUTH_TYPE.NONE || mountedAuthType == null; + const watchedExtraHeaders = useWatch({ name: "extra_headers" }); const hasAuthorizationHeader = Array.isArray(watchedExtraHeaders) && watchedExtraHeaders.some((h) => typeof h === "string" && h.toLowerCase() === "authorization"); @@ -39,8 +108,8 @@ const MCPPermissionManagement: React.FC = ({ // Kept as separate flags so neither silently implies the other and existing // oauth2 servers can't regress into pass-through behavior. const canEnableOAuthPassthrough = isNoneAuth && hasAuthorizationHeader; - const watchedDelegateAuth = Form.useWatch("delegate_auth_to_upstream", form); - const watchedPublicInternet = Form.useWatch("available_on_public_internet", form); + const watchedDelegateAuth = useWatch({ name: "delegate_auth_to_upstream" }); + const watchedPublicInternet = useWatch({ name: "available_on_public_internet" }); const showInternalDelegatePkceWarning = isOAuth2 && watchedDelegateAuth === true && watchedPublicInternet === false; // Set initial values when mcpServer changes @@ -51,10 +120,10 @@ const MCPPermissionManagement: React.FC = ({ header, value: value != null ? String(value) : "", })); - form.setFieldValue("static_headers", staticHeaders); + setValue("static_headers", staticHeaders); } if (Array.isArray(mcpServer.env_vars) && mcpServer.env_vars.length > 0) { - form.setFieldValue( + setValue( "env_vars", mcpServer.env_vars.map((entry) => ({ name: entry.name, @@ -65,41 +134,41 @@ const MCPPermissionManagement: React.FC = ({ ); } if (typeof mcpServer.allow_all_keys === "boolean") { - form.setFieldValue("allow_all_keys", mcpServer.allow_all_keys); + setValue("allow_all_keys", mcpServer.allow_all_keys); } if (typeof mcpServer.available_on_public_internet === "boolean") { - form.setFieldValue("available_on_public_internet", mcpServer.available_on_public_internet); + setValue("available_on_public_internet", mcpServer.available_on_public_internet); } if (typeof mcpServer.delegate_auth_to_upstream === "boolean") { - form.setFieldValue("delegate_auth_to_upstream", mcpServer.delegate_auth_to_upstream); + setValue("delegate_auth_to_upstream", mcpServer.delegate_auth_to_upstream); } if (typeof mcpServer.oauth_passthrough === "boolean") { - form.setFieldValue("oauth_passthrough", mcpServer.oauth_passthrough); + setValue("oauth_passthrough", mcpServer.oauth_passthrough); } } else { - form.setFieldValue("allow_all_keys", false); - form.setFieldValue("available_on_public_internet", true); - form.setFieldValue("delegate_auth_to_upstream", false); - form.setFieldValue("oauth_passthrough", false); + setValue("allow_all_keys", false); + setValue("available_on_public_internet", true); + setValue("delegate_auth_to_upstream", false); + setValue("oauth_passthrough", false); } - }, [mcpServer, form]); + }, [mcpServer, setValue]); // delegate_auth_to_upstream is only honored server-side for oauth2 servers. // Force it back to false whenever the user switches away from oauth2 so a // stale toggle value doesn't get persisted unexpectedly. useEffect(() => { if (!isOAuth2) { - form.setFieldValue("delegate_auth_to_upstream", false); + setValue("delegate_auth_to_upstream", false); } - }, [isOAuth2, form]); + }, [isOAuth2, setValue]); // oauth_passthrough is only honored for auth_type=none servers that forward // Authorization upstream. Force it back to false otherwise. useEffect(() => { if (!canEnableOAuthPassthrough) { - form.setFieldValue("oauth_passthrough", false); + setValue("oauth_passthrough", false); } - }, [canEnableOAuthPassthrough, form]); + }, [canEnableOAuthPassthrough, setValue]); return ( @@ -130,14 +199,9 @@ const MCPPermissionManagement: React.FC = ({ Enable if this server should be "public" to all keys.

- - - + + {(control) => } +
@@ -152,16 +216,9 @@ const MCPPermissionManagement: React.FC = ({ Turn on to restrict access to callers within your internal network only.

- ({ checked: !value })} - getValueFromEvent={(checked: boolean) => !checked} - initialValue={true} - className="mb-0" - > - - + + {(control) => } +
{isOAuth2 && ( @@ -177,14 +234,13 @@ const MCPPermissionManagement: React.FC = ({ Bypass LiteLLM auth so clients authenticate directly with the upstream OAuth MCP server.

- - - + {(control) => } +
)} @@ -202,14 +258,13 @@ const MCPPermissionManagement: React.FC = ({ upstream MCP server.

- - - + {(control) => } +
)} @@ -223,7 +278,7 @@ const MCPPermissionManagement: React.FC = ({ /> )} - MCP Access Groups @@ -235,21 +290,24 @@ const MCPPermissionManagement: React.FC = ({ name="mcp_access_groups" className="mb-4" > - (option?.value ?? "").toLowerCase().includes(input.toLowerCase())} + onSearch={(value) => setSearchValue(value)} + tokenSeparators={[","]} + options={getAccessGroupOptions()} + maxTagCount="responsive" + allowClear + /> + )} + - Extra Headers @@ -265,70 +323,34 @@ const MCPPermissionManagement: React.FC = ({ } name="extra_headers" > - 0 + ? `Currently: ${mcpServer.extra_headers.join(", ")}` + : "Enter header names (e.g., Authorization, X-Custom-Header)" + } + className="rounded-lg" + size="large" + tokenSeparators={[","]} + allowClear + /> + )} + - + Static Headers - } - required={false} - > - - {(fields, { add, remove }) => ( -
- {fields.map(({ key, name, ...restField }) => ( - - - - - - - - remove(name)} - className="text-gray-500 hover:text-red-500 cursor-pointer" - /> - - ))} - -
- )} - - + + +
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/McpFormTestHarness.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/McpFormTestHarness.tsx new file mode 100644 index 00000000000..0529c9c7c61 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/McpFormTestHarness.tsx @@ -0,0 +1,37 @@ +import * as React from "react"; +import { render, type RenderResult } from "@testing-library/react"; +import { FormProvider, useForm } from "react-hook-form"; + +import { + MountedFormProvider, + projectMountedValues, + useMountRegistry, + type MountedFormValues, +} from "@/components/common_components/MountedFormField"; + +export const McpFormHarness: React.FC<{ + defaultValues?: MountedFormValues; + onFinish?: (values: MountedFormValues) => void; + children: React.ReactNode; +}> = ({ defaultValues, onFinish, children }) => { + const form = useForm({ mode: "onChange", defaultValues }); + const registry = useMountRegistry(); + return ( + + +
{ + event.preventDefault(); + onFinish?.(projectMountedValues(registry, form.getValues)); + }} + > + {children} + +
+ + + ); +}; + +export const renderInMcpForm = (ui: React.ReactNode, defaultValues: MountedFormValues = {}): RenderResult => + render({ui}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.test.tsx index 48964490339..21bb2801e8c 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.test.tsx @@ -1,24 +1,8 @@ import React from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen, waitFor, act, fireEvent } from "@testing-library/react"; -import { Form } from "antd"; import OAuthFormFields from "./OAuthFormFields"; - -// ── helpers ────────────────────────────────────────────────────────────────── - -/** Minimal Ant Form wrapper so Form.Item registers correctly. */ -const WithForm: React.FC<{ children: React.ReactNode; onFinish?: (values: any) => void }> = ({ - children, - onFinish, -}) => { - const [form] = Form.useForm(); - return ( -
- {children} - -
- ); -}; +import { McpFormHarness as WithForm } from "./McpFormTestHarness"; // ── tests ───────────────────────────────────────────────────────────────────── diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.tsx index cbe6ac18d22..545efb910c4 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OAuthFormFields.tsx @@ -1,10 +1,13 @@ import React from "react"; -import { Form, Input as AntdInput, InputNumber, Select, Tooltip } from "antd"; +import { Input as AntdInput, InputNumber, Select, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { OAUTH_FLOW } from "@/components/mcp_tools/types"; +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; import TokenEndpointAuthMethodField from "./TokenEndpointAuthMethodField"; +import { numberControl, parsesAsJson, selectControl, textControl } from "./mcpFieldRules"; interface OAuthFlowStatus { startOAuthFlow: () => void; @@ -41,12 +44,14 @@ const FieldLabel: React.FC<{ label: string; tooltip: string }> = ({ label, toolt ); const UpstreamResourceField: React.FC = () => ( - } name={["credentials", "upstream_resource"]} > - - + {(control) => ( + + )} + ); const OAuthFormFields: React.FC = ({ @@ -57,11 +62,12 @@ const OAuthFormFields: React.FC = ({ docsUrl, }) => { const placeholderSuffix = isEditing ? " (leave blank to keep existing)" : ""; - const requiredWhenCreating = (message: string) => (isEditing ? [] : [{ required: true, message }]); + const requiredWhenCreating = (message: string) => + isEditing ? undefined : { validate: { required: antdRequired(message) } }; return ( <> - = ({ /> } name="oauth_flow_type" - {...(initialFlowType ? { initialValue: initialFlowType } : {})} + {...(initialFlowType ? { defaultValue: initialFlowType } : {})} > - - + {(control) => ( + + )} + {isM2M ? ( <> - } name={["credentials", "client_id"]} + required={!isEditing} rules={requiredWhenCreating("Client ID is required for M2M OAuth")} > - - - ( + + )} + + } name={["credentials", "client_secret"]} + required={!isEditing} rules={requiredWhenCreating("Client Secret is required for M2M OAuth")} > - - - ( + + )} + + } name="token_url" + required={!isEditing} rules={requiredWhenCreating("Token URL is required for M2M OAuth")} > - - + {(control) => ( + + )} + - = ({ } name={["credentials", "scopes"]} > - + )} + ) : ( <> - = ({ } name={["credentials", "client_id"]} > - - - ( + + )} + + = ({ } name={["credentials", "client_secret"]} > - - - ( + + )} + + = ({ } name={["credentials", "scopes"]} > - + )} + - = ({ } name="issuer" > - - - ( + + )} + + = ({ } name="authorization_url" > - - - ( + + )} + + } name="token_url" > - - + {(control) => ( + + )} + - = ({ } name="registration_url" > - - - ( + + )} + + = ({ /> } name="token_validation_json" - rules={[ - { - validator: (_: any, value: string) => { - if (!value || value.trim() === "") return Promise.resolve(); - try { - JSON.parse(value); - return Promise.resolve(); - } catch { - return Promise.reject(new Error("Must be valid JSON")); - } - }, - }, - ]} + rules={{ validate: { json: parsesAsJson("Must be valid JSON") } }} > - - - ( + + )} + + = ({ } name="token_storage_ttl_seconds" > - - + {(control) => ( + + )} + {oauthFlow && (

diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenAPIFormSection.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenAPIFormSection.tsx index 073780b359f..78c8bbe73a9 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenAPIFormSection.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenAPIFormSection.tsx @@ -1,12 +1,15 @@ import React, { useState } from "react"; -import { Form, Input, Tooltip } from "antd"; +import { Input, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; -import { FormInstance } from "antd/es/form"; import { AUTH_TYPE, OAUTH_FLOW } from "@/components/mcp_tools/types"; +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; import OpenAPIQuickPicker, { OpenAPIRegistryEntry, OpenAPIKeyTool } from "./OpenAPIQuickPicker"; +import { McpForm, resetFields, setFieldsValue } from "./mcpFormStore"; +import { textControl } from "./mcpFieldRules"; interface OpenAPIFormSectionProps { - form: FormInstance; + form: McpForm; accessToken: string | null; /** Called when a preset is selected so the parent can sync its formValues state. */ onValuesChange: (updates: Record) => void; @@ -47,13 +50,11 @@ const OpenAPIFormSection: React.FC = ({ updates.oauth_flow_type = OAUTH_FLOW.INTERACTIVE; updates.authorization_url = entry.oauth.authorization_url; updates.token_url = entry.oauth.token_url; - form.setFieldsValue(updates); + setFieldsValue(form, updates); onOAuthDocsUrlChange?.(entry.oauth.docs_url ?? null); } else { - // resetFields is required to visually clear Ant Design form fields — - // setFieldsValue with undefined silently skips undefined keys. - form.resetFields(["auth_type", "authorization_url", "token_url"]); - form.setFieldsValue(updates); + resetFields(form, ["auth_type", "authorization_url", "token_url"]); + setFieldsValue(form, updates); onOAuthDocsUrlChange?.(null); } onValuesChange(updates); @@ -63,7 +64,7 @@ const OpenAPIFormSection: React.FC = ({ <> - OpenAPI Spec URL @@ -73,20 +74,25 @@ const OpenAPIFormSection: React.FC = ({ } name="spec_path" - rules={[{ required: true, message: "Please enter an OpenAPI spec URL" }]} + required + rules={{ validate: { required: antdRequired("Please enter an OpenAPI spec URL") } }} > - { - // Clear the preset selection when the user manually edits the spec URL - // so stale suggested tools from a previous preset don't persist. - setSelectedPreset(null); - onKeyToolsChange?.([]); - onOAuthDocsUrlChange?.(null); - }} - /> - + {(control) => ( + { + control.onChange(event); + // Clear the preset selection when the user manually edits the spec URL + // so stale suggested tools from a previous preset don't persist. + setSelectedPreset(null); + onKeyToolsChange?.([]); + onOAuthDocsUrlChange?.(null); + }} + /> + )} + ); }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenApiByokFields.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenApiByokFields.tsx index 2ac4279e20a..83c841439f0 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenApiByokFields.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/OpenApiByokFields.tsx @@ -1,91 +1,101 @@ import React from "react"; -import { Form, Input, Select, Switch, Tooltip } from "antd"; +import { Input, Select, Switch, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; +import { useWatch } from "react-hook-form"; -const OpenApiByokFields: React.FC = () => ( - <> - - BYOK (Bring Your Own Key) - - - - - } - name="is_byok" - valuePropName="checked" - > - - +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { selectControl, switchControl, textControl } from "./mcpFieldRules"; - prev.is_byok !== cur.is_byok || prev.auth_type !== cur.auth_type}> - {({ getFieldValue }) => - getFieldValue("is_byok") ? ( - <> - {/* Auth format hint */} - {getFieldValue("auth_type") && getFieldValue("auth_type") !== "none" && ( -

- - - User keys will be sent as:{" "} - - {getFieldValue("auth_type") === "bearer_token" && "Authorization: Bearer {key}"} - {getFieldValue("auth_type") === "token" && "Authorization: token {key}"} - {getFieldValue("auth_type") === "api_key" && "x-api-key: {key}"} - {getFieldValue("auth_type") === "basic" && "Authorization: Basic {key}"} - {getFieldValue("auth_type") === "authorization" && "Authorization: {key}"} - - {!getFieldValue("auth_type") && "Set Authentication Type below to specify the format."} - -
- )} - {!getFieldValue("auth_type") && ( -
- - - Set the Authentication Type below to specify how user keys are sent (e.g., Bearer - Token, API Key header). - -
- )} - - Access Description - - - - - } - name="byok_description" - > +const AUTH_HEADER_FORMATS: Readonly> = { + bearer_token: "Authorization: Bearer {key}", + token: "Authorization: token {key}", + api_key: "x-api-key: {key}", + basic: "Authorization: Basic {key}", + authorization: "Authorization: {key}", +}; + +const OpenApiByokFields: React.FC = () => { + const isByok = Boolean(useWatch({ name: "is_byok" })); + const authType = useWatch({ name: "auth_type" }) as string | undefined; + const hasAuthType = Boolean(authType) && authType !== "none"; + + return ( + <> + + BYOK (Bring Your Own Key) + + + + + } + name="is_byok" + > + {(control) => } + + + {isByok && ( + <> + {hasAuthType && ( +
+ + + User keys will be sent as:{" "} + + {authType === undefined ? "" : AUTH_HEADER_FORMATS[authType]} + + +
+ )} + {!authType && ( +
+ + + Set the Authentication Type below to specify how user keys are sent (e.g., Bearer + Token, API Key header). + +
+ )} + + Access Description + + + + + } + name="byok_description" + > + {(control) => ( - - - ) : null - } - - -); + + API Key Help URL + + + + + } + name="byok_api_key_help_url" + > + {(control) => } + + + )} + + ); +}; export default OpenApiByokFields; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.test.tsx index 0a09ef3f856..a7577ccb781 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.test.tsx @@ -1,13 +1,10 @@ import React from "react"; import { describe, it, expect } from "vitest"; import { render, screen } from "@testing-library/react"; -import { Form } from "antd"; import PassthroughAuthorizeSection from "./PassthroughAuthorizeSection"; +import { McpFormHarness } from "./McpFormTestHarness"; -const WithForm: React.FC<{ children: React.ReactNode }> = ({ children }) => { - const [form] = Form.useForm(); - return
{children}
; -}; +const WithForm = McpFormHarness; const noopFlow = { startOAuthFlow: () => {}, status: "idle", error: null, tokenResponse: null }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.tsx index dc10f0f1392..375e1dfe515 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/PassthroughAuthorizeSection.tsx @@ -1,6 +1,8 @@ import React from "react"; -import { Button, Checkbox, Form, Input } from "antd"; +import { Button, Checkbox, Input } from "antd"; import DcrBridgeToggle from "./DcrBridgeToggle"; +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { textControl } from "./mcpFieldRules"; import { credentialAuthClass, isClientForwardedTokenMode } from "@/components/mcp_tools/types"; interface PassthroughOAuthFlow { @@ -81,27 +83,33 @@ export default function PassthroughAuthorizeSection({ and may not be valid. Update the client ID, or clear it to use dynamic client registration.

)} - OAuth Client ID (optional)} name={["credentials", "client_id"]} - extra={clientIdExtra} + help={clientIdExtra} > - - - ( + + )} + + OAuth Client Secret (optional)} name={["credentials", "client_secret"]} > - - + {(control) => ( + + )} + {isEditing && onRemoveStoredAppChange && ( onRemoveStoredAppChange(e.target.checked)}> diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/StdioConfiguration.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/StdioConfiguration.tsx index 476a5b61683..bb410aa19dc 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/StdioConfiguration.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/StdioConfiguration.tsx @@ -1,7 +1,11 @@ import React from "react"; -import { Form, Input, Tooltip } from "antd"; +import { Input, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; +import { parsesAsJson, textControl } from "./mcpFieldRules"; + interface StdioConfigurationProps { isVisible: boolean; /** @@ -11,37 +15,7 @@ interface StdioConfigurationProps { required?: boolean; } -const StdioConfiguration: React.FC = ({ isVisible, required = true }) => { - if (!isVisible) return null; - - return ( - - Stdio Configuration (JSON) - - - - - } - name="stdio_config" - rules={[ - ...(required ? [{ required: true, message: "Please enter stdio configuration" }] : []), - { - validator: (_, value) => { - if (!value) return Promise.resolve(); - try { - JSON.parse(value); - return Promise.resolve(); - } catch { - return Promise.reject("Please enter valid JSON"); - } - }, - }, - ]} - > - = ({ isVisible, requ } } } -}`} - rows={12} - className="rounded-lg border-gray-300 focus:border-blue-500 focus:ring-blue-500 font-mono text-sm" - /> - +}`; + +const StdioConfiguration: React.FC = ({ isVisible, required = true }) => { + if (!isVisible) return null; + + return ( + + Stdio Configuration (JSON) + + + + + } + name="stdio_config" + required={required} + rules={{ + validate: { + ...(required ? { required: antdRequired("Please enter stdio configuration") } : {}), + json: parsesAsJson("Please enter valid JSON"), + }, + }} + > + {(control) => ( + + )} + ); }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenEndpointAuthMethodField.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenEndpointAuthMethodField.tsx index c97ce96bfb1..38fa3573079 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenEndpointAuthMethodField.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenEndpointAuthMethodField.tsx @@ -1,7 +1,10 @@ import React from "react"; -import { Form, Select, Tooltip } from "antd"; +import { Select, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { selectControl } from "./mcpFieldRules"; + const TOKEN_ENDPOINT_AUTH_METHOD_OPTIONS = [ { value: "client_secret_basic", label: "Client Secret Basic" }, { value: "client_secret_post", label: "Client Secret Post" }, @@ -12,7 +15,7 @@ interface TokenEndpointAuthMethodFieldProps { } const TokenEndpointAuthMethodField: React.FC = ({ isEditing = false }) => ( - Token Endpoint Auth Method (optional) @@ -23,16 +26,19 @@ const TokenEndpointAuthMethodField: React.FC } name={["credentials", "token_endpoint_auth_method"]} > - + )} + ); export default TokenEndpointAuthMethodField; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenExchangeFormFields.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenExchangeFormFields.tsx index 9e1e1a85743..ba213b20655 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenExchangeFormFields.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/TokenExchangeFormFields.tsx @@ -1,6 +1,11 @@ import React from "react"; -import { Form, Input, Select, Tooltip } from "antd"; +import { Input, Select, Tooltip } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; +import { useWatch } from "react-hook-form"; + +import { MountedFormField } from "@/components/common_components/MountedFormField"; +import { antdRequired } from "@/components/common_components/antdFormRules"; +import { selectControl, textControl } from "./mcpFieldRules"; interface TokenExchangeFormFieldsProps { isEditing?: boolean; @@ -19,10 +24,13 @@ const FieldLabel: React.FC<{ label: string; tooltip: string }> = ({ label, toolt const TokenExchangeFormFields: React.FC = ({ isEditing = false }) => { const placeholderSuffix = isEditing ? " (leave blank to keep existing)" : ""; + const isEntraObo = useWatch({ name: "token_exchange_profile" }) === "entra_obo"; + const requiredWhenCreating = (message: string) => + isEditing ? undefined : { validate: { required: antdRequired(message) } }; return ( <> - = ({ isEdi /> } name="token_exchange_profile" - {...(isEditing ? {} : { initialValue: "rfc8693" })} + {...(isEditing ? {} : { defaultValue: "rfc8693" })} > - - - ( + + )} + + = ({ isEdi } name="token_exchange_endpoint" > - - - ( + + )} + + = ({ isEdi /> } name={["credentials", "client_id"]} - rules={[{ required: !isEditing, message: "Client ID is required for token exchange" }]} + required={!isEditing} + rules={requiredWhenCreating("Client ID is required for token exchange")} > - - - ( + + )} + + = ({ isEdi /> } name={["credentials", "client_secret"]} - rules={[{ required: !isEditing, message: "Client Secret is required for token exchange" }]} + required={!isEditing} + rules={requiredWhenCreating("Client Secret is required for token exchange")} > - - - prev.token_exchange_profile !== cur.token_exchange_profile}> - {({ getFieldValue }) => { - const isEntraObo = getFieldValue("token_exchange_profile") === "entra_obo"; - return ( - <> - {!isEntraObo && ( - <> - - } - name="audience" - > - - - - } - name="subject_token_type" - > - - - - )} - /.default)." - : "Optional scopes to request during the token exchange." - } - /> - } - name={["credentials", "scopes"]} - rules={ - isEntraObo - ? [ - { - required: true, - message: "Microsoft Entra OBO requires a scope, e.g. api:///.default", - }, - ] - : [] - } - > - + )} + + + } + name="subject_token_type" + > + {(control) => ( + + )} + + + )} + /.default)." + : "Optional scopes to request during the token exchange." + } + /> + } + name={["credentials", "scopes"]} + required={isEntraObo} + rules={ + isEntraObo + ? { + validate: { + required: antdRequired("Microsoft Entra OBO requires a scope, e.g. api:///.default"), + }, + } + : undefined + } + > + {(control) => ( + - - validateMCPServerName(value), - }, - ]} - > - setAliasManuallyEdited(true)} - className="rounded-lg border-gray-300 focus:border-blue-500 focus:ring-blue-500" - /> - - - - - - - - - - {/* URL field - only for HTTP/SSE */} - {isMCPTransport && ( - validateMCPServerUrl(value) }, - ]} - > - - - )} - - {/* OpenAPI Spec URL - only for OpenAPI transport */} - {isOpenAPITransport && ( - - OpenAPI Spec URL - - - - - } - name="spec_path" - rules={[{ required: true, message: "Please enter an OpenAPI spec URL" }]} - > - - - )} - - - Max Concurrent Requests (optional) - - - - - } - name="max_concurrent_requests" - > - - - - {/* Authentication - for HTTP, SSE, and OpenAPI */} - {!isStdioTransport && ( - <> - - - - - - - )} - - {isStdioTransport && ( -
-

- Configure the stdio transport used to launch the MCP server process. You can either fill in the fields - below or paste a JSON configuration. -

- - - - - - - - - - AWS Service Name - - - - - } - name={["credentials", "aws_service_name"]} - > - - - - AWS Access Key ID - - - - - } - name={["credentials", "aws_access_key_id"]} - rules={[]} - > - - - - AWS Secret Access Key - - - - - } - name={["credentials", "aws_secret_access_key"]} - rules={[]} - > - - - - AWS Session Token - - - - - } - name={["credentials", "aws_session_token"]} - > - - - - AWS Role ARN - - - - - } - name={["credentials", "aws_role_name"]} - > - - - - AWS Session Name - - - - - } - name={["credentials", "aws_session_name"]} - > - - - - )} - - {/* Environment Variables Section */} -
- -
- - {/* Permission Management / Access Control Section */} -
- -
- - {/* Tool Configuration Section */} -
- + +
{ + event.preventDefault(); + void submitForm(); }} - allowedTools={allowedTools} - existingAllowedTools={existingAllowedTools} - hasToolAllowlistInteraction={hasToolAllowlistInteraction} - isEditMode - onAllowedToolsChange={setAllowedTools} - onToolAllowlistInteraction={() => setHasToolAllowlistInteraction(true)} - toolNameToDisplayName={toolNameToDisplayName} - toolNameToDescription={toolNameToDescription} - onToolNameToDisplayNameChange={setToolNameToDisplayName} - onToolNameToDescriptionChange={setToolNameToDescription} - externalTools={tools} - externalIsLoading={isLoadingTools} - externalError={toolsError} - externalCanFetch={true} - /> -
+ > + validateMCPServerName(value) }) }} + > + {(control) => ( + + )} + + validateMCPServerName(value) }) }} + > + {(control) => ( + { + control.onChange(event); + setAliasManuallyEdited(true); + }} + className="rounded-lg border-gray-300 focus:border-blue-500 focus:ring-blue-500" + /> + )} + + + {(control) => ( + + )} + + + + {(control) => ( + + )} + -
- Cancel - -
- + {/* URL field - only for HTTP/SSE */} + {isMCPTransport && ( + validateMCPServerUrl(value) }), + }, + }} + > + {(control) => ( + + )} + + )} + + {/* OpenAPI Spec URL - only for OpenAPI transport */} + {isOpenAPITransport && ( + + OpenAPI Spec URL + + + + + } + name="spec_path" + required + rules={{ validate: { required: antdRequired("Please enter an OpenAPI spec URL") } }} + > + {(control) => ( + + )} + + )} + + + Max Concurrent Requests (optional) + + + + + } + name="max_concurrent_requests" + > + {(control) => ( + + )} + + + {/* Authentication - for HTTP, SSE, and OpenAPI */} + {!isStdioTransport && ( + <> + + {(control) => ( + + )} + + + + + )} + + {isStdioTransport && ( +
+

+ Configure the stdio transport used to launch the MCP server process. You can either fill in the + fields below or paste a JSON configuration. +

+ + + {(control) => ( + + )} + + + + {(control) => ( + + )} + + + AWS Service Name + + + + + } + name={["credentials", "aws_service_name"]} + > + {(control) => ( + + )} + + + AWS Access Key ID + + + + + } + name={["credentials", "aws_access_key_id"]} + > + {(control) => ( + + )} + + + AWS Secret Access Key + + + + + } + name={["credentials", "aws_secret_access_key"]} + > + {(control) => ( + + )} + + + AWS Session Token + + + + + } + name={["credentials", "aws_session_token"]} + > + {(control) => ( + + )} + + + AWS Role ARN + + + + + } + name={["credentials", "aws_role_name"]} + > + {(control) => ( + + )} + + + AWS Session Name + + + + + } + name={["credentials", "aws_session_name"]} + > + {(control) => ( + + )} + + + )} + + {/* Environment Variables Section */} +
+ +
+ + {/* Permission Management / Access Control Section */} +
+ +
+ + {/* Tool Configuration Section */} +
+ setHasToolAllowlistInteraction(true)} + toolNameToDisplayName={toolNameToDisplayName} + toolNameToDescription={toolNameToDescription} + onToolNameToDisplayNameChange={setToolNameToDisplayName} + onToolNameToDescriptionChange={setToolNameToDescription} + externalTools={tools} + externalIsLoading={isLoadingTools} + externalError={toolsError} + externalCanFetch={true} + /> +
+ +
+ Cancel + +
+ + + @@ -1186,7 +1285,7 @@ const MCPServerEdit: React.FC = ({
Cancel - +
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mountedServerFields.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mountedServerFields.test.ts index 5f06d03d595..dd9c8db6d30 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mountedServerFields.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mountedServerFields.test.ts @@ -482,7 +482,7 @@ describe("projection shape", () => { expect(projected.command).toBe("npx"); }); - it("passes Form.List rows through whole, since antd does not project a row to its mounted sub-fields", () => { + it("passes list rows through whole, since a list field is projected as one key and not per mounted sub-field", () => { const row = { name: "N", value: "V", scope: "user", description: "D" }; const projected = projectMountedEditValues({ ...HTTP_NONE, env_vars: [row] }); expect(projected.env_vars).toStrictEqual([row]); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/testUtils.ts b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/testUtils.ts index 7911c3eb800..c1692f436d7 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/testUtils.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/testUtils.ts @@ -4,6 +4,7 @@ import { expect } from "vitest"; export async function selectAntOption(labelText: string, optionText: string) { const label = screen.getByText(labelText); const select = + label.closest("[data-slot='field']")?.querySelector(".ant-select") ?? label.closest(".ant-form-item")?.querySelector(".ant-select") ?? label.closest(".ant-collapse-item")?.querySelector(".ant-select") ?? label.closest("div")?.querySelector(".ant-select") ?? diff --git a/ui/litellm-dashboard/src/components/common_components/MountedFormField.test.ts b/ui/litellm-dashboard/src/components/common_components/MountedFormField.test.ts deleted file mode 100644 index b619ec5748a..00000000000 --- a/ui/litellm-dashboard/src/components/common_components/MountedFormField.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { describe, expect, it } from "vitest"; -import type { UseFormGetValues } from "react-hook-form"; - -import { - projectMountedValues, - type MountedFieldName, - type MountedFormValues, - type MountRegistry, -} from "./MountedFormField"; - -const registryOf = (names: readonly MountedFieldName[]): MountRegistry => ({ - register: () => () => undefined, - mountedNames: () => names, -}); - -const getValuesOf = (store: Readonly>): UseFormGetValues => - ((names: readonly string[]) => names.map((name) => store[name])) as unknown as UseFormGetValues; - -const project = (store: Readonly>) => - projectMountedValues(registryOf(Object.keys(store)), getValuesOf(store)); - -const projectPaths = (entries: readonly (readonly [MountedFieldName, unknown])[]) => { - const store = Object.fromEntries( - entries.map(([name, value]) => [Array.isArray(name) ? name.join(".") : (name as string), value]), - ); - return projectMountedValues(registryOf(entries.map(([name]) => name)), getValuesOf(store)); -}; - -describe("projectMountedValues", () => { - it("keeps a flat name flat", () => { - expect(project({ server_name: "s1", transport: "http" })).toStrictEqual({ server_name: "s1", transport: "http" }); - }); - - it("nests an ARRAY name into a credentials object", () => { - expect( - projectPaths([ - [["credentials", "aws_region_name"], "us-east-1"], - [["credentials", "aws_access_key_id"], "AKIA"], - ]), - ).toStrictEqual({ credentials: { aws_region_name: "us-east-1", aws_access_key_id: "AKIA" } }); - }); - - it("keeps a literal dotted STRING name flat, matching antd getNamePath toArray", () => { - expect(projectPaths([["a.b", 1]])).toStrictEqual({ "a.b": 1 }); - expect(projectPaths([["schema.property.with.dots", "v"]])).toStrictEqual({ "schema.property.with.dots": "v" }); - }); - - it("rebuilds Form.List rows as an array, not an object keyed by digits", () => { - const projected = projectPaths([ - [["env_vars", "0", "name"], "API_KEY"], - [["env_vars", "0", "description"], "the key"], - [["env_vars", "1", "name"], "REGION"], - ]); - expect(projected).toStrictEqual({ - env_vars: [{ name: "API_KEY", description: "the key" }, { name: "REGION" }], - }); - expect(Array.isArray(projected.env_vars)).toBe(true); - }); - - it("rebuilds static_headers rows, the second Form.List site", () => { - expect( - projectPaths([ - [["static_headers", "0", "key"], "X-Tenant"], - [["static_headers", "0", "value"], "acme"], - ]), - ).toStrictEqual({ - static_headers: [{ key: "X-Tenant", value: "acme" }], - }); - }); - - it("emits a mounted-but-unset field as a key holding undefined, matching antd onFinish", () => { - const projected = project({ alias: undefined }); - expect(Object.keys(projected)).toStrictEqual(["alias"]); - expect(projected.alias).toBeUndefined(); - }); - - it("leaves a sparse row index as a hole rather than shifting later rows down", () => { - const projected = projectPaths([[["env_vars", "2", "name"], "THIRD"]]) as { env_vars: readonly unknown[] }; - expect(projected.env_vars).toHaveLength(3); - expect(projected.env_vars[2]).toStrictEqual({ name: "THIRD" }); - }); - - it("mixes flat, nested and list names in one projection", () => { - expect( - projectPaths([ - ["transport", "http"], - [["credentials", "client_id"], "cid"], - [["env_vars", "0", "name"], "K"], - ]), - ).toStrictEqual({ - transport: "http", - credentials: { client_id: "cid" }, - env_vars: [{ name: "K" }], - }); - }); -}); diff --git a/ui/litellm-dashboard/src/components/common_components/MountedFormField.test.tsx b/ui/litellm-dashboard/src/components/common_components/MountedFormField.test.tsx new file mode 100644 index 00000000000..62cdd935131 --- /dev/null +++ b/ui/litellm-dashboard/src/components/common_components/MountedFormField.test.tsx @@ -0,0 +1,187 @@ +import React from "react"; +import { render, renderHook, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import { useForm } from "react-hook-form"; +import type { UseFormGetValues } from "react-hook-form"; + +import { + MountedFormField, + MountedFormProvider, + projectMountedValues, + useMountRegistry, + type MountedFieldName, + type MountedFormValues, + type MountRegistry, +} from "./MountedFormField"; + +const registryOf = (names: readonly MountedFieldName[]): MountRegistry => ({ + register: () => () => undefined, + mountedNames: () => names, +}); + +const getValuesOf = (store: Readonly>): UseFormGetValues => + ((names: readonly string[]) => names.map((name) => store[name])) as unknown as UseFormGetValues; + +const project = (store: Readonly>) => + projectMountedValues(registryOf(Object.keys(store)), getValuesOf(store)); + +const projectPaths = (entries: readonly (readonly [MountedFieldName, unknown])[]) => { + const store = Object.fromEntries( + entries.map(([name, value]) => [Array.isArray(name) ? name.join(".") : (name as string), value]), + ); + return projectMountedValues(registryOf(entries.map(([name]) => name)), getValuesOf(store)); +}; + +describe("projectMountedValues", () => { + it("keeps a flat name flat", () => { + expect(project({ server_name: "s1", transport: "http" })).toStrictEqual({ server_name: "s1", transport: "http" }); + }); + + it("nests an ARRAY name into a credentials object", () => { + expect( + projectPaths([ + [["credentials", "aws_region_name"], "us-east-1"], + [["credentials", "aws_access_key_id"], "AKIA"], + ]), + ).toStrictEqual({ credentials: { aws_region_name: "us-east-1", aws_access_key_id: "AKIA" } }); + }); + + it("keeps a literal dotted STRING name flat, matching antd getNamePath toArray", () => { + expect(projectPaths([["a.b", 1]])).toStrictEqual({ "a.b": 1 }); + expect(projectPaths([["schema.property.with.dots", "v"]])).toStrictEqual({ "schema.property.with.dots": "v" }); + }); + + it("rebuilds Form.List rows as an array, not an object keyed by digits", () => { + const projected = projectPaths([ + [["env_vars", "0", "name"], "API_KEY"], + [["env_vars", "0", "description"], "the key"], + [["env_vars", "1", "name"], "REGION"], + ]); + expect(projected).toStrictEqual({ + env_vars: [{ name: "API_KEY", description: "the key" }, { name: "REGION" }], + }); + expect(Array.isArray(projected.env_vars)).toBe(true); + }); + + it("rebuilds static_headers rows, the second Form.List site", () => { + expect( + projectPaths([ + [["static_headers", "0", "key"], "X-Tenant"], + [["static_headers", "0", "value"], "acme"], + ]), + ).toStrictEqual({ + static_headers: [{ key: "X-Tenant", value: "acme" }], + }); + }); + + it("emits a mounted-but-unset field as a key holding undefined, matching antd onFinish", () => { + const projected = project({ alias: undefined }); + expect(Object.keys(projected)).toStrictEqual(["alias"]); + expect(projected.alias).toBeUndefined(); + }); + + it("leaves a sparse row index as a hole rather than shifting later rows down", () => { + const projected = projectPaths([[["env_vars", "2", "name"], "THIRD"]]) as { env_vars: readonly unknown[] }; + expect(projected.env_vars).toHaveLength(3); + expect(projected.env_vars[2]).toStrictEqual({ name: "THIRD" }); + }); + + it("mixes flat, nested and list names in one projection", () => { + expect( + projectPaths([ + ["transport", "http"], + [["credentials", "client_id"], "cid"], + [["env_vars", "0", "name"], "K"], + ]), + ).toStrictEqual({ + transport: "http", + credentials: { client_id: "cid" }, + env_vars: [{ name: "K" }], + }); + }); +}); + +describe("useMountRegistry lifecycle", () => { + const GatedForm: React.FC<{ + showOptional: boolean; + showRequired: boolean; + onFinish: (v: MountedFormValues) => void; + }> = ({ showOptional, showRequired, onFinish }) => { + const form = useForm({ mode: "onChange", defaultValues: { server_name: "keep" } }); + const registry = useMountRegistry(); + return ( + +
{ + event.preventDefault(); + void form + .trigger(registry.mountedNames().map((n) => (Array.isArray(n) ? n.join(".") : (n as string)))) + .then((valid) => { + if (valid) onFinish(projectMountedValues(registry, form.getValues)); + }); + }} + > + + {(control) => ( + + )} + + {showOptional && ( + + {(control) => ( + + )} + + )} + {showRequired && ( + + {(control) => ( + + )} + + )} + +
+ + ); + }; + + it("drops a field's key from the submitted payload once its gate unmounts it", async () => { + const onFinish = vi.fn(); + const { rerender } = render(); + + await userEvent.click(screen.getByRole("button", { name: "Submit" })); + await waitFor(() => expect(onFinish).toHaveBeenCalledTimes(1)); + expect(Object.keys(onFinish.mock.calls[0][0] as object)).toContain("alias"); + + rerender(); + await userEvent.click(screen.getByRole("button", { name: "Submit" })); + await waitFor(() => expect(onFinish).toHaveBeenCalledTimes(2)); + expect(Object.keys(onFinish.mock.calls[1][0] as object)).not.toContain("alias"); + }); + + it("submits after a required field is unmounted, rather than validating a field the user can no longer see", async () => { + const onFinish = vi.fn(); + const { rerender } = render(); + + await userEvent.click(screen.getByRole("button", { name: "Submit" })); + expect(await screen.findByText("Token URL is required")).toBeInTheDocument(); + expect(onFinish).not.toHaveBeenCalled(); + + rerender(); + await userEvent.click(screen.getByRole("button", { name: "Submit" })); + await waitFor(() => expect(onFinish).toHaveBeenCalledTimes(1)); + expect(Object.keys(onFinish.mock.calls[0][0] as object)).not.toContain("token_url"); + }); + + it("keeps a name mounted while a second field still holds a registration on it", () => { + const registry = renderHook(() => useMountRegistry()).result.current; + const releaseFirst = registry.register("credentials.scopes"); + registry.register("credentials.scopes"); + + releaseFirst(); + + expect(registry.mountedNames()).toStrictEqual(["credentials.scopes"]); + }); +}); diff --git a/ui/litellm-dashboard/src/components/common_components/MountedFormField.tsx b/ui/litellm-dashboard/src/components/common_components/MountedFormField.tsx index 0ae58e102e7..f0efa885d92 100644 --- a/ui/litellm-dashboard/src/components/common_components/MountedFormField.tsx +++ b/ui/litellm-dashboard/src/components/common_components/MountedFormField.tsx @@ -94,6 +94,11 @@ export const projectMountedValues = ( ); }; +export const useMountedName = (name: MountedFieldName): void => { + const { registry } = React.useContext(MountedFormContext); + React.useEffect(() => registry.register(name), [registry, name]); +}; + export type MountedFieldControlProps = { readonly id: string; readonly name: string; @@ -131,9 +136,9 @@ export const MountedFormField: React.FC = ({ className, children, }) => { - const { control, registry } = React.useContext(MountedFormContext); + const { control } = React.useContext(MountedFormContext); const path = fieldKey(name); - React.useEffect(() => registry.register(name), [registry, name]); + useMountedName(name); const helpId = `${path}_help`; const hasHelp = help !== undefined && help !== null;