diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 7ed7d7ffd133e..8f81281672add 100644 --- a/models/fixtures/user.yml +++ b/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 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index cbf8ae87327ea..64ad726182e90 100644 --- a/models/migrations/migrations.go +++ b/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 diff --git a/models/migrations/v158.go b/models/migrations/v158.go new file mode 100644 index 0000000000000..f7f4045eef519 --- /dev/null +++ b/models/migrations/v158.go @@ -0,0 +1,76 @@ +// 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 + } + + 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 +} diff --git a/models/user.go b/models/user.go index 42f70b4666ffe..72e230c6b45e2 100644 --- a/models/user.go +++ b/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. diff --git a/models/user_mail.go b/models/user_mail.go index 60354e23ffb22..ea94ae51f5ebc 100644 --- a/models/user_mail.go +++ b/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) } diff --git a/models/user_test.go b/models/user_test.go index 7a6f5aa5122b7..2e4cda1611769 100644 --- a/models/user_test.go +++ b/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