From c34b30a6e7fb707ae12e78a51e5bd776e5ca85ed Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Wed, 1 Aug 2018 15:18:14 -0400 Subject: MM-11521/MM-11522 Fix being able to create users with invalid emails through API (#9199) * MM-11522 Fix being able to create users with invalid emails through API * Ensure store tests are using valid emails * Add missing license header * Remove invalid test case * Fix TestUpdateOAuthUserAttrs --- model/user.go | 2 +- model/user_test.go | 21 ++++++++----- model/utils.go | 10 +++--- model/utils_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 101 insertions(+), 20 deletions(-) (limited to 'model') diff --git a/model/user.go b/model/user.go index 3e99c6fa4..c7ba6b9cb 100644 --- a/model/user.go +++ b/model/user.go @@ -132,7 +132,7 @@ func (u *User) IsValid() *AppError { return InvalidUserError("username", u.Id) } - if len(u.Email) > USER_EMAIL_MAX_LENGTH || len(u.Email) == 0 { + if len(u.Email) > USER_EMAIL_MAX_LENGTH || len(u.Email) == 0 || !IsValidEmail(u.Email) { return InvalidUserError("email", u.Id) } diff --git a/model/user_test.go b/model/user_test.go index a1953a40d..b3aaad522 100644 --- a/model/user_test.go +++ b/model/user_test.go @@ -118,34 +118,39 @@ func TestUserIsValid(t *testing.T) { user.Id = NewId() if err := user.IsValid(); !HasExpectedUserIsValidError(err, "create_at", user.Id) { - t.Fatal() + t.Fatal(err) } user.CreateAt = GetMillis() if err := user.IsValid(); !HasExpectedUserIsValidError(err, "update_at", user.Id) { - t.Fatal() + t.Fatal(err) } user.UpdateAt = GetMillis() if err := user.IsValid(); !HasExpectedUserIsValidError(err, "username", user.Id) { - t.Fatal() + t.Fatal(err) } user.Username = NewId() + "^hello#" if err := user.IsValid(); !HasExpectedUserIsValidError(err, "username", user.Id) { - t.Fatal() + t.Fatal(err) } user.Username = NewId() + if err := user.IsValid(); !HasExpectedUserIsValidError(err, "email", user.Id) { + t.Fatal(err) + } + user.Email = strings.Repeat("01234567890", 20) - if err := user.IsValid(); err == nil { - t.Fatal() + if err := user.IsValid(); !HasExpectedUserIsValidError(err, "email", user.Id) { + t.Fatal(err) } - user.Email = strings.Repeat("a", 128) + user.Email = "user@example.com" + user.Nickname = strings.Repeat("a", 65) if err := user.IsValid(); !HasExpectedUserIsValidError(err, "nickname", user.Id) { - t.Fatal() + t.Fatal(err) } user.Nickname = strings.Repeat("a", 64) diff --git a/model/utils.go b/model/utils.go index 892f8c278..0d8d359a6 100644 --- a/model/utils.go +++ b/model/utils.go @@ -279,16 +279,18 @@ func IsLower(s string) bool { } func IsValidEmail(email string) bool { - if !IsLower(email) { return false } - if _, err := mail.ParseAddress(email); err == nil { - return true + if addr, err := mail.ParseAddress(email); err != nil { + return false + } else if addr.Name != "" { + // mail.ParseAddress accepts input of the form "Billy Bob " which we don't allow + return false } - return false + return true } var reservedName = []string{ diff --git a/model/utils_test.go b/model/utils_test.go index b7f5dc628..9797c7090 100644 --- a/model/utils_test.go +++ b/model/utils_test.go @@ -77,13 +77,87 @@ func TestMapJson(t *testing.T) { } } -func TestValidEmail(t *testing.T) { - if !IsValidEmail("corey+test@hulen.com") { - t.Error("email should be valid") - } - - if IsValidEmail("@corey+test@hulen.com") { - t.Error("should be invalid") +func TestIsValidEmail(t *testing.T) { + for _, testCase := range []struct { + Input string + Expected bool + }{ + { + Input: "corey", + Expected: false, + }, + { + Input: "corey@example.com", + Expected: true, + }, + { + Input: "corey+test@example.com", + Expected: true, + }, + { + Input: "@corey+test@example.com", + Expected: false, + }, + { + Input: "firstname.lastname@example.com", + Expected: true, + }, + { + Input: "firstname.lastname@subdomain.example.com", + Expected: true, + }, + { + Input: "123454567@domain.com", + Expected: true, + }, + { + Input: "email@domain-one.com", + Expected: true, + }, + { + Input: "email@domain.co.jp", + Expected: true, + }, + { + Input: "firstname-lastname@domain.com", + Expected: true, + }, + { + Input: "@domain.com", + Expected: false, + }, + { + Input: "Billy Bob ", + Expected: false, + }, + { + Input: "email.domain.com", + Expected: false, + }, + { + Input: "email.@domain.com", + Expected: false, + }, + { + Input: "email@domain@domain.com", + Expected: false, + }, + { + Input: "(email@domain.com)", + Expected: false, + }, + { + Input: "email@汤.中国", + Expected: true, + }, + { + Input: "email1@domain.com, email2@domain.com", + Expected: false, + }, + } { + t.Run(testCase.Input, func(t *testing.T) { + assert.Equal(t, testCase.Expected, IsValidEmail(testCase.Input)) + }) } } -- cgit v1.2.3-1-g7c22