mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-12-01 13:44:06 +01:00
Merge pull request '[SEC] Notify owner about TOTP enrollment' (#4704) from gusted/sec-more-totp into forgejo
Some checks are pending
/ release (push) Waiting to run
testing / backend-checks (push) Waiting to run
testing / frontend-checks (push) Waiting to run
testing / test-unit (push) Blocked by required conditions
testing / test-remote-cacher (map[image:docker.io/valkey/valkey:7.2.5-alpine3.19 port:6379]) (push) Blocked by required conditions
testing / test-remote-cacher (map[image:ghcr.io/microsoft/garnet-alpine:1.0.14 port:6379]) (push) Blocked by required conditions
testing / test-remote-cacher (map[image:redis:7.2 port:6379]) (push) Blocked by required conditions
testing / test-remote-cacher (map[image:registry.redict.io/redict:7.3.0-scratch port:6379]) (push) Blocked by required conditions
testing / test-mysql (push) Blocked by required conditions
testing / test-pgsql (push) Blocked by required conditions
testing / test-sqlite (push) Blocked by required conditions
testing / security-check (push) Blocked by required conditions
Some checks are pending
/ release (push) Waiting to run
testing / backend-checks (push) Waiting to run
testing / frontend-checks (push) Waiting to run
testing / test-unit (push) Blocked by required conditions
testing / test-remote-cacher (map[image:docker.io/valkey/valkey:7.2.5-alpine3.19 port:6379]) (push) Blocked by required conditions
testing / test-remote-cacher (map[image:ghcr.io/microsoft/garnet-alpine:1.0.14 port:6379]) (push) Blocked by required conditions
testing / test-remote-cacher (map[image:redis:7.2 port:6379]) (push) Blocked by required conditions
testing / test-remote-cacher (map[image:registry.redict.io/redict:7.3.0-scratch port:6379]) (push) Blocked by required conditions
testing / test-mysql (push) Blocked by required conditions
testing / test-pgsql (push) Blocked by required conditions
testing / test-sqlite (push) Blocked by required conditions
testing / security-check (push) Blocked by required conditions
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4704 Reviewed-by: 0ko <0ko@noreply.codeberg.org>
This commit is contained in:
commit
4c40bf5d29
6 changed files with 137 additions and 0 deletions
|
@ -9,6 +9,7 @@ import (
|
||||||
"code.gitea.io/gitea/models/db"
|
"code.gitea.io/gitea/models/db"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
"xorm.io/builder"
|
"xorm.io/builder"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -130,6 +131,12 @@ func AssertSuccessfulInsert(t assert.TestingT, beans ...any) {
|
||||||
assert.NoError(t, err)
|
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
|
// AssertCount assert the count of a bean
|
||||||
func AssertCount(t assert.TestingT, bean, expected any) bool {
|
func AssertCount(t assert.TestingT, bean, expected any) bool {
|
||||||
return assert.EqualValues(t, expected, GetCount(t, bean))
|
return assert.EqualValues(t, expected, GetCount(t, bean))
|
||||||
|
|
|
@ -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_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.
|
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
|
register_success = Registration successful
|
||||||
|
|
||||||
issue_assigned.pull = @%[1]s assigned you to pull request %[2]s in repository %[3]s.
|
issue_assigned.pull = @%[1]s assigned you to pull request %[2]s in repository %[3]s.
|
||||||
|
|
|
@ -243,6 +243,11 @@ func EnrollTwoFactorPost(ctx *context.Context) {
|
||||||
log.Error("Unable to save changes to the session: %v", err)
|
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 {
|
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.
|
// 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
|
// If there is a unique constraint fail we should just tolerate the error
|
||||||
|
|
|
@ -44,6 +44,7 @@ const (
|
||||||
mailAuthPrimaryMailChange base.TplName = "auth/primary_mail_change"
|
mailAuthPrimaryMailChange base.TplName = "auth/primary_mail_change"
|
||||||
mailAuth2faDisabled base.TplName = "auth/2fa_disabled"
|
mailAuth2faDisabled base.TplName = "auth/2fa_disabled"
|
||||||
mailAuthRemovedSecurityKey base.TplName = "auth/removed_security_key"
|
mailAuthRemovedSecurityKey base.TplName = "auth/removed_security_key"
|
||||||
|
mailAuthTOTPEnrolled base.TplName = "auth/totp_enrolled"
|
||||||
|
|
||||||
mailNotifyCollaborator base.TplName = "notify/collaborator"
|
mailNotifyCollaborator base.TplName = "notify/collaborator"
|
||||||
|
|
||||||
|
@ -696,3 +697,36 @@ func SendRemovedSecurityKey(ctx context.Context, u *user_model.User, securityKey
|
||||||
SendAsync(msg)
|
SendAsync(msg)
|
||||||
return nil
|
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
|
||||||
|
}
|
||||||
|
|
15
templates/mail/auth/totp_enrolled.tmpl
Normal file
15
templates/mail/auth/totp_enrolled.tmpl
Normal file
|
@ -0,0 +1,15 @@
|
||||||
|
<!DOCTYPE html>
|
||||||
|
<html>
|
||||||
|
<head>
|
||||||
|
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
|
||||||
|
<meta name="format-detection" content="telephone=no,date=no,address=no,email=no,url=no">
|
||||||
|
</head>
|
||||||
|
|
||||||
|
<body>
|
||||||
|
<p>{{.locale.Tr "mail.hi_user_x" (.DisplayName|DotEscape)}}</p><br>
|
||||||
|
{{if .HasWebAuthn}}<p>{{.locale.Tr "mail.totp_enrolled.text_1.has_webauthn"}}</p>{{else}}<p>{{.locale.Tr "mail.totp_enrolled.text_1.no_webauthn"}}</p>{{end}}<br>
|
||||||
|
<p>{{.locale.Tr "mail.account_security_caution.text_1"}}</p><br>
|
||||||
|
<p>{{.locale.Tr "mail.account_security_caution.text_2"}}</p><br>
|
||||||
|
{{template "common/footer_simple" .}}
|
||||||
|
</body>
|
||||||
|
</html>
|
|
@ -10,6 +10,7 @@ import (
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
auth_model "code.gitea.io/gitea/models/auth"
|
auth_model "code.gitea.io/gitea/models/auth"
|
||||||
issues_model "code.gitea.io/gitea/models/issues"
|
issues_model "code.gitea.io/gitea/models/issues"
|
||||||
|
@ -21,10 +22,13 @@ import (
|
||||||
api "code.gitea.io/gitea/modules/structs"
|
api "code.gitea.io/gitea/modules/structs"
|
||||||
"code.gitea.io/gitea/modules/test"
|
"code.gitea.io/gitea/modules/test"
|
||||||
"code.gitea.io/gitea/modules/translation"
|
"code.gitea.io/gitea/modules/translation"
|
||||||
|
gitea_context "code.gitea.io/gitea/services/context"
|
||||||
"code.gitea.io/gitea/services/mailer"
|
"code.gitea.io/gitea/services/mailer"
|
||||||
"code.gitea.io/gitea/tests"
|
"code.gitea.io/gitea/tests"
|
||||||
|
|
||||||
|
"github.com/pquerna/otp/totp"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestViewUser(t *testing.T) {
|
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"})
|
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)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue