Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 49 additions & 9 deletions models/auth/twofactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ type TwoFactor struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"UNIQUE"`
Secret string
SecretSalt string
SecretAlgo string
ScratchSalt string
ScratchHash string
LastUsedPasscode string `xorm:"VARCHAR(10)"`
Expand Down Expand Up @@ -92,14 +94,38 @@ func (t *TwoFactor) VerifyScratchToken(token string) bool {
return subtle.ConstantTimeCompare([]byte(t.ScratchHash), []byte(tempHash)) == 1
}

func (t *TwoFactor) getEncryptionKey() []byte {
k := md5.Sum([]byte(setting.SecretKey))
return k[:]
const (
totpSecretKeyIterations = 10000
totpSecretKeyLength = 32
totpSecretSaltSize = 16
)

func (t *TwoFactor) getEncryptionKey() ([]byte, bool) {
if t.SecretAlgo == "" || t.SecretAlgo == "md5" || t.SecretSalt == "" {
k := md5.Sum([]byte(setting.SecretKey))
return k[:], true
}
key := pbkdf2.Key([]byte(setting.SecretKey), []byte(t.SecretSalt), totpSecretKeyIterations, totpSecretKeyLength, sha256.New)
return key, false
}

func (t *TwoFactor) rotateSecretSalt() error {
saltBytes, err := util.CryptoRandomBytes(totpSecretSaltSize)
if err != nil {
return err
}
t.SecretSalt = hex.EncodeToString(saltBytes)
t.SecretAlgo = "pbkdf2"
return nil
}

// SetSecret sets the 2FA secret.
func (t *TwoFactor) SetSecret(secretString string) error {
secretBytes, err := secret.AesEncrypt(t.getEncryptionKey(), []byte(secretString))
if err := t.rotateSecretSalt(); err != nil {
return err
}
key, _ := t.getEncryptionKey()
secretBytes, err := secret.AesEncrypt(key, []byte(secretString))
if err != nil {
return err
}
Expand All @@ -108,17 +134,31 @@ func (t *TwoFactor) SetSecret(secretString string) error {
}

// ValidateTOTP validates the provided passcode.
Comment thread
lunny marked this conversation as resolved.
func (t *TwoFactor) ValidateTOTP(passcode string) (bool, error) {
func (t *TwoFactor) ValidateTOTP(passcode string) (bool, bool, error) {
decodedStoredSecret, err := base64.StdEncoding.DecodeString(t.Secret)
if err != nil {
return false, fmt.Errorf("ValidateTOTP invalid base64: %w", err)
return false, false, fmt.Errorf("ValidateTOTP invalid base64: %w", err)
}
secretBytes, err := secret.AesDecrypt(t.getEncryptionKey(), decodedStoredSecret)
key, legacyKey := t.getEncryptionKey()
secretBytes, err := secret.AesDecrypt(key, decodedStoredSecret)
if err != nil {
return false, fmt.Errorf("ValidateTOTP unable to decrypt (maybe SECRET_KEY is wrong): %w", err)
return false, false, fmt.Errorf("ValidateTOTP unable to decrypt (maybe SECRET_KEY is wrong): %w", err)
}
secretStr := string(secretBytes)
return totp.Validate(passcode, secretStr), nil
ok := totp.Validate(passcode, secretStr)
if ok && legacyKey {
if err := t.rotateSecretSalt(); err != nil {
return ok, false, err
}
key, _ = t.getEncryptionKey()
secretBytes, err = secret.AesEncrypt(key, []byte(secretStr))
if err != nil {
return ok, false, err
Comment on lines +160 to +165

Copilot AI Mar 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When rotateSecretSalt() fails during a legacy-secret upgrade, the function returns ok (which is true in this branch) alongside a non-nil error. Returning false for ok when err != nil avoids ambiguous results and prevents future callers from accidentally accepting a passcode if they mishandle the error.

Suggested change
return ok, false, err
}
key, _ = t.getEncryptionKey()
secretBytes, err = secret.AesEncrypt(key, []byte(secretStr))
if err != nil {
return ok, false, err
return false, false, err
}
key, _ = t.getEncryptionKey()
secretBytes, err = secret.AesEncrypt(key, []byte(secretStr))
if err != nil {
return false, false, err

Copilot uses AI. Check for mistakes.
Comment on lines +160 to +165

Copilot AI Mar 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, if re-encryption of the legacy secret fails, this returns ok (true) with a non-nil error. Prefer returning false for ok whenever err is non-nil to keep the API contract consistent and reduce the risk of misuse by future callers.

Suggested change
return ok, false, err
}
key, _ = t.getEncryptionKey()
secretBytes, err = secret.AesEncrypt(key, []byte(secretStr))
if err != nil {
return ok, false, err
return false, false, err
}
key, _ = t.getEncryptionKey()
secretBytes, err = secret.AesEncrypt(key, []byte(secretStr))
if err != nil {
return false, false, err

Copilot uses AI. Check for mistakes.
}
t.Secret = base64.StdEncoding.EncodeToString(secretBytes)
return ok, true, nil
}
return ok, false, nil
}

// NewTwoFactor creates a new two-factor authentication token.
Expand Down
68 changes: 68 additions & 0 deletions models/auth/twofactor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package auth_test

import (
"crypto/md5"
"encoding/base64"
"testing"
"time"

auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/modules/secret"
"code.gitea.io/gitea/modules/setting"

"github.com/pquerna/otp/totp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestTwoFactorSetSecretUsesPBKDF2(t *testing.T) {
oldSecretKey := setting.SecretKey
setting.SecretKey = "twofactor-test-secret"
defer func() {
setting.SecretKey = oldSecretKey
}()

secretStr := "JBSWY3DPEHPK3PXP"
twofa := &auth_model.TwoFactor{}
require.NoError(t, twofa.SetSecret(secretStr))

assert.NotEmpty(t, twofa.SecretSalt)
assert.Equal(t, "pbkdf2", twofa.SecretAlgo)

passcode, err := totp.GenerateCode(secretStr, time.Now())
require.NoError(t, err)

ok, upgraded, err := twofa.ValidateTOTP(passcode)
require.NoError(t, err)
assert.True(t, ok)
assert.False(t, upgraded)
}

func TestTwoFactorLegacySecretUpgrade(t *testing.T) {
oldSecretKey := setting.SecretKey
setting.SecretKey = "twofactor-test-secret"
defer func() {
setting.SecretKey = oldSecretKey
}()

secretStr := "JBSWY3DPEHPK3PXP"
legacyKey := md5.Sum([]byte(setting.SecretKey))
ciphertext, err := secret.AesEncrypt(legacyKey[:], []byte(secretStr))
require.NoError(t, err)
legacySecret := base64.StdEncoding.EncodeToString(ciphertext)

twofa := &auth_model.TwoFactor{Secret: legacySecret}
passcode, err := totp.GenerateCode(secretStr, time.Now())
require.NoError(t, err)

ok, upgraded, err := twofa.ValidateTOTP(passcode)
require.NoError(t, err)
assert.True(t, ok)
assert.True(t, upgraded)
assert.NotEmpty(t, twofa.SecretSalt)
assert.Equal(t, "pbkdf2", twofa.SecretAlgo)
assert.NotEqual(t, legacySecret, twofa.Secret)
}
1 change: 1 addition & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ func prepareMigrationTasks() []*migration {
newMigration(327, "Add disabled state to action runners", v1_26.AddDisabledToActionRunner),
newMigration(328, "Add TokenPermissions column to ActionRunJob", v1_26.AddTokenPermissionsToActionRunJob),
newMigration(329, "Add unique constraint for user badge", v1_26.AddUniqueIndexForUserBadge),
newMigration(330, "Add secret_salt and secret_algo to two_factor", v1_26.AddSecretSaltToTwoFactor),
}
return preparedMigrations
}
Expand Down
21 changes: 21 additions & 0 deletions models/migrations/v1_26/v330.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_26

import "xorm.io/xorm"

// AddSecretSaltToTwoFactor adds a per-user salt column for TOTP secrets.
func AddSecretSaltToTwoFactor(x *xorm.Engine) error {
type TwoFactor struct {
SecretSalt string
SecretAlgo string
}

if err := x.Sync(new(TwoFactor)); err != nil {
return err
}

_, err := x.Exec("UPDATE two_factor SET secret_algo = 'md5' WHERE secret_algo = '' OR secret_algo IS NULL")
return err
}
31 changes: 31 additions & 0 deletions models/migrations/v1_26/v330_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_26

import (
"testing"

"code.gitea.io/gitea/models/migrations/base"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_AddSecretSaltToTwoFactor(t *testing.T) {
type TwoFactor struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"UNIQUE"`
Secret string
}

x, deferrable := base.PrepareTestEnv(t, 0, new(TwoFactor))
defer deferrable()

require.NoError(t, AddSecretSaltToTwoFactor(x))

table := base.LoadTableSchemasMap(t, x)["two_factor"]
require.NotNil(t, table)
assert.NotNil(t, table.GetColumn("secret_salt"))
assert.NotNil(t, table.GetColumn("secret_algo"))
}
2 changes: 1 addition & 1 deletion routers/web/auth/2fa.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TwoFactorPost(ctx *context.Context) {
}

// Validate the passcode with the stored TOTP secret.
ok, err := twofa.ValidateTOTP(form.Passcode)
ok, _, err := twofa.ValidateTOTP(form.Passcode)
if err != nil {
ctx.ServerError("UserSignIn", err)
return
Expand Down
2 changes: 1 addition & 1 deletion routers/web/auth/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func ResetPasswdPost(ctx *context.Context) {
regenerateScratchToken = true
} else {
passcode := ctx.FormString("passcode")
ok, err := twofa.ValidateTOTP(passcode)
ok, _, err := twofa.ValidateTOTP(passcode)
if err != nil {
ctx.HTTPError(http.StatusInternalServerError, "ValidateTOTP", err.Error())
return
Expand Down
11 changes: 9 additions & 2 deletions services/auth/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,18 @@ func validateTOTP(req *http.Request, u *user_model.User) error {
}
return err
}
if ok, err := twofa.ValidateTOTP(req.Header.Get("X-Gitea-OTP")); err != nil {
ok, upgraded, err := twofa.ValidateTOTP(req.Header.Get("X-Gitea-OTP"))
if err != nil {
return err
} else if !ok {
}
if !ok {
return util.NewInvalidArgumentErrorf("invalid provided OTP")
}
if upgraded {
if err := auth_model.UpdateTwoFactor(req.Context(), twofa); err != nil {
return err
}
}
return nil
}

Expand Down
Loading