* 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> * skip email validation on empty string (#13627) - move validation into its own function - use a session for UpdateUserSetting * rm TODO for backport Co-authored-by: Chris Shyi <chrisshyi13@gmail.com> Co-authored-by: zeripath <art27@cantab.net> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io>tags/v1.13.0
@@ -144,3 +144,22 @@ func TestAPIListUsersNonAdmin(t *testing.T) { | |||
req := NewRequestf(t, "GET", "/api/v1/admin/users?token=%s", token) | |||
session.MakeRequest(t, req, http.StatusForbidden) | |||
} | |||
func TestAPICreateUserInvalidEmail(t *testing.T) { | |||
defer prepareTestEnv(t)() | |||
adminUsername := "user1" | |||
session := loginUser(t, adminUsername) | |||
token := getTokenForLoggedInUser(t, session) | |||
urlStr := fmt.Sprintf("/api/v1/admin/users?token=%s", token) | |||
req := NewRequestWithValues(t, "POST", urlStr, map[string]string{ | |||
"email": "invalid_email@domain.com\r\n", | |||
"full_name": "invalid user", | |||
"login_name": "invalidUser", | |||
"must_change_password": "true", | |||
"password": "password", | |||
"send_notify": "true", | |||
"source_id": "0", | |||
"username": "invalidUser", | |||
}) | |||
session.MakeRequest(t, req, http.StatusUnprocessableEntity) | |||
} |
@@ -5,10 +5,14 @@ | |||
package integrations | |||
import ( | |||
"fmt" | |||
"net/http" | |||
"strings" | |||
"testing" | |||
"code.gitea.io/gitea/modules/setting" | |||
"github.com/stretchr/testify/assert" | |||
"github.com/unknwon/i18n" | |||
) | |||
func TestSignup(t *testing.T) { | |||
@@ -28,3 +32,37 @@ func TestSignup(t *testing.T) { | |||
req = NewRequest(t, "GET", "/exampleUser") | |||
MakeRequest(t, req, http.StatusOK) | |||
} | |||
func TestSignupEmail(t *testing.T) { | |||
defer prepareTestEnv(t)() | |||
setting.Service.EnableCaptcha = false | |||
tests := []struct { | |||
email string | |||
wantStatus int | |||
wantMsg string | |||
}{ | |||
{"exampleUser@example.com\r\n", http.StatusOK, i18n.Tr("en", "form.email_invalid", nil)}, | |||
{"exampleUser@example.com\r", http.StatusOK, i18n.Tr("en", "form.email_invalid", nil)}, | |||
{"exampleUser@example.com\n", http.StatusOK, i18n.Tr("en", "form.email_invalid", nil)}, | |||
{"exampleUser@example.com", http.StatusFound, ""}, | |||
} | |||
for i, test := range tests { | |||
req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{ | |||
"user_name": fmt.Sprintf("exampleUser%d", i), | |||
"email": test.email, | |||
"password": "examplePassword!1", | |||
"retype": "examplePassword!1", | |||
}) | |||
resp := MakeRequest(t, req, test.wantStatus) | |||
if test.wantMsg != "" { | |||
htmlDoc := NewHTMLParser(t, resp.Body) | |||
assert.Equal(t, | |||
test.wantMsg, | |||
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()), | |||
) | |||
} | |||
} | |||
} |
@@ -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 |
@@ -821,6 +821,10 @@ func CreateUser(u *User) (err error) { | |||
return ErrEmailAlreadyUsed{u.Email} | |||
} | |||
if err = ValidateEmail(u.Email); err != nil { | |||
return err | |||
} | |||
isExist, err = isEmailUsed(sess, u.Email) | |||
if err != nil { | |||
return err | |||
@@ -963,8 +967,12 @@ func checkDupEmail(e Engine, u *User) error { | |||
return nil | |||
} | |||
func updateUser(e Engine, u *User) error { | |||
_, err := e.ID(u.ID).AllCols().Update(u) | |||
func updateUser(e Engine, u *User) (err error) { | |||
u.Email = strings.ToLower(u.Email) | |||
if err = ValidateEmail(u.Email); err != nil { | |||
return err | |||
} | |||
_, err = e.ID(u.ID).AllCols().Update(u) | |||
return err | |||
} | |||
@@ -984,13 +992,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. |
@@ -8,6 +8,7 @@ package models | |||
import ( | |||
"errors" | |||
"fmt" | |||
"net/mail" | |||
"strings" | |||
"code.gitea.io/gitea/modules/log" | |||
@@ -32,6 +33,19 @@ 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} | |||
} | |||
return nil | |||
} | |||
// GetEmailAddresses returns all email addresses belongs to given user. | |||
func GetEmailAddresses(uid int64) ([]*EmailAddress, error) { | |||
emails := make([]*EmailAddress, 0, 5) | |||
@@ -143,6 +157,10 @@ func addEmailAddress(e Engine, email *EmailAddress) error { | |||
return ErrEmailAlreadyUsed{email.Email} | |||
} | |||
if err = ValidateEmail(email.Email); err != nil { | |||
return err | |||
} | |||
_, err = e.Insert(email) | |||
return err | |||
} | |||
@@ -167,6 +185,9 @@ func AddEmailAddresses(emails []*EmailAddress) error { | |||
} else if used { | |||
return ErrEmailAlreadyUsed{emails[i].Email} | |||
} | |||
if err = ValidateEmail(emails[i].Email); err != nil { | |||
return err | |||
} | |||
} | |||
if _, err := x.Insert(emails); err != nil { |
@@ -346,6 +346,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 |
@@ -366,6 +366,7 @@ org_name_been_taken = The organization name is already taken. | |||
team_name_been_taken = The team name is already taken. | |||
team_no_units_error = Allow access to at least one repository section. | |||
email_been_used = The email address is already used. | |||
email_invalid = The email address is invalid. | |||
openid_been_used = The OpenID address '%s' is already used. | |||
username_password_incorrect = Username or password is incorrect. | |||
password_complexity = Password does not pass complexity requirements: |
@@ -129,6 +129,9 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) { | |||
case models.IsErrEmailAlreadyUsed(err): | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserNew, &form) | |||
case models.IsErrEmailInvalid(err): | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form) | |||
case models.IsErrNameReserved(err): | |||
ctx.Data["Err_UserName"] = true | |||
ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplUserNew, &form) | |||
@@ -277,6 +280,9 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) { | |||
if models.IsErrEmailAlreadyUsed(err) { | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserEdit, &form) | |||
} else if models.IsErrEmailInvalid(err) { | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserEdit, &form) | |||
} else { | |||
ctx.ServerError("UpdateUser", err) | |||
} |
@@ -87,3 +87,33 @@ func TestNewUserPost_MustChangePasswordFalse(t *testing.T) { | |||
assert.Equal(t, email, u.Email) | |||
assert.False(t, u.MustChangePassword) | |||
} | |||
func TestNewUserPost_InvalidEmail(t *testing.T) { | |||
models.PrepareTestEnv(t) | |||
ctx := test.MockContext(t, "admin/users/new") | |||
u := models.AssertExistsAndLoadBean(t, &models.User{ | |||
IsAdmin: true, | |||
ID: 2, | |||
}).(*models.User) | |||
ctx.User = u | |||
username := "gitea" | |||
email := "gitea@gitea.io\r\n" | |||
form := auth.AdminCreateUserForm{ | |||
LoginType: "local", | |||
LoginName: "local", | |||
UserName: username, | |||
Email: email, | |||
Password: "abc123ABC!=$", | |||
SendNotify: false, | |||
MustChangePassword: false, | |||
} | |||
NewUserPost(ctx, form) | |||
assert.NotEmpty(t, ctx.Flash.ErrorMsg) | |||
} |
@@ -101,6 +101,7 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) { | |||
models.IsErrEmailAlreadyUsed(err) || | |||
models.IsErrNameReserved(err) || | |||
models.IsErrNameCharsNotAllowed(err) || | |||
models.IsErrEmailInvalid(err) || | |||
models.IsErrNamePatternNotAllowed(err) { | |||
ctx.Error(http.StatusUnprocessableEntity, "", err) | |||
} else { | |||
@@ -208,7 +209,7 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) { | |||
} | |||
if err := models.UpdateUser(u); err != nil { | |||
if models.IsErrEmailAlreadyUsed(err) { | |||
if models.IsErrEmailAlreadyUsed(err) || models.IsErrEmailInvalid(err) { | |||
ctx.Error(http.StatusUnprocessableEntity, "", err) | |||
} else { | |||
ctx.Error(http.StatusInternalServerError, "UpdateUser", err) |
@@ -5,6 +5,7 @@ | |||
package user | |||
import ( | |||
"fmt" | |||
"net/http" | |||
"code.gitea.io/gitea/models" | |||
@@ -78,6 +79,9 @@ func AddEmail(ctx *context.APIContext, form api.CreateEmailOption) { | |||
if err := models.AddEmailAddresses(emails); err != nil { | |||
if models.IsErrEmailAlreadyUsed(err) { | |||
ctx.Error(http.StatusUnprocessableEntity, "", "Email address has been used: "+err.(models.ErrEmailAlreadyUsed).Email) | |||
} else if models.IsErrEmailInvalid(err) { | |||
errMsg := fmt.Sprintf("Email address %s invalid", err.(models.ErrEmailInvalid).Email) | |||
ctx.Error(http.StatusUnprocessableEntity, "", errMsg) | |||
} else { | |||
ctx.Error(http.StatusInternalServerError, "AddEmailAddresses", err) | |||
} |
@@ -964,6 +964,9 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au | |||
case models.IsErrEmailAlreadyUsed(err): | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplLinkAccount, &form) | |||
case models.IsErrEmailInvalid(err): | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, &form) | |||
case models.IsErrNameReserved(err): | |||
ctx.Data["Err_UserName"] = true | |||
ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplLinkAccount, &form) | |||
@@ -1151,6 +1154,9 @@ func SignUpPost(ctx *context.Context, cpt *captcha.Captcha, form auth.RegisterFo | |||
case models.IsErrEmailAlreadyUsed(err): | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSignUp, &form) | |||
case models.IsErrEmailInvalid(err): | |||
ctx.Data["Err_Email"] = true | |||
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, &form) | |||
case models.IsErrNameReserved(err): | |||
ctx.Data["Err_UserName"] = true | |||
ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplSignUp, &form) |
@@ -179,6 +179,11 @@ func EmailPost(ctx *context.Context, form auth.AddEmailForm) { | |||
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSettingsAccount, &form) | |||
return | |||
} else if models.IsErrEmailInvalid(err) { | |||
loadAccountData(ctx) | |||
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsAccount, &form) | |||
return | |||
} | |||
ctx.ServerError("AddEmailAddress", err) | |||
return |