diff --git a/backend/internal/repository/redeem_cache.go b/backend/internal/repository/redeem_cache.go index 831aaf570a..9d0430724a 100644 --- a/backend/internal/repository/redeem_cache.go +++ b/backend/internal/repository/redeem_cache.go @@ -10,11 +10,22 @@ import ( ) const ( - redeemRateLimitKeyPrefix = "redeem:ratelimit:" + redeemRateLimitKeyPrefix = "redeem:ratelimit:v2:" redeemLockKeyPrefix = "redeem:lock:" - redeemRateLimitDuration = 24 * time.Hour + redeemRateLimitWindow = 10 * time.Minute ) +var incrementRedeemAttemptScript = redis.NewScript(` +local current = redis.call('INCR', KEYS[1]) +local ttl = redis.call('PTTL', KEYS[1]) + +if current == 1 or ttl == -1 then + redis.call('PEXPIRE', KEYS[1], ARGV[1]) +end + +return current +`) + // redeemRateLimitKey generates the Redis key for redeem attempt rate limiting. func redeemRateLimitKey(userID int64) string { return fmt.Sprintf("%s%d", redeemRateLimitKeyPrefix, userID) @@ -44,11 +55,12 @@ func (c *redeemCache) GetRedeemAttemptCount(ctx context.Context, userID int64) ( func (c *redeemCache) IncrementRedeemAttemptCount(ctx context.Context, userID int64) error { key := redeemRateLimitKey(userID) - pipe := c.rdb.Pipeline() - pipe.Incr(ctx, key) - pipe.Expire(ctx, key, redeemRateLimitDuration) - _, err := pipe.Exec(ctx) - return err + return incrementRedeemAttemptScript.Run( + ctx, + c.rdb, + []string{key}, + redeemRateLimitWindow.Milliseconds(), + ).Err() } func (c *redeemCache) AcquireRedeemLock(ctx context.Context, code string, ttl time.Duration) (bool, error) { diff --git a/backend/internal/repository/redeem_cache_integration_test.go b/backend/internal/repository/redeem_cache_integration_test.go index 6398a80141..fb7e207431 100644 --- a/backend/internal/repository/redeem_cache_integration_test.go +++ b/backend/internal/repository/redeem_cache_integration_test.go @@ -39,7 +39,35 @@ func (s *RedeemCacheSuite) TestIncrementAndGetRedeemAttemptCount() { ttl, err := s.rdb.TTL(s.ctx, key).Result() require.NoError(s.T(), err, "TTL") - s.AssertTTLWithin(ttl, 1*time.Second, redeemRateLimitDuration) + s.AssertTTLWithin(ttl, redeemRateLimitWindow-time.Second, redeemRateLimitWindow) +} + +func (s *RedeemCacheSuite) TestIncrementDoesNotRefreshFixedWindow() { + userID := int64(3) + key := redeemRateLimitKey(userID) + shortTTL := 5 * time.Minute + require.NoError(s.T(), s.cache.IncrementRedeemAttemptCount(s.ctx, userID)) + require.NoError(s.T(), s.rdb.PExpire(s.ctx, key, shortTTL).Err()) + require.NoError(s.T(), s.cache.IncrementRedeemAttemptCount(s.ctx, userID)) + ttl, err := s.rdb.PTTL(s.ctx, key).Result() + require.NoError(s.T(), err) + s.AssertTTLWithin(ttl, shortTTL-time.Second, shortTTL) +} + +func (s *RedeemCacheSuite) TestIncrementRepairsMissingTTL() { + userID := int64(4) + key := redeemRateLimitKey(userID) + require.NoError(s.T(), s.rdb.Set(s.ctx, key, 5, 0).Err()) + ttl, err := s.rdb.PTTL(s.ctx, key).Result() + require.NoError(s.T(), err) + require.Equal(s.T(), time.Duration(-1), ttl) + require.NoError(s.T(), s.cache.IncrementRedeemAttemptCount(s.ctx, userID)) + count, err := s.cache.GetRedeemAttemptCount(s.ctx, userID) + require.NoError(s.T(), err) + require.Equal(s.T(), 6, count) + ttl, err = s.rdb.PTTL(s.ctx, key).Result() + require.NoError(s.T(), err) + s.AssertTTLWithin(ttl, redeemRateLimitWindow-time.Second, redeemRateLimitWindow) } func (s *RedeemCacheSuite) TestMultipleIncrements() { diff --git a/backend/internal/repository/redeem_cache_test.go b/backend/internal/repository/redeem_cache_test.go index 9b547b7485..8e637241ef 100644 --- a/backend/internal/repository/redeem_cache_test.go +++ b/backend/internal/repository/redeem_cache_test.go @@ -18,22 +18,22 @@ func TestRedeemRateLimitKey(t *testing.T) { { name: "normal_user_id", userID: 123, - expected: "redeem:ratelimit:123", + expected: "redeem:ratelimit:v2:123", }, { name: "zero_user_id", userID: 0, - expected: "redeem:ratelimit:0", + expected: "redeem:ratelimit:v2:0", }, { name: "negative_user_id", userID: -1, - expected: "redeem:ratelimit:-1", + expected: "redeem:ratelimit:v2:-1", }, { name: "max_int64", userID: math.MaxInt64, - expected: "redeem:ratelimit:9223372036854775807", + expected: "redeem:ratelimit:v2:9223372036854775807", }, } diff --git a/backend/internal/service/redeem_rate_limit_test.go b/backend/internal/service/redeem_rate_limit_test.go new file mode 100644 index 0000000000..7beb302935 --- /dev/null +++ b/backend/internal/service/redeem_rate_limit_test.go @@ -0,0 +1,176 @@ +//go:build unit + +package service + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// redeemRateLimitCacheStub 实现 RedeemCache，记录各方法调用次数并允许注入错误。 +type redeemRateLimitCacheStub struct { + count int + getErr error + incrementErr error + getCalls int + incrementCalls int + acquireCalls int + releaseCalls int +} + +func (c *redeemRateLimitCacheStub) GetRedeemAttemptCount(context.Context, int64) (int, error) { + c.getCalls++ + return c.count, c.getErr +} + +func (c *redeemRateLimitCacheStub) IncrementRedeemAttemptCount(context.Context, int64) error { + c.incrementCalls++ + if c.incrementErr != nil { + return c.incrementErr + } + c.count++ + return nil +} + +func (c *redeemRateLimitCacheStub) AcquireRedeemLock(context.Context, string, time.Duration) (bool, error) { + c.acquireCalls++ + return true, nil +} + +func (c *redeemRateLimitCacheStub) ReleaseRedeemLock(context.Context, string) error { + c.releaseCalls++ + return nil +} + +func TestPublicRedeemAllowsLastAttemptBeforeLimit(t *testing.T) { + cache := &redeemRateLimitCacheStub{count: redeemMaxFailedAttempts - 1} + redeemRepo := &redeemRejectRepo{code: RedeemCode{Code: "OTHER"}} + svc := &RedeemService{redeemRepo: redeemRepo, cache: cache} + + result, err := svc.Redeem(context.Background(), 42, "MISSING") + + require.Nil(t, result) + require.ErrorIs(t, err, ErrRedeemCodeNotFound) + require.Equal(t, redeemMaxFailedAttempts, cache.count) + require.Equal(t, 1, cache.getCalls) + require.Equal(t, 1, cache.incrementCalls) + require.Equal(t, 1, cache.acquireCalls) + require.Equal(t, 1, cache.releaseCalls) +} + +func TestPublicRedeemCountsOnlyConfiguredDomainFailures(t *testing.T) { + past := time.Now().Add(-time.Minute) + tests := []struct { + name string + code RedeemCode + input string + wantErr error + }{ + { + name: "not found", + code: RedeemCode{Code: "OTHER"}, + input: "MISSING", + wantErr: ErrRedeemCodeNotFound, + }, + { + name: "expired", + code: RedeemCode{ + ID: 1, Code: "EXPIRED", Type: RedeemTypeBalance, Value: 10, + Status: StatusUnused, ExpiresAt: &past, + }, + input: "EXPIRED", + wantErr: ErrRedeemCodeExpired, + }, + { + name: "used", + code: RedeemCode{ + ID: 2, Code: "USED", Type: RedeemTypeBalance, Value: 10, + Status: StatusUsed, + }, + input: "USED", + wantErr: ErrRedeemCodeUsed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache := &redeemRateLimitCacheStub{} + svc := &RedeemService{ + redeemRepo: &redeemRejectRepo{code: tt.code}, + cache: cache, + } + + result, err := svc.Redeem(context.Background(), 42, tt.input) + + require.Nil(t, result) + require.ErrorIs(t, err, tt.wantErr) + require.Equal(t, 1, cache.getCalls) + require.Equal(t, 1, cache.incrementCalls) + require.Equal(t, 1, cache.count) + }) + } +} + +func TestPublicRedeemRateLimitFailsOpenWhenCounterReadFails(t *testing.T) { + cache := &redeemRateLimitCacheStub{ + count: redeemMaxFailedAttempts, + getErr: errors.New("redis unavailable"), + } + svc := &RedeemService{ + redeemRepo: &redeemRejectRepo{code: RedeemCode{Code: "OTHER"}}, + cache: cache, + } + + result, err := svc.Redeem(context.Background(), 42, "MISSING") + + require.Nil(t, result) + require.ErrorIs(t, err, ErrRedeemCodeNotFound) + require.Equal(t, 1, cache.getCalls) + require.Equal(t, 1, cache.incrementCalls) +} + +func TestPublicRedeemCounterWriteFailurePreservesDomainError(t *testing.T) { + cache := &redeemRateLimitCacheStub{incrementErr: errors.New("redis unavailable")} + svc := &RedeemService{ + redeemRepo: &redeemRejectRepo{code: RedeemCode{Code: "OTHER"}}, + cache: cache, + } + + result, err := svc.Redeem(context.Background(), 42, "MISSING") + + require.Nil(t, result) + require.ErrorIs(t, err, ErrRedeemCodeNotFound) + require.Equal(t, 1, cache.incrementCalls) + require.Zero(t, cache.count) +} + +func TestSuccessfulPublicRedeemDoesNotMutateFailureCounter(t *testing.T) { + ctx := context.Background() + client := newPaymentConfigServiceTestClient(t) + userID := int64(42) + code := &RedeemCode{ + ID: 101, Code: "PUBLIC-SUCCESS", Type: RedeemTypeBalance, Value: 10, Status: StatusUnused, + } + redeemRepo := &paymentOrderLifecycleRedeemRepo{ + codesByCode: map[string]*RedeemCode{code.Code: code}, + } + userRepo := &mockUserRepo{getByIDUser: &User{ID: userID}} + userRepo.updateBalanceFn = func(context.Context, int64, float64) error { return nil } + cache := &redeemRateLimitCacheStub{count: 7} + svc := NewRedeemService(redeemRepo, userRepo, nil, cache, nil, client, nil, nil) + + result, err := svc.Redeem(ctx, userID, code.Code) + + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, 1, cache.getCalls) + require.Zero(t, cache.incrementCalls) + require.Equal(t, 7, cache.count) + require.Equal(t, 1, cache.acquireCalls) + require.Equal(t, 1, cache.releaseCalls) + require.Len(t, redeemRepo.useCalls, 1) +} diff --git a/backend/internal/service/redeem_service.go b/backend/internal/service/redeem_service.go index 7be6871f07..5e7cdee685 100644 --- a/backend/internal/service/redeem_service.go +++ b/backend/internal/service/redeem_service.go @@ -25,8 +25,7 @@ var ( ) const ( - redeemMaxErrorsPerHour = 20 - redeemRateLimitDuration = time.Hour + redeemMaxFailedAttempts = 30 redeemLockDuration = 10 * time.Second // 锁超时时间，防止死锁 ) @@ -344,7 +343,7 @@ func (s *RedeemService) checkRedeemRateLimit(ctx context.Context, userID int64) return nil } - if count >= redeemMaxErrorsPerHour { + if count >= redeemMaxFailedAttempts { return ErrRedeemRateLimited }