diff --git a/modules/validation/email.go b/modules/validation/email.go index 8e1ffc203d..7960a80a1f 100644 --- a/modules/validation/email.go +++ b/modules/validation/email.go @@ -72,16 +72,23 @@ func validateEmailBasic(email string) error { } func validateEmailDomain(email string) error { - if !IsEmailDomainAllowed(email) { + if _, ok := IsEmailDomainAllowed(email); !ok { return ErrEmailInvalid{email} } return nil } -func IsEmailDomainAllowed(email string) bool { - return isEmailDomainAllowedInternal( - email, +func IsEmailDomainAllowed(email string) (validEmail, ok bool) { + // Normalized the address. This strips for example comments which could be + // used to smuggle a different domain + parsedAddress, err := mail.ParseAddress(email) + if err != nil { + return false, false + } + + return true, isEmailDomainAllowedInternal( + parsedAddress.Address, setting.Service.EmailDomainAllowList, setting.Service.EmailDomainBlockList) } diff --git a/modules/validation/email_test.go b/modules/validation/email_test.go index b7ee766ddb..28158cae53 100644 --- a/modules/validation/email_test.go +++ b/modules/validation/email_test.go @@ -67,8 +67,3 @@ func TestEmailAddressValidate(t *testing.T) { }) } } - -func TestEmailDomainAllowList(t *testing.T) { - res := IsEmailDomainAllowed("someuser@localhost.localdomain") - assert.True(t, res) -} diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index f3e321a047..6bc520f510 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -149,7 +149,7 @@ func CreateUser(ctx *context.APIContext) { return } - if !validation.IsEmailDomainAllowed(u.Email) { + if _, ok := validation.IsEmailDomainAllowed(u.Email); !ok { ctx.Resp.Header().Add("X-Gitea-Warning", fmt.Sprintf("the domain of user email %s conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", u.Email)) } @@ -235,7 +235,7 @@ func EditUser(ctx *context.APIContext) { return } - if !validation.IsEmailDomainAllowed(*form.Email) { + if _, ok := validation.IsEmailDomainAllowed(*form.Email); !ok { ctx.Resp.Header().Add("X-Gitea-Warning", fmt.Sprintf("the domain of user email %s conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", *form.Email)) } } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 964326291e..c4727177ba 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -201,7 +201,7 @@ func NewUserPost(ctx *context.Context) { return } - if !validation.IsEmailDomainAllowed(u.Email) { + if _, ok := validation.IsEmailDomainAllowed(u.Email); !ok { ctx.Flash.Warning(ctx.Tr("form.email_domain_is_not_allowed", u.Email)) } @@ -421,7 +421,7 @@ func EditUserPost(ctx *context.Context) { } return } - if !validation.IsEmailDomainAllowed(form.Email) { + if _, ok := validation.IsEmailDomainAllowed(form.Email); !ok { ctx.Flash.Warning(ctx.Tr("form.email_domain_is_not_allowed", form.Email)) } } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index daf3373dec..3b4e62c429 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -453,7 +453,10 @@ func SignUpPost(ctx *context.Context) { return } - if !form.IsEmailDomainAllowed() { + if emailValid, ok := form.IsEmailDomainAllowed(); !emailValid { + ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, form) + return + } else if !ok { ctx.RenderWithErr(ctx.Tr("auth.email_domain_blacklisted"), tplSignUp, &form) return } diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index 031998d3a1..d2cb0d29c7 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -226,7 +226,10 @@ func LinkAccountPostRegister(ctx *context.Context) { } } - if !form.IsEmailDomainAllowed() { + if emailValid, ok := form.IsEmailDomainAllowed(); !emailValid { + ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, form) + return + } else if !ok { ctx.RenderWithErr(ctx.Tr("auth.email_domain_blacklisted"), tplLinkAccount, &form) return } diff --git a/services/forms/user_form.go b/services/forms/user_form.go index dfd5b3da9b..8c95139e2c 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -109,7 +109,7 @@ func (f *RegisterForm) Validate(req *http.Request, errs binding.Errors) binding. // The email is marked as allowed if it matches any of the // domains in the whitelist or if it doesn't match any of // domains in the blocklist, if any such list is not empty. -func (f *RegisterForm) IsEmailDomainAllowed() bool { +func (f *RegisterForm) IsEmailDomainAllowed() (validEmail, ok bool) { return validation.IsEmailDomainAllowed(f.Email) } diff --git a/services/forms/user_form_test.go b/services/forms/user_form_test.go index ae08f65f23..4ce63ab552 100644 --- a/services/forms/user_form_test.go +++ b/services/forms/user_form_test.go @@ -20,7 +20,9 @@ func TestRegisterForm_IsDomainAllowed_Empty(t *testing.T) { form := RegisterForm{} - assert.True(t, form.IsEmailDomainAllowed()) + emailValid, ok := form.IsEmailDomainAllowed() + assert.False(t, emailValid) + assert.False(t, ok) } func TestRegisterForm_IsDomainAllowed_InvalidEmail(t *testing.T) { @@ -36,7 +38,8 @@ func TestRegisterForm_IsDomainAllowed_InvalidEmail(t *testing.T) { for _, v := range tt { form := RegisterForm{Email: v.email} - assert.False(t, form.IsEmailDomainAllowed()) + _, ok := form.IsEmailDomainAllowed() + assert.False(t, ok) } } @@ -59,7 +62,8 @@ func TestRegisterForm_IsDomainAllowed_AllowedEmail(t *testing.T) { for _, v := range tt { form := RegisterForm{Email: v.email} - assert.Equal(t, v.valid, form.IsEmailDomainAllowed()) + _, ok := form.IsEmailDomainAllowed() + assert.Equal(t, v.valid, ok) } } @@ -72,7 +76,6 @@ func TestRegisterForm_IsDomainAllowed_BlockedEmail(t *testing.T) { }{ {"security@gitea.io", false}, {"security@gitea.example", true}, - {"invalid", true}, {"user@my.block", false}, {"user@my.block1", true}, @@ -81,7 +84,8 @@ func TestRegisterForm_IsDomainAllowed_BlockedEmail(t *testing.T) { for _, v := range tt { form := RegisterForm{Email: v.email} - assert.Equal(t, v.valid, form.IsEmailDomainAllowed()) + _, ok := form.IsEmailDomainAllowed() + assert.Equal(t, v.valid, ok) } } diff --git a/tests/integration/email_block_allowlist_test.go b/tests/integration/email_block_allowlist_test.go new file mode 100644 index 0000000000..449a5cbf0c --- /dev/null +++ b/tests/integration/email_block_allowlist_test.go @@ -0,0 +1,66 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "testing" + + "forgejo.org/modules/setting" + "forgejo.org/modules/test" + "forgejo.org/modules/validation" + "forgejo.org/tests" + + "github.com/gobwas/glob" + "github.com/stretchr/testify/assert" +) + +func TestEmailBlocklist(t *testing.T) { + defer test.MockVariableValue( + &setting.Service.EmailDomainBlockList, + []glob.Glob{glob.MustCompile("evil")}, + )() + + defer tests.PrepareTestEnv(t)() + + emailValid, ok := validation.IsEmailDomainAllowed("🐸@pond") + assert.True(t, emailValid) + assert.True(t, ok) + + emailValid, ok = validation.IsEmailDomainAllowed("🐸@pond (what-is-this@evil)") + assert.True(t, emailValid) + assert.True(t, ok) + + emailValid, ok = validation.IsEmailDomainAllowed("jomo@evil") + assert.True(t, emailValid) + assert.False(t, ok) + + emailValid, ok = validation.IsEmailDomainAllowed("jomo@evil (but-does-it@break)") + assert.True(t, emailValid) + assert.False(t, ok) +} + +func TestEmailAllowlist(t *testing.T) { + defer test.MockVariableValue( + &setting.Service.EmailDomainAllowList, + []glob.Glob{glob.MustCompile("pond")}, + )() + + defer tests.PrepareTestEnv(t)() + + emailValid, ok := validation.IsEmailDomainAllowed("🐸@pond") + assert.True(t, emailValid) + assert.True(t, ok) + + emailValid, ok = validation.IsEmailDomainAllowed("🐸@pond (what-is-this@evil)") + assert.True(t, emailValid) + assert.True(t, ok) + + emailValid, ok = validation.IsEmailDomainAllowed("jomo@evil") + assert.True(t, emailValid) + assert.False(t, ok) + + emailValid, ok = validation.IsEmailDomainAllowed("jomo@evil (but-does-it@break)") + assert.True(t, emailValid) + assert.False(t, ok) +}