lunny/gitea
Archived
1
1
Fork 1
Code Issues 1.7k Pull Requests 154 Projects Releases 128 Wiki Activity

Always store primary email address into email_address table and also the state #15956

Merged
lunny merged 18 commits from lunny/email_address into main 2021-06-08 03:52:51 +00:00
Conversation 0 Commits 18 Files Changed 11 +594 -237
11 changed files with 594 additions and 237 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
integrations/api_gpg_keys_test.go
View File

@ -36,7 +36,6 @@ func TestGPGKeys(t *testing.T) {
}
for _, tc := range tt {
//Basic test on result code
t.Run(tc.name, func(t *testing.T) {
t.Run("ViewOwnGPGKeys", func(t *testing.T) {
@ -87,9 +86,9 @@ func TestGPGKeys(t *testing.T) {
assert.Empty(t, subKey.Emails)
primaryKey2 := keys[1] //Primary key 2
assert.EqualValues(t, "FABF39739FE1E927", primaryKey2.KeyID)
assert.EqualValues(t, "3CEF46EF40BEFC3E", primaryKey2.KeyID)
assert.Len(t, primaryKey2.Emails, 1)
assert.EqualValues(t, "user21@example.com", primaryKey2.Emails[0].Email)
assert.EqualValues(t, "user2-2@example.com", primaryKey2.Emails[0].Email)
assert.False(t, primaryKey2.Emails[0].Verified)
var key api.GPGKey
@ -110,9 +109,9 @@ func TestGPGKeys(t *testing.T) {
req = NewRequest(t, "GET", "/api/v1/user/gpg_keys/"+strconv.FormatInt(primaryKey2.ID, 10)+"?token="+token) //Primary key 2
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &key)
assert.EqualValues(t, "FABF39739FE1E927", key.KeyID)
assert.EqualValues(t, "3CEF46EF40BEFC3E", key.KeyID)
assert.Len(t, key.Emails, 1)
assert.EqualValues(t, "user21@example.com", key.Emails[0].Email)
assert.EqualValues(t, "user2-2@example.com", key.Emails[0].Email)
assert.False(t, key.Emails[0].Verified)
})
@ -231,35 +230,46 @@ uy6MA3VSB99SK9ducGmE1Jv8mcziREroz2TEGr0zPs6h
}
func testCreateValidSecondaryEmailGPGKey(t *testing.T, makeRequest makeRequestFunc, token string, expected int) {
//User2 <user21@example.com> //secondary and not activated
//User2 <user2-2@example.com> //secondary and not activated
testCreateGPGKey(t, makeRequest, token, expected, `-----BEGIN PGP PUBLIC KEY BLOCK-----
mQENBFmGWN4BCAC18V4tVGO65VLCV7p14FuXJlUtZ5CuYMvgEkcOqrvRaBSW9ao4
PGESOhJpfWpnW3QgJniYndLzPpsmdHEclEER6aZjiNgReWPOjHD5tykWocZAJqXD
eY1ym59gvVMLcfbV2yQsyR2hbJlc+dJsl16tigSEe3nwxZSw2IsW92pgEzT9JNUr
Q+mC8dw4dqY0tYmFazYUGNxufUc/twgQT/Or1aNs0az5Q6Jft4rrTRsh/S7We0VB
COKGkdcQyYgAls7HJBuPjQRi6DM9VhgBSHLAgSLyaUcZvhZBJr8Qe/q4PP3/kYDJ
wm4RMnjOLz2pFZPgtRqgcAwpmFtLrACbEB3JABEBAAG0GlVzZXIyIDx1c2VyMjFA
ZXhhbXBsZS5jb20+iQFUBBMBCAA+FiEEPOLHOjPSO42DWM57+r85c5/h6ScFAlmG
WN4CGwMFCQPCZwAFCwkIBwIGFQgJCgsCBBYCAwECHgECF4AACgkQ+r85c5/h6Sfx
Lgf/dq64NBV8+X9an3seaLxePRviva48e4K67/wV/JxtXNO5Z/DhMGz5kHXCsG9D
CXuWYO8ehlTjEnMZ6qqdDnY+H6bQsb2OS5oPn4RwpPXslAjEKtojPAr0dDsMS2DB
dUuIm1AoOnewOVO0OFRf1EqX1bivxnN0FVMcO0m8AczfnKDaGb0y/qg/Y9JAsKqp
j5pZNMWUkntRtGySeJ4CVJMmkVKJAHsa1Qj6MKdFeid4h4y94cBJ4ZdyBxNdpQOx
ydf0doicovfeqGNO4oWzsGP4RBK2CqGPCUT+EFl20jPvMkKwOjxgqc8p0z3b2UT9
+9bnmCGHgF/fW1HJ3iKmfFPqnLkBDQRZhljeAQgA5AirU/NJGgm19ZJYFOiHftjS
azbrPxGeD3cSqmvDPIMc1DNZGfQV5D4EVumnVbQBtL6xHFoGKz9KisUMbe4a/X2J
S8JmIphQWG0vMJX1DaZIzr2gT71MnPD7JMGsSUCh5dIKpTNTZX4w+oGPGOu0/UlL
x0448AryKwp30J2p6D4GeI0nb03n35S2lTOpnHDn1wj7Jl/8LS2fdFOdNaNHXSZe
twdSwJKhyBEiScgeHBDyKqo8zWkYoSb9eA2HiYlbVaiNtp24KP1mIEpiUdrRjWno
zauYSZGHZlOFMgF4dKWuetPiuH9m7UYZGKyMLfQ9vYFb+xcPh2bLCQHJ1OEmMQAR
AQABiQE8BBgBCAAmFiEEPOLHOjPSO42DWM57+r85c5/h6ScFAlmGWN4CGwwFCQPC
ZwAACgkQ+r85c5/h6Sfjfwf+O4WEjRdvPJLxNy7mfAGoAqDMHIwyH/tVzYgyVhnG
h/+cfRxJbGc3rpjYdr8dmvghzjEAout8uibPWaIqs63RCAPGPqgWLfxNO5c8+y8V
LZMVOTV26l2olkkdBWAuhLqKTNh6TiQva03yhOgHWj4XDvFfxICWPFXVd6t5ELpD
iApGu1OAj8JfhmzbG03Yzx+Ku7bWDxMonx3V/IDEu5LS5zrboHYDKCA53bXXghoi
Aceqql+PKrDwEjoY4bptwMHLmcjGjdCQ//Qx1neho7nZcS7xjTucY8gQuulwCyXF
y6wM+wMz8dunIG9gw4+Re6c4Rz9tX1kzxLrU7Pl21tMqfg==
=0N/9
mQGNBGC2K2cBDAC1+Xgk+8UfhASVgRngQi4rnQ8k0t+bWsBz4Czd26+cxVDRwlTT
8PALdrbrY/e9iXjcVcZ8Npo4UYe7/LfnL57dc7tgbenRGYYrWyVoNNv58BVw4xCY
RmgvdHWIIPGuz3aME0smHxbJ2KewYTqjTPuVKF/wrHTwCpVWdjYKC5KDo3yx0mro
xf9vOJOnkWNMiEw7TiZfkrbUqxyA53BVsSNKRX5C3b4FJcVT7eiAq7sDAaFxjEHy
ahZslmvg7XZxWzSVzxDNesR7f4xuop8HBjzaluJoVuwiyWculTvz1b6hyHVQr+ad
h8JGjj1tySI65OTFsTuptsfHXjtjl/NR4P6BXkf+FVwweaTQaEzpHkv0m9b9pY43
CY/8XtS4uNPermiLG/Z0BB1eOCdoOQVHpjOa55IXQWhxXB6NZVyowiUbrR7jLDQy
5JP7D1HmErTR8JRm3VDqGbSaCgugRgFX+lb/fpgFp9k02OeK+JQudolZOt1mVk+T
C4xmEWxfiH15/JMAEQEAAbQbdXNlcjIgPHVzZXIyLTJAZXhhbXBsZS5jb20+iQHU
BBMBCAA+FiEEB/Y4DM3Ba2H9iXmlPO9G70C+/D4FAmC2K2cCGwMFCQPCZwAFCwkI
BwIGFQoJCAsCBBYCAwECHgECF4AACgkQPO9G70C+/D59/Av/XZIhCH4X2FpxCO3d
oCa+sbYkBL5xeUoPfAx5ThXzqL/tllO88TKTMEGZF3k5pocXWH0xmhqlvDTcdb0i
W3O0CN8FLmuotU51c0JC1mt9zwJP9PeJNyqxrMm01Yzj55z/Dz3QHSTlDjrWTWjn
YBqDf2HfdM177oydfSYmevZni1aDmBalWpFPRvqISCO7uFnvg1hJQ5mD/0qie663
QJ8LAAANg32H9DyPnYi9wU62WX0DMUVTjKctT3cnYCbirjjJ7ZlCCm+cf61CRX1B
E1Ng/Ef3ZcUfXWitZSjfET/pKEMSNjsQawFpZ/LPCBl+UPHzaTPAASeGJvcbZ3py
wZQLQc1MCu2hmMBQ8zHQTdS2Pp0RISxCQLYvVQL6DrcJDNiSqn9p9RQt5c5r5Pjx
80BIPcjj3glOVP7PYE2azQAkt6reEjhimwCfjeDpiPnkBTY7Av2jCcUFhhemDY/j
TRXK1paLphhJ36zC22SeHGxNNakjjuUakqB85DEUeoWuVm6ouQGNBGC2K2cBDADx
G2rIAgMjdPtofhkEZXwv6zdNwmYOlIIM+59bam9Ep/vFq8F5f+xldevm5dvM8SeR
pNwDGSOUf5OKBWBdsJFhlYBl7+EcKd/Tent/XS6JoA9ffF33b+r04L543+ykiKON
WYeYi0F4WwYTIQgqZHJze1sPVkYGR5F0bL8PAcLuwd5dzZVi/q2HakrGdg29N8oY
b/XnoR7FflPrNYdzO6hawi5Inx7KS7aWa0ZkARb0F4HSct+/m6nAZVsoJINLudyQ
ut2NWeU8rWIm1hqyIxQFvuQJy46umq++10J/sWA98bkg41Rx+72+eP7DM5v8IgUp
clJsfljRXIBWbmRAVZvtNI7PX9fwMMhf4M7wHO7G2WV39o1exKps5xFFcn8PUQiX
jCSR81M145CgCdmLUR1y0pdkN/WIqjXBhkPIvO2dxEcodMNHb1aUUuUOnww6+xIP
8rGVw+a2DUiALc8Qr5RP21AYKRctfiwhSQh2KODveMtyLI3U9C/eLRPp+QM3XB8A
EQEAAYkBvAQYAQgAJhYhBAf2OAzNwWth/Yl5pTzvRu9Avvw+BQJgtitnAhsMBQkD
wmcAAAoJEDzvRu9Avvw+3FcMAJBwupyJ4zwQFxTJ5BkDlusG3U2FXEf3bDrXhvNd
qi8eS8Vo/vRiH/w/my5JFpz1o2tJToryF71D+uF5DTItalKquhsQ9reAEmXggqOh
9Jd9mWJIEEWcRORiLNDKENKvE8bouw4U4hRaSF0IaGzAe5mO+oOvwal8L97wFxrZ
4leM1GzkopiuNfbkkBBw2KJcMjYBHzzXSCALnVwhjbgkBEWPIg38APT3cr9KfnMM
q8+tvsGLj4piAl3Lww7+GhSsDOUXH8btR41BSAQDrbO5q6oi/h4nuxoNmQIDW/Ug
s+dd5hnY2FtHRjb4FCR9kAjdTE6stc8wzohWfbg1N+12TTA2ylByAumICVXixavH
RJ7l0OiWJk388qw9mqh3k8HcBxL7OfDlFC9oPmCS0iYiIwW/Yc80kBhoxcvl/Xa7
mIMMn8taHIaQO7v9ln2EVQYTzbNCmwTw9ovTM0j/Pbkg2EftfP1TCoxQHvBnsCED
6qgtsUdi5eviONRkBgeZtN3oxA==
=MgDv
-----END PGP PUBLIC KEY BLOCK-----`)
}

10
integrations/api_user_email_test.go
View File

@ -33,7 +33,7 @@ func TestAPIListEmails(t *testing.T) {
Primary: true,
},
{
Email: "user21@example.com",
Email: "user2-2@example.com",
Verified: false,
Primary: false,
},
@ -55,7 +55,7 @@ func TestAPIAddEmail(t *testing.T) {
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
opts = api.CreateEmailOption{
Emails: []string{"user22@example.com"},
Emails: []string{"user2-3@example.com"},
}
req = NewRequestWithJSON(t, "POST", "/api/v1/user/emails?token="+token, &opts)
resp := session.MakeRequest(t, req, http.StatusCreated)
@ -64,7 +64,7 @@ func TestAPIAddEmail(t *testing.T) {
DecodeJSON(t, resp, &emails)
assert.EqualValues(t, []*api.Email{
{
Email: "user22@example.com",
Email: "user2-3@example.com",
Verified: true,
Primary: false,
},
@ -79,13 +79,13 @@ func TestAPIDeleteEmail(t *testing.T) {
token := getTokenForLoggedInUser(t, session)
opts := api.DeleteEmailOption{
Emails: []string{"user22@example.com"},
Emails: []string{"user2-3@example.com"},
}
req := NewRequestWithJSON(t, "DELETE", "/api/v1/user/emails?token="+token, &opts)
session.MakeRequest(t, req, http.StatusNotFound)
opts = api.DeleteEmailOption{
Emails: []string{"user21@example.com"},
Emails: []string{"user2-2@example.com"},
}
req = NewRequestWithJSON(t, "DELETE", "/api/v1/user/emails?token="+token, &opts)
session.MakeRequest(t, req, http.StatusNoContent)

15
models/error.go
View File

@ -237,6 +237,21 @@ func (err ErrEmailAddressNotExist) Error() string {
return fmt.Sprintf("Email address does not exist [email: %s]", err.Email)
}
// ErrPrimaryEmailCannotDelete primary email address cannot be deleted
type ErrPrimaryEmailCannotDelete struct {
Email string
}
// IsErrPrimaryEmailCannotDelete checks if an error is an ErrPrimaryEmailCannotDelete
func IsErrPrimaryEmailCannotDelete(err error) bool {
_, ok := err.(ErrPrimaryEmailCannotDelete)
return ok
}
func (err ErrPrimaryEmailCannotDelete) Error() string {
return fmt.Sprintf("Primary email address cannot be deleted [email: %s]", err.Email)
}
// ErrOpenIDAlreadyUsed represents a "OpenIDAlreadyUsed" kind of error.
type ErrOpenIDAlreadyUsed struct {
OpenID string

256
models/fixtures/email_address.yml
View File

@ -1,35 +1,279 @@
-
id: 1
uid: 1
uid: 11
email: user11@example.com
lower_email: user11@example.com
is_activated: false
is_primary: true
-
id: 2
uid: 1
uid: 12
email: user12@example.com
is_activated: false
lower_email: user12@example.com
is_activated: true
is_primary: true
-
id: 3
uid: 2
email: user2@example.com
lower_email: user2@example.com
is_activated: true
is_primary: true
-
id: 4
uid: 2
uid: 21
email: user21@example.com
is_activated: false
lower_email: user21@example.com
is_activated: true
is_primary: true
-
id: 5
uid: 9999999
email: user9999999@example.com
lower_email: user9999999@example.com
is_activated: true
is_primary: false
-
id: 6
uid: 10
email: user101@example.com
email: user10@example.com
lower_email: user10@example.com
is_activated: true
is_primary: true
-
id: 7
uid: 10
email: user101@example.com
lower_email: user101@example.com
is_activated: true
is_primary: false
-
id: 8
uid: 9
email: user9@example.com
lower_email: user9@example.com
is_activated: false
is_primary: true
-
id: 9
uid: 1
email: user1@example.com
lower_email: user1@example.com
is_activated: true
is_primary: true
-
id: 10
uid: 3
email: user3@example.com
lower_email: user3@example.com
is_activated: true
is_primary: true
-
id: 11
uid: 4
email: user4@example.com
lower_email: user4@example.com
is_activated: true
is_primary: true
-
id: 12
uid: 5
email: user5@example.com
lower_email: user5@example.com
is_activated: true
is_primary: true
-
id: 13
uid: 6
email: user6@example.com
lower_email: user6@example.com
is_activated: true
is_primary: true
-
id: 14
uid: 7
email: user7@example.com
lower_email: user7@example.com
is_activated: true
is_primary: true
-
id: 15
uid: 8
email: user8@example.com
lower_email: user8@example.com
is_activated: true
is_primary: true
-
id: 16
uid: 13
email: user13@example.com
lower_email: user13@example.com
is_activated: true
is_primary: true
-
id: 17
uid: 14
email: user14@example.com
lower_email: user14@example.com
is_activated: true
is_primary: true
-
id: 18
uid: 15
email: user15@example.com
lower_email: user15@example.com
is_activated: true
is_primary: true
-
id: 19
uid: 16
email: user16@example.com
lower_email: user16@example.com
is_activated: true
is_primary: true
-
id: 20
uid: 17
email: user17@example.com
lower_email: user17@example.com
is_activated: true
is_primary: true
-
id: 21
uid: 18
email: user18@example.com
lower_email: user18@example.com
is_activated: true
is_primary: true
-
id: 22
uid: 19
email: user19@example.com
lower_email: user19@example.com
is_activated: true
is_primary: true
-
id: 23
uid: 20
email: user20@example.com
lower_email: user20@example.com
is_activated: true
is_primary: true
-
id: 24
uid: 22
email: limited_org@example.com
lower_email: limited_org@example.com
is_activated: true
is_primary: true
-
id: 25
uid: 23
email: privated_org@example.com
lower_email: privated_org@example.com
is_activated: true
is_primary: true
-
id: 26
uid: 24
email: user24@example.com
lower_email: user24@example.com
is_activated: true
is_primary: true
-
id: 27
uid: 25
email: org25@example.com
lower_email: org25@example.com
is_activated: true
is_primary: true
-
id: 28
uid: 26
email: org26@example.com
lower_email: org26@example.com
is_activated: true
is_primary: true
-
id: 29
uid: 27
email: user27@example.com
lower_email: user27@example.com
is_activated: true
is_primary: true
-
id: 30
uid: 28
email: user28@example.com
lower_email: user28@example.com
is_activated: true
is_primary: true
-
id: 31
uid: 29
email: user29@example.com
lower_email: user29@example.com
is_activated: true
is_primary: true
-
id: 32
uid: 30
email: user30@example.com
lower_email: user30@example.com
is_activated: true
is_primary: true
-
id: 33
uid: 1
email: user1-2@example.com
lower_email: user1-2@example.com
is_activated: true
is_primary: false
-
id: 34
uid: 1
email: user1-3@example.com
lower_email: user1-3@example.com
is_activated: true
is_primary: false
-
id: 35
uid: 2
email: user2-2@example.com
lower_email: user2-2@example.com
is_activated: false
is_primary: false

2
models/gpg_key.go
View File

@ -301,7 +301,7 @@ func parseGPGKey(ownerID int64, e *openpgp.Entity) (*GPGKey, error) {
}
email := strings.ToLower(strings.TrimSpace(ident.UserId.Email))
for _, e := range userEmails {
if e.Email == email {
if e.LowerEmail == email {
emails = append(emails, e)
break
}

2
models/migrations/migrations.go
View File

@ -311,6 +311,8 @@ var migrations = []Migration{
NewMigration("Convert avatar url to text", convertAvatarURLToText),
// v180 -> v181
NewMigration("Delete credentials from past migrations", deleteMigrationCredentials),
// v181 -> v182
NewMigration("Always save primary email on email address table", addPrimaryEmail2EmailAddress),
}
// GetCurrentDBVersion returns the current db version

92
models/migrations/v181.go Normal file
View File

@ -0,0 +1,92 @@
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package migrations
import (
"strings"
"xorm.io/xorm"
)
func addPrimaryEmail2EmailAddress(x *xorm.Engine) (err error) {
zeripath commented 2021-06-04 16:43:56 +00:00
Review
Copy Link

We need a testcase for this.

We need a testcase for this.
lunny commented 2021-06-05 02:18:42 +00:00
Review
Copy Link

Added

Added
type User struct {
ID int64 `xorm:"pk autoincr"`
Email string `xorm:"NOT NULL"`
IsActive bool `xorm:"INDEX"` // Activate primary email
}
type EmailAddress1 struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"INDEX NOT NULL"`
Email string `xorm:"UNIQUE NOT NULL"`
LowerEmail string
IsActivated bool
IsPrimary bool `xorm:"DEFAULT(false) NOT NULL"`
}
// Add lower_email and is_primary columns
if err = x.Table("email_address").Sync2(new(EmailAddress1)); err != nil {
return
}
if _, err = x.Exec("UPDATE email_address SET lower_email=LOWER(email), is_primary=?", false); err != nil {
return
}
type EmailAddress struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"INDEX NOT NULL"`
Email string `xorm:"UNIQUE NOT NULL"`
LowerEmail string `xorm:"UNIQUE NOT NULL"`
IsActivated bool
IsPrimary bool `xorm:"DEFAULT(false) NOT NULL"`
}
// change lower_email as unique
if err = x.Sync2(new(EmailAddress)); err != nil {
return
}
zeripath commented 2021-06-04 16:43:23 +00:00
Review
Copy Link

I'm not sure that the unique constraint will definitely get set here on every db.

I'm not sure that the unique constraint will definitely get set here on every db.
lunny commented 2021-06-05 02:00:09 +00:00
Review
Copy Link

It's a work of xorm to translate it to database's syntax.

It's a work of `xorm` to translate it to database's syntax.
zeripath commented 2021-06-05 10:24:07 +00:00
Review
Copy Link

cool. Just checked!

cool. Just checked!
sess := x.NewSession()
defer sess.Close()
const batchSize = 100
for start := 0; ; start += batchSize {
users := make([]*User, 0, batchSize)
if err = sess.Limit(batchSize, start).Find(&users); err != nil {
return
}
if len(users) == 0 {
break
}
for _, user := range users {
var exist bool
exist, err = sess.Where("email=?", user.Email).Table("email_address").Exist()
if err != nil {
return
}
if !exist {
if _, err = sess.Insert(&EmailAddress{
UID: user.ID,
Email: user.Email,
LowerEmail: strings.ToLower(user.Email),
IsActivated: user.IsActive,
IsPrimary: true,
}); err != nil {
return
}
} else {
if _, err = sess.Where("email=?", user.Email).Cols("is_primary").Update(&EmailAddress{
IsPrimary: true,
}); err != nil {
return
}
}
}
}
return nil
}

54
models/migrations/v181_test.go Normal file
View File

@ -0,0 +1,54 @@
// Copyright 2021 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 (
"strings"
"testing"
"github.com/stretchr/testify/assert"
)
func Test_addPrimaryEmail2EmailAddress(t *testing.T) {
type User struct {
ID int64
Email string
IsActive bool
}
// Prepare and load the testing database
x, deferable := prepareTestEnv(t, 0, new(User))
if x == nil || t.Failed() {
defer deferable()
return
}
defer deferable()
err := addPrimaryEmail2EmailAddress(x)
assert.NoError(t, err)
type EmailAddress struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"INDEX NOT NULL"`
Email string `xorm:"UNIQUE NOT NULL"`
LowerEmail string `xorm:"UNIQUE NOT NULL"`
IsActivated bool
IsPrimary bool `xorm:"DEFAULT(false) NOT NULL"`
}
var users = make([]User, 0, 20)
err = x.Find(&users)
assert.NoError(t, err)
for _, user := range users {
var emailAddress EmailAddress
has, err := x.Where("lower_email=?", strings.ToLower(user.Email)).Get(&emailAddress)
assert.NoError(t, err)
assert.True(t, has)
assert.True(t, emailAddress.IsPrimary)
assert.EqualValues(t, user.IsActive, emailAddress.IsActivated)
assert.EqualValues(t, user.ID, emailAddress.UID)
}
}

23
models/user.go
View File

@ -74,9 +74,6 @@ const (
)
var (
// ErrEmailNotExist e-mail does not exist error
ErrEmailNotExist = errors.New("E-mail does not exist")
// ErrEmailNotActivated e-mail address has not been activated error
ErrEmailNotActivated = errors.New("E-mail address has not been activated")
@ -876,15 +873,6 @@ func CreateUser(u *User) (err error) {
}
u.Email = strings.ToLower(u.Email)
isExist, err = sess.
Where("email=?", u.Email).
Get(new(User))
if err != nil {
return err
} else if isExist {
return ErrEmailAlreadyUsed{u.Email}
}
if err = ValidateEmail(u.Email); err != nil {
return err
}
@ -915,6 +903,17 @@ func CreateUser(u *User) (err error) {
return err
}
// insert email address
if _, err := sess.Insert(&EmailAddress{
UID: u.ID,
Email: u.Email,
LowerEmail: strings.ToLower(u.Email),
IsActivated: u.IsActive,
IsPrimary: true,
}); err != nil {
return err
}
return sess.Commit()
}

250
models/user_mail.go
View File

@ -17,14 +17,22 @@ import (
"xorm.io/builder"
)
// EmailAddress is the list of all email addresses of a user. Can contain the
// primary email address, but is not obligatory.
// EmailAddress is the list of all email addresses of a user. It also contains the
// primary email address which is saved in user table.
type EmailAddress struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"INDEX NOT NULL"`
Email string `xorm:"UNIQUE NOT NULL"`
LowerEmail string `xorm:"UNIQUE NOT NULL"`
IsActivated bool
IsPrimary bool `xorm:"-"`
IsPrimary bool `xorm:"DEFAULT(false) NOT NULL"`
}
// BeforeInsert will be invoked by XORM before inserting a record
func (email *EmailAddress) BeforeInsert() {
if email.LowerEmail == "" {
email.LowerEmail = strings.ToLower(email.Email)
}
}
// ValidateEmail check if email is a allowed address
@ -47,34 +55,10 @@ func GetEmailAddresses(uid int64) ([]*EmailAddress, error) {
emails := make([]*EmailAddress, 0, 5)
if err := x.
Where("uid=?", uid).
Asc("id").
Find(&emails); err != nil {
return nil, err
}
u, err := GetUserByID(uid)
if err != nil {
return nil, err
}
isPrimaryFound := false
for _, email := range emails {
if email.Email == u.Email {
isPrimaryFound = true
email.IsPrimary = true
} else {
email.IsPrimary = false
}
}
// We always want the primary email address displayed, even if it's not in
// the email address table (yet).
if !isPrimaryFound {
emails = append(emails, &EmailAddress{
Email: u.Email,
IsActivated: u.IsActive,
IsPrimary: true,
})
}
return emails, nil
}
@ -97,14 +81,13 @@ func isEmailActive(e Engine, email string, userID, emailID int64) (bool, error)
// Can't filter by boolean field unless it's explicit
cond := builder.NewCond()
cond = cond.And(builder.Eq{"email": email}, builder.Neq{"id": emailID})
cond = cond.And(builder.Eq{"lower_email": strings.ToLower(email)}, builder.Neq{"id": emailID})
if setting.Service.RegisterEmailConfirm {
// Inactive (unvalidated) addresses don't count as active if email validation is required
cond = cond.And(builder.Eq{"is_activated": true})
}
em := EmailAddress{}
var em EmailAddress
if has, err := e.Where(cond).Get(&em); has || err != nil {
if has {
log.Info("isEmailActive('%s',%d,%d) found duplicate in email ID %d", email, userID, emailID, em.ID)
@ -112,22 +95,6 @@ func isEmailActive(e Engine, email string, userID, emailID int64) (bool, error)
return has, err
}
// Can't filter by boolean field unless it's explicit
cond = builder.NewCond()
cond = cond.And(builder.Eq{"email": email}, builder.Neq{"id": userID})
if setting.Service.RegisterEmailConfirm {
cond = cond.And(builder.Eq{"is_active": true})
}
us := User{}
if has, err := e.Where(cond).Get(&us); has || err != nil {
if has {
log.Info("isEmailActive('%s',%d,%d) found duplicate in user ID %d", email, userID, emailID, us.ID)
}
return has, err
}
return false, nil
}
@ -136,7 +103,7 @@ func isEmailUsed(e Engine, email string) (bool, error) {
return true, nil
}
return e.Where("email=?", email).Get(&EmailAddress{})
return e.Where("lower_email=?", strings.ToLower(email)).Get(&EmailAddress{})
}
// IsEmailUsed returns true if the email has been used.
@ -145,7 +112,7 @@ func IsEmailUsed(email string) (bool, error) {
}
func addEmailAddress(e Engine, email *EmailAddress) error {
email.Email = strings.ToLower(strings.TrimSpace(email.Email))
email.Email = strings.TrimSpace(email.Email)
used, err := isEmailUsed(e, email.Email)
if err != nil {
return err
@ -174,7 +141,7 @@ func AddEmailAddresses(emails []*EmailAddress) error {
// Check if any of them has been used
for i := range emails {
emails[i].Email = strings.ToLower(strings.TrimSpace(emails[i].Email))
emails[i].Email = strings.TrimSpace(emails[i].Email)
used, err := IsEmailUsed(emails[i].Email)
if err != nil {
return err
@ -223,6 +190,10 @@ func (email *EmailAddress) updateActivation(e Engine, activate bool) error {
// DeleteEmailAddress deletes an email address of given user.
func DeleteEmailAddress(email *EmailAddress) (err error) {
if email.IsPrimary {
return ErrPrimaryEmailCannotDelete{Email: email.Email}
}
var deleted int64
// ask to check UID
address := EmailAddress{
@ -231,8 +202,11 @@ func DeleteEmailAddress(email *EmailAddress) (err error) {
if email.ID > 0 {
deleted, err = x.ID(email.ID).Delete(&address)
} else {
if email.Email != "" && email.LowerEmail == "" {
email.LowerEmail = strings.ToLower(email.Email)
}
deleted, err = x.
Where("email=?", email.Email).
Where("lower_email=?", email.LowerEmail).
Delete(&address)
}
@ -261,7 +235,7 @@ func MakeEmailPrimary(email *EmailAddress) error {
if err != nil {
return err
} else if !has {
return ErrEmailNotExist
return ErrEmailAddressNotExist{Email: email.Email}
}
if !email.IsActivated {
@ -276,32 +250,31 @@ func MakeEmailPrimary(email *EmailAddress) error {
return ErrUserNotExist{email.UID, "", 0}
}
// Make sure the former primary email doesn't disappear.
formerPrimaryEmail := &EmailAddress{UID: user.ID, Email: user.Email}
has, err = x.Get(formerPrimaryEmail)
if err != nil {
return err
}
sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
return err
}
if !has {
formerPrimaryEmail.UID = user.ID
formerPrimaryEmail.IsActivated = user.IsActive
if _, err = sess.Insert(formerPrimaryEmail); err != nil {
return err
}
}
// 1. Update user table
user.Email = email.Email
if _, err = sess.ID(user.ID).Cols("email").Update(user); err != nil {
return err
}
// 2. Update old primary email
if _, err = sess.Where("uid=? AND is_primary=?", email.UID, true).Cols("is_primary").Update(&EmailAddress{
IsPrimary: false,
}); err != nil {
return err
}
// 3. update new primay email
email.IsPrimary = true
if _, err = sess.ID(email.ID).Cols("is_primary").Update(email); err != nil {
return err
}
return sess.Commit()
}
@ -314,10 +287,10 @@ func (s SearchEmailOrderBy) String() string {
// Strings for sorting result
const (
SearchEmailOrderByEmail SearchEmailOrderBy = "emails.email ASC, is_primary DESC, sortid ASC"
SearchEmailOrderByEmailReverse SearchEmailOrderBy = "emails.email DESC, is_primary ASC, sortid DESC"
SearchEmailOrderByName SearchEmailOrderBy = "`user`.lower_name ASC, is_primary DESC, sortid ASC"
SearchEmailOrderByNameReverse SearchEmailOrderBy = "`user`.lower_name DESC, is_primary ASC, sortid DESC"
SearchEmailOrderByEmail SearchEmailOrderBy = "email_address.lower_email ASC, email_address.is_primary DESC, email_address.id ASC"
SearchEmailOrderByEmailReverse SearchEmailOrderBy = "email_address.lower_email DESC, email_address.is_primary ASC, email_address.id DESC"
SearchEmailOrderByName SearchEmailOrderBy = "`user`.lower_name ASC, email_address.is_primary DESC, email_address.id ASC"
SearchEmailOrderByNameReverse SearchEmailOrderBy = "`user`.lower_name DESC, email_address.is_primary ASC, email_address.id DESC"
)
// SearchEmailOptions are options to search e-mail addresses for the admin panel
@ -343,54 +316,32 @@ type SearchEmailResult struct {
// SearchEmails takes options i.e. keyword and part of email name to search,
// it returns results in given range and number of total results.
func SearchEmails(opts *SearchEmailOptions) ([]*SearchEmailResult, int64, error) {
// Unfortunately, UNION support for SQLite in xorm is currently broken, so we must
// build the SQL ourselves.
where := make([]string, 0, 5)
args := make([]interface{}, 0, 5)
emailsSQL := "(SELECT id as sortid, uid, email, is_activated, 0 as is_primary " +
"FROM email_address " +
"UNION ALL " +
"SELECT id as sortid, id AS uid, email, is_active AS is_activated, 1 as is_primary " +
"FROM `user` " +
"WHERE type = ?) AS emails"
args = append(args, UserTypeIndividual)
var cond builder.Cond = builder.Eq{"user.`type`": UserTypeIndividual}
if len(opts.Keyword) > 0 {
// Note: % can be injected in the Keyword parameter, but it won't do any harm.
where = append(where, "(lower(`user`.full_name) LIKE ? OR `user`.lower_name LIKE ? OR emails.email LIKE ?)")
likeStr := "%" + strings.ToLower(opts.Keyword) + "%"
args = append(args, likeStr)
args = append(args, likeStr)
args = append(args, likeStr)
cond = cond.And(builder.Or(
builder.Like{"lower(`user`.full_name)", likeStr},
builder.Like{"`user`.lower_name", likeStr},
builder.Like{"email_address.lower_email", likeStr},
))
}
switch {
case opts.IsPrimary.IsTrue():
where = append(where, "emails.is_primary = ?")
args = append(args, true)
cond = cond.And(builder.Eq{"email_address.is_primary": true})
case opts.IsPrimary.IsFalse():
where = append(where, "emails.is_primary = ?")
args = append(args, false)
cond = cond.And(builder.Eq{"email_address.is_primary": false})
}
switch {
case opts.IsActivated.IsTrue():
where = append(where, "emails.is_activated = ?")
args = append(args, true)
cond = cond.And(builder.Eq{"email_address.is_activated": true})
case opts.IsActivated.IsFalse():
where = append(where, "emails.is_activated = ?")
args = append(args, false)
cond = cond.And(builder.Eq{"email_address.is_activated": false})
}
var whereStr string
if len(where) > 0 {
whereStr = "WHERE " + strings.Join(where, " AND ")
}
joinSQL := "FROM " + emailsSQL + " INNER JOIN `user` ON `user`.id = emails.uid " + whereStr
count, err := x.SQL("SELECT count(*) "+joinSQL, args...).Count()
count, err := x.Join("INNER", "`user`", "`user`.ID = email_address.uid").
Where(cond).Count(new(EmailAddress))
if err != nil {
return nil, 0, fmt.Errorf("Count: %v", err)
}
@ -400,36 +351,16 @@ func SearchEmails(opts *SearchEmailOptions) ([]*SearchEmailResult, int64, error)
orderby = SearchEmailOrderByEmail.String()
}
querySQL := "SELECT emails.uid, emails.email, emails.is_activated, emails.is_primary, " +
"`user`.name, `user`.full_name " + joinSQL + " ORDER BY " + orderby
opts.setDefaultValues()
rows, err := x.SQL(querySQL, args...).Rows(new(SearchEmailResult))
if err != nil {
return nil, 0, fmt.Errorf("Emails: %v", err)
}
// Page manually because xorm can't handle Limit() with raw SQL
defer rows.Close()
emails := make([]*SearchEmailResult, 0, opts.PageSize)
skip := (opts.Page - 1) * opts.PageSize
for rows.Next() {
var email SearchEmailResult
if err := rows.Scan(&email); err != nil {
return nil, 0, err
}
if skip > 0 {
skip--
continue
}
emails = append(emails, &email)
if len(emails) == opts.PageSize {
break
}
}
err = x.Table("email_address").
Select("email_address.*, `user`.name, `user`.full_name").
Join("INNER", "`user`", "`user`.ID = email_address.uid").
Where(cond).
OrderBy(orderby).
Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
Find(&emails)
return emails, count, err
}
@ -442,6 +373,30 @@ func ActivateUserEmail(userID int64, email string, primary, activate bool) (err
if err = sess.Begin(); err != nil {
return err
}
// Activate/deactivate a user's secondary email address
// First check if there's another user active with the same address
addr := EmailAddress{UID: userID, LowerEmail: strings.ToLower(email)}
if has, err := sess.Get(&addr); err != nil {
return err
} else if !has {
return fmt.Errorf("no such email: %d (%s)", userID, email)
}
if addr.IsActivated == activate {
// Already in the desired state; no action
return nil
}
if activate {
if used, err := isEmailActive(sess, email, 0, addr.ID); err != nil {
return fmt.Errorf("isEmailActive(): %v", err)
} else if used {
return ErrEmailAlreadyUsed{Email: email}
}
}
if err = addr.updateActivation(sess, activate); err != nil {
return fmt.Errorf("updateActivation(): %v", err)
}
if primary {
// Activate/deactivate a user's primary email address
user := User{ID: userID, Email: email}
@ -454,13 +409,6 @@ func ActivateUserEmail(userID int64, email string, primary, activate bool) (err
// Already in the desired state; no action
return nil
}
if activate {
if used, err := isEmailActive(sess, email, userID, 0); err != nil {
return fmt.Errorf("isEmailActive(): %v", err)
} else if used {
return ErrEmailAlreadyUsed{Email: email}
}
}
user.IsActive = activate
if user.Rands, err = GetUserSalt(); err != nil {
return fmt.Errorf("generate salt: %v", err)
@ -468,29 +416,7 @@ func ActivateUserEmail(userID int64, email string, primary, activate bool) (err
if err = updateUserCols(sess, &user, "is_active", "rands"); err != nil {
return fmt.Errorf("updateUserCols(): %v", err)
}
} else {
// Activate/deactivate a user's secondary email address
// First check if there's another user active with the same address
addr := EmailAddress{UID: userID, Email: email}
if has, err := sess.Get(&addr); err != nil {
return err
} else if !has {
return fmt.Errorf("no such email: %d (%s)", userID, email)
}
if addr.IsActivated == activate {
// Already in the desired state; no action
return nil
}
if activate {
if used, err := isEmailActive(sess, email, 0, addr.ID); err != nil {
return fmt.Errorf("isEmailActive(): %v", err)
} else if used {
return ErrEmailAlreadyUsed{Email: email}
}
}
if err = addr.updateActivation(sess, activate); err != nil {
return fmt.Errorf("updateActivation(): %v", err)
}
}
return sess.Commit()
}

51
models/user_mail_test.go
View File

@ -17,9 +17,9 @@ func TestGetEmailAddresses(t *testing.T) {
emails, _ := GetEmailAddresses(int64(1))
if assert.Len(t, emails, 3) {
assert.False(t, emails[0].IsPrimary)
assert.True(t, emails[0].IsPrimary)
assert.True(t, emails[2].IsActivated)
assert.True(t, emails[2].IsPrimary)
assert.False(t, emails[2].IsPrimary)
}
emails, _ = GetEmailAddresses(int64(2))
@ -45,13 +45,15 @@ func TestAddEmailAddress(t *testing.T) {
assert.NoError(t, AddEmailAddress(&EmailAddress{
Email: "user1234567890@example.com",
LowerEmail: "user1234567890@example.com",
IsPrimary: true,
IsActivated: true,
}))
// ErrEmailAlreadyUsed
err := AddEmailAddress(&EmailAddress{
Email: "user1234567890@example.com",
Email: "user1234567890@example.com",
LowerEmail: "user1234567890@example.com",
})
assert.Error(t, err)
assert.True(t, IsErrEmailAlreadyUsed(err))
@ -64,10 +66,12 @@ func TestAddEmailAddresses(t *testing.T) {
emails := make([]*EmailAddress, 2)
emails[0] = &EmailAddress{
Email: "user1234@example.com",
LowerEmail: "user1234@example.com",
IsActivated: true,
}
emails[1] = &EmailAddress{
Email: "user5678@example.com",
LowerEmail: "user5678@example.com",
IsActivated: true,
}
assert.NoError(t, AddEmailAddresses(emails))
@ -82,20 +86,23 @@ func TestDeleteEmailAddress(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
assert.NoError(t, DeleteEmailAddress(&EmailAddress{
UID: int64(1),
ID: int64(1),
Email: "user11@example.com",
UID: int64(1),
ID: int64(33),
Email: "user1-2@example.com",
LowerEmail: "user1-2@example.com",
}))
assert.NoError(t, DeleteEmailAddress(&EmailAddress{
UID: int64(1),
Email: "user12@example.com",
UID: int64(1),
Email: "user1-3@example.com",
LowerEmail: "user1-3@example.com",
}))
// Email address does not exist
err := DeleteEmailAddress(&EmailAddress{
UID: int64(1),
Email: "user1234567890@example.com",
UID: int64(1),
Email: "user1234567890@example.com",
LowerEmail: "user1234567890@example.com",
})
assert.Error(t, err)
}
@ -106,13 +113,15 @@ func TestDeleteEmailAddresses(t *testing.T) {
// delete multiple email address
emails := make([]*EmailAddress, 2)
emails[0] = &EmailAddress{
UID: int64(2),
ID: int64(3),
Email: "user2@example.com",
UID: int64(2),
ID: int64(3),
Email: "user2@example.com",
LowerEmail: "user2@example.com",
}
emails[1] = &EmailAddress{
UID: int64(2),
Email: "user21@example.com",
UID: int64(2),
Email: "user2-2@example.com",
LowerEmail: "user2-2@example.com",
}
assert.NoError(t, DeleteEmailAddresses(emails))
@ -129,7 +138,7 @@ func TestMakeEmailPrimary(t *testing.T) {
}
err := MakeEmailPrimary(email)
assert.Error(t, err)
assert.EqualError(t, err, ErrEmailNotExist.Error())
assert.EqualError(t, err, ErrEmailAddressNotExist{email.Email}.Error())
email = &EmailAddress{
Email: "user11@example.com",
@ -168,15 +177,21 @@ func TestActivate(t *testing.T) {
emails, _ := GetEmailAddresses(int64(1))
assert.Len(t, emails, 3)
assert.True(t, emails[0].IsActivated)
assert.True(t, emails[0].IsPrimary)
assert.False(t, emails[1].IsPrimary)
assert.True(t, emails[2].IsActivated)
assert.True(t, emails[2].IsPrimary)
assert.False(t, emails[2].IsPrimary)
}
func TestListEmails(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
// Must find all users and their emails
opts := &SearchEmailOptions{}
opts := &SearchEmailOptions{
ListOptions: ListOptions{
PageSize: 10000,
},
}
emails, count, err := SearchEmails(opts)
assert.NoError(t, err)
assert.NotEqual(t, int64(0), count)