Skip to content

Skip email domain check when admin users adds user manually #29522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 5, 2024
75 changes: 47 additions & 28 deletions models/user/email_address.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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}
}

// 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}
if err := validateEmailBasic(email); err != nil {
return err
}
return validateEmailDomain(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) {
Expand Down Expand Up @@ -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
}
20 changes: 18 additions & 2 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -639,8 +649,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)
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/admin/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Expand Down
2 changes: 1 addition & 1 deletion routers/web/admin/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions tests/integration/api_admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
Loading