fix: reuse active security challenge before rate limit

server/im_http.go

@@ -6243,18 +6243,6 @@ 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")
-	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
-	}
 	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)
@@ -6267,6 +6255,18 @@ func createSensitiveChallenge(wrt http.ResponseWriter, req *http.Request, princi
 		})
 		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
+	}
 	challengeID, err := randomIMToken(18)
 	if err != nil {
 		writeIMError(wrt, req, http.StatusInternalServerError, 90000, imReasonInternal, "internal error", nil)

server/im_http_test.go

@@ -2182,6 +2182,56 @@ func TestServeIMSecurityChallengeVerifyIssuesActionToken(t *testing.T) {
 	}
 }
 
+func TestServeIMSecurityChallengeCreateReturnsActiveChallengeBeforeRateLimit(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	expectIMBearer(t, ctrl, "token-1", 7, 42)
+	oldUsers := store.Users
+	oldVerifications := store.Verifications
+	now := time.Now().UTC()
+	users := mock_store.NewMockUsersPersistenceInterface(ctrl)
+	users.EXPECT().Get(types.TenantID(7), types.Uid(42)).Return(&types.User{
+		ObjHeader: types.ObjHeader{Id: types.Uid(42).String()},
+		TenantID:  7,
+		Phone:     "+8613800138000",
+	}, nil)
+	verifications := &testVerificationStore{active: &types.VerificationCode{
+		TenantID:    7,
+		ChallengeID: "challenge-active",
+		Scene:       "sensitive_action",
+		Action:      "close_youth_mode",
+		PhoneHash:   hashIMSecret(7, "sensitive_action", "+8613800138000", "phone"),
+		TargetUser:  42,
+		State:       types.VerificationStatePending,
+		ExpiresAt:   now.Add(5 * time.Minute),
+		ResendAfter: now.Add(45 * time.Second),
+	}}
+	store.Users = users
+	store.Verifications = verifications
+	defer func() {
+		store.Users = oldUsers
+		store.Verifications = oldVerifications
+	}()
+
+	req := httptest.NewRequest(http.MethodPost, "/v1/im/security/challenges",
+		bytes.NewBufferString(`{"scene":"close_youth_mode","channel":"sms"}`))
+	req.Header.Set("Authorization", "Bearer token-1")
+	rec := httptest.NewRecorder()
+
+	serveIMSecurityChallenges(rec, req)
+
+	if rec.Code != http.StatusTooManyRequests {
+		t.Fatalf("status: got %d, want %d; body=%s", rec.Code, http.StatusTooManyRequests, 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)
+	}
+}
+
 func TestServeIMBootstrapIncludesHomeCapabilities(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()