From 576ada8a70c89988203d79adb7ace4d1734f9f2d Mon Sep 17 00:00:00 2001 From: derhornspieler <15236687+derhornspieler@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:37:37 -0400 Subject: [PATCH] fix(ui): let the internal-issuer flow save before a federation rule id exists The Add Provider wizard required anthropic_federation_rule_id up front, but with LiteLLM as the issuer that id does not exist yet: the operator gets it from Anthropic after registering the JWKS URL the credential itself publishes. So the step that needed a rule id could not be reached without one. Mark the field optional for wif_internal_issuer and let the JWKS step PATCH it in once it is known. The Keycloak flow is unchanged, where the rule id is known in advance. --- .../provider_create_fields.json | 5 +- .../public_endpoints/public_endpoints.py | 14 ++- .../public_endpoints/test_public_endpoints.py | 33 +++++++ .../AddProviderPanel.integration.test.tsx | 99 ++++++++++++++++++- .../provider_credential_variants.test.ts | 28 ++++++ .../add_model/provider_credential_variants.ts | 14 ++- .../src/components/networking.tsx | 1 + ui/litellm-dashboard/src/lib/http/schema.d.ts | 8 ++ 8 files changed, 194 insertions(+), 8 deletions(-) diff --git a/litellm/proxy/public_endpoints/provider_create_fields.json b/litellm/proxy/public_endpoints/provider_create_fields.json index 2a640bfc6e1..79d3232edcc 100644 --- a/litellm/proxy/public_endpoints/provider_create_fields.json +++ b/litellm/proxy/public_endpoints/provider_create_fields.json @@ -372,7 +372,7 @@ "key": "anthropic_federation_rule_id", "label": "Federation Rule ID", "placeholder": null, - "tooltip": "The workload identity federation rule id, created in the Anthropic Console.", + "tooltip": "The workload identity federation rule id, created in the Anthropic Console. With the LiteLLM-signed identity source you can leave this blank and fill it in on the next step, once the generated JWKS has been registered.", "required": true, "field_type": "text", "options": null, @@ -577,6 +577,9 @@ "anthropic_issuer_ttl_seconds", "anthropic_issuer_signing_key_ref" ], + "optional_field_keys": [ + "anthropic_federation_rule_id" + ], "fixed_values": { "anthropic_identity_source": "internal_issuer" } diff --git a/litellm/types/proxy/public_endpoints/public_endpoints.py b/litellm/types/proxy/public_endpoints/public_endpoints.py index cabc047816c..82b8d5f05fc 100644 --- a/litellm/types/proxy/public_endpoints/public_endpoints.py +++ b/litellm/types/proxy/public_endpoints/public_endpoints.py @@ -31,23 +31,31 @@ class ProviderCredentialVariant(BaseModel): federation, Keycloak'. ``field_keys`` names entries in the parent ``ProviderCredentialVariants.field_definitions`` to mount when this variant is active; ``fixed_values`` are litellm_params values the variant implies (e.g. a discriminator like - ``anthropic_identity_source: keycloak``) and are submitted without a form field for them.""" + ``anthropic_identity_source: keycloak``) and are submitted without a form field for them. + ``optional_field_keys`` relaxes a globally-required field for this variant alone, for a value + only obtainable after the credential exists (the federation rule id an operator can only read + off the Anthropic Console once the generated JWKS is registered).""" id: str label: str field_keys: tuple[str, ...] + optional_field_keys: tuple[str, ...] = () fixed_values: Mapping[str, str] = Field(default_factory=dict) def _validate_variant(variant: "ProviderCredentialVariant", defined_keys: frozenset[str]) -> None: - """Each variant may only reference declared fields, and a fixed value may not also be a field the - form would mount, or the form and the payload would disagree about who owns that key.""" + """Each variant may only reference declared fields, may only relax fields it actually mounts, and a + fixed value may not also be a field the form would mount, or the form and the payload would + disagree about who owns that key.""" unresolved: Final = tuple(key for key in variant.field_keys if key not in defined_keys) if unresolved: raise ValueError(f"variant {variant.id!r} references undefined field_keys: {unresolved}") overlap: Final = sorted(frozenset(variant.field_keys) & frozenset(variant.fixed_values)) if overlap: raise ValueError(f"variant {variant.id!r} has fixed_values overlapping field_keys: {overlap}") + unmounted: Final = tuple(key for key in variant.optional_field_keys if key not in frozenset(variant.field_keys)) + if unmounted: + raise ValueError(f"variant {variant.id!r} relaxes optional_field_keys it does not mount: {unmounted}") class ProviderCredentialVariants(BaseModel): diff --git a/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py b/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py index e6ffa8ef926..77954a9c0a8 100644 --- a/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py +++ b/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py @@ -1171,3 +1171,36 @@ def test_provider_fields_without_credential_variants_still_parse(): openai = next((p for p in providers if p["provider"] == "OpenAI"), None) assert openai is not None assert openai.get("credential_variants") is None + + +def test_credential_variants_rejects_optional_field_keys_the_variant_does_not_mount(): + with pytest.raises(ValidationError, match="optional_field_keys it does not mount"): + ProviderCredentialVariants( + selector_label="Auth method", + default_variant="a", + field_definitions=[_field("x"), _field("y")], + variants=[ProviderCredentialVariant(id="a", label="A", field_keys=["x"], optional_field_keys=["y"])], + ) + + +def test_anthropic_internal_issuer_variant_relaxes_only_the_federation_rule_id(): + """The federation rule id is read off the Anthropic Console only after the generated JWKS is + registered, and the JWKS only exists once the credential is saved, so demanding it up front + would deadlock first-time setup of the LiteLLM-signed variant. Every other variant collects it + on the one and only form it has, so there it stays required.""" + app_instance = FastAPI() + app_instance.include_router(router) + test_client = TestClient(app_instance) + + response = test_client.get("/public/providers/fields") + assert response.status_code == 200 + + anthropic = next(p for p in response.json() if p["provider"] == "Anthropic") + variants_block = anthropic["credential_variants"] + field_defs_by_key = {f["key"]: f for f in variants_block["field_definitions"]} + variants_by_id = {v["id"]: v for v in variants_block["variants"]} + + assert field_defs_by_key["anthropic_federation_rule_id"]["required"] is True + assert variants_by_id["wif_internal_issuer"]["optional_field_keys"] == ["anthropic_federation_rule_id"] + for variant_id in ("api_key", "wif_token", "wif_token_file", "wif_keycloak"): + assert variants_by_id[variant_id]["optional_field_keys"] == [] diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx index 7fd157ac057..c7493b3870a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx @@ -1,4 +1,11 @@ -import { renderWithProviders, screen, waitFor, within } from "../../../../../../tests/test-utils"; +import { + chooseSelectOption, + fireEvent, + renderWithProviders, + screen, + waitFor, + within, +} from "../../../../../../tests/test-utils"; import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import AddProviderPanel from "./AddProviderPanel"; @@ -10,6 +17,7 @@ const createProviderModelCall = vi.fn(); const listAllModelsCall = vi.fn(); const getCallbacksCall = vi.fn(); const setCallbacksCall = vi.fn(); +const getCredentialJwksCall = vi.fn(); const mockAuthorized = vi.fn(); vi.mock("@/components/networking", async (importOriginal) => { @@ -23,6 +31,7 @@ vi.mock("@/components/networking", async (importOriginal) => { listAllModelsCall: (...args: unknown[]) => listAllModelsCall(...args), getCallbacksCall: (...args: unknown[]) => getCallbacksCall(...args), setCallbacksCall: (...args: unknown[]) => setCallbacksCall(...args), + getCredentialJwksCall: (...args: unknown[]) => getCredentialJwksCall(...args), }; }); @@ -50,8 +59,39 @@ vi.mock("@/app/(dashboard)/hooks/providers/useProviderFields", () => ({ field_definitions: [ { key: "api_base", label: "Upstream API Base", field_type: "text" }, { key: "api_key", label: "API Key", field_type: "password" }, + { + key: "anthropic_federation_rule_id", + label: "Federation Rule ID", + field_type: "text", + required: true, + tooltip: "Can be left blank and filled in once the JWKS is registered.", + }, + { key: "anthropic_organization_id", label: "Organization ID", field_type: "text", required: true }, + { key: "anthropic_issuer_url", label: "Issuer URL", field_type: "text", required: true }, + { key: "anthropic_issuer_subject", label: "Issuer Subject", field_type: "text", required: true }, + { + key: "anthropic_issuer_signing_key_ref", + label: "Signing Key Reference", + field_type: "text", + required: true, + }, + ], + variants: [ + { id: "api_key", label: "API Key", field_keys: ["api_base", "api_key"], fixed_values: {} }, + { + id: "wif_internal_issuer", + label: "Workload Identity Federation (LiteLLM-signed)", + field_keys: [ + "anthropic_federation_rule_id", + "anthropic_organization_id", + "anthropic_issuer_url", + "anthropic_issuer_subject", + "anthropic_issuer_signing_key_ref", + ], + optional_field_keys: ["anthropic_federation_rule_id"], + fixed_values: { anthropic_identity_source: "internal_issuer" }, + }, ], - variants: [{ id: "api_key", label: "API Key", field_keys: ["api_base", "api_key"], fixed_values: {} }], }, }, ], @@ -86,6 +126,7 @@ describe("AddProviderPanel", () => { createProviderModelCall.mockResolvedValue({ model_id: "new-id" }); getCallbacksCall.mockResolvedValue({ router_settings: {} }); setCallbacksCall.mockResolvedValue({}); + getCredentialJwksCall.mockResolvedValue({ keys: [{ kid: "kid-1", kty: "RSA", n: "n", e: "AQAB" }] }); }); it("walks provider -> credential -> discover -> review -> create, with blocked and aliases wired correctly", async () => { @@ -238,4 +279,58 @@ describe("AddProviderPanel", () => { }), ); }); + + it("saves a LiteLLM-signed credential with a blank federation rule id, then PATCHes it from the JWKS step", async () => { + discoverProviderModelsCall.mockResolvedValue({ models: ["claude-3-opus"] }); + const { user } = await setup(); + + await chooseProvider(user, "Anthropic"); + await user.type(screen.getByLabelText("Credential name"), "anthropic-wif"); + await user.click(screen.getByRole("button", { name: /Next/ })); + + await chooseSelectOption( + user, + await screen.findByRole("combobox", { name: "Authentication method" }), + "Workload Identity Federation (LiteLLM-signed)", + ); + + fireEvent.change(await screen.findByLabelText("Organization ID"), { target: { value: "org-1" } }); + fireEvent.change(screen.getByLabelText("Issuer URL"), { target: { value: "https://proxy.example.com" } }); + fireEvent.change(screen.getByLabelText("Issuer Subject"), { target: { value: "litellm-proxy" } }); + fireEvent.change(screen.getByLabelText("Signing Key Reference"), { target: { value: "os.environ/SIGNING_KEY" } }); + expect(screen.getByLabelText("Federation Rule ID")).toHaveValue(""); + + await user.click(screen.getByRole("button", { name: "Save credential" })); + + // The rule id is only readable off the Anthropic Console once the JWKS below is registered, + // and the JWKS only exists once the credential is saved, so saving must not demand it first. + await waitFor(() => + expect(credentialCreateCall).toHaveBeenCalledWith("test-access-token", { + credential_name: "anthropic-wif", + credential_values: { + anthropic_organization_id: "org-1", + anthropic_issuer_url: "https://proxy.example.com", + anthropic_issuer_subject: "litellm-proxy", + anthropic_issuer_signing_key_ref: "os.environ/SIGNING_KEY", + anthropic_identity_source: "internal_issuer", + }, + credential_info: { custom_llm_provider: "anthropic" }, + }), + ); + + expect(await screen.findByText("Register this JWKS with Anthropic")).toBeInTheDocument(); + expect(getCredentialJwksCall).toHaveBeenCalledWith("test-access-token", "anthropic-wif"); + + fireEvent.change(screen.getByLabelText("Federation Rule ID"), { target: { value: "rule-abc" } }); + await user.click(screen.getByRole("button", { name: /Next/ })); + + await waitFor(() => + expect(credentialUpdateCall).toHaveBeenCalledWith("test-access-token", "anthropic-wif", { + credential_name: "anthropic-wif", + credential_values: { anthropic_federation_rule_id: "rule-abc" }, + credential_info: { custom_llm_provider: "anthropic" }, + }), + ); + expect(await screen.findByText("claude-3-opus")).toBeInTheDocument(); + }); }); diff --git a/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.test.ts b/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.test.ts index f338c59af94..7e1ab3d61a9 100644 --- a/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.test.ts +++ b/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.test.ts @@ -48,6 +48,7 @@ const anthropicVariants: ProviderCredentialVariants = { "anthropic_issuer_url", "anthropic_issuer_signing_key_ref", ], + optional_field_keys: ["anthropic_federation_rule_id"], fixed_values: { anthropic_identity_source: "internal_issuer" }, }, { @@ -96,6 +97,23 @@ describe("resolveVariantFieldDefs", () => { }; expect(resolveVariantFieldDefs(variants, "broken").map((f) => f.key)).toEqual(["api_key"]); }); + + it("relaxes a globally-required field the variant lists in optional_field_keys", () => { + const fields = resolveVariantFieldDefs(anthropicVariants, "wif_internal_issuer"); + expect(fields.find((f) => f.key === "anthropic_federation_rule_id")?.required).toBe(false); + expect(fields.find((f) => f.key === "anthropic_organization_id")?.required).toBe(true); + }); + + it("keeps the same field required on a variant that does not relax it", () => { + const fields = resolveVariantFieldDefs(anthropicVariants, "wif_keycloak"); + expect(fields.find((f) => f.key === "anthropic_federation_rule_id")?.required).toBe(true); + }); + + it("relaxes a copy, leaving the shared field_definitions entry untouched", () => { + resolveVariantFieldDefs(anthropicVariants, "wif_internal_issuer"); + const shared = anthropicVariants.field_definitions.find((f) => f.key === "anthropic_federation_rule_id"); + expect(shared?.required).toBe(true); + }); }); describe("inferActiveVariant", () => { @@ -154,4 +172,14 @@ describe("inferActiveVariant", () => { const values = { api_base: "https://api.anthropic.com" }; expect(inferActiveVariant(anthropicVariants, values)).toBe("api_key"); }); + + it("still matches wif_internal_issuer while its relaxed federation rule id is unset", () => { + const values = { + anthropic_identity_source: "internal_issuer", + anthropic_organization_id: "org-1", + anthropic_issuer_url: "https://issuer.example.com", + anthropic_issuer_signing_key_ref: "os.environ/SIGNING_KEY", + }; + expect(inferActiveVariant(anthropicVariants, values)).toBe("wif_internal_issuer"); + }); }); diff --git a/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.ts b/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.ts index d42ac1ec5b8..359c6a42ef7 100644 --- a/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.ts +++ b/ui/litellm-dashboard/src/components/add_model/provider_credential_variants.ts @@ -9,6 +9,11 @@ export const getVariant = ( variantId: string, ): ProviderCredentialVariant | undefined => variants.variants.find((variant) => variant.id === variantId); +/** + * A variant may relax a field_definition's global `required` for itself alone, when the value is + * only obtainable after the credential exists (see `optional_field_keys` on the API model), so the + * relaxation is applied here rather than by every caller that reads a field's `required`. + */ export const resolveVariantFieldDefs = ( variants: ProviderCredentialVariants, variantId: string, @@ -18,9 +23,11 @@ export const resolveVariantFieldDefs = ( return []; } const byKey = new Map(variants.field_definitions.map((field) => [field.key, field])); + const optionalKeys = new Set(variant.optional_field_keys ?? []); return variant.field_keys .map((key) => byKey.get(key)) - .filter((field): field is ProviderCredentialFieldMetadata => field !== undefined); + .filter((field): field is ProviderCredentialFieldMetadata => field !== undefined) + .map((field) => (optionalKeys.has(field.key) ? { ...field, required: false } : field)); }; const hasValue = (value: unknown): boolean => (typeof value === "string" ? value.trim() !== "" : value != null); @@ -34,7 +41,10 @@ const isFullySatisfied = ( if (!fixedValuesMatch) { return false; } - return variant.field_keys.every((key) => !fieldsByKey.get(key)?.required || hasValue(values[key])); + const optionalKeys = new Set(variant.optional_field_keys ?? []); + return variant.field_keys.every( + (key) => optionalKeys.has(key) || !fieldsByKey.get(key)?.required || hasValue(values[key]), + ); }; /** diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 42beb237002..45f59251663 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -279,6 +279,7 @@ export interface ProviderCredentialVariant { id: string; label: string; field_keys: string[]; + optional_field_keys?: string[]; fixed_values: Record; } diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index f34427812f5..099ab878c4b 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -32022,6 +32022,9 @@ export interface components { * ``ProviderCredentialVariants.field_definitions`` to mount when this variant is active; * ``fixed_values`` are litellm_params values the variant implies (e.g. a discriminator like * ``anthropic_identity_source: keycloak``) and are submitted without a form field for them. + * ``optional_field_keys`` relaxes a globally-required field for this variant alone, for a value + * only obtainable after the credential exists (the federation rule id an operator can only read + * off the Anthropic Console once the generated JWKS is registered). */ ProviderCredentialVariant: { /** Field Keys */ @@ -32034,6 +32037,11 @@ export interface components { id: string; /** Label */ label: string; + /** + * Optional Field Keys + * @default [] + */ + optional_field_keys: string[]; }; /** * ProviderCredentialVariants