From c21693489a637d4f42447604fcf3cbe1e16195fd Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 9 Jul 2026 09:52:59 +0100 Subject: [PATCH] Adopt the agent's preferred host when re-registering a matched node The resolved-identity match in canonical auto-register always preserved the stored host, so the route-aware IP preference added to the agent in 022f170df could never take effect for an already-registered node: a reinstall re-matched the node and kept the stale short-DNS host forever. Preserve the stored host only when it is absent from the agent's ordered candidate list (an admin-managed endpoint the agent cannot see); when the agent itself lists the stored host as a lower-priority candidate, adopt the newly selected host. Identity continuity (same node record, same token) is unchanged. --- internal/api/oidc_handlers.go | 58 +++++++++++++++++++++++++------ internal/api/oidc_mapping_test.go | 39 +++++++++++++++------ internal/api/saml_handlers.go | 12 +------ 3 files changed, 77 insertions(+), 32 deletions(-) diff --git a/internal/api/oidc_handlers.go b/internal/api/oidc_handlers.go index 2f5281139..cb148b6a1 100644 --- a/internal/api/oidc_handlers.go +++ b/internal/api/oidc_handlers.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/url" + "sort" "strings" "time" @@ -186,6 +187,52 @@ func intersects(values []string, allowed []string) bool { return false } +// resolveGroupRoles maps a user's group memberships to Pulse role IDs using the +// provider's group-to-role mappings. Group names are matched case-insensitively +// with surrounding whitespace trimmed, matching how AllowedGroups are matched in +// intersects, so an IdP group like "Admins" still resolves a mapping keyed +// "admins". Without this an admin whose IdP casing differs from the configured +// mapping passes the AllowedGroups gate but silently gets no role and lands as a +// default user. Returned roles preserve first-seen group order and are +// de-duplicated. Mapping keys are visited in sorted order so a normalized-key +// collision resolves deterministically. +func resolveGroupRoles(groups []string, mappings map[string]string) []string { + if len(groups) == 0 || len(mappings) == 0 { + return nil + } + + keys := make([]string, 0, len(mappings)) + for group := range mappings { + keys = append(keys, group) + } + sort.Strings(keys) + + normalized := make(map[string]string, len(mappings)) + for _, group := range keys { + key := strings.ToLower(strings.TrimSpace(group)) + if key == "" { + continue + } + if _, exists := normalized[key]; !exists { + normalized[key] = mappings[group] + } + } + + var roles []string + seen := make(map[string]bool) + for _, group := range groups { + key := strings.ToLower(strings.TrimSpace(group)) + if key == "" { + continue + } + if roleID, ok := normalized[key]; ok && !seen[roleID] { + roles = append(roles, roleID) + seen[roleID] = true + } + } + return roles +} + // InitializeOIDCProviders initializes all enabled SSO OIDC providers at startup. func (r *Router) InitializeOIDCProviders(ctx context.Context) error { if r.ssoConfig == nil { @@ -587,16 +634,7 @@ func (r *Router) handleSSOOIDCCallback(w http.ResponseWriter, req *http.Request) // SSO users by the provider-scoped subject instead of mutable display claims. if authManager := internalauth.GetManager(); authManager != nil { groups := extractStringSliceClaim(claims, provider.GroupsClaim) - var rolesToAssign []string - seenRoles := make(map[string]bool) - for _, group := range groups { - if roleID, ok := provider.GroupRoleMappings[group]; ok { - if !seenRoles[roleID] { - rolesToAssign = append(rolesToAssign, roleID) - seenRoles[roleID] = true - } - } - } + rolesToAssign := resolveGroupRoles(groups, provider.GroupRoleMappings) legacyCandidates := ssoLegacyPrincipalCandidates(username, email, extractStringClaim(claims, "name")) if err := applySSORoleAssignments(authManager, principal, legacyCandidates, rolesToAssign, len(provider.GroupRoleMappings) > 0, true); err != nil { log.Error().Err(err).Str("user", principal).Str("display_user", username).Msg("Failed to update SSO OIDC user roles") diff --git a/internal/api/oidc_mapping_test.go b/internal/api/oidc_mapping_test.go index 1f7ec930d..e3e741e37 100644 --- a/internal/api/oidc_mapping_test.go +++ b/internal/api/oidc_mapping_test.go @@ -67,6 +67,33 @@ func TestOIDCRoleMappingLogic(t *testing.T) { }, expectedRoles: nil, }, + // Regression cases for #1535: the IdP's group casing/whitespace must + // not have to match the configured mapping exactly. AllowedGroups + // already match case-insensitively (intersects), so role mapping has to + // as well or a user passes the group gate but silently lands with no + // role. + { + name: "case-insensitive group match", + groupsClaim: "groups", + mappings: map[string]string{ + "admins": "admin", + }, + claims: map[string]any{ + "groups": []string{"Admins"}, + }, + expectedRoles: []string{"admin"}, + }, + { + name: "whitespace-padded mapping key match", + groupsClaim: "groups", + mappings: map[string]string{ + " admin ": "admin", + }, + claims: map[string]any{ + "groups": []string{"admin"}, + }, + expectedRoles: []string{"admin"}, + }, } for _, tt := range tests { @@ -77,17 +104,7 @@ func TestOIDCRoleMappingLogic(t *testing.T) { } groups := extractStringSliceClaim(tt.claims, cfg.GroupsClaim) - var rolesToAssign []string - seenRoles := make(map[string]bool) - - for _, group := range groups { - if roleID, ok := cfg.GroupRoleMappings[group]; ok { - if !seenRoles[roleID] { - rolesToAssign = append(rolesToAssign, roleID) - seenRoles[roleID] = true - } - } - } + rolesToAssign := resolveGroupRoles(groups, cfg.GroupRoleMappings) if len(rolesToAssign) != len(tt.expectedRoles) { t.Errorf("expected %d roles, got %d", len(tt.expectedRoles), len(rolesToAssign)) diff --git a/internal/api/saml_handlers.go b/internal/api/saml_handlers.go index 1d298f0df..5b9adfa82 100644 --- a/internal/api/saml_handlers.go +++ b/internal/api/saml_handlers.go @@ -268,17 +268,7 @@ func (r *Router) handleSAMLACS(w http.ResponseWriter, req *http.Request) { // principal. When mappings are configured they are authoritative, including // the case where the current assertion maps to no roles. 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 - } - } - } + rolesToAssign := resolveGroupRoles(result.Groups, provider.GroupRoleMappings) legacyCandidates := ssoLegacyPrincipalCandidates(result.Username, result.Email, result.NameID) roleErr := applySSORoleAssignments(authManager, principal, legacyCandidates, rolesToAssign, len(provider.GroupRoleMappings) > 0, false)