fixup! feat: map user admin status from OIDC group claims

Author: jmattheis

api/oidc.go

@@ -445,15 +445,15 @@ func (a *OIDCAPI) resolveUser(idToken *oidc.IDTokenClaims, info *oidc.UserInfo)
 		return nil, http.StatusInternalServerError, fmt.Errorf("database error: %w", err)
 	}
 
-	admin, status, err := a.resolvePermission(idToken.Claims, info.Claims)
+	hasAdminGroup, status, err := a.resolvePermission(idToken.Claims, info.Claims)
 	if err != nil {
 		log.Err(err).Str("oidc_id", oidcID).Interface("idTokenClaims", idToken.Claims).Interface("userinfoClaims", info.Claims).Msg("OIDC: resolve permission")
 		return nil, status, err
 	}
 
 	if user != nil {
-		if admin != nil && user.Admin != *admin {
-			user.Admin = *admin
+		if len(a.GroupsAdmin) > 0 && user.Admin != hasAdminGroup {
+			user.Admin = hasAdminGroup
 			if err := a.DB.UpdateUser(user); err != nil {
 				return nil, http.StatusInternalServerError, fmt.Errorf("database error: %w", err)
 			}
@@ -476,12 +476,12 @@ func (a *OIDCAPI) resolveUser(idToken *oidc.IDTokenClaims, info *oidc.UserInfo)
 		return nil, http.StatusInternalServerError, fmt.Errorf("database error: %w", err)
 	}
 	if byUsername != nil {
-		return a.linkExistingUser(byUsername, oidcID, admin)
+		return a.linkExistingUser(byUsername, oidcID, hasAdminGroup)
 	}
-	return a.registerUser(username, oidcID, admin)
+	return a.registerUser(username, oidcID, hasAdminGroup)
 }
 
-func (a *OIDCAPI) linkExistingUser(user *model.User, oidcID string, admin *bool) (*model.User, int, error) {
+func (a *OIDCAPI) linkExistingUser(user *model.User, oidcID string, hasAdminGroup bool) (*model.User, int, error) {
 	if !a.LinkByUsername {
 		log.Warn().Str("oidc_id", oidcID).Str("username", user.Name).Msgf("OIDC login rejected: a local user with the username already exists and %s is disabled", config.EnvOIDCLinkByUsername)
 		return nil, http.StatusForbidden, fmt.Errorf("a local user with the username %s already exists and linking by username is disabled", user.Name)
@@ -491,8 +491,8 @@ func (a *OIDCAPI) linkExistingUser(user *model.User, oidcID string, admin *bool)
 		return nil, http.StatusForbidden, fmt.Errorf("the user %s is already bound to a different OIDC identity", user.Name)
 	}
 	user.OIDCID = &oidcID
-	if admin != nil {
-		user.Admin = *admin
+	if len(a.GroupsAdmin) > 0 {
+		user.Admin = hasAdminGroup
 	}
 	if err := a.DB.UpdateUser(user); err != nil {
 		return nil, http.StatusInternalServerError, fmt.Errorf("failed to bind user to OIDC identity: %w", err)
@@ -501,7 +501,7 @@ func (a *OIDCAPI) linkExistingUser(user *model.User, oidcID string, admin *bool)
 	return user, 0, nil
 }
 
-func (a *OIDCAPI) registerUser(username, oidcID string, admin *bool) (*model.User, int, error) {
+func (a *OIDCAPI) registerUser(username, oidcID string, hasAdminGroup bool) (*model.User, int, error) {
 	if !a.AutoRegister {
 		return nil, http.StatusForbidden, errors.New("user does not exist and auto-registration is disabled")
 	}
@@ -511,8 +511,8 @@ func (a *OIDCAPI) registerUser(username, oidcID string, admin *bool) (*model.Use
 		OIDCID: &oidcID,
 	}
 
-	if admin != nil {
-		user.Admin = *admin
+	if len(a.GroupsAdmin) > 0 {
+		user.Admin = hasAdminGroup
 	}
 
 	if err := a.DB.CreateUser(user); err != nil {
@@ -550,14 +550,14 @@ func (a *OIDCAPI) popPendingSession(key string) (*pendingOIDCSession, bool) {
 	return nil, false
 }
 
-func (a *OIDCAPI) resolvePermission(idTokenClaims, userInfoClaims map[string]any) (*bool, int, error) {
+func (a *OIDCAPI) resolvePermission(idTokenClaims, userInfoClaims map[string]any) (bool, int, error) {
 	if a.GroupsClaim == "" {
-		return nil, 0, nil
+		return false, 0, nil
 	}
 
 	groupsRaw, ok := lookupClaim(a.GroupsClaim, idTokenClaims, userInfoClaims)
 	if !ok {
-		return nil, http.StatusInternalServerError, fmt.Errorf("groups claim %q is missing", a.GroupsClaim)
+		return false, http.StatusInternalServerError, fmt.Errorf("groups claim %q is missing", a.GroupsClaim)
 	}
 
 	var groups []string
@@ -568,25 +568,23 @@ func (a *OIDCAPI) resolvePermission(idTokenClaims, userInfoClaims map[string]any
 		for _, groupRaw := range groupsRaw {
 			group, ok := groupRaw.(string)
 			if !ok {
-				return nil, http.StatusInternalServerError, fmt.Errorf("groups claim %q contains a non-string element: %#v", a.GroupsClaim, groupRaw)
+				return false, http.StatusInternalServerError, fmt.Errorf("groups claim %q contains a non-string element: %#v", a.GroupsClaim, groupRaw)
 			}
 			groups = append(groups, group)
 		}
 	case string:
 		groups = append(groups, groupsRaw)
 	default:
-		return nil, http.StatusInternalServerError, fmt.Errorf("groups claim %q is not a string or string array: %#v", a.GroupsClaim, groupsRaw)
+		return false, http.StatusInternalServerError, fmt.Errorf("groups claim %q is not a string or string array: %#v", a.GroupsClaim, groupsRaw)
 	}
 
 	switch {
 	case containsAny(a.GroupsAdmin, groups):
-		t := true
-		return &t, 0, nil
+		return true, 0, nil
 	case len(a.GroupsUser) == 0 || containsAny(a.GroupsUser, groups):
-		f := false
-		return &f, 0, nil
+		return false, 0, nil
 	default:
-		return nil, http.StatusForbidden, errors.New("user is not in any allowed group")
+		return false, http.StatusForbidden, errors.New("user is not in any allowed group")
 	}
 }