From b6057a34db38e563473db00543a1e39fd743ca34 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Tue, 5 Mar 2024 13:55:47 +0800 Subject: [PATCH] Skip email domain check when admin users adds user manually (#29522) Fix #27457 Administrators should be able to manually create any user even if the user's email address is not in `EMAIL_DOMAIN_ALLOWLIST`. (cherry picked from commit 4fd9c56ed09b31e2f6164a5f534a31c6624d0478) --- models/user/email_address.go | 75 ++++++++++++++++++----------- models/user/user.go | 20 +++++++- routers/api/v1/admin/user.go | 2 +- routers/web/admin/users.go | 2 +- tests/integration/api_admin_test.go | 26 ++++++++++ 5 files changed, 93 insertions(+), 32 deletions(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index 6f3d5b1dde..80dc7a7971 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -157,37 +157,18 @@ func UpdateEmailAddress(ctx context.Context, email *EmailAddress) error { var emailRegexp = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+-/=?^_`{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") -// ValidateEmail check if email is a allowed address +// ValidateEmail check if email is a valid & allowed address func ValidateEmail(email string) error { - if len(email) == 0 { - return ErrEmailInvalid{email} + if err := validateEmailBasic(email); err != nil { + return err } + return validateEmailDomain(email) +} - if !emailRegexp.MatchString(email) { - return ErrEmailCharIsNotSupported{email} - } - - if email[0] == '-' { - return ErrEmailInvalid{email} - } - - if _, err := mail.ParseAddress(email); err != nil { - return ErrEmailInvalid{email} - } - - // if there is no allow list, then check email against block list - if len(setting.Service.EmailDomainAllowList) == 0 && - validation.IsEmailDomainListed(setting.Service.EmailDomainBlockList, email) { - return ErrEmailInvalid{email} - } - - // if there is an allow list, then check email against allow list - if len(setting.Service.EmailDomainAllowList) > 0 && - !validation.IsEmailDomainListed(setting.Service.EmailDomainAllowList, email) { - return ErrEmailInvalid{email} - } - - return nil +// ValidateEmailForAdmin check if email is a valid address when admins manually add users +func ValidateEmailForAdmin(email string) error { + return validateEmailBasic(email) + // In this case we do not need to check the email domain } func GetEmailAddressByEmail(ctx context.Context, email string) (*EmailAddress, error) { @@ -543,3 +524,41 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate return committer.Commit() } + +// validateEmailBasic checks whether the email complies with the rules +func validateEmailBasic(email string) error { + if len(email) == 0 { + return ErrEmailInvalid{email} + } + + if !emailRegexp.MatchString(email) { + return ErrEmailCharIsNotSupported{email} + } + + if email[0] == '-' { + return ErrEmailInvalid{email} + } + + if _, err := mail.ParseAddress(email); err != nil { + return ErrEmailInvalid{email} + } + + return nil +} + +// validateEmailDomain checks whether the email domain is allowed or blocked +func validateEmailDomain(email string) error { + // if there is no allow list, then check email against block list + if len(setting.Service.EmailDomainAllowList) == 0 && + validation.IsEmailDomainListed(setting.Service.EmailDomainBlockList, email) { + return ErrEmailInvalid{email} + } + + // if there is an allow list, then check email against allow list + if len(setting.Service.EmailDomainAllowList) > 0 && + !validation.IsEmailDomainListed(setting.Service.EmailDomainAllowList, email) { + return ErrEmailInvalid{email} + } + + return nil +} diff --git a/models/user/user.go b/models/user/user.go index 710fab9f4e..6d8eea6585 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -598,6 +598,16 @@ type CreateUserOverwriteOptions struct { // CreateUser creates record of a new user. func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { + return createUser(ctx, u, false, overwriteDefault...) +} + +// AdminCreateUser is used by admins to manually create users +func AdminCreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { + return createUser(ctx, u, true, overwriteDefault...) +} + +// createUser creates record of a new user. +func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { if err = IsUsableUsername(u.Name); err != nil { return err } @@ -651,8 +661,14 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve return err } - if err := ValidateEmail(u.Email); err != nil { - return err + if createdByAdmin { + if err := ValidateEmailForAdmin(u.Email); err != nil { + return err + } + } else { + if err := ValidateEmail(u.Email); err != nil { + return err + } } ctx, committer, err := db.TxContext(ctx) diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 64315108b0..7f4200f684 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -133,7 +133,7 @@ func CreateUser(ctx *context.APIContext) { u.UpdatedUnix = u.CreatedUnix } - if err := user_model.CreateUser(ctx, u, overwriteDefault); err != nil { + if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil { if user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err) || db.IsErrNameReserved(err) || diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index a34e0d0f0d..dfb103c8ab 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -177,7 +177,7 @@ func NewUserPost(ctx *context.Context) { u.MustChangePassword = form.MustChangePassword } - if err := user_model.CreateUser(ctx, u, overwriteDefault); err != nil { + if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil { switch { case user_model.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 0748a75ba4..53bdd11afd 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -14,9 +14,11 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" + "github.com/gobwas/glob" "github.com/stretchr/testify/assert" ) @@ -333,3 +335,27 @@ func TestAPICron(t *testing.T) { } }) } + +func TestAPICreateUser_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) + + req := NewRequestWithValues(t, "POST", "/api/v1/admin/users", map[string]string{ + "email": "allowedUser1@example1.org", + "login_name": "allowedUser1", + "username": "allowedUser1", + "password": "allowedUser1_pass", + "must_change_password": "true", + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + + req = NewRequest(t, "DELETE", "/api/v1/admin/users/allowedUser1").AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) +}