diff --git a/EXAMPLES.md b/EXAMPLES.md index 27b748e0e..a656908e0 100644 --- a/EXAMPLES.md +++ b/EXAMPLES.md @@ -151,6 +151,7 @@ - [Customizing Transaction Cookie Expiration](#customizing-transaction-cookie-expiration) - [Transaction Management Modes](#transaction-management-modes) - [Transaction Cookie Options](#transaction-cookie-options) + - [Preventing "431 Request Header Fields Too Large" Errors](#preventing-431-request-header-fields-too-large-errors) - [Database sessions](#database-sessions) - [Using Client-Initiated Backchannel Authentication](#using-client-initiated-backchannel-authentication) - [Connected Accounts](#connected-accounts) @@ -242,6 +243,9 @@ The second option is through the query parameters to the `/auth/login` endpoint Login ``` +> [!NOTE] +> A default `` is safe — the SDK returns `204 No Content` on AUTO prefetches without writing a transaction cookie. Avoid `` (FULL prefetch), which is indistinguishable from a real navigation server-side. See [Preventing "431 Request Header Fields Too Large" Errors](#preventing-431-request-header-fields-too-large-errors). + ### Social Login To skip the Universal Login page and send users directly to a social provider, pass the `connection` parameter with the Auth0 connection name: @@ -329,6 +333,9 @@ For example: `/auth/login?returnTo=/dashboard` would redirect the user to the `/ > [!NOTE] > The URL specified as `returnTo` parameters must be registered in your client's **Allowed Callback URLs**. +> [!IMPORTANT] +> `returnTo`, `scope`, and `audience` query parameters on `/auth/login` are stored inside the encrypted transaction cookie. Any single field longer than 2 KB is silently clamped back to its default value and a one-time warning is logged, to keep the transaction cookie under the browser and proxy header limits (~4 KB per cookie). Keep these values short. For `returnTo` specifically, the fallback is `signInReturnToPath` (the SDK-configured default post-login path). + ### Redirecting the user after logging out The `returnTo` parameter can be appended to the logout to specify where you would like to redirect the user after they have logged out. @@ -575,6 +582,9 @@ export async function middleware(request: NextRequest) { ## Protecting a Server-Side Rendered (SSR) Page +> [!TIP] +> Prefer `withPageAuthRequired` (below) over redirecting to `/auth/login` from middleware for protected pages. When a prefetch follows the redirect to `/auth/login`, the SDK returns `204 No Content` (no transaction cookie written). A middleware redirect achieves the same result, but `withPageAuthRequired` keeps the auth logic co-located with the page. See [Preventing "431 Request Header Fields Too Large" Errors](#preventing-431-request-header-fields-too-large-errors). + #### Page Router Requests to `/pages/profile` without a valid session cookie will be redirected to the login page. @@ -617,6 +627,9 @@ export default auth0.withPageAuthRequired( To protect a Client-Side Rendered (CSR) page, you can use the `withPageAuthRequired` higher-order function. Requests to `/profile` without a valid session cookie will be redirected to the login page. +> [!TIP] +> Using `withPageAuthRequired` (rather than a middleware redirect to `/auth/login`) keeps auth logic co-located with the page. Prefetches that follow the redirect to `/auth/login` are handled by the SDK's `204` guard, so no transaction cookie is written. See [Preventing "431 Request Header Fields Too Large" Errors](#preventing-431-request-header-fields-too-large-errors). + ```tsx // app/profile/page.tsx "use client"; @@ -4131,20 +4144,90 @@ const authClient = new Auth0Client({ **Use Single Transaction Mode When:** -- You want to prevent cookie accumulation issues in applications with frequent login attempts -- You prefer simpler transaction management +- You want the simplest possible transaction management - Users typically don't need multiple concurrent login flows -- You're experiencing cookie header size limits due to abandoned transaction cookies edge cases + +> [!NOTE] +> In single transaction mode, starting a new login while one is already in progress overwrites the existing `__txn_` cookie rather than rejecting the new attempt. If a user has two tabs open and starts a login in both, only the most recently started login can complete; the other tab's callback will fail because its transaction state was overwritten. This is expected in single transaction mode — use the default parallel mode if concurrent logins across tabs need to succeed. ### Transaction Cookie Options -| Option | Type | Description | -| ---------------------- | ----------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| cookieOptions.maxAge | `number` | The expiration time for transaction cookies in seconds. Defaults to `3600` (1 hour). After this time, abandoned transaction cookies will expire automatically. | -| cookieOptions.prefix | `string` | The prefix for transaction cookie names. Defaults to `__txn_`. In parallel mode, cookies are named `__txn_{state}`. In single mode, just `__txn_`. | -| cookieOptions.sameSite | `"strict" \| "lax" \| "none"` | Controls when the cookie is sent with cross-site requests. Defaults to `"lax"`. | -| cookieOptions.secure | `boolean` | When `true`, the cookie will only be sent over HTTPS connections. Derived from `appBaseUrl` when available; enforced in production when `appBaseUrl` is omitted. | -| cookieOptions.path | `string` | Specifies the URL path for which the cookie is valid. Defaults to `"/"`. | +| Option | Type | Description | +| ----------------------------------------- | ----------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `transactionCookie.maxAge` | `number` | Expiration time for transaction cookies in seconds. Defaults to `3600` (1 hour). After this time, abandoned cookies expire automatically. | +| `transactionCookie.prefix` | `string` | Prefix for transaction cookie names. Defaults to `__txn_`. In parallel mode, cookies are named `__txn_{state}`; in single mode, just `__txn_`. | +| `transactionCookie.sameSite` | `"strict" \| "lax" \| "none"` | Controls when the cookie is sent with cross-site requests. Defaults to `"lax"`. | +| `transactionCookie.secure` | `boolean` | When `true`, the cookie is only sent over HTTPS. Derived from `appBaseUrl` when available; enforced in production when `appBaseUrl` is omitted. | +| `transactionCookie.path` | `string` | URL path for which the cookie is valid. Defaults to `"/"`. | + +### Preventing "431 Request Header Fields Too Large" Errors + +If your app shows `431 Request Header Fields Too Large` errors, `__txn_*` cookies have grown beyond your server's header size limit. + +**This is fixed in the current SDK version.** The SDK now: + +1. Returns `204 No Content` on Next.js prefetch requests to `/auth/login` (detected via prefetch headers such as `next-router-prefetch`, `purpose`, `sec-purpose`, and `x-middleware-prefetch`), so no `__txn_*` cookie is written for a flow that will never complete. +2. Automatically evicts accumulated `__txn_*` cookies once their combined size reaches a fixed internal limit (3500 bytes, roughly six concurrent in-flight logins) — oldest-first (FIFO) by creation timestamp — before writing the new cookie. Only transaction cookies are measured and evicted; the session and other cookies are never touched. This limit is not configurable. + +#### Recommended practices to avoid transaction cookie accumulation + +Even with the automatic protections above, follow these two practices so login flows are only started by real user navigation: + +**1. Avoid ``.** + +A default `` (AUTO prefetch) is safe — the SDK detects the prefetch header and returns `204 No Content` without writing a transaction cookie. However, `` triggers a FULL prefetch which sends no detectable prefetch header, so the SDK cannot distinguish it from a real navigation and will start a login flow. Use a plain `` tag or `` if you need to be explicit: + +```tsx +// ✅ Safe — default Link, AUTO prefetch is caught by the 204 guard +Sign In + +// ✅ Safe — plain anchor never prefetches +Sign In + +// ✅ Safe — prefetch explicitly disabled + + Sign In + + +// ❌ Avoid — FULL prefetch is indistinguishable from a real navigation server-side + + Sign In + +``` + +**2. Prefer `withPageAuthRequired` over middleware redirects to protect pages.** + +`withPageAuthRequired` redirects to the login route from inside the React Server Component render. When a prefetch follows that redirect to `/auth/login`, the SDK returns `204 No Content` — so `handleLogin` is never called and no `__txn_*` cookie is written. A middleware redirect to `/auth/login` behaves the same way: prefetches that follow it are also caught by the `204` guard. The preference for `withPageAuthRequired` is about keeping auth logic co-located with the page, not a difference in prefetch behaviour. + +```tsx +// ✅ Preferred — auth logic co-located with the page; prefetches caught by the 204 guard +export default auth0.withPageAuthRequired(async function Page() { + return
Protected content
; +}, { returnTo: "/protected" }); +``` + +```ts +// ✅ Also safe — prefetches following this redirect are caught by the 204 guard +export async function middleware(request: NextRequest) { + const session = await auth0.getSession(request); + if (!session) { + return NextResponse.redirect(new URL("/auth/login", request.nextUrl.origin)); + } + return NextResponse.next(); +} +``` + +If you are running an older SDK version without the automatic protections above, adding `prefetch={false}` to `` components pointing to your login route is the key fallback. + +If accumulation persists after upgrading, shorten the transaction cookie lifetime so abandoned logins expire sooner: + +```ts +export const auth0 = new Auth0Client({ + transactionCookie: { + maxAge: 600, // shorten TTL to 10 minutes (default 3600) + }, +}); +``` ## Database sessions diff --git a/README.md b/README.md index 92089e80a..4291ef160 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ export default async function Home() { ``` > [!IMPORTANT] -> You must use `` tags instead of the `` component to ensure that the routing is not done client-side as that may result in some unexpected behavior. +> A default `` is safe — the SDK detects the AUTO prefetch header and returns `204 No Content` without writing a transaction cookie. Avoid `` (FULL prefetch): it sends no detectable prefetch header, so the SDK cannot distinguish it from a real navigation and will start a login flow. Use a plain `` tag or `` if you need to be safe across all prefetch modes. See [Preventing "431 Request Header Fields Too Large" Errors](https://github.com/auth0/nextjs-auth0/blob/main/EXAMPLES.md#preventing-431-request-header-fields-too-large-errors) for details. ## Customizing the client diff --git a/src/server/auth-client.test.ts b/src/server/auth-client.test.ts index 3f0b0e6d4..8d9f4935c 100644 --- a/src/server/auth-client.test.ts +++ b/src/server/auth-client.test.ts @@ -27,7 +27,7 @@ import { TokenRevocationErrorCode } from "../errors/index.js"; import { getDefaultRoutes } from "../test/defaults.js"; -import { generateSecret } from "../test/utils.js"; +import { generateSecret, stripTransactionValuePrefix } from "../test/utils.js"; import { AccessTokenSet, RESPONSE_TYPES, @@ -1723,7 +1723,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -2053,7 +2053,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -2407,7 +2407,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -2454,7 +2454,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -2497,7 +2497,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -2548,7 +2548,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -2603,7 +2603,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -2793,7 +2793,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie.value, + stripTransactionValuePrefix(transactionCookie.value), secret )) as jose.JWTDecryptResult ).payload @@ -2957,7 +2957,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie.value, + stripTransactionValuePrefix(transactionCookie.value), secret )) as jose.JWTDecryptResult ).payload @@ -3044,7 +3044,10 @@ ca/T0LLtgmbMmxSv/MmzIg== const state = transactionCookie.name.replace("__txn_", ""); expect(transactionCookie).toBeDefined(); expect( - (await decrypt(transactionCookie!.value, secret))!.payload + (await decrypt( + stripTransactionValuePrefix(transactionCookie!.value), + secret + ))!.payload ).toEqual( expect.objectContaining({ nonce: expect.any(String), @@ -7593,7 +7596,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -7740,7 +7743,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload @@ -8183,7 +8186,7 @@ ca/T0LLtgmbMmxSv/MmzIg== expect( ( (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult ).payload diff --git a/src/server/auth-client.ts b/src/server/auth-client.ts index f5d5df5e4..a82a217c7 100644 --- a/src/server/auth-client.ts +++ b/src/server/auth-client.ts @@ -156,6 +156,7 @@ import { buildForwardedResponseHeaders, transformTargetUrl } from "../utils/proxy.js"; +import { isNonNavigationalRequest } from "../utils/request.js"; import { ensureDefaultScope, getScopeForAudience @@ -198,7 +199,12 @@ import { FetcherMinimalConfig } from "./fetcher.js"; import { AbstractSessionStore } from "./session/abstract-session-store.js"; -import { TransactionState, TransactionStore } from "./transaction-store.js"; +import { + clampReturnTo, + clampTransactionField, + TransactionState, + TransactionStore +} from "./transaction-store.js"; import { filterDefaultIdTokenClaims } from "./user.js"; export type BeforeSessionSavedHook = ( @@ -675,6 +681,19 @@ export class AuthClient { const method = req.method; if (method === "GET" && sanitizedPathname === this.routes.login) { + if (isNonNavigationalRequest(req)) { + // 204 No Content signals "intentionally did nothing" for prefetch/ + // non-navigational requests, avoiding polluting auth-failure telemetry + // and access logs. Next.js discards prefetch responses regardless, so + // behavior is unaffected. + // Cache-Control: no-store prevents CDNs and reverse proxies from caching + // this 204 — RFC 9111 makes 204 heuristically cacheable without an explicit + // directive, so a cached 204 would silently break real login navigations. + return new NextResponse(null, { + status: 204, + headers: { "Cache-Control": "no-store" } + }); + } return this.handleLogin(req); } else if (method === "GET" && sanitizedPathname === this.routes.logout) { return this.handleLogout(req); @@ -802,7 +821,8 @@ export class AuthClient { async startInteractiveLogin( options: StartInteractiveLoginOptions = {}, - req?: NextRequest + req?: NextRequest, + reqCookies?: RequestCookies | ReadonlyRequestCookies ): Promise { await this.ensureDpopValidated(); const appBaseUrl = resolveAppBaseUrl(this.appBaseUrl, req); @@ -821,10 +841,12 @@ export class AuthClient { const sanitizedReturnTo = toSafeRedirect(options.returnTo, safeBaseUrl); if (sanitizedReturnTo) { - returnTo = + returnTo = clampReturnTo( sanitizedReturnTo.pathname + - sanitizedReturnTo.search + - sanitizedReturnTo.hash; + sanitizedReturnTo.search + + sanitizedReturnTo.hash, + this.signInReturnToPath + ); } } @@ -920,6 +942,11 @@ export class AuthClient { } resolvedMaxAge = parsed; } + // scope and audience come from user-controllable query params on + // /auth/login, so clamp each independently to keep the resulting cookie + // under the byte cap. A ridiculous value gets replaced with `undefined` + // (equivalent to not passing the field), and the authorization server + // will reject any invalid value at /authorize. const transactionState: TransactionState = { nonce, maxAge: resolvedMaxAge, @@ -927,8 +954,16 @@ export class AuthClient { responseType: RESPONSE_TYPES.CODE, state, returnTo, - scope: authorizationParams.get("scope") || undefined, - audience: authorizationParams.get("audience") || undefined, + scope: clampTransactionField( + "scope", + authorizationParams.get("scope") || undefined, + undefined + ), + audience: clampTransactionField( + "audience", + authorizationParams.get("audience") || undefined, + undefined + ), challengeMode: challengeMode !== "redirect" ? challengeMode : undefined, // Store origin domain and issuer for callback delegation in resolver mode originDomain: this.provider?.isResolverMode ? this.domain : undefined, @@ -950,8 +985,11 @@ export class AuthClient { // Set response and save transaction const res = NextResponse.redirect(authorizationUrl.toString()); - // Save transaction state - await this.transactionStore.save(res.cookies, transactionState); + await this.transactionStore.save( + res.cookies, + transactionState, + req?.cookies ?? reqCookies + ); return res; } @@ -1601,7 +1639,6 @@ export class AuthClient { await this.sessionStore.set(req.cookies, res.cookies, session, true); addCacheControlHeadersForSession(res); - // Clean up the current transaction cookie after successful authentication await this.transactionStore.delete(res.cookies, state); return res; @@ -4140,10 +4177,12 @@ export class AuthClient { const sanitizedReturnTo = toSafeRedirect(options.returnTo, safeBaseUrl); if (sanitizedReturnTo) { - returnTo = + returnTo = clampReturnTo( sanitizedReturnTo.pathname + - sanitizedReturnTo.search + - sanitizedReturnTo.hash; + sanitizedReturnTo.search + + sanitizedReturnTo.hash, + this.signInReturnToPath + ); } } @@ -4181,7 +4220,11 @@ export class AuthClient { `${connectAccountResponse.connectUri}?ticket=${encodeURIComponent(connectAccountResponse.connectParams.ticket)}` ); - await this.transactionStore.save(res.cookies, transactionState); + await this.transactionStore.save( + res.cookies, + transactionState, + req?.cookies + ); return [null, res]; } @@ -5098,22 +5141,68 @@ export class AuthClient { ); } - // Decrypt token to extract audience - const { audience } = await decryptMfaToken( + // Decrypt token to extract audience and the requested scope + const { audience, scope: requestedScope } = await decryptMfaToken( encryptedMfaToken, this.sessionStore.secret ); session.accessTokens = session.accessTokens || []; - session.accessTokens.push({ + + const newAccessTokenSet = { accessToken: tokenResponse.access_token, scope: tokenResponse.scope, + requestedScope, // oauth4webapi TokenEndpointResponse does NOT include audience field audience: audience || "", expiresAt: Math.floor(Date.now() / 1000) + Number(tokenResponse.expires_in), token_type: tokenResponse.token_type - }); + }; + + // Replace an existing token for the same audience AND scope, or append a + // new one. Without this, each MFA step-up appends another full token set — + // growing the session cookie unbounded (and eventually a 431). The key is + // audience + normalized scope (not audience alone), so a differently-scoped + // step-up for the same audience does not evict a still-useful entry. + // + // The match is exact normalized-set equality on requestedScope (with a + // fallback to granted `scope` for legacy entries). `findAccessTokenSet` + // does a looser superset match via `compareScopes`; the two rules do NOT + // agree. Consequences of the divergence: + // - `findAccessTokenSet` may return a wider entry to satisfy a narrower + // request (superset match on read), while this dedup keeps them as + // separate entries (exact match on write). + // - Per audience, the session can accumulate up to one entry per distinct + // normalized-scope set. Bounded per session, but looser than + // `findAccessTokenSet` would suggest. + // Aligning to `compareScopes` here would be a behaviour change (wide + // entries would evict narrower ones), which is out of scope for this fix. + // See also `mergePopupTokenIntoSession` (session-helpers.ts) which uses a + // third rule — keyed on audience alone — pre-existing on main. + // + // Key on the requested scope (always present, with a fallback to the + // granted scope for legacy entries): the granted `scope` may be reduced or + // omitted by the server, which would otherwise collide distinct requests. + const normalizeScope = (scope?: string) => + (scope ?? "").trim().split(/\s+/).filter(Boolean).sort().join(" "); + const newScope = normalizeScope( + newAccessTokenSet.requestedScope ?? newAccessTokenSet.scope + ); + // Remove ALL existing entries for this audience + scope (not just the first) + // so sessions that accumulated duplicates before this fix deployed are + // compacted on the next step-up. Best-effort for legacy entries: those have + // no `requestedScope` and fall back to the granted `scope`, so if the server + // reduced scope, a legacy entry's fallback key may not match a new request's + // requestedScope key and it will be retained alongside the new entry. + session.accessTokens = session.accessTokens.filter( + (t) => + !( + t.audience === newAccessTokenSet.audience && + normalizeScope(t.requestedScope ?? t.scope) === newScope + ) + ); + session.accessTokens.push(newAccessTokenSet); // Persist updated session await this.sessionStore.set(reqCookies, resCookies, session); @@ -6032,7 +6121,11 @@ export class AuthClient { "Pass the NextResponse cookies (App Router: next/headers cookies; Pages Router: res.cookies)." ); } - await this.transactionStore.save(resCookies, magicLinkTransactionState); + await this.transactionStore.save( + resCookies, + magicLinkTransactionState, + req?.cookies + ); } } diff --git a/src/server/chunked-cookies.test.ts b/src/server/chunked-cookies.test.ts index b64e90c0c..74acc757c 100644 --- a/src/server/chunked-cookies.test.ts +++ b/src/server/chunked-cookies.test.ts @@ -130,10 +130,33 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, value, options, reqCookies, resCookies); - expect(resCookies.set).toHaveBeenCalledTimes(1); + // resCookies.set called 6 times: 1 set + 5 deletes for indices 0-4 + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(resCookies.set).toHaveBeenCalledWith(name, value, options); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__0`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__1`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__2`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { + maxAge: 0, + path: "/" + }); expect(reqCookies.set).toHaveBeenCalledTimes(1); expect(reqCookies.set).toHaveBeenCalledWith(name, value); + // reqCookies.delete called 5 times for indices 0-4 + expect(reqCookies.delete).toHaveBeenCalledTimes(5); }); it("should split cookie into chunks when value exceeds max size", () => { @@ -145,11 +168,11 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, largeValue, options, reqCookies, resCookies); - // Should create 3 chunks (8000 / 3500 ≈ 2.3, rounded up to 3) - // called 4 times: - // 3 calls to set the chunks - // 1 call to remove the non-chunked cookie - expect(resCookies.set).toHaveBeenCalledTimes(4); + // resCookies.set called 6 times: + // 3 calls to set the chunks (__0, __1, __2) + // 2 calls to delete higher indices (__3, __4) + // 1 call to delete the base cookie + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(reqCookies.set).toHaveBeenCalledTimes(3); // Check first chunk @@ -173,11 +196,22 @@ describe("Chunked Cookie Utils", () => { options ); - // Check removal of non-chunked cookie + // Check deletion of unused chunk indices and base cookie + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { + maxAge: 0, + path: "/" + }); expect(resCookies.set).toHaveBeenCalledWith(name, "", { maxAge: 0, path: "/" }); + + // reqCookies.delete called 3 times: __3, __4, and base name + expect(reqCookies.delete).toHaveBeenCalledTimes(3); }); it("should clear existing chunked cookies when setting a single cookie", () => { @@ -195,27 +229,39 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, value, options, reqCookies, resCookies); - // delete the 3 chunked cookies set above and then set the new cookie - expect(resCookies.set).toHaveBeenCalledTimes(4); - expect(resCookies.set).toHaveBeenNthCalledWith(1, name, value, options); - expect(resCookies.set).toHaveBeenNthCalledWith(2, `${name}__1`, "", { + // resCookies.set called 6 times: + // 1 set of main cookie, then 5 deletes of deterministic indices 0-4 + expect(resCookies.set).toHaveBeenCalledTimes(6); + expect(resCookies.set).toHaveBeenCalledWith(name, value, options); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__0`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__1`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__2`, "", { maxAge: 0, path: "/" }); - expect(resCookies.set).toHaveBeenNthCalledWith(3, `${name}__0`, "", { + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { maxAge: 0, path: "/" }); - expect(resCookies.set).toHaveBeenNthCalledWith(4, `${name}__2`, "", { + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { maxAge: 0, path: "/" }); expect(reqCookies.set).toHaveBeenCalledTimes(1); expect(reqCookies.set).toHaveBeenCalledWith(name, value); - expect(reqCookies.delete).toHaveBeenCalledTimes(3); + // reqCookies.delete called 5 times for deterministic indices 0-4 + expect(reqCookies.delete).toHaveBeenCalledTimes(5); expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__0`); expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__1`); expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__2`); + expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__3`); + expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__4`); }); it("should clear existing single cookies when setting a chunked cookie", () => { @@ -230,35 +276,71 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, largeValue, options, reqCookies, resCookies); - expect(reqCookies.delete).toHaveBeenCalledTimes(1); + // reqCookies.delete called 3 times: once for base name, then __3, __4 + expect(reqCookies.delete).toHaveBeenCalledTimes(3); expect(reqCookies.delete).toHaveBeenCalledWith(`${name}`); - // set a chunked cookie with 3 chunks and delete the existing single cookie - expect(resCookies.set).toHaveBeenCalledTimes(4); - expect(resCookies.set).toHaveBeenNthCalledWith( - 1, + expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__3`); + expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__4`); + + // resCookies.set called 6 times: + // 3 calls to set the chunks (__0, __1, __2) + // 2 calls to delete higher indices (__3, __4) + // 1 call to delete the base cookie + expect(resCookies.set).toHaveBeenCalledTimes(6); + expect(resCookies.set).toHaveBeenCalledWith( `${name}__0`, largeValue.slice(0, 3500), options ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 2, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__1`, largeValue.slice(3500, 7000), options ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 3, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__2`, largeValue.slice(7000), options ); - expect(resCookies.set).toHaveBeenNthCalledWith(4, name, "", { + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { + maxAge: 0, + path: "/" + }); + expect(resCookies.set).toHaveBeenCalledWith(name, "", { maxAge: 0, path: "/" }); expect(reqCookies.set).toHaveBeenCalledTimes(3); }); + it("deletes higher-index chunks even when absent from the request snapshot (cross-tab orphan)", () => { + // A concurrent tab wrote __session__2, but this request's reqCookies only + // shows __0/__1. The deterministic sweep must still issue a deletion for + // __2 (and the rest of the range) so no chunk is left orphaned. + const name = "__session"; + const options = { path: "/" } as CookieOptions; + + // reqCookies snapshot is missing the concurrently-written __2 chunk. + cookieStore.set(`${name}__0`, "old0"); + cookieStore.set(`${name}__1`, "old1"); + + // New value is small → single-cookie path, which sweeps chunk indices 0..4. + setChunkedCookie(name, "small", options, reqCookies, resCookies); + + // A deletion is issued for every index in the deterministic range, + // including __2 which was never in reqCookies. + for (let i = 0; i < 5; i++) { + expect(resCookies.set).toHaveBeenCalledWith(`${name}__${i}`, "", { + maxAge: 0, + path: "/" + }); + } + }); + it("should clean up unused chunks when cookie shrinks", () => { const name = "testCookie"; const options = { path: "/" } as CookieOptions; @@ -287,6 +369,33 @@ describe("Chunked Cookie Utils", () => { expect(reqCookies.delete).toHaveBeenCalledWith(name); }); + it("clears high-index chunks (>= MAX_CHUNKS) when a session that grew past MAX_CHUNKS shrinks", () => { + // Regression: a session that once wrote __session__5 (or higher) and + // then shrinks would otherwise leave the high-index chunk orphaned, + // because the deterministic sweep only covers __0..__4. The snapshot + // must extend the sweep to cover indices seen in the request. + const name = "__session"; + const options = { path: "/" } as CookieOptions; + + // Seed a 6-chunk state (as if a prior write produced them). + for (let i = 0; i < 6; i++) { + cookieStore.set(`${name}__${i}`, `chunk${i}`); + } + + // Shrink: write a value that fits in ~2 chunks. + const shrunkValue = "a".repeat(7000); + setChunkedCookie(name, shrunkValue, options, reqCookies, resCookies); + + // The high-index chunk written before the shrink must be deleted, + // otherwise it stays in the browser and poisons subsequent reads. + expect(resCookies.set).toHaveBeenCalledWith( + `${name}__5`, + "", + expect.objectContaining({ maxAge: 0 }) + ); + expect(reqCookies.delete).toHaveBeenCalledWith(`${name}__5`); + }); + // New tests for domain and transient options it("should set the domain property for a single cookie", () => { const name = "domainCookie"; @@ -301,7 +410,8 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, value, options, reqCookies, resCookies); - expect(resCookies.set).toHaveBeenCalledTimes(1); + // resCookies.set called 6 times: 1 set + 5 deletes for indices 0-4 + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(resCookies.set).toHaveBeenCalledWith( name, value, @@ -322,29 +432,43 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, largeValue, options, reqCookies, resCookies); - // called 4 times: - // 3 calls to set the chunks - // 1 call to remove the non-chunked cookie - expect(resCookies.set).toHaveBeenCalledTimes(4); - expect(resCookies.set).toHaveBeenNthCalledWith( - 1, + // resCookies.set called 6 times: + // 3 calls to set the chunks (__0, __1, __2) + // 2 calls to delete higher indices (__3, __4) + // 1 call to delete the base cookie + expect(resCookies.set).toHaveBeenCalledTimes(6); + expect(resCookies.set).toHaveBeenCalledWith( `${name}__0`, expect.any(String), expect.objectContaining({ domain: "example.com" }) ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 2, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__1`, expect.any(String), expect.objectContaining({ domain: "example.com" }) ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 3, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__2`, expect.any(String), expect.objectContaining({ domain: "example.com" }) ); - expect(resCookies.set).toHaveBeenNthCalledWith(4, name, "", { + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { + domain: "example.com", + httpOnly: true, + maxAge: 0, + path: "/", + sameSite: "lax", + secure: true + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { + domain: "example.com", + httpOnly: true, + maxAge: 0, + path: "/", + sameSite: "lax", + secure: true + }); + expect(resCookies.set).toHaveBeenCalledWith(name, "", { domain: "example.com", httpOnly: true, maxAge: 0, @@ -371,7 +495,8 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, value, options, reqCookies, resCookies); - expect(resCookies.set).toHaveBeenCalledTimes(1); + // resCookies.set called 6 times: 1 set + 5 deletes for indices 0-4 + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(resCookies.set).toHaveBeenCalledWith(name, value, expectedOptions); expect(resCookies.set).not.toHaveBeenCalledWith( name, @@ -397,29 +522,41 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, largeValue, options, reqCookies, resCookies); - // called 4 times: - // 3 calls to set the chunks - // 1 call to remove the non-chunked cookie - expect(resCookies.set).toHaveBeenCalledTimes(4); - expect(resCookies.set).toHaveBeenNthCalledWith( - 1, + // resCookies.set called 6 times: + // 3 calls to set the chunks (__0, __1, __2) + // 2 calls to delete higher indices (__3, __4) + // 1 call to delete the base cookie + expect(resCookies.set).toHaveBeenCalledTimes(6); + expect(resCookies.set).toHaveBeenCalledWith( `${name}__0`, expect.any(String), expectedOptions ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 2, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__1`, expect.any(String), expectedOptions ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 3, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__2`, expect.any(String), expectedOptions ); - expect(resCookies.set).toHaveBeenNthCalledWith(4, name, "", { + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { + httpOnly: true, + maxAge: 0, + path: "/", + sameSite: "lax", + secure: true + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { + httpOnly: true, + maxAge: 0, + path: "/", + sameSite: "lax", + secure: true + }); + expect(resCookies.set).toHaveBeenCalledWith(name, "", { httpOnly: true, maxAge: 0, path: "/", @@ -449,7 +586,8 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, value, options, reqCookies, resCookies); - expect(resCookies.set).toHaveBeenCalledTimes(1); + // resCookies.set called 6 times: 1 set + 5 deletes for indices 0-4 + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(resCookies.set).toHaveBeenCalledWith(name, value, expectedOptions); expect(resCookies.set).toHaveBeenCalledWith( name, @@ -474,29 +612,41 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, largeValue, options, reqCookies, resCookies); - // called 4 times: - // 3 calls to set the chunks - // 1 call to remove the non-chunked cookie - expect(resCookies.set).toHaveBeenCalledTimes(4); - expect(resCookies.set).toHaveBeenNthCalledWith( - 1, + // resCookies.set called 6 times: + // 3 calls to set the chunks (__0, __1, __2) + // 2 calls to delete higher indices (__3, __4) + // 1 call to delete the base cookie + expect(resCookies.set).toHaveBeenCalledTimes(6); + expect(resCookies.set).toHaveBeenCalledWith( `${name}__0`, expect.any(String), expectedOptions ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 2, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__1`, expect.any(String), expectedOptions ); - expect(resCookies.set).toHaveBeenNthCalledWith( - 3, + expect(resCookies.set).toHaveBeenCalledWith( `${name}__2`, expect.any(String), expectedOptions ); - expect(resCookies.set).toHaveBeenNthCalledWith(4, name, "", { + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { + httpOnly: true, + maxAge: 0, + path: "/", + sameSite: "lax", + secure: true + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { + httpOnly: true, + maxAge: 0, + path: "/", + sameSite: "lax", + secure: true + }); + expect(resCookies.set).toHaveBeenCalledWith(name, "", { httpOnly: true, maxAge: 0, path: "/", @@ -592,8 +742,8 @@ describe("Chunked Cookie Utils", () => { deleteChunkedCookie(name, reqCookies, resCookies); - // Should delete main cookie and 3 chunks - expect(resCookies.set).toHaveBeenCalledTimes(4); + // Should delete main cookie and deterministic range of chunks (0-4) + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(resCookies.set).toHaveBeenCalledWith(name, "", { maxAge: 0 }); @@ -606,11 +756,33 @@ describe("Chunked Cookie Utils", () => { expect(resCookies.set).toHaveBeenCalledWith(`${name}__2`, "", { maxAge: 0 }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__3`, "", { + maxAge: 0 + }); + expect(resCookies.set).toHaveBeenCalledWith(`${name}__4`, "", { + maxAge: 0 + }); // Should not delete unrelated cookies expect(resCookies.set).not.toHaveBeenCalledWith("otherCookie", "", { maxAge: 0 }); }); + + it("deletes high-index chunks (>= MAX_CHUNKS) present in the request snapshot", () => { + // Regression: a session that once grew past MAX_CHUNKS would leave + // __session__5+ orphaned on logout, because the deterministic sweep + // only covers __0..__4. The snapshot must extend the sweep. + const name = "__session"; + for (let i = 0; i < 6; i++) { + cookieStore.set(`${name}__${i}`, `chunk${i}`); + } + + deleteChunkedCookie(name, reqCookies, resCookies); + + expect(resCookies.set).toHaveBeenCalledWith(`${name}__5`, "", { + maxAge: 0 + }); + }); }); describe("Edge Cases", () => { @@ -621,7 +793,8 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, value, options, reqCookies, resCookies); - expect(resCookies.set).toHaveBeenCalledTimes(1); + // resCookies.set called 6 times: 1 set + 5 deletes for indices 0-4 + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(resCookies.set).toHaveBeenCalledWith(name, value, options); }); @@ -632,8 +805,9 @@ describe("Chunked Cookie Utils", () => { setChunkedCookie(name, value, options, reqCookies, resCookies); - // Should still fit in one cookie - expect(resCookies.set).toHaveBeenCalledTimes(1); + // Should still fit in one cookie, but deterministic deletes still happen + // resCookies.set called 6 times: 1 set + 5 deletes for indices 0-4 + expect(resCookies.set).toHaveBeenCalledTimes(6); expect(resCookies.set).toHaveBeenCalledWith(name, value, options); }); @@ -688,10 +862,11 @@ describe("Chunked Cookie Utils", () => { // Get chunks count (10000 / 3500 ≈ 2.86, so we need 3 chunks) const expectedChunks = Math.ceil(10000 / 3500); - // called 4 times: + // resCookies.set called 6 times: // 3 calls to set the chunks - // 1 call to remove the non-chunked cookie - expect(resCookies.set).toHaveBeenCalledTimes(expectedChunks + 1); + // 2 calls to delete higher indices (__3, __4) + // 1 call to delete the base cookie + expect(resCookies.set).toHaveBeenCalledTimes(6); // Clear and set up cookies for retrieval test cookieStore.clear(); @@ -1014,3 +1189,148 @@ describe("Regression #2595: MCD backfill session chunking at boundary", () => { expect(cookieStore.has("__session__1")).toBe(false); }); }); + +// --------------------------------------------------------------------------- +// End-to-end jar tests. These use the real RequestCookies / ResponseCookies +// implementations and apply Set-Cookie headers to a browser-like jar between +// writes, then assert on read via getChunkedCookie. This exercises the whole +// write → apply → read cycle, which is what a real user hits — as opposed to +// the mock-based tests above which only assert on `resCookies.set` spy calls. +// --------------------------------------------------------------------------- + +/** + * Minimal browser cookie jar: applies Set-Cookie writes from a `ResponseCookies` + * to an in-memory store (honouring `maxAge: 0` as deletion), and produces a + * fresh `RequestCookies` reflecting the current jar state for the next request. + */ +class BrowserJar { + private store = new Map(); + + apply(res: ResponseCookies) { + for (const c of res.getAll()) { + if (c.maxAge === 0 || c.value === "") { + this.store.delete(c.name); + } else { + this.store.set(c.name, c.value); + } + } + } + + req(): RequestCookies { + const headers = new Headers(); + const cookieHeader = Array.from(this.store) + .map(([k, v]) => `${k}=${v}`) + .join("; "); + if (cookieHeader) headers.append("cookie", cookieHeader); + return new RequestCookies(headers); + } + + names(): string[] { + return Array.from(this.store.keys()).sort(); + } +} + +describe("Chunked Cookie — end-to-end jar", () => { + const OPTS: CookieOptions = { + path: "/", + httpOnly: true, + secure: true, + sameSite: "lax" as const, + maxAge: 3600 + }; + + const write = (jar: BrowserJar, name: string, value: string) => { + const res = new ResponseCookies(new Headers()); + setChunkedCookie(name, value, OPTS, jar.req(), res); + jar.apply(res); + return res; + }; + + it("shrinking a 6-chunk session leaves no high-index orphan and the value reads back", () => { + // Regression: before the getClearUpTo fix, a session that grew past + // MAX_CHUNKS (5) then shrank would leave __session__5 orphaned in the + // browser and getChunkedCookie would return undefined forever. + const jar = new BrowserJar(); + + // Write a value that requires 6 chunks (6 × 3500 = 21000 bytes). + const big = "a".repeat(21000); + write(jar, "__session", big); + expect(jar.names()).toEqual([ + "__session__0", + "__session__1", + "__session__2", + "__session__3", + "__session__4", + "__session__5" + ]); + + // Shrink to a value that fits in 2 chunks (~7000 bytes). + const small = "b".repeat(7000); + write(jar, "__session", small); + + // No high-index orphan. + expect(jar.names()).toEqual(["__session__0", "__session__1"]); + + // Read returns the full new value. + const read = getChunkedCookie("__session", jar.req()); + expect(read).toBe(small); + }); + + it("logout of a 6-chunk session clears every chunk", () => { + const jar = new BrowserJar(); + write(jar, "__session", "a".repeat(21000)); + expect(jar.names().length).toBe(6); + + const res = new ResponseCookies(new Headers()); + deleteChunkedCookie("__session", jar.req(), res, false, { + path: "/", + domain: undefined + }); + jar.apply(res); + + expect(jar.names()).toEqual([]); + expect(getChunkedCookie("__session", jar.req())).toBeUndefined(); + }); + + it("residual case: concurrent-tab write above MAX_CHUNKS not in snapshot self-heals on next write", () => { + // Tab A writes 6 chunks. Tab B starts from an older browser state showing + // only __0 and __1 (Tab A's __2..__5 haven't reached Tab B's request yet). + // Tab B writes a 2-chunk value. Because Tab B's snapshot doesn't reveal + // __session__5, the sweep stops at MAX_CHUNKS (5), and __5 survives. + // Read on THAT response is broken (getChunkedCookie sees indices [0,1,5], + // highestIndex=5, count mismatch → undefined). The next write on this + // connection sees __5 in its snapshot and cleans up. One bad response, + // self-healing — not the permanent login loop from before. + + const browserJar = new BrowserJar(); + // Simulate the eventual browser state after Tab A finished. + write(browserJar, "__session", "a".repeat(21000)); + + // Tab B's stale snapshot: only __0/__1 as seen when Tab B's request started. + const staleReqHeaders = new Headers(); + staleReqHeaders.append( + "cookie", + `__session__0=${browserJar.req().get("__session__0")!.value};` + + `__session__1=${browserJar.req().get("__session__1")!.value}` + ); + const staleReq = new RequestCookies(staleReqHeaders); + + // Tab B writes 2 chunks using its stale snapshot. + const tabBRes = new ResponseCookies(new Headers()); + setChunkedCookie("__session", "b".repeat(7000), OPTS, staleReq, tabBRes); + browserJar.apply(tabBRes); + + // __session__5 survives — Tab B never saw it. Confirmed orphan. + expect(browserJar.names()).toContain("__session__5"); + // The immediate read is broken. + expect(getChunkedCookie("__session", browserJar.req())).toBeUndefined(); + + // Next write on this connection SEES __5 in the snapshot and cleans up. + write(browserJar, "__session", "c".repeat(7000)); + + expect(browserJar.names()).toEqual(["__session__0", "__session__1"]); + expect(getChunkedCookie("__session", browserJar.req())).toBe( + "c".repeat(7000) + ); + }); +}); diff --git a/src/server/client.test.ts b/src/server/client.test.ts index c90d96542..c57bb35be 100644 --- a/src/server/client.test.ts +++ b/src/server/client.test.ts @@ -3412,6 +3412,43 @@ describe("Auth0Client", () => { }); }); }); + + describe("startInteractiveLogin", () => { + it("forwards request cookies to AuthClient.startInteractiveLogin so transaction-cookie eviction can run", async () => { + process.env[ENV_VARS.DOMAIN] = "env.auth0.com"; + process.env[ENV_VARS.CLIENT_ID] = "env_client_id"; + process.env[ENV_VARS.CLIENT_SECRET] = "env_client_secret"; + process.env[ENV_VARS.APP_BASE_URL] = "https://myapp.com"; + process.env[ENV_VARS.SECRET] = "env_secret"; + + const client = new Auth0Client(); + + const mockCookieJar = { getAll: () => [] }; + const nextHeaders = await import("next/headers.js"); + vi.mocked(nextHeaders.cookies).mockResolvedValue(mockCookieJar as any); + + const mockAuthClient = { + startInteractiveLogin: vi + .fn() + .mockResolvedValue(NextResponse.redirect("https://example.com")) + }; + vi.spyOn(client["provider"] as any, "forRequest").mockResolvedValue( + mockAuthClient + ); + + await client.startInteractiveLogin(); + + expect(mockAuthClient.startInteractiveLogin).toHaveBeenCalledTimes(1); + const [, req, reqCookies] = + mockAuthClient.startInteractiveLogin.mock.calls[0]; + // No NextRequest is available from Server Components/Actions. + expect(req).toBeUndefined(); + // Cookies must be forwarded — otherwise TransactionStore.save() never + // runs eviction, and __txn_* cookies accumulate unbounded for logins + // started this way (e.g. from a Server Action). + expect(reqCookies).toBe(mockCookieJar); + }); + }); }); export type GetAccessTokenOptions = { diff --git a/src/server/client.ts b/src/server/client.ts index 86fbbb391..cb83791f9 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -802,7 +802,6 @@ export class Auth0Client { fetch: options.customFetch, mfaTokenTtl, cspNonce: options.cspNonce, - discoveryCache, provider: this.provider }); @@ -1835,7 +1834,14 @@ export class Auth0Client { ): Promise { const reqHeaders = await getHeaders(); const authClient = await this.provider.forRequest(reqHeaders, undefined); - return authClient.startInteractiveLogin(options); + // Pass request cookies so the transaction store can evict accumulated + // `__txn_*` cookies before writing the new one — otherwise logins started + // from Server Components/Actions never trigger eviction. + return authClient.startInteractiveLogin( + options, + undefined, + await cookies() + ); } /** diff --git a/src/server/cookies.ts b/src/server/cookies.ts index 961f30b70..b5f0c435c 100644 --- a/src/server/cookies.ts +++ b/src/server/cookies.ts @@ -157,6 +157,19 @@ const MAX_CHUNK_SIZE = 3500; // Slightly under 4KB const CHUNK_PREFIX = "__"; const CHUNK_INDEX_REGEX = new RegExp(`${CHUNK_PREFIX}(\\d+)$`); const LEGACY_CHUNK_INDEX_REGEX = /\.(\d+)$/; +// Minimum deterministic sweep range for chunk cleanup. Cleanup uses +// `getClearUpTo`, which sweeps at least `__0..MAX_CHUNKS-1` (this constant) and +// also anything higher the current request's snapshot reveals. The deterministic +// minimum covers the concurrent-write case within this range: a tab that wrote a +// chunk absent from this request's stale snapshot is still cleaned. The snapshot +// extension covers the shrink case: a session that once grew past this range +// and later shrinks has its high-index chunks removed instead of orphaned. +// Values above `MAX_CHUNKS` written by a concurrent tab and absent from this +// snapshot are the residual case — the next write on this connection sees them +// and cleans up (self-healing after one bad response). 5 × 3500 = 17,500 bytes +// covers the typical session envelope; sessions larger than that surface the +// oversized-session warning in `stateless-session-store.ts`. +const MAX_CHUNKS = 5; /** * Retrieves the index of a cookie based on its name. @@ -179,25 +192,70 @@ const getChunkedCookieIndex = ( }; /** - * Retrieves all cookies from the request that have names starting with a specific prefix. + * Retrieves all cookies from the request that have names matching the exact + * `{name}{CHUNK_PREFIX}{digits}` shape (or `{name}.{digits}` for the legacy + * format). The regex is built once per shape at module load rather than per + * call — `setChunkedCookie` runs on every authenticated request under rolling + * sessions, so avoiding a fresh RegExp per call matters. * * @param reqCookies - The cookies from the request. * @param name - The base name of the cookies to retrieve. - * @returns An array of cookies that have names starting with the specified prefix. + * @returns An array of cookies matching the chunked-cookie shape. */ const getAllChunkedCookies = ( reqCookies: RequestCookies, name: string, isLegacyCookie?: boolean ): RequestCookie[] => { - const chunkedCookieRegex = new RegExp( - isLegacyCookie - ? `^${name}${LEGACY_CHUNK_INDEX_REGEX.source}$` - : `^${name}${CHUNK_PREFIX}\\d+$` - ); - return reqCookies - .getAll() - .filter((cookie) => chunkedCookieRegex.test(cookie.name)); + const regex = isLegacyCookie + ? getLegacyChunkedCookieRegex(name) + : getChunkedCookieRegex(name); + return reqCookies.getAll().filter((cookie) => regex.test(cookie.name)); +}; + +// Per-name regex caches, populated on first use. Prevents rebuilding the same +// RegExp on every hot-path call. Names are bounded (session, legacy session, +// __FC cookies), so unbounded growth is not a concern. +const chunkedCookieRegexCache = new Map(); +const legacyChunkedCookieRegexCache = new Map(); + +const getChunkedCookieRegex = (name: string): RegExp => { + const cached = chunkedCookieRegexCache.get(name); + if (cached) return cached; + const regex = new RegExp(`^${name}${CHUNK_PREFIX}\\d+$`); + chunkedCookieRegexCache.set(name, regex); + return regex; +}; + +const getLegacyChunkedCookieRegex = (name: string): RegExp => { + const cached = legacyChunkedCookieRegexCache.get(name); + if (cached) return cached; + const regex = new RegExp(`^${name}${LEGACY_CHUNK_INDEX_REGEX.source}$`); + legacyChunkedCookieRegexCache.set(name, regex); + return regex; +}; + +/** + * Returns the exclusive upper bound for chunk-cleanup loops: `max(MAX_CHUNKS, + * highestSeenIndex + 1)`. Guarantees the deterministic `__0..MAX_CHUNKS-1` + * sweep (which covers concurrent-tab writes below `MAX_CHUNKS` that this + * request's snapshot missed), while also clearing anything the current + * snapshot reveals above that range — so a session that once grew past + * `MAX_CHUNKS` and later shrinks does not leave orphaned high-index chunks. + * + * Residual case: a concurrent tab that wrote `__session__N` with `N >= + * MAX_CHUNKS`, absent from THIS request's snapshot, is not cleared here. + * That single request's read returns `undefined` for the session. The next + * write on this connection sees `__N` in its snapshot and cleans it up + * (self-healing). Bounded to at most one bad response per orphan. + */ +const getClearUpTo = (reqCookies: RequestCookies, name: string): number => { + let highestSeen = -1; + for (const cookie of getAllChunkedCookies(reqCookies, name)) { + const idx = getChunkedCookieIndex(cookie.name); + if (idx !== undefined && idx > highestSeen) highestSeen = idx; + } + return Math.max(MAX_CHUNKS, highestSeen + 1); }; /** @@ -211,6 +269,9 @@ const getAllChunkedCookies = ( * @param options - Options for setting the cookie. * @param reqCookies - The request cookies object, used to enable read-after-write in the same request for middleware. * @param resCookies - The response cookies object, used to set the cookies in the response. + * @returns The total encoded `name=value` byte size of the cookie(s) written — + * lets callers check against header-size limits without re-scanning + * `resCookies` afterwards. */ export function setChunkedCookie( name: string, @@ -218,7 +279,7 @@ export function setChunkedCookie( options: CookieOptions, reqCookies: RequestCookies, resCookies: ResponseCookies -): void { +): number { const { transient, ...restOptions } = options; const finalOptions = { ...restOptions }; @@ -226,7 +287,22 @@ export function setChunkedCookie( delete finalOptions.maxAge; } - const valueBytes = new TextEncoder().encode(value).length; + const encoder = new TextEncoder(); + const sizeOf = (cookieName: string, cookieValue: string) => + encoder.encode(`${cookieName}=${cookieValue}`).length; + + const valueBytes = encoder.encode(value).length; + + // Hoist the delete-options object out of the loops. `setChunkedCookie` runs + // on every authenticated request under rolling sessions, so allocating one + // object per iteration adds up. + const deleteOptions = { + path: finalOptions.path, + domain: finalOptions.domain, + secure: finalOptions.secure, + sameSite: finalOptions.sameSite, + httpOnly: finalOptions.httpOnly + }; // If value fits in a single cookie, set it directly if (valueBytes <= MAX_CHUNK_SIZE) { @@ -234,25 +310,25 @@ export function setChunkedCookie( // to enable read-after-write in the same request for middleware reqCookies.set(name, value); - // When we are writing a non-chunked cookie, we should remove the chunked cookies - // Remove any previously stored chunks for this cookie name - getAllChunkedCookies(reqCookies, name).forEach((cookieChunk) => { - deleteCookie(resCookies, cookieChunk.name, { - path: finalOptions.path, - domain: finalOptions.domain, - secure: finalOptions.secure, - sameSite: finalOptions.sameSite, - httpOnly: finalOptions.httpOnly - }); - reqCookies.delete(cookieChunk.name); - }); + // When we are writing a non-chunked cookie, remove any previously stored + // chunks for this cookie name. Sweep at least `__0..MAX_CHUNKS-1` (covers + // concurrent-tab writes below MAX_CHUNKS not in this snapshot) and also + // anything higher the snapshot reveals (covers sessions that once grew past + // MAX_CHUNKS). The browser ignores deletions for cookies that do not exist. + const clearUpTo = getClearUpTo(reqCookies, name); + for (let i = 0; i < clearUpTo; i++) { + const chunkName = `${name}${CHUNK_PREFIX}${i}`; + deleteCookie(resCookies, chunkName, deleteOptions); + reqCookies.delete(chunkName); + } - return; + return sizeOf(name, value); } // Split value into chunks let position = 0; let chunkIndex = 0; + let totalBytes = 0; while (position < value.length) { const chunk = value.slice(position, position + MAX_CHUNK_SIZE); @@ -261,37 +337,28 @@ export function setChunkedCookie( resCookies.set(chunkName, chunk, finalOptions); // to enable read-after-write in the same request for middleware reqCookies.set(chunkName, chunk); + totalBytes += sizeOf(chunkName, chunk); position += MAX_CHUNK_SIZE; chunkIndex++; } - // clear unused chunks - const chunks = getAllChunkedCookies(reqCookies, name); - const chunksToRemove = chunks.length - chunkIndex; - if (chunksToRemove > 0) { - for (let i = 0; i < chunksToRemove; i++) { - const chunkIndexToRemove = chunkIndex + i; - const chunkName = `${name}${CHUNK_PREFIX}${chunkIndexToRemove}`; - deleteCookie(resCookies, chunkName, { - path: finalOptions.path, - domain: finalOptions.domain, - secure: finalOptions.secure, - sameSite: finalOptions.sameSite, - httpOnly: finalOptions.httpOnly - }); - reqCookies.delete(chunkName); - } + // Clear any now-unused higher-index chunks. Sweep at least up to + // `MAX_CHUNKS-1` (covers concurrent-tab writes below MAX_CHUNKS not in this + // snapshot) and also anything higher the snapshot reveals (covers sessions + // that once grew past MAX_CHUNKS). The browser ignores deletions for absent + // cookies. + const clearUpTo = getClearUpTo(reqCookies, name); + for (let i = chunkIndex; i < clearUpTo; i++) { + const chunkName = `${name}${CHUNK_PREFIX}${i}`; + deleteCookie(resCookies, chunkName, deleteOptions); + reqCookies.delete(chunkName); } // When we have written chunked cookies, we should remove the non-chunked cookie - deleteCookie(resCookies, name, { - path: finalOptions.path, - domain: finalOptions.domain, - secure: finalOptions.secure, - sameSite: finalOptions.sameSite, - httpOnly: finalOptions.httpOnly - }); + deleteCookie(resCookies, name, deleteOptions); reqCookies.delete(name); + + return totalBytes; } /** @@ -365,9 +432,23 @@ export function deleteChunkedCookie( // Delete main cookie deleteCookie(resCookies, name, options); - getAllChunkedCookies(reqCookies, name, isLegacyCookie).forEach((cookie) => { - deleteCookie(resCookies, cookie.name, options); // Delete each filtered cookie - }); + if (isLegacyCookie) { + // Legacy `{name}.{index}` chunks are no longer written, so their count is + // whatever a prior SDK version left behind — scan the request to find them. + getAllChunkedCookies(reqCookies, name, isLegacyCookie).forEach((cookie) => { + deleteCookie(resCookies, cookie.name, options); + }); + return; + } + + // Sweep at least `__0..MAX_CHUNKS-1` (covers concurrent-tab writes not in + // this snapshot) and also anything higher the snapshot reveals (covers + // sessions that once grew past MAX_CHUNKS). The browser ignores deletions + // for absent cookies. + const clearUpTo = getClearUpTo(reqCookies, name); + for (let i = 0; i < clearUpTo; i++) { + deleteCookie(resCookies, `${name}${CHUNK_PREFIX}${i}`, options); + } } /** diff --git a/src/server/mfa-popup.test.ts b/src/server/mfa-popup.test.ts index 773772ddd..3782bb63e 100644 --- a/src/server/mfa-popup.test.ts +++ b/src/server/mfa-popup.test.ts @@ -4,7 +4,7 @@ import * as oauth from "oauth4webapi"; import { describe, expect, it, vi } from "vitest"; import { getDefaultRoutes } from "../test/defaults.js"; -import { generateSecret } from "../test/utils.js"; +import { generateSecret, stripTransactionValuePrefix } from "../test/utils.js"; import { RESPONSE_TYPES, SessionData } from "../types/index.js"; import { createAuthCompletePostMessageResponse } from "../utils/html-helpers.js"; import { AuthClient } from "./auth-client.js"; @@ -166,7 +166,7 @@ describe("MFA Popup (challengeMode + postMessage)", async () => { const transactionCookie = response.cookies.get(`__txn_${state}`); expect(transactionCookie).toBeDefined(); const { payload: txn } = (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult; expect(txn.challengeMode).toBe("popup"); @@ -200,7 +200,7 @@ describe("MFA Popup (challengeMode + postMessage)", async () => { const state = authUrl.searchParams.get("state")!; const transactionCookie = response.cookies.get(`__txn_${state}`); const { payload: txn } = (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult; // When challengeMode is 'redirect' (default), it's not stored to minimize cookie size @@ -260,7 +260,7 @@ describe("MFA Popup (challengeMode + postMessage)", async () => { const state = authUrl.searchParams.get("state")!; const transactionCookie = response.cookies.get(`__txn_${state}`); const { payload: txn } = (await decrypt( - transactionCookie!.value, + stripTransactionValuePrefix(transactionCookie!.value), secret )) as jose.JWTDecryptResult; diff --git a/src/server/mfa-server.flow.test.ts b/src/server/mfa-server.flow.test.ts index a9e149bea..db6dbf453 100644 --- a/src/server/mfa-server.flow.test.ts +++ b/src/server/mfa-server.flow.test.ts @@ -496,6 +496,354 @@ describe("AuthClient MFA Methods", () => { ); }); + it("should replace (not append) the access token for the same audience on repeated step-up", async () => { + const { RequestCookies, ResponseCookies } = + await import("@edge-runtime/cookies"); + + const session: SessionData = { + user: { sub: DEFAULT.sub }, + tokenSet: { + idToken: "id-token", + accessToken: "old-access-token", + refreshToken: "refresh-token", + expiresAt: 123456 + }, + internal: { + sid: "session-id", + createdAt: Math.floor(Date.now() / 1000) + } + }; + + const sessionCookie = await createSessionCookie(session, secret); + const reqHeaders = new Headers(); + reqHeaders.append("cookie", `__session=${sessionCookie}`); + + const encryptedToken = await encryptMfaToken( + DEFAULT.mfaToken, + "https://api.example.com", + "read:data", + { challenge: [{ type: "otp" }] }, + secret, + 300 + ); + + let issued = 0; + server.use( + http.post(`https://${DEFAULT.domain}/oauth/token`, () => { + issued += 1; + return HttpResponse.json({ + access_token: `mfa-access-token-${issued}`, + token_type: "Bearer", + expires_in: 3600, + scope: "read:data" + }); + }) + ); + + const reqCookies = new RequestCookies(reqHeaders); + const resCookies = new ResponseCookies(new Headers()); + + // First step-up for the audience. + const first = await authClient.mfaVerify({ + mfaToken: encryptedToken, + otp: "123456" + }); + await authClient.cacheTokenFromMfaVerify( + first, + encryptedToken, + reqCookies, + resCookies + ); + + // Second step-up for the SAME audience — must replace, not stack. + const second = await authClient.mfaVerify({ + mfaToken: encryptedToken, + otp: "123456" + }); + await authClient.cacheTokenFromMfaVerify( + second, + encryptedToken, + reqCookies, + resCookies + ); + + const updatedSession = await sessionStore.get(reqCookies); + const forAudience = updatedSession?.accessTokens?.filter( + (t) => t.audience === "https://api.example.com" + ); + // Exactly one entry for the audience (replaced, not appended)... + expect(forAudience?.length).toBe(1); + // ...holding the latest token. + expect(forAudience?.[0].accessToken).toBe("mfa-access-token-2"); + }); + + it("purges all pre-existing duplicates for the same audience+scope on step-up", async () => { + const { RequestCookies, ResponseCookies } = + await import("@edge-runtime/cookies"); + + // Seed the session with two duplicate entries for the same audience+scope + // (the state a session could be in after accumulating them before this fix). + const session: SessionData = { + user: { sub: DEFAULT.sub }, + tokenSet: { + idToken: "id-token", + accessToken: "old-access-token", + refreshToken: "refresh-token", + expiresAt: 123456 + }, + internal: { + sid: "session-id", + createdAt: Math.floor(Date.now() / 1000) + }, + accessTokens: [ + { + accessToken: "stale-token-1", + scope: "read:data", + requestedScope: "read:data", + audience: "https://api.example.com", + expiresAt: Math.floor(Date.now() / 1000) + 3600, + token_type: "Bearer" + }, + { + accessToken: "stale-token-2", + scope: "read:data", + requestedScope: "read:data", + audience: "https://api.example.com", + expiresAt: Math.floor(Date.now() / 1000) + 3600, + token_type: "Bearer" + } + ] + }; + + const sessionCookie = await createSessionCookie(session, secret); + const reqHeaders = new Headers(); + reqHeaders.append("cookie", `__session=${sessionCookie}`); + + const encryptedToken = await encryptMfaToken( + DEFAULT.mfaToken, + "https://api.example.com", + "read:data", + { challenge: [{ type: "otp" }] }, + secret, + 300 + ); + + server.use( + http.post(`https://${DEFAULT.domain}/oauth/token`, () => + HttpResponse.json({ + access_token: "fresh-token", + token_type: "Bearer", + expires_in: 3600, + scope: "read:data" + }) + ) + ); + + const reqCookies = new RequestCookies(reqHeaders); + const resCookies = new ResponseCookies(new Headers()); + + const res = await authClient.mfaVerify({ + mfaToken: encryptedToken, + otp: "123456" + }); + await authClient.cacheTokenFromMfaVerify( + res, + encryptedToken, + reqCookies, + resCookies + ); + + const updatedSession = await sessionStore.get(reqCookies); + const forAudience = updatedSession?.accessTokens?.filter( + (t) => t.audience === "https://api.example.com" + ); + // Both stale duplicates must be gone; exactly one fresh entry remains. + expect(forAudience?.length).toBe(1); + expect(forAudience?.[0].accessToken).toBe("fresh-token"); + }); + + it("should keep separate entries for different audiences", async () => { + const { RequestCookies, ResponseCookies } = + await import("@edge-runtime/cookies"); + + const session: SessionData = { + user: { sub: DEFAULT.sub }, + tokenSet: { + idToken: "id-token", + accessToken: "old-access-token", + refreshToken: "refresh-token", + expiresAt: 123456 + }, + internal: { + sid: "session-id", + createdAt: Math.floor(Date.now() / 1000) + } + }; + + const sessionCookie = await createSessionCookie(session, secret); + const reqHeaders = new Headers(); + reqHeaders.append("cookie", `__session=${sessionCookie}`); + + const tokenA = await encryptMfaToken( + DEFAULT.mfaToken, + "https://api-a.example.com", + "read:data", + { challenge: [{ type: "otp" }] }, + secret, + 300 + ); + const tokenB = await encryptMfaToken( + DEFAULT.mfaToken, + "https://api-b.example.com", + "read:data", + { challenge: [{ type: "otp" }] }, + secret, + 300 + ); + + server.use( + http.post(`https://${DEFAULT.domain}/oauth/token`, () => { + return HttpResponse.json({ + access_token: "at", + token_type: "Bearer", + expires_in: 3600, + scope: "read:data" + }); + }) + ); + + const reqCookies = new RequestCookies(reqHeaders); + const resCookies = new ResponseCookies(new Headers()); + + const resA = await authClient.mfaVerify({ + mfaToken: tokenA, + otp: "123456" + }); + await authClient.cacheTokenFromMfaVerify( + resA, + tokenA, + reqCookies, + resCookies + ); + + const resB = await authClient.mfaVerify({ + mfaToken: tokenB, + otp: "123456" + }); + await authClient.cacheTokenFromMfaVerify( + resB, + tokenB, + reqCookies, + resCookies + ); + + const updatedSession = await sessionStore.get(reqCookies); + const audiences = updatedSession?.accessTokens?.map((t) => t.audience); + expect(audiences).toContain("https://api-a.example.com"); + expect(audiences).toContain("https://api-b.example.com"); + expect(updatedSession?.accessTokens?.length).toBe(2); + }); + + it("should keep separate entries for the same audience at different scopes", async () => { + const { RequestCookies, ResponseCookies } = + await import("@edge-runtime/cookies"); + + const session: SessionData = { + user: { sub: DEFAULT.sub }, + tokenSet: { + idToken: "id-token", + accessToken: "old-access-token", + refreshToken: "refresh-token", + expiresAt: 123456 + }, + internal: { + sid: "session-id", + createdAt: Math.floor(Date.now() / 1000) + } + }; + + const sessionCookie = await createSessionCookie(session, secret); + const reqHeaders = new Headers(); + reqHeaders.append("cookie", `__session=${sessionCookie}`); + + const audience = "https://api.example.com"; + const readToken = await encryptMfaToken( + DEFAULT.mfaToken, + audience, + "read:data", + { challenge: [{ type: "otp" }] }, + secret, + 300 + ); + const writeToken = await encryptMfaToken( + DEFAULT.mfaToken, + audience, + "write:data", + { challenge: [{ type: "otp" }] }, + secret, + 300 + ); + + // Return a scope matching the step-up so the two token sets differ only + // by scope (same audience). + const scopeByToken: Record = { + "read-access-token": "read:data", + "write-access-token": "write:data" + }; + let issued = 0; + server.use( + http.post(`https://${DEFAULT.domain}/oauth/token`, () => { + const accessToken = + issued === 0 ? "read-access-token" : "write-access-token"; + issued += 1; + return HttpResponse.json({ + access_token: accessToken, + token_type: "Bearer", + expires_in: 3600, + scope: scopeByToken[accessToken] + }); + }) + ); + + const reqCookies = new RequestCookies(reqHeaders); + const resCookies = new ResponseCookies(new Headers()); + + // Step up for the audience at scope read:data. + const readRes = await authClient.mfaVerify({ + mfaToken: readToken, + otp: "123456" + }); + await authClient.cacheTokenFromMfaVerify( + readRes, + readToken, + reqCookies, + resCookies + ); + + // Step up for the SAME audience at a different scope write:data — must + // append, not replace, so the read:data token survives. + const writeRes = await authClient.mfaVerify({ + mfaToken: writeToken, + otp: "123456" + }); + await authClient.cacheTokenFromMfaVerify( + writeRes, + writeToken, + reqCookies, + resCookies + ); + + const updatedSession = await sessionStore.get(reqCookies); + const forAudience = updatedSession?.accessTokens?.filter( + (t) => t.audience === audience + ); + // Both scope-distinct tokens are retained for the audience. + expect(forAudience?.length).toBe(2); + const scopes = forAudience?.map((t) => t.scope); + expect(scopes).toContain("read:data"); + expect(scopes).toContain("write:data"); + }); + it("should work without session (stateless operation)", async () => { const encryptedToken = await encryptMfaToken( DEFAULT.mfaToken, diff --git a/src/server/passwordless-server.flow.test.ts b/src/server/passwordless-server.flow.test.ts index ad29d1cff..da7b9b36f 100644 --- a/src/server/passwordless-server.flow.test.ts +++ b/src/server/passwordless-server.flow.test.ts @@ -10,7 +10,7 @@ import { getDefaultRoutes, setupMswLifecycle } from "../test/defaults.js"; -import { generateSecret } from "../test/utils.js"; +import { generateSecret, stripTransactionValuePrefix } from "../test/utils.js"; import type { SessionData } from "../types/index.js"; import { generateDpopKeyPair } from "../utils/dpopRetry.js"; import { AuthClientProvider } from "./auth-client-provider.js"; @@ -268,8 +268,9 @@ describe("AuthClient passwordless methods", () => { const state = authParams.state as string; const txnCookie = resCookies.get(`__txn_${state}`); expect(txnCookie).toBeDefined(); + const jweValue = stripTransactionValuePrefix(txnCookie!.value); const { payload } = (await decrypt( - txnCookie!.value, + jweValue, secret )) as jose.JWTDecryptResult; expect(payload.nonce).toBe(authParams.nonce); diff --git a/src/server/session/stateless-session-store.test.ts b/src/server/session/stateless-session-store.test.ts index 7b536354f..6db3cac32 100644 --- a/src/server/session/stateless-session-store.test.ts +++ b/src/server/session/stateless-session-store.test.ts @@ -615,15 +615,44 @@ describe("Stateless Session Store", async () => { await sessionStore.set(requestCookies, responseCookies, session); - expect(responseCookies.set).toHaveBeenCalledTimes(4); - expect(responseCookies.set).toHaveBeenNthCalledWith( - 1, + // setChunkedCookie for __session now makes 6 calls (1 set + 5 deletes of __session__0..4) + // Then legacy cookie deletion: 1 base + 2 legacy chunks = 3 + // Total: 6 + 1 + 2 = 9 calls + expect(responseCookies.set).toHaveBeenCalledTimes(9); + // Verify main __session cookie is set (without maxAge: 0) + expect(responseCookies.set).toHaveBeenCalledWith( "__session", expect.any(String), - expect.not.objectContaining({ maxAge: 0, path: "/" }) + expect.not.objectContaining({ maxAge: 0 }) + ); + // Verify deterministic deletes of __session__0..4 + expect(responseCookies.set).toHaveBeenCalledWith( + `__session__0`, + "", + expect.objectContaining({ maxAge: 0 }) + ); + expect(responseCookies.set).toHaveBeenCalledWith( + `__session__1`, + "", + expect.objectContaining({ maxAge: 0 }) + ); + expect(responseCookies.set).toHaveBeenCalledWith( + `__session__2`, + "", + expect.objectContaining({ maxAge: 0 }) + ); + expect(responseCookies.set).toHaveBeenCalledWith( + `__session__3`, + "", + expect.objectContaining({ maxAge: 0 }) + ); + expect(responseCookies.set).toHaveBeenCalledWith( + `__session__4`, + "", + expect.objectContaining({ maxAge: 0 }) ); - expect(responseCookies.set).toHaveBeenNthCalledWith( - 2, + // Verify legacy cookie base is deleted + expect(responseCookies.set).toHaveBeenCalledWith( LEGACY_COOKIE_NAME, "", { @@ -634,8 +663,8 @@ describe("Stateless Session Store", async () => { secure: false } ); - expect(responseCookies.set).toHaveBeenNthCalledWith( - 3, + // Verify legacy chunks .0 and .1 are deleted + expect(responseCookies.set).toHaveBeenCalledWith( `${LEGACY_COOKIE_NAME}.0`, "", { @@ -646,8 +675,7 @@ describe("Stateless Session Store", async () => { secure: false } ); - expect(responseCookies.set).toHaveBeenNthCalledWith( - 4, + expect(responseCookies.set).toHaveBeenCalledWith( `${LEGACY_COOKIE_NAME}.1`, "", { @@ -928,26 +956,295 @@ describe("Stateless Session Store", async () => { const decryptedPayload = decryptedNewSession!.payload; expect(decryptedPayload).toEqual(expect.objectContaining(sessionToSet)); - // set should be called once for setting the new session cookie and once for deleting the legacy cookie - expect(setSpy).toHaveBeenCalledTimes(2); - expect(setSpy).toHaveBeenNthCalledWith( - 1, + // setChunkedCookie for __session makes 6 calls (1 set + 5 deletes of __session__0..4) + // legacyCookiesInSetup will have entries from setChunkedCookie on the legacy cookie + // For a chunked legacy cookie, it would be: 3 sets + 2 deletes + 1 base delete = 6 calls in setup + // But those were on tempResCookies. In sessionStore.set, we have: + // - 6 calls for __session from setChunkedCookie + // - 1 call for deleting legacy cookie base + // Total: 7 calls + expect(setSpy).toHaveBeenCalledTimes(7); + + // Verify main __session cookie is set + expect(setSpy).toHaveBeenCalledWith( "__session", expect.any(String), - expect.not.objectContaining({ maxAge: 0, path: "/" }) + expect.not.objectContaining({ maxAge: 0 }) ); - expect(setSpy).toHaveBeenNthCalledWith( - 2, - legacyCookiesInSetup[0].name, - "", - { - httpOnly: true, - maxAge: 0, - path: "/", - sameSite: "lax", - secure: false + + // Verify legacy cookie base is deleted + expect(setSpy).toHaveBeenCalledWith(LEGACY_COOKIE_NAME, "", { + httpOnly: true, + maxAge: 0, + path: "/", + sameSite: "lax", + secure: false + }); + }); + + describe("session cookie size warning", async () => { + const baseSession = (createdAt: number): SessionData => ({ + user: { sub: "user_123" }, + tokenSet: { + accessToken: "at_123", + refreshToken: "rt_123", + expiresAt: 123456 + }, + internal: { sid: "auth0-sid", createdAt } + }); + + it("warns when the session cookie exceeds the size threshold", async () => { + const secret = await generateSecret(32); + const consoleWarnSpy = vi + .spyOn(console, "warn") + .mockImplementation(() => {}); + try { + const session = baseSession(Math.floor(Date.now() / 1000)); + // Large custom claim pushes the encoded session well past 4096 bytes + // (and across multiple __session chunks). + (session.user as Record).bigClaim = "x".repeat(6000); + + const sessionStore = new StatelessSessionStore({ secret }); + await sessionStore.set( + new RequestCookies(new Headers()), + new ResponseCookies(new Headers()), + session + ); + + const warned = consoleWarnSpy.mock.calls.some((c) => + String(c[0]).includes("__session cookie size") + ); + expect(warned).toBe(true); + } finally { + consoleWarnSpy.mockRestore(); } + }); + + it("does not warn for a small session cookie", async () => { + const secret = await generateSecret(32); + const consoleWarnSpy = vi + .spyOn(console, "warn") + .mockImplementation(() => {}); + try { + const sessionStore = new StatelessSessionStore({ secret }); + await sessionStore.set( + new RequestCookies(new Headers()), + new ResponseCookies(new Headers()), + baseSession(Math.floor(Date.now() / 1000)) + ); + + const warned = consoleWarnSpy.mock.calls.some((c) => + String(c[0]).includes("__session cookie size") + ); + expect(warned).toBe(false); + } finally { + consoleWarnSpy.mockRestore(); + } + }); + }); + }); + + describe("set — connection token sets (__FC_N cookies)", async () => { + const baseSession = (createdAt: number): SessionData => ({ + user: { sub: "user_123" }, + tokenSet: { + accessToken: "at_123", + refreshToken: "rt_123", + expiresAt: 9999999999 + }, + internal: { sid: "sid", createdAt } + }); + + it("writes one __FC_N cookie per connection token set", async () => { + const secret = await generateSecret(32); + const session: SessionData = { + ...baseSession(Math.floor(Date.now() / 1000)), + connectionTokenSets: [ + { + connection: "google-oauth2", + accessToken: "fc_g", + expiresAt: 9999999999 + }, + { connection: "github", accessToken: "fc_gh", expiresAt: 9999999999 } + ] + }; + const reqCookies = new RequestCookies(new Headers()); + const resCookies = new ResponseCookies(new Headers()); + const store = new StatelessSessionStore({ secret }); + + await store.set(reqCookies, resCookies, session); + + expect(resCookies.get("__FC_0")?.value).toBeTruthy(); + expect(resCookies.get("__FC_1")?.value).toBeTruthy(); + // Session cookie must not contain the connectionTokenSets payload. + expect(resCookies.get("__FC_2")).toBeUndefined(); + }); + + it("round-trips: get() reads back what set() wrote", async () => { + const secret = await generateSecret(32); + const createdAt = Math.floor(Date.now() / 1000); + const googleTokenSet = { + connection: "google-oauth2", + accessToken: "fc_g", + expiresAt: 9999999999 + }; + const session: SessionData = { + ...baseSession(createdAt), + connectionTokenSets: [googleTokenSet] + }; + const reqCookies = new RequestCookies(new Headers()); + const resCookies = new ResponseCookies(new Headers()); + const store = new StatelessSessionStore({ secret }); + + await store.set(reqCookies, resCookies, session); + + // Promote the response cookies into the next request's cookies. + const nextHeaders = new Headers(); + resCookies + .getAll() + .filter((c) => (c.maxAge ?? 1) > 0) + .forEach((c) => nextHeaders.append("cookie", `${c.name}=${c.value}`)); + const nextReqCookies = new RequestCookies(nextHeaders); + + const result = await store.get(nextReqCookies); + expect(result?.connectionTokenSets).toEqual([ + expect.objectContaining(googleTokenSet) + ]); + }); + + it("does not write __FC_N cookies when connectionTokenSets is absent", async () => { + const secret = await generateSecret(32); + const session: SessionData = baseSession(Math.floor(Date.now() / 1000)); + const reqCookies = new RequestCookies(new Headers()); + const resCookies = new ResponseCookies(new Headers()); + const store = new StatelessSessionStore({ secret }); + + await store.set(reqCookies, resCookies, session); + + const fcCookies = resCookies + .getAll() + .filter((c) => c.name.startsWith("__FC_")); + expect(fcCookies).toHaveLength(0); + }); + + it("warns when an individual __FC_N cookie exceeds 4096 bytes", async () => { + const secret = await generateSecret(32); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const session: SessionData = { + ...baseSession(Math.floor(Date.now() / 1000)), + connectionTokenSets: [ + // A 4000-char accessToken produces a JWE well over 4096 encoded bytes. + { + connection: "google-oauth2", + accessToken: "x".repeat(4000), + expiresAt: 9999999999 + } + ] + }; + const store = new StatelessSessionStore({ secret }); + await store.set( + new RequestCookies(new Headers()), + new ResponseCookies(new Headers()), + session + ); + + const warned = warnSpy.mock.calls.some((c) => + String(c[0]).includes("__FC_0 cookie size exceeds") + ); + expect(warned).toBe(true); + } finally { + warnSpy.mockRestore(); + } + }); + }); + + describe("set — session size warning emitted only once per process", async () => { + it("warns on the first oversized set() and suppresses the second", async () => { + // Reset the module-level flag by re-importing a fresh module instance. + vi.resetModules(); + const { StatelessSessionStore: FreshStore } = + await import("./stateless-session-store.js"); + const secret = await generateSecret(32); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const store = new FreshStore({ secret }); + const bigSession = (createdAt: number): SessionData => { + const s: SessionData = { + user: { sub: "user_123" }, + tokenSet: { accessToken: "at", expiresAt: 9999999999 }, + internal: { sid: "sid", createdAt } + }; + (s.user as Record).bigClaim = "x".repeat(6000); + return s; + }; + + await store.set( + new RequestCookies(new Headers()), + new ResponseCookies(new Headers()), + bigSession(Math.floor(Date.now() / 1000)) + ); + await store.set( + new RequestCookies(new Headers()), + new ResponseCookies(new Headers()), + bigSession(Math.floor(Date.now() / 1000)) + ); + + const sessionSizeWarns = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("cookie size") + ); + expect(sessionSizeWarns).toHaveLength(1); + } finally { + warnSpy.mockRestore(); + vi.resetModules(); + } + }); + }); + + describe("get — corrupted __FC cookie is silently skipped", async () => { + it("excludes an __FC cookie whose JWE cannot be decrypted", async () => { + const secret = await generateSecret(32); + const session: SessionData = { + user: { sub: "user_123" }, + tokenSet: { accessToken: "at_123", expiresAt: 9999999999 }, + internal: { sid: "sid", createdAt: Math.floor(Date.now() / 1000) } + }; + const expiration = Math.floor(Date.now() / 1000 + 3600); + const validJwe = await encrypt(session, secret, expiration); + + const headers = new Headers(); + headers.append("cookie", `__session=${validJwe};__FC_0=not-a-valid-jwe`); + const reqCookies = new RequestCookies(headers); + const store = new StatelessSessionStore({ secret }); + + const result = await store.get(reqCookies); + + // Session itself is intact; the bad FC cookie is dropped, not throwing. + expect(result?.user.sub).toBe("user_123"); + expect(result?.connectionTokenSets).toBeUndefined(); + }); + }); + + describe("delete — clears __FC_N connection-token cookies", async () => { + it("deletes all __FC_N cookies present in the request", async () => { + const secret = await generateSecret(32); + const expiration = Math.floor(Date.now() / 1000 + 3600); + const fakeJwe = await encrypt( + { connection: "google-oauth2", accessToken: "fc_g", expiresAt: 1 }, + secret, + expiration ); + + const headers = new Headers(); + headers.append("cookie", `__FC_0=${fakeJwe};__FC_1=${fakeJwe}`); + const reqCookies = new RequestCookies(headers); + const resCookies = new ResponseCookies(new Headers()); + const store = new StatelessSessionStore({ secret }); + + await store.delete(reqCookies, resCookies); + + expect(resCookies.get("__FC_0")?.maxAge).toBe(0); + expect(resCookies.get("__FC_1")?.maxAge).toBe(0); }); describe("__FC connection-token orphan cleanup", async () => { diff --git a/src/server/session/stateless-session-store.ts b/src/server/session/stateless-session-store.ts index a4522cf11..a6f6ae457 100644 --- a/src/server/session/stateless-session-store.ts +++ b/src/server/session/stateless-session-store.ts @@ -17,6 +17,30 @@ import { normalizeStatelessSession } from "./normalize-session.js"; +// Total encoded session-cookie size (across all `__session` chunks) above which +// we warn. A large session is the main remaining cause of `431 Request Header +// Fields Too Large`, since — unlike transaction cookies — the session is not +// evicted. 4096 bytes is a conservative threshold: at this size the session +// alone is large, and combined with transaction, connection-token, and +// application cookies the total `Cookie` header can exceed typical 8 KB proxy +// limits. Firing early gives developers a clear "trim your claims or go +// stateful" signal before requests start failing. +const SESSION_COOKIE_SIZE_WARN_BYTES = 4096; + +// Per-cookie size above which we warn for a single `__FC_*` connection-token +// cookie. Numerically the same as the session-total threshold above, but the +// meaning is different: 4096 bytes here is the per-cookie limit browsers are +// documented to guarantee. A single `__FC_*` cookie exceeding this may be +// rejected outright by some browsers, so the warning is per-cookie rather than +// per-request. +const FC_COOKIE_SIZE_WARN_BYTES = 4096; + +// Under rolling sessions, set() runs on ~every authenticated request, so a +// legitimately large-but-working session would otherwise log the size warning +// on every request. Emit it once per process to keep the diagnostic without +// spamming logs. +let sessionSizeWarningEmitted = false; + interface StatelessSessionStoreOptions { secret: string; @@ -117,7 +141,12 @@ export class StatelessSessionStore extends AbstractSessionStore { maxAge }; - cookies.setChunkedCookie( + // Warn when the session cookie is large. This is the main remaining cause of + // 431 errors: the session (unlike transaction cookies) is never evicted, so + // an oversized session can overflow the request-header limit on its own. + // setChunkedCookie returns the total bytes of the chunk(s) it wrote, so no + // separate re-scan of resCookies is needed. + const sessionCookieBytes = cookies.setChunkedCookie( this.sessionCookieName, cookieValue, options, @@ -125,6 +154,20 @@ export class StatelessSessionStore extends AbstractSessionStore { resCookies ); + if ( + sessionCookieBytes >= SESSION_COOKIE_SIZE_WARN_BYTES && + !sessionSizeWarningEmitted + ) { + sessionSizeWarningEmitted = true; + console.warn( + `The ${this.sessionCookieName} cookie size is ${sessionCookieBytes} bytes, which may ` + + "exceed request header size limits and cause 431 Request Header Fields Too Large errors " + + "on some servers, proxies, or CDNs. Consider removing unnecessary custom claims from the " + + "access token or the user profile, or use a stateful session implementation to store the " + + "session data in a data store." + ); + } + // Store connection access tokens, each in its own cookie const connectionTokenSetCount = connectionTokenSets?.length ?? 0; if (connectionTokenSetCount) { @@ -253,27 +296,25 @@ export class StatelessSessionStore extends AbstractSessionStore { // to enable read-after-write in the same request for middleware reqCookies.set(cookieName, cookieValue); - // check if the session cookie size exceeds 4096 bytes, and if so, log a warning + // Measure the encoded `Set-Cookie` string for the per-cookie size check. const cookieJarSizeTest = new cookies.ResponseCookies(new Headers()); cookieJarSizeTest.set(cookieName, cookieValue, { ...this.cookieConfig, maxAge }); - if (new TextEncoder().encode(cookieJarSizeTest.toString()).length >= 4096) { - // if the cookie is the session cookie, log a warning with additional information about the claims and user profile. - if (cookieName === this.sessionCookieName) { - console.warn( - `The ${cookieName} cookie size exceeds 4096 bytes, which may cause issues in some browsers. ` + - "Consider removing any unnecessary custom claims from the access token or the user profile. " + - "Alternatively, you can use a stateful session implementation to store the session data in a data store." - ); - } else { - console.warn( - `The ${cookieName} cookie size exceeds 4096 bytes, which may cause issues in some browsers. ` + - "You can use a stateful session implementation to store the session data in a data store." - ); - } + // storeInCookie only ever writes connection-token (`__FC_*`) cookies — the + // session cookie is written (and size-checked) separately in set(). Warn if + // an individual connection-token cookie exceeds the per-cookie limit + // browsers are documented to guarantee. + if ( + new TextEncoder().encode(cookieJarSizeTest.toString()).length >= + FC_COOKIE_SIZE_WARN_BYTES + ) { + console.warn( + `The ${cookieName} cookie size exceeds ${FC_COOKIE_SIZE_WARN_BYTES} bytes, which may cause issues in some browsers. ` + + "You can use a stateful session implementation to store the session data in a data store." + ); } } diff --git a/src/server/transaction-store.test.ts b/src/server/transaction-store.test.ts index a8446d9d2..bb72a62de 100644 --- a/src/server/transaction-store.test.ts +++ b/src/server/transaction-store.test.ts @@ -2,7 +2,7 @@ import * as jose from "jose"; import * as oauth from "oauth4webapi"; import { describe, expect, it } from "vitest"; -import { generateSecret } from "../test/utils.js"; +import { generateSecret, stripTransactionValuePrefix } from "../test/utils.js"; import { RESPONSE_TYPES } from "../types/connected-accounts.js"; import { decrypt, @@ -108,8 +108,12 @@ describe("Transaction Store", async () => { expect(cookie).toBeDefined(); expect( - ((await decrypt(cookie!.value, secret)) as jose.JWTDecryptResult) - .payload + ( + (await decrypt( + stripTransactionValuePrefix(cookie!.value), + secret + )) as jose.JWTDecryptResult + ).payload ).toEqual(expect.objectContaining(transactionState)); expect(cookie?.path).toEqual("/"); expect(cookie?.httpOnly).toEqual(true); @@ -172,8 +176,12 @@ describe("Transaction Store", async () => { expect(cookie).toBeDefined(); expect( - ((await decrypt(cookie!.value, secret)) as jose.JWTDecryptResult) - .payload + ( + (await decrypt( + stripTransactionValuePrefix(cookie!.value), + secret + )) as jose.JWTDecryptResult + ).payload ).toEqual(expect.objectContaining(transactionState)); expect(cookie?.path).toEqual("/"); expect(cookie?.httpOnly).toEqual(true); @@ -211,8 +219,12 @@ describe("Transaction Store", async () => { expect(cookie).toBeDefined(); expect( - ((await decrypt(cookie!.value, secret)) as jose.JWTDecryptResult) - .payload + ( + (await decrypt( + stripTransactionValuePrefix(cookie!.value), + secret + )) as jose.JWTDecryptResult + ).payload ).toEqual(expect.objectContaining(transactionState)); expect(cookie?.path).toEqual("/"); expect(cookie?.httpOnly).toEqual(true); @@ -250,8 +262,12 @@ describe("Transaction Store", async () => { expect(cookie).toBeDefined(); expect( - ((await decrypt(cookie!.value, secret)) as jose.JWTDecryptResult) - .payload + ( + (await decrypt( + stripTransactionValuePrefix(cookie!.value), + secret + )) as jose.JWTDecryptResult + ).payload ).toEqual(expect.objectContaining(transactionState)); expect(cookie?.path).toEqual("/custom-path"); }); @@ -285,8 +301,12 @@ describe("Transaction Store", async () => { expect(cookie).toBeDefined(); expect( - ((await decrypt(cookie!.value, secret)) as jose.JWTDecryptResult) - .payload + ( + (await decrypt( + stripTransactionValuePrefix(cookie!.value), + secret + )) as jose.JWTDecryptResult + ).payload ).toEqual(expect.objectContaining(transactionState)); expect(cookie?.path).toEqual("/"); expect(cookie?.httpOnly).toEqual(true); @@ -325,8 +345,12 @@ describe("Transaction Store", async () => { expect(cookie).toBeDefined(); expect( - ((await decrypt(cookie!.value, secret)) as jose.JWTDecryptResult) - .payload + ( + (await decrypt( + stripTransactionValuePrefix(cookie!.value), + secret + )) as jose.JWTDecryptResult + ).payload ).toEqual(expect.objectContaining(transactionState)); expect(cookie?.path).toEqual("/"); expect(cookie?.httpOnly).toEqual(true); diff --git a/src/server/transaction-store.ts b/src/server/transaction-store.ts index c0d59b23e..c1d4def5e 100644 --- a/src/server/transaction-store.ts +++ b/src/server/transaction-store.ts @@ -5,6 +5,90 @@ import * as cookies from "./cookies.js"; const TRANSACTION_COOKIE_PREFIX = "__txn_"; +// Maximum total byte size of all transaction (`__txn_*`) cookies +// combined. When the accumulated size meets or exceeds this limit, the oldest +// cookies are evicted (FIFO by creation timestamp) before a new one is written. +// One JWE is ~450–555 bytes, so this allows ~6 concurrent in-flight logins — +// enough for multi-tab use while staying well under the request-header limits +// enforced by browsers (~4 KB per cookie) and servers/proxies. Intentionally +// fixed and not configurable: it caps transaction-cookie accumulation regardless +// of the deployment's header limit, which the SDK cannot know. +const MAX_TRANSACTION_COOKIE_BYTES = 3500; + +// Ceiling on any single user-influenced string field in `TransactionState` +// (returnTo, scope, audience). A very long value in any of these can push the +// transaction cookie past `MAX_TRANSACTION_COOKIE_BYTES` on its own, which the +// FIFO eviction below cannot fix (there is nothing to evict that would make +// room). Anything longer is clamped back to a safe fallback by +// `clampTransactionField` and warned once. 2 KB is well above any realistic +// value for these fields and well under the ~3 KB budget available inside the +// JWE + cookie envelope. +export const MAX_TRANSACTION_FIELD_BYTES = 2048; + +// Backward-compatible alias — the export name callers migrated to first. +export const MAX_RETURN_TO_BYTES = MAX_TRANSACTION_FIELD_BYTES; + +// Emit once per process — same reasoning as sessionSizeWarningEmitted in +// stateless-session-store.ts: prefetch/bot traffic hits this path repeatedly, +// so a per-eviction warn would spam logs. +let txnEvictionWarningEmitted = false; +// Separate flag for the "single cookie exceeds cap on its own" case. Distinct +// from `txnEvictionWarningEmitted` so an operator sees the right diagnostic +// even if the other warn has already fired earlier in the process. +let txnOversizeWarningEmitted = false; + +// One flag per field so a scope clamp does not silence a later returnTo clamp +// (each field surfaces independently to the developer). +const clampWarnEmittedByField = new Set(); + +/** + * If a user-influenced transaction field would push the transaction cookie past + * `MAX_TRANSACTION_COOKIE_BYTES`, clamp it back to `fallback` and warn once per + * field. Returning `fallback` instead of throwing preserves the login flow so + * the user lands on the default post-login path (or an unspecified value if + * `fallback` is `undefined`) instead of hitting a 500. For `returnTo`, this is + * safe because `toSafeRedirect` has already validated same-origin; for `scope` + * and `audience`, the authorization server will reject any invalid value at + * `/authorize`, so silently clamping cannot escalate to something worse than a + * failed authorize call. + * + * @param fieldName - Which field is being clamped ("returnTo" | "scope" | "audience"). + * Used in the warning text and to guard the once-per-field emit. + * @param value - Current value of the field. + * @param fallback - Value to substitute when `value` exceeds the ceiling. + */ +export function clampTransactionField( + fieldName: string, + value: T, + fallback: T +): T { + if ( + value === undefined || + new TextEncoder().encode(value).length <= MAX_TRANSACTION_FIELD_BYTES + ) { + return value; + } + if (!clampWarnEmittedByField.has(fieldName)) { + clampWarnEmittedByField.add(fieldName); + console.warn( + `[auth0] ${fieldName} value exceeds ${MAX_TRANSACTION_FIELD_BYTES} bytes; ` + + `clamping to ${fallback ?? "undefined"} to keep the transaction cookie ` + + `under the header size limit. Shorten the ${fieldName} value in your ` + + `application.` + ); + } + return fallback; +} + +/** + * Backward-compatible wrapper: same behaviour as + * `clampTransactionField("returnTo", returnTo, fallback)` but with the + * original name callers already imported. + */ +export function clampReturnTo(returnTo: string, fallback: string): string { + return clampTransactionField("returnTo", returnTo, fallback); +} + export interface TransactionState extends jose.JWTPayload { codeVerifier?: string; responseType: RESPONSE_TYPES; @@ -149,46 +233,183 @@ export class TransactionStore { * * @param resCookies - The response cookies object to set the transaction cookie on * @param transactionState - The transaction state to save - * @param reqCookies - Optional request cookies to check for existing transactions. - * When provided and `enableParallelTransactions` is false, - * will check for existing transaction cookies. When omitted, - * the existence check is skipped for performance optimization. + * @param reqCookies - Optional request cookies. When provided, enables FIFO + * eviction of accumulated transaction cookies (capped at + * {@link MAX_TRANSACTION_COOKIE_BYTES}) before writing the + * new cookie. * @throws {Error} When transaction state is missing required state parameter */ async save( resCookies: cookies.ResponseCookies, transactionState: TransactionState, - reqCookies?: cookies.RequestCookies + reqCookies?: cookies.RequestCookies | cookies.ReadonlyRequestCookies ) { if (!transactionState.state) { throw new Error("Transaction state is required"); } - // When parallel transactions are disabled, check if a transaction already exists - if (reqCookies && !this.enableParallelTransactions) { - const cookieName = this.getTransactionCookieName(transactionState.state); - const existingCookie = reqCookies.get(cookieName); - if (existingCookie) { - console.warn( - "A transaction is already in progress. Only one transaction is allowed when parallel transactions are disabled." - ); - return; - } - } - - const expirationSeconds = this.cookieOptions.maxAge!; - const expiration = Math.floor(Date.now() / 1000 + expirationSeconds); + const expiration = Math.floor( + Date.now() / 1000 + this.cookieOptions.maxAge! + ); const jwe = await cookies.encrypt( transactionState, this.secret, expiration ); - resCookies.set( - this.getTransactionCookieName(transactionState.state), - jwe.toString(), - this.cookieOptions + // Encode creation timestamp in the value for O(1) FIFO ordering during eviction. + // Format: "{ts}:{jwe}" — cookie name is unchanged. + // + // Compatibility notes: + // - Forward (new code reads old cookie): get() strips a "{ts}:" prefix + // before decrypting, so legacy bare "{jwe}" values still read correctly. + // See `get()` below. + // - Backward (old code reads new cookie): a version of get() that predates + // this change passes the full "{ts}:{jwe}" string to decrypt(), which + // returns null (ERR_JWE_INVALID is swallowed). Any in-flight login + // started against a pod with this write-side change but completed against + // a pod without the read-side prefix stripping will fail once — the user + // sees "state parameter is invalid" and must re-initiate login. This + // affects rolling deploys where old and new pods coexist, and rollbacks. + // Transaction cookies are short-lived (default maxAge 1h), so the window + // is bounded to one hour after the deploy/rollback boundary. + // + // To eliminate the window, ship the get() prefix-stripping in a prior + // backfill release; then every pod can read the new format before the + // first pod writes it. If that is not feasible, call out the one-time + // in-flight login failure in the release notes. + const ts = Math.floor(Date.now() / 1000); + const newCookieName = this.getTransactionCookieName(transactionState.state); + const newCookieValue = `${ts}:${jwe}`; + + // Evict oldest transaction cookies FIFO before writing the new one, so the + // accumulated `__txn_*` cookies stay under the fixed byte limit. Only + // transaction cookies are measured/deleted — the session and other cookies + // are left untouched, and the cookie about to be written is never evicted. + if (reqCookies) { + this.evictOldestTransactionCookies( + reqCookies, + resCookies, + newCookieName, + newCookieValue + ); + } + + resCookies.set(newCookieName, newCookieValue, this.cookieOptions); + } + + /** + * Evicts the oldest transaction cookies (FIFO by the `{ts}:` value prefix) from + * the response so that the accumulated `__txn_*` cookies — including the one + * about to be written — stay under {@link MAX_TRANSACTION_COOKIE_BYTES}. + * + * Only cookies matching the transaction prefix are measured and deleted — the + * session, connection-token, and application cookies are never touched. The + * cookie about to be (re)written for the current transaction (`newCookieName`) + * is never evicted. No-op when the projected total is under the limit. + * + * @param newCookieName - Name of the cookie about to be written (never evicted). + * @param newCookieValue - Value of that cookie; its size is included in the cap + * so a large new cookie can still trigger eviction. + */ + private evictOldestTransactionCookies( + reqCookies: cookies.RequestCookies | cookies.ReadonlyRequestCookies, + resCookies: cookies.ResponseCookies, + newCookieName: string, + newCookieValue: string + ) { + const sizeOf = (name: string, value: string) => + new TextEncoder().encode(`${name}=${value}`).length; + + const txnCookies = reqCookies + .getAll() + .filter((c) => c.name.startsWith(this.transactionCookiePrefix)); + + // Existing transaction-cookie bytes, excluding any cookie with the same name + // as the one we're about to write — its old bytes are replaced, not added. + const existingBytes = txnCookies.reduce( + (sum, c) => + c.name === newCookieName ? sum : sum + sizeOf(c.name, c.value), + 0 + ); + // Project the total that will be on the request header after this write. + const projectedBytes = + existingBytes + sizeOf(newCookieName, newCookieValue); + + if (projectedBytes < MAX_TRANSACTION_COOKIE_BYTES) { + return; + } + + const deleteOptions = { + domain: this.cookieOptions.domain, + path: this.cookieOptions.path, + secure: this.cookieOptions.secure, + sameSite: this.cookieOptions.sameSite, + httpOnly: this.cookieOptions.httpOnly + }; + + // Sort by timestamp encoded in value prefix "{ts}:{jwe}". + // Legacy bare "{jwe}" values (no colon) get timestamp 0 — evicted first. + const sorted = [...txnCookies].sort( + (a, b) => + this.parseCookieTimestamp(a.value) - this.parseCookieTimestamp(b.value) ); + + let freed = 0; + const target = projectedBytes - MAX_TRANSACTION_COOKIE_BYTES + 1; + for (const c of sorted) { + // Never evict the cookie we are about to (re)write for this state. + if (c.name === newCookieName) continue; + cookies.deleteCookie(resCookies, c.name, deleteOptions); + freed += sizeOf(c.name, c.value); + if (freed >= target) break; + } + + if (freed > 0) { + // Something was actually evicted — the accumulation-of-abandoned-logins + // diagnostic is the right one to surface. Guarded by `freed > 0` so we + // do not send an operator hunting for phantom abandoned logins when the + // real issue is a single oversized cookie (see the else branch below). + if (!txnEvictionWarningEmitted) { + txnEvictionWarningEmitted = true; + console.warn( + `[auth0] Evicted the oldest transaction cookie(s) — projected total size ${projectedBytes} bytes ` + + `reached the ${MAX_TRANSACTION_COOKIE_BYTES} byte limit. This usually means many ` + + `login flows were started but never completed (e.g. prefetches or abandoned logins); ` + + `reduce transactionCookie.maxAge if in-flight logins are being evicted too aggressively.` + ); + } + } else { + // Nothing was evicted but the cap is still exceeded — the new cookie + // itself is bigger than the cap on its own. Usually means a very long + // `returnTo`, `scope`, or `audience` slipped past the field clamps. + // Different message so the operator does not go looking for accumulation. + if (!txnOversizeWarningEmitted) { + txnOversizeWarningEmitted = true; + console.warn( + `[auth0] Transaction cookie exceeds the ${MAX_TRANSACTION_COOKIE_BYTES} byte limit ` + + `on its own (projected size ${projectedBytes} bytes). This is usually caused by a very ` + + `long returnTo, scope, or audience value on /auth/login. Check the field clamps in ` + + `handleLogin; the cookie was still written but may be rejected by the browser or a proxy.` + ); + } + } + } + + /** + * Extracts the creation timestamp from a cookie value shaped "{ts}:{jwe}". + * Legacy bare "{jwe}" values (no colon) have no timestamp and sort first (0). + * Uses an explicit split on the first colon rather than `parseInt`, so a + * legacy JWE that happens to start with digits is never mistaken for a + * timestamp — consistent with the split used in {@link get}. + */ + private parseCookieTimestamp(value: string): number { + const colonIdx = value.indexOf(":"); + if (colonIdx === -1) { + return 0; + } + const ts = Number(value.slice(0, colonIdx)); + return Number.isFinite(ts) ? ts : 0; } async get(reqCookies: cookies.RequestCookies, state: string) { @@ -199,7 +420,11 @@ export class TransactionStore { return null; } - return cookies.decrypt(cookieValue, this.secret); + // Strip "{ts}:" prefix before decryption — backward compatible with legacy bare "{jwe}". + const colonIdx = cookieValue.indexOf(":"); + const jwe = colonIdx !== -1 ? cookieValue.slice(colonIdx + 1) : cookieValue; + + return cookies.decrypt(jwe, this.secret); } async delete(resCookies: cookies.ResponseCookies, state: string) { diff --git a/src/server/txn-cookie-accumulation.test.ts b/src/server/txn-cookie-accumulation.test.ts new file mode 100644 index 000000000..e7113fd8b --- /dev/null +++ b/src/server/txn-cookie-accumulation.test.ts @@ -0,0 +1,768 @@ +import { NextRequest } from "next/server.js"; +import * as jose from "jose"; +import * as oauth from "oauth4webapi"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { getDefaultRoutes } from "../test/defaults.js"; +import { generateSecret } from "../test/utils.js"; +import { RESPONSE_TYPES } from "../types/connected-accounts.js"; +import { AuthClient } from "./auth-client.js"; +import { RequestCookies, ResponseCookies } from "./cookies.js"; +import { StatelessSessionStore } from "./session/stateless-session-store.js"; +import { + clampReturnTo, + clampTransactionField, + MAX_RETURN_TO_BYTES, + MAX_TRANSACTION_FIELD_BYTES, + TransactionState, + TransactionStore +} from "./transaction-store.js"; + +vi.mock("oauth4webapi", async () => { + const actual = await vi.importActual("oauth4webapi"); + return { + ...actual, + generateRandomState: vi.fn(), + generateRandomNonce: vi.fn(), + generateRandomCodeVerifier: vi.fn(), + calculatePKCECodeChallenge: vi.fn(), + discoveryRequest: vi.fn(), + processDiscoveryResponse: vi.fn(), + validateAuthResponse: vi.fn(), + getValidatedIdTokenClaims: vi.fn(), + processAuthorizationCodeResponse: vi.fn(), + authorizationCodeGrantRequest: vi.fn() + }; +}); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const makeTransactionState = ( + state: string, + overrides: Partial = {} +): TransactionState => ({ + nonce: "test-nonce", + codeVerifier: "test-cv", + responseType: RESPONSE_TYPES.CODE, + maxAge: 3600, + returnTo: "/", + state, + ...overrides +}); + +/** Build a RequestCookies instance pre-populated with the given name=value pairs. */ +const makeRequestCookies = (pairs: Record): RequestCookies => { + const headers = new Headers(); + const cookieHeader = Object.entries(pairs) + .map(([k, v]) => `${k}=${v}`) + .join("; "); + headers.append("cookie", cookieHeader); + return new RequestCookies(headers); +}; + +/** Build an empty ResponseCookies. */ +const makeResponseCookies = (): ResponseCookies => { + return new ResponseCookies(new Headers()); +}; + +// --------------------------------------------------------------------------- +// Transaction cookie eviction in TransactionStore.save() +// The byte limit is fixed at 3500 bytes and not configurable. Tests exercise it +// by building transaction cookies whose combined size crosses that threshold. +// --------------------------------------------------------------------------- + +// A single transaction cookie value large enough that two of them exceed the +// fixed 3500-byte limit but one does not (~1900 bytes of value each). +const BIG_VALUE = (ts: number) => `${ts}:${"j".repeat(1900)}`; + +describe("transaction cookie eviction in TransactionStore.save()", () => { + let secret: string; + + beforeEach(async () => { + secret = await generateSecret(32); + }); + + it("logs a console.warn once per process when eviction fires", async () => { + vi.resetModules(); + const { TransactionStore: FreshStore } = + await import("./transaction-store.js"); + const store = new FreshStore({ secret }); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const bigCookies = () => + makeRequestCookies({ + __txn_old: BIG_VALUE(1000), + __txn_newer: BIG_VALUE(9999) + }); + + await store.save( + makeResponseCookies(), + makeTransactionState("s1"), + bigCookies() + ); + await store.save( + makeResponseCookies(), + makeTransactionState("s2"), + bigCookies() + ); + + const evictionWarns = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("[auth0] Evicted") + ); + expect(evictionWarns).toHaveLength(1); + warnSpy.mockRestore(); + }); + + it("does NOT log the 'Evicted' warning when nothing was freed (freed === 0)", async () => { + // Regression: previously the "Evicted the oldest transaction cookie(s)" + // warn fired whenever projectedBytes >= cap, even when `freed === 0` + // (nothing was evicted — e.g. only the same-name cookie exists in the + // snapshot, so it is exempt from eviction, but the new cookie itself is + // large enough to exceed the cap). That sent operators debugging a 431 + // hunting for phantom abandoned logins. + // + // The eviction warn is now gated on `freed > 0`, and a distinct warn + // ("Transaction cookie exceeds the … byte limit on its own") covers the + // case where nothing evictable exists. + vi.resetModules(); + const { TransactionStore: FreshStore } = + await import("./transaction-store.js"); + const store = new FreshStore({ secret }); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + // A TransactionState with a returnTo large enough that the resulting JWE + // pushes the new cookie past 3500 bytes on its own. The snapshot is empty, + // so `existingBytes = 0`, `projectedBytes = new cookie size >= cap`, and + // no cookies are available to evict (`freed === 0`). + const bigState = makeTransactionState("only", { + returnTo: "/" + "x".repeat(3200) + }); + + await store.save(makeResponseCookies(), bigState, makeRequestCookies({})); + + const evictionWarns = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("[auth0] Evicted the oldest") + ); + expect(evictionWarns).toHaveLength(0); + + const oversizeWarns = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("Transaction cookie exceeds") + ); + expect(oversizeWarns).toHaveLength(1); + + warnSpy.mockRestore(); + vi.resetModules(); + }); + + it("clampReturnTo returns the input unchanged when it fits under the ceiling", () => { + expect(clampReturnTo("/dash", "/")).toBe("/dash"); + expect(clampReturnTo("/a?q=" + "x".repeat(1000), "/")).toBe( + "/a?q=" + "x".repeat(1000) + ); + }); + + it("clampReturnTo returns the fallback and warns once when input exceeds the ceiling", async () => { + vi.resetModules(); + const { clampReturnTo: freshClamp, MAX_RETURN_TO_BYTES: fresh } = + await import("./transaction-store.js"); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const oversize = "/?" + "x".repeat(fresh + 10); + expect(freshClamp(oversize, "/dashboard")).toBe("/dashboard"); + // Second call also clamps but does not re-warn (once-per-process guard). + expect(freshClamp(oversize, "/dashboard")).toBe("/dashboard"); + + const clampWarns = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("returnTo value exceeds") + ); + expect(clampWarns).toHaveLength(1); + warnSpy.mockRestore(); + vi.resetModules(); + }); + + it("clampReturnTo boundary: exactly MAX_RETURN_TO_BYTES is accepted", () => { + // Encoded length equals MAX_RETURN_TO_BYTES → still <= ceiling, accepted. + const atLimit = "x".repeat(MAX_RETURN_TO_BYTES); + expect(clampReturnTo(atLimit, "/")).toBe(atLimit); + }); + + it("MAX_RETURN_TO_BYTES stays aliased to MAX_TRANSACTION_FIELD_BYTES for back-compat", () => { + expect(MAX_RETURN_TO_BYTES).toBe(MAX_TRANSACTION_FIELD_BYTES); + }); + + it("clampTransactionField clamps scope and audience independently", async () => { + vi.resetModules(); + const { + clampTransactionField: freshClamp, + MAX_TRANSACTION_FIELD_BYTES: fresh + } = await import("./transaction-store.js"); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const oversizeScope = "read:" + "x".repeat(fresh); + const oversizeAudience = "urn:" + "x".repeat(fresh); + expect(freshClamp("scope", oversizeScope, undefined)).toBeUndefined(); + expect(freshClamp("audience", oversizeAudience, undefined)).toBeUndefined(); + + // Each field warns once — a scope clamp does not silence a later audience clamp. + const scopeWarns = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("scope value exceeds") + ); + const audienceWarns = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("audience value exceeds") + ); + expect(scopeWarns).toHaveLength(1); + expect(audienceWarns).toHaveLength(1); + + // Second call for the same field does not re-warn. + freshClamp("scope", oversizeScope, undefined); + const scopeWarnsAfter = warnSpy.mock.calls.filter((c) => + String(c[0]).includes("scope value exceeds") + ); + expect(scopeWarnsAfter).toHaveLength(1); + + warnSpy.mockRestore(); + vi.resetModules(); + }); + + it("clampTransactionField passes through undefined and short values", () => { + expect( + clampTransactionField("scope", undefined, undefined) + ).toBeUndefined(); + expect(clampTransactionField("scope", "openid profile", undefined)).toBe( + "openid profile" + ); + expect(clampTransactionField("audience", "urn:api", undefined)).toBe( + "urn:api" + ); + }); + + it("does not evict when no reqCookies passed (no eviction without snapshot)", async () => { + const store = new TransactionStore({ secret }); + const resCookies = makeResponseCookies(); + const state = "state-no-evict"; + + // With no reqCookies snapshot, eviction is skipped entirely. + await expect( + store.save(resCookies, makeTransactionState(state)) + ).resolves.not.toThrow(); + + expect(resCookies.get(`__txn_${state}`)?.value).toBeTruthy(); + }); + + it("does not evict when accumulated bytes are below the limit", async () => { + const store = new TransactionStore({ secret }); + + const existingState = "existing-state"; + const reqCookies = makeRequestCookies({ + [`__txn_${existingState}`]: "1000:short" + }); + const resCookies = makeResponseCookies(); + const newState = "new-state"; + + await store.save(resCookies, makeTransactionState(newState), reqCookies); + + // Existing cookie was not evicted (no delete set on it) + const evicted = resCookies + .getAll() + .filter((c) => c.name === `__txn_${existingState}` && c.maxAge === 0); + expect(evicted).toHaveLength(0); + + // New cookie was written + expect(resCookies.get(`__txn_${newState}`)?.value).toBeTruthy(); + }); + + it("counts the new cookie in the cap: evicts when existing is under-limit but projected total reaches it", async () => { + const store = new TransactionStore({ secret }); + + // One existing cookie sized just under 3500 bytes on its own — no eviction + // would fire if only existing bytes were counted. The ~500-byte new cookie + // pushes the projected total over the limit, so eviction MUST fire. + const existingState = "existing"; + const nearLimitValue = `1000:${"j".repeat(3400)}`; // ~3413 bytes with name + const reqCookies = makeRequestCookies({ + [`__txn_${existingState}`]: nearLimitValue + }); + const resCookies = makeResponseCookies(); + + await store.save(resCookies, makeTransactionState("newstate"), reqCookies); + + // The existing (older) cookie is evicted so the projected total stays bounded + expect(resCookies.get(`__txn_${existingState}`)?.maxAge).toBe(0); + // New cookie still written + expect(resCookies.get("__txn_newstate")?.value).toBeTruthy(); + }); + + it("evicts oldest cookie first when the 3500 byte limit is exceeded", async () => { + const store = new TransactionStore({ secret }); + + const olderState = "older"; + const newerState = "newer"; + // Two big cookies together exceed 3500 bytes → eviction fires and only needs + // to remove the single oldest to get back under the limit. + const reqCookies = makeRequestCookies({ + [`__txn_${olderState}`]: BIG_VALUE(1000), + [`__txn_${newerState}`]: BIG_VALUE(9999) + }); + const resCookies = makeResponseCookies(); + + const newState = "newstate"; + await store.save(resCookies, makeTransactionState(newState), reqCookies); + + // Older cookie evicted first — present as a deletion tombstone (maxAge 0). + expect(resCookies.get(`__txn_${olderState}`)?.maxAge).toBe(0); + // Newer cookie untouched — eviction stopped after freeing enough. It must + // not appear on the response at all (not even as a deletion tombstone). + expect(resCookies.get(`__txn_${newerState}`)).toBeUndefined(); + // New cookie written + expect(resCookies.get(`__txn_${newState}`)?.value).toBeTruthy(); + }); + + it("evicts the two oldest first when three cookies must be freed (FIFO order)", async () => { + const store = new TransactionStore({ secret }); + + // Three cookies all sized so the total exceeds 3500 bytes and two must be + // evicted before the new one can be written within the cap. + const oldestState = "oldest"; + const middleState = "middle"; + const newestState = "newest"; + const reqCookies = makeRequestCookies({ + [`__txn_${oldestState}`]: BIG_VALUE(1000), // ts=1000 — evicted first + [`__txn_${middleState}`]: BIG_VALUE(5000), // ts=5000 — evicted second + [`__txn_${newestState}`]: BIG_VALUE(9999) // ts=9999 — must survive + }); + const resCookies = makeResponseCookies(); + + const newState = "latest"; + await store.save(resCookies, makeTransactionState(newState), reqCookies); + + // Oldest two evicted in timestamp order. + expect(resCookies.get(`__txn_${oldestState}`)?.maxAge).toBe(0); + expect(resCookies.get(`__txn_${middleState}`)?.maxAge).toBe(0); + // Newest existing cookie untouched — must not appear on the response. + expect(resCookies.get(`__txn_${newestState}`)).toBeUndefined(); + // New cookie written. + expect(resCookies.get(`__txn_${newState}`)?.value).toBeTruthy(); + }); + + it("evicts legacy cookies (no timestamp prefix) as oldest (timestamp=0)", async () => { + // Legacy format "{jwe}" has no prefix → gets timestamp 0 → oldest in FIFO + const store = new TransactionStore({ secret }); + + const legacyState = "legacy"; + const newerState = "newer"; + const reqCookies = makeRequestCookies({ + [`__txn_${legacyState}`]: "r".repeat(1900), // legacy bare value, no "{ts}:" + [`__txn_${newerState}`]: BIG_VALUE(9999), + other_cookie: "keep_me" + }); + const resCookies = makeResponseCookies(); + + const newState = "newstate"; + await store.save(resCookies, makeTransactionState(newState), reqCookies); + + // Legacy cookie evicted (ts=0, oldest) + expect(resCookies.get(`__txn_${legacyState}`)?.maxAge).toBe(0); + // New cookie written + expect(resCookies.get(`__txn_${newState}`)?.value).toBeTruthy(); + // Non-txn cookie untouched + expect(resCookies.get("other_cookie")).toBeUndefined(); + }); + + it("only evicts cookies matching the configured prefix", async () => { + const customPrefix = "__my_txn_"; + const store = new TransactionStore({ + secret, + cookieOptions: { prefix: customPrefix } + }); + + // Two big custom-prefix cookies exceed the limit; a same-sized cookie with a + // different prefix must not be counted toward the budget or evicted. + const reqCookies = makeRequestCookies({ + [`${customPrefix}state1`]: BIG_VALUE(1000), + [`${customPrefix}state2`]: BIG_VALUE(2000), + __txn_other: BIG_VALUE(1000) // different prefix — should NOT be evicted + }); + const resCookies = makeResponseCookies(); + + await store.save( + resCookies, + makeTransactionState("new", { state: "new" }), + reqCookies + ); + + // Oldest custom-prefix cookie evicted + expect(resCookies.get(`${customPrefix}state1`)?.maxAge).toBe(0); + // __txn_other has a different prefix — not touched by this store + expect(resCookies.get("__txn_other")).toBeUndefined(); + }); + + it("cookie value is encoded as '{ts}:{jwe}'", async () => { + const store = new TransactionStore({ secret }); + const resCookies = makeResponseCookies(); + const state = "login-state"; + + await store.save(resCookies, makeTransactionState(state)); + + const value = resCookies.get(`__txn_${state}`)?.value ?? ""; + const colonIdx = value.indexOf(":"); + expect(colonIdx).toBeGreaterThan(0); + const ts = parseInt(value.slice(0, colonIdx)); + expect(ts).toBeGreaterThan(0); + expect(value.slice(colonIdx + 1)).toBeTruthy(); + }); + + it("cookie gets full maxAge (1h default)", async () => { + const store = new TransactionStore({ secret }); + const resCookies = makeResponseCookies(); + const state = "full-ttl"; + + await store.save(resCookies, makeTransactionState(state)); + + expect(resCookies.get(`__txn_${state}`)?.maxAge).toBe(3600); + }); + + it("get() strips '{ts}:' prefix before decrypting", async () => { + const store = new TransactionStore({ secret }); + const resCookies = makeResponseCookies(); + const state = "get-test"; + + await store.save(resCookies, makeTransactionState(state)); + + const encodedValue = resCookies.get(`__txn_${state}`)?.value ?? ""; + expect(encodedValue.match(/^\d+:/)).toBeTruthy(); + + const reqCookies = makeRequestCookies({ [`__txn_${state}`]: encodedValue }); + const result = await store.get(reqCookies, state); + + expect(result).not.toBeNull(); + expect(result?.payload?.state).toBe(state); + }); +}); + +// --------------------------------------------------------------------------- +// Single-transaction mode: overwrite the fixed __txn_ cookie on repeated login +// --------------------------------------------------------------------------- + +describe("single-transaction mode does not lock out concurrent logins", () => { + let secret: string; + + beforeEach(async () => { + secret = await generateSecret(32); + }); + + it("overwrites stale __txn_ cookie when user retries login after abandonment", async () => { + const store = new TransactionStore({ + secret, + enableParallelTransactions: false + }); + + // Simulate stale cookie from abandoned login sitting in browser + const reqCookies = makeRequestCookies({ __txn_: "stale_jwe_value" }); + const resCookies = makeResponseCookies(); + + const newState = "new-login-state"; + + // Previously the single-transaction path returned early and skipped writing. + // A stale in-request cookie must not block a fresh login — save() must overwrite. + await store.save(resCookies, makeTransactionState(newState), reqCookies); + + const written = resCookies.get("__txn_"); + expect(written).toBeDefined(); + expect(written?.value).not.toBe("stale_jwe_value"); + expect(written?.value).toBeTruthy(); + expect(written?.maxAge).not.toBe(0); + }); + + it("uses fixed cookie name __txn_ regardless of state value", async () => { + const store = new TransactionStore({ + secret, + enableParallelTransactions: false + }); + + const resCookies = makeResponseCookies(); + const state = "some-state-value"; + await store.save(resCookies, makeTransactionState(state)); + + // Cookie name must be "__txn_", not "__txn_{state}" + expect(resCookies.get("__txn_")).toBeDefined(); + expect(resCookies.get(`__txn_${state}`)).toBeUndefined(); + }); + + it("creates unique __txn_{state} cookies in parallel mode (baseline)", async () => { + const store = new TransactionStore({ + secret, + enableParallelTransactions: true + }); + + const resCookies = makeResponseCookies(); + const stateA = "stateA"; + const stateB = "stateB"; + + await store.save(resCookies, makeTransactionState(stateA)); + await store.save(resCookies, makeTransactionState(stateB)); + + expect(resCookies.get(`__txn_${stateA}`)?.value).toBeTruthy(); + expect(resCookies.get(`__txn_${stateB}`)?.value).toBeTruthy(); + }); +}); + +// --------------------------------------------------------------------------- +// Callback cleanup: delete only the completing flow's cookie +// --------------------------------------------------------------------------- + +describe("callback cleanup: delete(state) removes only the completing cookie", () => { + let secret: string; + + beforeEach(async () => { + secret = await generateSecret(32); + }); + + it("deletes only the completing flow's cookie, leaves other real login cookies untouched", async () => { + const store = new TransactionStore({ secret }); + const resCookies = makeResponseCookies(); + resCookies.set("__txn_stateA", "1000:jwe_a"); + resCookies.set("__txn_stateB", "2000:jwe_b"); + + await store.delete(resCookies, "stateA"); + + expect(resCookies.get("__txn_stateA")?.maxAge).toBe(0); + // stateB must still be present with its original value and not a deletion + // tombstone (maxAge 0). + const stateB = resCookies.get("__txn_stateB"); + expect(stateB).toBeDefined(); + expect(stateB?.value).toBe("2000:jwe_b"); + expect(stateB?.maxAge).not.toBe(0); + }); + + it("does not throw when deleting a non-existent state", async () => { + const store = new TransactionStore({ secret }); + const resCookies = makeResponseCookies(); + + await expect( + store.delete(resCookies, "nonexistent-state") + ).resolves.not.toThrow(); + }); + + it("single-transaction mode: delete(state) resolves to __txn_ regardless of state value", async () => { + const store = new TransactionStore({ + secret, + enableParallelTransactions: false + }); + const resCookies = makeResponseCookies(); + resCookies.set("__txn_", "1000:stale_jwe"); + + await store.delete(resCookies, "any-state-value"); + + expect(resCookies.get("__txn_")?.maxAge).toBe(0); + }); +}); + +// --------------------------------------------------------------------------- +// Integration tests — handler() prefetch guard + handleCallback sweep +// These cover the three checklist items that require AuthClient + real flows. +// --------------------------------------------------------------------------- + +describe("Integration — prefetch guard and callback cleanup via AuthClient", () => { + const domain = "test.auth0.com"; + const clientId = "test-client-id"; + let keyPair: jose.GenerateKeyPairResult; + let secret: string; + + // Minimal mock authorization server used across all integration tests. + const makeFetch = () => + vi.fn(async (input: RequestInfo | URL): Promise => { + const url = new URL(input instanceof Request ? input.url : input); + if (url.pathname === "/.well-known/openid-configuration") { + return Response.json({ + issuer: `https://${domain}/`, + authorization_endpoint: `https://${domain}/authorize`, + token_endpoint: `https://${domain}/oauth/token`, + jwks_uri: `https://${domain}/.well-known/jwks.json` + }); + } + if (url.pathname === "/.well-known/jwks.json") { + return Response.json({ + keys: [ + { + ...(await jose.exportJWK(keyPair.publicKey)), + kid: "k1", + use: "sig" + } + ] + }); + } + if (url.pathname === "/oauth/token") { + const idToken = await new jose.SignJWT({ + sub: "user123", + sid: "sid123", + nonce: "test-nonce", + aud: clientId, + iss: `https://${domain}/` + }) + .setProtectedHeader({ alg: "RS256" }) + .setIssuedAt() + .setExpirationTime("2h") + .sign(keyPair.privateKey); + return Response.json({ + token_type: "Bearer", + access_token: "at_123", + id_token: idToken, + expires_in: 3600 + }); + } + return new Response(null, { status: 404 }); + }); + + const makeAuthClient = () => { + const transactionStore = new TransactionStore({ secret }); + const sessionStore = new StatelessSessionStore({ secret }); + return new AuthClient({ + domain, + clientId, + clientSecret: "test-secret", + appBaseUrl: "http://localhost:3000", + secret, + transactionStore, + sessionStore, + routes: getDefaultRoutes(), + fetch: makeFetch() + }); + }; + + beforeEach(async () => { + vi.clearAllMocks(); + secret = await generateSecret(32); + keyPair = await jose.generateKeyPair("RS256"); + + vi.mocked(oauth.generateRandomState).mockReturnValue("test-state"); + vi.mocked(oauth.generateRandomNonce).mockReturnValue("test-nonce"); + vi.mocked(oauth.generateRandomCodeVerifier).mockReturnValue("cv"); + vi.mocked(oauth.calculatePKCECodeChallenge).mockResolvedValue("cc"); + vi.mocked(oauth.validateAuthResponse).mockReturnValue( + new URLSearchParams("code=auth_code&state=test-state") + ); + vi.mocked(oauth.discoveryRequest).mockResolvedValue(new Response()); + vi.mocked(oauth.processDiscoveryResponse).mockResolvedValue({ + issuer: `https://${domain}/`, + authorization_endpoint: `https://${domain}/authorize`, + token_endpoint: `https://${domain}/oauth/token`, + jwks_uri: `https://${domain}/.well-known/jwks.json` + } as oauth.AuthorizationServer); + vi.mocked(oauth.authorizationCodeGrantRequest).mockResolvedValue( + new Response() + ); + vi.mocked(oauth.processAuthorizationCodeResponse).mockResolvedValue({ + token_type: "Bearer", + access_token: "at_123", + id_token: "id_token_placeholder", + expires_in: 3600 + } as oauth.TokenEndpointResponse); + vi.mocked(oauth.getValidatedIdTokenClaims).mockReturnValue({ + sub: "user123", + sid: "sid123", + nonce: "test-nonce", + aud: clientId, + iss: `https://${domain}/`, + iat: Math.floor(Date.now() / 1000) - 60, + exp: Math.floor(Date.now() / 1000) + 3600 + }); + }); + + it("known prefetch header returns 204 with no-store and no __txn_* cookie is written", async () => { + const authClient = makeAuthClient(); + const req = new NextRequest("http://localhost:3000/auth/login", { + headers: { "next-router-prefetch": "1" } + }); + + const res = await authClient.handler(req); + + expect(res.status).toBe(204); + // Cache-Control: no-store prevents CDNs/proxies from caching this 204 and + // serving it for real login navigations. + expect(res.headers.get("cache-control")).toBe("no-store"); + const txnCookies = res.cookies + .getAll() + .filter((c) => c.name.startsWith("__txn_") && c.maxAge !== 0); + expect(txnCookies).toHaveLength(0); + }); + + it("real navigation is allowed through and __txn_* cookie is written", async () => { + const authClient = makeAuthClient(); + const req = new NextRequest("http://localhost:3000/auth/login", { + headers: { "sec-fetch-mode": "navigate" } + }); + + const res = await authClient.handler(req); + + expect(res.status).toBeGreaterThanOrEqual(300); + expect(res.status).toBeLessThan(400); + const txnCookies = res.cookies + .getAll() + .filter((c) => c.name.startsWith("__txn_") && (c.maxAge ?? 0) > 0); + expect(txnCookies.length).toBeGreaterThan(0); + }); + + it("oversized returnTo does not 500 the handler — request completes and cookie is bounded", async () => { + // Regression: a very long user-supplied returnTo used to throw + // InvalidConfigurationError out of save(), which surfaced as an unhandled + // 500 from the /auth/login route. Now clamped back to signInReturnToPath + // at the input boundary, so the login flow proceeds normally. + const authClient = makeAuthClient(); + const bigReturnTo = "/dashboard?data=" + "x".repeat(4000); + const req = new NextRequest( + `http://localhost:3000/auth/login?returnTo=${encodeURIComponent(bigReturnTo)}`, + { headers: { "sec-fetch-mode": "navigate" } } + ); + + const res = await authClient.handler(req); + + // Login proceeds as a normal redirect, not a 500. + expect(res.status).toBeGreaterThanOrEqual(300); + expect(res.status).toBeLessThan(400); + + // The written __txn_* cookie stays under the byte cap. + const written = res.cookies + .getAll() + .find((c) => c.name.startsWith("__txn_") && (c.maxAge ?? 0) > 0); + expect(written).toBeDefined(); + const cookieBytes = new TextEncoder().encode( + `${written!.name}=${written!.value}` + ).length; + expect(cookieBytes).toBeLessThan(3500); + }); + + it("handleCallback deletes only the completing cookie, leaves Tab B cookie untouched", async () => { + const authClient = makeAuthClient(); + + const loginRes = await authClient.handleLogin( + new NextRequest("http://localhost:3000/auth/login") + ); + const state = new URL(loginRes.headers.get("Location")!).searchParams.get( + "state" + )!; + const txnCookie = loginRes.cookies.get(`__txn_${state}`); + expect(txnCookie).toBeDefined(); + expect(txnCookie!.value).toMatch(/^\d+:/); + + const callbackReq = new NextRequest( + `http://localhost:3000/auth/callback?code=auth_code&state=${state}` + ); + callbackReq.cookies.set(`__txn_${state}`, txnCookie!.value); + callbackReq.cookies.set("__txn_tabB", "9999999999:tab_b_real_login_jwe"); + + const callbackRes = await authClient.handleCallback(callbackReq); + + expect(callbackRes.status).toBeGreaterThanOrEqual(300); + expect(callbackRes.status).toBeLessThan(400); + + // Completing cookie deleted + expect(callbackRes.cookies.get(`__txn_${state}`)?.maxAge).toBe(0); + // Tab B real login cookie must NOT be deleted — it must not appear on the + // response at all (not even as a deletion tombstone). + expect(callbackRes.cookies.get("__txn_tabB")).toBeUndefined(); + // Session written + expect(callbackRes.cookies.get("__session")?.value).toBeTruthy(); + }); +}); diff --git a/src/test/utils.ts b/src/test/utils.ts index 536212434..d41700ce0 100644 --- a/src/test/utils.ts +++ b/src/test/utils.ts @@ -5,3 +5,15 @@ export async function generateSecret(length: number) { .map((b) => b.toString(16).padStart(2, "0")) .join(""); } + +/** + * Strip the value prefix that TransactionStore encodes in cookie values. + * "{ts}:{jwe}" → real login — strips "{ts}:" prefix + * "{jwe}" → legacy (no prefix) — returned as-is + * + * Use this in tests that decrypt transaction cookie values directly. + */ +export function stripTransactionValuePrefix(value: string): string { + const colonIdx = value.indexOf(":"); + return colonIdx !== -1 ? value.slice(colonIdx + 1) : value; +} diff --git a/src/utils/request.test.ts b/src/utils/request.test.ts index 2074ad3a5..7b484b5ee 100644 --- a/src/utils/request.test.ts +++ b/src/utils/request.test.ts @@ -1,6 +1,115 @@ +import { NextRequest } from "next/server.js"; import { describe, expect, it } from "vitest"; -import { isRequest } from "./request.js"; +import { isNonNavigationalRequest, isRequest } from "./request.js"; + +describe("isNonNavigationalRequest", () => { + const makeReq = (headers: Record) => { + const req = new NextRequest("http://localhost:3000/auth/login"); + Object.entries(headers).forEach(([k, v]) => req.headers.set(k, v)); + return req; + }; + + describe("known prefetch headers — positive detection only", () => { + it("returns true when next-router-prefetch is 1 (Next 15 AUTO prefetch)", () => { + expect( + isNonNavigationalRequest(makeReq({ "next-router-prefetch": "1" })) + ).toBe(true); + }); + + it("returns true when next-router-prefetch is 2 (Next 16 runtime prefetch)", () => { + expect( + isNonNavigationalRequest(makeReq({ "next-router-prefetch": "2" })) + ).toBe(true); + }); + + it("returns true when purpose is prefetch", () => { + expect(isNonNavigationalRequest(makeReq({ purpose: "prefetch" }))).toBe( + true + ); + }); + + it("returns true when sec-purpose is prefetch", () => { + expect( + isNonNavigationalRequest(makeReq({ "sec-purpose": "prefetch" })) + ).toBe(true); + }); + + it("returns true when x-middleware-prefetch is 1 (Pages Router)", () => { + expect( + isNonNavigationalRequest(makeReq({ "x-middleware-prefetch": "1" })) + ).toBe(true); + }); + }); + + describe("requests that must not be blocked", () => { + it("returns false for plain navigation with no prefetch headers", () => { + expect(isNonNavigationalRequest(makeReq({ accept: "text/html" }))).toBe( + false + ); + }); + + it("returns false for accept: text/x-component — real RSC navigation must not be blocked", () => { + // text/x-component is sent by ALL App Router RSC requests, including a + // genuine client-side click — not just prefetches. + expect( + isNonNavigationalRequest(makeReq({ accept: "text/x-component" })) + ).toBe(false); + }); + + it("returns false for sec-fetch-mode: navigate", () => { + expect( + isNonNavigationalRequest(makeReq({ "sec-fetch-mode": "navigate" })) + ).toBe(false); + }); + + it("returns false for sec-fetch-mode: cors — legitimate fetch()/XHR must not be blocked", () => { + expect( + isNonNavigationalRequest(makeReq({ "sec-fetch-mode": "cors" })) + ).toBe(false); + }); + + it("returns false for sec-fetch-mode: same-origin — legitimate fetch()/XHR must not be blocked", () => { + expect( + isNonNavigationalRequest(makeReq({ "sec-fetch-mode": "same-origin" })) + ).toBe(false); + }); + + it("returns false for sec-purpose: prefetch;prerender — Speculation Rules prerender must not be blocked", () => { + // A prerender is the real navigation executed ahead of time. Blocking it + // with 204 would silently swallow the login click when the user navigates. + expect( + isNonNavigationalRequest( + makeReq({ "sec-purpose": "prefetch;prerender" }) + ) + ).toBe(false); + }); + + it("returns false when no headers present", () => { + expect(isNonNavigationalRequest(makeReq({}))).toBe(false); + }); + + it("returns false when next-router-prefetch has a falsy value — proxies or clients setting it explicitly must not disable login", () => { + // Guard against a proxy or misconfigured client sending an explicit + // falsy value. Only genuine truthy prefetch values should trigger 204. + expect( + isNonNavigationalRequest(makeReq({ "next-router-prefetch": "" })) + ).toBe(false); + expect( + isNonNavigationalRequest(makeReq({ "next-router-prefetch": "0" })) + ).toBe(false); + expect( + isNonNavigationalRequest(makeReq({ "next-router-prefetch": "false" })) + ).toBe(false); + expect( + isNonNavigationalRequest(makeReq({ "next-router-prefetch": "FALSE" })) + ).toBe(false); + expect( + isNonNavigationalRequest(makeReq({ "next-router-prefetch": " " })) + ).toBe(false); + }); + }); +}); describe("isRequest", () => { it("returns true for a Fetch Request instance", () => { diff --git a/src/utils/request.ts b/src/utils/request.ts index 545916eef..832f2a4a8 100644 --- a/src/utils/request.ts +++ b/src/utils/request.ts @@ -16,3 +16,49 @@ export const isRequest = (req: Req): req is Request | NextRequest => { typeof (req as Request).bodyUsed === "boolean" ); }; + +/** + * Returns true only when a request carries an unambiguous prefetch signal. + * Used to block Next.js prefetch requests from triggering handleLogin. + * + * Only headers that are exclusive to prefetches are checked: + * - `next-router-prefetch` / `x-middleware-prefetch` — Next.js prefetch markers + * - `purpose` / `sec-purpose` = `prefetch` — W3C/browser prefetch hints + * + * `sec-purpose` matching: + * - `prefetch` alone → prefetch, block it. + * - `prefetch;prerender` → Speculation Rules prerender. The browser activates + * this as the real navigation when the user clicks, so blocking it with 204 + * would silently swallow the login click. Excluded by requiring the value + * does not contain `prerender`. + * + * Intentionally excludes: + * - `sec-fetch-mode` — also set on legitimate fetch()/XHR calls to /auth/login. + * - `accept: text/x-component` — sent by ALL App Router RSC requests, including + * real client-side `` navigations (e.g. ``), so + * matching it would 401 genuine login clicks, not just prefetches. + * + * Note: `x-middleware-prefetch` is a Pages Router signal only + * (`shared/lib/router/router.js`). It never fires on App Router prefetch paths, + * but is kept for Pages Router coverage. + */ +export const isNonNavigationalRequest = (req: NextRequest): boolean => { + const secPurpose = req.headers.get("sec-purpose") ?? ""; + const routerPrefetch = req.headers.get("next-router-prefetch"); + return ( + // next-router-prefetch: "1" = AUTO prefetch (Next 15); "2" = runtime + // prefetch (Next 16+). Accept any truthy non-"0"/non-"false" value to + // cover future Next versions, but reject falsy values a proxy or client + // might set explicitly — matching those would silently 204 real logins. + isTruthyHeaderValue(routerPrefetch) || + req.headers.get("purpose") === "prefetch" || + (secPurpose.includes("prefetch") && !secPurpose.includes("prerender")) || + req.headers.get("x-middleware-prefetch") === "1" + ); +}; + +const isTruthyHeaderValue = (value: string | null): boolean => { + if (!value) return false; + const normalized = value.trim().toLowerCase(); + return normalized !== "" && normalized !== "0" && normalized !== "false"; +}; diff --git a/src/utils/session-helpers.ts b/src/utils/session-helpers.ts index 9083d7ab3..91b16105d 100644 --- a/src/utils/session-helpers.ts +++ b/src/utils/session-helpers.ts @@ -98,7 +98,14 @@ export function mergePopupTokenIntoSession( token_type: oidcRes.token_type }; - // Replace existing token for same audience, or append new one + // Replace existing token for same audience, or append new one. + // + // NOTE: This dedup keys on audience alone — differently-scoped tokens for + // the same audience are evicted here. The MFA step-up dedup in + // `cacheTokenFromMfaVerify` (auth-client.ts) uses a stricter audience+scope + // key that preserves distinct-scope entries. Both rules are pre-existing on + // main and ship side by side. Unifying them into one shared helper is a + // separate refactor. const existingIdx = session.accessTokens.findIndex( (t) => t.audience === transactionState.audience );