fix(next-auth): don't duplicate the session cookie on internal auth action routes - #13499
Open
OsamaAnsar wants to merge 1 commit into
Open
OsamaAnsar wants to merge 1 commit into
OsamaAnsar wants to merge 1 commit into
Conversation
…ction routes
`handleAuth()` (the `export { auth as middleware }` wrapper) runs on every
middleware-matched request, including the POST to `/api/auth/signout`
itself. For a JWT session it unconditionally called `getSession()`, which
re-signs and re-sets the session cookie with a fresh expiry as a side
effect of merely reading it, and then unconditionally appended that
cookie onto whatever the actual route being hit later produced.
For `/api/auth/signout`, the real signout route handler (invoked
separately by Next.js once middleware lets the request through) already
sets its own `Max-Age=0` clearing cookie. With handleAuth also appending
its freshly-signed live cookie, the final response ends up carrying two
conflicting `Set-Cookie` headers for the same cookie name. Per RFC 6265
4.1.1 this is undefined behavior, and which one a given browser keeps
depends on header order, which differs by hosting runtime (Vercel/Node vs.
Cloudflare Workers vs. Netlify) -- so sign-out can silently fail to clear
the session on some hosts.
Fix: skip appending handleAuth's own session-refresh cookie when the
request itself is hitting one of NextAuth's internal action routes
(signout, callback, session, etc. -- reusing the existing `actions` set
already used by `isSameAuthAction`). Those routes manage the session
cookie themselves as part of handling the action. `auth`/`req.auth` is
still computed exactly as before for the `authorized` callback and
user middleware, so this only changes whether the redundant/conflicting
cookie gets appended, not how the session is read. Normal page requests
are unaffected.
Fixes nextauthjs#12909.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@OsamaAnsar is attempting to deploy a commit to the authjs Team on Vercel. A member of the Team first needs to authorize it. |
This branch has not been deployed
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
handleAuth()(the standardexport { auth as middleware }wrapper most Next.js middleware setups use) runs on every middleware-matched request, including the POST to/api/auth/signoutitself.packages/next-auth/src/lib/index.ts(handleAuth, previously around lines 249-305, now 249-320 after the fix): for a JWT session,getSession()(called unconditionally near the top ofhandleAuth) re-signs and re-sets the session cookie with a fresh expiry as a side effect of merely reading it.handleAuththen unconditionally appended that refreshedSet-Cookieonto whatever the actual route later produced (previously around lines 298-302), regardless of which route is being hit.packages/core/src/lib/actions/session.ts:71-79: the JWT branch of thesessionaction, which is whatgetSession()invokes internally — this is the code that re-signs the cookie on every read.Net effect: for a POST to
/api/auth/signout, the real signout route handler (invoked separately by Next.js once middleware lets the request through) sets its ownMax-Age=0clearing cookie, andhandleAuthalso appends its own freshly-signed live session cookie on top of that. The response ends up with two conflictingSet-Cookieheaders for the same cookie name. Per RFC 6265 §4.1.1 this is undefined behavior — which cookie a browser keeps depends on header order, which differs by hosting runtime (Vercel/Node vs. Cloudflare Workers vs. Netlify) — so sign-out can silently fail to actually clear the session on some hosts.Fixes #12909.
Fix
handleAuthnow skips appending its own session-refreshSet-Cookiewhen the request itself is hitting one of NextAuth's internal action routes (signin,signout,callback,session,csrf,providers,verify-request,error— reusing the existingactionsset already used by the neighboringisSameAuthActionhelper). Those routes manage the session cookie themselves as part of handling the action (e.g.signoutclears it), sohandleAuthappending a redundant/conflicting cookie on top serves no purpose and is actively wrong forsignout.auth/req.authis still computed exactly as before (thegetSession()call and theauthorizedcallback / user middleware logic are untouched) — the fix only guards the final cookie-append step, so it's scoped specifically to the duplicate-cookie case and doesn't change how the session is read or how authorization decisions are made. Ordinary page requests are completely unaffected.Test plan
Added
packages/next-auth/test/middleware-signout-cookie.test.tswith 3 tests:/api/auth/signoutthroughhandleAuth(viaauth(req, event)) results in zeroSet-Cookieheaders for the session cookie fromhandleAuthitself.Max-Age=0) value — proving normal session-refresh behavior is unaffected.handleAuth's response merged with the real/api/auth/signoutroute handler's response (driven directly through@auth/core'sAuth(), the way Next.js would merge middleware and route-handler headers) results in exactly oneSet-Cookiefor the session cookie, and it's theMax-Age=0clearing one — not a live one.Ran:
pnpm --filter next-auth exec vitest run -c ../utils/vitest.config.ts test/middleware-signout-cookie.test.ts— all 3 pass.pnpm --filter next-auth exec vitest run -c ../utils/vitest.config.ts(fullnext-authsuite) — all tests pass except a pre-existing, unrelated failure intest/env.test.ts(stale import path requiring the package to be pre-built; reproduces identically onmainwithout this change).pnpm --filter @auth/core exec vitest run -c ../utils/vitest.config.ts(full@auth/coresuite, includingtest/actions/session.test.ts) — all 160 tests pass, confirming normal session-refresh behavior is unaffected.tsc --noEmitandeslinton the changed files — clean (only pre-existing warnings on unrelated lines).Confirmed the test catches the regression: reverted just the source fix in
packages/next-auth/src/lib/index.ts(kept the new test), re-ran — 2 of the 3 new tests failed exactly as expected (one asserting 0 cookies got 1; the merged-response test asserting 1 cookie got 2, i.e. the live + the clearing cookie both present). Restored the fix and re-ran — all 3 pass again.🤖 Generated with Claude Code