From bffcc0ee0b625999a935dfcaef949ea83b149500 Mon Sep 17 00:00:00 2001 From: Dennis Henry Date: Wed, 13 May 2026 11:28:32 -0400 Subject: [PATCH] fixing duplicated provider detection --- .../src/components/SSOModals.tsx | 25 +------- .../Modals/EditSSOSettingsModal.tsx | 25 +------- .../AdminSettings/SSOSettings/utils.test.ts | 60 ++++++++++++++++++- .../AdminSettings/SSOSettings/utils.ts | 24 +++++++- 4 files changed, 86 insertions(+), 48 deletions(-) diff --git a/ui/litellm-dashboard/src/components/SSOModals.tsx b/ui/litellm-dashboard/src/components/SSOModals.tsx index 21140e673dc..280cba71e5c 100644 --- a/ui/litellm-dashboard/src/components/SSOModals.tsx +++ b/ui/litellm-dashboard/src/components/SSOModals.tsx @@ -2,7 +2,7 @@ import React, { useEffect, useState } from "react"; import { Modal, Form, Input, Button as Button2, Select, Checkbox } from "antd"; import { Text, TextInput } from "@tremor/react"; import { getSSOSettings, updateSSOSettings } from "./networking"; -import { detectSSOProvider } from "./Settings/AdminSettings/SSOSettings/utils"; +import { detectSSOProvider, extractRoleMappingFields } from "./Settings/AdminSettings/SSOSettings/utils"; import NotificationsManager from "./molecules/notifications_manager"; import { parseErrorMessage } from "./shared/errorUtils"; @@ -116,28 +116,7 @@ const SSOModals: React.FC = ({ const ssoData = await getSSOSettings(accessToken); if (ssoData && ssoData.values) { const selectedProvider = detectSSOProvider(ssoData.values); - - // Extract role mappings if they exist - let roleMappingFields = {}; - if (ssoData.values.role_mappings) { - const roleMappings = ssoData.values.role_mappings; - - // Helper function to join arrays into comma-separated strings - const joinTeams = (teams: string[] | undefined): string => { - if (!teams || teams.length === 0) return ""; - return teams.join(", "); - }; - - roleMappingFields = { - use_role_mappings: true, - group_claim: roleMappings.group_claim, - default_role: roleMappings.default_role || "internal_user", - proxy_admin_teams: joinTeams(roleMappings.roles?.proxy_admin), - admin_viewer_teams: joinTeams(roleMappings.roles?.proxy_admin_viewer), - internal_user_teams: joinTeams(roleMappings.roles?.internal_user), - internal_viewer_teams: joinTeams(roleMappings.roles?.internal_user_viewer), - }; - } + const roleMappingFields = extractRoleMappingFields(ssoData.values.role_mappings); // Set form values with existing data (excluding UI access control fields) const formValues = { 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 c00d039e614..7a9db3f3ac8 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 @@ -5,7 +5,7 @@ import React, { useEffect } from "react"; import BaseSSOSettingsForm from "./BaseSSOSettingsForm"; import NotificationsManager from "@/components/molecules/notifications_manager"; import { parseErrorMessage } from "@/components/shared/errorUtils"; -import { detectSSOProvider, processSSOSettingsPayload } from "../utils"; +import { detectSSOProvider, extractRoleMappingFields, processSSOSettingsPayload } from "../utils"; import { useSSOSettings } from "@/app/(dashboard)/hooks/sso/useSSOSettings"; import { useEditSSOSettings } from "@/app/(dashboard)/hooks/sso/useEditSSOSettings"; @@ -27,28 +27,7 @@ const EditSSOSettingsModal: React.FC = ({ isVisible, // Determine which SSO provider is configured const selectedProvider = detectSSOProvider(ssoData.values); - - // Extract role mappings if they exist - let roleMappingFields = {}; - if (ssoData.values.role_mappings) { - const roleMappings = ssoData.values.role_mappings; - - // Helper function to join arrays into comma-separated strings - const joinTeams = (teams: string[] | undefined): string => { - if (!teams || teams.length === 0) return ""; - return teams.join(", "); - }; - - roleMappingFields = { - use_role_mappings: true, - group_claim: roleMappings.group_claim, - default_role: roleMappings.default_role || "internal_user", - proxy_admin_teams: joinTeams(roleMappings.roles?.proxy_admin), - admin_viewer_teams: joinTeams(roleMappings.roles?.proxy_admin_viewer), - internal_user_teams: joinTeams(roleMappings.roles?.internal_user), - internal_viewer_teams: joinTeams(roleMappings.roles?.internal_user_viewer), - }; - } + const roleMappingFields = extractRoleMappingFields(ssoData.values.role_mappings); // Extract team mappings if they exist let teamMappingFields = {}; 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 3a1350a7076..d73d5551537 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,5 @@ -import { processSSOSettingsPayload } from "./utils"; +import { extractRoleMappingFields, processSSOSettingsPayload } from "./utils"; +import { RoleMappings } from "@/app/(dashboard)/hooks/sso/useSSOSettings"; import { describe, it, expect } from "vitest"; describe("processSSOSettingsPayload", () => { @@ -429,3 +430,60 @@ describe("processSSOSettingsPayload", () => { }); }); }); + +describe("extractRoleMappingFields", () => { + it("returns an empty object when role_mappings is null/undefined", () => { + expect(extractRoleMappingFields(null)).toEqual({}); + expect(extractRoleMappingFields(undefined)).toEqual({}); + }); + + it("joins role arrays into comma-separated strings", () => { + const roleMappings: RoleMappings = { + provider: "okta", + group_claim: "groups", + default_role: "internal_user", + roles: { + proxy_admin: ["admin1", "admin2"], + proxy_admin_viewer: ["viewer1"], + internal_user: ["user1", "user2"], + internal_user_viewer: ["ro1"], + }, + }; + + expect(extractRoleMappingFields(roleMappings)).toEqual({ + use_role_mappings: true, + group_claim: "groups", + default_role: "internal_user", + proxy_admin_teams: "admin1, admin2", + admin_viewer_teams: "viewer1", + internal_user_teams: "user1, user2", + internal_viewer_teams: "ro1", + }); + }); + + it("emits empty strings for missing role arrays", () => { + const roleMappings = { + provider: "generic", + group_claim: "groups", + default_role: "proxy_admin", + roles: {}, + } as unknown as RoleMappings; + + const result = extractRoleMappingFields(roleMappings); + expect(result.proxy_admin_teams).toBe(""); + expect(result.admin_viewer_teams).toBe(""); + expect(result.internal_user_teams).toBe(""); + expect(result.internal_viewer_teams).toBe(""); + }); + + it("falls back to internal_user when default_role is missing", () => { + const roleMappings = { + provider: "okta", + group_claim: "groups", + default_role: "", + roles: { proxy_admin: [], proxy_admin_viewer: [], internal_user: [], internal_user_viewer: [] }, + } as unknown as RoleMappings; + + expect(extractRoleMappingFields(roleMappings).default_role).toBe("internal_user"); + }); +}); 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 d70d7f950fd..f0aeb7272a0 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/SSOSettings/utils.ts @@ -1,4 +1,4 @@ -import { SSOSettingsValues } from "@/app/(dashboard)/hooks/sso/useSSOSettings"; +import { RoleMappings, SSOSettingsValues } from "@/app/(dashboard)/hooks/sso/useSSOSettings"; /** * Processes SSO settings form values and transforms them into the payload format expected by the API @@ -67,6 +67,28 @@ export const processSSOSettingsPayload = (formValues: Record): Reco return payload; }; +// Build form fields to prefill the role mappings section from existing SSO settings. +// Shared by Add (SSOModals) and Edit (EditSSOSettingsModal) flows so detection and +// extraction rules stay in one place. +export const extractRoleMappingFields = (roleMappings: RoleMappings | null | undefined): Record => { + if (!roleMappings) return {}; + + const joinTeams = (teams: string[] | undefined): string => { + if (!teams || teams.length === 0) return ""; + return teams.join(", "); + }; + + return { + use_role_mappings: true, + group_claim: roleMappings.group_claim, + default_role: roleMappings.default_role || "internal_user", + proxy_admin_teams: joinTeams(roleMappings.roles?.proxy_admin), + admin_viewer_teams: joinTeams(roleMappings.roles?.proxy_admin_viewer), + internal_user_teams: joinTeams(roleMappings.roles?.internal_user), + internal_viewer_teams: joinTeams(roleMappings.roles?.internal_user_viewer), + }; +}; + // Determine the SSO provider based on the configuration export const detectSSOProvider = (values: SSOSettingsValues): string | null => { if (values.google_client_id) return "google";