diff --git a/api/login.go b/api/login.go index fb15b27d1..f87610a6e 100644 --- a/api/login.go +++ b/api/login.go @@ -555,6 +555,7 @@ type claimResult struct { username string name string email string + admin bool } func parseClaim(str string, claims map[string]any) (string, bool) { @@ -629,6 +630,10 @@ func parseClaims(claims map[string]any, provider util.ClaimsProvider) (res claim res.name = getRandomProfileName() } + if provider.IsAdminMappingEnable() { + res.admin = provider.IsAdminUserClaims(claims) + } + return } @@ -778,6 +783,7 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) { Email: claims.email, External: true, Pro: true, + Admin: provider.IsAdminMappingEnable() && claims.admin, } user, err = helpers.Store(r).CreateUserWithoutPassword(user) if err != nil { @@ -785,12 +791,23 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) return } - } + } else { + if !user.External { + log.Error(fmt.Errorf("OIDC user '%s' conflicts with local user", user.Username)) + http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) + return + } - if !user.External { - log.Error(fmt.Errorf("OIDC user '%s' conflicts with local user", user.Username)) - http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) - return + // For existing users: update only if admin mapping is enabled and user admin field needs to change. + if provider.IsAdminMappingEnable() && user.Admin != claims.admin { + user.Admin = claims.admin + err = helpers.Store(r).UpdateUser(db.UserWithPwd{Pwd: "", User: user}) + if err != nil { + log.Error(fmt.Errorf("Failed update OIDC user '%s': %v", user.Username, err)) + http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) + return + } + } } createSession(w, r, user, true) diff --git a/util/OdbcProvider.go b/util/OdbcProvider.go index 24b68f098..9d9fc1b83 100644 --- a/util/OdbcProvider.go +++ b/util/OdbcProvider.go @@ -1,5 +1,7 @@ package util +import log "github.com/sirupsen/logrus" + type OidcProvider struct { ClientID string `json:"client_id"` ClientIDFile string `json:"client_id_file"` @@ -15,15 +17,49 @@ type OidcProvider struct { UsernameClaim string `json:"username_claim" default:"preferred_username"` NameClaim string `json:"name_claim" default:"preferred_username"` EmailClaim string `json:"email_claim" default:"email"` + GroupsClaim string `json:"groups_claim" default:"groups"` + AdminGroup string `json:"admin_group" default:""` // Group from the groups in the claims that members will be mapped as admin on semaphore Order int `json:"order"` // ReturnViaState when true, passes the return path via the OAuth state parameter instead of the redirect URL path. This is useful for OAuth providers that have strict redirect URL validation. - ReturnViaState bool `json:"return_via_state"` + ReturnViaState bool `json:"return_via_state"` } type ClaimsProvider interface { GetUsernameClaim() string GetEmailClaim() string GetNameClaim() string + IsAdminMappingEnable() bool + IsAdminUserClaims(claims map[string]any) bool +} + +func (p *OidcProvider) IsAdminMappingEnable() bool { + return p.AdminGroup != "" +} + +func (p *OidcProvider) IsAdminUserClaims(claims map[string]any) bool { + // SECURITY NOTE: Group membership check is CASE-SENSITIVE. + // We match exact string values from the groups claim. + // This follows JWT/OIDC conventions and prevents accidental privilege escalation + // due to inconsistent casing from IdPs (e.g. "Admin" ≠ "admin"). + rawList, ok := claims[p.GroupsClaim].([]any) + + if !ok { + log.Info("Warning: groups claim is not a list/array of strings → cannot reliably detect admin group membership. Assuming non-admin for security.") + return false + } + + for _, g := range rawList { + group, ok := g.(string) + + if ok { + if p.AdminGroup == group { + return true + } + } else { + log.Info("Warning: groups claim contains non-string value(s) — ignoring invalid entries") + } + } + return false } func (p *OidcProvider) GetUsernameClaim() string { diff --git a/util/OdbcProvider_test.go b/util/OdbcProvider_test.go new file mode 100644 index 000000000..2397293b3 --- /dev/null +++ b/util/OdbcProvider_test.go @@ -0,0 +1,69 @@ +package util + +import "testing" + +func TestIsAdminMappingEnable(t *testing.T) { + p := OidcProvider{ + AdminGroup: "admin", + } + if !p.IsAdminMappingEnable() { + t.Errorf("OIDC admin mapping is not enabled even though \"AdminGroup\" claim is not empty") + } + + p = OidcProvider{} + if p.IsAdminMappingEnable() { + t.Errorf("OIDC admin mapping is enabled even though \"AdminGroup\" claim is empty") + } +} + +func TestIsAdminUserClaims(t *testing.T) { + // Test IsAdminUserClaims correctly identifies admin users when they're in the admin group + p := OidcProvider{ + GroupsClaim: "groups", + AdminGroup: "admin", + } + claims := map[string]any{ + "groups": []string{"group1", "admin", "group2"}, + } + + if p.IsAdminUserClaims(claims) { + t.Errorf("IsAdminUserClaims does not correctly identify admin users when they are in the admin group") + } + + // Test IsAdminUserClaims returns false when groups claim is missing + p = OidcProvider{ + GroupsClaim: "groups", + AdminGroup: "admin", + } + claims = map[string]any{} + + if p.IsAdminUserClaims(claims) { + t.Errorf("IsAdminUserClaims does not returns false when groups claim is missing") + } + + // Test IsAdminUserClaims handles non-array group claims gracefully + p = OidcProvider{ + GroupsClaim: "groups", + AdminGroup: "admin", + } + claims = map[string]any{ + "groups": "admin", + } + + if p.IsAdminUserClaims(claims) { + t.Errorf("IsAdminUserClaims does not handles non-array group claims gracefully") + } + + // Test IsAdminUserClaims handles non-string group values within the array + p = OidcProvider{ + GroupsClaim: "groups", + AdminGroup: "admin", + } + claims = map[string]any{ + "groups": []int{1, 2}, + } + + if p.IsAdminUserClaims(claims) { + t.Errorf("IsAdminUserClaims does not handles non-string group values within the array") + } +} diff --git a/util/config.go b/util/config.go index b2d66f38c..b81914d7e 100644 --- a/util/config.go +++ b/util/config.go @@ -64,6 +64,20 @@ type LdapMappings struct { CN string `json:"cn" env:"SEMAPHORE_LDAP_MAPPING_CN" default:"cn"` } +func (p *LdapMappings) IsAdminMappingEnable() bool { + // LDAP admin mapping is not supported. Always returns false, meaning users + // authenticated via LDAP will not automatically receive admin privileges + // based on group membership. + return false +} + +func (p *LdapMappings) IsAdminUserClaims(claims map[string]any) bool { + // LDAP admin mapping is not supported. Always returns false, meaning users + // authenticated via LDAP will not automatically receive admin privileges + // based on group membership. + return false +} + func (p *LdapMappings) GetUsernameClaim() string { return p.UID }