From 1d3095b71849d7084a26638af831e284d942cb43 Mon Sep 17 00:00:00 2001
From: Alexander Shimchik <alexsh42@gmail.com>
Date: Thu, 29 Sep 2022 15:36:29 +0300
Subject: [PATCH] Check if email is used when updating user (#21289)

Fix #21075
When updating user data should check if email is used by other users
---
 models/user/user.go      | 17 +++++++++++------
 models/user/user_test.go | 16 ++++++++++++++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/models/user/user.go b/models/user/user.go
index 32484a487..a3c10c249 100644
--- a/models/user/user.go
+++ b/models/user/user.go
@@ -893,14 +893,19 @@ func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...s
 		if err != nil {
 			return err
 		}
-		if !has {
-			// 1. Update old primary email
-			if _, err = e.Where("uid=? AND is_primary=?", u.ID, true).Cols("is_primary").Update(&EmailAddress{
-				IsPrimary: false,
-			}); err != nil {
-				return err
+		if has && emailAddress.UID != u.ID {
+			return ErrEmailAlreadyUsed{
+				Email: u.Email,
 			}
+		}
+		// 1. Update old primary email
+		if _, err = e.Where("uid=? AND is_primary=?", u.ID, true).Cols("is_primary").Update(&EmailAddress{
+			IsPrimary: false,
+		}); err != nil {
+			return err
+		}
 
+		if !has {
 			emailAddress.Email = u.Email
 			emailAddress.UID = u.ID
 			emailAddress.IsActivated = true
diff --git a/models/user/user_test.go b/models/user/user_test.go
index 848c978a9..678d6c186 100644
--- a/models/user/user_test.go
+++ b/models/user/user_test.go
@@ -302,10 +302,26 @@ func TestUpdateUser(t *testing.T) {
 	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
 	assert.True(t, user.KeepActivityPrivate)
 
+	newEmail := "new_" + user.Email
+	user.Email = newEmail
+	assert.NoError(t, user_model.UpdateUser(db.DefaultContext, user, true))
+	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	assert.Equal(t, newEmail, user.Email)
+
 	user.Email = "no mail@mail.org"
 	assert.Error(t, user_model.UpdateUser(db.DefaultContext, user, true))
 }
 
+func TestUpdateUserEmailAlreadyUsed(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
+
+	user2.Email = user3.Email
+	err := user_model.UpdateUser(db.DefaultContext, user2, true)
+	assert.True(t, user_model.IsErrEmailAlreadyUsed(err))
+}
+
 func TestNewUserRedirect(t *testing.T) {
 	// redirect to a completely new name
 	assert.NoError(t, unittest.PrepareTestDatabase())