diff --git a/ui/litellm-dashboard/src/components/templates/keyEditFieldNormalizers.test.ts b/ui/litellm-dashboard/src/components/templates/keyEditFieldNormalizers.test.ts new file mode 100644 index 00000000000..44766e3737e --- /dev/null +++ b/ui/litellm-dashboard/src/components/templates/keyEditFieldNormalizers.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "vitest"; +import { keyTypeFromRoutes, normalizeKeyEditRoutePayload } from "./keyEditFieldNormalizers"; + +describe("key edit route payload normalization", () => { + it("parses comma-separated custom routes", () => { + expect(normalizeKeyEditRoutePayload({ allowed_routes: "route1, route2, route3" }, [])).toEqual({ + allowed_routes: ["route1", "route2", "route3"], + }); + }); + + it("submits the default preset without derived routes", () => { + expect(normalizeKeyEditRoutePayload({ allowed_routes: "" }, ["llm_api_routes"])).toEqual({ + key_type: "default", + }); + }); + + it("submits the AI APIs preset without derived routes", () => { + expect(normalizeKeyEditRoutePayload({ allowed_routes: "llm_api_routes" }, [])).toEqual({ + key_type: "llm_api", + }); + }); + + it("preserves mixed allowlists without applying a preset", () => { + expect(normalizeKeyEditRoutePayload({ allowed_routes: "llm_api_routes, /custom" }, [])).toEqual({ + allowed_routes: ["llm_api_routes", "/custom"], + }); + }); + + it.each([ + { original: ["llm_api_routes"], submitted: "llm_api_routes" }, + { original: null, submitted: "" }, + { original: ["beta_routes", "alpha_routes"], submitted: "alpha_routes, beta_routes" }, + ])("omits unchanged routes: $original", ({ original, submitted }) => { + expect(normalizeKeyEditRoutePayload({ allowed_routes: submitted }, original)).toEqual({}); + }); + + it("does not label a mixed allowlist as Full Access", () => { + expect(keyTypeFromRoutes(["llm_api_routes", "/custom"])).toBeUndefined(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/templates/keyEditFieldNormalizers.ts b/ui/litellm-dashboard/src/components/templates/keyEditFieldNormalizers.ts index 3e4e271c19f..9487de5bbeb 100644 --- a/ui/litellm-dashboard/src/components/templates/keyEditFieldNormalizers.ts +++ b/ui/litellm-dashboard/src/components/templates/keyEditFieldNormalizers.ts @@ -50,3 +50,45 @@ export const currentValuePlaceholder = ( if (!premiumUser) return premiumHint; return Array.isArray(current) && current.length > 0 ? `Current: ${current.join(", ")}` : emptyHint; }; + +export type KeyEditRoutePayload = { + allowed_routes?: string[] | string | null; + key_type?: string; + [key: string]: unknown; +}; + +export const normalizeKeyEditRoutePayload = ( + values: KeyEditRoutePayload, + originalAllowedRoutes: string[] | null | undefined, +): KeyEditRoutePayload => { + const normalized = { ...values }; + + if (typeof normalized.allowed_routes === "string") { + normalized.allowed_routes = normalized.allowed_routes + .split(",") + .map((route) => route.trim()) + .filter(Boolean); + } + + const originalRoutes = new Set(originalAllowedRoutes ?? []); + const submittedRoutes = new Set(Array.isArray(normalized.allowed_routes) ? normalized.allowed_routes : []); + const routesUnchanged = + originalRoutes.size === submittedRoutes.size && [...submittedRoutes].every((route) => originalRoutes.has(route)); + + if (routesUnchanged) { + delete normalized.allowed_routes; + return normalized; + } + + const preset = exactKeyTypePresetFromRoutes( + Array.isArray(normalized.allowed_routes) ? normalized.allowed_routes : undefined, + ); + if (preset) { + normalized.key_type = preset; + delete normalized.allowed_routes; + } else { + delete normalized.key_type; + } + + return normalized; +}; diff --git a/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx b/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx index 5938256f070..36b872ed7b8 100644 --- a/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx @@ -11,7 +11,6 @@ import { modelAvailableCall, vectorStoreListCall, } from "../networking"; -import { keyTypeFromRoutes } from "./keyEditFieldNormalizers"; import { KeyEditView } from "./key_edit_view"; const can = vi.fn(); @@ -135,13 +134,6 @@ vi.mock("../agent_management/AgentSelector", () => ({ ), })); - -describe("keyTypeFromRoutes", () => { - it("does not label a mixed allowlist as Full Access", () => { - expect(keyTypeFromRoutes(["llm_api_routes", "/custom"])).toBeUndefined(); - }); -}); - vi.mock("../common_components/AccessGroupSelector", () => ({ default: ({ value = [], onChange }: { value?: string[]; onChange?: (v: string[]) => void }) => ( { }); }); - it("should parse comma-separated allowed routes on submit", async () => { - const onSubmitMock = vi.fn().mockResolvedValue(undefined); - renderWithProviders( - {}} - onSubmit={onSubmitMock} - accessToken={"test-token"} - userID={"test-user"} - userRole={"admin"} - premiumUser={false} - />, - ); - - await waitFor(() => { - expect(screen.getByLabelText(/allowed routes/i)).toBeInTheDocument(); - }); - - const allowedRoutesInput = screen.getByLabelText(/allowed routes/i); - await userEvent.clear(allowedRoutesInput); - await userEvent.type(allowedRoutesInput, "route1, route2, route3"); - - const submitButton = screen.getByRole("button", { name: /save changes/i }); - await userEvent.click(submitButton); - - await waitFor(() => { - expect(onSubmitMock).toHaveBeenCalled(); - const callArgs = onSubmitMock.mock.calls[0][0]; - expect(Array.isArray(callArgs.allowed_routes)).toBe(true); - expect(callArgs.allowed_routes).toEqual(["route1", "route2", "route3"]); - }); - }); - - it("should submit the default preset without derived allowed routes", async () => { - const onSubmitMock = vi.fn().mockResolvedValue(undefined); - const keyDataWithRoutes = { - ...MOCK_KEY_DATA, - allowed_routes: ["llm_api_routes"], - }; - renderWithProviders( - {}} - onSubmit={onSubmitMock} - accessToken={"test-token"} - userID={"test-user"} - userRole={"admin"} - premiumUser={false} - />, - ); - - await waitFor(() => { - expect(screen.getByLabelText(/allowed routes/i)).toBeInTheDocument(); - }); - - const allowedRoutesInput = screen.getByLabelText(/allowed routes/i); - await userEvent.clear(allowedRoutesInput); - - const submitButton = screen.getByRole("button", { name: /save changes/i }); - await userEvent.click(submitButton); - - await waitFor(() => { - expect(onSubmitMock).toHaveBeenCalled(); - const callArgs = onSubmitMock.mock.calls[0][0]; - expect(callArgs.allowed_routes).toBeUndefined(); - expect(callArgs.key_type).toBe("default"); - }); - }); - - it("should submit the AI APIs preset without derived allowed routes", async () => { - const onSubmitMock = vi.fn().mockResolvedValue(undefined); - renderWithProviders( - {}} - onSubmit={onSubmitMock} - accessToken={"test-token"} - userID={"test-user"} - userRole={"admin"} - premiumUser={false} - />, - ); - - const allowedRoutesInput = await screen.findByLabelText(/allowed routes/i); - await userEvent.type(allowedRoutesInput, "llm_api_routes"); - await userEvent.click(screen.getByRole("button", { name: /save changes/i })); - - await waitFor(() => { - const callArgs = onSubmitMock.mock.calls[0][0]; - expect(callArgs.allowed_routes).toBeUndefined(); - expect(callArgs.key_type).toBe("llm_api"); - }); - }); - - it("should preserve mixed route allowlists without applying a preset", async () => { - const onSubmitMock = vi.fn().mockResolvedValue(undefined); - renderWithProviders( - {}} - onSubmit={onSubmitMock} - accessToken={"test-token"} - userID={"test-user"} - userRole={"admin"} - premiumUser={false} - />, - ); - - const allowedRoutesInput = await screen.findByLabelText(/allowed routes/i); - await userEvent.clear(allowedRoutesInput); - await userEvent.type(allowedRoutesInput, "llm_api_routes, /custom"); - await userEvent.click(screen.getByRole("button", { name: /save changes/i })); - - await waitFor(() => { - const callArgs = onSubmitMock.mock.calls[0][0]; - expect(callArgs.allowed_routes).toEqual(["llm_api_routes", "/custom"]); - expect(callArgs.key_type).toBeUndefined(); - }); - }); - - it("should omit allowed_routes from submit when value is unchanged", async () => { - const onSubmitMock = vi.fn().mockResolvedValue(undefined); - const aiApisKeyData = { - ...MOCK_KEY_DATA, - allowed_routes: ["llm_api_routes"], - }; - renderWithProviders( - {}} - onSubmit={onSubmitMock} - accessToken={"test-token"} - userID={"test-user"} - userRole={"admin"} - premiumUser={false} - />, - ); - - await waitFor(() => { - expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument(); - }); - - const submitButton = screen.getByRole("button", { name: /save changes/i }); - await userEvent.click(submitButton); - - await waitFor(() => { - expect(onSubmitMock).toHaveBeenCalled(); - const callArgs = onSubmitMock.mock.calls[0][0]; - expect("allowed_routes" in callArgs).toBe(false); - }); - }); - - it("should omit allowed_routes from submit when keyData.allowed_routes is null and form is untouched", async () => { - const onSubmitMock = vi.fn().mockResolvedValue(undefined); - const keyDataNullRoutes = { - ...MOCK_KEY_DATA, - allowed_routes: null as unknown as string[], - }; - renderWithProviders( - {}} - onSubmit={onSubmitMock} - accessToken={"test-token"} - userID={"test-user"} - userRole={"admin"} - premiumUser={false} - />, - ); - - await waitFor(() => { - expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument(); - }); - - const submitButton = screen.getByRole("button", { name: /save changes/i }); - await userEvent.click(submitButton); - - await waitFor(() => { - expect(onSubmitMock).toHaveBeenCalled(); - const callArgs = onSubmitMock.mock.calls[0][0]; - expect("allowed_routes" in callArgs).toBe(false); - }); - }); - - it("should omit allowed_routes from submit when server returned routes in a different order", async () => { - const onSubmitMock = vi.fn().mockResolvedValue(undefined); - const keyDataReordered = { - ...MOCK_KEY_DATA, - allowed_routes: ["beta_routes", "alpha_routes"], - }; - renderWithProviders( - {}} - onSubmit={onSubmitMock} - accessToken={"test-token"} - userID={"test-user"} - userRole={"admin"} - premiumUser={false} - />, - ); - - await waitFor(() => { - expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument(); - }); - - const submitButton = screen.getByRole("button", { name: /save changes/i }); - await userEvent.click(submitButton); - - await waitFor(() => { - expect(onSubmitMock).toHaveBeenCalled(); - const callArgs = onSubmitMock.mock.calls[0][0]; - expect("allowed_routes" in callArgs).toBe(false); - }); - }); - it("should pass access_group_ids to onSubmit when saving key with access groups", async () => { const onSubmitMock = vi.fn().mockResolvedValue(undefined); const keyDataWithAccessGroups = { diff --git a/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx b/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx index 8ef875761dc..6cb131a321f 100644 --- a/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx @@ -27,11 +27,10 @@ import RouterSettingsAccordion, { RouterSettingsAccordionRef } from "../common_c import { routerSettingsEditorValue, routerSettingsUpdate } from "../common_components/routerSettingsPayload"; import { estimateTooltips, withNormalizedEstimates } from "./estimatedOutputTokens"; import { - canonicalBudgetDuration, currentValuePlaceholder, - exactKeyTypePresetFromRoutes, keyTypeFromRoutes, modelSentinelOptions, + normalizeKeyEditRoutePayload, parseAllowedRoutes, } from "./keyEditFieldNormalizers"; import { KeyTypeSelect, labelWithHint } from "./KeyEditViewControls"; @@ -215,40 +214,7 @@ export function KeyEditView({ try { setIsKeySaving(true); - // Parse allowed_routes from comma-separated string to array - if (typeof values.allowed_routes === "string") { - const trimmedInput = values.allowed_routes.trim(); - if (trimmedInput === "") { - values.allowed_routes = []; - } else { - values.allowed_routes = trimmedInput - .split(",") - .map((route: string) => route.trim()) - .filter((route: string) => route.length > 0); - } - } - // If it's already an array (shouldn't happen, but handle it), keep as is - - // Backend rejects non-empty allowed_routes from non-admins, so re-sending - // an unchanged value 403s a team admin. Set compare tolerates reorder. - const originalRoutesSet = new Set(Array.isArray(keyData.allowed_routes) ? keyData.allowed_routes : []); - const submittedRoutesSet = new Set(Array.isArray(values.allowed_routes) ? values.allowed_routes : []); - const allowedRoutesUnchanged = - originalRoutesSet.size === submittedRoutesSet.size && - [...submittedRoutesSet].every((r) => originalRoutesSet.has(r)); - if (allowedRoutesUnchanged) { - delete values.allowed_routes; - } else { - const exactKeyTypePreset = exactKeyTypePresetFromRoutes(values.allowed_routes); - if (exactKeyTypePreset) { - values.key_type = exactKeyTypePreset; - // The backend derives allowed_routes from key_type. Omitting the - // derived routes also lets authorized non-admins use safe presets. - delete values.allowed_routes; - } else { - delete values.key_type; - } - } + values = normalizeKeyEditRoutePayload(values, keyData.allowed_routes); if (neverExpire) { values.duration = null;