fix: purge authentication data on hard user deletion (#6168)

* fix: purge authentication data on hard user deletion * fix: fail closed when 2FA status lookup fails * fix: reject stale Telegram login callbacks * fix(twofa): prevent concurrent backup code and lockout bypasses * fix(auth): harden user deletion and Telegram verification
2026-07-14 14:25:54 +08:00
parent 7c28993f6b
commit b6b97a66e3
8 changed files with 371 additions and 51 deletions
@@ -38,6 +38,9 @@ func TestMain(m *testing.M) {
 		&Task{},
 		&User{},
 		&Token{},
+		&PasskeyCredential{},
+		&TwoFA{},
+		&TwoFABackupCode{},
 		&Log{},
 		&Channel{},
 		&QuotaData{},
@@ -62,8 +65,12 @@ func truncateTables(t *testing.T) {
 	t.Helper()
 	t.Cleanup(func() {
 		DB.Exec("DELETE FROM tasks")
-		DB.Exec("DELETE FROM users")
+		DB.Exec("DELETE FROM passkey_credentials")
+		DB.Exec("DELETE FROM two_fa_backup_codes")
+		DB.Exec("DELETE FROM two_fas")
 		DB.Exec("DELETE FROM tokens")
+		DB.Exec("DELETE FROM user_oauth_bindings")
+		DB.Exec("DELETE FROM users")
 		DB.Exec("DELETE FROM logs")
 		DB.Exec("DELETE FROM channels")
 		DB.Exec("DELETE FROM quota_data")
@@ -72,7 +79,6 @@ func truncateTables(t *testing.T) {
 		DB.Exec("DELETE FROM subscription_orders")
 		DB.Exec("DELETE FROM subscription_plans")
 		DB.Exec("DELETE FROM user_subscriptions")
-		DB.Exec("DELETE FROM user_oauth_bindings")
 		DB.Exec("DELETE FROM perf_metrics")
 		DB.Exec("DELETE FROM system_instances")
 		DB.Exec("DELETE FROM system_task_locks")
@@ -505,6 +505,13 @@ func InvalidateUserTokensCache(userId int) error {
 		Find(&tokens).Error; err != nil {
 		return err
 	}
+	return invalidateTokensCache(tokens)
+}
+
+func invalidateTokensCache(tokens []Token) error {
+	if !common.RedisEnabled {
+		return nil
+	}
 	var firstErr error
 	for _, t := range tokens {
 		if t.Key == "" {
@@ -54,12 +54,12 @@ func GetTwoFAByUserId(userId int) (*TwoFA, error) {
 }

 // IsTwoFAEnabled 检查用户是否启用了2FA
-func IsTwoFAEnabled(userId int) bool {
+func IsTwoFAEnabled(userId int) (bool, error) {
 	twoFA, err := GetTwoFAByUserId(userId)
-	if err != nil || twoFA == nil {
-		return false
+	if err != nil {
+		return false, err
 	}
-	return twoFA.IsEnabled
+	return twoFA != nil && twoFA.IsEnabled, nil
 }

 // CreateTwoFA 创建2FA设置
@@ -120,15 +120,50 @@ func (t *TwoFA) ResetFailedAttempts() error {

 // IncrementFailedAttempts 增加失败尝试次数
 func (t *TwoFA) IncrementFailedAttempts() error {
-	t.FailedAttempts++
-
-	// 检查是否需要锁定
-	if t.FailedAttempts >= common.MaxFailAttempts {
-		lockUntil := time.Now().Add(time.Duration(common.LockoutDuration) * time.Second)
-		t.LockedUntil = &lockUntil
+	if t.Id == 0 {
+		return errors.New("2FA记录ID不能为空")
 	}

-	return t.Update()
+	const maxUpdateRetries = 5
+	for range maxUpdateRetries {
+		var current TwoFA
+		if err := DB.Select("id", "failed_attempts", "locked_until").First(&current, t.Id).Error; err != nil {
+			return err
+		}
+
+		now := time.Now()
+		if current.LockedUntil != nil && now.Before(*current.LockedUntil) {
+			t.FailedAttempts = current.FailedAttempts
+			t.LockedUntil = current.LockedUntil
+			return nil
+		}
+
+		nextFailedAttempts := current.FailedAttempts + 1
+		nextLockedUntil := current.LockedUntil
+		if nextFailedAttempts >= common.MaxFailAttempts {
+			lockUntil := now.Add(time.Duration(common.LockoutDuration) * time.Second)
+			nextLockedUntil = &lockUntil
+		}
+
+		result := DB.Model(&TwoFA{}).
+			Where("id = ? AND failed_attempts = ? AND (locked_until IS NULL OR locked_until <= ?)", current.Id, current.FailedAttempts, now).
+			Updates(map[string]interface{}{
+				"failed_attempts": nextFailedAttempts,
+				"locked_until":    nextLockedUntil,
+			})
+		if result.Error != nil {
+			return result.Error
+		}
+		if result.RowsAffected == 0 {
+			continue
+		}
+
+		t.FailedAttempts = nextFailedAttempts
+		t.LockedUntil = nextLockedUntil
+		return nil
+	}
+
+	return errors.New("更新2FA失败次数冲突，请重试")
 }

 // IsLocked 检查账户是否被锁定
@@ -186,16 +221,17 @@ func ValidateBackupCode(userId int, code string) (bool, error) {
 	// 验证备用码
 	for _, bc := range backupCodes {
 		if common.ValidatePasswordAndHash(normalizedCode, bc.CodeHash) {
-			// 标记为已使用
 			now := time.Now()
-			bc.IsUsed = true
-			bc.UsedAt = &now
-
-			if err := DB.Save(&bc).Error; err != nil {
-				return false, err
+			result := DB.Model(&TwoFABackupCode{}).
+				Where("id = ? AND is_used = ?", bc.Id, false).
+				Updates(map[string]interface{}{
+					"is_used": true,
+					"used_at": now,
+				})
+			if result.Error != nil {
+				return false, result.Error
 			}
-
-			return true, nil
+			return result.RowsAffected == 1, nil
 		}
 	}

@@ -423,12 +423,8 @@ func HardDeleteUserById(id int) error {
 	if id == 0 {
 		return errors.New("id 为空！")
 	}
-	return DB.Transaction(func(tx *gorm.DB) error {
-		if err := deleteUserOAuthBindingsByUserId(tx, id); err != nil {
-			return err
-		}
-		return tx.Unscoped().Delete(&User{}, "id = ?", id).Error
-	})
+	user := User{Id: id}
+	return user.HardDelete()
 }

 func inviteUser(inviterId int) (err error) {
@@ -754,12 +750,42 @@ func (user *User) HardDelete() error {
 	if user.Id == 0 {
 		return errors.New("id 为空！")
 	}
-	return DB.Transaction(func(tx *gorm.DB) error {
-		if err := deleteUserOAuthBindingsByUserId(tx, user.Id); err != nil {
+	var tokens []Token
+	err := DB.Transaction(func(tx *gorm.DB) error {
+		if common.RedisEnabled {
+			if err := tx.Unscoped().Select("id", commonKeyCol).Where("user_id = ?", user.Id).Find(&tokens).Error; err != nil {
+				return err
+			}
+		}
+		if err := deleteUserAuthenticationData(tx, user.Id); err != nil {
 			return err
 		}
 		return tx.Unscoped().Delete(user).Error
 	})
+	if err != nil {
+		return err
+	}
+	if err := invalidateTokensCache(tokens); err != nil {
+		common.SysError(fmt.Sprintf("failed to invalidate token cache after hard deleting user %d: %v", user.Id, err))
+	}
+	if err := invalidateUserCache(user.Id); err != nil {
+		common.SysError(fmt.Sprintf("failed to invalidate user cache after hard deleting user %d: %v", user.Id, err))
+	}
+	return nil
+}
+
+func deleteUserAuthenticationData(tx *gorm.DB, userId int) error {
+	for _, authenticationData := range []any{
+		&TwoFABackupCode{},
+		&TwoFA{},
+		&PasskeyCredential{},
+		&Token{},
+	} {
+		if err := tx.Unscoped().Where("user_id = ?", userId).Delete(authenticationData).Error; err != nil {
+			return err
+		}
+	}
+	return deleteUserOAuthBindingsByUserId(tx, userId)
 }

 // ValidateAndFill check password & user status
@@ -0,0 +1,130 @@
+package model
+
+import (
+	"context"
+	"errors"
+	"net"
+	"sync"
+	"sync/atomic"
+	"testing"
+
+	"github.com/QuantumNous/new-api/common"
+	"github.com/go-redis/redis/v8"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestHardDeleteUserPurgesAuthenticationDataWhenRedisFails(t *testing.T) {
+	truncateTables(t)
+
+	user := User{Username: "hard-delete-user", Password: "password"}
+	require.NoError(t, DB.Create(&user).Error)
+	require.NoError(t, DB.Create(&Token{UserId: user.Id, Key: "hard-delete-token"}).Error)
+	require.NoError(t, DB.Create(&TwoFA{UserId: user.Id, Secret: "secret", IsEnabled: true}).Error)
+	require.NoError(t, DB.Create(&TwoFABackupCode{UserId: user.Id, CodeHash: "hash"}).Error)
+	require.NoError(t, DB.Create(&PasskeyCredential{UserID: user.Id, CredentialID: "credential", PublicKey: "public-key"}).Error)
+	require.NoError(t, DB.Create(&UserOAuthBinding{UserId: user.Id, ProviderId: 1, ProviderUserId: "provider-user"}).Error)
+
+	oldRedisEnabled, oldRDB := common.RedisEnabled, common.RDB
+	common.RedisEnabled = true
+	var cacheInvalidatedAfterCommit atomic.Bool
+	common.RDB = redis.NewClient(&redis.Options{
+		Dialer: func(context.Context, string, string) (net.Conn, error) {
+			var count int64
+			if err := DB.Unscoped().Model(&User{}).Where("id = ?", user.Id).Count(&count).Error; err == nil && count == 0 {
+				cacheInvalidatedAfterCommit.Store(true)
+			}
+			return nil, errors.New("forced redis failure")
+		},
+		MaxRetries: -1,
+	})
+	t.Cleanup(func() {
+		_ = common.RDB.Close()
+		common.RedisEnabled, common.RDB = oldRedisEnabled, oldRDB
+	})
+
+	require.NoError(t, HardDeleteUserById(user.Id))
+	assert.True(t, cacheInvalidatedAfterCommit.Load())
+
+	var count int64
+	require.NoError(t, DB.Unscoped().Model(&User{}).Where("id = ?", user.Id).Count(&count).Error)
+	assert.Zero(t, count)
+	for _, record := range []any{
+		&Token{},
+		&TwoFA{},
+		&TwoFABackupCode{},
+		&PasskeyCredential{},
+		&UserOAuthBinding{},
+	} {
+		require.NoError(t, DB.Unscoped().Model(record).Where("user_id = ?", user.Id).Count(&count).Error)
+		assert.Zero(t, count)
+	}
+}
+
+func TestIncrementFailedAttemptsCountsConcurrentFailures(t *testing.T) {
+	truncateTables(t)
+
+	user := User{Username: "twofa-cas-user", Password: "password"}
+	require.NoError(t, DB.Create(&user).Error)
+	twoFA := TwoFA{UserId: user.Id, Secret: "secret", IsEnabled: true}
+	require.NoError(t, DB.Create(&twoFA).Error)
+
+	const attempts = 4
+	errs := make(chan error, attempts)
+	var wg sync.WaitGroup
+	for range attempts {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			errs <- (&TwoFA{Id: twoFA.Id}).IncrementFailedAttempts()
+		}()
+	}
+	wg.Wait()
+	close(errs)
+	for err := range errs {
+		require.NoError(t, err)
+	}
+
+	var reloaded TwoFA
+	require.NoError(t, DB.First(&reloaded, twoFA.Id).Error)
+	assert.Equal(t, attempts, reloaded.FailedAttempts)
+}
+
+func TestValidateBackupCodeCanOnlySucceedOnce(t *testing.T) {
+	truncateTables(t)
+
+	const code = "ABCD-1234"
+	require.NoError(t, CreateBackupCodes(123, []string{code}))
+
+	const attempts = 2
+	results := make(chan bool, attempts)
+	errs := make(chan error, attempts)
+	var wg sync.WaitGroup
+	for range attempts {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			valid, err := ValidateBackupCode(123, code)
+			results <- valid
+			errs <- err
+		}()
+	}
+	wg.Wait()
+	close(results)
+	close(errs)
+
+	for err := range errs {
+		require.NoError(t, err)
+	}
+	wins := 0
+	for valid := range results {
+		if valid {
+			wins++
+		}
+	}
+	assert.Equal(t, 1, wins)
+
+	remaining, err := GetUnusedBackupCodeCount(123)
+	require.NoError(t, err)
+	assert.Zero(t, remaining)
+}