From 9331e97d360f1acdd70b6ef00837673202d887e8 Mon Sep 17 00:00:00 2001 From: jesus Date: Sat, 31 Oct 2026 23:08:41 +0000 Subject: [PATCH] fix(proxy): make /update/sso_settings a true PATCH and send only changed SSO fields from the UI --- .../proxy_setting_endpoints.py | 11 +- .../test_proxy_setting_endpoints.py | 109 ++++++++++++++ .../src/components/SSOModals.test.tsx | 23 +-- .../src/components/SSOModals.tsx | 134 ++++++++---------- .../Modals/AddSSOSettingsModal.tsx | 7 +- .../Modals/DeleteSSOSettingsModal.tsx | 26 +--- .../EditSSOSettingsModal.integration.test.tsx | 52 ++++++- .../Modals/EditSSOSettingsModal.test.tsx | 2 + .../Modals/EditSSOSettingsModal.tsx | 7 +- .../AdminSettings/SSOSettings/utils.test.ts | 57 +++++++- .../AdminSettings/SSOSettings/utils.ts | 43 ++++++ 11 files changed, 339 insertions(+), 132 deletions(-) diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index e5348684d78..8b8b9c11746 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -1310,9 +1310,14 @@ async def update_sso_settings( if "general_settings" not in config: config["general_settings"] = {} - # Update environment variables in config and in memory - sso_data: Final = sso_config.model_dump() - for field_name, value in sso_data.items(): + # PATCH semantics: omitted fields keep their stored value, an explicit null clears it + submitted: Final = sso_config.model_dump(include=sso_config.model_fields_set) + stored_values: Final = before_sso_data or {} + sso_data: Final = { + field_name: submitted[field_name] if field_name in submitted else stored_values.get(field_name) + for field_name in SSOConfig.model_fields + } + for field_name, value in submitted.items(): if field_name in SSO_FIELD_ENV_VARS: env_var_name = SSO_FIELD_ENV_VARS[field_name] if value: diff --git a/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py b/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py index ea39d5d59e2..2992995f28e 100644 --- a/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py +++ b/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py @@ -722,6 +722,115 @@ class TestProxySettingEndpoints: create_sso_settings = json.loads(create_data["sso_settings"]) assert create_sso_settings["google_client_id"] == "new_google_client_id" + def _patch_sso_settings(self, monkeypatch, stored, body): + """PATCH /update/sso_settings against a stored row; returns (response, saved row).""" + import json + from unittest.mock import AsyncMock, MagicMock + + monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", True) + mock_prisma = MagicMock() + stored_record = None + if stored is not None: + stored_record = MagicMock() + stored_record.sso_settings = stored + mock_prisma.db.litellm_ssoconfig.find_unique = AsyncMock( + return_value=stored_record + ) + mock_prisma.db.litellm_ssoconfig.upsert = AsyncMock() + mock_prisma.writer_db.litellm_config.find_unique = AsyncMock(return_value=None) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma) + + from litellm.proxy.proxy_server import proxy_config + + monkeypatch.setattr( + proxy_config, "encrypt_env_variables", lambda environment_variables: environment_variables + ) + monkeypatch.setattr(proxy_config, "decrypt_db_variables", lambda variables: variables) + + response = client.patch("/update/sso_settings", json=body) + upsert_kwargs = mock_prisma.db.litellm_ssoconfig.upsert.call_args.kwargs + return response, json.loads(upsert_kwargs["data"]["update"]["sso_settings"]) + + _STORED_GENERIC_SSO = { + "generic_client_id": "stored-client-id", + "generic_client_secret": "stored-client-secret", + "generic_authorization_endpoint": "https://idp.example.com/authorize", + "user_email": "admin@example.com", + "role_mappings": { + "provider": "generic", + "group_claim": "groups", + "default_role": "internal_user", + "roles": {"proxy_admin": ["admins"]}, + }, + } + + def test_update_sso_settings_omitted_fields_keep_stored_values( + self, mock_proxy_config, mock_auth, monkeypatch + ): + monkeypatch.setenv("GENERIC_CLIENT_SECRET", "stored-client-secret") + new_role_mappings = { + "provider": "generic", + "group_claim": "roles", + "default_role": "internal_user", + "roles": {"proxy_admin": ["platform-admins"]}, + } + + response, saved = self._patch_sso_settings( + monkeypatch, self._STORED_GENERIC_SSO, {"role_mappings": new_role_mappings} + ) + + assert response.status_code == 200 + assert saved["generic_client_secret"] == "stored-client-secret" + assert saved["generic_client_id"] == "stored-client-id" + assert saved["user_email"] == "admin@example.com" + assert saved["role_mappings"]["group_claim"] == "roles" + assert os.environ["GENERIC_CLIENT_SECRET"] == "stored-client-secret" + + def test_update_sso_settings_null_clears_only_that_field( + self, mock_proxy_config, mock_auth, monkeypatch + ): + monkeypatch.setenv("GENERIC_CLIENT_SECRET", "stored-client-secret") + monkeypatch.setenv("GENERIC_CLIENT_ID", "stored-client-id") + + response, saved = self._patch_sso_settings( + monkeypatch, + self._STORED_GENERIC_SSO, + {"generic_client_secret": None, "role_mappings": None}, + ) + + assert response.status_code == 200 + assert saved["generic_client_secret"] is None + assert saved["role_mappings"] is None + assert saved["generic_client_id"] == "stored-client-id" + assert "GENERIC_CLIENT_SECRET" not in os.environ + assert os.environ["GENERIC_CLIENT_ID"] == "stored-client-id" + + def test_update_sso_settings_new_value_replaces_stored_value( + self, mock_proxy_config, mock_auth, monkeypatch + ): + response, saved = self._patch_sso_settings( + monkeypatch, self._STORED_GENERIC_SSO, {"generic_client_secret": "rotated-secret"} + ) + + assert response.status_code == 200 + assert saved["generic_client_secret"] == "rotated-secret" + assert saved["generic_client_id"] == "stored-client-id" + assert os.environ["GENERIC_CLIENT_SECRET"] == "rotated-secret" + + def test_update_sso_settings_omitted_field_leaves_env_only_value_alone( + self, mock_proxy_config, mock_auth, monkeypatch + ): + monkeypatch.setenv("GENERIC_CLIENT_SECRET", "env-only-secret") + + response, saved = self._patch_sso_settings( + monkeypatch, None, {"user_email": "admin@example.com"} + ) + + assert response.status_code == 200 + assert saved["user_email"] == "admin@example.com" + assert saved["generic_client_secret"] is None + assert os.environ["GENERIC_CLIENT_SECRET"] == "env-only-secret" + def test_update_sso_settings_maps_saml_fields_to_env_vars( self, mock_proxy_config, mock_auth, monkeypatch ): diff --git a/ui/litellm-dashboard/src/components/SSOModals.test.tsx b/ui/litellm-dashboard/src/components/SSOModals.test.tsx index bee5f13c626..d78de8adaef 100644 --- a/ui/litellm-dashboard/src/components/SSOModals.test.tsx +++ b/ui/litellm-dashboard/src/components/SSOModals.test.tsx @@ -19,6 +19,7 @@ vi.mock("./shared/errorUtils", () => ({ import { toast } from "@/lib/toast"; import { getSSOSettings, updateSSOSettings } from "./networking"; +import { CLEARED_SSO_SETTINGS } from "./Settings/AdminSettings/SSOSettings/utils"; describe("SSOModals", () => { it("should render the SSOModals component", () => { @@ -515,27 +516,7 @@ describe("SSOModals", () => { // Verify the clear API was called with null values await waitFor(() => { - expect(updateSSOSettings).toHaveBeenCalledWith("test-token", { - google_client_id: null, - google_client_secret: null, - microsoft_client_id: null, - microsoft_client_secret: null, - microsoft_tenant: null, - generic_client_id: null, - generic_client_secret: null, - generic_authorization_endpoint: null, - generic_token_endpoint: null, - generic_userinfo_endpoint: null, - saml_idp_metadata_url: null, - saml_idp_metadata_xml: null, - saml_sp_entity_id: null, - saml_allow_unsolicited: null, - generic_scope: null, - proxy_base_url: null, - user_email: null, - sso_provider: null, - role_mappings: null, - }); + expect(updateSSOSettings).toHaveBeenCalledWith("test-token", CLEARED_SSO_SETTINGS); }); expect(toast.success).toHaveBeenCalledWith("SSO settings cleared successfully"); diff --git a/ui/litellm-dashboard/src/components/SSOModals.tsx b/ui/litellm-dashboard/src/components/SSOModals.tsx index 609c0efcd99..5c4825698a0 100644 --- a/ui/litellm-dashboard/src/components/SSOModals.tsx +++ b/ui/litellm-dashboard/src/components/SSOModals.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useState } from "react"; import { FormProvider, useWatch, type UseFormReturn } from "react-hook-form"; import { getSSOSettings, updateSSOSettings } from "./networking"; +import { CLEARED_SSO_SETTINGS, diffSSOSettingsPayload } from "./Settings/AdminSettings/SSOSettings/utils"; import { toast } from "@/lib/toast"; import { parseErrorMessage } from "./shared/errorUtils"; import { Button } from "@/components/ui/button"; @@ -32,6 +33,60 @@ interface SSOModalsProps { ssoConfigured?: boolean; // Add optional prop to indicate if SSO is configured } +const buildLegacySSOPayload = (formValues: SSOSettingsFormValues): Record => { + const { + proxy_admin_teams, + admin_viewer_teams, + internal_user_teams, + internal_viewer_teams, + default_role, + group_claim, + use_role_mappings, + ...rest + } = formValues; + + const payload: Record = { + ...rest, + }; + + if (typeof payload.saml_allow_unsolicited === "boolean") { + payload.saml_allow_unsolicited = payload.saml_allow_unsolicited ? "true" : "false"; + } + + // Add role mappings if use_role_mappings is checked + if (use_role_mappings) { + // Helper function to split comma-separated string into array + const splitTeams = (teams: string | undefined): string[] => { + if (!teams || teams.trim() === "") return []; + return teams + .split(",") + .map((team) => team.trim()) + .filter((team) => team.length > 0); + }; + + // Map default role display values to backend values + const defaultRoleMapping: Record = { + internal_user_viewer: "internal_user_viewer", + internal_user: "internal_user", + proxy_admin_viewer: "proxy_admin_viewer", + proxy_admin: "proxy_admin", + }; + + payload.role_mappings = { + provider: "generic", + group_claim, + default_role: (default_role ? defaultRoleMapping[default_role] : undefined) || "internal_user", + roles: { + proxy_admin: splitTeams(proxy_admin_teams), + proxy_admin_viewer: splitTeams(admin_viewer_teams), + internal_user: splitTeams(internal_user_teams), + internal_user_viewer: splitTeams(internal_viewer_teams), + }, + }; + } + return payload; +}; + const detectSSOProvider = (values: Record): string | null => { if (values.google_client_id) return "google"; if (values.microsoft_client_id) return "microsoft"; @@ -134,56 +189,10 @@ const SSOModals: React.FC = ({ } try { - const { - proxy_admin_teams, - admin_viewer_teams, - internal_user_teams, - internal_viewer_teams, - default_role, - group_claim, - use_role_mappings, - ...rest - } = formValues; - - const payload: Record = { - ...rest, - }; - - if (typeof payload.saml_allow_unsolicited === "boolean") { - payload.saml_allow_unsolicited = payload.saml_allow_unsolicited ? "true" : "false"; - } - - // Add role mappings if use_role_mappings is checked - if (use_role_mappings) { - // Helper function to split comma-separated string into array - const splitTeams = (teams: string | undefined): string[] => { - if (!teams || teams.trim() === "") return []; - return teams - .split(",") - .map((team) => team.trim()) - .filter((team) => team.length > 0); - }; - - // Map default role display values to backend values - const defaultRoleMapping: Record = { - internal_user_viewer: "internal_user_viewer", - internal_user: "internal_user", - proxy_admin_viewer: "proxy_admin_viewer", - proxy_admin: "proxy_admin", - }; - - payload.role_mappings = { - provider: "generic", - group_claim, - default_role: (default_role ? defaultRoleMapping[default_role] : undefined) || "internal_user", - roles: { - proxy_admin: splitTeams(proxy_admin_teams), - proxy_admin_viewer: splitTeams(admin_viewer_teams), - internal_user: splitTeams(internal_user_teams), - internal_user_viewer: splitTeams(internal_viewer_teams), - }, - }; - } + const payload = diffSSOSettingsPayload( + buildLegacySSOPayload({ ...emptySSOSettingsFormValues, ...form.formState.defaultValues }), + buildLegacySSOPayload(formValues), + ); // Save SSO settings using the new API await updateSSOSettings(accessToken, payload); @@ -203,30 +212,7 @@ const SSOModals: React.FC = ({ } try { - // Clear all SSO settings - const clearSettings = { - google_client_id: null, - google_client_secret: null, - microsoft_client_id: null, - microsoft_client_secret: null, - microsoft_tenant: null, - generic_client_id: null, - generic_client_secret: null, - generic_authorization_endpoint: null, - generic_token_endpoint: null, - generic_userinfo_endpoint: null, - saml_idp_metadata_url: null, - saml_idp_metadata_xml: null, - saml_sp_entity_id: null, - saml_allow_unsolicited: null, - generic_scope: null, - proxy_base_url: null, - user_email: null, - sso_provider: null, - role_mappings: null, - }; - - await updateSSOSettings(accessToken, clearSettings); + await updateSSOSettings(accessToken, CLEARED_SSO_SETTINGS); // Clear the form form.reset(emptySSOSettingsFormValues); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/AddSSOSettingsModal.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/AddSSOSettingsModal.tsx index 7fa16515bf5..a16598653cf 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/AddSSOSettingsModal.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/AddSSOSettingsModal.tsx @@ -12,7 +12,7 @@ import BaseSSOSettingsForm, { import { Button } from "@/components/ui/button"; import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; import { useEditSSOSettings } from "@/app/(dashboard)/hooks/sso/useEditSSOSettings"; -import { processSSOSettingsPayload } from "../utils"; +import { diffSSOSettingsPayload, processSSOSettingsPayload } from "../utils"; import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog"; interface AddSSOSettingsModalProps { @@ -26,7 +26,10 @@ const AddSSOSettingsModal: React.FC = ({ isVisible, on const { mutateAsync, isPending } = useEditSSOSettings(); const handleFormSubmit = async (formValues: SSOSettingsFormValues) => { - const payload = processSSOSettingsPayload(formValues); + const payload = diffSSOSettingsPayload( + processSSOSettingsPayload(emptySSOSettingsFormValues), + processSSOSettingsPayload(formValues), + ); await mutateAsync(payload, { onSuccess: () => { diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/DeleteSSOSettingsModal.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/DeleteSSOSettingsModal.tsx index 053dac499bd..416ec4d2432 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/DeleteSSOSettingsModal.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/DeleteSSOSettingsModal.tsx @@ -4,7 +4,7 @@ import React from "react"; import DeleteResourceModal from "../../../../common_components/DeleteResourceModal"; import { toast } from "@/lib/toast"; import { parseErrorMessage } from "../../../../shared/errorUtils"; -import { detectSSOProvider } from "../utils"; +import { CLEARED_SSO_SETTINGS, detectSSOProvider } from "../utils"; interface DeleteSSOSettingsModalProps { isVisible: boolean; @@ -18,29 +18,7 @@ const DeleteSSOSettingsModal: React.FC = ({ isVisib // Handle clearing SSO settings const handleClearSSO = async () => { - const clearSettings = { - google_client_id: null, - google_client_secret: null, - microsoft_client_id: null, - microsoft_client_secret: null, - microsoft_tenant: null, - generic_client_id: null, - generic_client_secret: null, - generic_authorization_endpoint: null, - generic_token_endpoint: null, - generic_userinfo_endpoint: null, - saml_idp_metadata_url: null, - saml_idp_metadata_xml: null, - saml_sp_entity_id: null, - saml_allow_unsolicited: null, - proxy_base_url: null, - user_email: null, - sso_provider: null, - role_mappings: null, - team_mappings: null, - }; - - await editSSOSettings(clearSettings, { + await editSSOSettings(CLEARED_SSO_SETTINGS, { onSuccess: () => { toast.success("SSO settings cleared successfully"); onCancel(); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.integration.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.integration.test.tsx index 14bfe5d6ac9..cddcffc6926 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.integration.test.tsx @@ -49,7 +49,7 @@ describe("EditSSOSettingsModal (real form tree)", () => { expect(mutateAsync).not.toHaveBeenCalled(); }); - it("sends only the mounted fields when the form is valid", async () => { + it("sends only the changed field and leaves the masked secret out", async () => { const user = userEvent.setup({ pointerEventsCheck: 0 }); renderWithProviders(); @@ -58,12 +58,54 @@ describe("EditSSOSettingsModal (real form tree)", () => { await user.click(saveButton()); await waitFor(() => expect(mutateAsync).toHaveBeenCalledTimes(1)); - expect(mutateAsync.mock.calls[0][0]).toEqual({ - sso_provider: "google", - google_client_id: "rotated-client-id", - google_client_secret: "stored-client-secret", + expect(mutateAsync.mock.calls[0][0]).toEqual({ google_client_id: "rotated-client-id" }); + }); + + it("sends an empty patch when nothing changed", async () => { + const user = userEvent.setup({ pointerEventsCheck: 0 }); + renderWithProviders(); + + await screen.findByLabelText("Google Client ID"); + await user.click(saveButton()); + + await waitFor(() => expect(mutateAsync).toHaveBeenCalledTimes(1)); + expect(mutateAsync.mock.calls[0][0]).toEqual({}); + }); + + it("sends a new secret when the admin types one", async () => { + const user = userEvent.setup({ pointerEventsCheck: 0 }); + renderWithProviders(); + + fireEvent.change(await screen.findByLabelText("Google Client Secret"), { target: { value: "new-secret" } }); + await user.click(saveButton()); + + await waitFor(() => expect(mutateAsync).toHaveBeenCalledTimes(1)); + expect(mutateAsync.mock.calls[0][0]).toEqual({ google_client_secret: "new-secret" }); + }); + + it("sends role_mappings null when the admin unchecks role mappings", async () => { + seed({ + generic_client_id: "stored-client-id", + generic_client_secret: "kc-s****6e0d", + generic_authorization_endpoint: "https://idp.example.com/authorize", + generic_token_endpoint: "https://idp.example.com/token", + generic_userinfo_endpoint: "https://idp.example.com/userinfo", user_email: "admin@example.com", proxy_base_url: "https://gateway.example.com", + role_mappings: { + provider: "generic", + group_claim: "groups", + default_role: "internal_user", + roles: { proxy_admin: ["admins"], proxy_admin_viewer: [], internal_user: [], internal_user_viewer: [] }, + }, }); + const user = userEvent.setup({ pointerEventsCheck: 0 }); + renderWithProviders(); + + await user.click((await screen.findAllByLabelText("Use Role Mappings"))[0]); + await user.click(saveButton()); + + await waitFor(() => expect(mutateAsync).toHaveBeenCalledTimes(1)); + expect(mutateAsync.mock.calls[0][0]).toEqual({ role_mappings: null }); }); }); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.test.tsx index ef78e3827d8..132800faf9f 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.test.tsx @@ -293,6 +293,8 @@ describe("EditSSOSettingsModal", () => { useEditSSOSettings: { mutateAsync: mockMutateAsync, isPending: false }, }); + (processSSOSettingsPayload as any).mockReturnValueOnce({}).mockReturnValueOnce(processedPayload); + renderComponent(); fireEvent.click(screen.getByTestId(TEST_IDS.TRIGGER_FORM_SUBMIT)); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.tsx index 58c03eda979..706597e305b 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/EditSSOSettingsModal.tsx @@ -11,7 +11,7 @@ import { Button } from "@/components/ui/button"; import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; import { toast } from "@/lib/toast"; import { parseErrorMessage } from "@/components/shared/errorUtils"; -import { detectSSOProvider, processSSOSettingsPayload } from "../utils"; +import { detectSSOProvider, diffSSOSettingsPayload, processSSOSettingsPayload } from "../utils"; import { useSSOSettings, type SSOSettingsValues } from "@/app/(dashboard)/hooks/sso/useSSOSettings"; import { useEditSSOSettings } from "@/app/(dashboard)/hooks/sso/useEditSSOSettings"; import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog"; @@ -86,7 +86,10 @@ const EditSSOSettingsModal: React.FC = ({ isVisible, const handleFormSubmit = async (formValues: SSOSettingsFormValues) => { try { - const payload = processSSOSettingsPayload(formValues); + const payload = diffSSOSettingsPayload( + processSSOSettingsPayload(seededValues), + processSSOSettingsPayload(formValues), + ); await mutateAsync(payload, { onSuccess: () => { diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.test.ts b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.test.ts index b280f5e92ec..b0d519afcbc 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.test.ts +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.test.ts @@ -1,4 +1,4 @@ -import { detectSSOProvider, processSSOSettingsPayload } from "./utils"; +import { detectSSOProvider, diffSSOSettingsPayload, processSSOSettingsPayload } from "./utils"; import { describe, it, expect } from "vitest"; import type { SSOSettingsValues } from "@/app/(dashboard)/hooks/sso/useSSOSettings"; @@ -452,3 +452,58 @@ describe("processSSOSettingsPayload with SAML", () => { ).toBe("false"); }); }); + +describe("diffSSOSettingsPayload", () => { + const stored = { + sso_provider: "generic", + generic_client_id: "client-id", + generic_client_secret: "kc-s****6e0d", + user_email: "admin@example.com", + role_mappings: { group_claim: "groups", roles: { proxy_admin: ["admins"] } }, + }; + + it("returns an empty patch when nothing changed", () => { + expect(diffSSOSettingsPayload(stored, { ...stored })).toEqual({}); + }); + + it("leaves an untouched masked secret out of the patch", () => { + expect(diffSSOSettingsPayload(stored, { ...stored, user_email: "new@example.com" })).toEqual({ + user_email: "new@example.com", + }); + }); + + it("sends null for a field that was emptied", () => { + expect(diffSSOSettingsPayload(stored, { ...stored, generic_client_secret: "" })).toEqual({ + generic_client_secret: null, + }); + }); + + it("sends null for fields dropped from the payload, like another provider's or disabled mappings", () => { + const { generic_client_id, generic_client_secret, role_mappings, ...rest } = stored; + expect(diffSSOSettingsPayload(stored, { ...rest, sso_provider: "google", google_client_id: "g-id" })).toEqual({ + sso_provider: "google", + google_client_id: "g-id", + generic_client_id: null, + generic_client_secret: null, + role_mappings: null, + }); + }); + + it("sends the whole mapping object when one part of it changed", () => { + const role_mappings = { group_claim: "roles", roles: { proxy_admin: ["admins"] } }; + expect(diffSSOSettingsPayload(stored, { ...stored, role_mappings })).toEqual({ role_mappings }); + }); + + it("treats empty values on both sides as unchanged", () => { + expect(diffSSOSettingsPayload({ generic_scope: undefined, proxy_base_url: "" }, { proxy_base_url: null })).toEqual( + {}, + ); + }); + + it("treats an unchecked saml_allow_unsolicited as unset, but sends false when it was true", () => { + expect(diffSSOSettingsPayload({ saml_allow_unsolicited: "false" }, {})).toEqual({}); + expect(diffSSOSettingsPayload({ saml_allow_unsolicited: "true" }, { saml_allow_unsolicited: "false" })).toEqual({ + saml_allow_unsolicited: "false", + }); + }); +}); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts index 768fc7e8e0e..93af987adde 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts @@ -71,6 +71,49 @@ export const processSSOSettingsPayload = (formValues: Record): Reco return payload; }; +const isEmptySSOValue = (value: unknown): boolean => value === undefined || value === null || value === ""; + +// An unchecked saml_allow_unsolicited is the same as never having set it +const comparableSSOValue = (key: string, value: unknown): unknown => + isEmptySSOValue(value) || (key === "saml_allow_unsolicited" && value === "false") ? undefined : value; + +// /update/sso_settings is a PATCH: send only the fields that changed, with null for the ones that were emptied +export const diffSSOSettingsPayload = ( + before: Record, + after: Record, +): Record => + Object.fromEntries( + [...new Set([...Object.keys(before), ...Object.keys(after)])] + .filter( + (key) => + JSON.stringify(comparableSSOValue(key, before[key])) !== JSON.stringify(comparableSSOValue(key, after[key])), + ) + .map((key) => [key, isEmptySSOValue(after[key]) ? null : after[key]]), + ); + +export const CLEARED_SSO_SETTINGS = { + google_client_id: null, + google_client_secret: null, + microsoft_client_id: null, + microsoft_client_secret: null, + microsoft_tenant: null, + generic_client_id: null, + generic_client_secret: null, + generic_authorization_endpoint: null, + generic_token_endpoint: null, + generic_userinfo_endpoint: null, + generic_scope: null, + saml_idp_metadata_url: null, + saml_idp_metadata_xml: null, + saml_sp_entity_id: null, + saml_allow_unsolicited: null, + proxy_base_url: null, + user_email: null, + ui_access_mode: null, + role_mappings: null, + team_mappings: null, +} as const; + // Determine the SSO provider based on the configuration export const detectSSOProvider = (values: SSOSettingsValues): string | null => { if (values.google_client_id) return "google";