diff --git a/models/asymkey/asymkey.go b/models/asymkey/asymkey.go new file mode 100644 index 0000000000..1f04e8dada --- /dev/null +++ b/models/asymkey/asymkey.go @@ -0,0 +1,29 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later +package asymkey + +import ( + "context" + + "forgejo.org/models/db" +) + +// HasAsymKeyByUID returns true if the user has a GPG key or SSH key associated +// with its account. +func HasAsymKeyByUID(ctx context.Context, userID int64) (bool, error) { + hasGPGKey, err := db.Exist[GPGKey](ctx, FindGPGKeyOptions{ + OwnerID: userID, + IncludeSubKeys: true, + }.ToConds()) + if err != nil { + return false, err + } + if hasGPGKey { + return true, nil + } + + return db.Exist[PublicKey](ctx, FindPublicKeyOptions{ + OwnerID: userID, + KeyTypes: []KeyType{KeyTypeUser}, + }.ToConds()) +} diff --git a/models/asymkey/asymkey_test.go b/models/asymkey/asymkey_test.go new file mode 100644 index 0000000000..1eeb593e40 --- /dev/null +++ b/models/asymkey/asymkey_test.go @@ -0,0 +1,34 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later +package asymkey + +import ( + "testing" + + "forgejo.org/models/unittest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestUserHasAsymKey(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("No key", func(t *testing.T) { + ok, err := HasAsymKeyByUID(t.Context(), 1) + require.NoError(t, err) + assert.False(t, ok) + }) + + t.Run("SSH key", func(t *testing.T) { + ok, err := HasAsymKeyByUID(t.Context(), 2) + require.NoError(t, err) + assert.True(t, ok) + }) + + t.Run("GPG key", func(t *testing.T) { + ok, err := HasAsymKeyByUID(t.Context(), 36) + require.NoError(t, err) + assert.True(t, ok) + }) +} diff --git a/models/auth/two_factor.go b/models/auth/two_factor.go new file mode 100644 index 0000000000..e8f19c33cc --- /dev/null +++ b/models/auth/two_factor.go @@ -0,0 +1,21 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later +package auth + +import ( + "context" +) + +// HasTwoFactorByUID returns true if the user has TOTP or WebAuthn enabled for +// their account. +func HasTwoFactorByUID(ctx context.Context, userID int64) (bool, error) { + hasTOTP, err := HasTOTPByUID(ctx, userID) + if err != nil { + return false, err + } + if hasTOTP { + return true, nil + } + + return HasWebAuthnRegistrationsByUID(ctx, userID) +} diff --git a/models/auth/two_factor_test.go b/models/auth/two_factor_test.go new file mode 100644 index 0000000000..36e0404ae2 --- /dev/null +++ b/models/auth/two_factor_test.go @@ -0,0 +1,34 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later +package auth + +import ( + "testing" + + "forgejo.org/models/unittest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHasTwoFactorByUID(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("No twofactor", func(t *testing.T) { + ok, err := HasTwoFactorByUID(t.Context(), 2) + require.NoError(t, err) + assert.False(t, ok) + }) + + t.Run("WebAuthn credential", func(t *testing.T) { + ok, err := HasTwoFactorByUID(t.Context(), 32) + require.NoError(t, err) + assert.True(t, ok) + }) + + t.Run("TOTP", func(t *testing.T) { + ok, err := HasTwoFactorByUID(t.Context(), 24) + require.NoError(t, err) + assert.True(t, ok) + }) +} diff --git a/models/auth/twofactor.go b/models/auth/twofactor.go index fa3bc68781..9a53ad30e0 100644 --- a/models/auth/twofactor.go +++ b/models/auth/twofactor.go @@ -135,9 +135,9 @@ func GetTwoFactorByUID(ctx context.Context, uid int64) (*TwoFactor, error) { return twofa, nil } -// HasTwoFactorByUID returns the two-factor authentication token associated with -// the user, if any. -func HasTwoFactorByUID(ctx context.Context, uid int64) (bool, error) { +// HasTOTPByUID returns the TOTP authentication token associated with +// the user, if the user has TOTP enabled for their account. +func HasTOTPByUID(ctx context.Context, uid int64) (bool, error) { return db.GetEngine(ctx).Where("uid=?", uid).Exist(&TwoFactor{}) } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index cefdfadc53..33b3d43571 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -248,17 +248,12 @@ func prepareUserInfo(ctx *context.Context) *user_model.User { } ctx.Data["Sources"] = sources - hasTOTP, err := auth.HasTwoFactorByUID(ctx, u.ID) + hasTwoFactor, err := auth.HasTwoFactorByUID(ctx, u.ID) if err != nil { - ctx.ServerError("auth.HasTwoFactorByUID", err) + ctx.ServerError("HasTwoFactorByUID", err) return nil } - hasWebAuthn, err := auth.HasWebAuthnRegistrationsByUID(ctx, u.ID) - if err != nil { - ctx.ServerError("auth.HasWebAuthnRegistrationsByUID", err) - return nil - } - ctx.Data["TwoFactorEnabled"] = hasTOTP || hasWebAuthn + ctx.Data["TwoFactorEnabled"] = hasTwoFactor return u } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 722091a606..9b5364c813 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -242,7 +242,7 @@ func SignInPost(ctx *context.Context) { // If this user is enrolled in 2FA TOTP, we can't sign the user in just yet. // Instead, redirect them to the 2FA authentication page. - hasTOTPtwofa, err := auth.HasTwoFactorByUID(ctx, u.ID) + hasTOTPtwofa, err := auth.HasTOTPByUID(ctx, u.ID) if err != nil { ctx.ServerError("UserSignIn", err) return diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index 9566652751..2bba614d8c 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -155,15 +155,14 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r // If this user is enrolled in 2FA, we can't sign the user in just yet. // Instead, redirect them to the 2FA authentication page. // We deliberately ignore the skip local 2fa setting here because we are linking to a previous user here - _, err := auth.GetTwoFactorByUID(ctx, u.ID) + hasTwoFactor, err := auth.HasTwoFactorByUID(ctx, u.ID) if err != nil { - if !auth.IsErrTwoFactorNotEnrolled(err) { - ctx.ServerError("UserLinkAccount", err) - return - } + ctx.ServerError("HasTwoFactorByUID", err) + return + } - err = externalaccount.LinkAccountToUser(ctx, u, gothUser) - if err != nil { + if !hasTwoFactor { + if err := externalaccount.LinkAccountToUser(ctx, u, gothUser); err != nil { ctx.ServerError("UserLinkAccount", err) return } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index c55397da88..aa599bd252 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1243,12 +1243,11 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model needs2FA := false if !source.Cfg.(*oauth2.Source).SkipLocalTwoFA { - _, err := auth.GetTwoFactorByUID(ctx, u.ID) - if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { + needs2FA, err = auth.HasTwoFactorByUID(ctx, u.ID) + if err != nil { ctx.ServerError("UserSignIn", err) return } - needs2FA = err == nil } oauth2Source := source.Cfg.(*oauth2.Source) diff --git a/routers/web/auth/webauthn.go b/routers/web/auth/webauthn.go index ac69e03389..3da6199b6e 100644 --- a/routers/web/auth/webauthn.go +++ b/routers/web/auth/webauthn.go @@ -36,7 +36,7 @@ func WebAuthn(ctx *context.Context) { return } - hasTwoFactor, err := auth.HasTwoFactorByUID(ctx, ctx.Session.Get("twofaUid").(int64)) + hasTwoFactor, err := auth.HasTOTPByUID(ctx, ctx.Session.Get("twofaUid").(int64)) if err != nil { ctx.ServerError("HasTwoFactorByUID", err) return diff --git a/routers/web/user/setting/security/security.go b/routers/web/user/setting/security/security.go index 9acc6ab6f0..8b801cfebd 100644 --- a/routers/web/user/setting/security/security.go +++ b/routers/web/user/setting/security/security.go @@ -55,7 +55,7 @@ func DeleteAccountLink(ctx *context.Context) { } func loadSecurityData(ctx *context.Context) { - enrolled, err := auth_model.HasTwoFactorByUID(ctx, ctx.Doer.ID) + enrolled, err := auth_model.HasTOTPByUID(ctx, ctx.Doer.ID) if err != nil { ctx.ServerError("SettingsTwoFactor", err) return diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 592396769d..527f6edd92 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -10,7 +10,6 @@ import ( asymkey_model "forgejo.org/models/asymkey" "forgejo.org/models/auth" - "forgejo.org/models/db" git_model "forgejo.org/models/git" issues_model "forgejo.org/models/issues" repo_model "forgejo.org/models/repo" @@ -152,22 +151,19 @@ Loop: case always: break Loop case pubkey: - keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - OwnerID: u.ID, - IncludeSubKeys: true, - }) + hasPubKey, err := asymkey_model.HasAsymKeyByUID(ctx, u.ID) if err != nil { return false, "", nil, err } - if len(keys) == 0 { + if !hasPubKey { return false, "", nil, &ErrWontSign{pubkey} } case twofa: - twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) - if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { + hasTwoFactor, err := auth.HasTwoFactorByUID(ctx, u.ID) + if err != nil { return false, "", nil, err } - if twofaModel == nil { + if !hasTwoFactor { return false, "", nil, &ErrWontSign{twofa} } } @@ -192,22 +188,19 @@ Loop: case always: break Loop case pubkey: - keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - OwnerID: u.ID, - IncludeSubKeys: true, - }) + hasPubKey, err := asymkey_model.HasAsymKeyByUID(ctx, u.ID) if err != nil { return false, "", nil, err } - if len(keys) == 0 { + if !hasPubKey { return false, "", nil, &ErrWontSign{pubkey} } case twofa: - twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) - if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { + hasTwoFactor, err := auth.HasTwoFactorByUID(ctx, u.ID) + if err != nil { return false, "", nil, err } - if twofaModel == nil { + if !hasTwoFactor { return false, "", nil, &ErrWontSign{twofa} } case parentSigned: @@ -248,22 +241,19 @@ Loop: case always: break Loop case pubkey: - keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - OwnerID: u.ID, - IncludeSubKeys: true, - }) + hasPubKey, err := asymkey_model.HasAsymKeyByUID(ctx, u.ID) if err != nil { return false, "", nil, err } - if len(keys) == 0 { + if !hasPubKey { return false, "", nil, &ErrWontSign{pubkey} } case twofa: - twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) - if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { + hasTwoFactor, err := auth.HasTwoFactorByUID(ctx, u.ID) + if err != nil { return false, "", nil, err } - if twofaModel == nil { + if !hasTwoFactor { return false, "", nil, &ErrWontSign{twofa} } case parentSigned: @@ -313,22 +303,19 @@ Loop: case always: break Loop case pubkey: - keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - OwnerID: u.ID, - IncludeSubKeys: true, - }) + hasPubKey, err := asymkey_model.HasAsymKeyByUID(ctx, u.ID) if err != nil { return false, "", nil, err } - if len(keys) == 0 { + if !hasPubKey { return false, "", nil, &ErrWontSign{pubkey} } case twofa: - twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) - if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { + hasTwoFactor, err := auth.HasTwoFactorByUID(ctx, u.ID) + if err != nil { return false, "", nil, err } - if twofaModel == nil { + if !hasTwoFactor { return false, "", nil, &ErrWontSign{twofa} } case approved: diff --git a/services/mailer/mail.go b/services/mailer/mail.go index c0a37d9fb2..410fdf6894 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -685,19 +685,14 @@ func SendRemovedSecurityKey(ctx context.Context, u *user_model.User, securityKey } 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) + hasTwoFactor, err := auth_model.HasTwoFactorByUID(ctx, u.ID) if err != nil { return err } data := map[string]any{ "locale": locale, - "HasWebAuthn": hasWebAuthn, - "HasTOTP": hasTOTP, + "HasTwoFactor": hasTwoFactor, "SecurityKeyName": securityKeyName, "DisplayName": u.DisplayName(), "Username": u.Name, diff --git a/templates/mail/auth/removed_security_key.tmpl b/templates/mail/auth/removed_security_key.tmpl index 18ae18725e..55c355f875 100644 --- a/templates/mail/auth/removed_security_key.tmpl +++ b/templates/mail/auth/removed_security_key.tmpl @@ -6,7 +6,7 @@
{{.locale.Tr "mail.hi_user_x" (.DisplayName|DotEscape)}}
{{.locale.Tr "mail.removed_security_key.text_1" .SecurityKeyName}}
{{.locale.Tr "mail.removed_security_key.no_2fa"}}
{{.locale.Tr "mail.removed_security_key.no_2fa"}}
{{.locale.Tr "mail.account_security_caution.text_1"}}
{{.locale.Tr "mail.account_security_caution.text_2"}}