fix(sso): return full provider config for edit form and register SSO users in RBAC (#1255)

Two remaining issues from #1255 after the 5.1.10 fixes:

1. OIDC/SAML provider edit fields appeared blank because the GET
   endpoint returned a flattened response while the frontend reads
   nested oidc/saml objects. Now returns the full provider config
   with secrets redacted (client secret, SP private key).

2. SSO users didn't appear in Settings > Users because RBAC entries
   were only created when group-role mappings matched. Now ensures
   every SSO user is registered in RBAC on login, even without
   role mappings.

Also fixes: SAML SP private key and certificate lost on edit (no
preservation logic existed), OIDC client secret preservation
hardened to check actual secret presence not just flag.
This commit is contained in:
rcourtman 2026-02-20 10:27:08 +00:00
parent 0ae2806f18
commit afb4dcc2bd
3 changed files with 48 additions and 10 deletions

View file

@ -247,7 +247,7 @@ func (r *Router) handleOIDCCallback(w http.ResponseWriter, req *http.Request) {
log.Debug().Msg("User group membership verified")
}
// RBAC Integration: Map OIDC groups to Pulse roles
// RBAC Integration: Map OIDC groups to Pulse roles and ensure user is registered
if authManager := internalauth.GetManager(); authManager != nil {
groups := extractStringSliceClaim(claims, cfg.GroupsClaim)
var rolesToAssign []string
@ -274,6 +274,9 @@ func (r *Router) handleOIDCCallback(w http.ResponseWriter, req *http.Request) {
} else {
LogAuditEventForTenant(GetOrgID(req.Context()), "oidc_role_assignment", username, GetClientIP(req), req.URL.Path, true, "Auto-assigned roles: "+strings.Join(rolesToAssign, ", "))
}
} else if _, exists := authManager.GetUserAssignment(username); !exists {
// Ensure SSO user appears in the Users list even without role mappings
_ = authManager.UpdateUserRoles(username, []string{})
}
}

View file

@ -205,16 +205,18 @@ func (r *Router) handleSAMLACS(w http.ResponseWriter, req *http.Request) {
}
}
// RBAC Integration: Map SAML groups to Pulse roles
if authManager := internalauth.GetManager(); authManager != nil && len(provider.GroupRoleMappings) > 0 {
// RBAC Integration: Map SAML groups to Pulse roles and ensure user is registered
if authManager := internalauth.GetManager(); authManager != nil {
var rolesToAssign []string
seenRoles := make(map[string]bool)
for _, group := range result.Groups {
if roleID, ok := provider.GroupRoleMappings[group]; ok {
if !seenRoles[roleID] {
rolesToAssign = append(rolesToAssign, roleID)
seenRoles[roleID] = true
if len(provider.GroupRoleMappings) > 0 {
for _, group := range result.Groups {
if roleID, ok := provider.GroupRoleMappings[group]; ok {
if !seenRoles[roleID] {
rolesToAssign = append(rolesToAssign, roleID)
seenRoles[roleID] = true
}
}
}
}
@ -230,6 +232,9 @@ func (r *Router) handleSAMLACS(w http.ResponseWriter, req *http.Request) {
} else {
LogAuditEventForTenant(GetOrgID(req.Context()), "saml_role_assignment", result.Username, GetClientIP(req), req.URL.Path, true, "Auto-assigned roles: "+strings.Join(rolesToAssign, ", "))
}
} else if _, exists := authManager.GetUserAssignment(result.Username); !exists {
// Ensure SSO user appears in the Users list even without role mappings
_ = authManager.UpdateUserRoles(result.Username, []string{})
}
}

View file

@ -180,8 +180,23 @@ func (r *Router) handleGetSSOProvider(w http.ResponseWriter, req *http.Request,
return
}
// Return the full provider config so the edit form can populate all fields.
// We make a shallow copy and redact sensitive secrets.
safe := *provider
if safe.OIDC != nil {
oidcCopy := *safe.OIDC
oidcCopy.ClientSecretSet = oidcCopy.ClientSecret != "" || oidcCopy.ClientSecretSet
oidcCopy.ClientSecret = "" // Never expose the secret
safe.OIDC = &oidcCopy
}
if safe.SAML != nil {
samlCopy := *safe.SAML
samlCopy.SPPrivateKey = "" // Never expose private key
safe.SAML = &samlCopy
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(providerToResponse(provider, r.config.PublicURL))
json.NewEncoder(w).Encode(safe)
}
func (r *Router) handleCreateSSOProvider(w http.ResponseWriter, req *http.Request) {
@ -271,6 +286,11 @@ func (r *Router) handleCreateSSOProvider(w http.ResponseWriter, req *http.Reques
return
}
// Track whether secrets are configured (so updates can preserve them)
if provider.OIDC != nil && provider.OIDC.ClientSecret != "" {
provider.OIDC.ClientSecretSet = true
}
// Add provider
if err := r.ssoConfig.AddProvider(provider); err != nil {
writeErrorResponse(w, http.StatusBadRequest, "add_error", err.Error(), nil)
@ -379,9 +399,19 @@ func (r *Router) handleUpdateSSOProvider(w http.ResponseWriter, req *http.Reques
// Preserve secrets if not provided in update
if updated.Type == config.SSOProviderTypeOIDC && updated.OIDC != nil && existing.OIDC != nil {
if updated.OIDC.ClientSecret == "" && existing.OIDC.ClientSecretSet {
if updated.OIDC.ClientSecret == "" && (existing.OIDC.ClientSecretSet || existing.OIDC.ClientSecret != "") {
updated.OIDC.ClientSecret = existing.OIDC.ClientSecret
updated.OIDC.ClientSecretSet = true
} else if updated.OIDC.ClientSecret != "" {
updated.OIDC.ClientSecretSet = true
}
}
if updated.Type == config.SSOProviderTypeSAML && updated.SAML != nil && existing.SAML != nil {
if updated.SAML.SPPrivateKey == "" && existing.SAML.SPPrivateKey != "" {
updated.SAML.SPPrivateKey = existing.SAML.SPPrivateKey
}
if updated.SAML.SPCertificate == "" && existing.SAML.SPCertificate != "" {
updated.SAML.SPCertificate = existing.SAML.SPCertificate
}
}