From f6ecc40326d8b93631a32a44f693456bdb9cd099 Mon Sep 17 00:00:00 2001 From: jesus Date: Fri, 30 Oct 2026 01:40:29 +0000 Subject: [PATCH] fix(proxy): clear provider and mapping fields the SSO form drops, keep env out of the row Omitted fields now keep only their stored value, so a partial save no longer copies env-only values into the row or touches their env vars. The dashboard SSO forms send null for other providers' fields and unchecked role or team mappings, Clear also nulls team_mappings and ui_access_mode, and an unreadable stored row no longer 500s the update --- litellm/proxy/config_resolvers/sso.py | 13 ++ .../proxy_setting_endpoints.py | 47 ++++---- .../test_proxy_setting_endpoints.py | 112 ++++++++++++++---- .../src/components/SSOModals.test.tsx | 11 ++ .../src/components/SSOModals.tsx | 2 + .../Modals/BaseSSOSettingsForm.test.tsx | 25 ++++ .../Modals/BaseSSOSettingsForm.tsx | 15 ++- .../Modals/DeleteSSOSettingsModal.tsx | 1 + .../EditSSOSettingsModal.integration.test.tsx | 17 ++- .../AdminSettings/SSOSettings/utils.test.ts | 40 +++++-- .../AdminSettings/SSOSettings/utils.ts | 4 + 11 files changed, 228 insertions(+), 59 deletions(-) diff --git a/litellm/proxy/config_resolvers/sso.py b/litellm/proxy/config_resolvers/sso.py index c61f5b6724d..7e6bc6d197d 100644 --- a/litellm/proxy/config_resolvers/sso.py +++ b/litellm/proxy/config_resolvers/sso.py @@ -9,6 +9,7 @@ save endpoint so the two can never drift. from collections.abc import Mapping from dataclasses import dataclass +from types import MappingProxyType from typing import Final from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helper @@ -97,3 +98,15 @@ def resolve_sso_config(sso_db_settings: Mapping[str, object] | None, env: Mappin } config: Final = SSOConfig(**{**values, **structured}) return ResolvedSSOConfig(config=config, provenance=provenance) + + +_SSO_SECRET_DESCRIPTORS: Final = tuple(d for d in SSO_DESCRIPTORS if d.is_secret) + + +def resolve_sso_secrets(sso_db_settings: Mapping[str, object] | None, env: Mapping[str, str]) -> Mapping[str, str]: + """Effective client secrets (stored row, then env), decrypted. Reads only the secret keys so a + malformed role or team mapping on the row cannot block a settings update.""" + raw: Final = dict(sso_db_settings) if sso_db_settings else {} + stored: Final = _decrypt({d.db_key: raw[d.db_key] for d in _SSO_SECRET_DESCRIPTORS if d.db_key in raw}) + values, _ = resolve_fields(_SSO_SECRET_DESCRIPTORS, stored, env) + return MappingProxyType({name: value for name, value in values.items() if value}) diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index 72d671ebf7b..4c5806da1ed 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -33,8 +33,8 @@ from litellm.proxy.config_resolvers.settings_store import ConfigOwnedKeyError from litellm.proxy.config_resolvers.sso import ( SSO_FIELD_ENV_VARS, SSO_SECRET_FIELDS, - ResolvedSSOConfig, resolve_sso_config, + resolve_sso_secrets, ) from litellm.proxy.management_endpoints.team_admin_field_permissions import ( SUPPORTED_TEAM_ADMIN_PERMISSIONS, @@ -1238,38 +1238,33 @@ def _stored_sso_settings(record: _StoredSsoSettingsRow | None) -> Mapping[str, J if record is None or not record.sso_settings: return None stored: Final = record.sso_settings - decoded: Final = _DECODED_JSON.validate_python(json.loads(stored)) if isinstance(stored, str) else stored try: + decoded: Final = _DECODED_JSON.validate_python(json.loads(stored)) if isinstance(stored, str) else stored return _SSO_SETTINGS_VALUES.validate_python(decoded) - except ValidationError: + except ValueError: return None -def _current_sso_value(current: ResolvedSSOConfig, field_name: str, value: JsonValue) -> JsonValue: - return None if current.provenance.get(field_name) in ("default", "unset") else value - - -def _merged_sso_field(field_name: str, incoming: JsonValue, current: JsonValue, submitted: bool) -> JsonValue: - if not submitted: - return current - if field_name not in SSO_SECRET_FIELDS or not isinstance(current, str) or not current: +def _kept_sso_secret(field_name: str, incoming: JsonValue, current_secret: str | None) -> JsonValue: + if current_secret is None: return incoming - if incoming == mask_sensitive_keys({field_name: current}, {field_name})[field_name]: - return current - return incoming + shown_secret: Final = mask_sensitive_keys({field_name: current_secret}, {field_name})[field_name] + return current_secret if incoming == shown_secret else incoming -def merge_sso_settings_update(sso_config: SSOConfig, current: ResolvedSSOConfig) -> Mapping[str, JsonValue]: - """Fields the request omits keep their effective value, and a client secret sent back - as the mask /get/sso_settings returned keeps the real secret. An explicit null or empty - string still clears a field.""" +def merge_sso_settings_update( + sso_config: SSOConfig, stored: Mapping[str, JsonValue], current_secrets: Mapping[str, str] +) -> Mapping[str, JsonValue]: + """Fields the request omits keep their stored value (never an env-only value, so a save does not + freeze env into the row), and a client secret sent back as the mask /get/sso_settings returned + keeps the real secret. An explicit null or empty string still clears a field.""" incoming: Final = _SSO_SETTINGS_VALUES.validate_python(sso_config.model_dump(mode="json")) - current_values: Final = { - name: _current_sso_value(current, name, value) - for name, value in _SSO_SETTINGS_VALUES.validate_python(current.config.model_dump(mode="json")).items() - } return { - name: _merged_sso_field(name, value, current_values[name], name in sso_config.model_fields_set) + name: ( + _kept_sso_secret(name, value, current_secrets.get(name)) + if name in sso_config.model_fields_set + else stored.get(name) + ) for name, value in incoming.items() } @@ -1335,9 +1330,11 @@ async def update_sso_settings( config["general_settings"] = {} # Update environment variables in config and in memory - sso_data: Final = merge_sso_settings_update(sso_config, resolve_sso_config(stored_sso_settings, os.environ)) + sso_data: Final = merge_sso_settings_update( + sso_config, before_sso_data or {}, resolve_sso_secrets(stored_sso_settings, os.environ) + ) for field_name, value in sso_data.items(): - if field_name in SSO_FIELD_ENV_VARS: + if field_name in SSO_FIELD_ENV_VARS and field_name in sso_config.model_fields_set: env_var_name = SSO_FIELD_ENV_VARS[field_name] if isinstance(value, str) and value: os.environ[env_var_name] = 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 98266efdd23..1bdffbb3f78 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 @@ -1137,7 +1137,6 @@ class TestProxySettingEndpoints: assert create_sso_settings["user_email"] is None def _mock_sso_update_db(self, monkeypatch, stored_sso_settings): - """Point /update/sso_settings at a stored SSO row (or None) with encryption as identity.""" from unittest.mock import AsyncMock, MagicMock from litellm.proxy.proxy_server import proxy_config @@ -1145,27 +1144,29 @@ class TestProxySettingEndpoints: monkeypatch.setenv("LITELLM_SALT_KEY", "test_salt_key") monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", True) mock_prisma = MagicMock() - if stored_sso_settings is None: - record = None - else: + record = None + if stored_sso_settings is not None: record = MagicMock() - record.sso_settings = stored_sso_settings + record.sso_settings = ( + stored_sso_settings + if isinstance(stored_sso_settings, str) + else proxy_config.encrypt_env_variables(environment_variables=stored_sso_settings) + ) mock_prisma.db.litellm_ssoconfig.find_unique = AsyncMock(return_value=record) mock_prisma.db.litellm_ssoconfig.upsert = AsyncMock() mock_prisma.writer_db.litellm_config.find_unique = AsyncMock(return_value=None) mock_prisma.db.litellm_config.update = AsyncMock() monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma) - monkeypatch.setattr(proxy_config, "encrypt_env_variables", lambda environment_variables: environment_variables) - identity = lambda value, key=None, exception_type="error", return_original_value=False: value # noqa: E731 - monkeypatch.setattr("litellm.proxy.config_resolvers.sso.decrypt_value_helper", identity) - monkeypatch.setattr("litellm.proxy.proxy_server.decrypt_value_helper", identity) return mock_prisma @staticmethod def _upserted_sso_settings(mock_prisma): import json - return json.loads(mock_prisma.db.litellm_ssoconfig.upsert.call_args.kwargs["data"]["update"]["sso_settings"]) + from litellm.proxy.proxy_server import proxy_config + + stored = json.loads(mock_prisma.db.litellm_ssoconfig.upsert.call_args.kwargs["data"]["update"]["sso_settings"]) + return proxy_config.decrypt_db_variables(stored) @pytest.mark.parametrize( "secret_field,env_var", @@ -1178,8 +1179,6 @@ class TestProxySettingEndpoints: def test_update_sso_settings_round_tripped_mask_keeps_stored_secret( self, mock_proxy_config, mock_auth, monkeypatch, secret_field, env_var ): - """Regression for LIT-8683: saving the settings page (which only ever holds the masked - secret from /get/sso_settings) must not overwrite the stored client secret with the mask.""" monkeypatch.delenv(env_var, raising=False) stored_secret = "stored-client-secret-0123456789" mock_prisma = self._mock_sso_update_db(monkeypatch, {secret_field: stored_secret}) @@ -1199,8 +1198,6 @@ class TestProxySettingEndpoints: def test_update_sso_settings_round_tripped_mask_keeps_env_secret( self, mock_proxy_config, mock_auth, monkeypatch ): - """A secret that only lives in the process env is shown masked too, and saving the - page persists that secret rather than the mask.""" monkeypatch.setenv("GENERIC_CLIENT_SECRET", "env-only-secret-0123456789") mock_prisma = self._mock_sso_update_db(monkeypatch, None) @@ -1214,14 +1211,13 @@ class TestProxySettingEndpoints: def test_update_sso_settings_omitted_fields_keep_stored_values( self, mock_proxy_config, mock_auth, monkeypatch ): - """The UI access control form sends only ui_access_mode. That must not null out the - rest of the stored SSO configuration.""" - monkeypatch.delenv("GENERIC_CLIENT_SECRET", raising=False) + monkeypatch.setenv("GENERIC_CLIENT_SECRET", "stored-client-secret-0123456789") stored = { "generic_client_id": "stored-client-id", "generic_client_secret": "stored-client-secret-0123456789", "generic_token_endpoint": "https://idp.example.com/token", "role_mappings": {"provider": "generic", "group_claim": "groups", "roles": {"proxy_admin": ["admins"]}}, + "team_mappings": {"team_ids_jwt_field": "teams"}, } mock_prisma = self._mock_sso_update_db(monkeypatch, stored) @@ -1230,12 +1226,55 @@ class TestProxySettingEndpoints: assert response.status_code == 200, response.text persisted = self._upserted_sso_settings(mock_prisma) assert persisted["ui_access_mode"] == "none" - assert persisted["generic_client_id"] == "stored-client-id" - assert persisted["generic_client_secret"] == "stored-client-secret-0123456789" - assert persisted["generic_token_endpoint"] == "https://idp.example.com/token" - assert persisted["role_mappings"]["roles"]["proxy_admin"] == ["admins"] + assert {name: persisted[name] for name in stored} == stored assert os.environ["GENERIC_CLIENT_SECRET"] == "stored-client-secret-0123456789" + def test_update_sso_settings_omitted_env_only_field_is_not_frozen_into_row( + self, mock_proxy_config, mock_auth, monkeypatch + ): + monkeypatch.setenv("GENERIC_CLIENT_ID", "env-client-id") + mock_prisma = self._mock_sso_update_db(monkeypatch, {"user_email": "admin@example.com"}) + + response = client.patch("/update/sso_settings", json={"ui_access_mode": "none"}) + + assert response.status_code == 200, response.text + persisted = self._upserted_sso_settings(mock_prisma) + assert persisted["user_email"] == "admin@example.com" + assert persisted["generic_client_id"] is None + assert os.environ["GENERIC_CLIENT_ID"] == "env-client-id" + + def test_update_sso_settings_explicit_nulls_clear_previous_provider_and_mappings( + self, mock_proxy_config, mock_auth, monkeypatch + ): + monkeypatch.setenv("GOOGLE_CLIENT_ID", "stored-google-id") + mock_prisma = self._mock_sso_update_db( + monkeypatch, + { + "google_client_id": "stored-google-id", + "google_client_secret": "stored-google-secret-0123456789", + "role_mappings": {"provider": "generic", "group_claim": "groups", "roles": {"proxy_admin": ["a"]}}, + "team_mappings": {"team_ids_jwt_field": "teams"}, + }, + ) + + response = client.patch( + "/update/sso_settings", + json={ + "microsoft_client_id": "new-microsoft-id", + "google_client_id": None, + "google_client_secret": None, + "role_mappings": None, + "team_mappings": None, + }, + ) + + assert response.status_code == 200, response.text + persisted = self._upserted_sso_settings(mock_prisma) + assert persisted["microsoft_client_id"] == "new-microsoft-id" + cleared = ("google_client_id", "google_client_secret", "role_mappings", "team_mappings") + assert {name: persisted[name] for name in cleared} == dict.fromkeys(cleared) + assert "GOOGLE_CLIENT_ID" not in os.environ + def test_update_sso_settings_new_secret_replaces_stored_secret( self, mock_proxy_config, mock_auth, monkeypatch ): @@ -1250,6 +1289,20 @@ class TestProxySettingEndpoints: assert os.environ["GENERIC_CLIENT_SECRET"] == "rotated-secret-9876543210" assert "rotated-secret-9876543210" not in response.text + def test_update_sso_settings_mask_of_another_field_secret_is_not_swapped( + self, mock_proxy_config, mock_auth, monkeypatch + ): + monkeypatch.delenv("GOOGLE_CLIENT_SECRET", raising=False) + mock_prisma = self._mock_sso_update_db( + monkeypatch, {"generic_client_secret": "stored-client-secret-0123456789"} + ) + generic_mask = client.get("/get/sso_settings").json()["values"]["generic_client_secret"] + + response = client.patch("/update/sso_settings", json={"google_client_secret": generic_mask}) + + assert response.status_code == 200, response.text + assert self._upserted_sso_settings(mock_prisma)["google_client_secret"] == generic_mask + @pytest.mark.parametrize("cleared", [None, ""]) def test_update_sso_settings_explicit_clear_drops_stored_secret( self, mock_proxy_config, mock_auth, monkeypatch, cleared @@ -1264,6 +1317,23 @@ class TestProxySettingEndpoints: assert not self._upserted_sso_settings(mock_prisma)["generic_client_secret"] assert "GENERIC_CLIENT_SECRET" not in os.environ + @pytest.mark.parametrize( + "stored_row", + [ + "{not json", + {"generic_client_secret": "stored-client-secret-0123456789", "role_mappings": {"roles": "not-a-dict"}}, + ], + ) + def test_update_sso_settings_overwrites_unreadable_stored_row( + self, mock_proxy_config, mock_auth, monkeypatch, stored_row + ): + mock_prisma = self._mock_sso_update_db(monkeypatch, stored_row) + + response = client.patch("/update/sso_settings", json={"generic_client_id": "new-client-id"}) + + assert response.status_code == 200, response.text + assert self._upserted_sso_settings(mock_prisma)["generic_client_id"] == "new-client-id" + def test_get_ui_theme_settings(self, mock_proxy_config): """Test getting UI theme settings without authentication""" response = client.get("/get/ui_theme_settings") diff --git a/ui/litellm-dashboard/src/components/SSOModals.test.tsx b/ui/litellm-dashboard/src/components/SSOModals.test.tsx index bee5f13c626..63885bb787d 100644 --- a/ui/litellm-dashboard/src/components/SSOModals.test.tsx +++ b/ui/litellm-dashboard/src/components/SSOModals.test.tsx @@ -389,6 +389,15 @@ describe("SSOModals", () => { generic_authorization_endpoint: "https://example.okta.com/authorize", generic_token_endpoint: "https://example.okta.com/token", generic_userinfo_endpoint: "https://example.okta.com/userinfo", + google_client_id: null, + google_client_secret: null, + microsoft_client_id: null, + microsoft_client_secret: null, + microsoft_tenant: null, + saml_idp_metadata_url: null, + saml_idp_metadata_xml: null, + saml_sp_entity_id: null, + saml_allow_unsolicited: null, role_mappings: { provider: "generic", group_claim: "groups", @@ -535,6 +544,8 @@ describe("SSOModals", () => { user_email: null, sso_provider: null, role_mappings: null, + team_mappings: null, + ui_access_mode: null, }); }); diff --git a/ui/litellm-dashboard/src/components/SSOModals.tsx b/ui/litellm-dashboard/src/components/SSOModals.tsx index 609c0efcd99..601a22bbd9e 100644 --- a/ui/litellm-dashboard/src/components/SSOModals.tsx +++ b/ui/litellm-dashboard/src/components/SSOModals.tsx @@ -224,6 +224,8 @@ const SSOModals: React.FC = ({ user_email: null, sso_provider: null, role_mappings: null, + team_mappings: null, + ui_access_mode: null, }; await updateSSOSettings(accessToken, clearSettings); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.test.tsx index eddc5716305..5d9531bc5b0 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.test.tsx @@ -4,6 +4,7 @@ import { renderWithProviders } from "../../../../../../tests/test-utils"; import { afterEach, describe, expect, it, vi } from "vitest"; import BaseSSOSettingsForm, { emptySSOSettingsFormValues, + pickMountedSSOValues, renderProviderFields, ssoProviderConfigs, submitMountedSSOValues, @@ -436,3 +437,27 @@ describe("renderProviderFields", () => { expect(await screen.findByLabelText("Default Role")).toHaveTextContent("Internal User"); }); }); + +describe("pickMountedSSOValues", () => { + it("nulls the previous provider's fields after a provider switch so the save clears them", () => { + const picked = pickMountedSSOValues( + { + sso_provider: "microsoft", + google_client_id: "old-google-id", + google_client_secret: "old-google-secret", + microsoft_client_id: "new-microsoft-id", + user_email: "admin@example.com", + }, + "sso-settings", + ); + + expect(picked).toMatchObject({ + sso_provider: "microsoft", + microsoft_client_id: "new-microsoft-id", + google_client_id: null, + google_client_secret: null, + generic_client_id: null, + saml_idp_metadata_url: null, + }); + }); +}); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.tsx index e4725294e05..ba40ce97e56 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/Modals/BaseSSOSettingsForm.tsx @@ -199,10 +199,17 @@ export const mountedSSOFieldNames = (values: SSOSettingsFormValues, variant: SSO ]; }; -export const pickMountedSSOValues = (values: SSOSettingsFormValues, variant: SSOFormVariant): SSOSettingsFormValues => - Object.fromEntries( - mountedSSOFieldNames(values, variant).map((name) => [name, values[name as keyof SSOSettingsFormValues]]), - ); +const ALL_PROVIDER_FIELD_NAMES: readonly string[] = [ + ...new Set(Object.values(ssoProviderConfigs).flatMap((config) => config.fields.map((field) => field.name))), +]; + +export const pickMountedSSOValues = (values: SSOSettingsFormValues, variant: SSOFormVariant): SSOSettingsFormValues => { + const mounted = mountedSSOFieldNames(values, variant); + return Object.fromEntries([ + ...ALL_PROVIDER_FIELD_NAMES.filter((name) => !mounted.includes(name)).map((name) => [name, null]), + ...mounted.map((name) => [name, values[name as keyof SSOSettingsFormValues]]), + ]); +}; export const submitMountedSSOValues = ( 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..0e61eb1f3ce 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 @@ -38,6 +38,7 @@ const DeleteSSOSettingsModal: React.FC = ({ isVisib sso_provider: null, role_mappings: null, team_mappings: null, + ui_access_mode: null, }; await editSSOSettings(clearSettings, { 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..ce7075cd480 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 the mounted fields and clears every other provider's fields and the unchecked mappings", async () => { const user = userEvent.setup({ pointerEventsCheck: 0 }); renderWithProviders(); @@ -64,6 +64,21 @@ describe("EditSSOSettingsModal (real form tree)", () => { google_client_secret: "stored-client-secret", user_email: "admin@example.com", proxy_base_url: "https://gateway.example.com", + 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, + role_mappings: null, + team_mappings: null, }); }); }); 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..2eede750c91 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 @@ -24,9 +24,9 @@ describe("processSSOSettingsPayload", () => { expect(result).toEqual({ other_field: "value", another_field: 123, + role_mappings: null, + team_mappings: null, }); - expect(result.role_mappings).toBeUndefined(); - expect(result.team_mappings).toBeUndefined(); }); it("should return all fields except role mapping fields when use_role_mappings is not present", () => { @@ -46,9 +46,29 @@ describe("processSSOSettingsPayload", () => { expect(result).toEqual({ other_field: "value", + role_mappings: null, + team_mappings: null, }); - expect(result.role_mappings).toBeUndefined(); - expect(result.team_mappings).toBeUndefined(); + }); + + it("clears mappings the user unchecked on a provider that supports them", () => { + const result = processSSOSettingsPayload({ + sso_provider: "generic", + use_role_mappings: false, + use_team_mappings: false, + group_claim: "groups", + team_ids_jwt_field: "teams", + }); + + expect(result.role_mappings).toBeNull(); + expect(result.team_mappings).toBeNull(); + }); + + it("leaves mappings alone when the form has no toggle for them", () => { + const result = processSSOSettingsPayload({ sso_provider: "generic", use_role_mappings: false }); + + expect(result.role_mappings).toBeNull(); + expect(result).not.toHaveProperty("team_mappings"); }); }); @@ -274,8 +294,8 @@ describe("processSSOSettingsPayload", () => { expect(result).toEqual({ sso_provider: "okta", other_field: "value", + team_mappings: null, }); - expect(result.team_mappings).toBeUndefined(); }); it("should return all fields except team mapping fields when use_team_mappings is not present", () => { @@ -307,8 +327,9 @@ describe("processSSOSettingsPayload", () => { expect(result).toEqual({ sso_provider: "google", other_field: "value", + role_mappings: null, + team_mappings: null, }); - expect(result.team_mappings).toBeUndefined(); }); it("should not include team mappings for microsoft provider even when use_team_mappings is true", () => { @@ -324,8 +345,9 @@ describe("processSSOSettingsPayload", () => { expect(result).toEqual({ sso_provider: "microsoft", other_field: "value", + role_mappings: null, + team_mappings: null, }); - expect(result.team_mappings).toBeUndefined(); }); }); @@ -401,7 +423,7 @@ describe("processSSOSettingsPayload", () => { it("should handle empty form values", () => { const result = processSSOSettingsPayload({}); - expect(result).toEqual({}); + expect(result).toEqual({ role_mappings: null, team_mappings: null }); }); it("should preserve other fields in the payload", () => { @@ -425,6 +447,8 @@ describe("processSSOSettingsPayload", () => { redirect_url: "http://example.com", custom_field: { nested: "value" }, array_field: [1, 2, 3], + role_mappings: null, + team_mappings: null, }); }); }); 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..1307e6c781b 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts @@ -58,6 +58,8 @@ export const processSSOSettingsPayload = (formValues: Record): Reco internal_user_viewer: splitTeams(internal_viewer_teams), }, }; + } else if (!supportsRoleMappings || "use_role_mappings" in formValues) { + payload.role_mappings = null; } // Add team mappings only if use_team_mappings is checked AND provider supports team mappings @@ -66,6 +68,8 @@ export const processSSOSettingsPayload = (formValues: Record): Reco payload.team_mappings = { team_ids_jwt_field: team_ids_jwt_field, }; + } else if (!supportsTeamMappings || "use_team_mappings" in formValues) { + payload.team_mappings = null; } return payload;