diff --git a/app/user.go b/app/user.go index 1f7674e5ff..0eeb7544ed 100644 --- a/app/user.go +++ b/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) { - if !prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() && user.Email != prev.Email { - return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_domain.app_error", nil, "", http.StatusBadRequest) + var newEmail string + if user.Email != prev.Email { + 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 prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() && user.Email != prev.Email { - return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_guest_domain.app_error", nil, "", http.StatusBadRequest) + if !CheckUserDomain(user, *a.Config().GuestAccountsSettings.RestrictCreationToDomains) { + if prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() { + 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 - // 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 { + if _, appErr := a.GetUserByEmail(user.Email); 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, - // 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 + // Don't set new eMail on user account if email verification is required, this will be done as a post-verification action + // to avoid users being able to set non-controlled eMails as their account email + + if *a.Config().EmailSettings.RequireEmailVerification { + newEmail = user.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 + } } } diff --git a/app/user_test.go b/app/user_test.go index 38125cec62..ac38a51511 100644 --- a/app/user_test.go +++ b/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 {