Ensure user email is always in email address table

In order to avoid email duplication, ensure that once an user is created
or updated his new email address is added to the email address table.

Author: ivanvc

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,73 @@
+// 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 (
+	"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 {
+			if has, err := sess.Get(&EmailAddress{UID: user.ID, Email: user.Email}); err != nil {
+				return err
+			} else if has {
+				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: "[email protected]",
+	}
+	invalidUser := &User{
+		Name:  "GiteaBot2",
+		Email: "[email protected]",
+	}
+	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{
+		&User{
+			Name:  "GiteaBot",
+			Email: "[email protected]",
+		},
+		&User{
+			Name:  "GiteaBot2",
+			Email: "[email protected]",
+		},
+	}
+	for _, u := range users {
+		assert.NoError(t, CreateUser(u))
+	}
+
+	users[0].Email = "[email protected]"
+	assert.Error(t, AddEmailAddress(&EmailAddress{UID: users[0].ID, Email: users[0].Email}))
+	assert.Error(t, UpdateUserSetting(users[0]))
+	users[0].Email = "[email protected]"
+	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