skip email validation on empty string (#13627)

- move validation into its own function
- use a session for UpdateUserSetting

diff --git a/models/user.go b/models/user.go
index 9489ff4e8bb58bef67d19626385dc9f08f5e1f7c..2a7699557864996a90ba447064d330363fab53ce 100644 (file)
--- a/models/user.go
+++ b/models/user.go
@@ -14,7 +14,6 @@ import (
        "errors"
        "fmt"
        _ "image/jpeg" // Needed for jpeg support
-       "net/mail"
        "os"
        "path/filepath"
        "regexp"
@@ -809,9 +808,8 @@ func CreateUser(u *User) (err error) {
                return ErrEmailAlreadyUsed{u.Email}
        }
 
-       _, err = mail.ParseAddress(u.Email)
-       if err != nil {
-               return ErrEmailInvalid{u.Email}
+       if err = ValidateEmail(u.Email); err != nil {
+               return err
        }
 
        isExist, err = isEmailUsed(sess, u.Email)
@@ -956,11 +954,10 @@ func checkDupEmail(e Engine, u *User) error {
        return nil
 }
 
-func updateUser(e Engine, u *User) error {
+func updateUser(e Engine, u *User) (err error) {
        u.Email = strings.ToLower(u.Email)
-       _, err := mail.ParseAddress(u.Email)
-       if err != nil {
-               return ErrEmailInvalid{u.Email}
+       if err = ValidateEmail(u.Email); err != nil {
+               return err
        }
        _, err = e.ID(u.ID).AllCols().Update(u)
        return err
@@ -982,13 +979,21 @@ func updateUserCols(e Engine, u *User, cols ...string) error {
 }
 
 // UpdateUserSetting updates user's settings.
-func UpdateUserSetting(u *User) error {
+func UpdateUserSetting(u *User) (err error) {
+       sess := x.NewSession()
+       defer sess.Close()
+       if err = sess.Begin(); err != nil {
+               return err
+       }
        if !u.IsOrganization() {
-               if err := checkDupEmail(x, u); err != nil {
+               if err = checkDupEmail(sess, u); err != nil {
                        return err
                }
        }
-       return updateUser(x, u)
+       if err = updateUser(sess, u); err != nil {
+               return err
+       }
+       return sess.Commit()
 }
 
 // deleteBeans deletes all given beans, beans should contain delete conditions.

diff --git a/models/user_mail.go b/models/user_mail.go
index e15b5a3adfa964b128217f8a8c3c0b842f92805b..1e56a5cd8363edfe0cd0b966b338a372daf9f922 100644 (file)
--- a/models/user_mail.go
+++ b/models/user_mail.go
@@ -33,6 +33,21 @@ type EmailAddress struct {
        IsPrimary   bool `xorm:"-"`
 }
 
+// ValidateEmail check if email is a allowed address
+func ValidateEmail(email string) error {
+       if len(email) == 0 {
+               return nil
+       }
+
+       if _, err := mail.ParseAddress(email); err != nil {
+               return ErrEmailInvalid{email}
+       }
+
+       // TODO: add an email allow/block list
+
+       return nil
+}
+
 // GetEmailAddresses returns all email addresses belongs to given user.
 func GetEmailAddresses(uid int64) ([]*EmailAddress, error) {
        emails := make([]*EmailAddress, 0, 5)
@@ -144,9 +159,8 @@ func addEmailAddress(e Engine, email *EmailAddress) error {
                return ErrEmailAlreadyUsed{email.Email}
        }
 
-       _, err = mail.ParseAddress(email.Email)
-       if err != nil {
-               return ErrEmailInvalid{email.Email}
+       if err = ValidateEmail(email.Email); err != nil {
+               return err
        }
 
        _, err = e.Insert(email)
@@ -173,9 +187,8 @@ 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 = ValidateEmail(emails[i].Email); err != nil {
+                       return err
                }
        }