diff --git a/backend/controllers/login_methods_test.go b/backend/controllers/login_methods_test.go index 4ce80cb..3031094 100644 --- a/backend/controllers/login_methods_test.go +++ b/backend/controllers/login_methods_test.go @@ -305,6 +305,11 @@ func TestEmailLoginAfterAsyncDelivery(t *testing.T) { if code, _ := callLoginHandler(t, CompleteLoginVerification, verify); code == 0 { t.Fatal("consumed code accepted again") } + if _, err := currentSecurityRuntime().verification.Issue(verification.IssueRequest{ + Purpose: verification.PurposePasswordChange, Account: user.Account, Email: "owner@example.net", + }, "127.0.0.1"); err != nil { + t.Fatalf("登录完成后其他业务无法发送验证码：%v", err) + } } func TestLoginMethodsRejectInvalidState(t *testing.T) { diff --git a/backend/services/verification/completion_test.go b/backend/services/verification/completion_test.go new file mode 100644 index 0000000..8d70844 --- /dev/null +++ b/backend/services/verification/completion_test.go @@ -0,0 +1,199 @@ +package verification + +import ( + "errors" + "fmt" + "sync" + "testing" + "time" +) + +func completionService(t *testing.T, persistent bool) (*Service, *string, *time.Time) { + t.Helper() + store, err := NewStore(64) + if err != nil { + t.Fatal(err) + } + var code string + service, err := NewService(store, func(_ Challenge, value string) error { + code = value + return nil + }, nil) + if err != nil { + t.Fatal(err) + } + now := time.Now().UTC() + service.now = func() time.Time { return now } + store.now = service.now + if persistent { + engine := newRateLimitEngine(t) + t.Cleanup(func() { _ = engine.Close() }) + limiter, err := NewPersistentRateLimiter(engine) + if err != nil { + t.Fatal(err) + } + service.SetRateLimitStore(limiter) + } + return service, &code, &now +} + +func completionRequest(purpose Purpose) IssueRequest { + return IssueRequest{Purpose: purpose, Account: "owner", Email: "owner@example.com", + UserID: 7, ResourceID: 9, BrowserTransaction: "browser", Environment: "production", CredentialVersion: 2} +} + +func TestCompletionReleasesAllPurposeCooldownsAndPreservesQuota(t *testing.T) { + for _, persistent := range []bool{false, true} { + t.Run(fmt.Sprint(persistent), func(t *testing.T) { + service, code, _ := completionService(t, persistent) + purposes := []Purpose{PurposeLogin, PurposePasswordChange, PurposePasswordRecover, PurposeExternalEmailBind, PurposeExternalEmailChange} + for _, purpose := range purposes { + request := completionRequest(purpose) + challenge, err := service.Issue(request, "192.0.2.1") + if err != nil { + t.Fatalf("下一业务被已完成的验证阻塞：%s: %v", purpose, err) + } + if _, err := service.Issue(request, "192.0.2.1"); !errors.Is(err, ErrRateLimited) { + t.Fatalf("未使用时应保留发送冷却：%v", err) + } + verify := func(value string) error { + return service.VerifyBoundWithContext(challenge.ID, request.Account, request.Email, value, purpose, request.ResourceID, "192.0.2.99", request.BrowserTransaction) + } + if err := verify("invalid"); !errors.Is(err, ErrInvalidChallenge) { + t.Fatal(err) + } + if _, err := service.Issue(request, "192.0.2.1"); !errors.Is(err, ErrRateLimited) { + t.Fatalf("错误验证码释放了冷却：%v", err) + } + if err := verify(*code); err != nil { + t.Fatal(err) + } + if service.Store.Size() != 0 { + t.Fatal("验证码使用后仍有待验证状态") + } + if err := verify(*code); !errors.Is(err, ErrInvalidChallenge) { + t.Fatalf("已使用验证码被重放：%v", err) + } + } + if _, err := service.Issue(completionRequest(PurposeLogin), "192.0.2.1"); !errors.Is(err, ErrRateLimited) { + t.Fatalf("成功验证不应清除累计配额：%v", err) + } + }) + } +} + +func TestCompletionDoesNotReleaseNewerRequest(t *testing.T) { + for _, persistent := range []bool{false, true} { + t.Run(fmt.Sprint(persistent), func(t *testing.T) { + service, code, now := completionService(t, persistent) + first, err := service.Issue(completionRequest(PurposeLogin), "192.0.2.1") + if err != nil { + t.Fatal(err) + } + firstCode := *code + *now = now.Add(issueInterval) + second, err := service.Issue(completionRequest(PurposePasswordChange), "192.0.2.1") + if err != nil { + t.Fatal(err) + } + secondCode := *code + if err := service.VerifyBoundWithContext(first.ID, first.Account, first.Email, firstCode, first.Purpose, 9, "192.0.2.1", "browser"); err != nil { + t.Fatal(err) + } + if _, err := service.Issue(completionRequest(PurposeExternalEmailChange), "192.0.2.1"); !errors.Is(err, ErrRateLimited) { + t.Fatalf("旧验证码清除了新请求的冷却：%v", err) + } + if err := service.VerifyBoundWithContext(second.ID, second.Account, second.Email, secondCode, second.Purpose, 9, "192.0.2.1", "browser"); err != nil { + t.Fatal(err) + } + if _, err := service.Issue(completionRequest(PurposeExternalEmailChange), "192.0.2.1"); err != nil { + t.Fatal(err) + } + }) + } +} + +func TestCompletionConcurrentVerificationAndPersistentRestart(t *testing.T) { + service, code, now := completionService(t, true) + challenge, err := service.Issue(completionRequest(PurposeLogin), "192.0.2.1") + if err != nil { + t.Fatal(err) + } + results := make(chan error, 2) + var group sync.WaitGroup + for i := 0; i < 2; i++ { + group.Add(1) + go func() { + defer group.Done() + results <- service.VerifyBoundWithContext(challenge.ID, challenge.Account, challenge.Email, *code, challenge.Purpose, 9, "192.0.2.1", "browser") + }() + } + group.Wait() + close(results) + success := 0 + for err := range results { + if err == nil { + success++ + } + } + if success != 1 { + t.Fatalf("并发验证成功次数：%d", success) + } + restarted, _, _ := completionService(t, false) + restarted.now = func() time.Time { return *now } + restarted.SetRateLimitStore(service.RateLimits) + if _, err := restarted.Issue(completionRequest(PurposePasswordChange), "192.0.2.1"); err != nil { + t.Fatalf("重启后已释放的冷却仍阻塞：%v", err) + } +} + +func TestCompletionKeepsDailyLimit(t *testing.T) { + for _, persistent := range []bool{false, true} { + t.Run(fmt.Sprint(persistent), func(t *testing.T) { + service, code, now := completionService(t, persistent) + for i := 0; i < maxPerDay; i++ { + if i == maxPerHour { + *now = now.Add(issueWindow) + } + challenge, err := service.Issue(completionRequest(PurposeLogin), "192.0.2.1") + if err != nil { + t.Fatal(err) + } + if err := service.VerifyBoundWithContext(challenge.ID, challenge.Account, challenge.Email, *code, challenge.Purpose, 9, "192.0.2.1", "browser"); err != nil { + t.Fatal(err) + } + } + *now = now.Add(issueWindow) + if _, err := service.Issue(completionRequest(PurposeLogin), "192.0.2.1"); !errors.Is(err, ErrRateLimited) { + t.Fatalf("每日配额被清除：%v", err) + } + }) + } +} + +func TestCompletionCleanupFailureCanRetryWithoutCompletingBusiness(t *testing.T) { + service, code, _ := completionService(t, false) + failing := true + challenge, err := service.Store.issue(completionRequest(PurposeLogin), service.Send, func() error { + if failing { + return ErrRateLimitStoreUnavailable + } + return nil + }) + if err != nil { + t.Fatal(err) + } + verify := func() error { + return service.VerifyBoundWithContext(challenge.ID, challenge.Account, challenge.Email, *code, challenge.Purpose, 9, "192.0.2.1", "browser") + } + if err := verify(); !errors.Is(err, ErrServiceDown) { + t.Fatalf("清理失败不应返回业务成功：%v", err) + } + failing = false + if err := verify(); err != nil { + t.Fatalf("清理恢复后无法重试：%v", err) + } + if err := verify(); !errors.Is(err, ErrInvalidChallenge) { + t.Fatalf("重试成功后仍可重放：%v", err) + } +} diff --git a/backend/services/verification/rate_limit.go b/backend/services/verification/rate_limit.go index c9f11fd..571be7e 100644 --- a/backend/services/verification/rate_limit.go +++ b/backend/services/verification/rate_limit.go @@ -64,8 +64,13 @@ func NewPersistentRateLimiter(engine *xorm.Engine) (*PersistentRateLimiter, erro } func (r *PersistentRateLimiter) Allow(ctx context.Context, request RateLimitRequest, now time.Time) (bool, error) { + allowed, _, err := r.allowWithRelease(ctx, request, now) + return allowed, err +} + +func (r *PersistentRateLimiter) allowWithRelease(ctx context.Context, request RateLimitRequest, now time.Time) (bool, func() error, error) { if r == nil || r.engine == nil { - return false, ErrRateLimitStoreUnavailable + return false, nil, ErrRateLimitStoreUnavailable } if ctx == nil { ctx = context.Background() @@ -75,19 +80,20 @@ func (r *PersistentRateLimiter) Allow(ctx context.Context, request RateLimitRequ } dimensions := buildRateLimitDimensions(request) if len(dimensions) == 0 { - return false, ErrRateLimitStoreUnavailable + return false, nil, ErrRateLimitStoreUnavailable } // SQLite 使用单连接和数据库锁，MySQL/PostgreSQL 通过行锁保证跨进程原子更新。 r.mu.Lock() defer r.mu.Unlock() session := r.engine.NewSession().Context(ctx) + defer session.Close() if err := session.Begin(); err != nil { - return false, fmt.Errorf("begin verification rate limit transaction: %w", err) + return false, nil, fmt.Errorf("begin verification rate limit transaction: %w", err) } - rollback := func(err error) (bool, error) { + rollback := func(err error) (bool, func() error, error) { _ = session.Rollback() - return false, err + return false, nil, err } rows := make([]models.VerificationRateLimit, 0, len(dimensions)) for _, dimension := range dimensions { @@ -128,14 +134,49 @@ func (r *PersistentRateLimiter) Allow(ctx context.Context, request RateLimitRequ row.LastIssuedAt = timePtr(now) row.ExpiresAt = updated.Add(PersistentRateLimitStateRetention) row.UpdatedAt = updated + rows[i] = row if _, err := session.ID(row.ID).Cols("hour_started_at", "hour_count", "day_started_at", "day_count", "last_issued_at", "expires_at", "updated_at").Update(&row); err != nil { return rollback(fmt.Errorf("update verification rate limit %s: %w", dimension.typ, err)) } } if err := session.Commit(); err != nil { - return false, fmt.Errorf("commit verification rate limit transaction: %w", err) + return false, nil, fmt.Errorf("commit verification rate limit transaction: %w", err) } - return true, nil + return true, func() error { return r.releaseCooldown(rows) }, nil +} + +func (r *PersistentRateLimiter) releaseCooldown(rows []models.VerificationRateLimit) error { + r.mu.Lock() + defer r.mu.Unlock() + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + session := r.engine.NewSession().Context(ctx) + defer session.Close() + if err := session.Begin(); err != nil { + return err + } + defer session.Rollback() + for _, row := range rows { + // 同一维度已有新发送时，不清理新请求的冷却，也不重置小时和每日计数。 + var current models.VerificationRateLimit + query := session.ID(row.ID) + if r.engine.DriverName() != "sqlite3" && r.engine.DriverName() != "sqlite" { + query = query.ForUpdate() + } + has, err := query.Get(¤t) + if err != nil { + return err + } + if !has || current.LastIssuedAt == nil || current.HourCount != row.HourCount || current.DayCount != row.DayCount || current.LastIssuedAt.Unix() != row.LastIssuedAt.Unix() { + continue + } + _, err = session.Table(new(models.VerificationRateLimit)).ID(row.ID). + Update(map[string]interface{}{"last_issued_at": nil}) + if err != nil { + return err + } + } + return session.Commit() } func (r *PersistentRateLimiter) Cleanup(ctx context.Context, now time.Time) (int, error) { diff --git a/backend/services/verification/service.go b/backend/services/verification/service.go index 18828ff..ebfdd25 100644 --- a/backend/services/verification/service.go +++ b/backend/services/verification/service.go @@ -67,7 +67,7 @@ func (s *Service) Issue(request IssueRequest, remoteIP string) (Challenge, error if request.Account == "" || request.Email == "" { return Challenge{}, ErrInvalidChallenge } - allowed, limitErr := s.allow(request, remoteIP) + allowed, release, limitErr := s.allow(request, remoteIP) if limitErr != nil { s.record(EventInput{Purpose: request.Purpose, Action: models.VerificationActionIssue, Outcome: models.VerificationOutcomeFailure, Account: request.Account, Email: request.Email, RemoteIP: remoteIP}) return Challenge{}, ErrServiceDown @@ -76,7 +76,7 @@ func (s *Service) Issue(request IssueRequest, remoteIP string) (Challenge, error s.record(EventInput{Purpose: request.Purpose, Action: models.VerificationActionIssue, Outcome: models.VerificationOutcomeBlocked, Account: request.Account, Email: request.Email, RemoteIP: remoteIP}) return Challenge{}, ErrRateLimited } - challenge, err := s.Store.Issue(request, s.Send) + challenge, err := s.Store.issue(request, s.Send, release) if err != nil { s.record(EventInput{Purpose: request.Purpose, Action: models.VerificationActionIssue, Outcome: models.VerificationOutcomeFailure, Account: request.Account, Email: request.Email, RemoteIP: remoteIP}) return Challenge{}, err @@ -122,21 +122,15 @@ func (s *Service) record(input EventInput) { _ = s.Events.Record(input) } -func (s *Service) allow(request IssueRequest, remoteIP string) (bool, error) { +func (s *Service) allow(request IssueRequest, remoteIP string) (bool, func() error, error) { now := s.now() if s.RateLimits != nil { - allowed, err := s.RateLimits.Allow(context.Background(), RateLimitRequest{ + return s.RateLimits.allowWithRelease(context.Background(), RateLimitRequest{ Account: request.Account, Email: request.Email, RemoteIP: remoteIP, UserID: request.UserID, Purpose: request.Purpose, BrowserTransaction: request.BrowserTransaction, Environment: request.Environment, CredentialVersion: request.CredentialVersion, }, now) - if err != nil { - return false, err - } - if !allowed { - return false, nil - } } s.mu.Lock() defer s.mu.Unlock() @@ -146,7 +140,7 @@ func (s *Service) allow(request IssueRequest, remoteIP string) (bool, error) { } if len(s.limits) > 10000 { for key, value := range s.limits { - if now.Sub(value.last) > issueDaily { + if now.Sub(value.day) >= issueDaily { delete(s.limits, key) } } @@ -154,9 +148,10 @@ func (s *Service) allow(request IssueRequest, remoteIP string) (bool, error) { for _, key := range keys { hourLimit, dayLimit := keyLimit(key) if !counterAllowed(s.limits[key], now, hourLimit, dayLimit) { - return false, nil + return false, nil, nil } } + reserved := make(map[string]issueCounter, len(keys)) for _, key := range keys { item := s.limits[key] if item.hour.IsZero() || now.Sub(item.hour) >= issueWindow { @@ -171,8 +166,20 @@ func (s *Service) allow(request IssueRequest, remoteIP string) (bool, error) { item.hourN++ item.dayN++ s.limits[key] = item + reserved[key] = item } - return true, nil + return true, func() error { + s.mu.Lock() + defer s.mu.Unlock() + // 只释放本次发送的冷却，保留累计配额及后续发送的状态。 + for key, expected := range reserved { + if item := s.limits[key]; item == expected { + item.last = time.Time{} + s.limits[key] = item + } + } + return nil + }, nil } func keyLimit(key string) (int, int) { diff --git a/backend/services/verification/service_test.go b/backend/services/verification/service_test.go index 5f46b6b..deff19d 100644 --- a/backend/services/verification/service_test.go +++ b/backend/services/verification/service_test.go @@ -29,7 +29,7 @@ func TestServiceIssuesVerifiesAndAppliesMultiDimensionCooldown(t *testing.T) { if err := service.Verify(challenge.ID, "smail", "user@example.com", code, PurposeLogin, "127.0.0.1"); err != nil { t.Fatal(err) } - if _, err := service.Issue(IssueRequest{Purpose: PurposeLogin, Account: "smail", Email: "user@example.com"}, "127.0.0.1"); !errors.Is(err, ErrRateLimited) { + if _, err := service.Issue(IssueRequest{Purpose: PurposeLogin, Account: "smail", Email: "user@example.com"}, "127.0.0.1"); err != nil { t.Fatalf("second issue error = %v", err) } } diff --git a/backend/services/verification/store.go b/backend/services/verification/store.go index e7354a2..3f56eb0 100644 --- a/backend/services/verification/store.go +++ b/backend/services/verification/store.go @@ -67,6 +67,7 @@ type IssueRequest struct { type Delivery func(challenge Challenge, code string) error type entry struct { + onVerified func() error challenge Challenge mac [32]byte browserContextDigest [32]byte @@ -107,6 +108,10 @@ func NewStore(maxEntries int) (*Store, error) { } func (s *Store) Issue(request IssueRequest, delivery Delivery) (Challenge, error) { + return s.issue(request, delivery, nil) +} + +func (s *Store) issue(request IssueRequest, delivery Delivery, onVerified func() error) (Challenge, error) { if s == nil || delivery == nil || !IsSupportedPurpose(request.Purpose) || strings.TrimSpace(request.Account) == "" || strings.TrimSpace(request.Email) == "" { return Challenge{}, ErrInvalidChallenge } @@ -169,7 +174,7 @@ func (s *Store) Issue(request IssueRequest, delivery Delivery) (Challenge, error _, _ = mac.Write([]byte(code)) var digest [32]byte copy(digest[:], mac.Sum(nil)) - s.entries[challenge.ID] = entry{challenge: challenge, mac: digest, browserContextDigest: browserContextDigest, hasBrowserContextDigest: browserContext != ""} + s.entries[challenge.ID] = entry{challenge: challenge, mac: digest, browserContextDigest: browserContextDigest, hasBrowserContextDigest: browserContext != "", onVerified: onVerified} s.active[activeKey] = challenge.ID s.perUser[account]++ s.mu.Unlock() @@ -252,6 +257,11 @@ func (s *Store) VerifyBoundWithContext(id, account, email, code string, purpose s.entries[id] = item return ErrInvalidChallenge } + if item.onVerified != nil { + if err := item.onVerified(); err != nil { + return ErrServiceDown + } + } s.deleteLocked(id) return nil }