Allow clearing SSO provider restriction fields from Settings

Empty allowedGroups, allowedDomains, allowedEmails, groupsClaim and
groupRoleMappings in a provider PUT were silently restored from the
existing config, so an admin could never clear them. The guards
shielded against lossy round-trips that no longer exist: the detail
GET and the flat list response both carry these fields, so an empty
value is an intentional clear. Nested OIDC/SAML config and the client
secret stay preserved since toggle payloads omit them and secrets are
never echoed in reads.

Also expose groupsClaim and groupRoleMappings in the shared extensions
list response so the enterprise list matches the core one, and make
the Settings enable/disable toggle send only writable fields; the old
list-item spread included computed response fields that the enterprise
strict PUT decoder rejects.
This commit is contained in:
rcourtman 2026-07-07 22:04:46 +01:00
parent 887513c020
commit c6fdd9058c
5 changed files with 92 additions and 37 deletions

View file

@ -16,6 +16,8 @@ export interface SSOProvider {
allowedGroups?: string[];
allowedDomains?: string[];
allowedEmails?: string[];
groupsClaim?: string;
groupRoleMappings?: Record<string, string>;
}
export interface SSOProvidersResponse {

View file

@ -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) {

View file

@ -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 {

View file

@ -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)

View file

@ -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.