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`.

models/user/email_address.go

@@ -154,37 +154,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 {
+	if err := validateEmailBasic(email); err != nil {
-		return ErrEmailInvalid{email}
+		return err
 	}
+	return validateEmailDomain(email)
+}
 
-	if !emailRegexp.MatchString(email) {
+// ValidateEmailForAdmin check if email is a valid address when admins manually add users
-		return ErrEmailCharIsNotSupported{email}
+func ValidateEmailForAdmin(email string) error {
-	}
+	return validateEmailBasic(email)
-
+	// In this case we do not need to check the email domain
-	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
 }
 
 func GetEmailAddressByEmail(ctx context.Context, email string) (*EmailAddress, error) {
@@ -534,3 +515,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
+}

models/user/user.go

@@ -586,6 +586,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
 	}
@@ -639,9 +649,15 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve
 		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)
 	if err != nil {

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) ||

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

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)
+}