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

4 files changed, 101 insertions, 20 deletions

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 <billy@example.com>" 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 <billy@example.com>",
+			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))
+		})
 	}
 }