From e7afba21ce2b02eb4230ba03752bd8b937f3e6ef Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Wed, 6 Mar 2024 00:51:56 +0800 Subject: [PATCH] Skip email domain check when admins edit user emails (#29609) Follow #29522 Administrators should be able to set a user's email address even if the email address is not in `EMAIL_DOMAIN_ALLOWLIST` (cherry picked from commit 136dd99e86eea9c8bfe61b972a12b395655171e8) --- models/user/email_address.go | 2 +- routers/api/v1/admin/user.go | 2 +- routers/web/admin/users.go | 2 +- services/user/email.go | 5 +++-- services/user/email_test.go | 22 ++++++++++++++++++---- tests/integration/api_admin_test.go | 29 +++++++++++++++++++++++++++++ 6 files changed, 53 insertions(+), 9 deletions(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index 80dc7a797..1d90b127b 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -165,7 +165,7 @@ func ValidateEmail(email string) error { return validateEmailDomain(email) } -// ValidateEmailForAdmin check if email is a valid address when admins manually add users +// ValidateEmailForAdmin check if email is a valid address when admins manually add or edit users func ValidateEmailForAdmin(email string) error { return validateEmailBasic(email) // In this case we do not need to check the email domain diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 7f4200f68..986305d42 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -209,7 +209,7 @@ func EditUser(ctx *context.APIContext) { } if form.Email != nil { - if err := user_service.AddOrSetPrimaryEmailAddress(ctx, ctx.ContextUser, *form.Email); err != nil { + if err := user_service.AdminAddOrSetPrimaryEmailAddress(ctx, ctx.ContextUser, *form.Email); err != nil { switch { case user_model.IsErrEmailCharIsNotSupported(err), user_model.IsErrEmailInvalid(err): ctx.Error(http.StatusBadRequest, "EmailInvalid", err) diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index dfb103c8a..671a0d888 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -412,7 +412,7 @@ func EditUserPost(ctx *context.Context) { } if form.Email != "" { - if err := user_service.AddOrSetPrimaryEmailAddress(ctx, u, form.Email); err != nil { + if err := user_service.AdminAddOrSetPrimaryEmailAddress(ctx, u, form.Email); err != nil { switch { case user_model.IsErrEmailCharIsNotSupported(err), user_model.IsErrEmailInvalid(err): ctx.Data["Err_Email"] = true diff --git a/services/user/email.go b/services/user/email.go index 0b579cf79..9dc527084 100644 --- a/services/user/email.go +++ b/services/user/email.go @@ -14,12 +14,13 @@ import ( "code.gitea.io/gitea/modules/util" ) -func AddOrSetPrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { +// AdminAddOrSetPrimaryEmailAddress is used by admins to add or set a user's primary email address +func AdminAddOrSetPrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { if strings.EqualFold(u.Email, emailStr) { return nil } - if err := user_model.ValidateEmail(emailStr); err != nil { + if err := user_model.ValidateEmailForAdmin(emailStr); err != nil { return err } diff --git a/services/user/email_test.go b/services/user/email_test.go index 66d482134..0784b4f80 100644 --- a/services/user/email_test.go +++ b/services/user/email_test.go @@ -10,11 +10,13 @@ import ( organization_model "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + "github.com/gobwas/glob" "github.com/stretchr/testify/assert" ) -func TestAddOrSetPrimaryEmailAddress(t *testing.T) { +func TestAdminAddOrSetPrimaryEmailAddress(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 27}) @@ -28,7 +30,7 @@ func TestAddOrSetPrimaryEmailAddress(t *testing.T) { assert.NotEqual(t, "new-primary@example.com", primary.Email) assert.Equal(t, user.Email, primary.Email) - assert.NoError(t, AddOrSetPrimaryEmailAddress(db.DefaultContext, user, "new-primary@example.com")) + assert.NoError(t, AdminAddOrSetPrimaryEmailAddress(db.DefaultContext, user, "new-primary@example.com")) primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) assert.NoError(t, err) @@ -39,7 +41,19 @@ func TestAddOrSetPrimaryEmailAddress(t *testing.T) { assert.NoError(t, err) assert.Len(t, emails, 2) - assert.NoError(t, AddOrSetPrimaryEmailAddress(db.DefaultContext, user, "user27@example.com")) + setting.Service.EmailDomainAllowList = []glob.Glob{glob.MustCompile("example.org")} + defer func() { + setting.Service.EmailDomainAllowList = []glob.Glob{} + }() + + assert.NoError(t, AdminAddOrSetPrimaryEmailAddress(db.DefaultContext, user, "new-primary2@example2.com")) + + primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Equal(t, "new-primary2@example2.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + assert.NoError(t, AdminAddOrSetPrimaryEmailAddress(db.DefaultContext, user, "user27@example.com")) primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) assert.NoError(t, err) @@ -48,7 +62,7 @@ func TestAddOrSetPrimaryEmailAddress(t *testing.T) { emails, err = user_model.GetEmailAddresses(db.DefaultContext, user.ID) assert.NoError(t, err) - assert.Len(t, emails, 2) + assert.Len(t, emails, 3) } func TestReplacePrimaryEmailAddress(t *testing.T) { diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 53bdd11af..8a330a68e 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -359,3 +359,32 @@ func TestAPICreateUser_NotAllowedEmailDomain(t *testing.T) { req = NewRequest(t, "DELETE", "/api/v1/admin/users/allowedUser1").AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) } + +func TestAPIEditUser_NotAllowedEmailDomain(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + setting.Service.EmailDomainAllowList = []glob.Glob{glob.MustCompile("example.org")} + defer func() { + setting.Service.EmailDomainAllowList = []glob.Glob{} + }() + + adminUsername := "user1" + token := getUserToken(t, adminUsername, auth_model.AccessTokenScopeWriteAdmin) + urlStr := fmt.Sprintf("/api/v1/admin/users/%s", "user2") + + newEmail := "user2@example1.com" + req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ + LoginName: "user2", + SourceID: 0, + Email: &newEmail, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) + + originalEmail := "user2@example.com" + req = NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ + LoginName: "user2", + SourceID: 0, + Email: &originalEmail, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) +}