Ensure user email is always in email address table

models/fixtures/user.yml

@@ -186,14 +186,14 @@
   lower_name: user11
   name: user11
   full_name: User Eleven
-  email: user11@example.com
+  email: user011@example.com
   passwd_hash_algo: argon2
   passwd: a3d5fcd92bae586c2e3dbe72daea7a0d27833a8d0227aa1704f4bbd775c1f3b03535b76dd93b0d4d8d22a519dca47df1547b # password
   type: 0 # individual
   salt: ZogKvWdyEx
   is_admin: false
   avatar: avatar11
-  avatar_email: user11@example.com
+  avatar_email: user011@example.com
   num_repos: 1
   is_active: true
 
@@ -202,14 +202,14 @@
   lower_name: user12
   name: user12
   full_name: User 12
-  email: user12@example.com
+  email: user012@example.com
   passwd_hash_algo: argon2
   passwd: a3d5fcd92bae586c2e3dbe72daea7a0d27833a8d0227aa1704f4bbd775c1f3b03535b76dd93b0d4d8d22a519dca47df1547b # password
   type: 0 # individual
   salt: ZogKvWdyEx
   is_admin: false
   avatar: avatar12
-  avatar_email: user12@example.com
+  avatar_email: user012@example.com
   num_repos: 1
   is_active: true
 
@@ -350,14 +350,14 @@
   lower_name: user21
   name: user21
   full_name: User 21
-  email: user21@example.com
+  email: user021@example.com
   passwd_hash_algo: argon2
   passwd: a3d5fcd92bae586c2e3dbe72daea7a0d27833a8d0227aa1704f4bbd775c1f3b03535b76dd93b0d4d8d22a519dca47df1547b # password
   type: 0 # individual
   salt: ZogKvWdyEx
   is_admin: false
   avatar: avatar21
-  avatar_email: user21@example.com
+  avatar_email: user021@example.com
   num_repos: 2
   is_active: true
 

models/migrations/migrations.go

@@ -250,6 +250,8 @@ var migrations = []Migration{
 	NewMigration("fix publisher ID for tag releases", fixPublisherIDforTagReleases),
 	// v157 -> v158
 	NewMigration("ensure repo topics are up-to-date", fixRepoTopics),
+	// v158 -> v159
+	NewMigration("Ensure user primary email address is in email address table", addUserPrimaryEmailToUserMails),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v158.go

@@ -0,0 +1,80 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"code.gitea.io/gitea/modules/log"
+
+	"xorm.io/xorm"
+)
+
+func addUserPrimaryEmailToUserMails(x *xorm.Engine) error {
+	type User struct {
+		ID       int64  `xorm:"pk autoincr"`
+		Email    string `xorm:"NOT NULL"`
+		IsActive bool   `xorm:"INDEX"`
+	}
+	type EmailAddress struct {
+		ID          int64  `xorm:"pk autoincr"`
+		UID         int64  `xorm:"INDEX NOT NULL"`
+		Email       string `xorm:"UNIQUE NOT NULL"`
+		IsActivated bool
+	}
+
+	if err := x.Sync2(new(User), new(EmailAddress)); err != nil {
+		return err
+	}
+
+	updateUsers := func(users []*User) error {
+		sess := x.NewSession()
+		defer sess.Close()
+		if err := sess.Begin(); err != nil {
+			return err
+		}
+		for _, user := range users {
+			email := new(EmailAddress)
+			if isExist, err := sess.Where("email=?", user.Email).Get(email); err != nil {
+				return err
+			} else if isExist {
+				if email.UID != user.ID {
+					log.Warn("Email (%s) from user with ID %d is taken by user with ID %d", email.Email, user.ID, email.UID)
+					log.Warn("Skipping to avoid conflicts, should be manually fixed")
+				}
+				continue
+			}
+			email = &EmailAddress{
+				Email:       user.Email,
+				UID:         user.ID,
+				IsActivated: user.IsActive,
+			}
+			if _, err := sess.Insert(email); err != nil {
+				return err
+			}
+		}
+
+		return sess.Commit()
+	}
+
+	var start = 0
+	var batchSize = 100
+	for {
+		var users = make([]*User, 0, batchSize)
+		if err := x.Limit(batchSize, start).Find(&users); err != nil {
+			return err
+		}
+
+		if err := updateUsers(users); err != nil {
+			return err
+		}
+
+		start += len(users)
+
+		if len(users) < batchSize {
+			break
+		}
+	}
+
+	return nil
+}

models/user.go

@@ -834,6 +834,9 @@ func CreateUser(u *User) (err error) {
 	if _, err = sess.Insert(u); err != nil {
 		return err
 	}
+	if err = addEmailAddress(sess, &EmailAddress{UID: u.ID, Email: u.Email}); err != nil {
+		return err
+	}
 
 	return sess.Commit()
 }
@@ -937,16 +940,24 @@ func ChangeUserName(u *User, newUserName string) (err error) {
 // checkDupEmail checks whether there are the same email with the user
 func checkDupEmail(e Engine, u *User) error {
 	u.Email = strings.ToLower(u.Email)
-	has, err := e.
+	if has, err := e.
 		Where("id!=?", u.ID).
+		And("email=?", u.Email).
 		And("type=?", u.Type).
+		Get(new(User)); err != nil {
+		return err
+	} else if has {
+		return ErrEmailAlreadyUsed{u.Email}
+	}
+	if has, err := e.
+		Where("uid!=?", u.ID).
 		And("email=?", u.Email).
-		Get(new(User))
-	if err != nil {
+		Get(new(EmailAddress)); err != nil {
 		return err
 	} else if has {
 		return ErrEmailAlreadyUsed{u.Email}
 	}
+
 	return nil
 }
 
@@ -972,12 +983,34 @@ func updateUserCols(e Engine, u *User, cols ...string) error {
 
 // UpdateUserSetting updates user's settings.
 func UpdateUserSetting(u *User) error {
+	isEmailFound := false
+	sess := x.NewSession()
+
 	if !u.IsOrganization() {
-		if err := checkDupEmail(x, u); err != nil {
+		if err := checkDupEmail(sess, u); err != nil {
 			return err
 		}
+		emails, err := getEmailAddresses(sess, u.ID)
+		if err != nil {
+			return err
+		}
+		for _, email := range emails {
+			if email.Email == u.Email {
+				isEmailFound = true
+				break
+			}
+		}
 	}
-	return updateUser(x, u)
+
+	if err := updateUser(sess, u); err != nil {
+		return err
+	}
+	if !isEmailFound {
+		if err := addEmailAddress(sess, &EmailAddress{UID: u.ID, Email: u.Email}); err != nil {
+			return err
+		}
+	}
+	return sess.Commit()
 }
 
 // deleteBeans deletes all given beans, beans should contain delete conditions.

models/user_mail.go

@@ -34,8 +34,12 @@ type EmailAddress struct {
 
 // GetEmailAddresses returns all email addresses belongs to given user.
 func GetEmailAddresses(uid int64) ([]*EmailAddress, error) {
+	return getEmailAddresses(x, uid)
+}
+
+func getEmailAddresses(e Engine, uid int64) ([]*EmailAddress, error) {
 	emails := make([]*EmailAddress, 0, 5)
-	if err := x.
+	if err := e.
 		Where("uid=?", uid).
 		Find(&emails); err != nil {
 		return nil, err
@@ -136,19 +140,21 @@ func IsEmailUsed(email string) (bool, error) {
 
 func addEmailAddress(e Engine, email *EmailAddress) error {
 	email.Email = strings.ToLower(strings.TrimSpace(email.Email))
-	used, err := isEmailUsed(e, email.Email)
+	_, err := e.Insert(email)
+	return err
+}
+
+// AddEmailAddress adds an email address to given user.
+func AddEmailAddress(email *EmailAddress) error {
+	email.Email = strings.ToLower(strings.TrimSpace(email.Email))
+
+	used, err := isEmailUsed(x, email.Email)
 	if err != nil {
 		return err
 	} else if used {
 		return ErrEmailAlreadyUsed{email.Email}
 	}
 
-	_, err = e.Insert(email)
-	return err
-}
-
-// AddEmailAddress adds an email address to given user.
-func AddEmailAddress(email *EmailAddress) error {
 	return addEmailAddress(x, email)
 }
 

models/user_test.go

@@ -329,6 +329,64 @@ func TestCreateUser(t *testing.T) {
 	assert.NoError(t, DeleteUser(user))
 }
 
+func TestCreateUserInsertsEmailAddressRecord(t *testing.T) {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	validUser := &User{
+		Name:  "GiteaBot",
+		Email: "GiteaBot@gitea.io",
+	}
+	invalidUser := &User{
+		Name:  "GiteaBot2",
+		Email: "GiteaBot@gitea.io",
+	}
+	assert.NoError(t, CreateUser(validUser))
+	email := new(EmailAddress)
+	found, err := sess.Where("email=?", validUser.Email).Get(email)
+	assert.NoError(t, err)
+	assert.True(t, found)
+	assert.EqualValues(t, email.UID, validUser.ID)
+	assert.Error(t, CreateUser(invalidUser))
+
+	assert.NoError(t, DeleteUser(validUser))
+}
+
+func TestUpdateUser(t *testing.T) {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	users := []*User{
+		{
+			Name:  "GiteaBot",
+			Email: "GiteaBot@gitea.io",
+		},
+		{
+			Name:  "GiteaBot2",
+			Email: "GiteaBot2@gitea.io",
+		},
+	}
+	for _, u := range users {
+		assert.NoError(t, CreateUser(u))
+	}
+
+	users[0].Email = "GiteaBot2@gitea.io"
+	assert.Error(t, AddEmailAddress(&EmailAddress{UID: users[0].ID, Email: users[0].Email}))
+	assert.Error(t, UpdateUserSetting(users[0]))
+	users[0].Email = "GiteaBot3@gitea.io"
+	assert.NoError(t, UpdateUserSetting(users[0]))
+
+	email := new(EmailAddress)
+	found, err := sess.Where("email=?", users[0].Email).Get(email)
+	assert.NoError(t, err)
+	assert.True(t, found)
+	assert.EqualValues(t, email.UID, users[0].ID)
+
+	for _, u := range users {
+		assert.NoError(t, DeleteUser(u))
+	}
+}
+
 func TestCreateUser_Issue5882(t *testing.T) {
 
 	// Init settings