fix: key rate limiter by session user with per-IP fallback - #152
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.RateLimiterMiddlewareresolves the session viasession.GetSession(SessionRequired=false, AntiCsrfCheck=false). Only a signature-verified access token yields auser:<id>key, so a forged token cannot mint its own bucket; missing/expired/invalid tokens fall back toip:<host>. Verification is local (cached JWKS), no core round-trip.RATELIMITER_IP_REQUESTS_COUNT(default 200/5s) for the fallback bucket, independent of the per-userRATELIMITER_REQUESTS_COUNT(20/5s). Unauthenticated/v1traffic 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./v1route group, so the SPA shell and its assets are never throttled./auth/*is unchanged (handled bysupertokens.Middlewarebefore the limiter, as before).RemoteAddrincludes the ephemeral port, which made every connection its own bucket in local dev.user:…/ip:…) for diagnosing venue issues.Limiter.Allow(ip)is renamed toAllow(key);applicationgainsipRateLimiterand an injectablesessionUserIDresolver so tests can vary the user per request without a running core.Test plan
gofmt,go vet,staticcheck ./...,go test -race ./...cleanTestRateLimiterMiddlewarecases: IP fallback, port stripping, real resolver with an unparseablesAccessTokencookie, same user across IPs shares a bucket, many users behind one IP get independent buckets, user and IP budgets are separate,/v1throttled but/,/assets/*, SPA routes never are (viamount())airwith 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 → 200rate limit exceededlogs carrykeyand that venue traffic showsuser:keysNotes for deploy
RATELIMITER_IP_REQUESTS_COUNTdefaults to 200 if unset; tune per deployment.access_token_validityon the SuperTokens core for the event weekend so expired-token cold-opens (which land in the shared IP bucket) are rarer./auth/*login endpoints remain unlimited, exactly as before this PR.