Skip to content

fix: force token refresh and clear cookies on failure to avoid logout loop#511

Merged
peppescg merged 4 commits into
mainfrom
fix/token-refresh-loop-and-logout
May 20, 2026
Merged

fix: force token refresh and clear cookies on failure to avoid logout loop#511
peppescg merged 4 commits into
mainfrom
fix/token-refresh-loop-and-logout

Conversation

@peppescg
Copy link
Copy Markdown
Collaborator

Summary

When a user's access token expires, the Cloud UI gets stuck in a redirect loop between /catalog and /api/auth/token-refresh — eventually hitting Firefox's "page isn't redirecting properly" error — instead of being cleanly logged out and bounced to /signin. Reported on staging; logs show [API Client] token near expiry, redirecting to token-refresh | path=/catalog repeating.

Root cause

The route handler called auth.api.getAccessToken, which only triggers a real refresh when the token has <5s left (Better Auth's internal threshold). The caller's near-expiry margin (isTokenNearExpiry) fires at <10s. In the 5–10s window:

  1. /catalogisTokenNearExpiry = true → redirect to /api/auth/token-refresh
  2. Route handler → getAccessToken → 8s > 5s → returns 200 OK with no Set-Cookie (no refresh happened)
  3. Browser redirected back to /catalog with unchanged cookie → loop until the browser bails out

Independently, when the refresh fails irrecoverably (e.g. refresh token revoked at the provider), the !ok → /signin branch left session_token and account_data cookies in place. The user could navigate back to /catalog and re-enter the loop because isTokenNearExpiry would still be true.

Fix

  • Switched to auth.api.refreshToken (unconditional refresh) — eliminates the 5–10s race window. If we redirected here, we actually refresh.
  • On any refresh failure, call auth.api.signOut({ asResponse: true }) and forward its Set-Cookie headers onto the /signin redirect, so session_token and account_data are genuinely cleared.
  • Export the handler as both GET and POST: Next.js redirect() is 307 (method-preserving) outside Server Actions, so a redirect originating from a Server Action POST reaches this route as POST and would otherwise 405.

Test plan

  • pnpm test src/app/api/auth/token-refresh/route.test.ts — 14/14 pass (new tests for failure-cleanup, signOut-failure resilience, and GET/POST parity)
  • pnpm test src/lib/auth src/app/api/auth — 50/50 pass
  • pnpm type-check — clean
  • pnpm lint — clean for changed files
  • Manual: let token expire on staging deploy, confirm redirect to /signin without loop
  • Manual: clear refresh token at provider, confirm clean logout instead of loop

🤖 Generated with Claude Code

… loop

Switch the token-refresh route from `auth.api.getAccessToken` (which only
refreshes inside Better Auth's 5s threshold) to `auth.api.refreshToken`
(unconditional refresh). This eliminates the 5–10s race window where the
caller's near-expiry margin would redirect to the route but Better Auth
would return 200 OK with no Set-Cookie, sending the browser back into a
redirect loop ("page isn't redirecting properly").

On refresh failure, call `auth.api.signOut` and forward its Set-Cookie
headers onto the /signin redirect so `session_token` and `account_data`
are actually cleared — otherwise the stale account_data cookie keeps
`isTokenNearExpiry` returning true and the user can fall back into the
loop on the next navigation.

Also export the handler as both GET and POST: Next.js `redirect()` uses
307 (method-preserving) outside Server Actions, so a redirect triggered
from a Server Component render that follows a Server Action POST would
otherwise hit this route as POST and 405.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 20, 2026 08:53
@peppescg peppescg self-assigned this May 20, 2026
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label May 20, 2026
- next 16.2.3 → 16.2.6 (clears GHSA advisories for App Router middleware
  bypass, image-API DoS, SSRF, RSC cache poisoning, beforeInteractive XSS,
  etc.)
- pnpm.overrides: hono ≥4.12.18 (cache middleware Vary leakage, CSS-in-style
  injection, JWT NumericDate validation)
- pnpm.overrides: kysely ≥0.28.17 (JSON-path traversal)
- pnpm.overrides: fast-uri ≥3.1.2 (path traversal + host confusion via ajv)
- pnpm.overrides: ip-address ≥10.1.1 (XSS in Address6 HTML emitters via
  @modelcontextprotocol/sdk → express-rate-limit)

`pnpm audit` now reports no known vulnerabilities.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an auth redirect loop when access tokens are near expiry by ensuring the /api/auth/token-refresh route handler performs an unconditional refresh and performs a full cookie cleanup on refresh failures, preventing users from getting stuck bouncing between /catalog and the refresh endpoint.

Changes:

  • Switch token refresh logic from auth.api.getAccessToken to auth.api.refreshToken to avoid Better Auth’s internal refresh threshold race.
  • On refresh failure/exception, call auth.api.signOut({ asResponse: true }) and forward its Set-Cookie headers onto the /signin redirect to clear auth cookies.
  • Export both GET and POST handlers and add Vitest coverage for cookie-forwarding, failure cleanup, and method parity.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/app/api/auth/token-refresh/route.ts Forces refresh, forwards Set-Cookie, adds sign-out cleanup on failure, and exports GET/POST.
src/app/api/auth/token-refresh/route.test.ts Updates mocks and adds tests for refresh + cookie forwarding, cleanup behavior, and POST handler parity.
Comments suppressed due to low confidence (2)

src/app/api/auth/token-refresh/route.ts:67

  • NextResponse.redirect() defaults to 307 (method-preserving). Since this handler is now callable via POST (per the PR description), a POST to /api/auth/token-refresh will be redirected back to safeRedirect with 307 and the browser will re-POST to /catalog (likely 405 / unintended). Consider returning 303 See Other (or conditional 303 for POST, 307 for GET) for redirects back to app pages.

This issue also appears on line 88 of the same file.

    const redirectResponse = NextResponse.redirect(
      new URL(safeRedirect, BASE_URL),
    );

src/app/api/auth/token-refresh/route.ts:93

  • Same issue on the failure path: NextResponse.redirect(new URL("/signin", BASE_URL)) will be a 307 by default, so a POST refresh attempt could be redirected to /signin as a POST. Use 303 (or conditional based on request.method) to ensure the browser navigates to /signin with GET.
async function signOutAndRedirect(
  requestHeaders: Headers,
): Promise<NextResponse> {
  const response = NextResponse.redirect(new URL("/signin", BASE_URL));
  try {
    const signOutResponse = await auth.api.signOut({

Comment thread src/app/api/auth/token-refresh/route.test.ts
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/S Small PR: 100-299 lines changed labels May 20, 2026
kysely@0.29 dropped the root re-exports of `DEFAULT_MIGRATION_LOCK_TABLE`
and `DEFAULT_MIGRATION_TABLE` (moved under `kysely/migration`), which
@better-auth/kysely-adapter@1.6.2 still imports from the root entry. The
prior `kysely >=0.28.17` override therefore floated to 0.29.2 and broke
the Turbopack build with 12 module-export errors (this also cascaded into
the Playwright E2E job, which builds before running).

Pin to `>=0.28.17 <0.29.0`: still covers the JSON-path traversal CVE
(advisory GHSA, patched in 0.28.17) without bumping past the adapter's
compatibility line.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels May 20, 2026
`NextResponse.redirect()` defaults to 307 (method-preserving). Now that this
route also accepts POST (added in the previous commit), a refresh that
arrived via POST would have its outbound redirect to /catalog or /signin
sent as POST too — both of which only handle GET, producing 405.

Switch both the success and failure redirects to 303 See Other, which
unconditionally instructs the browser to follow with GET. Caught by Copilot
review on the PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels May 20, 2026
@kantord kantord self-requested a review May 20, 2026 14:05
@peppescg peppescg merged commit 83c8c70 into main May 20, 2026
10 checks passed
@peppescg peppescg deleted the fix/token-refresh-loop-and-logout branch May 20, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants