app/user: fix update user error when an e-mail address already in use (#16269)

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

app/user.go

@@ -1251,35 +1251,37 @@ func (a *App) UpdateUser(user *model.User, sendNotifications bool) (*model.User,
 		}
 	}
 
-	if !CheckUserDomain(user, *a.Config().TeamSettings.RestrictCreationToDomains) {
+	var newEmail string
-		if !prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() && user.Email != prev.Email {
+	if user.Email != prev.Email {
-			return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_domain.app_error", nil, "", http.StatusBadRequest)
+		if !CheckUserDomain(user, *a.Config().TeamSettings.RestrictCreationToDomains) {
+			if !prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() {
+				return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_domain.app_error", nil, "", http.StatusBadRequest)
+			}
 		}
-	}
 
-	if !CheckUserDomain(user, *a.Config().GuestAccountsSettings.RestrictCreationToDomains) {
+		if !CheckUserDomain(user, *a.Config().GuestAccountsSettings.RestrictCreationToDomains) {
-		if prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() && user.Email != prev.Email {
+			if prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() {
-			return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_guest_domain.app_error", nil, "", http.StatusBadRequest)
+				return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_guest_domain.app_error", nil, "", http.StatusBadRequest)
+			}
 		}
-	}
 
-	// Don't set new eMail on user account if email verification is required, this will be done as a post-verification action
+		if _, appErr := a.GetUserByEmail(user.Email); appErr == nil {
-	// to avoid users being able to set non-controlled eMails as their account email
-	newEmail := ""
-	if *a.Config().EmailSettings.RequireEmailVerification && prev.Email != user.Email {
-		newEmail = user.Email
-
-		_, appErr := a.GetUserByEmail(newEmail)
-		if appErr == nil {
 			return nil, model.NewAppError("UpdateUser", "store.sql_user.update.email_taken.app_error", nil, "user_id="+user.Id, http.StatusBadRequest)
 		}
 
-		//  When a bot is created, prev.Email will be an autogenerated faked email,
+		// Don't set new eMail on user account if email verification is required, this will be done as a post-verification action
-		//  which will not match a CLI email input during bot to user conversions.
+		// to avoid users being able to set non-controlled eMails as their account email
-		//  To update a bot users email, do not set the email to the faked email
+
-		//  stored in prev.Email.  Allow using the email defined in the CLI
+		if *a.Config().EmailSettings.RequireEmailVerification {
-		if !user.IsBot {
+			newEmail = user.Email
-			user.Email = prev.Email
+
+			//  When a bot is created, prev.Email will be an autogenerated faked email,
+			//  which will not match a CLI email input during bot to user conversions.
+			//  To update a bot users email, do not set the email to the faked email
+			//  stored in prev.Email.  Allow using the email defined in the CLI
+			if !user.IsBot {
+				user.Email = prev.Email
+			}
 		}
 	}
 

app/user_test.go

@@ -461,7 +461,8 @@ func TestUpdateUserEmail(t *testing.T) {
 
 		user.Email = newEmail
 		user3, err := th.App.UpdateUser(user, false)
-		assert.NotNil(t, err)
+		require.NotNil(t, err)
+		assert.Equal(t, err.Id, "store.sql_user.update.email_taken.app_error")
 		assert.Nil(t, user3)
 	})
 
@@ -492,6 +493,21 @@ func TestUpdateUserEmail(t *testing.T) {
 		assert.Nil(t, err)
 		assert.Equal(t, botuser2.Email, newBotEmail)
 	})
+
+	t.Run("NoVerificationAlreadyUsedEmail", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.EmailSettings.RequireEmailVerification = false
+		})
+
+		user2 := th.CreateUser()
+		newEmail := user2.Email
+
+		user.Email = newEmail
+		user3, err := th.App.UpdateUser(user, false)
+		require.NotNil(t, err)
+		assert.Equal(t, err.Id, "store.sql_user.update.email_taken.app_error")
+		assert.Nil(t, user3)
+	})
 }
 
 func getUserFromDB(a *App, id string, t *testing.T) *model.User {