From dee567e34001ecb5c2d7d483fe536e30334259dd Mon Sep 17 00:00:00 2001 From: Saqib Date: Thu, 3 Sep 2026 14:47:23 +0530 Subject: [PATCH] fix: stop the NextAuth signout loop 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, 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. --- app/api/auth/[...nextauth]/options.ts | 15 +++++++++++++ components/SessionGuard.tsx | 32 ++++++++++++++++++++++++--- types/next-auth-d.ts | 16 ++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/app/api/auth/[...nextauth]/options.ts b/app/api/auth/[...nextauth]/options.ts index 6bec0f42..4478a5f4 100644 --- a/app/api/auth/[...nextauth]/options.ts +++ b/app/api/auth/[...nextauth]/options.ts @@ -55,6 +55,21 @@ export const authOptions: AuthOptions = { token.expires_at = account.expires_at; token.refresh_token = account.refresh_token; return token; + } else if (token.error === 'RefreshAccessTokenError') { + // A previous refresh already failed. Return the token as-is instead of + // retrying. + // + // This callback runs on every session fetch, and a failed refresh used to + // leave `expires_at` in the past - so every subsequent fetch attempted the + // same refresh, and failed the same way. Combined with SessionGuard reacting + // to the error by signing out (which triggers another session fetch), that + // produced an unbounded loop: 35,685 signout calls and 107,612 session + // fetches from a single client in one day, peaking at 6,230 requests/minute. + // + // The session is unrecoverable at this point. The user has to sign in again, + // which SessionGuard handles; hammering Keycloak in the meantime helps + // nobody. + return token; } else if (nowTimeStamp < (token.expires_at as number)) { // token has not expired yet, return it return token; diff --git a/components/SessionGuard.tsx b/components/SessionGuard.tsx index 6fc530a4..51c61989 100644 --- a/components/SessionGuard.tsx +++ b/components/SessionGuard.tsx @@ -1,6 +1,6 @@ 'use client'; -import { ReactNode, useEffect } from 'react'; +import { ReactNode, useEffect, useRef } from 'react'; import { usePathname } from 'next/navigation'; import { signIn, signOut, useSession } from 'next-auth/react'; @@ -8,11 +8,37 @@ export default function SessionGuard({ children }: { children: ReactNode }) { const { data } = useSession(); const pathname = usePathname(); + // Guards against re-running the cleanup for an error we have already acted on. + // + // Without this, an expired session produced an unbounded loop: the effect called + // signOut(), useSession refetched, the session still carried + // RefreshAccessTokenError, and the effect fired again - as fast as the network + // allowed. One client produced 35,685 signout calls and 107,612 session fetches in + // a day, peaking at 6,230 requests/minute, which exhausted the backend's per-IP + // rate limit and returned 429s to every other user behind the same NAT address. + // + // The loop was self-sustaining: once rate-limited, the signOut call itself started + // failing, so the session was never cleared and the condition never resolved. + const cleanupAttempted = useRef(false); + useEffect(() => { - if (data?.error !== 'RefreshAccessTokenError') return; + if (data?.error !== 'RefreshAccessTokenError') { + // Session recovered (or the user signed in again) - allow a future cleanup. + cleanupAttempted.current = false; + return; + } + + if (cleanupAttempted.current) return; + cleanupAttempted.current = true; const clearExpiredSession = async () => { - await signOut({ redirect: false }); + try { + await signOut({ redirect: false }); + } catch { + // Deliberately swallowed. If signOut fails - the backend being rate-limited + // is exactly when it will - retrying immediately is what created the loop. + // The flag above stays set, so this settles instead of spinning. + } if (pathname.includes('dashboard')) { signIn('keycloak'); diff --git a/types/next-auth-d.ts b/types/next-auth-d.ts index e10b54a7..fe092a75 100644 --- a/types/next-auth-d.ts +++ b/types/next-auth-d.ts @@ -15,3 +15,19 @@ declare module 'next-auth' { } & DefaultSession['user']; } } + +declare module 'next-auth/jwt' { + /** + * The shape of the token returned by the `jwt` callback in + * app/api/auth/[...nextauth]/options.ts. + * + * `error` is declared here because that callback both writes it (on a failed + * refresh) and reads it (to avoid retrying a refresh that has already failed). + * JWT extends Record, so this compiled without the declaration - + * but the read was typed `unknown`, which is exactly the sort of thing that + * silently stops meaning what you think it means. + */ + interface JWT { + error?: string; + } +}