Skip to content

Commit 4fd9c56

Browse files
authored
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`.
1 parent 7e8c1c5 commit 4fd9c56

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
@@ -154,37 +154,18 @@ func UpdateEmailAddress(ctx context.Context, email *EmailAddress) error {
154154

155155
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])?)*$")
156156

157-
// ValidateEmail check if email is a allowed address
157+
// ValidateEmail check if email is a valid & allowed address
158158
func ValidateEmail(email string) error {
159-
if len(email) == 0 {
160-
return ErrEmailInvalid{email}
161-
}
162-
163-
if !emailRegexp.MatchString(email) {
164-
return ErrEmailCharIsNotSupported{email}
165-
}
166-
167-
if email[0] == '-' {
168-
return ErrEmailInvalid{email}
169-
}
170-
171-
if _, err := mail.ParseAddress(email); err != nil {
172-
return ErrEmailInvalid{email}
173-
}
174-
175-
// if there is no allow list, then check email against block list
176-
if len(setting.Service.EmailDomainAllowList) == 0 &&
177-
validation.IsEmailDomainListed(setting.Service.EmailDomainBlockList, email) {
178-
return ErrEmailInvalid{email}
179-
}
180-
181-
// if there is an allow list, then check email against allow list
182-
if len(setting.Service.EmailDomainAllowList) > 0 &&
183-
!validation.IsEmailDomainListed(setting.Service.EmailDomainAllowList, email) {
184-
return ErrEmailInvalid{email}
159+
if err := validateEmailBasic(email); err != nil {
160+
return err
185161
}
162+
return validateEmailDomain(email)
163+
}
186164

187-
return nil
165+
// ValidateEmailForAdmin check if email is a valid address when admins manually add users
166+
func ValidateEmailForAdmin(email string) error {
167+
return validateEmailBasic(email)
168+
// In this case we do not need to check the email domain
188169
}
189170

190171
func GetEmailAddressByEmail(ctx context.Context, email string) (*EmailAddress, error) {
@@ -534,3 +515,41 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate
534515

535516
return committer.Commit()
536517
}
518+
519+
// validateEmailBasic checks whether the email complies with the rules
520+
func validateEmailBasic(email string) error {
521+
if len(email) == 0 {
522+
return ErrEmailInvalid{email}
523+
}
524+
525+
if !emailRegexp.MatchString(email) {
526+
return ErrEmailCharIsNotSupported{email}
527+
}
528+
529+
if email[0] == '-' {
530+
return ErrEmailInvalid{email}
531+
}
532+
533+
if _, err := mail.ParseAddress(email); err != nil {
534+
return ErrEmailInvalid{email}
535+
}
536+
537+
return nil
538+
}
539+
540+
// validateEmailDomain checks whether the email domain is allowed or blocked
541+
func validateEmailDomain(email string) error {
542+
// if there is no allow list, then check email against block list
543+
if len(setting.Service.EmailDomainAllowList) == 0 &&
544+
validation.IsEmailDomainListed(setting.Service.EmailDomainBlockList, email) {
545+
return ErrEmailInvalid{email}
546+
}
547+
548+
// if there is an allow list, then check email against allow list
549+
if len(setting.Service.EmailDomainAllowList) > 0 &&
550+
!validation.IsEmailDomainListed(setting.Service.EmailDomainAllowList, email) {
551+
return ErrEmailInvalid{email}
552+
}
553+
554+
return nil
555+
}

models/user/user.go

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

587587
// CreateUser creates record of a new user.
588588
func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) {
589+
return createUser(ctx, u, false, overwriteDefault...)
590+
}
591+
592+
// AdminCreateUser is used by admins to manually create users
593+
func AdminCreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) {
594+
return createUser(ctx, u, true, overwriteDefault...)
595+
}
596+
597+
// createUser creates record of a new user.
598+
func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefault ...*CreateUserOverwriteOptions) (err error) {
589599
if err = IsUsableUsername(u.Name); err != nil {
590600
return err
591601
}
@@ -639,8 +649,14 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve
639649
return err
640650
}
641651

642-
if err := ValidateEmail(u.Email); err != nil {
643-
return err
652+
if createdByAdmin {
653+
if err := ValidateEmailForAdmin(u.Email); err != nil {
654+
return err
655+
}
656+
} else {
657+
if err := ValidateEmail(u.Email); err != nil {
658+
return err
659+
}
644660
}
645661

646662
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
@@ -133,7 +133,7 @@ func CreateUser(ctx *context.APIContext) {
133133
u.UpdatedUnix = u.CreatedUnix
134134
}
135135

136-
if err := user_model.CreateUser(ctx, u, overwriteDefault); err != nil {
136+
if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil {
137137
if user_model.IsErrUserAlreadyExist(err) ||
138138
user_model.IsErrEmailAlreadyUsed(err) ||
139139
db.IsErrNameReserved(err) ||

routers/web/admin/users.go

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

180-
if err := user_model.CreateUser(ctx, u, overwriteDefault); err != nil {
180+
if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil {
181181
switch {
182182
case user_model.IsErrUserAlreadyExist(err):
183183
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

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

0 commit comments

Comments
 (0)