Skip to content
Open
27 changes: 22 additions & 5 deletions api/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Comment thread
gaetan-steininger marked this conversation as resolved.

return
}

Expand Down Expand Up @@ -778,19 +783,31 @@ 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 {
log.Error(err.Error())
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)
Expand Down
38 changes: 37 additions & 1 deletion util/OdbcProvider.go
Original file line number Diff line number Diff line change
@@ -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"`
Expand All @@ -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
}
Comment thread
gaetan-steininger marked this conversation as resolved.
Comment thread
gaetan-steininger marked this conversation as resolved.

func (p *OidcProvider) GetUsernameClaim() string {
Expand Down
69 changes: 69 additions & 0 deletions util/OdbcProvider_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
14 changes: 14 additions & 0 deletions util/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading