1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-08-01 17:38:33 +00:00

[SECURITY] Notify users about account security changes

- Currently if the password, primary mail, TOTP or security keys are
changed, no notification is made of that and makes compromising an
account a bit easier as it's essentially undetectable until the original
person tries to log in. Although other changes should be made as
well (re-authing before allowing a password change), this should go a
long way of improving the account security in Forgejo.
- Adds a mail notification for password and primary mail changes. For
the primary mail change, a mail notification is sent to the old primary
mail.
- Add a mail notification when TOTP or a security keys is removed, if no
other 2FA method is configured the mail will also contain that 2FA is
no longer needed to log into their account.
- `MakeEmailAddressPrimary` is refactored to the user service package,
as it now involves calling the mailer service.
- Unit tests added.
- Integration tests added.
This commit is contained in:
Gusted 2024-07-23 00:17:06 +02:00
parent ded237ee77
commit 4383da91bd
No known key found for this signature in database
GPG key ID: FD821B732837125F
24 changed files with 543 additions and 116 deletions

View file

@ -17,6 +17,7 @@ import (
"time"
activities_model "code.gitea.io/gitea/models/activities"
auth_model "code.gitea.io/gitea/models/auth"
issues_model "code.gitea.io/gitea/models/issues"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
@ -35,10 +36,14 @@ import (
)
const (
mailAuthActivate base.TplName = "auth/activate"
mailAuthActivateEmail base.TplName = "auth/activate_email"
mailAuthResetPassword base.TplName = "auth/reset_passwd"
mailAuthRegisterNotify base.TplName = "auth/register_notify"
mailAuthActivate base.TplName = "auth/activate"
mailAuthActivateEmail base.TplName = "auth/activate_email"
mailAuthResetPassword base.TplName = "auth/reset_passwd"
mailAuthRegisterNotify base.TplName = "auth/register_notify"
mailAuthPasswordChange base.TplName = "auth/password_change"
mailAuthPrimaryMailChange base.TplName = "auth/primary_mail_change"
mailAuth2faDisabled base.TplName = "auth/2fa_disabled"
mailAuthRemovedSecurityKey base.TplName = "auth/removed_security_key"
mailNotifyCollaborator base.TplName = "notify/collaborator"
@ -561,3 +566,133 @@ func fromDisplayName(u *user_model.User) string {
}
return u.GetCompleteName()
}
// SendPasswordChange informs the user on their primary email address that
// their password was changed.
func SendPasswordChange(u *user_model.User) error {
if setting.MailService == nil {
return nil
}
locale := translation.NewLocale(u.Language)
data := map[string]any{
"locale": locale,
"DisplayName": u.DisplayName(),
"Username": u.Name,
"Language": locale.Language(),
}
var content bytes.Buffer
if err := bodyTemplates.ExecuteTemplate(&content, string(mailAuthPasswordChange), data); err != nil {
return err
}
msg := NewMessage(u.EmailTo(), locale.TrString("mail.password_change.subject"), content.String())
msg.Info = fmt.Sprintf("UID: %d, password change notification", u.ID)
SendAsync(msg)
return nil
}
// SendPrimaryMailChange informs the user on their old primary email address
// that it's no longer used as primary mail and will no longer receive
// notification on that email address.
func SendPrimaryMailChange(u *user_model.User, oldPrimaryEmail string) error {
if setting.MailService == nil {
return nil
}
locale := translation.NewLocale(u.Language)
data := map[string]any{
"locale": locale,
"NewPrimaryMail": u.Email,
"DisplayName": u.DisplayName(),
"Username": u.Name,
"Language": locale.Language(),
}
var content bytes.Buffer
if err := bodyTemplates.ExecuteTemplate(&content, string(mailAuthPrimaryMailChange), data); err != nil {
return err
}
msg := NewMessage(u.EmailTo(oldPrimaryEmail), locale.TrString("mail.primary_mail_change.subject"), content.String())
msg.Info = fmt.Sprintf("UID: %d, primary email change notification", u.ID)
SendAsync(msg)
return nil
}
// SendDisabledTOTP informs the user that their totp has been disabled.
func SendDisabledTOTP(ctx context.Context, u *user_model.User) error {
if setting.MailService == nil {
return nil
}
locale := translation.NewLocale(u.Language)
hasWebAuthn, err := auth_model.HasWebAuthnRegistrationsByUID(ctx, u.ID)
if err != nil {
return err
}
data := map[string]any{
"locale": locale,
"HasWebAuthn": hasWebAuthn,
"DisplayName": u.DisplayName(),
"Username": u.Name,
"Language": locale.Language(),
}
var content bytes.Buffer
if err := bodyTemplates.ExecuteTemplate(&content, string(mailAuth2faDisabled), data); err != nil {
return err
}
msg := NewMessage(u.EmailTo(), locale.TrString("mail.totp_disabled.subject"), content.String())
msg.Info = fmt.Sprintf("UID: %d, 2fa disabled notification", u.ID)
SendAsync(msg)
return nil
}
// SendRemovedWebAuthn informs the user that one of their security keys has been removed.
func SendRemovedSecurityKey(ctx context.Context, u *user_model.User, securityKeyName string) error {
if setting.MailService == nil {
return nil
}
locale := translation.NewLocale(u.Language)
hasWebAuthn, err := auth_model.HasWebAuthnRegistrationsByUID(ctx, u.ID)
if err != nil {
return err
}
hasTOTP, err := auth_model.HasTwoFactorByUID(ctx, u.ID)
if err != nil {
return err
}
data := map[string]any{
"locale": locale,
"HasWebAuthn": hasWebAuthn,
"HasTOTP": hasTOTP,
"SecurityKeyName": securityKeyName,
"DisplayName": u.DisplayName(),
"Username": u.Name,
"Language": locale.Language(),
}
var content bytes.Buffer
if err := bodyTemplates.ExecuteTemplate(&content, string(mailAuthRemovedSecurityKey), data); err != nil {
return err
}
msg := NewMessage(u.EmailTo(), locale.TrString("mail.removed_security_key.subject"), content.String())
msg.Info = fmt.Sprintf("UID: %d, security key removed notification", u.ID)
SendAsync(msg)
return nil
}

View file

@ -55,14 +55,14 @@ func TestAdminNotificationMail_test(t *testing.T) {
defer test.MockVariableValue(&setting.Admin.SendNotificationEmailOnNewUser, true)()
called := false
defer mockMailSettings(func(msgs ...*Message) {
defer MockMailSettings(func(msgs ...*Message) {
assert.Equal(t, len(msgs), 1, "Test provides only one admin user, so only one email must be sent")
assert.Equal(t, msgs[0].To, users[0].Email, "checks if the recipient is the admin of the instance")
manageUserURL := setting.AppURL + "admin/users/" + strconv.FormatInt(users[1].ID, 10)
assert.Contains(t, msgs[0].Body, manageUserURL)
assert.Contains(t, msgs[0].Body, users[1].HTMLURL())
assert.Contains(t, msgs[0].Body, users[1].Name, "user name of the newly created user")
assertTranslatedLocale(t, msgs[0].Body, "mail.admin", "admin.users")
AssertTranslatedLocale(t, msgs[0].Body, "mail.admin", "admin.users")
called = true
})()
MailNewUser(ctx, users[1])
@ -71,7 +71,7 @@ func TestAdminNotificationMail_test(t *testing.T) {
t.Run("SendNotificationEmailOnNewUser_false", func(t *testing.T) {
defer test.MockVariableValue(&setting.Admin.SendNotificationEmailOnNewUser, false)()
defer mockMailSettings(func(msgs ...*Message) {
defer MockMailSettings(func(msgs ...*Message) {
assert.Equal(t, 1, 0, "this shouldn't execute. MailNewUser must exit early since SEND_NOTIFICATION_EMAIL_ON_NEW_USER is disabled")
})()
MailNewUser(ctx, users[1])

View file

@ -0,0 +1,60 @@
// Copyright 2024 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package mailer_test
import (
"testing"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/translation"
"code.gitea.io/gitea/services/mailer"
user_service "code.gitea.io/gitea/services/user"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPasswordChangeMail(t *testing.T) {
defer require.NoError(t, unittest.PrepareTestDatabase())
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
called := false
defer mailer.MockMailSettings(func(msgs ...*mailer.Message) {
assert.Len(t, msgs, 1)
assert.Equal(t, user.EmailTo(), msgs[0].To)
assert.EqualValues(t, translation.NewLocale("en-US").Tr("mail.password_change.subject"), msgs[0].Subject)
mailer.AssertTranslatedLocale(t, msgs[0].Body, "mail.password_change.text_1", "mail.password_change.text_2", "mail.password_change.text_3")
called = true
})()
require.NoError(t, user_service.UpdateAuth(db.DefaultContext, user, &user_service.UpdateAuthOptions{Password: optional.Some("NewPasswordYolo!")}))
assert.True(t, called)
}
func TestPrimaryMailChange(t *testing.T) {
defer require.NoError(t, unittest.PrepareTestDatabase())
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
firstEmail := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 3, UID: user.ID, IsPrimary: true})
secondEmail := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 35, UID: user.ID}, "is_primary = false")
called := false
defer mailer.MockMailSettings(func(msgs ...*mailer.Message) {
assert.False(t, called)
assert.Len(t, msgs, 1)
assert.Equal(t, user.EmailTo(firstEmail.Email), msgs[0].To)
assert.EqualValues(t, translation.NewLocale("en-US").Tr("mail.primary_mail_change.subject"), msgs[0].Subject)
assert.Contains(t, msgs[0].Body, secondEmail.Email)
mailer.AssertTranslatedLocale(t, msgs[0].Body, "mail.primary_mail_change.text_1", "mail.primary_mail_change.text_2", "mail.primary_mail_change.text_3")
called = true
})()
require.NoError(t, user_service.MakeEmailAddressPrimary(db.DefaultContext, user, secondEmail, true))
assert.True(t, called)
require.NoError(t, user_service.MakeEmailAddressPrimary(db.DefaultContext, user, firstEmail, false))
}

View file

@ -62,7 +62,7 @@ func prepareMailerTest(t *testing.T) (doer *user_model.User, repo *repo_model.Re
}
func TestComposeIssueCommentMessage(t *testing.T) {
defer mockMailSettings(nil)()
defer MockMailSettings(nil)()
doer, _, issue, comment := prepareMailerTest(t)
markup.Init(&markup.ProcessorHelper{
@ -117,7 +117,7 @@ func TestComposeIssueCommentMessage(t *testing.T) {
}
func TestComposeIssueMessage(t *testing.T) {
defer mockMailSettings(nil)()
defer MockMailSettings(nil)()
doer, _, issue, _ := prepareMailerTest(t)
recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}, {Name: "Test2", Email: "test2@gitea.com"}}
@ -146,7 +146,7 @@ func TestComposeIssueMessage(t *testing.T) {
}
func TestMailerIssueTemplate(t *testing.T) {
defer mockMailSettings(nil)()
defer MockMailSettings(nil)()
assert.NoError(t, unittest.PrepareTestDatabase())
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
@ -160,7 +160,7 @@ func TestMailerIssueTemplate(t *testing.T) {
for _, s := range expected {
assert.Contains(t, wholemsg, s)
}
assertTranslatedLocale(t, wholemsg, "mail.issue")
AssertTranslatedLocale(t, wholemsg, "mail.issue")
}
testCompose := func(t *testing.T, ctx *mailCommentContext) *Message {
@ -241,7 +241,7 @@ func TestMailerIssueTemplate(t *testing.T) {
}
func TestTemplateSelection(t *testing.T) {
defer mockMailSettings(nil)()
defer MockMailSettings(nil)()
doer, repo, issue, comment := prepareMailerTest(t)
recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}}
@ -296,7 +296,7 @@ func TestTemplateSelection(t *testing.T) {
}
func TestTemplateServices(t *testing.T) {
defer mockMailSettings(nil)()
defer MockMailSettings(nil)()
doer, _, issue, comment := prepareMailerTest(t)
assert.NoError(t, issue.LoadRepo(db.DefaultContext))
@ -349,7 +349,7 @@ func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, recip
}
func TestGenerateAdditionalHeaders(t *testing.T) {
defer mockMailSettings(nil)()
defer MockMailSettings(nil)()
doer, _, issue, _ := prepareMailerTest(t)
ctx := &mailCommentContext{Context: context.TODO() /* TODO: use a correct context */, Issue: issue, Doer: doer}
@ -382,7 +382,7 @@ func TestGenerateAdditionalHeaders(t *testing.T) {
}
func Test_createReference(t *testing.T) {
defer mockMailSettings(nil)()
defer MockMailSettings(nil)()
_, _, issue, comment := prepareMailerTest(t)
_, _, pullIssue, _ := prepareMailerTest(t)
pullIssue.IsPull = true

View file

@ -22,14 +22,14 @@ func TestMain(m *testing.M) {
unittest.MainTest(m)
}
func assertTranslatedLocale(t *testing.T, message string, prefixes ...string) {
func AssertTranslatedLocale(t *testing.T, message string, prefixes ...string) {
t.Helper()
for _, prefix := range prefixes {
assert.NotContains(t, message, prefix, "there is an untranslated locale prefix")
}
}
func mockMailSettings(send func(msgs ...*Message)) func() {
func MockMailSettings(send func(msgs ...*Message)) func() {
translation.InitLocales(context.Background())
subjectTemplates, bodyTemplates = templates.Mailer(context.Background())
mailService := setting.Mailer{