Skip to content

Commit 421a76f

Browse files
committed
Skip email domain check when admin users adds user manually (go-gitea#29522)
Fix go-gitea#27457 Administrators should be able to manually create any user even if the user's email address is not in `EMAIL_DOMAIN_ALLOWLIST`.
1 parent 4ef7e49 commit 421a76f

File tree

5 files changed

+93
-32
lines changed

5 files changed

+93
-32
lines changed

models/user/email_address.go

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -144,37 +144,18 @@ func (email *EmailAddress) BeforeInsert() {
144144

145145
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])?)*$")
146146

147-
// ValidateEmail check if email is a allowed address
147+
// ValidateEmail check if email is a valid & allowed address
148148
func ValidateEmail(email string) error {
149-
if len(email) == 0 {
150-
return nil
151-
}
152-
153-
if !emailRegexp.MatchString(email) {
154-
return ErrEmailCharIsNotSupported{email}
155-
}
156-
157-
if email[0] == '-' {
158-
return ErrEmailInvalid{email}
159-
}
160-
161-
if _, err := mail.ParseAddress(email); err != nil {
162-
return ErrEmailInvalid{email}
163-
}
164-
165-
// if there is no allow list, then check email against block list
166-
if len(setting.Service.EmailDomainAllowList) == 0 &&
167-
validation.IsEmailDomainListed(setting.Service.EmailDomainBlockList, email) {
168-
return ErrEmailInvalid{email}
169-
}
170-
171-
// if there is an allow list, then check email against allow list
172-
if len(setting.Service.EmailDomainAllowList) > 0 &&
173-
!validation.IsEmailDomainListed(setting.Service.EmailDomainAllowList, email) {
174-
return ErrEmailInvalid{email}
149+
if err := validateEmailBasic(email); err != nil {
150+
return err
175151
}
152+
return validateEmailDomain(email)
153+
}
176154

177-
return nil
155+
// ValidateEmailForAdmin check if email is a valid address when admins manually add users
156+
func ValidateEmailForAdmin(email string) error {
157+
return validateEmailBasic(email)
158+
// In this case we do not need to check the email domain
178159
}
179160

180161
// GetEmailAddresses returns all email addresses belongs to given user.
@@ -570,3 +551,41 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate
570551

571552
return committer.Commit()
572553
}
554+
555+
// validateEmailBasic checks whether the email complies with the rules
556+
func validateEmailBasic(email string) error {
557+
if len(email) == 0 {
558+
return ErrEmailInvalid{email}
559+
}
560+
561+
if !emailRegexp.MatchString(email) {
562+
return ErrEmailCharIsNotSupported{email}
563+
}
564+
565+
if email[0] == '-' {
566+
return ErrEmailInvalid{email}
567+
}
568+
569+
if _, err := mail.ParseAddress(email); err != nil {
570+
return ErrEmailInvalid{email}
571+
}
572+
573+
return nil
574+
}
575+
576+
// validateEmailDomain checks whether the email domain is allowed or blocked
577+
func validateEmailDomain(email string) error {
578+
// if there is no allow list, then check email against block list
579+
if len(setting.Service.EmailDomainAllowList) == 0 &&
580+
validation.IsEmailDomainListed(setting.Service.EmailDomainBlockList, email) {
581+
return ErrEmailInvalid{email}
582+
}
583+
584+
// if there is an allow list, then check email against allow list
585+
if len(setting.Service.EmailDomainAllowList) > 0 &&
586+
!validation.IsEmailDomainListed(setting.Service.EmailDomainAllowList, email) {
587+
return ErrEmailInvalid{email}
588+
}
589+
590+
return nil
591+
}

models/user/user.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,16 @@ type CreateUserOverwriteOptions struct {
583583

584584
// CreateUser creates record of a new user.
585585
func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) {
586+
return createUser(ctx, u, false, overwriteDefault...)
587+
}
588+
589+
// AdminCreateUser is used by admins to manually create users
590+
func AdminCreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) {
591+
return createUser(ctx, u, true, overwriteDefault...)
592+
}
593+
594+
// createUser creates record of a new user.
595+
func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefault ...*CreateUserOverwriteOptions) (err error) {
586596
if err = IsUsableUsername(u.Name); err != nil {
587597
return err
588598
}
@@ -636,8 +646,14 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve
636646
return err
637647
}
638648

639-
if err := ValidateEmail(u.Email); err != nil {
640-
return err
649+
if createdByAdmin {
650+
if err := ValidateEmailForAdmin(u.Email); err != nil {
651+
return err
652+
}
653+
} else {
654+
if err := ValidateEmail(u.Email); err != nil {
655+
return err
656+
}
641657
}
642658

643659
ctx, committer, err := db.TxContext(ctx)

routers/api/v1/admin/user.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func CreateUser(ctx *context.APIContext) {
138138
u.UpdatedUnix = u.CreatedUnix
139139
}
140140

141-
if err := user_model.CreateUser(ctx, u, overwriteDefault); err != nil {
141+
if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil {
142142
if user_model.IsErrUserAlreadyExist(err) ||
143143
user_model.IsErrEmailAlreadyUsed(err) ||
144144
db.IsErrNameReserved(err) ||

routers/web/admin/users.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func NewUserPost(ctx *context.Context) {
169169
u.MustChangePassword = form.MustChangePassword
170170
}
171171

172-
if err := user_model.CreateUser(ctx, u, overwriteDefault); err != nil {
172+
if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil {
173173
switch {
174174
case user_model.IsErrUserAlreadyExist(err):
175175
ctx.Data["Err_UserName"] = true

tests/integration/api_admin_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ import (
1414
"code.gitea.io/gitea/models/unittest"
1515
user_model "code.gitea.io/gitea/models/user"
1616
"code.gitea.io/gitea/modules/json"
17+
"code.gitea.io/gitea/modules/setting"
1718
api "code.gitea.io/gitea/modules/structs"
1819
"code.gitea.io/gitea/tests"
1920

21+
"github.com/gobwas/glob"
2022
"github.com/stretchr/testify/assert"
2123
)
2224

@@ -332,3 +334,27 @@ func TestAPICron(t *testing.T) {
332334
}
333335
})
334336
}
337+
338+
func TestAPICreateUser_NotAllowedEmailDomain(t *testing.T) {
339+
defer tests.PrepareTestEnv(t)()
340+
341+
setting.Service.EmailDomainAllowList = []glob.Glob{glob.MustCompile("example.org")}
342+
defer func() {
343+
setting.Service.EmailDomainAllowList = []glob.Glob{}
344+
}()
345+
346+
adminUsername := "user1"
347+
token := getUserToken(t, adminUsername, auth_model.AccessTokenScopeWriteAdmin)
348+
349+
req := NewRequestWithValues(t, "POST", "/api/v1/admin/users", map[string]string{
350+
"email": "[email protected]",
351+
"login_name": "allowedUser1",
352+
"username": "allowedUser1",
353+
"password": "allowedUser1_pass",
354+
"must_change_password": "true",
355+
}).AddTokenAuth(token)
356+
MakeRequest(t, req, http.StatusCreated)
357+
358+
req = NewRequest(t, "DELETE", "/api/v1/admin/users/allowedUser1").AddTokenAuth(token)
359+
MakeRequest(t, req, http.StatusNoContent)
360+
}

0 commit comments

Comments
 (0)