Skip to content

Commit 18033f4

Browse files
lunnylunny
authored
Restrict email address validation (#17688)
This didn't follow the RFC but it's a subset of that. I think we should narrow the allowed chars at first and discuss more possibility in future PRs.
1 parent 49db87a commit 18033f4

File tree

9 files changed

+110
-11
lines changed

9 files changed

+110
-11
lines changed

‎models/user/email_address.go

+29-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"errors"
1111
"fmt"
1212
"net/mail"
13+
"regexp"
1314
"strings"
1415

1516
"code.gitea.io/gitea/models/db"
@@ -22,7 +23,22 @@ import (
2223
)
2324

2425
// ErrEmailNotActivated e-mail address has not been activated error
25-
var ErrEmailNotActivated = errors.New("E-mail address has not been activated")
26+
var ErrEmailNotActivated = errors.New("e-mail address has not been activated")
27+
28+
// ErrEmailCharIsNotSupported e-mail address contains unsupported character
29+
type ErrEmailCharIsNotSupported struct {
30+
Email string
31+
}
32+
33+
// IsErrEmailCharIsNotSupported checks if an error is an ErrEmailCharIsNotSupported
34+
func IsErrEmailCharIsNotSupported(err error) bool {
35+
_, ok := err.(ErrEmailCharIsNotSupported)
36+
return ok
37+
}
38+
39+
func (err ErrEmailCharIsNotSupported) Error() string {
40+
return fmt.Sprintf("e-mail address contains unsupported character [email: %s]", err.Email)
41+
}
2642

2743
// ErrEmailInvalid represents an error where the email address does not comply with RFC 5322
2844
type ErrEmailInvalid struct {
@@ -106,12 +122,24 @@ func (email *EmailAddress) BeforeInsert() {
106122
}
107123
}
108124

125+
var emailRegexp = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+-/=?^_`{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$")
126+
109127
// ValidateEmail check if email is a allowed address
110128
func ValidateEmail(email string) error {
111129
if len(email) == 0 {
112130
return nil
113131
}
114132

133+
if !emailRegexp.MatchString(email) {
134+
return ErrEmailCharIsNotSupported{email}
135+
}
136+
137+
if !(email[0] >= 'a' && email[0] <= 'z') &&
138+
!(email[0] >= 'A' && email[0] <= 'Z') &&
139+
!(email[0] >= '0' && email[0] <= '9') {
140+
return ErrEmailInvalid{email}
141+
}
142+
115143
if _, err := mail.ParseAddress(email); err != nil {
116144
return ErrEmailInvalid{email}
117145
}

‎models/user/email_address_test.go

+55
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,58 @@ func TestListEmails(t *testing.T) {
252252
assert.Len(t, emails, 5)
253253
assert.Greater(t, count, int64(len(emails)))
254254
}
255+
256+
func TestEmailAddressValidate(t *testing.T) {
257+
kases := map[string]error{
258+
"[email protected]": nil,
259+
"[email protected]": nil,
260+
"[email protected]": nil,
261+
"[email protected]": nil,
262+
"[email protected]": nil,
263+
`[email protected]`: nil,
264+
`[email protected]`: nil,
265+
`first#[email protected]`: nil,
266+
`[email protected]`: nil,
267+
`first%[email protected]`: nil,
268+
`first&[email protected]`: nil,
269+
`first'[email protected]`: nil,
270+
`first*[email protected]`: nil,
271+
`[email protected]`: nil,
272+
`first/[email protected]`: nil,
273+
`[email protected]`: nil,
274+
`[email protected]`: nil,
275+
`first^[email protected]`: nil,
276+
"first`[email protected]": nil,
277+
`first{[email protected]`: nil,
278+
`first|[email protected]`: nil,
279+
`first}[email protected]`: nil,
280+
`[email protected]`: nil,
281+
`first;[email protected]`: ErrEmailCharIsNotSupported{`first;[email protected]`},
282+
"[email protected]": ErrEmailInvalid{"[email protected]"},
283+
"[email protected]": ErrEmailInvalid{"[email protected]"},
284+
"#[email protected]": ErrEmailInvalid{"#[email protected]"},
285+
"[email protected]": ErrEmailInvalid{"[email protected]"},
286+
"%[email protected]": ErrEmailInvalid{"%[email protected]"},
287+
"&[email protected]": ErrEmailInvalid{"&[email protected]"},
288+
"'[email protected]": ErrEmailInvalid{"'[email protected]"},
289+
"*[email protected]": ErrEmailInvalid{"*[email protected]"},
290+
"[email protected]": ErrEmailInvalid{"[email protected]"},
291+
"/[email protected]": ErrEmailInvalid{"/[email protected]"},
292+
"[email protected]": ErrEmailInvalid{"[email protected]"},
293+
"[email protected]": ErrEmailInvalid{"[email protected]"},
294+
"^[email protected]": ErrEmailInvalid{"^[email protected]"},
295+
"`[email protected]": ErrEmailInvalid{"`[email protected]"},
296+
"{[email protected]": ErrEmailInvalid{"{[email protected]"},
297+
"|[email protected]": ErrEmailInvalid{"|[email protected]"},
298+
"}[email protected]": ErrEmailInvalid{"}[email protected]"},
299+
"[email protected]": ErrEmailInvalid{"[email protected]"},
300+
";[email protected]": ErrEmailCharIsNotSupported{";[email protected]"},
301+
"Foo <[email protected]>": ErrEmailCharIsNotSupported{"Foo <[email protected]>"},
302+
string([]byte{0xE2, 0x84, 0xAA}): ErrEmailCharIsNotSupported{string([]byte{0xE2, 0x84, 0xAA})},
303+
}
304+
for kase, err := range kases {
305+
t.Run(kase, func(t *testing.T) {
306+
assert.EqualValues(t, err, ValidateEmail(kase))
307+
})
308+
}
309+
}

‎models/user/user.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,15 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e
644644
u.Visibility = overwriteDefault[0].Visibility
645645
}
646646

647+
// validate data
648+
if err := validateUser(u); err != nil {
649+
return err
650+
}
651+
652+
if err := ValidateEmail(u.Email); err != nil {
653+
return err
654+
}
655+
647656
ctx, committer, err := db.TxContext()
648657
if err != nil {
649658
return err
@@ -652,11 +661,6 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e
652661

653662
sess := db.GetEngine(ctx)
654663

655-
// validate data
656-
if err := validateUser(u); err != nil {
657-
return err
658-
}
659-
660664
isExist, err := isUserExist(sess, 0, u.Name)
661665
if err != nil {
662666
return err

‎models/user/user_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func TestCreateUserInvalidEmail(t *testing.T) {
232232

233233
err := CreateUser(user)
234234
assert.Error(t, err)
235-
assert.True(t, IsErrEmailInvalid(err))
235+
assert.True(t, IsErrEmailCharIsNotSupported(err))
236236
}
237237

238238
func TestCreateUserEmailAlreadyUsed(t *testing.T) {

‎routers/api/v1/admin/user.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ func CreateUser(ctx *context.APIContext) {
119119
user_model.IsErrEmailAlreadyUsed(err) ||
120120
db.IsErrNameReserved(err) ||
121121
db.IsErrNameCharsNotAllowed(err) ||
122+
user_model.IsErrEmailCharIsNotSupported(err) ||
122123
user_model.IsErrEmailInvalid(err) ||
123124
db.IsErrNamePatternNotAllowed(err) {
124125
ctx.Error(http.StatusUnprocessableEntity, "", err)
@@ -265,7 +266,9 @@ func EditUser(ctx *context.APIContext) {
265266
}
266267

267268
if err := user_model.UpdateUser(u, emailChanged); err != nil {
268-
if user_model.IsErrEmailAlreadyUsed(err) || user_model.IsErrEmailInvalid(err) {
269+
if user_model.IsErrEmailAlreadyUsed(err) ||
270+
user_model.IsErrEmailCharIsNotSupported(err) ||
271+
user_model.IsErrEmailInvalid(err) {
269272
ctx.Error(http.StatusUnprocessableEntity, "", err)
270273
} else {
271274
ctx.Error(http.StatusInternalServerError, "UpdateUser", err)

‎routers/api/v1/user/email.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ func AddEmail(ctx *context.APIContext) {
8080
if err := user_model.AddEmailAddresses(emails); err != nil {
8181
if user_model.IsErrEmailAlreadyUsed(err) {
8282
ctx.Error(http.StatusUnprocessableEntity, "", "Email address has been used: "+err.(user_model.ErrEmailAlreadyUsed).Email)
83-
} else if user_model.IsErrEmailInvalid(err) {
83+
} else if user_model.IsErrEmailCharIsNotSupported(err) ||
84+
user_model.IsErrEmailInvalid(err) {
8485
errMsg := fmt.Sprintf("Email address %s invalid", err.(user_model.ErrEmailInvalid).Email)
8586
ctx.Error(http.StatusUnprocessableEntity, "", errMsg)
8687
} else {

‎routers/web/admin/users.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,9 @@ func NewUserPost(ctx *context.Context) {
171171
case user_model.IsErrEmailAlreadyUsed(err):
172172
ctx.Data["Err_Email"] = true
173173
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserNew, &form)
174+
case user_model.IsErrEmailCharIsNotSupported(err):
175+
ctx.Data["Err_Email"] = true
176+
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form)
174177
case user_model.IsErrEmailInvalid(err):
175178
ctx.Data["Err_Email"] = true
176179
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form)
@@ -386,7 +389,8 @@ func EditUserPost(ctx *context.Context) {
386389
if user_model.IsErrEmailAlreadyUsed(err) {
387390
ctx.Data["Err_Email"] = true
388391
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserEdit, &form)
389-
} else if user_model.IsErrEmailInvalid(err) {
392+
} else if user_model.IsErrEmailCharIsNotSupported(err) ||
393+
user_model.IsErrEmailInvalid(err) {
390394
ctx.Data["Err_Email"] = true
391395
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserEdit, &form)
392396
} else {

‎routers/web/auth/auth.go

+3
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,9 @@ func createUserInContext(ctx *context.Context, tpl base.TplName, form interface{
573573
case user_model.IsErrEmailAlreadyUsed(err):
574574
ctx.Data["Err_Email"] = true
575575
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tpl, form)
576+
case user_model.IsErrEmailCharIsNotSupported(err):
577+
ctx.Data["Err_Email"] = true
578+
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tpl, form)
576579
case user_model.IsErrEmailInvalid(err):
577580
ctx.Data["Err_Email"] = true
578581
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tpl, form)

‎routers/web/user/setting/account.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ func EmailPost(ctx *context.Context) {
188188

189189
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSettingsAccount, &form)
190190
return
191-
} else if user_model.IsErrEmailInvalid(err) {
191+
} else if user_model.IsErrEmailCharIsNotSupported(err) ||
192+
user_model.IsErrEmailInvalid(err) {
192193
loadAccountData(ctx)
193194

194195
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsAccount, &form)

0 commit comments

Comments
 (0)