fix mostly bugs on system management, and user roles and group assignment

backend/internal/iam/group.go

@@ -88,11 +88,14 @@ func GetUserGroups(db *database.DB, userID string) ([]string, error) {
 	for rows.Next() {
 		var groupName string
 		if err := rows.Scan(&groupName); err != nil {
-			return nil, err
+			return []string{}, err
 		}
 		groups = append(groups, groupName)
 	}
 
+	if groups == nil {
+		groups = []string{}
+	}
 	return groups, rows.Err()
 }
 

backend/internal/iam/handler.go

@@ -69,6 +69,17 @@ func (h *Handler) ListUsers(c *gin.Context) {
 		permissions, _ := GetUserPermissions(h.db, u.ID)
 		groups, _ := GetUserGroups(h.db, u.ID)
 
+		// Ensure arrays are never nil (use empty slice instead)
+		if roles == nil {
+			roles = []string{}
+		}
+		if permissions == nil {
+			permissions = []string{}
+		}
+		if groups == nil {
+			groups = []string{}
+		}
+
 		users = append(users, map[string]interface{}{
 			"id":            u.ID,
 			"username":      u.Username,
@@ -138,6 +149,17 @@ func (h *Handler) GetUser(c *gin.Context) {
 	permissions, _ := GetUserPermissions(h.db, userID)
 	groups, _ := GetUserGroups(h.db, userID)
 
+	// Ensure arrays are never nil (use empty slice instead)
+	if roles == nil {
+		roles = []string{}
+	}
+	if permissions == nil {
+		permissions = []string{}
+	}
+	if groups == nil {
+		groups = []string{}
+	}
+
 	c.JSON(http.StatusOK, gin.H{
 		"id":            user.ID,
 		"username":      user.Username,
@@ -236,6 +258,8 @@ func (h *Handler) UpdateUser(c *gin.Context) {
 	}
 
 	// Allow update if roles or groups are provided, even if no other fields are updated
+	// Note: req.Roles and req.Groups can be empty arrays ([]), which is different from nil
+	// Empty array means "remove all roles/groups", nil means "don't change roles/groups"
 	if len(updates) == 1 && req.Roles == nil && req.Groups == nil {
 		c.JSON(http.StatusBadRequest, gin.H{"error": "no fields to update"})
 		return
@@ -259,13 +283,14 @@ func (h *Handler) UpdateUser(c *gin.Context) {
 
 	// Update roles if provided
 	if req.Roles != nil {
-		h.logger.Info("Updating user roles", "user_id", userID, "roles", *req.Roles)
+		h.logger.Info("Updating user roles", "user_id", userID, "requested_roles", *req.Roles)
 		currentRoles, err := GetUserRoles(h.db, userID)
 		if err != nil {
 			h.logger.Error("Failed to get current roles for user", "user_id", userID, "error", err)
 			c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to process user roles"})
 			return
 		}
+		h.logger.Info("Current user roles", "user_id", userID, "current_roles", currentRoles)
 
 		rolesToAdd := []string{}
 		rolesToRemove := []string{}
@@ -298,8 +323,15 @@ func (h *Handler) UpdateUser(c *gin.Context) {
 			}
 		}
 
+		h.logger.Info("Roles to add", "user_id", userID, "roles_to_add", rolesToAdd, "count", len(rolesToAdd))
+		h.logger.Info("Roles to remove", "user_id", userID, "roles_to_remove", rolesToRemove, "count", len(rolesToRemove))
+
 		// Add new roles
+		if len(rolesToAdd) == 0 {
+			h.logger.Info("No roles to add", "user_id", userID)
+		}
 		for _, roleName := range rolesToAdd {
+			h.logger.Info("Processing role to add", "user_id", userID, "role_name", roleName)
 			roleID, err := GetRoleIDByName(h.db, roleName)
 			if err != nil {
 				if err == sql.ErrNoRows {
@@ -311,12 +343,13 @@ func (h *Handler) UpdateUser(c *gin.Context) {
 				c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to process roles"})
 				return
 			}
+			h.logger.Info("Attempting to add role", "user_id", userID, "role_id", roleID, "role_name", roleName, "assigned_by", currentUser.ID)
 			if err := AddUserRole(h.db, userID, roleID, currentUser.ID); err != nil {
-				h.logger.Error("Failed to add role to user", "user_id", userID, "role_id", roleID, "error", err)
-				// Don't return early, continue with other roles
-				continue
+				h.logger.Error("Failed to add role to user", "user_id", userID, "role_id", roleID, "role_name", roleName, "error", err)
+				c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to add role '%s': %v", roleName, err)})
+				return
 			}
-			h.logger.Info("Role added to user", "user_id", userID, "role_name", roleName)
+			h.logger.Info("Role successfully added to user", "user_id", userID, "role_id", roleID, "role_name", roleName)
 		}
 
 		// Remove old roles
@@ -415,8 +448,48 @@ func (h *Handler) UpdateUser(c *gin.Context) {
 		}
 	}
 
-	h.logger.Info("User updated", "user_id", userID)
-	c.JSON(http.StatusOK, gin.H{"message": "user updated successfully"})
+	// Fetch updated user data to return
+	updatedUser, err := GetUserByID(h.db, userID)
+	if err != nil {
+		h.logger.Error("Failed to fetch updated user", "user_id", userID, "error", err)
+		c.JSON(http.StatusOK, gin.H{"message": "user updated successfully"})
+		return
+	}
+
+	// Get updated roles, permissions, and groups
+	updatedRoles, _ := GetUserRoles(h.db, userID)
+	updatedPermissions, _ := GetUserPermissions(h.db, userID)
+	updatedGroups, _ := GetUserGroups(h.db, userID)
+
+	// Ensure arrays are never nil
+	if updatedRoles == nil {
+		updatedRoles = []string{}
+	}
+	if updatedPermissions == nil {
+		updatedPermissions = []string{}
+	}
+	if updatedGroups == nil {
+		updatedGroups = []string{}
+	}
+
+	h.logger.Info("User updated", "user_id", userID, "roles", updatedRoles, "groups", updatedGroups)
+	c.JSON(http.StatusOK, gin.H{
+		"message": "user updated successfully",
+		"user": gin.H{
+			"id":            updatedUser.ID,
+			"username":      updatedUser.Username,
+			"email":         updatedUser.Email,
+			"full_name":     updatedUser.FullName,
+			"is_active":     updatedUser.IsActive,
+			"is_system":     updatedUser.IsSystem,
+			"roles":         updatedRoles,
+			"permissions":   updatedPermissions,
+			"groups":        updatedGroups,
+			"created_at":    updatedUser.CreatedAt,
+			"updated_at":    updatedUser.UpdatedAt,
+			"last_login_at": updatedUser.LastLoginAt,
+		},
+	})
 }
 
 // DeleteUser deletes a user

backend/internal/iam/user.go

@@ -2,6 +2,7 @@ package iam
 
 import (
 	"database/sql"
+	"fmt"
 	"time"
 
 	"github.com/atlasos/calypso/internal/common/database"
@@ -90,11 +91,14 @@ func GetUserRoles(db *database.DB, userID string) ([]string, error) {
 	for rows.Next() {
 		var role string
 		if err := rows.Scan(&role); err != nil {
-			return nil, err
+			return []string{}, err
 		}
 		roles = append(roles, role)
 	}
 
+	if roles == nil {
+		roles = []string{}
+	}
 	return roles, rows.Err()
 }
 
@@ -118,11 +122,14 @@ func GetUserPermissions(db *database.DB, userID string) ([]string, error) {
 	for rows.Next() {
 		var perm string
 		if err := rows.Scan(&perm); err != nil {
-			return nil, err
+			return []string{}, err
 		}
 		permissions = append(permissions, perm)
 	}
 
+	if permissions == nil {
+		permissions = []string{}
+	}
 	return permissions, rows.Err()
 }
 
@@ -133,8 +140,23 @@ func AddUserRole(db *database.DB, userID, roleID, assignedBy string) error {
 		VALUES ($1, $2, $3)
 		ON CONFLICT (user_id, role_id) DO NOTHING
 	`
-	_, err := db.Exec(query, userID, roleID, assignedBy)
-	return err
+	result, err := db.Exec(query, userID, roleID, assignedBy)
+	if err != nil {
+		return fmt.Errorf("failed to insert user role: %w", err)
+	}
+
+	// Check if row was actually inserted (not just skipped due to conflict)
+	rowsAffected, err := result.RowsAffected()
+	if err != nil {
+		return fmt.Errorf("failed to get rows affected: %w", err)
+	}
+
+	if rowsAffected == 0 {
+		// Row already exists, this is not an error but we should know about it
+		return nil // ON CONFLICT DO NOTHING means this is expected
+	}
+
+	return nil
 }
 
 // RemoveUserRole removes a role from a user