From a799351a5fc2df21cc053b3ca51189aba06805fd Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 18 Aug 2026 19:55:03 -0700 Subject: [PATCH] refactor(ui): extract the create-key payload builder out of create_key_button (#37397) * refactor(ui): extract the create-key payload builder out of create_key_button handleCreate built the POST /key/generate body inline by mutating the object antd handed it, across roughly 170 lines mixing form values, twelve pieces of React state and five sources that all fold into object_permission. Nothing could assert that shape without rendering the whole modal. The construction now lives in createKeyPayload.ts as a pure function returning a tagged union, with 46 unit tests that run in single digit milliseconds and pin whole payloads with toStrictEqual, following the createServerPayload pattern the dashboard CLAUDE.md documents. Behaviour is unchanged. The extraction was checked against the pre-extraction handler with a differential harness over 50 input combinations, comparing the built object, its key order and its serialised bytes, and eight mutations of the new module were each confirmed to fail the committed tests. * refactor(ui): align the key payload builder to the programme's entry-condition spec Folds the duplicate-alias guard and the endpoint choice into the builder, so KeyPayloadResult now carries three variants and handleCreate holds no payload decision of its own. The two failure branches keep their original position around toast.info and setIsModalVisible, and the builder checks the alias before the agent selection, so the observable order is unchanged. Pins the serialised wire shape as well as the object. The closed form registers team_id at null through initialValue while organization_id has no initialValue and stays undefined, so an untouched create sends seven of its eight keys. Opening Optional Settings takes the object to 23 keys and the wire to nine. Both directions of the definedness contract are now covered: undefined must not become null, and null must not be dropped. The earlier fixture fed a team_id the closed form cannot produce and pinned a six-key wire as a result. * refactor(ui): fold the service-account metadata write into its only caller Removes assignServiceAccountId as a separate helper so there is no mutating function available for reuse, which was the substance of the review finding. The write now sits two lines below the JSON.parse that produced the value, so it is visibly local and cannot reach a caller-owned object. The write itself stays. Metadata has no validation rules, so a user can submit a JSON body that parses to a primitive or an array. On a primitive the property write raises a TypeError and the existing catch surfaces an error toast, and on an array it leaves the array intact through JSON.stringify. A spread coerces both to plain objects instead, silently creating a key from input the form rejects today. Two tests pin those cases and go red against the spread. --- .../organisms/createKeyPayload.test.ts | 556 ++++++++++++++++++ .../components/organisms/createKeyPayload.ts | 212 +++++++ .../organisms/create_key_button.tsx | 216 +------ 3 files changed, 798 insertions(+), 186 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/organisms/createKeyPayload.test.ts create mode 100644 ui/litellm-dashboard/src/components/organisms/createKeyPayload.ts diff --git a/ui/litellm-dashboard/src/components/organisms/createKeyPayload.test.ts b/ui/litellm-dashboard/src/components/organisms/createKeyPayload.test.ts new file mode 100644 index 00000000000..67729d63fc3 --- /dev/null +++ b/ui/litellm-dashboard/src/components/organisms/createKeyPayload.test.ts @@ -0,0 +1,556 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { buildKeyCreatePayload, type KeyCreateInput, type KeyPayloadResult } from "./createKeyPayload"; + +const baseInput: KeyCreateInput = { + formValues: {}, + existingKeys: null, + keyOwner: "you", + userID: "test-user", + selectedAgentId: null, + loggingSettings: [], + disabledCallbacks: [], + autoRotationEnabled: false, + rotationInterval: "30d", + modelAliases: {}, + routerSettings: null, + budgetLimits: [], + tagRateLimits: [], + budgetFallbacks: {}, +}; + +const build = (formValues: Record, overrides: Partial = {}): KeyPayloadResult => + buildKeyCreatePayload({ ...baseInput, ...overrides, formValues }); + +const payloadOf = (result: KeyPayloadResult): Record => { + expect(result.kind).toBe("ok"); + if (result.kind !== "ok") throw new Error("unreachable"); + return result.payload; +}; + +const wireKeys = (payload: Record): string[] => + Object.keys(JSON.parse(JSON.stringify(payload)) as Record); + +const DROPPED_AT_SERIALISATION = [ + "access_group_ids", + "allowed_passthrough_routes", + "allowed_vector_store_ids", + "budget_duration", + "enable_prompt_caching", + "guardrails", + "max_budget", + "organization_id", + "policies", + "prompts", + "rpm_limit", + "tags", + "throttle_on_budget_exceeded", + "tpm_limit", +]; + +const CLOSED_SECTIONS_VALUES = { + organization_id: undefined, + team_id: null, + key_alias: "my-key", + models: [], + key_type: "llm_api", +}; + +const OPTIONAL_SETTINGS_VALUES = { + ...CLOSED_SECTIONS_VALUES, + max_budget: undefined, + budget_duration: undefined, + tpm_limit: undefined, + tpm_limit_type: "key", + rpm_limit: undefined, + rpm_limit_type: "key", + throttle_on_budget_exceeded: undefined, + enable_prompt_caching: undefined, + guardrails: undefined, + disable_global_guardrails: undefined, + policies: undefined, + prompts: undefined, + access_group_ids: undefined, + allowed_passthrough_routes: undefined, + allowed_vector_store_ids: undefined, + tags: undefined, +}; + +const aliasOnly = (overrides: Record = {}): Record => ({ + key_alias: "my-key", + user_id: "test-user", + duration: null, + metadata: "{}", + ...overrides, +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("always-present keys", () => { + it("emits the eight keys the closed form sends, and nothing else", () => { + const closedFormValues = { ...CLOSED_SECTIONS_VALUES }; + const closedFormPayload = { + ...closedFormValues, + user_id: "test-user", + duration: null, + metadata: "{}", + }; + expect(payloadOf(build(closedFormValues))).toStrictEqual(closedFormPayload); + }); + + it("injects user_id, duration and metadata even when the form reported none of them", () => { + expect(payloadOf(build({ key_alias: "my-key" }))).toStrictEqual(aliasOnly()); + }); + + it("keeps a mounted-but-untouched field as an undefined-valued key rather than dropping it", () => { + expect(payloadOf(build({ key_alias: "my-key", guardrails: undefined, tags: undefined }))).toStrictEqual( + aliasOnly({ guardrails: undefined, tags: undefined }), + ); + }); +}); + +describe("duration", () => { + it("forwards a typed duration by value", () => { + expect(payloadOf(build({ key_alias: "my-key", duration: "45d" }))).toStrictEqual(aliasOnly({ duration: "45d" })); + }); + + it.each([ + ["an empty string", ""], + ["a whitespace-only string", " "], + ["undefined", undefined], + ])("coalesces %s to null", (_label, duration) => { + expect(payloadOf(build({ key_alias: "my-key", duration }))).toStrictEqual(aliasOnly({ duration: null })); + }); + + it("does not coerce a non-string duration", () => { + expect(() => build({ key_alias: "my-key", duration: 30 })).toThrow(TypeError); + }); +}); + +describe("key ownership", () => { + it("overwrites user_id with the signed-in user when the key is owned by you", () => { + expect(payloadOf(build({ key_alias: "my-key", user_id: "someone-else" }))).toStrictEqual( + aliasOnly({ user_id: "test-user" }), + ); + }); + + it("leaves the form's user_id alone for another_user", () => { + const expected = { key_alias: "my-key", user_id: "someone-else", duration: null, metadata: "{}" }; + expect( + payloadOf(build({ key_alias: "my-key", user_id: "someone-else" }, { keyOwner: "another_user" })), + ).toStrictEqual(expected); + }); + + it("adds the selected agent id for an agent-owned key", () => { + const expected = { key_alias: "my-key", agent_id: "agent-1", duration: null, metadata: "{}" }; + expect(payloadOf(build({ key_alias: "my-key" }, { keyOwner: "agent", selectedAgentId: "agent-1" }))).toStrictEqual( + expected, + ); + }); + + it("reports agent_not_selected instead of building a payload when no agent is selected", () => { + expect(build({ key_alias: "my-key" }, { keyOwner: "agent", selectedAgentId: null })).toStrictEqual({ + kind: "agent_not_selected", + }); + }); + + it("stamps the alias into metadata as the service account id and sends no user_id", () => { + expect(payloadOf(build({ key_alias: "svc-key" }, { keyOwner: "service_account" }))).toStrictEqual({ + key_alias: "svc-key", + duration: null, + metadata: '{"service_account_id":"svc-key"}', + }); + }); +}); + +describe("metadata", () => { + it("re-serialises the parsed form value", () => { + expect(payloadOf(build({ key_alias: "my-key", metadata: '{"team":"core"}' }))).toStrictEqual( + aliasOnly({ metadata: '{"team":"core"}' }), + ); + }); + + it("falls back to an empty object and logs when the JSON is malformed", () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + expect(payloadOf(build({ key_alias: "my-key", metadata: "{not json" }))).toStrictEqual(aliasOnly()); + expect(consoleError).toHaveBeenCalledWith("Error parsing metadata:", expect.any(SyntaxError)); + }); + + it("merges logging configs, dropping rows with no callback selected", () => { + expect( + payloadOf( + build( + { key_alias: "my-key", metadata: '{"team":"core"}' }, + { loggingSettings: [{ callback_name: "langfuse" }, { callback_name: "" }] }, + ), + ), + ).toStrictEqual(aliasOnly({ metadata: '{"team":"core","logging":[{"callback_name":"langfuse"}]}' })); + }); + + it("maps disabled callbacks from display names to internal names", () => { + expect(payloadOf(build({ key_alias: "my-key" }, { disabledCallbacks: ["Langfuse"] }))).toStrictEqual( + aliasOnly({ metadata: '{"litellm_disabled_callbacks":["langfuse"]}' }), + ); + }); + + it("keeps an array metadata as an array when stamping the service account id", () => { + expect( + payloadOf(build({ key_alias: "svc-key", metadata: '["a"]' }, { keyOwner: "service_account" })), + ).toStrictEqual({ key_alias: "svc-key", duration: null, metadata: '["a"]' }); + }); + + it("rejects a service account whose metadata parses to a primitive", () => { + expect(() => build({ key_alias: "svc-key", metadata: "5" }, { keyOwner: "service_account" })).toThrow(TypeError); + }); + + it("keeps every metadata contributor in one object", () => { + expect( + payloadOf( + build( + { key_alias: "svc-key", metadata: '{"team":"core"}' }, + { + keyOwner: "service_account", + loggingSettings: [{ callback_name: "otel" }], + disabledCallbacks: ["Datadog"], + }, + ), + ), + ).toStrictEqual({ + key_alias: "svc-key", + duration: null, + metadata: + '{"team":"core","service_account_id":"svc-key","logging":[{"callback_name":"otel"}],"litellm_disabled_callbacks":["datadog"]}', + }); + }); +}); + +describe("object_permission", () => { + it("is absent when nothing contributes to it", () => { + expect(payloadOf(build({ key_alias: "my-key", allowed_vector_store_ids: [] }))).toStrictEqual( + aliasOnly({ allowed_vector_store_ids: [] }), + ); + }); + + it("moves selected vector stores off the top level", () => { + expect(payloadOf(build({ key_alias: "my-key", allowed_vector_store_ids: ["vs-1"] }))).toStrictEqual( + aliasOnly({ object_permission: { vector_stores: ["vs-1"] } }), + ); + }); + + it("splits an MCP selection into servers, access groups and toolsets", () => { + expect( + payloadOf( + build({ + key_alias: "my-key", + allowed_mcp_servers_and_groups: { servers: ["s-1"], accessGroups: ["g-1"], toolsets: ["t-1"] }, + }), + ), + ).toStrictEqual( + aliasOnly({ + object_permission: { mcp_servers: ["s-1"], mcp_access_groups: ["g-1"], mcp_toolsets: ["t-1"] }, + }), + ); + }); + + it("omits the empty halves of an MCP selection", () => { + expect( + payloadOf( + build({ + key_alias: "my-key", + allowed_mcp_servers_and_groups: { servers: ["s-1"], accessGroups: [], toolsets: [] }, + }), + ), + ).toStrictEqual(aliasOnly({ object_permission: { mcp_servers: ["s-1"] } })); + }); + + it("leaves an all-empty MCP selection on the top level", () => { + expect( + payloadOf( + build({ key_alias: "my-key", allowed_mcp_servers_and_groups: { servers: [], accessGroups: [], toolsets: [] } }), + ), + ).toStrictEqual(aliasOnly({ allowed_mcp_servers_and_groups: { servers: [], accessGroups: [], toolsets: [] } })); + }); + + it("nests configured MCP tool permissions", () => { + expect(payloadOf(build({ key_alias: "my-key", mcp_tool_permissions: { "s-1": ["read"] } }))).toStrictEqual( + aliasOnly({ object_permission: { mcp_tool_permissions: { "s-1": ["read"] } } }), + ); + }); + + it("always strips mcp_tool_permissions from the top level, even when empty", () => { + expect(payloadOf(build({ key_alias: "my-key", mcp_tool_permissions: {} }))).toStrictEqual(aliasOnly()); + }); + + it("lets a standalone access group list win over the one from the MCP selection", () => { + expect( + payloadOf( + build({ + key_alias: "my-key", + allowed_mcp_servers_and_groups: { servers: ["s-1"], accessGroups: ["from-selector"] }, + allowed_mcp_access_groups: ["standalone"], + }), + ), + ).toStrictEqual(aliasOnly({ object_permission: { mcp_servers: ["s-1"], mcp_access_groups: ["standalone"] } })); + }); + + it("splits an agent selection into agents and agent access groups", () => { + expect( + payloadOf(build({ key_alias: "my-key", allowed_agents_and_groups: { agents: ["a-1"], accessGroups: ["ag-1"] } })), + ).toStrictEqual(aliasOnly({ object_permission: { agents: ["a-1"], agent_access_groups: ["ag-1"] } })); + }); + + it("merges every source into a single object_permission", () => { + const everySource = { + key_alias: "my-key", + allowed_vector_store_ids: ["vs-1"], + allowed_mcp_servers_and_groups: { servers: ["s-1"], accessGroups: ["g-1"], toolsets: ["t-1"] }, + mcp_tool_permissions: { "s-1": ["read"] }, + allowed_agents_and_groups: { agents: ["a-1"], accessGroups: ["ag-1"] }, + }; + expect(payloadOf(build(everySource))).toStrictEqual( + aliasOnly({ + object_permission: { + vector_stores: ["vs-1"], + mcp_servers: ["s-1"], + mcp_access_groups: ["g-1"], + mcp_toolsets: ["t-1"], + mcp_tool_permissions: { "s-1": ["read"] }, + agents: ["a-1"], + agent_access_groups: ["ag-1"], + }, + }), + ); + }); +}); + +describe("premium and rotation flags", () => { + it("drops disable_global_guardrails when it is off", () => { + expect(payloadOf(build({ key_alias: "my-key", disable_global_guardrails: false }))).toStrictEqual(aliasOnly()); + }); + + it("keeps disable_global_guardrails when it is on", () => { + expect(payloadOf(build({ key_alias: "my-key", disable_global_guardrails: true }))).toStrictEqual( + aliasOnly({ disable_global_guardrails: true }), + ); + }); + + it("adds the rotation fields only when auto rotation is enabled", () => { + expect( + payloadOf(build({ key_alias: "my-key" }, { autoRotationEnabled: true, rotationInterval: "7d" })), + ).toStrictEqual(aliasOnly({ auto_rotate: true, rotation_interval: "7d" })); + }); + + it("sends no rotation fields when auto rotation is off", () => { + expect(payloadOf(build({ key_alias: "my-key" }, { rotationInterval: "7d" }))).toStrictEqual(aliasOnly()); + }); +}); + +describe("keys sourced from component state", () => { + it("serialises model aliases", () => { + expect(payloadOf(build({ key_alias: "my-key" }, { modelAliases: { fast: "gpt-4o-mini" } }))).toStrictEqual( + aliasOnly({ aliases: '{"fast":"gpt-4o-mini"}' }), + ); + }); + + it("sends router settings that hold at least one value", () => { + expect( + payloadOf(build({ key_alias: "my-key" }, { routerSettings: { router_settings: { num_retries: 3 } } })), + ).toStrictEqual(aliasOnly({ router_settings: { num_retries: 3 } })); + }); + + it("skips router settings whose every field is blank", () => { + expect( + payloadOf( + build( + { key_alias: "my-key" }, + { routerSettings: { router_settings: { num_retries: null, timeout: undefined, routing_strategy: "" } } }, + ), + ), + ).toStrictEqual(aliasOnly()); + }); + + it("keeps only budget windows that carry both a duration and a limit", () => { + expect( + payloadOf( + build( + { key_alias: "my-key" }, + { + budgetLimits: [ + { budget_duration: "1h", max_budget: 5 }, + { budget_duration: "", max_budget: 3 }, + { budget_duration: "7d", max_budget: null }, + ], + }, + ), + ), + ).toStrictEqual(aliasOnly({ budget_limits: [{ budget_duration: "1h", max_budget: 5 }] })); + }); + + it("keeps a zero budget window rather than treating it as unset", () => { + expect( + payloadOf(build({ key_alias: "my-key" }, { budgetLimits: [{ budget_duration: "1h", max_budget: 0 }] })), + ).toStrictEqual(aliasOnly({ budget_limits: [{ budget_duration: "1h", max_budget: 0 }] })); + }); + + it("omits budget_limits when no window is complete", () => { + expect( + payloadOf(build({ key_alias: "my-key" }, { budgetLimits: [{ budget_duration: "7d", max_budget: null }] })), + ).toStrictEqual(aliasOnly()); + }); + + it("reduces tag rows into a tag_rpm_limit map", () => { + expect( + payloadOf( + build( + { key_alias: "my-key" }, + { + tagRateLimits: [ + { id: "r-1", tag: "prod", rpm_limit: 10 }, + { id: "r-2", tag: " ", rpm_limit: 5 }, + { id: "r-3", tag: "dev", rpm_limit: null }, + ], + }, + ), + ), + ).toStrictEqual(aliasOnly({ tag_rpm_limit: { prod: 10 } })); + }); + + it("omits tag_rpm_limit when no row is complete", () => { + expect( + payloadOf(build({ key_alias: "my-key" }, { tagRateLimits: [{ id: "r-1", tag: "", rpm_limit: 10 }] })), + ).toStrictEqual(aliasOnly()); + }); + + it("sends configured budget fallbacks", () => { + expect(payloadOf(build({ key_alias: "my-key" }, { budgetFallbacks: { "gpt-4": ["gpt-4o"] } }))).toStrictEqual( + aliasOnly({ budget_fallbacks: { "gpt-4": ["gpt-4o"] } }), + ); + }); +}); + +describe("budget duration", () => { + it("turns the never-resets sentinel into null", () => { + expect(payloadOf(build({ key_alias: "my-key", budget_duration: "none" }))).toStrictEqual( + aliasOnly({ budget_duration: null }), + ); + }); + + it("forwards a real budget duration untouched", () => { + expect(payloadOf(build({ key_alias: "my-key", budget_duration: "30d" }))).toStrictEqual( + aliasOnly({ budget_duration: "30d" }), + ); + }); +}); + +describe("purity", () => { + it("leaves the submitted form values untouched", () => { + const values = { + key_alias: "my-key", + mcp_tool_permissions: { "s-1": ["read"] }, + allowed_vector_store_ids: ["vs-1"], + disable_global_guardrails: false, + duration: "", + }; + const before = structuredClone(values); + build(values); + expect(values).toStrictEqual(before); + }); +}); + +describe("serialised wire shape", () => { + it("keeps an untouched closed form at eight object keys and seven wire keys", () => { + const payload = payloadOf(build(CLOSED_SECTIONS_VALUES)); + expect(Object.keys(payload)).toHaveLength(8); + expect(wireKeys(payload)).toStrictEqual([ + "team_id", + "key_alias", + "models", + "key_type", + "user_id", + "duration", + "metadata", + ]); + expect(payload.duration).toBeNull(); + }); + + it("drops the undefined picker and keeps the null one, which is what the two Form.Items differ on", () => { + const payload = payloadOf(build(CLOSED_SECTIONS_VALUES)); + expect(payload.organization_id).toBeUndefined(); + expect(payload.team_id).toBeNull(); + expect(wireKeys(payload)).not.toContain("organization_id"); + expect(wireKeys(payload)).toContain("team_id"); + }); + + it("forwards a selected team by value", () => { + expect(payloadOf(build({ ...CLOSED_SECTIONS_VALUES, team_id: "team-1" })).team_id).toBe("team-1"); + }); + + it("adds fifteen keys to the object and only the two limit types to the wire when Optional Settings opens", () => { + const payload = payloadOf(build(OPTIONAL_SETTINGS_VALUES)); + expect(Object.keys(payload)).toHaveLength(23); + expect(wireKeys(payload)).toStrictEqual([ + "team_id", + "key_alias", + "models", + "key_type", + "tpm_limit_type", + "rpm_limit_type", + "user_id", + "duration", + "metadata", + ]); + }); + + it("never turns an undefined-valued key into null or an empty string", () => { + const payload = payloadOf(build(OPTIONAL_SETTINGS_VALUES)); + DROPPED_AT_SERIALISATION.forEach((key) => { + expect(payload[key]).toBeUndefined(); + }); + expect(wireKeys(payload)).toEqual(expect.not.arrayContaining(DROPPED_AT_SERIALISATION)); + }); +}); + +describe("duplicate alias", () => { + it("reports the clash instead of building a payload", () => { + expect( + build({ key_alias: "taken", team_id: "team-1" }, { existingKeys: [{ team_id: "team-1", key_alias: "taken" }] }), + ).toStrictEqual({ kind: "duplicate_alias", alias: "taken", teamId: "team-1" }); + }); + + it("scopes the clash to the same team", () => { + expect( + payloadOf( + build({ key_alias: "taken", team_id: "team-2" }, { existingKeys: [{ team_id: "team-1", key_alias: "taken" }] }), + ).key_alias, + ).toBe("taken"); + }); + + it("treats a keyless form and a teamless key as the same bucket", () => { + expect(build({}, { existingKeys: [{ team_id: null, key_alias: "" }] })).toStrictEqual({ + kind: "duplicate_alias", + alias: "", + teamId: null, + }); + }); + + it("checks the alias before the agent selection", () => { + expect( + build( + { key_alias: "taken" }, + { existingKeys: [{ team_id: null, key_alias: "taken" }], keyOwner: "agent", selectedAgentId: null }, + ).kind, + ).toBe("duplicate_alias"); + }); +}); + +describe("endpoint", () => { + it.each([ + ["you", "standard"], + ["another_user", "standard"], + ["service_account", "service_account"], + ])("routes a %s key to the %s endpoint", (keyOwner, endpoint) => { + const result = build({ key_alias: "my-key" }, { keyOwner }); + expect(result.kind === "ok" && result.endpoint).toBe(endpoint); + }); +}); diff --git a/ui/litellm-dashboard/src/components/organisms/createKeyPayload.ts b/ui/litellm-dashboard/src/components/organisms/createKeyPayload.ts new file mode 100644 index 00000000000..2b14e8372f7 --- /dev/null +++ b/ui/litellm-dashboard/src/components/organisms/createKeyPayload.ts @@ -0,0 +1,212 @@ +import { mapDisplayToInternalNames } from "../callback_info_helpers"; +import { NEVER_RESETS_BUDGET_DURATION } from "../common_components/budget_duration_dropdown"; +import type { RouterSettingsAccordionValue } from "../common_components/RouterSettingsAccordion"; +import type { BudgetWindowEntry } from "../key_team_helpers/BudgetWindowsEditor"; +import { tagRowsToLimits, type TagRateLimitEntry } from "../key_team_helpers/TagRateLimitEditor"; + +export interface KeyLoggingSetting { + callback_name?: string; +} + +export interface ExistingKey { + readonly team_id?: string | null; + readonly key_alias?: string | null; +} + +export interface KeyCreateInput { + readonly formValues: Record; + readonly existingKeys: readonly ExistingKey[] | null; + readonly keyOwner: string; + readonly userID: string | null; + readonly selectedAgentId: string | null; + readonly loggingSettings: KeyLoggingSetting[]; + readonly disabledCallbacks: string[]; + readonly autoRotationEnabled: boolean; + readonly rotationInterval: string; + readonly modelAliases: Record; + readonly routerSettings: RouterSettingsAccordionValue | null; + readonly budgetLimits: BudgetWindowEntry[]; + readonly tagRateLimits: TagRateLimitEntry[]; + readonly budgetFallbacks: Record; +} + +export type KeyPayloadResult = + | { + readonly kind: "ok"; + readonly payload: Record; + readonly endpoint: "standard" | "service_account"; + } + | { readonly kind: "duplicate_alias"; readonly alias: string; readonly teamId: string | null } + | { readonly kind: "agent_not_selected" }; + +interface McpSelection { + readonly servers?: unknown[]; + readonly accessGroups?: unknown[]; + readonly toolsets?: unknown[]; +} + +interface AgentSelection { + readonly agents?: unknown[]; + readonly accessGroups?: unknown[]; +} + +const nonEmptyList = (raw: unknown): unknown[] | undefined => { + const list = raw as unknown[] | undefined; + return list && list.length > 0 ? list : undefined; +}; + +const readMcpSelection = (raw: unknown): McpSelection | undefined => { + const selection = raw as McpSelection | undefined; + if (!selection) return undefined; + const servers = nonEmptyList(selection.servers); + const accessGroups = nonEmptyList(selection.accessGroups); + const toolsets = nonEmptyList(selection.toolsets); + if (!servers && !accessGroups && !toolsets) return undefined; + return { servers, accessGroups, toolsets }; +}; + +const readAgentSelection = (raw: unknown): AgentSelection | undefined => { + const selection = raw as AgentSelection | undefined; + if (!selection) return undefined; + const agents = nonEmptyList(selection.agents); + const accessGroups = nonEmptyList(selection.accessGroups); + if (!agents && !accessGroups) return undefined; + return { agents, accessGroups }; +}; + +const readToolPermissions = (raw: unknown): unknown | undefined => { + const permissions = raw || {}; + return Object.keys(permissions as object).length > 0 ? permissions : undefined; +}; + +const parseMetadata = (raw: unknown): unknown => { + try { + return JSON.parse((raw as string) || "{}"); + } catch (error) { + console.error("Error parsing metadata:", error); + return {}; + } +}; + +const buildMetadataJson = (values: Record, input: KeyCreateInput): string => { + const parsed = parseMetadata(values.metadata); + if (input.keyOwner === "service_account") { + (parsed as Record).service_account_id = values.key_alias; + } + const logged = + input.loggingSettings.length > 0 + ? { ...(parsed as object), logging: input.loggingSettings.filter((config) => config.callback_name) } + : parsed; + const disabled = + input.disabledCallbacks.length > 0 + ? { ...(logged as object), litellm_disabled_callbacks: mapDisplayToInternalNames(input.disabledCallbacks) } + : logged; + return JSON.stringify(disabled); +}; + +interface PermissionSources { + readonly vectorStores: unknown[] | undefined; + readonly mcp: McpSelection | undefined; + readonly toolPermissions: unknown | undefined; + readonly extraMcpAccessGroups: unknown[] | undefined; + readonly agents: AgentSelection | undefined; +} + +const readPermissionSources = (values: Record): PermissionSources => ({ + vectorStores: nonEmptyList(values.allowed_vector_store_ids), + mcp: readMcpSelection(values.allowed_mcp_servers_and_groups), + toolPermissions: readToolPermissions(values.mcp_tool_permissions), + extraMcpAccessGroups: nonEmptyList(values.allowed_mcp_access_groups), + agents: readAgentSelection(values.allowed_agents_and_groups), +}); + +const buildObjectPermission = ({ + vectorStores, + mcp, + toolPermissions, + extraMcpAccessGroups, + agents, +}: PermissionSources): Record | undefined => { + const permission: Record = { + ...(vectorStores && { vector_stores: vectorStores }), + ...(mcp?.servers && { mcp_servers: mcp.servers }), + ...(mcp?.accessGroups && { mcp_access_groups: mcp.accessGroups }), + ...(mcp?.toolsets && { mcp_toolsets: mcp.toolsets }), + ...(toolPermissions !== undefined && { mcp_tool_permissions: toolPermissions }), + ...(extraMcpAccessGroups && { mcp_access_groups: extraMcpAccessGroups }), + ...(agents?.agents && { agents: agents.agents }), + ...(agents?.accessGroups && { agent_access_groups: agents.accessGroups }), + }; + return Object.keys(permission).length > 0 ? permission : undefined; +}; + +const consumedSourceKeys = ( + values: Record, + { vectorStores, mcp, extraMcpAccessGroups, agents }: PermissionSources, +): ReadonlySet => + new Set([ + "mcp_tool_permissions", + ...(values.disable_global_guardrails ? [] : ["disable_global_guardrails"]), + ...(vectorStores ? ["allowed_vector_store_ids"] : []), + ...(mcp ? ["allowed_mcp_servers_and_groups"] : []), + ...(extraMcpAccessGroups ? ["allowed_mcp_access_groups"] : []), + ...(agents ? ["allowed_agents_and_groups"] : []), + ]); + +const withoutKeys = (values: Record, dropped: ReadonlySet): Record => + Object.fromEntries(Object.entries(values).filter(([key]) => !dropped.has(key))); + +const duplicateAlias = (input: KeyCreateInput): { alias: string; teamId: string | null } | undefined => { + const alias = (input.formValues?.key_alias as string | undefined) ?? ""; + const teamId = (input.formValues?.team_id as string | undefined) ?? null; + const taken = (input.existingKeys ?? []).filter((key) => key.team_id === teamId).map((key) => key.key_alias); + return taken.includes(alias) ? { alias, teamId } : undefined; +}; + +export const buildKeyCreatePayload = (input: KeyCreateInput): KeyPayloadResult => { + const duplicate = duplicateAlias(input); + if (duplicate) { + return { kind: "duplicate_alias", ...duplicate }; + } + if (input.keyOwner === "agent" && !input.selectedAgentId) { + return { kind: "agent_not_selected" }; + } + + const values = input.formValues; + + const sources = readPermissionSources(values); + const objectPermission = buildObjectPermission(sources); + const dropped = consumedSourceKeys(values, sources); + + const duration = values.duration; + const validWindows = input.budgetLimits.filter( + (window) => window.budget_duration && window.max_budget !== null && window.max_budget !== undefined, + ); + const { tag_rpm_limit } = tagRowsToLimits(input.tagRateLimits); + const routerSettings = input.routerSettings?.router_settings; + const configuredRouterSettings = + routerSettings && + Object.values(routerSettings).some((value) => value !== null && value !== undefined && value !== "") + ? routerSettings + : undefined; + + return { + kind: "ok", + endpoint: input.keyOwner === "service_account" ? "service_account" : "standard", + payload: { + ...withoutKeys(values, dropped), + ...(input.keyOwner === "you" && { user_id: input.userID }), + ...(input.keyOwner === "agent" && { agent_id: input.selectedAgentId }), + ...(input.autoRotationEnabled && { auto_rotate: true, rotation_interval: input.rotationInterval }), + duration: !duration || (duration as string).trim() === "" ? null : duration, + metadata: buildMetadataJson(values, input), + ...(objectPermission && { object_permission: objectPermission }), + ...(Object.keys(input.modelAliases).length > 0 && { aliases: JSON.stringify(input.modelAliases) }), + ...(configuredRouterSettings && { router_settings: configuredRouterSettings }), + ...(validWindows.length > 0 && { budget_limits: validWindows }), + ...(Object.keys(tag_rpm_limit).length > 0 && { tag_rpm_limit }), + ...(Object.keys(input.budgetFallbacks).length > 0 && { budget_fallbacks: input.budgetFallbacks }), + ...(values.budget_duration === NEVER_RESETS_BUDGET_DURATION && { budget_duration: null }), + }, + }; +}; diff --git a/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx b/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx index 9bb5ed51bfa..77ef31e0ede 100644 --- a/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx +++ b/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx @@ -30,9 +30,8 @@ import { DEBOUNCE_WAIT_MS } from "@/utils/debounceConstants"; import React, { useEffect, useState } from "react"; import { rolesWithWriteAccess } from "../../utils/roles"; import AgentSelector from "../agent_management/AgentSelector"; -import { mapDisplayToInternalNames } from "../callback_info_helpers"; import AccessGroupSelector from "../common_components/AccessGroupSelector"; -import BudgetDurationDropdown, { NEVER_RESETS_BUDGET_DURATION } from "../common_components/budget_duration_dropdown"; +import BudgetDurationDropdown from "../common_components/budget_duration_dropdown"; import SchemaFormFields from "../common_components/check_openapi_schema"; import KeyLifecycleSettings from "../common_components/KeyLifecycleSettings"; import ModelAliasManager from "../common_components/ModelAliasManager"; @@ -46,7 +45,7 @@ import ProjectDropdown from "../common_components/ProjectDropdown"; import { CreateUserButton } from "../CreateUserButton"; import { BudgetFallbacksEditor } from "../key_team_helpers/BudgetFallbacksEditor"; import { BudgetWindowEntry, BudgetWindowsEditor } from "../key_team_helpers/BudgetWindowsEditor"; -import { TagRateLimitEditor, TagRateLimitEntry, tagRowsToLimits } from "../key_team_helpers/TagRateLimitEditor"; +import { TagRateLimitEditor, TagRateLimitEntry } from "../key_team_helpers/TagRateLimitEditor"; import { excludeProxyWideSentinel, getModelDisplayName, @@ -72,6 +71,7 @@ import { import CreatedKeyDisplay from "../shared/CreatedKeyDisplay"; import NumericalInput from "../shared/numerical_input"; import VectorStoreSelector from "../vector_store_management/VectorStoreSelector"; +import { buildKeyCreatePayload, type KeyCreateInput } from "./createKeyPayload"; import { simplifyKeyGenerateError } from "./utils"; const { Option } = Select; @@ -378,198 +378,42 @@ const CreateKey: React.FC = ({ team, teams, data, addKey, autoOp const handleCreate = async (formValues: Record) => { try { - const newKeyAlias = formValues?.key_alias ?? ""; - const newKeyTeamId = formValues?.team_id ?? null; - - const existingKeyAliases = data?.filter((k) => k.team_id === newKeyTeamId).map((k) => k.key_alias) ?? []; - - if (existingKeyAliases.includes(newKeyAlias)) { + const input: KeyCreateInput = { + formValues, + existingKeys: data, + keyOwner, + userID, + selectedAgentId, + loggingSettings, + disabledCallbacks, + autoRotationEnabled, + rotationInterval, + modelAliases, + routerSettings, + budgetLimits, + tagRateLimits, + budgetFallbacks, + }; + const built = buildKeyCreatePayload(input); + if (built.kind === "duplicate_alias") { throw new Error( - `Key alias ${newKeyAlias} already exists for team with ID ${newKeyTeamId}, please provide another key alias`, + `Key alias ${built.alias} already exists for team with ID ${built.teamId}, please provide another key alias`, ); } toast.info("Making API Call"); setIsModalVisible(true); - if (keyOwner === "you") { - formValues.user_id = userID; - } else if (keyOwner === "agent") { - if (!selectedAgentId) { - toast.fromError("Please select an agent"); - return; - } - formValues.agent_id = selectedAgentId; + if (built.kind === "agent_not_selected") { + toast.fromError("Please select an agent"); + return; } + const { payload, endpoint } = built; - // Handle metadata for all key types - let metadata: Record = {}; - try { - metadata = JSON.parse(formValues.metadata || "{}"); - } catch (error) { - console.error("Error parsing metadata:", error); - } - - // If it's a service account, add the service_account_id to the metadata - if (keyOwner === "service_account") { - metadata["service_account_id"] = formValues.key_alias; - } - - // Add logging settings to the metadata - if (loggingSettings.length > 0) { - metadata = { - ...metadata, - logging: loggingSettings.filter((config) => config.callback_name), - }; - } - - // Add disabled callbacks to the metadata - if (disabledCallbacks.length > 0) { - // Map display names to internal callback values - const mappedDisabledCallbacks = mapDisplayToInternalNames(disabledCallbacks); - metadata = { - ...metadata, - litellm_disabled_callbacks: mappedDisabledCallbacks, - }; - } - - // Add auto-rotation settings as top-level fields - if (autoRotationEnabled) { - formValues.auto_rotate = true; - formValues.rotation_interval = rotationInterval; - } - - // Handle duration field for key expiry - convert empty string to null - if (!formValues.duration || formValues.duration.trim() === "") { - formValues.duration = null; - } - - // Update the formValues with the final metadata - formValues.metadata = JSON.stringify(metadata); - - // disable_global_guardrails is premium-gated server-side; only send it when enabled - // so non-premium key creation isn't blocked by that gate. - if (!formValues.disable_global_guardrails) { - delete formValues.disable_global_guardrails; - } - - // Transform allowed_vector_store_ids and allowed_mcp_servers_and_groups into object_permission format - if (formValues.allowed_vector_store_ids && formValues.allowed_vector_store_ids.length > 0) { - formValues.object_permission = { - vector_stores: formValues.allowed_vector_store_ids, - }; - // Remove the original field as it's now part of object_permission - delete formValues.allowed_vector_store_ids; - } - - // Transform allowed_mcp_servers_and_groups into object_permission format - if ( - formValues.allowed_mcp_servers_and_groups && - (formValues.allowed_mcp_servers_and_groups.servers?.length > 0 || - formValues.allowed_mcp_servers_and_groups.accessGroups?.length > 0 || - formValues.allowed_mcp_servers_and_groups.toolsets?.length > 0) - ) { - if (!formValues.object_permission) { - formValues.object_permission = {}; - } - const { servers, accessGroups, toolsets } = formValues.allowed_mcp_servers_and_groups; - if (servers && servers.length > 0) { - formValues.object_permission.mcp_servers = servers; - } - if (accessGroups && accessGroups.length > 0) { - formValues.object_permission.mcp_access_groups = accessGroups; - } - if (toolsets && toolsets.length > 0) { - formValues.object_permission.mcp_toolsets = toolsets; - } - // Remove the original field as it's now part of object_permission - delete formValues.allowed_mcp_servers_and_groups; - } - - // Add MCP tool permissions to object_permission - const mcpToolPermissions = formValues.mcp_tool_permissions || {}; - if (Object.keys(mcpToolPermissions).length > 0) { - if (!formValues.object_permission) { - formValues.object_permission = {}; - } - formValues.object_permission.mcp_tool_permissions = mcpToolPermissions; - } - delete formValues.mcp_tool_permissions; - - // Transform allowed_mcp_access_groups into object_permission format - if (formValues.allowed_mcp_access_groups && formValues.allowed_mcp_access_groups.length > 0) { - if (!formValues.object_permission) { - formValues.object_permission = {}; - } - formValues.object_permission.mcp_access_groups = formValues.allowed_mcp_access_groups; - // Remove the original field as it's now part of object_permission - delete formValues.allowed_mcp_access_groups; - } - - // Transform allowed_agents_and_groups into object_permission format - if ( - formValues.allowed_agents_and_groups && - (formValues.allowed_agents_and_groups.agents?.length > 0 || - formValues.allowed_agents_and_groups.accessGroups?.length > 0) - ) { - if (!formValues.object_permission) { - formValues.object_permission = {}; - } - const { agents, accessGroups } = formValues.allowed_agents_and_groups; - if (agents && agents.length > 0) { - formValues.object_permission.agents = agents; - } - if (accessGroups && accessGroups.length > 0) { - formValues.object_permission.agent_access_groups = accessGroups; - } - // Remove the original field as it's now part of object_permission - delete formValues.allowed_agents_and_groups; - } - - // Add model_aliases if any are defined - if (Object.keys(modelAliases).length > 0) { - formValues.aliases = JSON.stringify(modelAliases); - } - - // Add router_settings if any are defined - if (routerSettings?.router_settings) { - // Only include router_settings if it has at least one non-null value - const hasValues = Object.values(routerSettings.router_settings).some( - (value) => value !== null && value !== undefined && value !== "", - ); - if (hasValues) { - formValues.router_settings = routerSettings.router_settings; - } - } - - // Add multi-window budget limits (filter out incomplete entries) - const validWindows = budgetLimits.filter( - (w) => w.budget_duration && w.max_budget !== null && w.max_budget !== undefined, - ); - if (validWindows.length > 0) { - formValues.budget_limits = validWindows; - } - - // Add per-tag rate limits (only when at least one row is configured) - const { tag_rpm_limit } = tagRowsToLimits(tagRateLimits); - if (Object.keys(tag_rpm_limit).length > 0) { - formValues.tag_rpm_limit = tag_rpm_limit; - } - - if (Object.keys(budgetFallbacks).length > 0) { - formValues.budget_fallbacks = budgetFallbacks; - } - - if (formValues.budget_duration === NEVER_RESETS_BUDGET_DURATION) { - formValues.budget_duration = null; - } - - let response; - if (keyOwner === "service_account") { - response = await keyCreateServiceAccountCall(accessToken, formValues); - } else { - response = await keyCreateCall(accessToken, userID, formValues); - } + const response = + endpoint === "service_account" + ? await keyCreateServiceAccountCall(accessToken, payload) + : await keyCreateCall(accessToken, userID, payload); // Add the data to the state in the parent component // Also directly update the keys list in VirtualKeysTable without an API call