From aa90828811ffb7939d644aeb846b0e9f6ef46fc8 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 18 Aug 2026 11:47:06 -0700 Subject: [PATCH] refactor(ui): migrate prompt, UI access, plugin and MCP filter forms to react-hook-form and shadcn (#37297) * refactor(ui): migrate prompt, UI access, plugin and MCP filter forms to react-hook-form and shadcn Moves four more admin dashboard forms off antd Form onto react-hook-form with the shared shadcn form kit, keeping the submitted payload byte-identical in every case. Each form was pinned with a characterization test proven green against the antd original before any production code changed, then re-run unedited afterwards. Behaviours that needed reproducing by hand rather than falling out of the port: - antd onFinish reports only mounted fields, so UIAccessControlForm blanks a seeded but hidden restricted_sso_group at submit time instead of sending it - antd InputNumber returns null on empty and clamps on blur, so MCPSemanticFilterSettings keeps top_k as null when cleared and clamps to [1, 100] rather than sending "" or NaN - PluginSettings seeds plugin_key blank on edit so an untouched save preserves the stored credential instead of overwriting it with the redacted placeholder - antd's url rule and zod's .url() disagree in both directions, so the async-validator pattern is ported verbatim to avoid silently changing which URLs are accepted Forms with no onFinish keep preventDefault so no Enter-to-submit is introduced, and the plugin key regains a reveal toggle built on InputGroup. * chore(ui): prune stale eslint suppressions left by concurrent form-migration PRs * Revert "chore(ui): prune stale eslint suppressions left by concurrent form-migration PRs" This reverts commit e36bcc586271922814c00712d6d07a62212c9395. * fix(ui): keep the embedding model unclearable, matching the antd Select The antd Select for embedding_model had no allowClear, so an admin could never empty it. SearchSelect renders a clear button whenever a value is set and emits an empty string, so the migration silently added a way to persist an empty embedding_model and break semantic filtering. Adds an opt-out to SearchSelect that defaults to the current behaviour, leaving the other twenty callers unaffected, and opts this one field out. Pinned with a test that fails when the opt-out is removed. --- ui/litellm-dashboard/eslint-suppressions.json | 7 +- .../add_prompt_form.integration.test.tsx | 121 ++++++ .../prompts/_components/add_prompt_form.tsx | 225 ++++++----- .../MCPSemanticFilterSettings.test.tsx | 150 +++++++- .../MCPSemanticFilterSettings.tsx | 363 +++++++++++------- .../PluginSettings.integration.test.tsx | 134 +++++++ .../PluginSettings/PluginSettings.tsx | 103 +++-- .../AdminSettings/PluginSettings/schema.ts | 39 ++ .../UIAccessControlForm.integration.test.tsx | 241 ++++++++++++ .../src/components/UIAccessControlForm.tsx | 280 +++++++++----- .../src/components/shared/SearchSelect.tsx | 4 +- 11 files changed, 1279 insertions(+), 388 deletions(-) create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/schema.ts create mode 100644 ui/litellm-dashboard/src/components/UIAccessControlForm.integration.test.tsx diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 5cc055a50ff..c879eea3687 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -1251,7 +1251,7 @@ "count": 1 }, "no-restricted-imports": { - "count": 3 + "count": 2 } }, "src/app/(dashboard)/prompts/_components/index.tsx": { @@ -1838,11 +1838,6 @@ "count": 1 } }, - "src/components/UIAccessControlForm.tsx": { - "no-restricted-imports": { - "count": 2 - } - }, "src/components/UsagePage/components/EntityUsage/TopKeyView.tsx": { "no-restricted-imports": { "count": 1 diff --git a/ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.integration.test.tsx new file mode 100644 index 00000000000..bf8bc7bd4af --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.integration.test.tsx @@ -0,0 +1,121 @@ +import React from "react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { toast } from "@/lib/toast"; +import { convertPromptFileToJson, createPromptCall } from "@/components/networking"; + +import AddPromptForm from "./add_prompt_form"; + +vi.mock("@/components/networking", () => ({ + convertPromptFileToJson: vi.fn(), + createPromptCall: vi.fn(), +})); + +vi.mock("@/lib/toast", () => ({ + toast: { success: vi.fn(), fromError: vi.fn() }, +})); + +const mockConvert = vi.mocked(convertPromptFileToJson); +const mockCreate = vi.mocked(createPromptCall); +const mockFromBackend = vi.mocked(toast.fromError); +const mockSuccess = vi.mocked(toast.success); + +const PROMPT_ID_PLACEHOLDER = "Enter unique prompt ID (e.g., my_prompt_id)"; + +const CONVERTED_JSON = { model: "gpt-4o", messages: [{ role: "user", content: "hi {{name}}" }] }; + +const renderForm = () => { + const onClose = vi.fn(); + const onSuccess = vi.fn(); + render(); + return { onClose, onSuccess }; +}; + +const typePromptId = (value: string) => + fireEvent.change(screen.getByPlaceholderText(PROMPT_ID_PLACEHOLDER), { target: { value } }); + +const attachPromptFile = async (file: File) => { + const fileInput = document.querySelector('input[type="file"]') as HTMLInputElement; + await act(async () => { + fireEvent.change(fileInput, { target: { files: [file] } }); + }); + await screen.findByText(`Selected: ${file.name}`); +}; + +const submit = async () => { + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Create Prompt" })); + }); +}; + +describe("AddPromptForm", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockConvert.mockResolvedValue({ prompt_id: "converted_prompt_id", json_data: CONVERTED_JSON }); + mockCreate.mockResolvedValue({ status: "success" }); + }); + + it("sends the converted upload as the exact create-prompt payload, then closes and refreshes", async () => { + const { onClose, onSuccess } = renderForm(); + const file = new File(["model: gpt-4o"], "greeting.prompt", { type: "text/plain" }); + + typePromptId("my_prompt_id"); + await attachPromptFile(file); + await submit(); + + await waitFor(() => { + expect(mockCreate).toHaveBeenCalledWith("sk-test", { + prompt_id: "my_prompt_id", + litellm_params: { + prompt_integration: "dotprompt", + prompt_id: "converted_prompt_id", + prompt_data: CONVERTED_JSON, + }, + prompt_info: { + prompt_type: "db", + }, + }); + }); + expect(mockConvert).toHaveBeenCalledWith("sk-test", file); + expect(mockSuccess).toHaveBeenCalledWith("Prompt created successfully!"); + expect(onClose).toHaveBeenCalledTimes(1); + expect(onSuccess).toHaveBeenCalledTimes(1); + }); + + it("refuses to submit without an uploaded file", async () => { + renderForm(); + + typePromptId("my_prompt_id"); + await submit(); + + await waitFor(() => { + expect(mockFromBackend).toHaveBeenCalledWith("Please upload a .prompt file"); + }); + expect(mockConvert).not.toHaveBeenCalled(); + expect(mockCreate).not.toHaveBeenCalled(); + }); + + it("blocks submission and reports a missing prompt ID", async () => { + renderForm(); + + await submit(); + + expect(await screen.findByText("Please enter a prompt ID")).toBeInTheDocument(); + expect(mockCreate).not.toHaveBeenCalled(); + }); + + it("blocks submission and reports a prompt ID with unsupported characters", async () => { + renderForm(); + const file = new File(["model: gpt-4o"], "greeting.prompt", { type: "text/plain" }); + + typePromptId("my prompt!"); + await attachPromptFile(file); + await submit(); + + expect( + await screen.findByText("Prompt ID can only contain letters, numbers, underscores, and hyphens"), + ).toBeInTheDocument(); + expect(mockCreate).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.tsx b/ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.tsx index 0eae86cd68b..dc5221f9983 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/prompts/_components/add_prompt_form.tsx @@ -1,12 +1,17 @@ import React, { useState } from "react"; -import { Modal, Form, Select, Upload, Button, Divider } from "antd"; -import { TextInput } from "@tremor/react"; -import { UploadOutlined } from "@ant-design/icons"; +import { Modal, Upload } from "antd"; import type { UploadFile, UploadProps } from "antd"; +import { Upload as UploadIcon } from "lucide-react"; +import { z } from "zod/v4"; import { convertPromptFileToJson, createPromptCall } from "@/components/networking"; import { toast } from "@/lib/toast"; - -const { Option } = Select; +import { Field, FieldDescription, FieldGroup, FieldSeparator, FieldTitle } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; +import { useZodForm } from "@/lib/forms/useZodForm"; interface AddPromptFormProps { visible: boolean; @@ -15,76 +20,107 @@ interface AddPromptFormProps { onSuccess: () => void; } +interface CreatePromptRequest { + prompt_id: string; + litellm_params: { + prompt_integration: string; + prompt_id: string; + prompt_data: unknown; + }; + prompt_info: { + prompt_type: string; + }; +} + +const PROMPT_INTEGRATION_OPTIONS = [{ label: "dotprompt", value: "dotprompt" }]; + +const addPromptSchema = z.object({ + prompt_id: z + .string() + .min(1, "Please enter a prompt ID") + .regex(/^[a-zA-Z0-9_-]+$/, "Prompt ID can only contain letters, numbers, underscores, and hyphens"), + prompt_integration: z.string(), +}); + +type AddPromptFormValues = z.infer; + +const EMPTY_VALUES: AddPromptFormValues = { prompt_id: "", prompt_integration: "dotprompt" }; + const AddPromptForm: React.FC = ({ visible, onClose, accessToken, onSuccess }) => { - const [form] = Form.useForm(); + const form = useZodForm(addPromptSchema, { defaultValues: EMPTY_VALUES }); const [loading, setLoading] = useState(false); const [fileList, setFileList] = useState([]); const [promptIntegration, setPromptIntegration] = useState("dotprompt"); const handleCancel = () => { - form.resetFields(); + form.reset(EMPTY_VALUES); setFileList([]); setPromptIntegration("dotprompt"); onClose(); }; - const handleSubmit = async () => { + const handleIntegrationChange = (selected: string | null) => { + if (selected === null) return; + form.setValue("prompt_integration", selected); + setPromptIntegration(selected); + }; + + const convertUploadedFile = async (token: string, promptId: string): Promise => { + const file = fileList[0].originFileObj as File; + try { - const values = await form.validateFields(); + const conversionResult = await convertPromptFileToJson(token, file); - if (!accessToken) { - toast.fromError("Access token is required"); - return; - } + return { + prompt_id: promptId, + litellm_params: { + prompt_integration: "dotprompt", + prompt_id: conversionResult.prompt_id, + prompt_data: conversionResult.json_data, + }, + prompt_info: { + prompt_type: "db", + }, + }; + } catch (conversionError) { + console.error("Error converting prompt file:", conversionError); + toast.fromError("Failed to convert prompt file to JSON"); + return null; + } + }; - if (promptIntegration === "dotprompt" && fileList.length === 0) { - toast.fromError("Please upload a .prompt file"); - return; - } + const handleSubmit = async (values: AddPromptFormValues) => { + if (!accessToken) { + toast.fromError("Access token is required"); + return; + } - setLoading(true); + const isDotprompt = promptIntegration === "dotprompt"; - let promptData: any = {}; + if (isDotprompt && fileList.length === 0) { + toast.fromError("Please upload a .prompt file"); + return; + } - if (promptIntegration === "dotprompt" && fileList.length > 0) { - // Convert the uploaded file to JSON - const file = fileList[0].originFileObj as File; + setLoading(true); - try { - const conversionResult = await convertPromptFileToJson(accessToken, file); + const promptData: CreatePromptRequest | Record | null = isDotprompt + ? await convertUploadedFile(accessToken, values.prompt_id) + : {}; - // Prepare prompt data for creation - promptData = { - prompt_id: values.prompt_id, - litellm_params: { - prompt_integration: "dotprompt", - prompt_id: conversionResult.prompt_id, - prompt_data: conversionResult.json_data, - }, - prompt_info: { - prompt_type: "db", - }, - }; - } catch (conversionError) { - console.error("Error converting prompt file:", conversionError); - toast.fromError("Failed to convert prompt file to JSON"); - setLoading(false); - return; - } - } + if (promptData === null) { + setLoading(false); + return; + } - // Create the prompt - try { - await createPromptCall(accessToken, promptData); - toast.success("Prompt created successfully!"); - handleCancel(); - onSuccess(); - } catch (createError) { - console.error("Error creating prompt:", createError); - toast.fromError("Failed to create prompt"); - } - } catch (error) { - console.error("Form validation error:", error); + try { + await createPromptCall(accessToken, promptData); + toast.success("Prompt created successfully!"); + handleCancel(); + onSuccess(); + } catch (createError) { + console.error("Error creating prompt:", createError); + toast.fromError("Failed to create prompt"); } finally { setLoading(false); } @@ -113,48 +149,61 @@ const AddPromptForm: React.FC = ({ visible, onClose, accessT open={visible} onCancel={handleCancel} footer={[ - , - , ]} width={600} > -
- - - + event.preventDefault()} noValidate> + + + {({ ref, ...field }) => ( + + )} + - - - + + {({ id, value, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( + + )} + - {promptIntegration === "dotprompt" && ( - <> - - - - - - {fileList.length > 0 &&
Selected: {fileList[0].name}
} - - - )} - + {promptIntegration === "dotprompt" && ( + <> + + + Prompt File + + + + {fileList.length > 0 && ( +
Selected: {fileList[0].name}
+ )} + Upload a .prompt file that follows the Dotprompt specification + + + )} +
+ ); }; diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.test.tsx index af2cc0beb7e..b2ede55e3c4 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.test.tsx @@ -1,10 +1,11 @@ import React from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, act } from "@testing-library/react"; +import { render, screen, act, fireEvent } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import MCPSemanticFilterSettings from "./MCPSemanticFilterSettings"; import { useMCPSemanticFilterSettings } from "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useMCPSemanticFilterSettings"; import { useUpdateMCPSemanticFilterSettings } from "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useUpdateMCPSemanticFilterSettings"; +import { fetchAvailableModels } from "@/components/llm_calls/fetch_models"; vi.mock("@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useMCPSemanticFilterSettings", () => ({ useMCPSemanticFilterSettings: vi.fn(), @@ -43,13 +44,61 @@ const defaultSettingsData = { }, }; +const AVAILABLE_MODELS = [ + { model_group: "text-embedding-3-large", mode: "embedding" }, + { model_group: "gpt-4o", mode: "chat" }, +]; + +const SWITCH_ONLY_PAYLOAD = { + enabled: true, + embedding_model: "text-embedding-3-small", + top_k: 10, + similarity_threshold: 0.3, +}; + +const DEFAULTED_PAYLOAD = { + enabled: false, + embedding_model: "text-embedding-3-small", + top_k: 7, + similarity_threshold: 0.3, +}; + +const FULLY_EDITED_PAYLOAD = { + enabled: true, + embedding_model: "text-embedding-3-large", + top_k: 25, + similarity_threshold: 0.35, +}; + +const CLEARED_TOP_K_PAYLOAD = { + enabled: false, + embedding_model: "text-embedding-3-small", + top_k: null, + similarity_threshold: 0.3, +}; + +const CLAMPED_MAX_PAYLOAD = { + enabled: false, + embedding_model: "text-embedding-3-small", + top_k: 100, + similarity_threshold: 0.3, +}; + +const CLAMPED_MIN_PAYLOAD = { + enabled: false, + embedding_model: "text-embedding-3-small", + top_k: 1, + similarity_threshold: 0.3, +}; + // Helper that renders the component and flushes the fetchAvailableModels effect async function renderSettings(props: React.ComponentProps) { - render(); + const result = render(); if (props.accessToken) { // Let the async fetchAvailableModels effect settle to avoid act() warnings await act(async () => {}); } + return result; } describe("MCPSemanticFilterSettings", () => { @@ -156,4 +205,101 @@ describe("MCPSemanticFilterSettings", () => { expect(screen.getByText("Could not update settings")).toBeInTheDocument(); expect(screen.getByText("Failed to update settings")).toBeInTheDocument(); }); + + it("should send the default values as the payload when only the switch is toggled", async () => { + const user = userEvent.setup(); + await renderSettings({ accessToken: "test-token" }); + + await user.click(screen.getByRole("switch")); + await user.click(screen.getByRole("button", { name: /save settings/i })); + + expect(mockMutate).toHaveBeenCalledWith(SWITCH_ONLY_PAYLOAD, expect.anything()); + }); + + it("should fall back to the hardcoded defaults when the backend returns no values", async () => { + vi.mocked(useMCPSemanticFilterSettings).mockReturnValue({ + data: { field_schema: defaultSettingsData.field_schema, values: {} }, + isLoading: false, + isError: false, + error: null, + } as any); + const user = userEvent.setup(); + await renderSettings({ accessToken: "test-token" }); + + const topK = screen.getByRole("spinbutton"); + await user.clear(topK); + await user.type(topK, "7"); + await user.click(screen.getByRole("button", { name: /save settings/i })); + + expect(mockMutate).toHaveBeenCalledWith(DEFAULTED_PAYLOAD, expect.anything()); + }); + + it("should send every edited field in the payload", async () => { + vi.mocked(fetchAvailableModels).mockResolvedValueOnce(AVAILABLE_MODELS); + const user = userEvent.setup(); + await renderSettings({ accessToken: "test-token" }); + + await user.click(screen.getByRole("switch")); + + const topK = screen.getByRole("spinbutton"); + await user.clear(topK); + await user.type(topK, "25"); + + const slider = screen.getByRole("slider", { hidden: true }); + fireEvent.keyDown(slider, { key: "ArrowRight", keyCode: 39, which: 39 }); + + const embeddingModel = screen.getByRole("combobox"); + await user.click(embeddingModel); + await user.clear(embeddingModel); + await user.type(embeddingModel, "large"); + fireEvent.keyDown(embeddingModel, { key: "ArrowDown", keyCode: 40, which: 40 }); + fireEvent.keyDown(embeddingModel, { key: "Enter", keyCode: 13, which: 13 }); + + await user.click(screen.getByRole("button", { name: /save settings/i })); + + expect(mockMutate).toHaveBeenCalledWith(FULLY_EDITED_PAYLOAD, expect.anything()); + }); + + it("should send top_k as null when the field is cleared", async () => { + const user = userEvent.setup(); + await renderSettings({ accessToken: "test-token" }); + + const topK = screen.getByRole("spinbutton"); + await user.clear(topK); + await user.click(screen.getByRole("button", { name: /save settings/i })); + + expect(mockMutate).toHaveBeenCalledWith(CLEARED_TOP_K_PAYLOAD, expect.anything()); + }); + + it("should clamp top_k above the maximum back into range on blur", async () => { + const user = userEvent.setup(); + await renderSettings({ accessToken: "test-token" }); + + const topK = screen.getByRole("spinbutton"); + await user.clear(topK); + await user.type(topK, "500"); + await user.tab(); + await user.click(screen.getByRole("button", { name: /save settings/i })); + + expect(mockMutate).toHaveBeenCalledWith(CLAMPED_MAX_PAYLOAD, expect.anything()); + }); + + it("should clamp top_k below the minimum back into range on blur", async () => { + const user = userEvent.setup(); + await renderSettings({ accessToken: "test-token" }); + + const topK = screen.getByRole("spinbutton"); + await user.clear(topK); + await user.type(topK, "0"); + await user.tab(); + await user.click(screen.getByRole("button", { name: /save settings/i })); + + expect(mockMutate).toHaveBeenCalledWith(CLAMPED_MIN_PAYLOAD, expect.anything()); + }); + it("offers no way to clear the embedding model, as the antd Select had no allowClear", async () => { + const { container } = await renderSettings({ accessToken: "test-token" }); + + expect(screen.getByRole("combobox")).toHaveValue("text-embedding-3-small"); + expect(container.querySelector('[data-slot="combobox-clear"]')).not.toBeInTheDocument(); + }); }); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.tsx index b9dafd08f50..d8305171447 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/MCPSemanticFilterSettings/MCPSemanticFilterSettings.tsx @@ -3,25 +3,21 @@ import { useMCPSemanticFilterSettings } from "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useMCPSemanticFilterSettings"; import { useUpdateMCPSemanticFilterSettings } from "@/app/(dashboard)/hooks/mcpSemanticFilterSettings/useUpdateMCPSemanticFilterSettings"; import { toast } from "@/lib/toast"; -import { - Alert, - Button, - Card, - Col, - Form, - InputNumber, - Row, - Select, - Skeleton, - Slider, - Space, - Switch, - Typography, - Tooltip, -} from "antd"; -import { QuestionCircleOutlined, CheckCircleOutlined, SaveOutlined } from "@ant-design/icons"; +import { Alert, Card, Col, Row, Skeleton } from "antd"; +import { CheckCircleOutlined } from "@ant-design/icons"; +import { CircleHelp, Save } from "lucide-react"; import { useEffect, useState } from "react"; +import { useForm } from "react-hook-form"; import { fetchAvailableModels, ModelGroup } from "@/components/llm_calls/fetch_models"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { SearchSelect } from "@/components/shared/SearchSelect"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Slider } from "@/components/ui/slider"; +import { Switch } from "@/components/ui/switch"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; import MCPSemanticFilterTestPanel from "./MCPSemanticFilterTestPanel"; import { getCurlCommand, runSemanticFilterTest, TestResult } from "./semanticFilterTestUtils"; @@ -29,6 +25,60 @@ interface MCPSemanticFilterSettingsProps { accessToken: string | null; } +interface MCPSemanticFilterStoredValues { + enabled?: boolean; + embedding_model?: string; + top_k?: number; + similarity_threshold?: number; +} + +interface MCPSemanticFilterFieldSchema { + properties?: { enabled?: { description?: string } }; +} + +interface MCPSemanticFilterFormValues { + enabled: boolean; + embedding_model: string; + top_k: number | null; + similarity_threshold: number; +} + +const DEFAULT_FORM_VALUES: MCPSemanticFilterFormValues = { + enabled: false, + embedding_model: "text-embedding-3-small", + top_k: 10, + similarity_threshold: 0.3, +}; + +const NO_STORED_VALUES: MCPSemanticFilterStoredValues = {}; + +const TOP_K_MIN = 1; +const TOP_K_MAX = 100; + +const SIMILARITY_THRESHOLD_MARKS = [ + { value: 0, label: "0.0" }, + { value: 0.3, label: "0.3" }, + { value: 0.5, label: "0.5" }, + { value: 0.7, label: "0.7" }, + { value: 1, label: "1.0" }, +]; + +const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); + +const clampTopK = (value: number | null): number | null => + value === null ? null : Math.min(TOP_K_MAX, Math.max(TOP_K_MIN, value)); + +const parseTopK = (raw: string, rawAsNumber: number): number | null => + raw === "" || Number.isNaN(rawAsNumber) ? null : rawAsNumber; + export default function MCPSemanticFilterSettings({ accessToken }: MCPSemanticFilterSettingsProps) { const { data, isLoading, isError, error } = useMCPSemanticFilterSettings(); const { @@ -36,7 +86,7 @@ export default function MCPSemanticFilterSettings({ accessToken }: MCPSemanticFi isPending: isUpdating, error: updateError, } = useUpdateMCPSemanticFilterSettings(accessToken || ""); - const [form] = Form.useForm(); + const form = useForm({ defaultValues: DEFAULT_FORM_VALUES }); const [saveSuccess, setSaveSuccess] = useState(false); const [isDirty, setIsDirty] = useState(false); const [embeddingModels, setEmbeddingModels] = useState([]); @@ -49,8 +99,8 @@ export default function MCPSemanticFilterSettings({ accessToken }: MCPSemanticFi const [testError, setTestError] = useState(null); const [isTesting, setIsTesting] = useState(false); - const schema = data?.field_schema; - const values = data?.values ?? {}; + const schema: MCPSemanticFilterFieldSchema | undefined = data?.field_schema; + const values: MCPSemanticFilterStoredValues = data?.values ?? NO_STORED_VALUES; useEffect(() => { const loadEmbeddingModels = async () => { @@ -72,33 +122,33 @@ export default function MCPSemanticFilterSettings({ accessToken }: MCPSemanticFi useEffect(() => { if (values) { - form.setFieldsValue({ - enabled: values.enabled ?? false, - embedding_model: values.embedding_model ?? "text-embedding-3-small", - top_k: values.top_k ?? 10, - similarity_threshold: values.similarity_threshold ?? 0.3, + form.reset({ + enabled: values.enabled ?? DEFAULT_FORM_VALUES.enabled, + embedding_model: values.embedding_model ?? DEFAULT_FORM_VALUES.embedding_model, + top_k: values.top_k ?? DEFAULT_FORM_VALUES.top_k, + similarity_threshold: values.similarity_threshold ?? DEFAULT_FORM_VALUES.similarity_threshold, }); setIsDirty(false); } }, [values, form]); - const handleSave = async () => { - try { - const formValues = await form.validateFields(); - updateSettings(formValues, { - onSuccess: () => { - setIsDirty(false); - setSaveSuccess(true); - setTimeout(() => setSaveSuccess(false), 3000); - toast.success("Settings updated successfully. Changes will be applied across all pods within 10 seconds."); - }, - onError: (error) => { - toast.fromError(error); - }, - }); - } catch (error) { - console.error("Form validation failed:", error); - } + const commitChange = (onChange: (value: TValue) => void, value: TValue) => { + onChange(value); + setIsDirty(true); + }; + + const handleSave = (formValues: MCPSemanticFilterFormValues) => { + updateSettings(formValues, { + onSuccess: () => { + setIsDirty(false); + setSaveSuccess(true); + setTimeout(() => setSaveSuccess(false), 3000); + toast.success("Settings updated successfully. Changes will be applied across all pods within 10 seconds."); + }, + onError: (error) => { + toast.fromError(error); + }, + }); }; const handleTest = async () => { @@ -117,7 +167,9 @@ export default function MCPSemanticFilterSettings({ accessToken }: MCPSemanticFi }; if (!accessToken) { - return
Please log in to configure semantic filter settings.
; + return ( +
Please log in to configure semantic filter settings.
+ ); } return ( @@ -164,113 +216,132 @@ export default function MCPSemanticFilterSettings({ accessToken }: MCPSemanticFi {/* Left Column - Settings */} -
{ - setIsDirty(true); - }} - > - - - Enable Semantic Filtering - - - - - } - valuePropName="checked" - > - - + + event.preventDefault()} noValidate> + + + + {({ value, onChange, onBlur, id }) => ( + commitChange(onChange, checked)} + onBlur={onBlur} + disabled={isUpdating} + /> + )} + + + - - {schema?.properties?.enabled?.description} - - + + + + {({ value, onChange, id }) => ( + ({ + label: model.model_group, + value: model.model_group, + }))} + value={value} + onValueChange={(selected) => commitChange(onChange, selected)} + allowClear={false} + placeholder={loadingModels ? "Loading models..." : "Select embedding model"} + emptyText={loadingModels ? "Loading..." : "No embedding models available"} + disabled={isUpdating || loadingModels} + /> + )} + - - - Embedding Model - - - - - } - > - + commitChange(onChange, parseTopK(event.target.value, event.target.valueAsNumber)) + } + onBlur={() => { + onChange(clampTopK(value)); + onBlur(); + }} + disabled={isUpdating} + /> + )} + - - Top K Results - - - - - } - > - - + + {({ value, onChange, id }) => ( +
+ commitChange(onChange, Array.isArray(next) ? next[0] : next)} + disabled={isUpdating} + /> +
+ {SIMILARITY_THRESHOLD_MARKS.map((mark) => ( + + {mark.label} + + ))} +
+
+ )} + + + - - Similarity Threshold - - - - - } - > - - - - -
- -
-
+
+ +
+ + {/* Right Column - Test Configuration */} diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.integration.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.integration.test.tsx new file mode 100644 index 00000000000..bd2cfbcb57d --- /dev/null +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.integration.test.tsx @@ -0,0 +1,134 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import PluginSettings from "./PluginSettings"; + +const { getConfigFieldSettingMock, updateConfigFieldSettingMock } = vi.hoisted(() => ({ + getConfigFieldSettingMock: vi.fn(), + updateConfigFieldSettingMock: vi.fn(), +})); + +vi.mock("@/components/networking", () => ({ + getConfigFieldSetting: getConfigFieldSettingMock, + updateConfigFieldSetting: updateConfigFieldSettingMock, +})); + +const REDACTED_PLUGIN = { + name: "alpha", + display_name: "Alpha", + url: "https://alpha.example.com", + plugin_key: "***", +}; + +const savedPayload = () => updateConfigFieldSettingMock.mock.calls[0]; + +describe("PluginSettings config payload", () => { + beforeEach(() => { + vi.clearAllMocks(); + updateConfigFieldSettingMock.mockResolvedValue({}); + }); + + it("sends a new plugin with no plugin_key when the key field is left blank", async () => { + const user = userEvent.setup(); + getConfigFieldSettingMock.mockResolvedValue({ field_value: [] }); + render(); + expect(await screen.findAllByText("No data")).not.toHaveLength(0); + + await user.click(screen.getByRole("button", { name: /add plugin/i })); + await user.type(await screen.findByLabelText(/Name \(identifier\)/), "beta"); + await user.type(screen.getByLabelText(/Display Name/), "Beta"); + await user.type(screen.getByLabelText(/^URL/), "https://beta.example.com"); + await user.click(screen.getByRole("button", { name: "Save" })); + + await waitFor(() => expect(updateConfigFieldSettingMock).toHaveBeenCalledTimes(1)); + expect(savedPayload()).toStrictEqual([ + "123", + "plugins", + [ + { + name: "beta", + display_name: "Beta", + url: "https://beta.example.com", + plugin_key: undefined, + }, + ], + ]); + }); + + it("seeds the key field blank on edit and sends a blank key when it is left untouched", async () => { + const user = userEvent.setup(); + getConfigFieldSettingMock.mockResolvedValue({ field_value: [REDACTED_PLUGIN] }); + render(); + expect(await screen.findByText("Alpha")).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "edit" })); + expect(await screen.findByLabelText(/Plugin Key/)).toHaveValue(""); + + await user.click(screen.getByRole("button", { name: "Save" })); + + await waitFor(() => expect(updateConfigFieldSettingMock).toHaveBeenCalledTimes(1)); + expect(savedPayload()).toStrictEqual([ + "123", + "plugins", + [ + { + name: "alpha", + display_name: "Alpha", + url: "https://alpha.example.com", + plugin_key: "", + }, + ], + ]); + }); + + it("sends the typed key on edit when the key field is filled in", async () => { + const user = userEvent.setup(); + getConfigFieldSettingMock.mockResolvedValue({ field_value: [REDACTED_PLUGIN] }); + render(); + expect(await screen.findByText("Alpha")).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "edit" })); + await user.type(await screen.findByLabelText(/Plugin Key/), "sk-brand-new"); + await user.click(screen.getByRole("button", { name: "Save" })); + + await waitFor(() => expect(updateConfigFieldSettingMock).toHaveBeenCalledTimes(1)); + expect(savedPayload()).toStrictEqual([ + "123", + "plugins", + [ + { + name: "alpha", + display_name: "Alpha", + url: "https://alpha.example.com", + plugin_key: "sk-brand-new", + }, + ], + ]); + }); +}); + +describe("PluginSettings plugin key reveal (post-migration shadcn affordance)", () => { + beforeEach(() => { + vi.clearAllMocks(); + getConfigFieldSettingMock.mockResolvedValue({ field_value: [REDACTED_PLUGIN] }); + }); + + it("flips the key field between hidden and revealed and relabels the toggle", async () => { + const user = userEvent.setup(); + render(); + expect(await screen.findByText("Alpha")).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "edit" })); + const keyInput = await screen.findByLabelText(/Plugin Key/); + expect(keyInput).toHaveAttribute("type", "password"); + + await user.click(screen.getByRole("button", { name: "Show plugin key" })); + expect(keyInput).toHaveAttribute("type", "text"); + expect(screen.queryByRole("button", { name: "Show plugin key" })).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Hide plugin key" })); + expect(keyInput).toHaveAttribute("type", "password"); + expect(screen.queryByRole("button", { name: "Hide plugin key" })).not.toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.tsx index 8e5d6865916..c93cf812738 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/PluginSettings.tsx @@ -1,10 +1,17 @@ "use client"; import { useState, useEffect } from "react"; -import { Button, Card, Form, Input, Modal, Space, Table, Typography } from "antd"; +import { Button, Card, Modal, Space, Table, Typography } from "antd"; import { DeleteOutlined, EditOutlined, PlusOutlined } from "@ant-design/icons"; +import { Eye, EyeOff } from "lucide-react"; import { getConfigFieldSetting, updateConfigFieldSetting } from "@/components/networking"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Input } from "@/components/ui/input"; +import { InputGroup, InputGroupAddon, InputGroupButton, InputGroupInput } from "@/components/ui/input-group"; +import { useZodForm } from "@/lib/forms/useZodForm"; +import { pluginSchema, type PluginFormValues } from "./schema"; const { Title, Text, Paragraph } = Typography; @@ -15,6 +22,8 @@ interface Plugin { plugin_key?: string; } +const BLANK_PLUGIN: PluginFormValues = { name: "", display_name: "", url: "", plugin_key: undefined }; + export default function PluginSettings() { const { accessToken } = useAuthorized(); const [plugins, setPlugins] = useState([]); @@ -22,7 +31,8 @@ export default function PluginSettings() { const [saving, setSaving] = useState(false); const [modalOpen, setModalOpen] = useState(false); const [editingIndex, setEditingIndex] = useState(null); - const [form] = Form.useForm(); + const [keyVisible, setKeyVisible] = useState(false); + const form = useZodForm(pluginSchema, { defaultValues: BLANK_PLUGIN }); useEffect(() => { if (!accessToken) return; @@ -48,15 +58,17 @@ export default function PluginSettings() { const openAdd = () => { setEditingIndex(null); - form.resetFields(); + setKeyVisible(false); + form.reset(BLANK_PLUGIN); setModalOpen(true); }; const openEdit = (idx: number) => { setEditingIndex(idx); + setKeyVisible(false); // plugin_key arrives redacted ("***"); start it blank so an untouched save // keeps the stored credential instead of overwriting it with the placeholder. - form.setFieldsValue({ ...plugins[idx], plugin_key: "" }); + form.reset({ ...plugins[idx], plugin_key: "" }); setModalOpen(true); }; @@ -65,8 +77,7 @@ export default function PluginSettings() { save(updated); }; - const handleOk = async () => { - const values = await form.validateFields(); + const handleOk = async (values: PluginFormValues) => { const updated = editingIndex !== null ? plugins.map((p, i) => (i === editingIndex ? values : p)) : [...plugins, values]; await save(updated); @@ -129,44 +140,56 @@ export default function PluginSettings() { setModalOpen(false)} confirmLoading={saving} okText="Save" > -
- - - - - - - - - - - - -
+
event.preventDefault()} noValidate style={{ marginTop: 16 }}> + + + {({ ref, ...field }) => } + + + {({ ref, ...field }) => } + + + {({ ref, ...field }) => } + + + {({ ref, ...field }) => ( + + + + setKeyVisible(!keyVisible)} + aria-label={keyVisible ? "Hide plugin key" : "Show plugin key"} + > + {keyVisible ? : } + + + + )} + + +
); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/schema.ts b/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/schema.ts new file mode 100644 index 00000000000..fbf0223b157 --- /dev/null +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/PluginSettings/schema.ts @@ -0,0 +1,39 @@ +import { z } from "zod/v4"; + +const IPV4 = "(?:25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]\\d|\\d)(?:\\.(?:25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]\\d|\\d)){3}"; +const SEG = "[a-fA-F\\d]{1,4}"; +const IPV6 = + `(?:(?:${SEG}:){7}(?:${SEG}|:)|` + + `(?:${SEG}:){6}(?:${IPV4}|:${SEG}|:)|` + + `(?:${SEG}:){5}(?::${IPV4}|(?::${SEG}){1,2}|:)|` + + `(?:${SEG}:){4}(?:(?::${SEG}){0,1}:${IPV4}|(?::${SEG}){1,3}|:)|` + + `(?:${SEG}:){3}(?:(?::${SEG}){0,2}:${IPV4}|(?::${SEG}){1,4}|:)|` + + `(?:${SEG}:){2}(?:(?::${SEG}){0,3}:${IPV4}|(?::${SEG}){1,5}|:)|` + + `(?:${SEG}:){1}(?:(?::${SEG}){0,4}:${IPV4}|(?::${SEG}){1,6}|:)|` + + `(?::(?:(?::${SEG}){0,5}:${IPV4}|(?::${SEG}){1,7}|:)))(?:%[0-9a-zA-Z]{1,})?`; +const HOST = "(?:(?:[a-z\\u00a1-\\uffff0-9][-_]*)*[a-z\\u00a1-\\uffff0-9]+)"; +const DOMAIN = "(?:\\.(?:[a-z\\u00a1-\\uffff0-9]-*)*[a-z\\u00a1-\\uffff0-9]+)*"; +const TLD = "(?:\\.(?:[a-z\\u00a1-\\uffff]{2,}))"; + +const ANTD_URL_RULE_PATTERN = new RegExp( + `(?:^(?:(?:(?:[a-z]+:)?//)|www\\.)(?:\\S+(?::\\S*)?@)?` + + `(?:localhost|${IPV4}|${IPV6}|${HOST}${DOMAIN}${TLD})` + + `(?::\\d{2,5})?(?:[/?#][^\\s"]*)?$)`, + "i", +); + +const isUrl = (value: string): boolean => value.length <= 2048 && ANTD_URL_RULE_PATTERN.test(value); + +const pluginShape = { + name: z.string().min(1, "Required"), + display_name: z.string().min(1, "Required"), + url: z + .string() + .min(1, "Required") + .refine((value) => value === "" || isUrl(value), "Must be a valid URL"), + plugin_key: z.string().optional(), +}; + +export const pluginSchema = z.object(pluginShape); + +export type PluginFormValues = z.output; diff --git a/ui/litellm-dashboard/src/components/UIAccessControlForm.integration.test.tsx b/ui/litellm-dashboard/src/components/UIAccessControlForm.integration.test.tsx new file mode 100644 index 00000000000..8cbb5021423 --- /dev/null +++ b/ui/litellm-dashboard/src/components/UIAccessControlForm.integration.test.tsx @@ -0,0 +1,241 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { getSSOSettings, updateSSOSettings } from "@/components/networking"; + +import UIAccessControlForm from "./UIAccessControlForm"; + +vi.mock("@/components/networking", () => ({ + getSSOSettings: vi.fn(), + updateSSOSettings: vi.fn(), +})); + +const mockGetSSOSettings = vi.mocked(getSSOSettings); +const mockUpdateSSOSettings = vi.mocked(updateSSOSettings); + +const RESTRICTED_GROUP_PLACEHOLDER = "ui-access-group"; +const JWT_FIELD_PLACEHOLDER = "groups"; +const SUBMIT_LABEL = "Update UI Access Control"; + +// jsdom has no layout, so a Base UI popup can never leave its unpositioned `pointer-events: none` state. +const renderForm = (accessToken: string | null = "sk-test") => { + const onSuccess = vi.fn(); + render(); + return { onSuccess, user: userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }) }; +}; + +const chooseAccessMode = async (user: ReturnType, optionLabel: string) => { + await user.click(screen.getByRole("combobox")); + const options = await screen.findAllByText(optionLabel); + await user.click(options[options.length - 1]); +}; + +const typeInto = async (user: ReturnType, placeholder: string, value: string) => { + await user.type(screen.getByPlaceholderText(placeholder), value); +}; + +const submit = async (user: ReturnType) => { + await user.click(screen.getByRole("button", { name: SUBMIT_LABEL })); +}; + +const submittedPayload = () => mockUpdateSSOSettings.mock.calls[0]; + +describe("UIAccessControlForm", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetSSOSettings.mockResolvedValue({ values: {} }); + mockUpdateSSOSettings.mockResolvedValue({}); + }); + + it("sends the nested ui_access_mode payload with every field the user filled in", async () => { + const { onSuccess, user } = renderForm(); + + await chooseAccessMode(user, "Restricted SSO Group"); + await typeInto(user, RESTRICTED_GROUP_PLACEHOLDER, "admin-team"); + await typeInto(user, JWT_FIELD_PLACEHOLDER, "team_groups"); + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(1)); + expect(submittedPayload()).toStrictEqual([ + "sk-test", + { + ui_access_mode: { + type: "restricted_sso_group", + restricted_sso_group: "admin-team", + sso_group_jwt_field: "team_groups", + }, + }, + ]); + await waitFor(() => expect(onSuccess).toHaveBeenCalledTimes(1)); + }); + + it('collapses the payload to ui_access_mode "none" for all authenticated users', async () => { + const { onSuccess, user } = renderForm(); + + await chooseAccessMode(user, "All Authenticated Users"); + await typeInto(user, JWT_FIELD_PLACEHOLDER, "team_groups"); + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(1)); + expect(submittedPayload()).toStrictEqual(["sk-test", { ui_access_mode: "none" }]); + expect(screen.queryByPlaceholderText(RESTRICTED_GROUP_PLACEHOLDER)).not.toBeInTheDocument(); + await waitFor(() => expect(onSuccess).toHaveBeenCalledTimes(1)); + }); + + it("sends undefined for every field the user never touched", async () => { + const { user } = renderForm(); + + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(1)); + expect(submittedPayload()).toStrictEqual([ + "sk-test", + { + ui_access_mode: { + type: undefined, + restricted_sso_group: undefined, + sso_group_jwt_field: undefined, + }, + }, + ]); + }); + + it("blocks submission while the restricted SSO group is empty", async () => { + const { onSuccess, user } = renderForm(); + + await chooseAccessMode(user, "Restricted SSO Group"); + await submit(user); + + expect(await screen.findByText("Please enter the restricted SSO group")).toBeInTheDocument(); + expect(mockUpdateSSOSettings).not.toHaveBeenCalled(); + expect(onSuccess).not.toHaveBeenCalled(); + }); + + it("seeds the fields from a nested ui_access_mode object and resubmits them unchanged", async () => { + mockGetSSOSettings.mockResolvedValue({ + values: { + ui_access_mode: { + type: "restricted_sso_group", + restricted_sso_group: "loaded-group", + sso_group_jwt_field: "loaded_field", + }, + }, + }); + const { user } = renderForm(); + + expect(await screen.findByDisplayValue("loaded-group")).toBeInTheDocument(); + expect(screen.getByDisplayValue("loaded_field")).toBeInTheDocument(); + + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(1)); + expect(submittedPayload()).toStrictEqual([ + "sk-test", + { + ui_access_mode: { + type: "restricted_sso_group", + restricted_sso_group: "loaded-group", + sso_group_jwt_field: "loaded_field", + }, + }, + ]); + expect(mockGetSSOSettings).toHaveBeenCalledWith("sk-test"); + }); + + it("seeds the fields from the legacy flat structure, preferring team_ids_jwt_field", async () => { + mockGetSSOSettings.mockResolvedValue({ + values: { + ui_access_mode: "restricted_sso_group", + restricted_sso_group: "legacy-group", + team_ids_jwt_field: "legacy_team_ids", + sso_group_jwt_field: "legacy_groups", + }, + }); + const { user } = renderForm(); + + expect(await screen.findByDisplayValue("legacy-group")).toBeInTheDocument(); + expect(screen.getByDisplayValue("legacy_team_ids")).toBeInTheDocument(); + + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(1)); + expect(submittedPayload()).toStrictEqual([ + "sk-test", + { + ui_access_mode: { + type: "restricted_sso_group", + restricted_sso_group: "legacy-group", + sso_group_jwt_field: "legacy_team_ids", + }, + }, + ]); + }); + + it("keeps a seeded restricted SSO group out of the payload while its field is hidden", async () => { + mockGetSSOSettings.mockResolvedValue({ + values: { + ui_access_mode: "admin_only", + restricted_sso_group: "seeded-but-hidden", + sso_group_jwt_field: "seeded_field", + }, + }); + const { user } = renderForm(); + + expect(await screen.findByDisplayValue("seeded_field")).toBeInTheDocument(); + expect(screen.queryByPlaceholderText(RESTRICTED_GROUP_PLACEHOLDER)).not.toBeInTheDocument(); + + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(1)); + expect(submittedPayload()).toStrictEqual([ + "sk-test", + { + ui_access_mode: { + type: "admin_only", + restricted_sso_group: undefined, + sso_group_jwt_field: "seeded_field", + }, + }, + ]); + }); + + it("restores a typed restricted SSO group when its field is shown again", async () => { + const { user } = renderForm(); + + await chooseAccessMode(user, "Restricted SSO Group"); + await typeInto(user, RESTRICTED_GROUP_PLACEHOLDER, "typed-then-hidden"); + await chooseAccessMode(user, "All Authenticated Users"); + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(1)); + expect(submittedPayload()).toStrictEqual(["sk-test", { ui_access_mode: "none" }]); + + await chooseAccessMode(user, "Restricted SSO Group"); + expect(screen.getByPlaceholderText(RESTRICTED_GROUP_PLACEHOLDER)).toHaveValue("typed-then-hidden"); + + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).toHaveBeenCalledTimes(2)); + expect(mockUpdateSSOSettings.mock.calls[1]).toStrictEqual([ + "sk-test", + { + ui_access_mode: { + type: "restricted_sso_group", + restricted_sso_group: "typed-then-hidden", + sso_group_jwt_field: undefined, + }, + }, + ]); + }); + + it("never calls the API without an access token", async () => { + const { onSuccess, user } = renderForm(null); + + await submit(user); + + await waitFor(() => expect(mockUpdateSSOSettings).not.toHaveBeenCalled()); + expect(mockGetSSOSettings).not.toHaveBeenCalled(); + expect(onSuccess).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/UIAccessControlForm.tsx b/ui/litellm-dashboard/src/components/UIAccessControlForm.tsx index 656142a4765..8d85d46d011 100644 --- a/ui/litellm-dashboard/src/components/UIAccessControlForm.tsx +++ b/ui/litellm-dashboard/src/components/UIAccessControlForm.tsx @@ -1,46 +1,105 @@ +import { CircleHelp } from "lucide-react"; import React, { useEffect, useState } from "react"; -import { Form, Button as Button2, Select } from "antd"; -import { Text, TextInput } from "@tremor/react"; -import { getSSOSettings, updateSSOSettings } from "./networking"; +import { useWatch } from "react-hook-form"; +import { z } from "zod/v4"; + +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; +import { useZodForm } from "@/lib/forms/useZodForm"; + import { toast } from "@/lib/toast"; +import { getSSOSettings, updateSSOSettings } from "./networking"; interface UIAccessControlFormProps { accessToken: string | null; onSuccess: () => void; } -// Separate UI Access Control Form Component -const UIAccessControlForm: React.FC = ({ accessToken, onSuccess }) => { - const [form] = Form.useForm(); - const [loading, setLoading] = useState(false); +const uiAccessControlSchema = z + .object({ + ui_access_mode_type: z.string().optional(), + restricted_sso_group: z.string().optional(), + sso_group_jwt_field: z.string().optional(), + }) + .superRefine((values, ctx) => { + if (values.ui_access_mode_type !== "restricted_sso_group" || values.restricted_sso_group) { + return; + } + ctx.addIssue({ + code: "custom", + path: ["restricted_sso_group"], + message: "Please enter the restricted SSO group", + }); + }); + +type UIAccessControlFormValues = z.output; + +const UI_ACCESS_MODE_OPTIONS = [ + { value: "all_authenticated_users", label: "All Authenticated Users" }, + { value: "restricted_sso_group", label: "Restricted SSO Group" }, +] as const; + +const asRecord = (value: unknown): Record | null => + typeof value === "object" && value !== null ? (value as Record) : null; + +const asString = (value: unknown): string | undefined => (typeof value === "string" ? value : undefined); + +const toFormValues = (ssoData: unknown): UIAccessControlFormValues | null => { + const values = asRecord(asRecord(ssoData)?.values); + if (!values) { + return null; + } + + const nestedAccessMode = asRecord(values.ui_access_mode); + if (nestedAccessMode) { + return { + ui_access_mode_type: asString(nestedAccessMode.type), + restricted_sso_group: asString(nestedAccessMode.restricted_sso_group), + sso_group_jwt_field: asString(nestedAccessMode.sso_group_jwt_field), + }; + } + + const legacyAccessMode = asString(values.ui_access_mode); + if (legacyAccessMode !== undefined) { + return { + ui_access_mode_type: legacyAccessMode, + restricted_sso_group: asString(values.restricted_sso_group), + sso_group_jwt_field: asString(values.team_ids_jwt_field) || asString(values.sso_group_jwt_field), + }; + } + + return null; +}; + +const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); + +const UIAccessControlForm: React.FC = ({ accessToken, onSuccess }) => { + const form = useZodForm(uiAccessControlSchema, { defaultValues: {} }); + const [loading, setLoading] = useState(false); + const uiAccessModeType = useWatch({ control: form.control, name: "ui_access_mode_type" }); - // Load existing UI access control settings useEffect(() => { const loadUIAccessSettings = async () => { if (accessToken) { try { - const ssoData = await getSSOSettings(accessToken); - if (ssoData && ssoData.values) { - // Handle nested ui_access_mode structure - const uiAccessMode = ssoData.values.ui_access_mode; - let formValues = {}; - - if (uiAccessMode && typeof uiAccessMode === "object") { - formValues = { - ui_access_mode_type: uiAccessMode.type, - restricted_sso_group: uiAccessMode.restricted_sso_group, - sso_group_jwt_field: uiAccessMode.sso_group_jwt_field, - }; - } else if (typeof uiAccessMode === "string") { - // Handle legacy flat structure - formValues = { - ui_access_mode_type: uiAccessMode, - restricted_sso_group: ssoData.values.restricted_sso_group, - sso_group_jwt_field: ssoData.values.team_ids_jwt_field || ssoData.values.sso_group_jwt_field, - }; - } - - form.setFieldsValue(formValues); + const formValues = toFormValues(await getSSOSettings(accessToken)); + if (formValues) { + form.setValue("ui_access_mode_type", formValues.ui_access_mode_type); + form.setValue("restricted_sso_group", formValues.restricted_sso_group); + form.setValue("sso_group_jwt_field", formValues.sso_group_jwt_field); } } catch (error) { console.error("Failed to load UI access settings:", error); @@ -51,7 +110,7 @@ const UIAccessControlForm: React.FC = ({ accessToken, loadUIAccessSettings(); }, [accessToken, form]); - const handleUIAccessSubmit = async (formValues: Record) => { + const handleUIAccessSubmit = async (formValues: UIAccessControlFormValues) => { if (!accessToken) { toast.fromError("No access token available"); return; @@ -59,23 +118,16 @@ const UIAccessControlForm: React.FC = ({ accessToken, setLoading(true); try { - // Transform form data to match API expected structure - let apiPayload; - - if (formValues.ui_access_mode_type === "all_authenticated_users") { - // Set ui_access_mode to none when all_authenticated_users is selected - apiPayload = { - ui_access_mode: "none", - }; - } else { - apiPayload = { - ui_access_mode: { - type: formValues.ui_access_mode_type, - restricted_sso_group: formValues.restricted_sso_group, - sso_group_jwt_field: formValues.sso_group_jwt_field, - }, - }; - } + const apiPayload = + formValues.ui_access_mode_type === "all_authenticated_users" + ? { ui_access_mode: "none" } + : { + ui_access_mode: { + type: formValues.ui_access_mode_type, + restricted_sso_group: formValues.restricted_sso_group, + sso_group_jwt_field: formValues.sso_group_jwt_field, + }, + }; await updateSSOSettings(accessToken, apiPayload); onSuccess(); @@ -87,65 +139,83 @@ const UIAccessControlForm: React.FC = ({ accessToken, } }; + const submitMountedValues = (formValues: UIAccessControlFormValues) => + handleUIAccessSubmit( + formValues.ui_access_mode_type === "restricted_sso_group" + ? formValues + : { ...formValues, restricted_sso_group: undefined }, + ); + return ( -
-
- - Configure who can access the UI interface and how group information is extracted from JWT tokens. - -
- -
- - - - - - prevValues.ui_access_mode_type !== currentValues.ui_access_mode_type - } - > - {({ getFieldValue }) => { - const uiAccessModeType = getFieldValue("ui_access_mode_type"); - return uiAccessModeType === "restricted_sso_group" ? ( - - - - ) : null; - }} - - - - - - -
- - Update UI Access Control - + +
+
+

+ Configure who can access the UI interface and how group information is extracted from JWT tokens. +

- -
+ +
+ + + {({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( + + )} + + + {uiAccessModeType === "restricted_sso_group" && ( + + {({ ref, value, ...field }) => ( + + )} + + )} + + + {({ ref, value, ...field }) => } + + + +
+ +
+
+
+ ); }; diff --git a/ui/litellm-dashboard/src/components/shared/SearchSelect.tsx b/ui/litellm-dashboard/src/components/shared/SearchSelect.tsx index c6ae11f5729..e2ac466e275 100644 --- a/ui/litellm-dashboard/src/components/shared/SearchSelect.tsx +++ b/ui/litellm-dashboard/src/components/shared/SearchSelect.tsx @@ -25,6 +25,7 @@ interface SearchSelectProps { disabled?: boolean; className?: string; inputId?: string; + allowClear?: boolean; } const matchesQuery = (option: SearchSelectOption, query: string): boolean => { @@ -42,6 +43,7 @@ export function SearchSelect({ disabled = false, className, inputId, + allowClear = true, }: SearchSelectProps) { const selected = value === undefined || value === "" @@ -63,7 +65,7 @@ export function SearchSelect({