From 7f6995d8fec578e35afd42bffaeb4ced91899762 Mon Sep 17 00:00:00 2001 From: richjohnrambo Date: Wed, 2 Sep 2026 10:51:15 +0800 Subject: [PATCH] Disable sensitive challenge rate limit for test --- server/im_http.go | 73 ++++++++++++++++++++++-------------------- server/im_http_test.go | 17 +++++----- 2 files changed, 46 insertions(+), 44 deletions(-) diff --git a/server/im_http.go b/server/im_http.go index b870b20..b2d5fbb 100644 --- a/server/im_http.go +++ b/server/im_http.go @@ -57,19 +57,20 @@ const ( ) const ( - imVerificationCodeLength = 6 - imVerificationExpiresInSeconds = 300 - imVerificationResendAfterSeconds = 60 - imVerificationMaxAttempts = 5 - imVerificationCreateRateWindow = time.Hour - imActionTokenExpiresInSeconds = 300 - imIndependentPasswordMaxAttempts = 5 - imIndependentPasswordLockSeconds = 300 - imQRCodeDefaultExpiresInSeconds = 86400 - imQRCodeMaxExpiresInSeconds = 7 * 86400 - imPublicUploadMaxBytes = 10 << 20 - groupMaxAdminCount = 10 - reportPreviewMaxRunes = 120 + imVerificationCodeLength = 6 + imVerificationExpiresInSeconds = 300 + imVerificationResendAfterSeconds = 60 + imVerificationMaxAttempts = 5 + imVerificationCreateRateWindow = time.Hour + imSensitiveChallengeRateLimitEnabled = false + imActionTokenExpiresInSeconds = 300 + imIndependentPasswordMaxAttempts = 5 + imIndependentPasswordLockSeconds = 300 + imQRCodeDefaultExpiresInSeconds = 86400 + imQRCodeMaxExpiresInSeconds = 7 * 86400 + imPublicUploadMaxBytes = 10 << 20 + groupMaxAdminCount = 10 + reportPreviewMaxRunes = 120 ) type imHTTPResponse struct { @@ -6245,29 +6246,31 @@ func currentUserPhone(tenantID types.TenantID, uid types.Uid) (string, error) { func createSensitiveChallenge(wrt http.ResponseWriter, req *http.Request, principal *imPrincipal, action, phone string) { now := time.Now().UTC().Round(time.Millisecond) phoneHash := hashIMSecret(principal.TenantID, "sensitive_action", phone, "phone") - active, err := store.Verifications.GetActive(principal.TenantID, "sensitive_action", phoneHash, now) - if err != nil && !errors.Is(err, types.ErrNotFound) { - writeIMError(wrt, req, http.StatusInternalServerError, 90000, imReasonInternal, "internal error", nil) - return - } - if active != nil && now.Before(active.ResendAfter) { - writeIMError(wrt, req, http.StatusTooManyRequests, 30020, imReasonVerificationRate, "verification rate limited", map[string]any{ - "challenge_id": active.ChallengeID, - "retry_after_seconds": int(time.Until(active.ResendAfter).Seconds()) + 1, - }) - return - } - if allowed, _, ttl, err := store.AllowRate(principal.TenantID, "verification:create:sensitive_action:"+phoneHash, imVerificationMaxAttempts, imVerificationCreateRateWindow); err != nil { - logs.Warn.Println("im sensitive verification create: redis rate limit failed", principal.TenantID, err) - } else if !allowed { - retryAfter := int(ttl.Seconds()) - if retryAfter < 1 { - retryAfter = int(imVerificationCreateRateWindow.Seconds()) + if imSensitiveChallengeRateLimitEnabled { + active, err := store.Verifications.GetActive(principal.TenantID, "sensitive_action", phoneHash, now) + if err != nil && !errors.Is(err, types.ErrNotFound) { + writeIMError(wrt, req, http.StatusInternalServerError, 90000, imReasonInternal, "internal error", nil) + return + } + if active != nil && now.Before(active.ResendAfter) { + writeIMError(wrt, req, http.StatusTooManyRequests, 30020, imReasonVerificationRate, "verification rate limited", map[string]any{ + "challenge_id": active.ChallengeID, + "retry_after_seconds": int(time.Until(active.ResendAfter).Seconds()) + 1, + }) + return + } + if allowed, _, ttl, err := store.AllowRate(principal.TenantID, "verification:create:sensitive_action:"+phoneHash, imVerificationMaxAttempts, imVerificationCreateRateWindow); err != nil { + logs.Warn.Println("im sensitive verification create: redis rate limit failed", principal.TenantID, err) + } else if !allowed { + retryAfter := int(ttl.Seconds()) + if retryAfter < 1 { + retryAfter = int(imVerificationCreateRateWindow.Seconds()) + } + writeIMError(wrt, req, http.StatusTooManyRequests, 30020, imReasonVerificationRate, "verification rate limited", map[string]any{ + "retry_after_seconds": retryAfter, + }) + return } - writeIMError(wrt, req, http.StatusTooManyRequests, 30020, imReasonVerificationRate, "verification rate limited", map[string]any{ - "retry_after_seconds": retryAfter, - }) - return } challengeID, err := randomIMToken(18) if err != nil { diff --git a/server/im_http_test.go b/server/im_http_test.go index 72e0e0d..622397b 100644 --- a/server/im_http_test.go +++ b/server/im_http_test.go @@ -2194,12 +2194,13 @@ func TestServeIMSecurityChallengeVerifyIssuesActionToken(t *testing.T) { } } -func TestServeIMSecurityChallengeCreateReturnsActiveChallengeBeforeRateLimit(t *testing.T) { +func TestServeIMSecurityChallengeCreateSkipsRateLimitWhenDisabled(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() expectIMBearer(t, ctrl, "token-1", 7, 42) oldUsers := store.Users oldVerifications := store.Verifications + oldSender := sendIMVerificationCode now := time.Now().UTC() users := mock_store.NewMockUsersPersistenceInterface(ctrl) users.EXPECT().Get(types.TenantID(7), types.Uid(42)).Return(&types.User{ @@ -2220,9 +2221,11 @@ func TestServeIMSecurityChallengeCreateReturnsActiveChallengeBeforeRateLimit(t * }} store.Users = users store.Verifications = verifications + sendIMVerificationCode = func(_ types.TenantID, _, _, _, _ string) error { return nil } defer func() { store.Users = oldUsers store.Verifications = oldVerifications + sendIMVerificationCode = oldSender }() req := httptest.NewRequest(http.MethodPost, "/v1/im/security/challenges", @@ -2232,15 +2235,11 @@ func TestServeIMSecurityChallengeCreateReturnsActiveChallengeBeforeRateLimit(t * serveIMSecurityChallenges(rec, req) - if rec.Code != http.StatusTooManyRequests { - t.Fatalf("status: got %d, want %d; body=%s", rec.Code, http.StatusTooManyRequests, rec.Body.String()) + if rec.Code != http.StatusOK { + t.Fatalf("status: got %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) } - data := decodeIMHTTPResponse(t, rec).Details.(map[string]any) - if data["challenge_id"] != "challenge-active" { - t.Fatalf("expected active challenge id, got %#v", data) - } - if verifications.created != nil { - t.Fatalf("should not create a new challenge while resend is cooling down: %#v", verifications.created) + if verifications.created == nil || verifications.created.Action != "close_youth_mode" { + t.Fatalf("expected a new challenge while sensitive challenge rate limit is disabled: %#v", verifications.created) } }