From a7e96aae66169f5d6f502710aeeaa563eb856534 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 26 Jul 2024 19:26:44 +0200 Subject: [PATCH] [SEC] Notify owner about TOTP enrollment - In the spirit of #4635 - Notify the owner when their account is getting enrolled into TOTP. The message is changed according if they have security keys or not. - Integration test added. --- models/unittest/unit_tests.go | 7 +++ options/locale/locale_en-US.ini | 4 ++ routers/web/user/setting/security/2fa.go | 5 ++ services/mailer/mail.go | 34 +++++++++++ templates/mail/auth/totp_enrolled.tmpl | 15 +++++ tests/integration/user_test.go | 72 ++++++++++++++++++++++++ 6 files changed, 137 insertions(+) create mode 100644 templates/mail/auth/totp_enrolled.tmpl diff --git a/models/unittest/unit_tests.go b/models/unittest/unit_tests.go index 75898436fc..00c7dd8453 100644 --- a/models/unittest/unit_tests.go +++ b/models/unittest/unit_tests.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "xorm.io/builder" ) @@ -130,6 +131,12 @@ func AssertSuccessfulInsert(t assert.TestingT, beans ...any) { assert.NoError(t, err) } +// AssertSuccessfulDelete assert that beans is successfully deleted +func AssertSuccessfulDelete(t require.TestingT, beans ...any) { + err := db.DeleteBeans(db.DefaultContext, beans...) + require.NoError(t, err) +} + // AssertCount assert the count of a bean func AssertCount(t assert.TestingT, bean, expected any) bool { return assert.EqualValues(t, expected, GetCount(t, bean)) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4720207a68..5bca75a1b4 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -517,6 +517,10 @@ removed_security_key.no_2fa = There are no other 2FA methods configured anymore, account_security_caution.text_1 = If this was you, then you can safely ignore this mail. account_security_caution.text_2 = If this wasn't you, your account is compromised. Please contact the admins of this site. +totp_enrolled.subject = You have activated TOTP as 2FA method +totp_enrolled.text_1.no_webauthn = You have just enabled TOTP for your account. This means that for all future logins to your account, you must use TOTP as a 2FA method. +totp_enrolled.text_1.has_webauthn = You have just enabled TOTP for your account. This means that for all future logins to your account, you could use TOTP as a 2FA method or use any of your security keys. + register_success = Registration successful issue_assigned.pull = @%[1]s assigned you to pull request %[2]s in repository %[3]s. diff --git a/routers/web/user/setting/security/2fa.go b/routers/web/user/setting/security/2fa.go index 1b1d385af8..a145867ea4 100644 --- a/routers/web/user/setting/security/2fa.go +++ b/routers/web/user/setting/security/2fa.go @@ -243,6 +243,11 @@ func EnrollTwoFactorPost(ctx *context.Context) { log.Error("Unable to save changes to the session: %v", err) } + if err := mailer.SendTOTPEnrolled(ctx, ctx.Doer); err != nil { + ctx.ServerError("SendTOTPEnrolled", err) + return + } + if err = auth.NewTwoFactor(ctx, t); err != nil { // FIXME: We need to handle a unique constraint fail here it's entirely possible that another request has beaten us. // If there is a unique constraint fail we should just tolerate the error diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 87df3d1397..01ab84bcf5 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -44,6 +44,7 @@ const ( mailAuthPrimaryMailChange base.TplName = "auth/primary_mail_change" mailAuth2faDisabled base.TplName = "auth/2fa_disabled" mailAuthRemovedSecurityKey base.TplName = "auth/removed_security_key" + mailAuthTOTPEnrolled base.TplName = "auth/totp_enrolled" mailNotifyCollaborator base.TplName = "notify/collaborator" @@ -696,3 +697,36 @@ func SendRemovedSecurityKey(ctx context.Context, u *user_model.User, securityKey SendAsync(msg) return nil } + +// SendTOTPEnrolled informs the user that they've been enrolled into TOTP. +func SendTOTPEnrolled(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(mailAuthTOTPEnrolled), data); err != nil { + return err + } + + msg := NewMessage(u.EmailTo(), locale.TrString("mail.totp_enrolled.subject"), content.String()) + msg.Info = fmt.Sprintf("UID: %d, enrolled into TOTP notification", u.ID) + + SendAsync(msg) + return nil +} diff --git a/templates/mail/auth/totp_enrolled.tmpl b/templates/mail/auth/totp_enrolled.tmpl new file mode 100644 index 0000000000..9c665e028c --- /dev/null +++ b/templates/mail/auth/totp_enrolled.tmpl @@ -0,0 +1,15 @@ + + + + + + + + +

{{.locale.Tr "mail.hi_user_x" (.DisplayName|DotEscape)}}


+ {{if .HasWebAuthn}}

{{.locale.Tr "mail.totp_enrolled.text_1.has_webauthn"}}

{{else}}

{{.locale.Tr "mail.totp_enrolled.text_1.no_webauthn"}}

{{end}}
+

{{.locale.Tr "mail.account_security_caution.text_1"}}


+

{{.locale.Tr "mail.account_security_caution.text_2"}}


+ {{template "common/footer_simple" .}} + + diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 91de3ece31..baf8cee82f 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" "testing" + "time" auth_model "code.gitea.io/gitea/models/auth" issues_model "code.gitea.io/gitea/models/issues" @@ -21,10 +22,13 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" + gitea_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/mailer" "code.gitea.io/gitea/tests" + "github.com/pquerna/otp/totp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestViewUser(t *testing.T) { @@ -747,3 +751,71 @@ func TestUserSecurityKeyMail(t *testing.T) { unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{UserID: user.ID, Name: "Little Bobby Tables's evil key"}) }) } + +func TestUserTOTPEnrolled(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user.Name) + + enrollTOTP := func(t *testing.T) { + t.Helper() + + req := NewRequest(t, "GET", "/user/settings/security/two_factor/enroll") + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + totpSecretKey, has := htmlDoc.Find(".twofa img[src^='data:image/png;base64']").Attr("alt") + assert.True(t, has) + + currentTOTP, err := totp.GenerateCode(totpSecretKey, time.Now()) + require.NoError(t, err) + + req = NewRequestWithValues(t, "POST", "/user/settings/security/two_factor/enroll", map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "passcode": currentTOTP, + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Contains(t, flashCookie.Value, "success%3DYour%2Baccount%2Bhas%2Bbeen%2Bsuccessfully%2Benrolled.") + + unittest.AssertSuccessfulDelete(t, &auth_model.TwoFactor{UID: user.ID}) + } + + t.Run("No WebAuthn enabled", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + called := false + defer test.MockVariableValue(&mailer.SendAsync, 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.totp_enrolled.subject"), msgs[0].Subject) + assert.Contains(t, msgs[0].Body, translation.NewLocale("en-US").Tr("mail.totp_enrolled.text_1.no_webauthn")) + called = true + })() + + enrollTOTP(t) + + assert.True(t, called) + }) + + t.Run("With WebAuthn enabled", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + called := false + defer test.MockVariableValue(&mailer.SendAsync, 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.totp_enrolled.subject"), msgs[0].Subject) + assert.Contains(t, msgs[0].Body, translation.NewLocale("en-US").Tr("mail.totp_enrolled.text_1.has_webauthn")) + called = true + })() + + unittest.AssertSuccessfulInsert(t, &auth_model.WebAuthnCredential{UserID: user.ID, Name: "Cueball's primary key"}) + enrollTOTP(t) + + assert.True(t, called) + }) +}