Skip to content

fix: key rate limiter by session user with per-IP fallback - #152

Merged
balebbae merged 1 commit into
mainfrom
caleb/ratelimit-per-session
Sep 2, 2026
Merged

fix: key rate limiter by session user with per-IP fallback#152
balebbae merged 1 commit into
mainfrom
caleb/ratelimit-per-session

Conversation

@balebbae

@balebbae balebbae commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

At the venue, 1200+ hackers sit behind one NAT IP, and the rate limiter keyed every request by r.RemoteAddr, so the whole room shared a single 20 req/5s budget. This PR keys the limiter by SuperTokens user ID when the request carries a verified session, and falls back to the client IP otherwise.

  • Per-user buckets. RateLimiterMiddleware resolves the session via session.GetSession(SessionRequired=false, AntiCsrfCheck=false). Only a signature-verified access token yields a user:<id> key, so a forged token cannot mint its own bucket; missing/expired/invalid tokens fall back to ip:<host>. Verification is local (cached JWKS), no core round-trip.
  • Separate IP budget. New RATELIMITER_IP_REQUESTS_COUNT (default 200/5s) for the fallback bucket, independent of the per-user RATELIMITER_REQUESTS_COUNT (20/5s). Unauthenticated /v1 traffic from a shared venue IP (login-page lookups, the first request after an access token expires) no longer competes with a single-user-sized budget.
  • Static assets exempt. The limiter moved from the router root into the /v1 route group, so the SPA shell and its assets are never throttled. /auth/* is unchanged (handled by supertokens.Middleware before the limiter, as before).
  • Port stripped from IP keys. Without a proxy header RemoteAddr includes the ephemeral port, which made every connection its own bucket in local dev.
  • 429 log lines now include the bucket key (user:… / ip:…) for diagnosing venue issues.

Limiter.Allow(ip) is renamed to Allow(key); application gains ipRateLimiter and an injectable sessionUserID resolver so tests can vary the user per request without a running core.

Test plan

  • gofmt, go vet, staticcheck ./..., go test -race ./... clean
  • New TestRateLimiterMiddleware cases: IP fallback, port stripping, real resolver with an unparseable sAccessToken cookie, same user across IPs shares a bucket, many users behind one IP get independent buckets, user and IP budgets are separate, /v1 throttled but /, /assets/*, SPA routes never are (via mount())
  • Local smoke test against air with real SuperTokens sessions for two seed users: user A 25× /v1/auth/me → 20×200, 5×429; user B on the same IP → 200; user A from a different IP → 429; anonymous 205× /v1/legal → 200×200, 5×429; / and /assets/* with the IP bucket exhausted → not 429; user A with the IP bucket exhausted → 200
  • After deploy: confirm rate limit exceeded logs carry key and that venue traffic shows user: keys

Notes for deploy

  • RATELIMITER_IP_REQUESTS_COUNT defaults to 200 if unset; tune per deployment.
  • Optional complement outside this repo: raise access_token_validity on the SuperTokens core for the event weekend so expired-token cold-opens (which land in the shared IP bucket) are rarer.
  • Follow-up, not in scope: /auth/* login endpoints remain unlimited, exactly as before this PR.

@balebbae
balebbae merged commit cca2761 into main Sep 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant