diff --git a/frontend-modern/src/components/Settings/ssoProvidersModel.ts b/frontend-modern/src/components/Settings/ssoProvidersModel.ts index 4a5e7e52c..139fcb5f9 100644 --- a/frontend-modern/src/components/Settings/ssoProvidersModel.ts +++ b/frontend-modern/src/components/Settings/ssoProvidersModel.ts @@ -16,6 +16,8 @@ export interface SSOProvider { allowedGroups?: string[]; allowedDomains?: string[]; allowedEmails?: string[]; + groupsClaim?: string; + groupRoleMappings?: Record; } export interface SSOProvidersResponse { diff --git a/frontend-modern/src/components/Settings/useSSOProvidersState.ts b/frontend-modern/src/components/Settings/useSSOProvidersState.ts index 7f9085b7e..4093acfc4 100644 --- a/frontend-modern/src/components/Settings/useSSOProvidersState.ts +++ b/frontend-modern/src/components/Settings/useSSOProvidersState.ts @@ -185,10 +185,27 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { } try { const { apiFetch } = await import('@/utils/apiClient'); + // Send only writable provider fields: the list item also carries + // computed response fields (oidcLoginUrl, samlAcsUrl, ...) that the + // strict PUT decoder rejects. Nested oidc/saml config and the client + // secret are intentionally absent; the backend preserves them. const response = await apiFetch(`/api/security/sso/providers/${provider.id}`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ ...provider, enabled: !provider.enabled }), + body: JSON.stringify({ + id: provider.id, + name: provider.name, + type: provider.type, + enabled: !provider.enabled, + displayName: provider.displayName, + iconUrl: provider.iconUrl, + priority: provider.priority, + allowedGroups: provider.allowedGroups ?? [], + allowedDomains: provider.allowedDomains ?? [], + allowedEmails: provider.allowedEmails ?? [], + groupsClaim: provider.groupsClaim, + groupRoleMappings: provider.groupRoleMappings, + }), }); if (!response.ok) { diff --git a/internal/api/identity_sso_handlers.go b/internal/api/identity_sso_handlers.go index 9359bfa78..01ce29fb9 100644 --- a/internal/api/identity_sso_handlers.go +++ b/internal/api/identity_sso_handlers.go @@ -500,21 +500,10 @@ func (r *Router) handleUpdateSSOProvider(w http.ResponseWriter, req *http.Reques if updated.Type == config.SSOProviderTypeSAML && updated.SAML == nil && existing.SAML != nil { updated.SAML = existing.SAML } - if updated.GroupsClaim == "" && existing.GroupsClaim != "" { - updated.GroupsClaim = existing.GroupsClaim - } - if len(updated.GroupRoleMappings) == 0 && len(existing.GroupRoleMappings) > 0 { - updated.GroupRoleMappings = existing.GroupRoleMappings - } - if len(updated.AllowedGroups) == 0 && len(existing.AllowedGroups) > 0 { - updated.AllowedGroups = existing.AllowedGroups - } - if len(updated.AllowedDomains) == 0 && len(existing.AllowedDomains) > 0 { - updated.AllowedDomains = existing.AllowedDomains - } - if len(updated.AllowedEmails) == 0 && len(existing.AllowedEmails) > 0 { - updated.AllowedEmails = existing.AllowedEmails - } + // groupsClaim, groupRoleMappings, allowedGroups, allowedDomains and + // allowedEmails are NOT preserved on empty: both the detail GET and the + // flat list response round-trip them, so an empty value in a PUT is an + // intentional clear. // Preserve secrets if not provided in update if updated.Type == config.SSOProviderTypeOIDC && updated.OIDC != nil && existing.OIDC != nil { diff --git a/internal/api/sso_handlers_crud_test.go b/internal/api/sso_handlers_crud_test.go index af2adedb2..28e9ff9bb 100644 --- a/internal/api/sso_handlers_crud_test.go +++ b/internal/api/sso_handlers_crud_test.go @@ -13,8 +13,6 @@ import ( "github.com/stretchr/testify/require" ) -// ... (skipping unchanged parts until test) - func TestSanitizeProviderName(t *testing.T) { tests := []struct { input string @@ -243,6 +241,53 @@ func TestSSOProviderDetailRoundTripPreservesOIDCEditableSettings(t *testing.T) { assert.Equal(t, map[string]string{"admins": "admin"}, stored.GroupRoleMappings) } +func TestUpdateSSOProvider_EmptyValuesClearRestrictionFields(t *testing.T) { + router, _ := setupTestRouter(t) + + provider := config.SSOProvider{ + ID: "corp-oidc", + Name: "Corporate OIDC", + Type: config.SSOProviderTypeOIDC, + Enabled: true, + AllowedGroups: []string{"admins"}, + AllowedDomains: []string{"example.com"}, + AllowedEmails: []string{"owner@example.com"}, + GroupsClaim: "groups", + GroupRoleMappings: map[string]string{ + "admins": "admin", + }, + OIDC: &config.OIDCProviderConfig{ + IssuerURL: "https://idp.example.com/realms/pulse", + ClientID: "pulse-client", + ClientSecret: "super-secret", + }, + } + require.NoError(t, router.ssoConfig.AddProvider(provider)) + require.NoError(t, router.saveSSOConfig()) + + // An intentionally emptied field in a PUT clears it; only the nested + // config (absent from toggle payloads) and the client secret (never + // echoed in reads) are preserved. + body := `{"name":"Corporate OIDC","type":"oidc","enabled":true,"allowedGroups":[],"allowedDomains":[],"allowedEmails":[],"groupsClaim":"","groupRoleMappings":{}}` + req := httptest.NewRequest(http.MethodPut, "/api/security/sso/providers/corp-oidc", strings.NewReader(body)) + w := httptest.NewRecorder() + router.handleSSOProvider(w, req) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + + loadedConfig, err := router.persistence.LoadSSOConfig() + require.NoError(t, err) + stored := loadedConfig.GetProvider("corp-oidc") + require.NotNil(t, stored) + assert.Empty(t, stored.AllowedGroups) + assert.Empty(t, stored.AllowedDomains) + assert.Empty(t, stored.AllowedEmails) + assert.Empty(t, stored.GroupsClaim) + assert.Empty(t, stored.GroupRoleMappings) + require.NotNil(t, stored.OIDC) + assert.Equal(t, "https://idp.example.com/realms/pulse", stored.OIDC.IssuerURL) + assert.Equal(t, "super-secret", stored.OIDC.ClientSecret) +} + func TestCreateSSOProvider_Validation(t *testing.T) { router, _ := setupTestRouter(t) diff --git a/pkg/extensions/sso_admin.go b/pkg/extensions/sso_admin.go index c3470033b..bd2bfbd34 100644 --- a/pkg/extensions/sso_admin.go +++ b/pkg/extensions/sso_admin.go @@ -94,25 +94,27 @@ type SSOConfigSnapshot struct { // SSOProviderResponse is the API response shape for SSO providers. type SSOProviderResponse struct { - ID string `json:"id"` - Name string `json:"name"` - Type string `json:"type"` - Enabled bool `json:"enabled"` - DisplayName string `json:"displayName,omitempty"` - IconURL string `json:"iconUrl,omitempty"` - Priority int `json:"priority"` - OIDCIssuerURL string `json:"oidcIssuerUrl,omitempty"` - OIDCClientID string `json:"oidcClientId,omitempty"` - OIDCClientSecretSet bool `json:"oidcClientSecretSet,omitempty"` - OIDCLoginURL string `json:"oidcLoginUrl,omitempty"` - OIDCCallbackURL string `json:"oidcCallbackUrl,omitempty"` - SAMLIDPEntityID string `json:"samlIdpEntityId,omitempty"` - SAMLSPEntityID string `json:"samlSpEntityId,omitempty"` - SAMLMetadataURL string `json:"samlMetadataUrl,omitempty"` - SAMLACSURL string `json:"samlAcsUrl,omitempty"` - AllowedGroups []string `json:"allowedGroups,omitempty"` - AllowedDomains []string `json:"allowedDomains,omitempty"` - AllowedEmails []string `json:"allowedEmails,omitempty"` + ID string `json:"id"` + Name string `json:"name"` + Type string `json:"type"` + Enabled bool `json:"enabled"` + DisplayName string `json:"displayName,omitempty"` + IconURL string `json:"iconUrl,omitempty"` + Priority int `json:"priority"` + OIDCIssuerURL string `json:"oidcIssuerUrl,omitempty"` + OIDCClientID string `json:"oidcClientId,omitempty"` + OIDCClientSecretSet bool `json:"oidcClientSecretSet,omitempty"` + OIDCLoginURL string `json:"oidcLoginUrl,omitempty"` + OIDCCallbackURL string `json:"oidcCallbackUrl,omitempty"` + SAMLIDPEntityID string `json:"samlIdpEntityId,omitempty"` + SAMLSPEntityID string `json:"samlSpEntityId,omitempty"` + SAMLMetadataURL string `json:"samlMetadataUrl,omitempty"` + SAMLACSURL string `json:"samlAcsUrl,omitempty"` + AllowedGroups []string `json:"allowedGroups,omitempty"` + AllowedDomains []string `json:"allowedDomains,omitempty"` + AllowedEmails []string `json:"allowedEmails,omitempty"` + GroupsClaim string `json:"groupsClaim,omitempty"` + GroupRoleMappings map[string]string `json:"groupRoleMappings,omitempty"` } // SSOProvidersListResponse is the API response shape for list requests.