working on the backup management parts

backend/internal/iam/handler.go

@@ -260,118 +260,158 @@ 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)
-		// Get current roles
 		currentRoles, err := GetUserRoles(h.db, userID)
 		if err != nil {
-			h.logger.Error("Failed to get current roles", "error", err)
-		}
-		h.logger.Info("Current roles", "user_id", userID, "current_roles", currentRoles)
-
-		// Remove roles that are not in the new list
-		for _, role := range currentRoles {
-			found := false
-			for _, newRole := range *req.Roles {
-				if role == newRole {
-					found = true
-					break
-				}
-			}
-			if !found {
-				roleID, err := GetRoleIDByName(h.db, role)
-				if err == nil {
-					err = RemoveUserRole(h.db, userID, roleID)
-					if err != nil {
-						h.logger.Error("Failed to remove role", "error", err, "role", role)
-					} else {
-						h.logger.Info("Role removed", "user_id", userID, "role", role)
-					}
-				} else {
-					h.logger.Error("Failed to get role ID", "error", err, "role", role)
-				}
-			}
+			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
 		}
 
-		// Add new roles that are not in the current list
-		for _, roleName := range *req.Roles {
+		rolesToAdd := []string{}
+		rolesToRemove := []string{}
+
+		// Find roles to add
+		for _, newRole := range *req.Roles {
 			found := false
 			for _, currentRole := range currentRoles {
-				if roleName == currentRole {
+				if newRole == currentRole {
 					found = true
 					break
 				}
 			}
 			if !found {
-				roleID, err := GetRoleIDByName(h.db, roleName)
-				if err == nil {
-					err = AddUserRole(h.db, userID, roleID, currentUser.ID)
-					if err != nil {
-						h.logger.Error("Failed to add role", "error", err, "role", roleName)
-					} else {
-						h.logger.Info("Role added", "user_id", userID, "role", roleName)
-					}
-				} else {
-					h.logger.Error("Failed to get role ID", "error", err, "role", roleName)
+				rolesToAdd = append(rolesToAdd, newRole)
+			}
+		}
+
+		// Find roles to remove
+		for _, currentRole := range currentRoles {
+			found := false
+			for _, newRole := range *req.Roles {
+				if currentRole == newRole {
+					found = true
+					break
 				}
 			}
+			if !found {
+				rolesToRemove = append(rolesToRemove, currentRole)
+			}
+		}
+
+		// Add new roles
+		for _, roleName := range rolesToAdd {
+			roleID, err := GetRoleIDByName(h.db, roleName)
+			if err != nil {
+				if err == sql.ErrNoRows {
+					h.logger.Warn("Attempted to add non-existent role to user", "user_id", userID, "role_name", roleName)
+					c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("role '%s' not found", roleName)})
+					return
+				}
+				h.logger.Error("Failed to get role ID by name", "role_name", roleName, "error", err)
+				c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to process roles"})
+				return
+			}
+			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.Info("Role added to user", "user_id", userID, "role_name", roleName)
+		}
+
+		// Remove old roles
+		for _, roleName := range rolesToRemove {
+			roleID, err := GetRoleIDByName(h.db, roleName)
+			if err != nil {
+				// This case should be rare, but handle it defensively
+				h.logger.Error("Failed to get role ID for role to be removed", "role_name", roleName, "error", err)
+				continue
+			}
+			if err := RemoveUserRole(h.db, userID, roleID); err != nil {
+				h.logger.Error("Failed to remove role from user", "user_id", userID, "role_id", roleID, "error", err)
+				c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to remove role"})
+				return
+			}
+			h.logger.Info("Role removed from user", "user_id", userID, "role_name", roleName)
 		}
 	}
 
 	// Update groups if provided
 	if req.Groups != nil {
 		h.logger.Info("Updating user groups", "user_id", userID, "groups", *req.Groups)
-		// Get current groups
 		currentGroups, err := GetUserGroups(h.db, userID)
 		if err != nil {
-			h.logger.Error("Failed to get current groups", "error", err)
-		}
-		h.logger.Info("Current groups", "user_id", userID, "current_groups", currentGroups)
-
-		// Remove groups that are not in the new list
-		for _, group := range currentGroups {
-			found := false
-			for _, newGroup := range *req.Groups {
-				if group == newGroup {
-					found = true
-					break
-				}
-			}
-			if !found {
-				groupObj, err := GetGroupByName(h.db, group)
-				if err == nil {
-					err = RemoveUserFromGroup(h.db, userID, groupObj.ID)
-					if err != nil {
-						h.logger.Error("Failed to remove group", "error", err, "group", group)
-					} else {
-						h.logger.Info("Group removed", "user_id", userID, "group", group)
-					}
-				} else {
-					h.logger.Error("Failed to get group", "error", err, "group", group)
-				}
-			}
+			h.logger.Error("Failed to get current groups for user", "user_id", userID, "error", err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to process user groups"})
+			return
 		}
 
-		// Add new groups that are not in the current list
-		for _, groupName := range *req.Groups {
+		groupsToAdd := []string{}
+		groupsToRemove := []string{}
+
+		// Find groups to add
+		for _, newGroup := range *req.Groups {
 			found := false
 			for _, currentGroup := range currentGroups {
-				if groupName == currentGroup {
+				if newGroup == currentGroup {
 					found = true
 					break
 				}
 			}
 			if !found {
-				groupObj, err := GetGroupByName(h.db, groupName)
-				if err == nil {
-					err = AddUserToGroup(h.db, userID, groupObj.ID, currentUser.ID)
-					if err != nil {
-						h.logger.Error("Failed to add group", "error", err, "group", groupName)
-					} else {
-						h.logger.Info("Group added", "user_id", userID, "group", groupName)
-					}
-				} else {
-					h.logger.Error("Failed to get group", "error", err, "group", groupName)
+				groupsToAdd = append(groupsToAdd, newGroup)
+			}
+		}
+
+		// Find groups to remove
+		for _, currentGroup := range currentGroups {
+			found := false
+			for _, newGroup := range *req.Groups {
+				if currentGroup == newGroup {
+					found = true
+					break
 				}
 			}
+			if !found {
+				groupsToRemove = append(groupsToRemove, currentGroup)
+			}
+		}
+
+		// Add new groups
+		for _, groupName := range groupsToAdd {
+			group, err := GetGroupByName(h.db, groupName)
+			if err != nil {
+				if err == sql.ErrNoRows {
+					h.logger.Warn("Attempted to add user to non-existent group", "user_id", userID, "group_name", groupName)
+					c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("group '%s' not found", groupName)})
+					return
+				}
+				h.logger.Error("Failed to get group by name", "group_name", groupName, "error", err)
+				c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to process groups"})
+				return
+			}
+			if err := AddUserToGroup(h.db, userID, group.ID, currentUser.ID); err != nil {
+				h.logger.Error("Failed to add user to group", "user_id", userID, "group_id", group.ID, "error", err)
+				// Don't return early, continue with other groups
+				continue
+			}
+			h.logger.Info("User added to group", "user_id", userID, "group_name", groupName)
+		}
+
+		// Remove old groups
+		for _, groupName := range groupsToRemove {
+			group, err := GetGroupByName(h.db, groupName)
+			if err != nil {
+				// This case should be rare, but handle it defensively
+				h.logger.Error("Failed to get group ID for group to be removed", "group_name", groupName, "error", err)
+				continue
+			}
+			if err := RemoveUserFromGroup(h.db, userID, group.ID); err != nil {
+				h.logger.Error("Failed to remove user from group", "user_id", userID, "group_id", group.ID, "error", err)
+				c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to remove user from group"})
+				return
+			}
+			h.logger.Info("User removed from group", "user_id", userID, "group_name", groupName)
 		}
 	}