Add email validity check (#13475)

* Improve error feedback for duplicate deploy keys

Instead of a generic HTTP 500 error page, a flash message is rendered
with the deploy key page template so inform the user that a key with the
intended title already exists.

* API returns 422 error when key with name exists

* Add email validity checking

Add email validity checking for the following routes:
[Web interface]
1. User registration
2. User creation by admin
3. Adding an email through user settings
[API]
1. POST /admin/users
2. PATCH /admin/users/:username
3. POST /user/emails

* Add further tests

* Add signup email tests

* Add email validity check for linking existing account

* Address PR comments

* Remove unneeded DB session

* Move email check to updateUser

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>

4 files changed, 52 insertions, 1 deletions

diff --git a/models/error.go b/models/error.go
index b2273f74c9..83354ff173 100644
--- a/models/error.go
+++ b/models/error.go
@@ -193,6 +193,21 @@ func (err ErrEmailAlreadyUsed) Error() string {
 	return fmt.Sprintf("e-mail already in use [email: %s]", err.Email)
 }
 
+// ErrEmailInvalid represents an error where the email address does not comply with RFC 5322
+type ErrEmailInvalid struct {
+	Email string
+}
+
+// IsErrEmailInvalid checks if an error is an ErrEmailInvalid
+func IsErrEmailInvalid(err error) bool {
+	_, ok := err.(ErrEmailInvalid)
+	return ok
+}
+
+func (err ErrEmailInvalid) Error() string {
+	return fmt.Sprintf("e-mail invalid [email: %s]", err.Email)
+}
+
 // ErrOpenIDAlreadyUsed represents a "OpenIDAlreadyUsed" kind of error.
 type ErrOpenIDAlreadyUsed struct {
 	OpenID string
diff --git a/models/user.go b/models/user.go
index 42f70b4666..9489ff4e8b 100644
--- a/models/user.go
+++ b/models/user.go
@@ -14,6 +14,7 @@ import (
 	"errors"
 	"fmt"
 	_ "image/jpeg" // Needed for jpeg support
+	"net/mail"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -808,6 +809,11 @@ func CreateUser(u *User) (err error) {
 		return ErrEmailAlreadyUsed{u.Email}
 	}
 
+	_, err = mail.ParseAddress(u.Email)
+	if err != nil {
+		return ErrEmailInvalid{u.Email}
+	}
+
 	isExist, err = isEmailUsed(sess, u.Email)
 	if err != nil {
 		return err
@@ -951,7 +957,12 @@ func checkDupEmail(e Engine, u *User) error {
 }
 
 func updateUser(e Engine, u *User) error {
-	_, err := e.ID(u.ID).AllCols().Update(u)
+	u.Email = strings.ToLower(u.Email)
+	_, err := mail.ParseAddress(u.Email)
+	if err != nil {
+		return ErrEmailInvalid{u.Email}
+	}
+	_, err = e.ID(u.ID).AllCols().Update(u)
 	return err
 }
 
diff --git a/models/user_mail.go b/models/user_mail.go
index 60354e23ff..e15b5a3adf 100644
--- a/models/user_mail.go
+++ b/models/user_mail.go
@@ -8,6 +8,7 @@ package models
 import (
 	"errors"
 	"fmt"
+	"net/mail"
 	"strings"
 
 	"code.gitea.io/gitea/modules/log"
@@ -143,6 +144,11 @@ func addEmailAddress(e Engine, email *EmailAddress) error {
 		return ErrEmailAlreadyUsed{email.Email}
 	}
 
+	_, err = mail.ParseAddress(email.Email)
+	if err != nil {
+		return ErrEmailInvalid{email.Email}
+	}
+
 	_, err = e.Insert(email)
 	return err
 }
@@ -167,6 +173,10 @@ func AddEmailAddresses(emails []*EmailAddress) error {
 		} else if used {
 			return ErrEmailAlreadyUsed{emails[i].Email}
 		}
+		_, err = mail.ParseAddress(emails[i].Email)
+		if err != nil {
+			return ErrEmailInvalid{emails[i].Email}
+		}
 	}
 
 	if _, err := x.Insert(emails); err != nil {
diff --git a/models/user_test.go b/models/user_test.go
index 7a6f5aa512..216cd44c59 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -329,6 +329,21 @@ func TestCreateUser(t *testing.T) {
 	assert.NoError(t, DeleteUser(user))
 }
 
+func TestCreateUserInvalidEmail(t *testing.T) {
+	user := &User{
+		Name:               "GiteaBot",
+		Email:              "GiteaBot@gitea.io\r\n",
+		Passwd:             ";p['////..-++']",
+		IsAdmin:            false,
+		Theme:              setting.UI.DefaultTheme,
+		MustChangePassword: false,
+	}
+
+	err := CreateUser(user)
+	assert.Error(t, err)
+	assert.True(t, IsErrEmailInvalid(err))
+}
+
 func TestCreateUser_Issue5882(t *testing.T) {
 
 	// Init settings