fix: stop the NextAuth signout loop that rate-limited every user behind one IP - #453
Merged
Conversation
One client issued 228,211 requests in a day, peaking at 6,230 per
minute: 107,612 session fetches, 36,565 csrf, 35,685 signouts. It
exhausted the backend's per-IP rate limit (1000/hour for non-GET) and
returned 429s to every other user behind the same NAT address. 429s went
0 -> 7 -> 19,991 over three days.
Two pieces combined into an unbounded cycle:
1. The jwt callback returned {...token, error: 'RefreshAccessTokenError'}
while keeping the stale expires_at, so the next session fetch saw an
expired token and retried the same refresh, which failed the same way.
2. SessionGuard reacted to that error by calling signOut({redirect:
false}) with no guard - so useSession refetched, the error was still
present, and the effect fired again. Nothing bounded it.
It was also self-sustaining: once over the limit, the signOut call
itself returned 429, so the session was never cleared and the condition
could not resolve.
Fixes both halves. The jwt callback short-circuits when the token
already carries the error rather than retrying a refresh that cannot
succeed - the session is unrecoverable at that point and hammering
Keycloak helps nobody. SessionGuard tracks whether it has already acted,
so cleanup runs once per error rather than once per render, and swallows
a failed signOut instead of spinning on it. The flag clears when the
session recovers, so a later expiry is still handled.
Also declares `error` on the JWT type. It compiled without that because
JWT extends Record<string, unknown>, but the read was typed `unknown`.
Trigger, for the record: the backend row-lock fix took requests from
3-60s to 0.2-0.5s. The loop pre-existed; the slow backend had been
throttling it below the rate limit.
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.
One client issued 228,211 requests in a day, peaking at 6,230 per minute. It exhausted the backend's per-IP rate limit and returned 429 to every other user behind the same NAT address — which is what was reported as "dev is broken for most users".
GET /api/auth/sessionGET /api/auth/csrfPOST /api/auth/signoutPOST /api/graphql429s by day: 0 → 7 → 19,991.
The loop
Two pieces, both required:
1.
options.ts— a failed refresh kept the stale tokenexpires_atstayed in the past, so the next session fetch saw an expired token and retried the same refresh, which failed the same way.2.
SessionGuard.tsx— reacted by signing out, without leavingNo guard, no backoff, no redirect.
useSessionrefetched, the error was still present, the effect fired again — at network speed.Self-sustaining: once over the rate limit, the
signOutcall itself returned 429, so the session was never cleared and the condition could not resolve. That is why the curve escalated rather than settling.The fix
SessionGuardtracks whether it has already acted, so cleanup runs once per error rather than once per render, and swallows a failedsignOutrather than spinning on it. The flag clears when the session recovers, so a later expiry is still handled.erroron theJWTtype. It compiled without it becauseJWT extends Record<string, unknown>— but the read was typedunknown.Trigger, for the record
The backend row-lock fix (DataSpaceBackend#136) took requests from 3–60s to 0.2–0.5s. The loop pre-existed; the slow backend had been throttling it below the rate limit. Making the backend fast removed that accidental brake.
Coverage
CivicDataSpace-test#24 adds regression tests: page loads must not issue a runaway number of
/api/auth/requests (healthy dev measures 4 on the homepage, 1 on/datasets; ceiling 15), and an anonymous load must issue zero signouts.No test could have caught this before — every page rendered, every element was present, no console error appeared. The damage was purely in request volume, which nothing asserted on.