fix(login): rate-limit no longer latches a phantom offline on the login screen #255
+2
-1
@@ -121,7 +121,8 @@ web code exchange via `internal/vkid`, and both forward the trusted `external_id
|
||||
Rate-limit defaults (built-in): public 30/min·IP (burst 10), authenticated
|
||||
300/min·user (burst 80, sized for multi-device play), admin
|
||||
60/min·IP (burst 20, guarding the `/_gm` mount ahead of its Basic-Auth),
|
||||
email-code 5/10 min·IP (burst 2).
|
||||
email-code 5/10 min·IP (burst 4, covering request + a mistyped code + the
|
||||
correct one; defence-in-depth over the backend's per-code 5-attempt/15-min cap).
|
||||
|
||||
Every rejection increments `gateway_rate_limited_total{class}`
|
||||
(`user`/`public`/`email`/`admin`) and logs one Debug line; a reporter drains the
|
||||
|
||||
@@ -165,7 +165,12 @@ func DefaultRateLimit() RateLimitConfig {
|
||||
// because multi-device play tripped the old 120/40.
|
||||
UserPerMinute: 300, UserBurst: 80,
|
||||
AdminPerMinute: 60, AdminBurst: 20,
|
||||
EmailPer10Min: 5, EmailBurst: 2,
|
||||
// Email-code path (per IP), defence-in-depth over the backend's own per-code guards
|
||||
// (a 5-attempt cap + 15-min TTL + per-recipient send throttle). Burst 4 so the honest flow
|
||||
// — request a code, mistype once or twice, then enter the right one — is not throttled
|
||||
// mid-login: a request + wrong-code + right-code sequence exhausted the old burst of 2 and
|
||||
// tripped the limit on the correct code, which the client mis-read as going offline.
|
||||
EmailPer10Min: 5, EmailBurst: 4,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"scrabble/gateway/internal/config"
|
||||
"scrabble/gateway/internal/ratelimit"
|
||||
)
|
||||
|
||||
@@ -44,3 +45,20 @@ func TestPerWindow(t *testing.T) {
|
||||
t.Fatalf("per-window burst = %v, want [true true false]", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestEmailBurstAllowsHonestLoginFlow guards the default email-code burst against being
|
||||
// tightened back below the honest login sequence: requesting a code, submitting one wrong
|
||||
// code, then the correct one are three immediate email-class events from one IP, and all
|
||||
// must pass. A burst of 2 denied the correct-code login, which the client mis-read as going
|
||||
// offline and could not recover from on the session-less login screen. This is defence in
|
||||
// depth over the backend's own per-code attempt cap and code TTL.
|
||||
func TestEmailBurstAllowsHonestLoginFlow(t *testing.T) {
|
||||
rl := config.DefaultRateLimit()
|
||||
p := ratelimit.Per(rl.EmailPer10Min, 10*time.Minute, rl.EmailBurst)
|
||||
l := ratelimit.New()
|
||||
for i, want := range []bool{true, true, true} { // request code, wrong code, right code
|
||||
if got := l.Allow("email:198.51.100.7", p); got != want {
|
||||
t.Fatalf("honest email event %d allowed = %v, want %v", i+1, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,10 +28,15 @@ describe('toGatewayError', () => {
|
||||
});
|
||||
|
||||
describe('retryable', () => {
|
||||
it('retries any op on a rate-limit rejection (it never reached the backend)', () => {
|
||||
expect(retryable('rate_limited', 'game.submit_play')).toBe(true);
|
||||
expect(retryable('rate_limited', 'games.list')).toBe(true);
|
||||
expect(retryable('rate_limited', 'chat.post')).toBe(true);
|
||||
it('does not auto-retry a rate-limit rejection (a deliberate throttle, surfaced not spun)', () => {
|
||||
// A rate-limit is the server saying "slow down": auto-retrying cannot succeed until the bucket
|
||||
// refills, only freezes the calling button for the whole backoff and feeds the gateway's ban
|
||||
// tripwire with more rejections. It is surfaced to the caller instead. (Regression guard: the
|
||||
// old retry path also called reportOffline, which on the session-less login screen — where the
|
||||
// reachability probe can never heal — latched a stuck phantom offline.)
|
||||
expect(retryable('rate_limited', 'game.submit_play')).toBe(false);
|
||||
expect(retryable('rate_limited', 'games.list')).toBe(false);
|
||||
expect(retryable('rate_limited', 'auth.email.login')).toBe(false);
|
||||
});
|
||||
|
||||
it('retries only read-only ops on a transport unavailable (a mutation could double-apply)', () => {
|
||||
@@ -53,9 +58,12 @@ describe('retryable', () => {
|
||||
});
|
||||
|
||||
describe('isConnectionCode', () => {
|
||||
it('flags the transport/rate-limit codes the indicator covers', () => {
|
||||
it('flags only the transport connectivity code (a rate-limit is a surfaced message, not connectivity)', () => {
|
||||
expect(isConnectionCode('unavailable')).toBe(true);
|
||||
expect(isConnectionCode('rate_limited')).toBe(true);
|
||||
// A rate-limit must NOT read as connectivity: isConnectionCode gates both reportOffline (the
|
||||
// offline chrome) and handleError's toast suppression, so treating it as connectivity produced
|
||||
// a silent, stuck phantom offline on the login screen. It surfaces as error.rate_limited.
|
||||
expect(isConnectionCode('rate_limited')).toBe(false);
|
||||
expect(isConnectionCode('not_your_turn')).toBe(false);
|
||||
expect(isConnectionCode('internal')).toBe(false);
|
||||
});
|
||||
|
||||
+16
-11
@@ -2,11 +2,14 @@
|
||||
// fails at the transport level the app retries it with capped exponential backoff while showing
|
||||
// the "Connecting…" indicator, instead of flashing a red toast each time.
|
||||
//
|
||||
// Idempotency: a rate-limit rejection (ResourceExhausted) never reached the backend, so any op is
|
||||
// safe to retry. A transport 'unavailable' is ambiguous for a mutation (its response could have
|
||||
// been lost after the backend applied it), so only **read-only** ops are auto-retried on
|
||||
// 'unavailable'; a mutation is surfaced instead (its button is disabled while offline and
|
||||
// re-enables on reconnect, so the player re-issues it deliberately).
|
||||
// Retry policy: a transport 'unavailable' is ambiguous for a mutation (its response could have been
|
||||
// lost after the backend applied it), so only **read-only** ops are auto-retried on 'unavailable'; a
|
||||
// mutation is surfaced instead (its button is disabled while offline and re-enables on reconnect, so
|
||||
// the player re-issues it deliberately). A rate-limit rejection (ResourceExhausted) is NOT retried and
|
||||
// is NOT a connectivity code: it is a deliberate "slow down", so auto-retrying cannot succeed until the
|
||||
// bucket refills — it only freezes the calling button for the whole backoff and feeds the gateway's ban
|
||||
// tripwire with more rejections. It is surfaced as its own message (error.rate_limited) and never flips
|
||||
// the offline chrome, which the session-less login screen has no session-bearing probe to recover from.
|
||||
|
||||
import { Code, ConnectError } from '@connectrpc/connect';
|
||||
import { GatewayError } from './client';
|
||||
@@ -65,20 +68,22 @@ export const READ_OPS: ReadonlySet<string> = new Set([
|
||||
]);
|
||||
|
||||
/**
|
||||
* retryable reports whether a failed op should be auto-retried. A rate-limit rejection is always
|
||||
* safe (the gateway rejected it before processing); a transport 'unavailable' is retried only for
|
||||
* read-only ops, never a mutation; every other code (a domain rejection, not-found, …) is final.
|
||||
* retryable reports whether a failed op should be auto-retried. A transport 'unavailable' is retried
|
||||
* only for read-only ops, never a mutation; every other code — a rate-limit (a deliberate throttle,
|
||||
* surfaced not spun), a domain rejection, not-found, … — is final.
|
||||
*/
|
||||
export function retryable(code: string, op: string): boolean {
|
||||
if (code === 'rate_limited') return true;
|
||||
if (code === 'unavailable') return READ_OPS.has(op);
|
||||
return false;
|
||||
}
|
||||
|
||||
/** isConnectionCode reports whether a code is a transport/connectivity failure the Connecting
|
||||
* indicator covers (so the UI suppresses its red toast). */
|
||||
* indicator covers — so the UI suppresses its red toast and reportOffline may flip the offline
|
||||
* chrome. A rate-limit is deliberately NOT one: it is a genuine server signal, surfaced as its own
|
||||
* "slow down" message, never the offline chrome (which the session-less login screen, whose
|
||||
* reachability probe needs a bearer token, cannot recover from). */
|
||||
export function isConnectionCode(code: string): boolean {
|
||||
return code === 'unavailable' || code === 'rate_limited';
|
||||
return code === 'unavailable';
|
||||
}
|
||||
|
||||
/** backoffMs is the delay before retry attempt n (1-based): capped exponential growth plus a
|
||||
|
||||
Reference in New Issue
Block a user