fix(panel): share TOTP skew tolerance with VerifyTwoFactorCode

Move the +/-1 window helper to internal/util/totp so both 2FA
acceptance points use it: login (CheckUser) and disable/rebind plus
username/password changes (VerifyTwoFactorCode). Also shrink comments
to the 2-line house rule and anchor the unit test mid-window to avoid
a step-boundary flake.

Addresses review on #6546 (MEDIUM + 2 LOWs).
This commit is contained in:
sdhfsl
2026-09-15 18:56:09 +08:00
parent 959e6fd62b
commit f3096bb3c1
6 changed files with 64 additions and 57 deletions
+22
View File
@@ -0,0 +1,22 @@
package totp
import (
"time"
"github.com/xlzd/gotp"
)
// SkewWindows is how many 30s steps around now VerifyWithSkew accepts.
// Standard TOTP clock-drift tolerance, see MHSanaei/3x-ui#6535.
const SkewWindows = 1
// VerifyWithSkew accepts the code for the current step plus/minus SkewWindows.
func VerifyWithSkew(secret, code string, now time.Time) bool {
totp := gotp.NewDefaultTOTP(secret)
for i := -SkewWindows; i <= SkewWindows; i++ {
if totp.AtTime(now.Add(time.Duration(i*30)*time.Second)) == code {
return true
}
}
return false
}
+34
View File
@@ -0,0 +1,34 @@
package totp
import (
"testing"
"time"
"github.com/xlzd/gotp"
)
func TestVerifyWithSkew(t *testing.T) {
secret := "JBSWY3DPEHPK3PXP"
totp := gotp.NewDefaultTOTP(secret)
// Anchor mid-window so a step boundary can't fall between sampling and verify.
now := time.Unix((time.Now().Unix()/30)*30+15, 0).UTC()
if !VerifyWithSkew(secret, totp.AtTime(now), now) {
t.Fatal("current window code should verify")
}
if !VerifyWithSkew(secret, totp.AtTime(now.Add(-30*time.Second)), now) {
t.Fatal("previous window code should verify (clock skew)")
}
if !VerifyWithSkew(secret, totp.AtTime(now.Add(30*time.Second)), now) {
t.Fatal("next window code should verify (clock skew)")
}
if VerifyWithSkew(secret, totp.AtTime(now.Add(-60*time.Second)), now) {
t.Fatal("code two windows old should not verify")
}
if VerifyWithSkew(secret, totp.AtTime(now.Add(60*time.Second)), now) {
t.Fatal("code two windows ahead should not verify")
}
if VerifyWithSkew(secret, "000000", now) {
t.Fatal("wrong code should not verify")
}
}
+2 -22
View File
@@ -4,7 +4,6 @@ import (
"errors"
"time"
"github.com/xlzd/gotp"
"gorm.io/gorm"
"github.com/mhsanaei/3x-ui/v3/internal/database"
@@ -12,6 +11,7 @@ import (
"github.com/mhsanaei/3x-ui/v3/internal/logger"
"github.com/mhsanaei/3x-ui/v3/internal/util/crypto"
ldaputil "github.com/mhsanaei/3x-ui/v3/internal/util/ldap"
"github.com/mhsanaei/3x-ui/v3/internal/util/totp"
"github.com/mhsanaei/3x-ui/v3/internal/web/service"
)
@@ -98,7 +98,7 @@ func (s *UserService) CheckUser(username string, password string, twoFactorCode
return nil, err
}
if !verifyTOTPWithSkew(twoFactorToken, twoFactorCode) {
if !totp.VerifyWithSkew(twoFactorToken, twoFactorCode, time.Now()) {
return nil, errors.New("invalid 2fa code")
}
}
@@ -106,26 +106,6 @@ func (s *UserService) CheckUser(username string, password string, twoFactorCode
return user, nil
}
// totpSkewWindows is how many 30s steps around now are accepted. Client and
// server clocks are rarely perfectly in sync, and a code submitted at the end
// of its window may arrive after the server has rolled over — without skew
// the first attempt fails and the immediate retry (in the next window)
// succeeds, see #6535.
const totpSkewWindows = 1
// verifyTOTPWithSkew accepts the code for the current step plus/minus
// totpSkewWindows steps, the standard tolerance for TOTP clock drift.
func verifyTOTPWithSkew(secret, code string) bool {
totp := gotp.NewDefaultTOTP(secret)
now := time.Now()
for i := -totpSkewWindows; i <= totpSkewWindows; i++ {
if totp.AtTime(now.Add(time.Duration(i*30)*time.Second)) == code {
return true
}
}
return false
}
func (s *UserService) BumpLoginEpoch() error {
db := database.GetDB()
return db.Model(model.User{}).
@@ -1,33 +0,0 @@
package panel
import (
"testing"
"time"
"github.com/xlzd/gotp"
)
func TestVerifyTOTPWithSkew(t *testing.T) {
secret := "JBSWY3DPEHPK3PXP"
totp := gotp.NewDefaultTOTP(secret)
now := time.Now()
if !verifyTOTPWithSkew(secret, totp.AtTime(now)) {
t.Fatal("current window code should verify")
}
if !verifyTOTPWithSkew(secret, totp.AtTime(now.Add(-30*time.Second))) {
t.Fatal("previous window code should verify (clock skew)")
}
if !verifyTOTPWithSkew(secret, totp.AtTime(now.Add(30*time.Second))) {
t.Fatal("next window code should verify (clock skew)")
}
if verifyTOTPWithSkew(secret, totp.AtTime(now.Add(-60*time.Second))) {
t.Fatal("code two windows old should not verify")
}
if verifyTOTPWithSkew(secret, totp.AtTime(now.Add(60*time.Second))) {
t.Fatal("code two windows ahead should not verify")
}
if verifyTOTPWithSkew(secret, "000000") {
t.Fatal("wrong code should not verify")
}
}
+2 -2
View File
@@ -15,7 +15,6 @@ import (
"time"
"github.com/google/uuid"
"github.com/xlzd/gotp"
"gorm.io/gorm"
"github.com/mhsanaei/3x-ui/v3/internal/config"
@@ -26,6 +25,7 @@ import (
"github.com/mhsanaei/3x-ui/v3/internal/util/netproxy"
"github.com/mhsanaei/3x-ui/v3/internal/util/random"
"github.com/mhsanaei/3x-ui/v3/internal/util/reflect_util"
"github.com/mhsanaei/3x-ui/v3/internal/util/totp"
"github.com/mhsanaei/3x-ui/v3/internal/web/entity"
"github.com/mhsanaei/3x-ui/v3/internal/xray"
"github.com/mhsanaei/3x-ui/v3/internal/xray/dnsconf"
@@ -655,7 +655,7 @@ func (s *SettingService) VerifyTwoFactorCode(code string) error {
if err != nil {
return err
}
if strings.TrimSpace(token) == "" || !gotp.NewDefaultTOTP(token).Verify(strings.TrimSpace(code), time.Now().Unix()) {
if strings.TrimSpace(token) == "" || !totp.VerifyWithSkew(token, strings.TrimSpace(code), time.Now()) {
return common.NewError("invalid two factor code")
}
return nil
@@ -4,6 +4,7 @@ import (
"path/filepath"
"regexp"
"testing"
"time"
"github.com/xlzd/gotp"
@@ -230,6 +231,9 @@ func TestVerifyTwoFactorCode(t *testing.T) {
if err := s.VerifyTwoFactorCode(gotp.NewDefaultTOTP(token).Now()); err != nil {
t.Fatalf("valid code rejected: %v", err)
}
if err := s.VerifyTwoFactorCode(gotp.NewDefaultTOTP(token).AtTime(time.Now().Add(-30 * time.Second))); err != nil {
t.Fatalf("previous window code rejected: %v", err)
}
if err := s.VerifyTwoFactorCode("000000"); err == nil {
t.Fatal("invalid code accepted")
}