feat: handle null account item issue in CheckPermissionForUpdateUser() (#3202)

* feat: improve the logic of the permission check code for users to modify account items * fix: add skip operation for deleted account items in update-user API * fix: add the function of removing deleted account item
2025-07-02 03:00:18 +08:00 · 2024-09-14 15:00:10 +08:00
parent 64491abc64
commit e2ce9ad625
1 changed files with 195 additions and 39 deletions
--- a/object/user_util.go
+++ b/object/user_util.go
@ -271,113 +271,213 @@ func CheckPermissionForUpdateUser(oldUser, newUser *User, isAdmin bool, lang str

 	if oldUser.Owner != newUser.Owner {
 		item := GetAccountItemByName("Organization", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Owner = oldUser.Owner
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Name != newUser.Name {
 		item := GetAccountItemByName("Name", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Name = oldUser.Name
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Id != newUser.Id {
 		item := GetAccountItemByName("ID", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Id = oldUser.Id
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.DisplayName != newUser.DisplayName {
 		item := GetAccountItemByName("Display name", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.DisplayName = oldUser.DisplayName
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Avatar != newUser.Avatar {
 		item := GetAccountItemByName("Avatar", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Avatar = oldUser.Avatar
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Type != newUser.Type {
 		item := GetAccountItemByName("User type", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Type = oldUser.Type
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	// The password is *** when not modified
 	if oldUser.Password != newUser.Password && newUser.Password != "***" {
 		item := GetAccountItemByName("Password", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Password = oldUser.Password
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Email != newUser.Email {
 		item := GetAccountItemByName("Email", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Email = oldUser.Email
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Phone != newUser.Phone {
 		item := GetAccountItemByName("Phone", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Phone = oldUser.Phone
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.CountryCode != newUser.CountryCode {
 		item := GetAccountItemByName("Country code", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.CountryCode = oldUser.CountryCode
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Region != newUser.Region {
 		item := GetAccountItemByName("Country/Region", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Region = oldUser.Region
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Location != newUser.Location {
 		item := GetAccountItemByName("Location", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Location = oldUser.Location
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Affiliation != newUser.Affiliation {
 		item := GetAccountItemByName("Affiliation", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Affiliation = oldUser.Affiliation
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Title != newUser.Title {
 		item := GetAccountItemByName("Title", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Title = oldUser.Title
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Homepage != newUser.Homepage {
 		item := GetAccountItemByName("Homepage", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Homepage = oldUser.Homepage
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Bio != newUser.Bio {
 		item := GetAccountItemByName("Bio", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Bio = oldUser.Bio
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.Tag != newUser.Tag {
 		item := GetAccountItemByName("Tag", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Tag = oldUser.Tag
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.SignupApplication != newUser.SignupApplication {
 		item := GetAccountItemByName("Signup application", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.SignupApplication = oldUser.SignupApplication
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Gender != newUser.Gender {
 		item := GetAccountItemByName("Gender", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Gender = oldUser.Gender
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Birthday != newUser.Birthday {
 		item := GetAccountItemByName("Birthday", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Birthday = oldUser.Birthday
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Education != newUser.Education {
 		item := GetAccountItemByName("Education", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Education = oldUser.Education
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.IdCard != newUser.IdCard {
 		item := GetAccountItemByName("ID card", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.IdCard = oldUser.IdCard
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.IdCardType != newUser.IdCardType {
 		item := GetAccountItemByName("ID card type", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.IdCardType = oldUser.IdCardType
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	oldUserPropertiesJson, _ := json.Marshal(oldUser.Properties)
 	newUserPropertiesJson, _ := json.Marshal(newUser.Properties)
 	if string(oldUserPropertiesJson) != string(newUserPropertiesJson) {
 		item := GetAccountItemByName("Properties", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Properties = oldUser.Properties
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.PreferredMfaType != newUser.PreferredMfaType {
 		item := GetAccountItemByName("Multi-factor authentication", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.PreferredMfaType = oldUser.PreferredMfaType
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Groups == nil {
@ -390,7 +490,11 @@ func CheckPermissionForUpdateUser(oldUser, newUser *User, isAdmin bool, lang str
 	newUserGroupsJson, _ := json.Marshal(newUser.Groups)
 	if string(oldUserGroupsJson) != string(newUserGroupsJson) {
 		item := GetAccountItemByName("Groups", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Groups = oldUser.Groups
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Address == nil {
@ -404,65 +508,117 @@ func CheckPermissionForUpdateUser(oldUser, newUser *User, isAdmin bool, lang str
 	newUserAddressJson, _ := json.Marshal(newUser.Address)
 	if string(oldUserAddressJson) != string(newUserAddressJson) {
 		item := GetAccountItemByName("Address", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Address = oldUser.Address
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if newUser.FaceIds != nil {
 		item := GetAccountItemByName("Face ID", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.FaceIds = oldUser.FaceIds
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.IsAdmin != newUser.IsAdmin {
 		item := GetAccountItemByName("Is admin", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.IsAdmin = oldUser.IsAdmin
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.IsForbidden != newUser.IsForbidden {
 		item := GetAccountItemByName("Is forbidden", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.IsForbidden = oldUser.IsForbidden
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.IsDeleted != newUser.IsDeleted {
 		item := GetAccountItemByName("Is deleted", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.IsDeleted = oldUser.IsDeleted
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}
 	if oldUser.NeedUpdatePassword != newUser.NeedUpdatePassword {
 		item := GetAccountItemByName("Need update password", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.NeedUpdatePassword = oldUser.NeedUpdatePassword
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Balance != newUser.Balance {
 		item := GetAccountItemByName("Balance", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Balance = oldUser.Balance
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Score != newUser.Score {
 		item := GetAccountItemByName("Score", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Score = oldUser.Score
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Karma != newUser.Karma {
 		item := GetAccountItemByName("Karma", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Karma = oldUser.Karma
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Language != newUser.Language {
 		item := GetAccountItemByName("Language", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Language = oldUser.Language
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Ranking != newUser.Ranking {
 		item := GetAccountItemByName("Ranking", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Ranking = oldUser.Ranking
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Currency != newUser.Currency {
 		item := GetAccountItemByName("Currency", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Currency = oldUser.Currency
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	if oldUser.Hash != newUser.Hash {
 		item := GetAccountItemByName("Hash", organization)
-		itemsChanged = append(itemsChanged, item)
+		if item == nil {
+			newUser.Hash = oldUser.Hash
+		} else {
+			itemsChanged = append(itemsChanged, item)
+		}
 	}

 	for _, accountItem := range itemsChanged {