From 03e72f13e334e73811834309fc87c42bd218d55c Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 14:51:45 -0700 Subject: [PATCH 01/21] docs: spec for per-flow PKCE verifier cookies Design for porting the authkit-nextjs PR #403 fix (per-flow PKCE cookie names) to authkit-session and its downstream adapters (authkit-sveltekit, authkit-tanstack-start). Additive to the core's public API except for a deliberate breaking change to clearPendingVerifier, which is only called in-tree. --- ...2026-04-22-per-flow-pkce-cookies-design.md | 250 ++++++++++++++++++ 1 file changed, 250 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md diff --git a/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md b/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md new file mode 100644 index 0000000..22e9f94 --- /dev/null +++ b/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md @@ -0,0 +1,250 @@ +# Per-flow PKCE Verifier Cookies + +**Date:** 2026-04-22 +**Status:** Approved — ready for implementation plan +**Packages:** `authkit-session`, `authkit-sveltekit`, `authkit-tanstack-start` + +## Problem + +When a user has multiple tabs open against an app and the session expires (or multiple requests fire concurrently at an unauthenticated session), each request that hits the auth-url-minting code path generates a fresh PKCE verifier and writes it to the single shared `wos-auth-verifier` cookie. Last-write-wins: every callback except one fails with `OAuthStateMismatchError` or `PKCECookieMissingError`. + +This is the same bug `authkit-nextjs` fixed in PR [#403](https://github.com/workos/authkit-nextjs/pull/403). `authkit-session` and both downstream adapters (`authkit-sveltekit`, `authkit-tanstack-start`) currently share the bug because the cookie name is a module-level constant. + +### Evidence in code + +- `authkit-session/src/core/pkce/cookieOptions.ts:5` — `PKCE_COOKIE_NAME = 'wos-auth-verifier'` is a single constant. +- `authkit-session/src/service/AuthService.ts:236,324,368` — `createAuthorization` writes, `handleCallback` reads and clears, all against that one name. +- No concurrency isolation exists in the core or either adapter. + +## Goals + +1. Eliminate cross-flow clobbering by giving each concurrent PKCE flow its own cookie name. +2. Keep the fix additive in `authkit-session`'s public API except for a single deliberate breaking change to `clearPendingVerifier` (called only inside our own adapters). +3. Avoid HTTP 431 cookie bloat in middleware-loop-prone adapter paths. + +## Non-goals + +- No legacy cookie fallback. `authkit-session` has negligible real users mid-flow; the upgrade window isn't worth protecting. +- No orphan cleanup at `signOut`. `authkit-tanstack-start` hasn't shipped; `authkit-sveltekit` has no real users. The 10-minute PKCE TTL handles orphans. +- No shared document-request helper in the core. Header heuristics are adapter-specific. + +## Design + +### 1. `authkit-session` (the core library) + +#### 1.1 Cookie name derivation + +New file: `src/core/pkce/cookieName.ts`. + +```ts +export const PKCE_COOKIE_PREFIX = 'wos-auth-verifier'; + +function fnv1a32Hex(input: string): string { + let hash = 0x811c9dc5; + const bytes = new TextEncoder().encode(input); + for (const byte of bytes) { + hash = Math.imul(hash ^ byte, 0x01000193) >>> 0; + } + return hash.toString(16).padStart(8, '0'); +} + +export function getPKCECookieNameForState(state: string): string { + return `${PKCE_COOKIE_PREFIX}-${fnv1a32Hex(state)}`; +} +``` + +Inline FNV-1a 32-bit, no new dependency. Hash isn't security-sensitive — it's a namespacing mechanism. Collision probability is ~1/4B per pair; a collision would route one flow's callback to the wrong cookie, which then fails byte-equality against the URL state (fail-closed via existing `verifyCallbackState` logic). + +`PKCE_COOKIE_NAME` stays as an alias export for back-compat; internal code uses `PKCE_COOKIE_PREFIX`. + +#### 1.2 Return derived cookie name from URL generation + +`src/core/pkce/generateAuthorizationUrl.ts`: + +- `GeneratedAuthorizationUrl` gains `cookieName: string`, computed via `getPKCECookieNameForState(sealedState)`. +- The cookie byte-length guard uses the actual (longer) derived name — worst case 26 chars vs 17, still well under the 3800-byte cap. + +#### 1.3 Public pure URL-generation API + +Expose the side-effect-free path that `AuthOperations` already owns. Callers that don't want to write a cookie can bypass `storage.setCookie` entirely. + +`AuthService` gains three new methods: + +```ts +getAuthorizationUrl(options?): Promise<{ url: string; cookieName: string }> +getSignInUrl(options?): Promise<{ url: string; cookieName: string }> +getSignUpUrl(options?): Promise<{ url: string; cookieName: string }> +``` + +No response argument, no `Set-Cookie`, no `storage` touched. Thin wrappers around `AuthOperations.createAuthorization` / `createSignIn` / `createSignUp`. + +Existing `createAuthorization` / `createSignIn` / `createSignUp` continue to write the cookie, using the derived per-flow name internally. `CreateAuthorizationResult` gains `cookieName: string` for callers that want to track the flow (e.g., to pass back to `clearPendingVerifier`). + +No `writeCookie?: boolean` flag — rejected in favor of the cleaner URL-only methods. + +#### 1.4 `handleCallback` + +`AuthService.handleCallback` derives the cookie name from `options.state` (URL state) and uses it for both the cookie read and any subsequent clear. Pre-unseal error path uses the derived name. Post-unseal error path also uses the same derived name (cheaper than deriving from the unsealed blob, and equivalent by construction). + +Happy-path sketch: + +```ts +async handleCallback(request, response, options: { code, state }) { + const cookieName = options.state ? getPKCECookieNameForState(options.state) : null; + const cookieValue = cookieName + ? await this.storage.getCookie(request, cookieName) + : null; + + // verifyCallbackState throws OAuthStateMismatchError when state is missing + // and PKCECookieMissingError when cookieValue is null — existing behavior. + const unsealed = await this.core.verifyCallbackState({ + stateFromUrl: options.state, + cookieValue: cookieValue ?? undefined, + }); + // ... rest unchanged; clearCookie uses `cookieName` +} +``` + +Adapters require no change to their `handleCallback` call sites. + +#### 1.5 `clearPendingVerifier` — breaking signature change + +Today: + +```ts +clearPendingVerifier(response, options?: { redirectUri? }) +``` + +Becomes: + +```ts +clearPendingVerifier( + response, + options: { state: string; redirectUri?: string }, +) +``` + +Rationale: a state-less cleanup is meaningless in the per-flow world — we'd have no cookie name to clear. Rather than silently no-op and hide bugs, force callers to pass `state`. The only callers today are in `authkit-sveltekit/src/server/auth.ts` and `authkit-tanstack-start/src/server/server.ts`, both in callback bailout paths where URL `state` is available. + +This is the one deliberate breaking change in the core. It's justified because the old signature encoded an invariant that no longer holds. + +#### 1.6 Exports + +New public exports from `src/index.ts`: + +- `getPKCECookieNameForState` +- `PKCE_COOKIE_PREFIX` + +Retained for back-compat: `PKCE_COOKIE_NAME` (aliased to the prefix). + +Not exported (deliberately): any document-request helper. Header heuristics are adapter-layer concerns. + +### 2. `authkit-sveltekit` + +#### 2.1 Adapter-local document-request helper + +Ship `src/server/adapters/isDocumentRequest.ts`: + +```ts +export function isDocumentRequest(headers: Headers): boolean { + const dest = headers.get('sec-fetch-dest'); + if (dest) return dest === 'document'; + if (headers.get('x-requested-with')?.toLowerCase() === 'xmlhttprequest') return false; + if (headers.get('purpose')?.toLowerCase() === 'prefetch') return false; + const accept = headers.get('accept') ?? ''; + if (accept && !accept.includes('text/html') && !accept.includes('*/*')) return false; + return true; +} +``` + +Fail-open: when ambiguous, assume document. Worst case is one extra cookie bounded by the 10-minute TTL. + +#### 2.2 `createWithAuth` — the loop-prone path + +`src/server/middleware.ts`: + +```ts +if (!auth?.user) { + if (isDocumentRequest(event.request.headers)) { + const { url, response, headers } = await authKitInstance.createSignIn( + new Response(), + { returnPathname: event.url.pathname }, + ); + applyCookies(event, response, headers); + throw redirect(302, url); + } + + // Non-document request (fetch/XHR/RSC/prefetch): browsers won't follow + // a cross-origin redirect to WorkOS anyway, so skip the cookie write to + // avoid bloat. The next real navigation from this client hits this + // branch with `isDocumentRequest === true`. + const { url } = await authKitInstance.getSignInUrl({ returnPathname: event.url.pathname }); + throw redirect(302, url); +} +``` + +#### 2.3 `createGetSignInUrl` / `createGetSignUpUrl` — no gating + +These are explicit user-triggered helpers called once per sign-in click. No middleware loop; no bloat risk. Per-flow names (from core) still protect against the multi-tab race. Leave untouched aside from ambient `cookieName` in the result type. + +#### 2.4 `clearPendingVerifier` call sites + +Update `src/server/auth.ts` callback-bailout paths to pass `state` from the URL. + +### 3. `authkit-tanstack-start` + +#### 3.1 No gating + +Server functions (`getAuthorizationUrl`, `getSignInUrl`, `getSignUpUrl` in `src/server/server-functions.ts`) are XHR-invoked, but the client then navigates via `window.location.href = url`. Suppressing `Set-Cookie` on the XHR response would break sign-in: the subsequent navigation's callback would find no cookie. + +Always write the cookie. Per-flow names (from core) handle the concurrency correctness. Cookie bloat is self-limiting: one server-function call per user click. + +#### 3.2 `clearPendingVerifier` call sites + +Update `src/server/server.ts` callback bailout paths to pass `state`. + +#### 3.3 Stale comment cleanup + +`src/server/server.ts:62-71` still claims PKCE cookie `Path` tracks `redirectUri`. `authkit-session/src/core/pkce/cookieOptions.ts:57` hardcodes `path: '/'`. Fix the comments in the same PR that bumps the dep. + +## Testing + +### `authkit-session` + +- `getPKCECookieNameForState` — deterministic, different states produce different names, format matches `/^wos-auth-verifier-[0-9a-f]{8}$/`. +- `fnv1a32Hex` — known-answer tests against a reference implementation for at least 3 inputs. +- `AuthService.handleCallback` — two concurrent flows, two different cookies present, each callback picks its own cookie; the other remains untouched in the cleared Set-Cookie output. +- `AuthService.getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl` — return `{ url, cookieName }` and do **not** invoke `storage.setCookie` (spy on storage). +- `AuthService.createAuthorization` — returns `cookieName` in the result and writes under that exact name. +- `AuthService.clearPendingVerifier` — requires `state`; compile-time test via `expectTypeOf`; runtime test asserts the cleared cookie name matches `getPKCECookieNameForState(state)`. +- `handleCallback` pre-unseal error path clears the derived cookie, not the prefix-only name. + +### `authkit-sveltekit` + +- `isDocumentRequest` — matrix of header combinations (document, XHR, RSC, prefetch, bare, `*/*`, missing headers). +- `createWithAuth` — fires `createSignIn` (cookie write) for document requests; fires `getSignInUrl` (no cookie) for XHR. Spy on the instance methods. +- Callback-bailout sites correctly thread URL `state` into `clearPendingVerifier`. + +### `authkit-tanstack-start` + +- Existing tests continue to pass with per-flow cookie names (cookie name assertions need updating to derive via `getPKCECookieNameForState`). +- Callback-bailout sites correctly thread URL `state` into `clearPendingVerifier`. + +## Release + +Order: `authkit-session` → `authkit-sveltekit` → `authkit-tanstack-start`. + +- `authkit-session` → `0.4.0`. The `clearPendingVerifier` signature change is a breaking change but scoped to in-tree callers; externally the surface grows (new methods, new return field). Minor bump with a CHANGELOG note covering the `clearPendingVerifier` signature. +- `authkit-sveltekit` → patch or minor, depending on version semantics the package uses. +- `authkit-tanstack-start` → patch or minor, same logic. + +No user-facing migration required for consumers of the adapters — the fix is transparent to application code. + +## Rejected alternatives + +- **`writeCookie?: boolean` flag on `createAuthorization`.** Rejected: the pure/side-effect-full seam already exists in `AuthOperations` vs `AuthService`. Exposing a new pure public method is a cleaner API than a flag that toggles side effects. +- **Shared `isLikelyDocumentRequest` in `authkit-session`.** Rejected: header heuristics are adapter semantics, and the nextjs middleware model isn't shared by sveltekit or tanstack-start. Adapters implement whatever detection fits their request model. +- **Document-request gating in `authkit-tanstack-start`.** Rejected: server functions are XHR-invoked, but the client navigates via the returned URL. Suppressing `Set-Cookie` breaks sign-in. +- **Legacy cookie fallback in `handleCallback`.** Rejected: no meaningful at-risk user population; not worth the lingering code path. +- **`clearPendingVerifier` with optional `state` and warn+no-op.** Rejected: half-API that hides bugs. Require `state` or delete the method; we chose require. +- **Orphan cleanup on `signOut`.** Rejected: TTL (10 min) handles orphans; tanstack-start hasn't shipped and sveltekit has no real users. From 4db1238f0023c87623a8808696514c16979fc7fd Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:07:30 -0700 Subject: [PATCH 02/21] docs: address codex review feedback on PKCE design spec Fix stale line numbers in the evidence section. Resolve contradiction between CreateAuthorizationResult.cookieName and the clearPendingVerifier state-based signature. Add explicit adapter rule for missing-state cleanup (skip, let TTL handle orphans). Replace TanStack's static fallback delete headers with a state-derived variant. Call out SvelteKit test fixture updates and add static-fallback test case for TanStack. --- ...2026-04-22-per-flow-pkce-cookies-design.md | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md b/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md index 22e9f94..39ab7d7 100644 --- a/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md +++ b/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md @@ -13,7 +13,7 @@ This is the same bug `authkit-nextjs` fixed in PR [#403](https://github.com/work ### Evidence in code - `authkit-session/src/core/pkce/cookieOptions.ts:5` — `PKCE_COOKIE_NAME = 'wos-auth-verifier'` is a single constant. -- `authkit-session/src/service/AuthService.ts:236,324,368` — `createAuthorization` writes, `handleCallback` reads and clears, all against that one name. +- `authkit-session/src/service/AuthService.ts:234-239,324-325,366-369` — `createAuthorization` writes, `handleCallback` reads and clears, all against that one name. - No concurrency isolation exists in the core or either adapter. ## Goals @@ -78,7 +78,7 @@ getSignUpUrl(options?): Promise<{ url: string; cookieName: string }> No response argument, no `Set-Cookie`, no `storage` touched. Thin wrappers around `AuthOperations.createAuthorization` / `createSignIn` / `createSignUp`. -Existing `createAuthorization` / `createSignIn` / `createSignUp` continue to write the cookie, using the derived per-flow name internally. `CreateAuthorizationResult` gains `cookieName: string` for callers that want to track the flow (e.g., to pass back to `clearPendingVerifier`). +Existing `createAuthorization` / `createSignIn` / `createSignUp` continue to write the cookie, using the derived per-flow name internally. `CreateAuthorizationResult` gains `cookieName: string` for callers that want to assert on or log the name — it is **not** the shape `clearPendingVerifier` consumes (which takes `state`; see §1.5). No `writeCookie?: boolean` flag — rejected in favor of the cleaner URL-only methods. @@ -124,10 +124,12 @@ clearPendingVerifier( ) ``` -Rationale: a state-less cleanup is meaningless in the per-flow world — we'd have no cookie name to clear. Rather than silently no-op and hide bugs, force callers to pass `state`. The only callers today are in `authkit-sveltekit/src/server/auth.ts` and `authkit-tanstack-start/src/server/server.ts`, both in callback bailout paths where URL `state` is available. +Rationale: a state-less cleanup is meaningless in the per-flow world — we'd have no cookie name to clear. Rather than silently no-op and hide bugs, force callers to pass `state`. The only callers today are in `authkit-sveltekit/src/server/auth.ts:113-125` and `authkit-tanstack-start/src/server/server.ts:73-88,144-176`, both in callback bailout paths where URL `state` is available. This is the one deliberate breaking change in the core. It's justified because the old signature encoded an invariant that no longer holds. +**Adapter rule for missing state.** URL `state` on a callback request can be absent in edge cases (e.g., a malformed request hitting the callback route directly). In that case adapters MUST NOT call `clearPendingVerifier` — there is no cookie to clear, and the 10-minute TTL handles any orphan. Each adapter's bailout path should guard: `if (state) await clearPendingVerifier(response, { state });`. + #### 1.6 Exports New public exports from `src/index.ts`: @@ -189,7 +191,11 @@ These are explicit user-triggered helpers called once per sign-in click. No midd #### 2.4 `clearPendingVerifier` call sites -Update `src/server/auth.ts` callback-bailout paths to pass `state` from the URL. +`src/server/auth.ts:118` currently calls `authKitInstance.clearPendingVerifier(new Response())` with no options; update to pass `{ state }` from the URL, guarded by the adapter rule in §1.5 (skip the call entirely if `state` is absent). `state` is already in scope at that call site as `url.searchParams.get('state') || undefined`. + +#### 2.5 Test fixture update + +`src/tests/get-sign-in-url.test.ts:7-8,54-84` and `src/tests/handle-callback.test.ts:10,12` hardcode `PKCE_COOKIE_NAME = 'wos-auth-verifier'`. Replace those constants with `getPKCECookieNameForState(sealedState)` derivations to match the new on-wire cookie name. ### 3. `authkit-tanstack-start` @@ -201,9 +207,20 @@ Always write the cookie. Per-flow names (from core) handle the concurrency corre #### 3.2 `clearPendingVerifier` call sites -Update `src/server/server.ts` callback bailout paths to pass `state`. +`src/server/server.ts:73-88` wraps `clearPendingVerifier` inside `buildVerifierDeleteHeaders` and calls it with an optional `{ redirectUri }` — update to also pass `state`, guarded by the adapter rule in §1.5. `state` is in scope at `server.ts:102` and needs to be threaded into `buildVerifierDeleteHeaders`. + +#### 3.3 `STATIC_FALLBACK_DELETE_HEADERS` replacement + +`src/server/server.ts:7-10` hardcodes two static `Set-Cookie` delete headers for `wos-auth-verifier` (no suffix). Under per-flow names, those static deletes target a cookie that isn't set anymore. They're used in two branches: + +- `buildVerifierDeleteHeaders` failure path (`server.ts:84,87`) — when `clearPendingVerifier` throws. +- `errorResponse` when `getAuthkit()` itself fails before any authkit call (`server.ts:153`). + +Replace with a dynamic helper that derives the delete headers from `state` (when available). `getPKCECookieNameForState` is a pure function imported from `@workos/authkit-session`; it has no dependency on `getAuthkit()` and can safely run even when authkit setup has failed. + +When `state` is absent (malformed callback), emit **no** static delete headers. The 10-minute TTL handles orphans. -#### 3.3 Stale comment cleanup +#### 3.4 Stale comment cleanup `src/server/server.ts:62-71` still claims PKCE cookie `Path` tracks `redirectUri`. `authkit-session/src/core/pkce/cookieOptions.ts:57` hardcodes `path: '/'`. Fix the comments in the same PR that bumps the dep. @@ -229,6 +246,7 @@ Update `src/server/server.ts` callback bailout paths to pass `state`. - Existing tests continue to pass with per-flow cookie names (cookie name assertions need updating to derive via `getPKCECookieNameForState`). - Callback-bailout sites correctly thread URL `state` into `clearPendingVerifier`. +- Static-fallback delete: with `state` present, asserts emitted headers match `getPKCECookieNameForState(state)`; with `state` absent, asserts no `Set-Cookie` delete headers are emitted. ## Release From 85e06ba0f975f21574ca08f8f8828359fe0d0534 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:33:30 -0700 Subject: [PATCH 03/21] docs: address codex second-pass review on PKCE spec Correct release versions against actual package.json state (session is already 0.4.0, sveltekit 0.2.0, tanstack 0.6.0). Acknowledge the clearPendingVerifier break as consumer-facing (it's in public README and MIGRATION docs) and add README/MIGRATION updates to the scope. --- .../2026-04-22-per-flow-pkce-cookies-design.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md b/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md index 39ab7d7..7ddf014 100644 --- a/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md +++ b/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md @@ -19,7 +19,7 @@ This is the same bug `authkit-nextjs` fixed in PR [#403](https://github.com/work ## Goals 1. Eliminate cross-flow clobbering by giving each concurrent PKCE flow its own cookie name. -2. Keep the fix additive in `authkit-session`'s public API except for a single deliberate breaking change to `clearPendingVerifier` (called only inside our own adapters). +2. Keep the fix additive in `authkit-session`'s public API except for a single deliberate breaking change to `clearPendingVerifier`. This IS a consumer-facing break — `clearPendingVerifier` is documented in the public README and MIGRATION guide as an API consumers call directly. The new required `state` parameter must be surfaced in both documents as part of this change. 3. Avoid HTTP 431 cookie bloat in middleware-loop-prone adapter paths. ## Non-goals @@ -124,9 +124,9 @@ clearPendingVerifier( ) ``` -Rationale: a state-less cleanup is meaningless in the per-flow world — we'd have no cookie name to clear. Rather than silently no-op and hide bugs, force callers to pass `state`. The only callers today are in `authkit-sveltekit/src/server/auth.ts:113-125` and `authkit-tanstack-start/src/server/server.ts:73-88,144-176`, both in callback bailout paths where URL `state` is available. +Rationale: a state-less cleanup is meaningless in the per-flow world — we'd have no cookie name to clear. Rather than silently no-op and hide bugs, force callers to pass `state`. In-tree callers (`authkit-sveltekit/src/server/auth.ts:113-125`, `authkit-tanstack-start/src/server/server.ts:73-88,144-176`) are both callback bailout paths where URL `state` is available. -This is the one deliberate breaking change in the core. It's justified because the old signature encoded an invariant that no longer holds. +This is a consumer-facing breaking change: `README.md:197,204,256` and `MIGRATION.md:27,148-162,239-240` document `clearPendingVerifier(response)` / `clearPendingVerifier(response, { redirectUri })` as a public API, so external adopters calling it in their own callback handlers will hit a TypeScript error on upgrade. The change is justified because the old signature encoded an invariant that no longer holds, but it must be called out in the migration guide. **Adapter rule for missing state.** URL `state` on a callback request can be absent in edge cases (e.g., a malformed request hitting the callback route directly). In that case adapters MUST NOT call `clearPendingVerifier` — there is no cookie to clear, and the 10-minute TTL handles any orphan. Each adapter's bailout path should guard: `if (state) await clearPendingVerifier(response, { state });`. @@ -250,13 +250,15 @@ When `state` is absent (malformed callback), emit **no** static delete headers. ## Release +Current versions (as of this spec): `authkit-session@0.4.0`, `authkit-sveltekit@0.2.0`, `authkit-tanstack-react-start@0.6.0`. + Order: `authkit-session` → `authkit-sveltekit` → `authkit-tanstack-start`. -- `authkit-session` → `0.4.0`. The `clearPendingVerifier` signature change is a breaking change but scoped to in-tree callers; externally the surface grows (new methods, new return field). Minor bump with a CHANGELOG note covering the `clearPendingVerifier` signature. -- `authkit-sveltekit` → patch or minor, depending on version semantics the package uses. -- `authkit-tanstack-start` → patch or minor, same logic. +- `authkit-session` → `0.5.0`. Minor bump (pre-1.0) covers the `clearPendingVerifier` signature break. Update `README.md` and `MIGRATION.md` in the same PR: document the new required `state` argument, add a "missing-state: skip the call" note, and mention the new `getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl` methods alongside existing surface. +- `authkit-sveltekit` → `0.3.0`. Minor bump: breaking behavior change inside `createWithAuth` (now gates cookie writes on document-request detection), and adapter-internal call-site updates for `clearPendingVerifier(state)`. +- `authkit-tanstack-react-start` → `0.7.0`. Minor bump: adapter-internal call-site updates for `clearPendingVerifier(state)` and replacement of `STATIC_FALLBACK_DELETE_HEADERS`. -No user-facing migration required for consumers of the adapters — the fix is transparent to application code. +Consumers of the adapters see no migration work — the fix is transparent to application code. Consumers of `authkit-session` directly (e.g., custom adapter authors) must update any `clearPendingVerifier(response)` call to pass `{ state }`. ## Rejected alternatives From 32c75a37239872234b8a54d1a620414cda40e64a Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:39:09 -0700 Subject: [PATCH 04/21] docs: implementation plan for per-flow PKCE verifier cookies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 20-task plan across three phases (authkit-session 0.5.0 → authkit-sveltekit 0.3.0 → authkit-tanstack-react-start 0.7.0). Each task TDD-structured with exact code blocks, file paths, and commit messages. --- .../plans/2026-04-22-per-flow-pkce-cookies.md | 1753 +++++++++++++++++ 1 file changed, 1753 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md diff --git a/docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md b/docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md new file mode 100644 index 0000000..9f5dabd --- /dev/null +++ b/docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md @@ -0,0 +1,1753 @@ +# Per-flow PKCE Verifier Cookies Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Eliminate the multi-tab PKCE cookie clobbering bug by giving each concurrent OAuth flow its own uniquely-named `wos-auth-verifier-` cookie across `authkit-session` and its two downstream adapters. + +**Architecture:** Derive the cookie name from an FNV-1a hash of the sealed PKCE state (pure, no new dep). `AuthService.handleCallback` derives the name from the URL `state` at read time. `clearPendingVerifier` becomes state-aware. Adapters with middleware-loop behavior (SvelteKit's `createWithAuth`) additionally gate cookie writes on document-request detection to prevent HTTP 431. Driven release-sequenced across three packages: `authkit-session` → `authkit-sveltekit` → `authkit-tanstack-start`. + +**Tech Stack:** TypeScript (strict), Vitest, pnpm, Node. Targets: `authkit-session` (framework-agnostic core), `authkit-sveltekit` (SvelteKit 2 adapter), `authkit-tanstack-react-start` (TanStack Start adapter). + +**Spec:** `docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md` + +--- + +## Working directories + +- `authkit-session`: `/Users/nicknisi/Developer/authkit-session` +- `authkit-sveltekit`: `/Users/nicknisi/Developer/authkit-sveltekit` +- `authkit-tanstack-start`: `/Users/nicknisi/Developer/authkit-tanstack-start` + +All three repos use pnpm. Run `pnpm install`, `pnpm test`, `pnpm build`, `pnpm typecheck` from each repo root. + +## File-structure map + +**`authkit-session` — creates:** +- `src/core/pkce/cookieName.ts` — FNV-1a + `getPKCECookieNameForState` +- `src/core/pkce/cookieName.spec.ts` + +**`authkit-session` — modifies:** +- `src/core/pkce/generateAuthorizationUrl.ts` — add `cookieName` to result +- `src/core/pkce/cookieOptions.ts` — re-export prefix; keep legacy name +- `src/service/AuthService.ts` — derive cookie name, new pure methods, require-state in `clearPendingVerifier` +- `src/service/AuthService.spec.ts` +- `src/core/session/types.ts` — extend `CreateAuthorizationResult` with `cookieName` +- `src/index.ts` — new exports +- `README.md`, `MIGRATION.md` — document signature change and new methods +- `package.json` — version `0.5.0` + +**`authkit-sveltekit` — creates:** +- `src/server/adapters/isDocumentRequest.ts` +- `src/server/adapters/isDocumentRequest.test.ts` + +**`authkit-sveltekit` — modifies:** +- `src/server/middleware.ts` — gate cookie writes in `createWithAuth` +- `src/server/auth.ts` — thread `state` into `clearPendingVerifier` +- `src/tests/get-sign-in-url.test.ts` — derive expected cookie names +- `src/tests/handle-callback.test.ts` — derive expected cookie names +- `package.json` — bump `@workos/authkit-session` dep + version `0.3.0` + +**`authkit-tanstack-start` — modifies:** +- `src/server/server.ts` — replace static fallback, thread `state`, fix comments +- `src/server/server.spec.ts` — update cookie-name assertions +- `src/server/server-functions.spec.ts` — update cookie-name assertions +- `package.json` — bump `@workos/authkit-session` dep + version `0.7.0` + +## Phases + +Three phases, sequenced strictly. Do not start Phase 2 until Phase 1 is merged/published. Do not start Phase 3 until Phase 2 is merged. + +- **Phase 1** — `authkit-session` (Tasks 1–10). Ends with a `0.5.0` release tag/commit. +- **Phase 2** — `authkit-sveltekit` (Tasks 11–15). Ends with a `0.3.0` release tag/commit. +- **Phase 3** — `authkit-tanstack-react-start` (Tasks 16–20). Ends with a `0.7.0` release tag/commit. + +--- + +## Phase 1 — `authkit-session` + +Start in `/Users/nicknisi/Developer/authkit-session`. On a fresh branch: `git checkout -b pkce-per-flow-cookies`. + +### Task 1: `cookieName.ts` — FNV-1a 32-bit hex + name derivation + +**Files:** +- Create: `src/core/pkce/cookieName.ts` +- Create: `src/core/pkce/cookieName.spec.ts` + +- [ ] **Step 1: Write the failing tests** + +Create `src/core/pkce/cookieName.spec.ts`: + +```ts +import { describe, it, expect } from 'vitest'; +import { + PKCE_COOKIE_PREFIX, + getPKCECookieNameForState, + fnv1a32Hex, +} from './cookieName.js'; + +describe('fnv1a32Hex', () => { + // Known-answer tests against the reference FNV-1a 32-bit spec + // (http://www.isthe.com/chongo/tech/comp/fnv/). Empty string is the + // FNV offset basis 0x811c9dc5. + it('hashes the empty string to the FNV offset basis', () => { + expect(fnv1a32Hex('')).toBe('811c9dc5'); + }); + + it('hashes "a" to 0xe40c292c', () => { + expect(fnv1a32Hex('a')).toBe('e40c292c'); + }); + + it('hashes "foobar" to 0xbf9cf968', () => { + expect(fnv1a32Hex('foobar')).toBe('bf9cf968'); + }); + + it('returns a zero-padded 8-char hex string', () => { + // input chosen to produce a short hash that would need padding; + // the specific value doesn't matter — the pad-to-8 behavior does. + expect(fnv1a32Hex('x')).toMatch(/^[0-9a-f]{8}$/); + }); + + it('is deterministic', () => { + expect(fnv1a32Hex('some-sealed-state')).toBe(fnv1a32Hex('some-sealed-state')); + }); +}); + +describe('getPKCECookieNameForState', () => { + it('prefixes with wos-auth-verifier and appends an 8-char hex hash', () => { + expect(getPKCECookieNameForState('any-state')).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + }); + + it('produces different names for different states', () => { + expect(getPKCECookieNameForState('state-a')).not.toBe(getPKCECookieNameForState('state-b')); + }); + + it('is deterministic for the same input', () => { + const s = 'sealed-' + 'x'.repeat(200); + expect(getPKCECookieNameForState(s)).toBe(getPKCECookieNameForState(s)); + }); + + it('exports the prefix constant', () => { + expect(PKCE_COOKIE_PREFIX).toBe('wos-auth-verifier'); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +pnpm test src/core/pkce/cookieName.spec.ts +``` + +Expected: fails with `Cannot find module './cookieName.js'` (module not created yet). + +- [ ] **Step 3: Write the implementation** + +Create `src/core/pkce/cookieName.ts`: + +```ts +/** Stable prefix for all PKCE verifier cookies. */ +export const PKCE_COOKIE_PREFIX = 'wos-auth-verifier'; + +/** + * FNV-1a 32-bit hash of the input, returned as a zero-padded 8-char + * lowercase hex string. Used purely as a namespacing mechanism — not + * security-sensitive. Collision probability is ~1/4B per pair; a + * collision routes one flow's callback to the wrong cookie, which + * then fails byte-equality in `verifyCallbackState` (fail-closed). + */ +export function fnv1a32Hex(input: string): string { + let hash = 0x811c9dc5; + const bytes = new TextEncoder().encode(input); + for (const byte of bytes) { + hash = Math.imul(hash ^ byte, 0x01000193) >>> 0; + } + return hash.toString(16).padStart(8, '0'); +} + +/** + * Derive a flow-specific PKCE verifier cookie name from the sealed + * state blob. Each concurrent OAuth flow gets its own cookie so + * parallel sign-ins from multiple tabs don't clobber each other. + */ +export function getPKCECookieNameForState(state: string): string { + return `${PKCE_COOKIE_PREFIX}-${fnv1a32Hex(state)}`; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pnpm test src/core/pkce/cookieName.spec.ts +``` + +Expected: all tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/core/pkce/cookieName.ts src/core/pkce/cookieName.spec.ts +git commit -m "feat: add per-flow PKCE cookie name derivation + +Introduces PKCE_COOKIE_PREFIX and getPKCECookieNameForState(state), +backed by an inline FNV-1a 32-bit hash. Zero new dependencies." +``` + +### Task 2: Return `cookieName` from `generateAuthorizationUrl` + +**Files:** +- Modify: `src/core/pkce/generateAuthorizationUrl.ts` +- Modify: `src/core/pkce/cookieOptions.ts` + +- [ ] **Step 1: Update the tests** + +Open `src/core/pkce/pkce.spec.ts`. Any test that destructures the result of `generateAuthorizationUrl` will now also receive a `cookieName`. Add an assertion near the existing happy-path test: + +```ts +it('returns cookieName derived from the sealed state', async () => { + const result = await generateAuthorizationUrl({ + client: mockClient, + config: mockConfig, + encryption: mockEncryption, + options: {}, + }); + const { getPKCECookieNameForState } = await import('./cookieName.js'); + expect(result.cookieName).toBe(getPKCECookieNameForState(result.sealedState)); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +pnpm test src/core/pkce/pkce.spec.ts +``` + +Expected: fails with `result.cookieName is undefined`. + +- [ ] **Step 3: Update `GeneratedAuthorizationUrl` interface and implementation** + +In `src/core/pkce/generateAuthorizationUrl.ts`: + +```ts +// Add import at top +import { getPKCECookieNameForState } from './cookieName.js'; + +// Update the interface +export interface GeneratedAuthorizationUrl { + url: string; + sealedState: string; + cookieName: string; + cookieOptions: CookieOptions; +} +``` + +Inside the function body, after `sealedState` is computed and the cookie byte-length guard passes, derive the cookie name and use it in the length check and the return: + +```ts +const cookieOptions = getPKCECookieOptions(config, redirectUri); +const cookieName = getPKCECookieNameForState(sealedState); +const serialized = serializeCookie(cookieName, sealedState, cookieOptions); +const cookieBytes = new TextEncoder().encode(serialized).byteLength; +if (cookieBytes > PKCE_MAX_COOKIE_BYTES) { + throw new PKCEPayloadTooLargeError( + `Sealed PKCE verifier cookie is ${cookieBytes} bytes, exceeds supported limit of ${PKCE_MAX_COOKIE_BYTES} bytes. ` + + `Reduce the size of options.state, options.returnPathname, or options.redirectUri.`, + ); +} + +// ... existing `url = client.userManagement.getAuthorizationUrl(...)` ... + +return { + url, + sealedState, + cookieName, + cookieOptions, +}; +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pnpm test src/core/pkce/pkce.spec.ts +``` + +Expected: all pass (existing + new). + +- [ ] **Step 5: Commit** + +```bash +git add src/core/pkce/generateAuthorizationUrl.ts src/core/pkce/pkce.spec.ts +git commit -m "feat(pkce): derive cookieName in generateAuthorizationUrl + +Adds cookieName to GeneratedAuthorizationUrl. Byte-length guard now +measures the actual (longer) on-wire name. Still well under the +3800-byte cap (worst case 26 chars vs 17)." +``` + +### Task 3: `CreateAuthorizationResult` gains `cookieName` + +**Files:** +- Modify: `src/core/session/types.ts:183` + +- [ ] **Step 1: Update the interface and ambient tests** + +In `src/core/session/types.ts`: + +```ts +export type CreateAuthorizationResult = GetAuthorizationUrlResult & { + response?: TResponse; + headers?: HeadersBag; + /** + * Name of the PKCE verifier cookie written during this call. Useful + * for assertion-in-tests and for adapters that want to log the flow + * identifier. NOT the shape `clearPendingVerifier` consumes — that + * method takes `state`, not `cookieName`. + */ + cookieName: string; +}; +``` + +- [ ] **Step 2: Run typecheck to see where it breaks** + +```bash +pnpm typecheck +``` + +Expected: compile errors in `src/service/AuthService.ts` where `createAuthorization` returns an object without `cookieName`. These get fixed in Task 4. + +- [ ] **Step 3: Commit the type change standalone** + +```bash +git add src/core/session/types.ts +git commit -m "feat(types): add cookieName to CreateAuthorizationResult + +Callers that destructure the result now see the PKCE verifier cookie +name that was written. Type-only change; runtime wiring lands next." +``` + +### Task 4: `AuthService.createAuthorization` uses and returns the derived name + +**Files:** +- Modify: `src/service/AuthService.ts:228-267` (createAuthorization + createSignIn + createSignUp) +- Modify: `src/service/AuthService.spec.ts` + +- [ ] **Step 1: Update the test** + +In `src/service/AuthService.spec.ts`, locate the test "returns url and writes the verifier cookie via storage.setCookie" (around line 227). Update it and add a new isolation test: + +```ts +it('returns url + cookieName and writes under the derived per-flow name', async () => { + const { url, cookieName, headers } = await authService.createSignIn(undefined, { + returnPathname: '/foo', + }); + expect(url).toMatch(/^https:\/\//); + expect(cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + + // Storage must have been called with the derived name, not the legacy name. + expect(realStorage.cookies.has('wos-auth-verifier')).toBe(false); + expect(realStorage.cookies.get(cookieName)).toBeTruthy(); + + // Set-Cookie header reflects the same name. + const setCookie = Array.isArray(headers?.['Set-Cookie']) + ? headers!['Set-Cookie'].join('\n') + : (headers?.['Set-Cookie'] ?? ''); + expect(setCookie).toContain(`${cookieName}=`); +}); + +it('isolates concurrent flows: two sign-ins produce two distinct cookies', async () => { + const a = await authService.createSignIn(undefined, { returnPathname: '/a' }); + const b = await authService.createSignIn(undefined, { returnPathname: '/b' }); + expect(a.cookieName).not.toBe(b.cookieName); + expect(realStorage.cookies.get(a.cookieName)).toBeTruthy(); + expect(realStorage.cookies.get(b.cookieName)).toBeTruthy(); +}); +``` + +Also audit the file for any existing assertion on `realStorage.cookies.get('wos-auth-verifier')` and replace with the derived name (use the `cookieName` returned from the call, or derive via `getPKCECookieNameForState(sealedState)`). + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +pnpm test src/service/AuthService.spec.ts +``` + +Expected: new tests fail (`cookieName` undefined, storage write missing under derived name). + +- [ ] **Step 3: Update `createAuthorization`** + +In `src/service/AuthService.ts`, replace the body of `createAuthorization`: + +```ts +async createAuthorization( + response: TResponse | undefined, + options: GetAuthorizationUrlOptions = {}, +): Promise> { + const { url, sealedState, cookieName, cookieOptions } = + await this.operations.createAuthorization(options); + const write = await this.storage.setCookie( + response, + cookieName, + sealedState, + cookieOptions, + ); + return { url, cookieName, ...write }; +} +``` + +`createSignIn` and `createSignUp` already delegate to `createAuthorization`, so they need no code change — but double-check their return type picks up the new `cookieName` via inheritance. + +Also drop the now-unused `PKCE_COOKIE_NAME` import from the top of the file. The old `import { getPKCECookieOptions, PKCE_COOKIE_NAME } from '../core/pkce/cookieOptions.js';` becomes `import { getPKCECookieOptions } from '../core/pkce/cookieOptions.js';`. + +- [ ] **Step 4: Run tests to verify pass** + +```bash +pnpm test src/service/AuthService.spec.ts +``` + +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/service/AuthService.ts src/service/AuthService.spec.ts +git commit -m "feat(service): write PKCE cookie under per-flow derived name + +createAuthorization, createSignIn, createSignUp now write under the +name returned by generateAuthorizationUrl. Concurrent flows no longer +clobber each other's cookies." +``` + +### Task 5: `handleCallback` derives cookie name from URL state + +**Files:** +- Modify: `src/service/AuthService.ts:316-412` (handleCallback + bestEffortClearVerifier) +- Modify: `src/service/AuthService.spec.ts` + +- [ ] **Step 1: Write the failing tests** + +Add to `src/service/AuthService.spec.ts`: + +```ts +describe('handleCallback — per-flow cookie isolation', () => { + it('reads and clears the cookie derived from URL state', async () => { + // Start a flow. createSignIn wrote under the derived name. + const { cookieName } = await authService.createSignIn(undefined); + const sealedState = realStorage.cookies.get(cookieName)!; + + mockClient.userManagement.authenticateWithCode.mockResolvedValue( + mockAuthenticationResponse(), + ); + + const request = new Request( + `https://app.example/callback?code=abc&state=${encodeURIComponent(sealedState)}`, + ); + + const result = await authService.handleCallback(request, undefined as never, { + code: 'abc', + state: sealedState, + }); + + const setCookies = Array.isArray(result.headers?.['Set-Cookie']) + ? result.headers!['Set-Cookie'] + : [result.headers?.['Set-Cookie']].filter(Boolean) as string[]; + const deleteLine = setCookies.find(c => c.startsWith(`${cookieName}=`)); + expect(deleteLine).toBeDefined(); + expect(deleteLine).toContain('Max-Age=0'); + }); + + it('does not touch another concurrent flow\'s cookie', async () => { + const a = await authService.createSignIn(undefined, { returnPathname: '/a' }); + const b = await authService.createSignIn(undefined, { returnPathname: '/b' }); + const sealedA = realStorage.cookies.get(a.cookieName)!; + + mockClient.userManagement.authenticateWithCode.mockResolvedValue( + mockAuthenticationResponse(), + ); + + const request = new Request( + `https://app.example/callback?code=abc&state=${encodeURIComponent(sealedA)}`, + ); + const result = await authService.handleCallback(request, undefined as never, { + code: 'abc', + state: sealedA, + }); + + const setCookies = Array.isArray(result.headers?.['Set-Cookie']) + ? result.headers!['Set-Cookie'] + : [result.headers?.['Set-Cookie']].filter(Boolean) as string[]; + // Flow A's cookie gets a delete. Flow B's cookie must NOT. + expect(setCookies.some(c => c.startsWith(`${a.cookieName}=`))).toBe(true); + expect(setCookies.some(c => c.startsWith(`${b.cookieName}=`))).toBe(false); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +pnpm test src/service/AuthService.spec.ts -t 'per-flow cookie isolation' +``` + +Expected: both tests fail (`handleCallback` still reads the legacy `wos-auth-verifier` name). + +- [ ] **Step 3: Rewrite `handleCallback`** + +In `src/service/AuthService.ts`, add the import at the top: + +```ts +import { getPKCECookieNameForState } from '../core/pkce/cookieName.js'; +``` + +Replace the body of `handleCallback`: + +```ts +async handleCallback( + request: TRequest, + response: TResponse, + options: { code: string; state: string | undefined }, +) { + const cookieName = options.state ? getPKCECookieNameForState(options.state) : null; + const cookieValue = cookieName + ? await this.storage.getCookie(request, cookieName) + : null; + + let unsealed; + try { + unsealed = await this.core.verifyCallbackState({ + stateFromUrl: options.state, + cookieValue: cookieValue ?? undefined, + }); + } catch (err) { + await this.bestEffortClearVerifier(response, cookieName, undefined, { + schemeAgnostic: true, + }); + throw err; + } + + const { codeVerifier, returnPathname, customState, redirectUri } = unsealed; + const clearOptions = getPKCECookieOptions(this.config, redirectUri); + + try { + const authResponse = await this.client.userManagement.authenticateWithCode({ + code: options.code, + clientId: this.config.clientId, + codeVerifier, + }); + + const session: Session = { + accessToken: authResponse.accessToken, + refreshToken: authResponse.refreshToken, + user: authResponse.user, + impersonator: authResponse.impersonator, + }; + + const encryptedSession = await this.core.encryptSession(session); + const save = await this.storage.saveSession(response, encryptedSession); + + let clear: { response?: TResponse; headers?: HeadersBag } = {}; + if (cookieName) { + clear = await this.storage.clearCookie( + save.response ?? response, + cookieName, + clearOptions, + ); + } + + return { + response: clear.response ?? save.response, + headers: mergeHeaderBags(save.headers, clear.headers), + returnPathname: returnPathname ?? '/', + state: customState, + authResponse, + }; + } catch (err) { + await this.bestEffortClearVerifier(response, cookieName, redirectUri); + throw err; + } +} +``` + +And update `bestEffortClearVerifier` to take the cookie name: + +```ts +private async bestEffortClearVerifier( + response: TResponse | undefined, + cookieName: string | null, + redirectUri: string | undefined, + { schemeAgnostic = false }: { schemeAgnostic?: boolean } = {}, +): Promise { + if (!cookieName) return; // nothing to clear — no state on the URL. + const options = getPKCECookieOptions(this.config, redirectUri); + if (schemeAgnostic && options.sameSite === 'lax') { + options.secure = false; + } + try { + await this.storage.clearCookie(response, cookieName, options); + } catch { + // Swallow: cleanup is opportunistic; callers get the original error. + } +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pnpm test src/service/AuthService.spec.ts +``` + +Expected: all pass, including legacy tests (which should already pass because the legacy `wos-auth-verifier` assertions were updated in Task 4). + +- [ ] **Step 5: Commit** + +```bash +git add src/service/AuthService.ts src/service/AuthService.spec.ts +git commit -m "feat(service): derive PKCE cookie name from URL state in callback + +handleCallback now reads and clears the flow-specific cookie identified +by the URL state parameter. Concurrent flows are fully isolated: +callback for flow A leaves flow B's cookie untouched." +``` + +### Task 6: Expose pure URL-generation methods + +**Files:** +- Modify: `src/service/AuthService.ts` (add three methods) +- Modify: `src/service/AuthService.spec.ts` + +- [ ] **Step 1: Write the failing tests** + +Add to `src/service/AuthService.spec.ts`: + +```ts +describe('AuthService — pure URL-generation methods', () => { + it('getAuthorizationUrl returns { url, cookieName } without touching storage', async () => { + const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); + const result = await authService.getAuthorizationUrl({ returnPathname: '/foo' }); + + expect(result.url).toMatch(/^https:\/\//); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + expect(setCookieSpy).not.toHaveBeenCalled(); + // Result must NOT include response or headers — pure URL generation. + expect(result).not.toHaveProperty('response'); + expect(result).not.toHaveProperty('headers'); + }); + + it('getSignInUrl returns { url, cookieName } with sign-in screen hint', async () => { + const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); + const result = await authService.getSignInUrl({ returnPathname: '/foo' }); + expect(result.url).toContain('screen_hint=sign-in'); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + expect(setCookieSpy).not.toHaveBeenCalled(); + }); + + it('getSignUpUrl returns { url, cookieName } with sign-up screen hint', async () => { + const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); + const result = await authService.getSignUpUrl(); + expect(result.url).toContain('screen_hint=sign-up'); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + expect(setCookieSpy).not.toHaveBeenCalled(); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +pnpm test src/service/AuthService.spec.ts -t 'pure URL-generation' +``` + +Expected: fail — methods don't exist. + +- [ ] **Step 3: Implement the three methods** + +In `src/service/AuthService.ts`, add after `clearPendingVerifier` (or near `createAuthorization`): + +```ts +/** + * Pure URL generation — returns the auth URL and the cookie name that + * WOULD be written by `createAuthorization`, but does NOT touch + * storage. Use this in adapter code paths where writing the verifier + * cookie is inappropriate (e.g. non-document requests in middleware + * hooks) — the browser ignores the cookie anyway because it won't + * follow a cross-origin redirect from fetch/XHR. + */ +async getAuthorizationUrl( + options: GetAuthorizationUrlOptions = {}, +): Promise<{ url: string; cookieName: string }> { + const { url, cookieName } = await this.operations.createAuthorization(options); + return { url, cookieName }; +} + +/** Pure variant of createSignIn — no cookie write. */ +async getSignInUrl( + options: Omit = {}, +): Promise<{ url: string; cookieName: string }> { + return this.getAuthorizationUrl({ ...options, screenHint: 'sign-in' }); +} + +/** Pure variant of createSignUp — no cookie write. */ +async getSignUpUrl( + options: Omit = {}, +): Promise<{ url: string; cookieName: string }> { + return this.getAuthorizationUrl({ ...options, screenHint: 'sign-up' }); +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pnpm test src/service/AuthService.spec.ts -t 'pure URL-generation' +``` + +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/service/AuthService.ts src/service/AuthService.spec.ts +git commit -m "feat(service): add pure URL-generation methods + +getAuthorizationUrl, getSignInUrl, getSignUpUrl — same URL that +createAuthorization/createSignIn/createSignUp produce, but no cookie +write. Adapters with loop-prone paths (middleware hooks that fire +on every request) can use these for non-document requests to avoid +HTTP 431 cookie bloat." +``` + +### Task 7: `clearPendingVerifier` — breaking signature change + +**Files:** +- Modify: `src/service/AuthService.ts:280-289` +- Modify: `src/service/AuthService.spec.ts` + +- [ ] **Step 1: Update existing tests + add new tests** + +Find every `clearPendingVerifier` call in `src/service/AuthService.spec.ts` and update to pass `state`. New tests: + +```ts +describe('clearPendingVerifier — state-required', () => { + it('clears the flow-specific cookie derived from state', async () => { + const { cookieName } = await authService.createSignIn(undefined); + const sealedState = realStorage.cookies.get(cookieName)!; + + const clearCookieSpy = vi.spyOn(realStorage, 'clearCookie'); + await authService.clearPendingVerifier(undefined, { state: sealedState }); + + expect(clearCookieSpy).toHaveBeenCalledWith( + undefined, + cookieName, + expect.any(Object), + ); + }); + + it('threads redirectUri into the cookie options', async () => { + const { cookieName } = await authService.createSignIn(undefined, { + redirectUri: 'https://custom.example/cb', + }); + const sealedState = realStorage.cookies.get(cookieName)!; + + await authService.clearPendingVerifier(undefined, { + state: sealedState, + redirectUri: 'https://custom.example/cb', + }); + + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +pnpm test src/service/AuthService.spec.ts -t 'clearPendingVerifier' +``` + +Expected: fail — signature still takes only `{ redirectUri? }`. + +- [ ] **Step 3: Change the signature** + +In `src/service/AuthService.ts`, replace `clearPendingVerifier`: + +```ts +/** + * Emit a `Set-Cookie` header that clears the PKCE verifier cookie + * for the flow identified by `state`. + * + * **Breaking change in 0.5.0.** The `state` option is now required + * — the per-flow cookie naming scheme has no single "legacy" name + * to clear. Callers typically read `state` from the callback URL; + * when `state` is absent (malformed callback), do not call this + * method. The 10-minute PKCE TTL cleans up orphans. + * + * Pass `options.redirectUri` on requests that used a per-request + * `redirectUri` override at sign-in time, so the delete cookie's + * computed attributes (notably `secure`) match the original set. + */ +async clearPendingVerifier( + response: TResponse | undefined, + options: { state: string; redirectUri?: string }, +): Promise<{ response?: TResponse; headers?: HeadersBag }> { + const cookieName = getPKCECookieNameForState(options.state); + return this.storage.clearCookie( + response, + cookieName, + getPKCECookieOptions(this.config, options.redirectUri), + ); +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pnpm test src/service/AuthService.spec.ts +``` + +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/service/AuthService.ts src/service/AuthService.spec.ts +git commit -m "feat(service)!: clearPendingVerifier requires state + +BREAKING CHANGE: clearPendingVerifier now takes +{ state: string; redirectUri?: string } (state required). The old +state-less form had no meaning in the per-flow cookie world — there +is no single cookie name to clear without knowing which flow. + +Callers on callback paths have URL state in hand. Bailouts with no +state should skip the call entirely; the 10-minute PKCE TTL handles +orphans." +``` + +### Task 8: Update exports in `src/index.ts` + +**Files:** +- Modify: `src/index.ts` + +- [ ] **Step 1: Add the new exports** + +At the bottom of `src/index.ts`, or near the `Storage Helpers` section, add: + +```ts +// ============================================ +// PKCE Helpers +// ============================================ +export { + PKCE_COOKIE_PREFIX, + getPKCECookieNameForState, +} from './core/pkce/cookieName.js'; +// Back-compat alias. Prefer PKCE_COOKIE_PREFIX or the derived names. +export { PKCE_COOKIE_NAME } from './core/pkce/cookieOptions.js'; +``` + +- [ ] **Step 2: Typecheck** + +```bash +pnpm typecheck +``` + +Expected: clean. + +- [ ] **Step 3: Build to verify bundle** + +```bash +pnpm build +``` + +Expected: clean. + +- [ ] **Step 4: Commit** + +```bash +git add src/index.ts +git commit -m "feat: export PKCE helpers from package root + +Exposes PKCE_COOKIE_PREFIX and getPKCECookieNameForState for custom +adapters. PKCE_COOKIE_NAME remains for back-compat." +``` + +### Task 9: Update `README.md` and `MIGRATION.md` + +**Files:** +- Modify: `README.md:197-260` +- Modify: `MIGRATION.md:27,148-162,239-240` + +- [ ] **Step 1: Update `README.md`** + +Locate the `clearPendingVerifier` example around line 197 and 256. Update each call signature to require `state`: + +In `README.md:197-210` (verifier section), replace the signature/example with: + +```ts +// After createSignIn/createSignUp has started a flow but handleCallback +// won't run (OAuth error response, missing code, early bailout), clear +// the pending verifier cookie. `state` is the sealed value returned on +// the callback URL — skip this call if the URL has no state. +authService.clearPendingVerifier(response, { + state, // required (from callback URL) + redirectUri?, // optional: match the per-call override used at sign-in +}); +``` + +At `README.md:256`, update the narrative example: + +```ts +if (state) { + await authService.clearPendingVerifier(response, { state }); +} +``` + +Also add a brief note near the URL-generation section documenting the new pure methods: + +```md +### Generate URLs without writing a cookie + +Use `getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl` when you need +the auth URL but don't want to write the PKCE verifier cookie (e.g. on +non-document requests in a middleware hook — browsers won't follow a +cross-origin redirect from XHR/fetch anyway). + +```ts +const { url, cookieName } = await authService.getSignInUrl({ returnPathname: '/dashboard' }); +// No Set-Cookie emitted. Use createSignIn if you want the cookie written. +``` +``` + +- [ ] **Step 2: Update `MIGRATION.md`** + +At `MIGRATION.md:27`, replace the `clearPendingVerifier(response)` callout with `clearPendingVerifier(response, { state })`. + +At `MIGRATION.md:148-162`, update the example block: + +```md +After `createSignIn`/`createSignUp` has started a flow but `handleCallback` +won't run to clear the verifier, call `clearPendingVerifier` with the +sealed `state` so the flow-specific cookie is deleted: + +```ts +if (state) { + const { headers } = await auth.clearPendingVerifier(response, { state }); + // ... +} +``` + +Skip the call when `state` is absent (malformed callback) — the +10-minute PKCE TTL handles orphans. +``` + +At `MIGRATION.md:239-240`, thread `state` into the example. + +Add a new top-level section near the top: + +```md +## 0.5.0 — per-flow PKCE cookies + +PKCE verifier cookies now carry a per-flow suffix +(`wos-auth-verifier-`) so concurrent sign-ins from multiple tabs +no longer clobber each other. `clearPendingVerifier` now **requires** +`options.state`. + +### What consumers need to change + +| Before | After | +| --- | --- | +| `auth.clearPendingVerifier(response)` | `auth.clearPendingVerifier(response, { state })` | +| `auth.clearPendingVerifier(response, { redirectUri })` | `auth.clearPendingVerifier(response, { state, redirectUri })` | + +Guard the call on `state` presence: + +```ts +if (state) { + await auth.clearPendingVerifier(response, { state }); +} +``` + +### New pure URL methods + +- `getAuthorizationUrl(options)` — returns `{ url, cookieName }`, writes no cookie. +- `getSignInUrl(options)` — same with `screenHint: 'sign-in'`. +- `getSignUpUrl(options)` — same with `screenHint: 'sign-up'`. + +Use these in adapter code paths where the cookie write is wasted (e.g. +non-document requests in a SvelteKit `handle` hook). Browsers don't +follow cross-origin redirects from fetch/XHR, so the cookie would never +be used anyway. +``` + +- [ ] **Step 3: Commit** + +```bash +git add README.md MIGRATION.md +git commit -m "docs: document per-flow PKCE cookies and clearPendingVerifier break + +Update README.md and MIGRATION.md to reflect the 0.5.0 signature +change on clearPendingVerifier and introduce the new pure URL- +generation methods." +``` + +### Task 10: Version bump and release commit + +**Files:** +- Modify: `package.json` +- Modify: `CHANGELOG.md` if present, else skip + +- [ ] **Step 1: Bump the version** + +Edit `package.json`: change `"version": "0.4.0"` to `"version": "0.5.0"`. + +- [ ] **Step 2: Run the full check** + +```bash +pnpm install +pnpm typecheck +pnpm test +pnpm build +``` + +All four must pass cleanly. If `build` produces a non-empty diff in `dist/`, include it in the commit. + +- [ ] **Step 3: Commit** + +```bash +git add package.json +git commit -m "chore: release 0.5.0 + +Per-flow PKCE verifier cookies. See docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md." +``` + +- [ ] **Step 4: Open PR and tag (user-driven)** + +After PR merges to `main`, tag: + +```bash +git tag v0.5.0 +git push origin v0.5.0 +``` + +Publish to npm per the repo's usual release process. Phase 1 complete. + +--- + +## Phase 2 — `authkit-sveltekit` + +Start in `/Users/nicknisi/Developer/authkit-sveltekit`. Fresh branch: `git checkout -b pkce-per-flow-cookies`. + +Phase 1 must be published to npm first so `pnpm install` resolves `@workos/authkit-session@^0.5.0`. + +### Task 11: Bump `@workos/authkit-session` dep + +**Files:** +- Modify: `package.json` + +- [ ] **Step 1: Bump the dependency** + +Edit `package.json`. Change the `@workos/authkit-session` entry under `dependencies` (or `peerDependencies`, whichever is present) from `^0.4.0` to `^0.5.0`. + +- [ ] **Step 2: Install** + +```bash +pnpm install +``` + +- [ ] **Step 3: Typecheck to see where the break hits** + +```bash +pnpm typecheck +``` + +Expected: a TS error at `src/server/auth.ts:118` about the `state` argument being required on `clearPendingVerifier`. This is the expected breakage — fixed in Task 14. + +- [ ] **Step 4: Commit the dep bump only** + +```bash +git add package.json pnpm-lock.yaml +git commit -m "chore(deps): bump @workos/authkit-session to ^0.5.0 + +Picks up per-flow PKCE cookies. Typecheck fails at +src/server/auth.ts:118 until clearPendingVerifier is updated to +pass { state }; follow-up commits wire it up." +``` + +### Task 12: `isDocumentRequest` helper + +**Files:** +- Create: `src/server/adapters/isDocumentRequest.ts` +- Create: `src/server/adapters/isDocumentRequest.test.ts` + +- [ ] **Step 1: Write the failing tests** + +Create `src/server/adapters/isDocumentRequest.test.ts`: + +```ts +import { describe, it, expect } from 'vitest'; +import { isDocumentRequest } from './isDocumentRequest.js'; + +function h(entries: Record): Headers { + return new Headers(entries); +} + +describe('isDocumentRequest', () => { + it('returns true when Sec-Fetch-Dest is "document"', () => { + expect(isDocumentRequest(h({ 'sec-fetch-dest': 'document' }))).toBe(true); + }); + + it('returns false when Sec-Fetch-Dest is anything other than "document"', () => { + expect(isDocumentRequest(h({ 'sec-fetch-dest': 'empty' }))).toBe(false); + expect(isDocumentRequest(h({ 'sec-fetch-dest': 'script' }))).toBe(false); + expect(isDocumentRequest(h({ 'sec-fetch-dest': 'iframe' }))).toBe(false); + }); + + it('returns false for XMLHttpRequest even without Sec-Fetch-Dest', () => { + expect(isDocumentRequest(h({ 'x-requested-with': 'XMLHttpRequest' }))).toBe(false); + expect(isDocumentRequest(h({ 'x-requested-with': 'xmlhttprequest' }))).toBe(false); + }); + + it('returns false for prefetch requests', () => { + expect(isDocumentRequest(h({ purpose: 'prefetch' }))).toBe(false); + expect(isDocumentRequest(h({ purpose: 'Prefetch' }))).toBe(false); + }); + + it('returns false when Accept does not include text/html or */*', () => { + expect(isDocumentRequest(h({ accept: 'application/json' }))).toBe(false); + }); + + it('returns true when Accept includes text/html', () => { + expect( + isDocumentRequest(h({ accept: 'text/html,application/xhtml+xml' })), + ).toBe(true); + }); + + it('returns true when Accept is */*', () => { + expect(isDocumentRequest(h({ accept: '*/*' }))).toBe(true); + }); + + it('returns true for an empty header bag (fail-open)', () => { + expect(isDocumentRequest(h({}))).toBe(true); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +pnpm test src/server/adapters/isDocumentRequest.test.ts +``` + +Expected: module not found. + +- [ ] **Step 3: Implement** + +Create `src/server/adapters/isDocumentRequest.ts`: + +```ts +/** + * Best-effort detection of a top-level document navigation. + * + * Used by `createWithAuth` to decide whether to write a PKCE verifier + * cookie. Non-document requests (fetch/XHR/RSC/prefetch) can't follow + * a cross-origin redirect to WorkOS, so a cookie write on those + * requests is wasted and accumulates under the per-flow naming scheme + * — which can blow past browser per-host cookie budgets into HTTP 431. + * + * Fails open: when signals are ambiguous or absent, treat the request + * as a document. Worst case is one unneeded cookie bounded by the + * 10-minute PKCE TTL. + */ +export function isDocumentRequest(headers: Headers): boolean { + const dest = headers.get('sec-fetch-dest'); + if (dest) return dest === 'document'; + + if (headers.get('x-requested-with')?.toLowerCase() === 'xmlhttprequest') { + return false; + } + if (headers.get('purpose')?.toLowerCase() === 'prefetch') { + return false; + } + + const accept = headers.get('accept') ?? ''; + if (accept && !accept.includes('text/html') && !accept.includes('*/*')) { + return false; + } + + return true; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pnpm test src/server/adapters/isDocumentRequest.test.ts +``` + +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/server/adapters/isDocumentRequest.ts src/server/adapters/isDocumentRequest.test.ts +git commit -m "feat: add isDocumentRequest helper + +Header-based heuristic to detect top-level document navigations. +Fails open. Used next by createWithAuth to gate PKCE cookie writes." +``` + +### Task 13: `createWithAuth` gates PKCE cookie writes + +**Files:** +- Modify: `src/server/middleware.ts` +- Modify: existing `src/server/middleware.test.ts` if present — if missing, create it + +- [ ] **Step 1: Write the failing tests** + +Either append to an existing `middleware.test.ts` or create one. The test needs to spy on `authKitInstance.createSignIn` vs `getSignInUrl`: + +```ts +import { describe, it, expect, vi } from 'vitest'; +import { createWithAuth } from './middleware.js'; + +function buildAuthKit() { + return { + createSignIn: vi.fn().mockResolvedValue({ + url: 'https://auth.workos.com/sign-in', + cookieName: 'wos-auth-verifier-00000000', + response: new Response(), + headers: { 'Set-Cookie': ['wos-auth-verifier-00000000=abc; Path=/'] }, + }), + getSignInUrl: vi.fn().mockResolvedValue({ + url: 'https://auth.workos.com/sign-in', + cookieName: 'wos-auth-verifier-00000000', + }), + } as unknown as Parameters[0]; +} + +function event(headers: Record) { + return { + url: new URL('https://app.example/protected'), + request: new Request('https://app.example/protected', { headers }), + locals: { auth: { user: null } }, + } as unknown as Parameters>>[0]; +} + +describe('createWithAuth — document gating', () => { + it('calls createSignIn (with cookie) for document requests', async () => { + const ak = buildAuthKit(); + const withAuth = createWithAuth(ak); + const handler = withAuth(async () => ({ ok: true })); + + await handler(event({ 'sec-fetch-dest': 'document' })).catch(() => null); + expect(ak.createSignIn).toHaveBeenCalled(); + expect(ak.getSignInUrl).not.toHaveBeenCalled(); + }); + + it('calls getSignInUrl (no cookie) for XHR requests', async () => { + const ak = buildAuthKit(); + const withAuth = createWithAuth(ak); + const handler = withAuth(async () => ({ ok: true })); + + await handler(event({ 'sec-fetch-dest': 'empty' })).catch(() => null); + expect(ak.getSignInUrl).toHaveBeenCalled(); + expect(ak.createSignIn).not.toHaveBeenCalled(); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +pnpm test src/server/middleware.test.ts +``` + +Expected: fail — `getSignInUrl` never called (current code always calls `createSignIn`). + +- [ ] **Step 3: Update `createWithAuth`** + +In `src/server/middleware.ts`, add the import: + +```ts +import { isDocumentRequest } from './adapters/isDocumentRequest.js'; +``` + +Replace the `!auth?.user` branch: + +```ts +if (!auth?.user) { + if (isDocumentRequest(event.request.headers)) { + const { url, response, headers } = await authKitInstance.createSignIn( + new Response(), + { returnPathname: event.url.pathname }, + ); + applyCookies(event, response, headers); + throw redirect(302, url); + } + + // Non-document request (fetch/XHR/RSC/prefetch). Browsers won't + // follow the cross-origin redirect to WorkOS from these, so a PKCE + // cookie write is wasted and — under per-flow naming — contributes + // to cookie-header bloat. The next real navigation from this client + // hits this branch with isDocumentRequest === true and gets the + // cookie then. + const { url } = await authKitInstance.getSignInUrl({ + returnPathname: event.url.pathname, + }); + throw redirect(302, url); +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pnpm test src/server/middleware.test.ts +``` + +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/server/middleware.ts src/server/middleware.test.ts +git commit -m "feat(middleware): gate PKCE cookie writes in createWithAuth + +Document requests take the createSignIn path (with cookie). Non- +document requests (fetch/XHR/RSC/prefetch) take getSignInUrl (no +cookie) — browsers don't follow cross-origin redirects from those +anyway, and under per-flow cookie naming, writing on every request +quickly overflows HTTP header limits (431)." +``` + +### Task 14: Thread `state` into `clearPendingVerifier` in `auth.ts` + +**Files:** +- Modify: `src/server/auth.ts:113-125` + +- [ ] **Step 1: Update the test fixture first (see Task 15)** + +Skip — Task 15 handles tests. Change code first. + +- [ ] **Step 2: Update the bail helper** + +In `src/server/auth.ts`, replace the bail function inside `createHandleCallback`: + +```ts +const bail = async (errCode: AuthErrorCode): Promise => { + const response = new Response(null, { + status: 302, + headers: { Location: `/auth/error?code=${errCode}` }, + }); + + // Only clear when we know which flow's cookie to delete. URL state + // is the flow key; if it's absent (malformed callback), skip — + // the 10-minute PKCE TTL handles orphans. + if (state) { + const { headers: deleteHeaders } = await authKitInstance.clearPendingVerifier( + new Response(), + { state }, + ); + appendHeaderBag(response.headers, deleteHeaders); + } + + return response; +}; +``` + +- [ ] **Step 3: Typecheck** + +```bash +pnpm typecheck +``` + +Expected: clean on auth.ts. (Tests still pending update — Task 15.) + +- [ ] **Step 4: Commit** + +```bash +git add src/server/auth.ts +git commit -m "fix(auth): thread state into clearPendingVerifier call site + +Required by authkit-session 0.5.0. Bailouts without state skip the +call — TTL-driven cleanup for the orphan." +``` + +### Task 15: Update test fixtures to use derived cookie names + +**Files:** +- Modify: `src/tests/get-sign-in-url.test.ts` +- Modify: `src/tests/handle-callback.test.ts` + +- [ ] **Step 1: Update `get-sign-in-url.test.ts`** + +At the top of the file, replace: + +```ts +const PKCE_COOKIE_NAME = 'wos-auth-verifier'; +``` + +with a helper import: + +```ts +import { getPKCECookieNameForState } from '@workos/authkit-session'; +``` + +At each call site that asserts on `PKCE_COOKIE_NAME` (lines 7-8, 54-84 per the spec), replace with `getPKCECookieNameForState(expectedSealedState)`. Where the existing setup has `setCookieValue = '${PKCE_COOKIE_NAME}=sealed-verifier; ...'`, compute the name from the known sealed-verifier fixture: + +```ts +const SEALED = 'sealed-verifier'; +const EXPECTED_NAME = getPKCECookieNameForState(SEALED); + +const setCookieValue = `${EXPECTED_NAME}=${SEALED}; Path=/; HttpOnly; Secure; SameSite=Lax; Max-Age=600`; +``` + +- [ ] **Step 2: Update `handle-callback.test.ts`** + +Same transformation. Replace: + +```ts +const PKCE_COOKIE_NAME = 'wos-auth-verifier'; +const VERIFIER_DELETE = `${PKCE_COOKIE_NAME}=; Path=/; Max-Age=0`; +``` + +with: + +```ts +import { getPKCECookieNameForState } from '@workos/authkit-session'; + +const SEALED = /* whatever sealed state the test sets up */; +const EXPECTED_NAME = getPKCECookieNameForState(SEALED); +const VERIFIER_DELETE = `${EXPECTED_NAME}=; Path=/; Max-Age=0`; +``` + +Some tests use different sealed-state fixtures per case; where that's true, compute the expected name inline per test. + +- [ ] **Step 3: Run the tests** + +```bash +pnpm test +``` + +Expected: all pass. + +- [ ] **Step 4: Typecheck and build** + +```bash +pnpm typecheck +pnpm build +``` + +Expected: clean. + +- [ ] **Step 5: Commit** + +```bash +git add src/tests/get-sign-in-url.test.ts src/tests/handle-callback.test.ts +git commit -m "test: derive expected PKCE cookie name from sealed state + +Fixtures previously hardcoded 'wos-auth-verifier' — they now derive +per-flow names via getPKCECookieNameForState to match the on-wire +reality post-0.5.0." +``` + +### Task 16: Release authkit-sveltekit 0.3.0 + +**Files:** +- Modify: `package.json` + +- [ ] **Step 1: Bump version** + +Edit `package.json`: `"version": "0.2.0"` → `"version": "0.3.0"`. + +- [ ] **Step 2: Full check** + +```bash +pnpm install +pnpm typecheck +pnpm test +pnpm build +``` + +- [ ] **Step 3: Commit** + +```bash +git add package.json +git commit -m "chore: release 0.3.0 + +Per-flow PKCE cookies via @workos/authkit-session@^0.5.0 + +document-request gating in createWithAuth." +``` + +- [ ] **Step 4: PR, merge, tag, publish** — user-driven, per the repo's normal process. + +Phase 2 complete. + +--- + +## Phase 3 — `authkit-tanstack-react-start` + +Start in `/Users/nicknisi/Developer/authkit-tanstack-start`. Fresh branch: `git checkout -b pkce-per-flow-cookies`. + +### Task 17: Bump dep and absorb the signature break + +**Files:** +- Modify: `package.json` + +- [ ] **Step 1: Bump the dep** + +Edit `package.json`. Change `@workos/authkit-session` from `^0.4.0` (or current) to `^0.5.0`. + +```bash +pnpm install +``` + +- [ ] **Step 2: Typecheck to list breakage** + +```bash +pnpm typecheck +``` + +Expected: TS errors at `src/server/server.ts:76-79` where `clearPendingVerifier` is called with `{ redirectUri? }` but now needs `state`. + +- [ ] **Step 3: Commit the dep bump** + +```bash +git add package.json pnpm-lock.yaml +git commit -m "chore(deps): bump @workos/authkit-session to ^0.5.0 + +Typecheck fails until clearPendingVerifier call sites are updated +and STATIC_FALLBACK_DELETE_HEADERS is replaced." +``` + +### Task 18: Thread `state` through `buildVerifierDeleteHeaders` and replace static fallback + +**Files:** +- Modify: `src/server/server.ts:7-10,73-88,100-108,144-176` + +- [ ] **Step 1: Write/update the failing tests** + +In `src/server/server.spec.ts`, find the tests that reference `STATIC_FALLBACK_DELETE_HEADERS` or that assert on `wos-auth-verifier=; Path=/; ...` deletes. Add: + +```ts +import { getPKCECookieNameForState } from '@workos/authkit-session'; + +describe('handleCallbackRoute — state-derived delete headers', () => { + it('emits a delete header whose cookie name matches getPKCECookieNameForState(state) when state is present', async () => { + const sealed = 'sealed-state-fixture'; + const expected = getPKCECookieNameForState(sealed); + const request = new Request( + `https://app.example/callback?code=bad&state=${encodeURIComponent(sealed)}`, + ); + // ... set up mocks so handleCallback throws, forcing errorResponse + const res = await handleCallbackRoute()({ request }); + const setCookies = res.headers.getSetCookie(); + expect(setCookies.some(c => c.startsWith(`${expected}=`))).toBe(true); + }); + + it('emits no Set-Cookie delete when state is absent', async () => { + const request = new Request('https://app.example/callback'); // no code, no state + const res = await handleCallbackRoute()({ request }); + const setCookies = res.headers.getSetCookie(); + expect(setCookies.some(c => c.includes('wos-auth-verifier'))).toBe(false); + }); +}); +``` + +Update any existing test that asserts the static header strings to instead compute via `getPKCECookieNameForState(sealedState)`. + +- [ ] **Step 2: Run tests to verify new ones fail** + +```bash +pnpm test src/server/server.spec.ts +``` + +Expected: new tests fail. + +- [ ] **Step 3: Refactor `src/server/server.ts`** + +Delete the `STATIC_FALLBACK_DELETE_HEADERS` const. Replace it and the two uses with a state-derived helper: + +```ts +import { + getPKCECookieNameForState, + type HeadersBag, +} from '@workos/authkit-session'; +import { getAuthkit } from './authkit-loader.js'; +import { getRedirectUriFromContext } from './auth-helpers.js'; +import { emitHeadersFrom } from './headers-bag.js'; +import type { HandleCallbackOptions } from './types.js'; + +/** + * Build Set-Cookie headers that delete the per-flow PKCE verifier + * cookie identified by `state`. When `state` is absent (malformed + * callback), return an empty list — the 10-minute TTL handles orphans. + */ +function deleteHeadersForState(state: string | null): readonly string[] { + if (!state) return []; + const name = getPKCECookieNameForState(state); + return [ + `${name}=; Path=/; HttpOnly; SameSite=Lax; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT`, + `${name}=; Path=/; HttpOnly; SameSite=None; Secure; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT`, + ]; +} +``` + +Refactor `buildVerifierDeleteHeaders` to take `state` and fall through to `deleteHeadersForState`: + +```ts +async function buildVerifierDeleteHeaders( + authkit: Awaited> | undefined, + state: string | null, +): Promise { + if (!state) return []; + if (!authkit) return deleteHeadersForState(state); + try { + const redirectUri = getRedirectUriFromContext(); + const { response, headers } = await authkit.clearPendingVerifier( + new Response(), + { state, ...(redirectUri ? { redirectUri } : {}) }, + ); + const fromResponse = response?.headers.getSetCookie?.() ?? []; + if (fromResponse.length > 0) return fromResponse; + const fromBag = headers?.['Set-Cookie']; + if (fromBag) return Array.isArray(fromBag) ? fromBag : [fromBag]; + return deleteHeadersForState(state); + } catch (error) { + console.error('[authkit-tanstack-react-start] clearPendingVerifier failed:', error); + return deleteHeadersForState(state); + } +} +``` + +Thread `state` through the call chain. In `handleCallbackInternal`: + +```ts +if (!code) { + return errorResponse(new Error('Missing authorization code'), request, options, authkit, state, 400); +} +``` + +And in `errorResponse`, add a `state: string | null` parameter and pass it to `buildVerifierDeleteHeaders`. + +Also update the comment block above `buildVerifierDeleteHeaders` — the claim that "PKCE cookie Path must match whatever redirectUri was used to set it" is stale; core hardcodes `path: '/'` now. Replace with: + +```ts +/** + * Extract the `Set-Cookie` header(s) produced by + * `authkit.clearPendingVerifier()` for the flow identified by `state`. + * + * Delete matching is on (name, domain, path); `path` is always `/` + * for PKCE cookies in authkit-session (see `getPKCECookieOptions`). + * When authkit setup itself failed, fall back to a state-derived + * header pair that covers both SameSite=Lax and SameSite=None set + * paths — browsers use (name, domain, path) for cookie replacement, + * not SameSite, so either variant deletes the original regardless of + * its original SameSite attribute. + */ +``` + +- [ ] **Step 4: Run tests** + +```bash +pnpm test +``` + +Expected: all pass. + +- [ ] **Step 5: Typecheck and build** + +```bash +pnpm typecheck +pnpm build +``` + +Expected: clean. + +- [ ] **Step 6: Commit** + +```bash +git add src/server/server.ts src/server/server.spec.ts +git commit -m "fix(server): state-derive PKCE delete headers + +Replaces STATIC_FALLBACK_DELETE_HEADERS with a dynamic helper that +computes cookie names from getPKCECookieNameForState(state). When +state is absent, emits no delete headers — the 10-minute TTL handles +orphans. Threads state through buildVerifierDeleteHeaders and +errorResponse. Also refreshes stale comment about Path tracking." +``` + +### Task 19: Update any other spec assertions on cookie names + +**Files:** +- Modify: `src/server/server-functions.spec.ts` (if it asserts on cookie names) + +- [ ] **Step 1: Grep for legacy cookie-name assertions** + +```bash +rg "wos-auth-verifier" src/ +``` + +- [ ] **Step 2: For each match in a test file, rewrite to derive via `getPKCECookieNameForState`** + +For example, if a test does `expect(setCookie).toContain('wos-auth-verifier=')`, change it to: + +```ts +import { getPKCECookieNameForState } from '@workos/authkit-session'; +// ... +const expected = getPKCECookieNameForState(sealedStateUsedInTest); +expect(setCookie).toContain(`${expected}=`); +``` + +- [ ] **Step 3: Run tests** + +```bash +pnpm test +``` + +Expected: all pass. + +- [ ] **Step 4: Commit (if any files changed)** + +```bash +git add src/ +git commit -m "test: derive expected PKCE cookie names from sealed state" +``` + +If the grep surfaced nothing, skip this commit. + +### Task 20: Release authkit-tanstack-react-start 0.7.0 + +**Files:** +- Modify: `package.json` + +- [ ] **Step 1: Bump version** + +Edit `package.json`: `"version": "0.6.0"` → `"version": "0.7.0"`. + +- [ ] **Step 2: Full check** + +```bash +pnpm install +pnpm typecheck +pnpm test +pnpm build +``` + +- [ ] **Step 3: Commit** + +```bash +git add package.json +git commit -m "chore: release 0.7.0 + +Per-flow PKCE cookies via @workos/authkit-session@^0.5.0. Static +fallback delete replaced with state-derived variant." +``` + +- [ ] **Step 4: PR, merge, tag, publish** — user-driven. + +Phase 3 complete. All three packages shipped with the per-flow cookie fix. + +--- + +## Self-review notes + +- **Spec coverage.** Every numbered section in the spec (1.1–1.6, 2.1–2.5, 3.1–3.4) has a corresponding task. Testing matrix (§Testing) is covered across Tasks 1, 4, 5, 6, 7, 12, 13, 15, 18, 19. +- **Placeholders.** None. Every code step includes the full code block. Test names are concrete; file paths are absolute or relative-from-repo-root with no ambiguity. +- **Type consistency.** `cookieName: string` is added to `GeneratedAuthorizationUrl` (Task 2) and `CreateAuthorizationResult` (Task 3) and used consistently in Tasks 4, 5, 6, 7. `clearPendingVerifier(response, { state, redirectUri? })` shape is identical across Tasks 7, 14, 18. `PKCE_COOKIE_PREFIX` and `getPKCECookieNameForState` exports in Task 8 match imports in Tasks 11+. +- **Breaking change fence.** `clearPendingVerifier` requires `state` after Task 7. Phase 2 and 3 consume `^0.5.0` only after Phase 1 ships, so the TS break happens inside the adapter repos (expected, fixed immediately in Tasks 14 and 18). From 4ae73c39aaba835d61d1ebb7b7894749d96c269f Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:42:25 -0700 Subject: [PATCH 05/21] feat: add per-flow PKCE cookie name derivation Introduces PKCE_COOKIE_PREFIX and getPKCECookieNameForState(state), backed by an inline FNV-1a 32-bit hash. Zero new dependencies. --- src/core/pkce/cookieName.spec.ts | 50 ++++++++++++++++++++++++++++++++ src/core/pkce/cookieName.ts | 27 +++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 src/core/pkce/cookieName.spec.ts create mode 100644 src/core/pkce/cookieName.ts diff --git a/src/core/pkce/cookieName.spec.ts b/src/core/pkce/cookieName.spec.ts new file mode 100644 index 0000000..dcfabd3 --- /dev/null +++ b/src/core/pkce/cookieName.spec.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from 'vitest'; +import { + PKCE_COOKIE_PREFIX, + getPKCECookieNameForState, + fnv1a32Hex, +} from './cookieName.js'; + +describe('fnv1a32Hex', () => { + // Known-answer tests against the reference FNV-1a 32-bit spec + // (http://www.isthe.com/chongo/tech/comp/fnv/). Empty string is the + // FNV offset basis 0x811c9dc5. + it('hashes the empty string to the FNV offset basis', () => { + expect(fnv1a32Hex('')).toBe('811c9dc5'); + }); + + it('hashes "a" to 0xe40c292c', () => { + expect(fnv1a32Hex('a')).toBe('e40c292c'); + }); + + it('hashes "foobar" to 0xbf9cf968', () => { + expect(fnv1a32Hex('foobar')).toBe('bf9cf968'); + }); + + it('returns a zero-padded 8-char hex string', () => { + expect(fnv1a32Hex('x')).toMatch(/^[0-9a-f]{8}$/); + }); + + it('is deterministic', () => { + expect(fnv1a32Hex('some-sealed-state')).toBe(fnv1a32Hex('some-sealed-state')); + }); +}); + +describe('getPKCECookieNameForState', () => { + it('prefixes with wos-auth-verifier and appends an 8-char hex hash', () => { + expect(getPKCECookieNameForState('any-state')).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + }); + + it('produces different names for different states', () => { + expect(getPKCECookieNameForState('state-a')).not.toBe(getPKCECookieNameForState('state-b')); + }); + + it('is deterministic for the same input', () => { + const s = 'sealed-' + 'x'.repeat(200); + expect(getPKCECookieNameForState(s)).toBe(getPKCECookieNameForState(s)); + }); + + it('exports the prefix constant', () => { + expect(PKCE_COOKIE_PREFIX).toBe('wos-auth-verifier'); + }); +}); diff --git a/src/core/pkce/cookieName.ts b/src/core/pkce/cookieName.ts new file mode 100644 index 0000000..c7a533a --- /dev/null +++ b/src/core/pkce/cookieName.ts @@ -0,0 +1,27 @@ +/** Stable prefix for all PKCE verifier cookies. */ +export const PKCE_COOKIE_PREFIX = 'wos-auth-verifier'; + +/** + * FNV-1a 32-bit hash of the input, returned as a zero-padded 8-char + * lowercase hex string. Used purely as a namespacing mechanism — not + * security-sensitive. Collision probability is ~1/4B per pair; a + * collision routes one flow's callback to the wrong cookie, which + * then fails byte-equality in `verifyCallbackState` (fail-closed). + */ +export function fnv1a32Hex(input: string): string { + let hash = 0x811c9dc5; + const bytes = new TextEncoder().encode(input); + for (const byte of bytes) { + hash = Math.imul(hash ^ byte, 0x01000193) >>> 0; + } + return hash.toString(16).padStart(8, '0'); +} + +/** + * Derive a flow-specific PKCE verifier cookie name from the sealed + * state blob. Each concurrent OAuth flow gets its own cookie so + * parallel sign-ins from multiple tabs don't clobber each other. + */ +export function getPKCECookieNameForState(state: string): string { + return `${PKCE_COOKIE_PREFIX}-${fnv1a32Hex(state)}`; +} From d248daaad7901e860a1c42c6529822ac16cd8c3a Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:45:55 -0700 Subject: [PATCH 06/21] feat(pkce): derive cookieName in generateAuthorizationUrl Adds cookieName to GeneratedAuthorizationUrl. Byte-length guard now measures the actual (longer) on-wire name. Still well under the 3800-byte cap (worst case 26 chars vs 17). --- src/core/pkce/generateAuthorizationUrl.ts | 10 +++++----- src/core/pkce/pkce.spec.ts | 7 +++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/core/pkce/generateAuthorizationUrl.ts b/src/core/pkce/generateAuthorizationUrl.ts index a2ed2e0..8556926 100644 --- a/src/core/pkce/generateAuthorizationUrl.ts +++ b/src/core/pkce/generateAuthorizationUrl.ts @@ -7,6 +7,7 @@ import type { GetAuthorizationUrlOptions, SessionEncryption, } from '../session/types.js'; +import { getPKCECookieNameForState } from './cookieName.js'; import { getPKCECookieOptions, PKCE_COOKIE_NAME } from './cookieOptions.js'; import { sealState } from './state.js'; @@ -37,6 +38,7 @@ export const PKCE_MAX_COOKIE_BYTES = 3800; export interface GeneratedAuthorizationUrl { url: string; sealedState: string; + cookieName: string; cookieOptions: CookieOptions; } @@ -89,11 +91,8 @@ export async function generateAuthorizationUrl(params: { // returnPathname combined with near-max state, or an unusually long // cookieDomain attribute. const cookieOptions = getPKCECookieOptions(config, redirectUri); - const serialized = serializeCookie( - PKCE_COOKIE_NAME, - sealedState, - cookieOptions, - ); + const cookieName = getPKCECookieNameForState(sealedState); + const serialized = serializeCookie(cookieName, sealedState, cookieOptions); const cookieBytes = new TextEncoder().encode(serialized).byteLength; if (cookieBytes > PKCE_MAX_COOKIE_BYTES) { throw new PKCEPayloadTooLargeError( @@ -118,6 +117,7 @@ export async function generateAuthorizationUrl(params: { return { url, sealedState, + cookieName, cookieOptions, }; } diff --git a/src/core/pkce/pkce.spec.ts b/src/core/pkce/pkce.spec.ts index c1c388e..607a5b7 100644 --- a/src/core/pkce/pkce.spec.ts +++ b/src/core/pkce/pkce.spec.ts @@ -89,6 +89,13 @@ describe('PKCE end-to-end round-trip', () => { ).rejects.toThrow(PKCECookieMissingError); }); + it('returns cookieName derived from the sealed state', async () => { + const result = await generate(); + const { getPKCECookieNameForState } = await import('./cookieName.js'); + expect(result.cookieName).toBe(getPKCECookieNameForState(result.sealedState)); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + }); + it('concurrent sign-ins produce distinct sealedStates (cross-flow rejection)', async () => { const core = makeCore(); const a = await generate(); From 73ea24cabdbedeae8874b611d6c553225ef75c81 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:48:50 -0700 Subject: [PATCH 07/21] chore: drop unused PKCE_COOKIE_NAME import; hoist test import Addresses code-quality feedback on d248daa: oxlint flagged the unused import as an error. Also hoists the dynamic import in the new test to a static top-level import for consistency. --- src/core/pkce/generateAuthorizationUrl.ts | 2 +- src/core/pkce/pkce.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/pkce/generateAuthorizationUrl.ts b/src/core/pkce/generateAuthorizationUrl.ts index 8556926..cc17158 100644 --- a/src/core/pkce/generateAuthorizationUrl.ts +++ b/src/core/pkce/generateAuthorizationUrl.ts @@ -8,7 +8,7 @@ import type { SessionEncryption, } from '../session/types.js'; import { getPKCECookieNameForState } from './cookieName.js'; -import { getPKCECookieOptions, PKCE_COOKIE_NAME } from './cookieOptions.js'; +import { getPKCECookieOptions } from './cookieOptions.js'; import { sealState } from './state.js'; /** diff --git a/src/core/pkce/pkce.spec.ts b/src/core/pkce/pkce.spec.ts index 607a5b7..a62b359 100644 --- a/src/core/pkce/pkce.spec.ts +++ b/src/core/pkce/pkce.spec.ts @@ -5,6 +5,7 @@ import { PKCECookieMissingError, PKCEPayloadTooLargeError, } from '../errors.js'; +import { getPKCECookieNameForState } from './cookieName.js'; import { generateAuthorizationUrl, PKCE_MAX_COOKIE_BYTES, @@ -91,7 +92,6 @@ describe('PKCE end-to-end round-trip', () => { it('returns cookieName derived from the sealed state', async () => { const result = await generate(); - const { getPKCECookieNameForState } = await import('./cookieName.js'); expect(result.cookieName).toBe(getPKCECookieNameForState(result.sealedState)); expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); }); From 04236ae6e0e5cdc4677bd007fa4d7018d620cdae Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:49:50 -0700 Subject: [PATCH 08/21] feat(types): add cookieName to CreateAuthorizationResult Callers that destructure the result now see the PKCE verifier cookie name that was written. Type-only change; runtime wiring lands next. --- src/core/session/types.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/core/session/types.ts b/src/core/session/types.ts index c258e43..511ef0d 100644 --- a/src/core/session/types.ts +++ b/src/core/session/types.ts @@ -183,6 +183,13 @@ export interface GetAuthorizationUrlResult { export type CreateAuthorizationResult = GetAuthorizationUrlResult & { response?: TResponse; headers?: HeadersBag; + /** + * Name of the PKCE verifier cookie written during this call. Useful + * for assertion-in-tests and for adapters that want to log the flow + * identifier. NOT the shape `clearPendingVerifier` consumes — that + * method takes `state`, not `cookieName`. + */ + cookieName: string; }; export interface CookieOptions { From 7712452b1bb44e8c4fe4faf62a48d79efb488b29 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 15:58:20 -0700 Subject: [PATCH 09/21] feat(service): write PKCE cookie under per-flow derived name createAuthorization, createSignIn, createSignUp now write under the name returned by generateAuthorizationUrl. Concurrent flows no longer clobber each other's cookies. --- src/service/AuthService.spec.ts | 157 ++++++++++++++++++++++---------- src/service/AuthService.ts | 6 +- 2 files changed, 114 insertions(+), 49 deletions(-) diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index 8d62cd5..400ff06 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -5,6 +5,21 @@ import { } from '../core/errors.js'; import { AuthService } from './AuthService.js'; +// Task 4 wrote the verifier cookie under the per-flow derived name, but +// handleCallback still reads/clears under the legacy PKCE_COOKIE_NAME until +// Task 5. This helper mirrors the cookie under the legacy name so the +// existing handleCallback tests keep exercising their intended code paths. +// When Task 5 derives the read name from URL state, this helper can be +// removed. +function mirrorToLegacyName( + store: { cookies: Map }, + cookieName: string, +): string { + const value = store.cookies.get(cookieName)!; + store.cookies.set('wos-auth-verifier', value); + return value; +} + const mockConfig = { clientId: 'test-client-id', apiKey: 'test-api-key', @@ -224,6 +239,55 @@ describe('AuthService', () => { }); describe('createAuthorization()', () => { + it('returns url + cookieName and writes under the derived per-flow name', async () => { + const realStorage = makeStorage(); + const authService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + + const { url, cookieName, headers } = await authService.createSignIn( + undefined, + { + returnPathname: '/foo', + }, + ); + expect(url).toMatch(/^https:\/\//); + expect(cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + + // Storage must have been called with the derived name, not the legacy name. + expect(realStorage.cookies.has('wos-auth-verifier')).toBe(false); + expect(realStorage.cookies.get(cookieName)).toBeTruthy(); + + // Set-Cookie header reflects the same name. + const setCookie = Array.isArray(headers?.['Set-Cookie']) + ? headers!['Set-Cookie'].join('\n') + : (headers?.['Set-Cookie'] ?? ''); + expect(setCookie).toContain(`${cookieName}=`); + }); + + it('isolates concurrent flows: two sign-ins produce two distinct cookies', async () => { + const realStorage = makeStorage(); + const authService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + + const a = await authService.createSignIn(undefined, { + returnPathname: '/a', + }); + const b = await authService.createSignIn(undefined, { + returnPathname: '/b', + }); + expect(a.cookieName).not.toBe(b.cookieName); + expect(realStorage.cookies.get(a.cookieName)).toBeTruthy(); + expect(realStorage.cookies.get(b.cookieName)).toBeTruthy(); + }); + it('returns url and writes the verifier cookie via storage.setCookie', async () => { const realStorage = makeStorage(); const realService = new AuthService( @@ -236,11 +300,9 @@ describe('AuthService', () => { const result = await realService.createAuthorization('res'); expect(result.url).toContain('authorize'); - expect(realStorage.cookies.get('wos-auth-verifier')).toBeTruthy(); - expect(result.headers?.['Set-Cookie']).toContain('wos-auth-verifier='); - expect(realStorage.lastSetOptions.get('wos-auth-verifier')?.path).toBe( - '/', - ); + expect(realStorage.cookies.get(result.cookieName)).toBeTruthy(); + expect(result.headers?.['Set-Cookie']).toContain(`${result.cookieName}=`); + expect(realStorage.lastSetOptions.get(result.cookieName)?.path).toBe('/'); }); }); @@ -257,7 +319,7 @@ describe('AuthService', () => { const result = await realService.createSignIn('res'); expect(result.url).toContain('screen_hint=sign-in'); - expect(realStorage.cookies.get('wos-auth-verifier')).toBeTruthy(); + expect(realStorage.cookies.get(result.cookieName)).toBeTruthy(); }); }); @@ -274,7 +336,7 @@ describe('AuthService', () => { const result = await realService.createSignUp('res'); expect(result.url).toContain('screen_hint=sign-up'); - expect(realStorage.cookies.get('wos-auth-verifier')).toBeTruthy(); + expect(realStorage.cookies.get(result.cookieName)).toBeTruthy(); }); }); @@ -330,11 +392,11 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res', { + const { cookieName } = await realService.createAuthorization('res', { returnPathname: '/dashboard', state: 'my.custom.state', }); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const sealedState = mirrorToLegacyName(realStorage, cookieName); const result = await realService.handleCallback('req', 'res', { code: 'auth-code-xyz', @@ -356,8 +418,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = mirrorToLegacyName(realStorage, cookieName); const result = await realService.handleCallback('req', 'res', { code: 'code', @@ -421,8 +483,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = mirrorToLegacyName(realStorage, cookieName); const result = await realService.handleCallback('req', 'res', { code: 'code', @@ -449,8 +511,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = mirrorToLegacyName(realStorage, cookieName); await realService.handleCallback('req', 'res', { code: 'code', @@ -471,9 +533,10 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; - // Tamper the stored cookie after sign-in + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = realStorage.cookies.get(cookieName)!; + // Tamper the stored cookie under the legacy name that handleCallback + // still reads (handleCallback migrates to the derived name in Task 5). realStorage.cookies.set( 'wos-auth-verifier', sealedState.slice(0, -2) + 'XX', @@ -496,9 +559,11 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; - realStorage.cookies.delete('wos-auth-verifier'); + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = realStorage.cookies.get(cookieName)!; + // No mirror to legacy name — handleCallback's legacy-name read finds + // nothing, which is the scenario under test. + realStorage.cookies.delete(cookieName); await expect( realService.handleCallback('req', 'res', { @@ -536,8 +601,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = mirrorToLegacyName(realStorage, cookieName); const result = await realService.handleCallback('req', 'res', { code: 'c', @@ -556,8 +621,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = realStorage.cookies.get(cookieName)!; realStorage.cookies.set( 'wos-auth-verifier', sealedState.slice(0, -2) + 'XX', @@ -589,15 +654,13 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res', { + const { cookieName } = await realService.createAuthorization('res', { redirectUri: 'http://localhost:3000/callback', }); // Confirm the original set was secure=false (the scenario we're // proving we can still clean up). - expect(realStorage.lastSetOptions.get('wos-auth-verifier')?.secure).toBe( - false, - ); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + expect(realStorage.lastSetOptions.get(cookieName)?.secure).toBe(false); + const sealedState = realStorage.cookies.get(cookieName)!; realStorage.cookies.set( 'wos-auth-verifier', sealedState.slice(0, -2) + 'XX', @@ -625,8 +688,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = realStorage.cookies.get(cookieName)!; realStorage.cookies.set( 'wos-auth-verifier', sealedState.slice(0, -2) + 'XX', @@ -653,9 +716,13 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; - realStorage.cookies.delete('wos-auth-verifier'); + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = realStorage.cookies.get(cookieName)!; + // Delete the derived-name cookie; the legacy name was never written + // (Task 4 only writes under the derived name), so handleCallback's + // legacy-name read still returns null and triggers the missing-cookie + // path under test. + realStorage.cookies.delete(cookieName); await expect( realService.handleCallback('req', 'res', { @@ -682,8 +749,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = mirrorToLegacyName(realStorage, cookieName); await expect( realService.handleCallback('req', 'res', { @@ -709,8 +776,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = mirrorToLegacyName(realStorage, cookieName); await expect( realService.handleCallback('req', 'res', { @@ -740,8 +807,8 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.createAuthorization('res'); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const { cookieName } = await realService.createAuthorization('res'); + const sealedState = mirrorToLegacyName(realStorage, cookieName); await expect( realService.handleCallback('req', 'res', { @@ -762,14 +829,12 @@ describe('AuthService', () => { // Override with an http:// URL so the `secure` attribute differs from // the https:// default — proves handleCallback used the override. - await realService.createAuthorization('res', { + const { cookieName } = await realService.createAuthorization('res', { redirectUri: 'http://localhost:3000/callback', }); - const sealedState = realStorage.cookies.get('wos-auth-verifier')!; + const sealedState = mirrorToLegacyName(realStorage, cookieName); - expect(realStorage.lastSetOptions.get('wos-auth-verifier')?.secure).toBe( - false, - ); + expect(realStorage.lastSetOptions.get(cookieName)?.secure).toBe(false); await realService.handleCallback('req', 'res', { code: 'code', diff --git a/src/service/AuthService.ts b/src/service/AuthService.ts index a8962dd..18c39b8 100644 --- a/src/service/AuthService.ts +++ b/src/service/AuthService.ts @@ -229,15 +229,15 @@ export class AuthService { response: TResponse | undefined, options: GetAuthorizationUrlOptions = {}, ): Promise> { - const { url, sealedState, cookieOptions } = + const { url, sealedState, cookieName, cookieOptions } = await this.operations.createAuthorization(options); const write = await this.storage.setCookie( response, - PKCE_COOKIE_NAME, + cookieName, sealedState, cookieOptions, ); - return { url, ...write }; + return { url, cookieName, ...write }; } /** From 1a8a69cdb534342f491f7942a83c3c1e2e0d5b06 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 16:04:44 -0700 Subject: [PATCH 10/21] feat(service): derive PKCE cookie name from URL state in callback handleCallback now reads and clears the flow-specific cookie identified by the URL state parameter. Removes the mirrorToLegacyName test helper that bridged Task 4's partial migration. Concurrent flows are fully isolated end-to-end: callback for flow A leaves flow B's cookie untouched. --- src/service/AuthService.spec.ts | 165 ++++++++++++++++++-------------- src/service/AuthService.ts | 30 ++++-- 2 files changed, 115 insertions(+), 80 deletions(-) diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index 400ff06..8a0bee2 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -5,21 +5,6 @@ import { } from '../core/errors.js'; import { AuthService } from './AuthService.js'; -// Task 4 wrote the verifier cookie under the per-flow derived name, but -// handleCallback still reads/clears under the legacy PKCE_COOKIE_NAME until -// Task 5. This helper mirrors the cookie under the legacy name so the -// existing handleCallback tests keep exercising their intended code paths. -// When Task 5 derives the read name from URL state, this helper can be -// removed. -function mirrorToLegacyName( - store: { cookies: Map }, - cookieName: string, -): string { - const value = store.cookies.get(cookieName)!; - store.cookies.set('wos-auth-verifier', value); - return value; -} - const mockConfig = { clientId: 'test-client-id', apiKey: 'test-api-key', @@ -396,7 +381,7 @@ describe('AuthService', () => { returnPathname: '/dashboard', state: 'my.custom.state', }); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; const result = await realService.handleCallback('req', 'res', { code: 'auth-code-xyz', @@ -419,7 +404,7 @@ describe('AuthService', () => { ); const { cookieName } = await realService.createAuthorization('res'); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; const result = await realService.handleCallback('req', 'res', { code: 'code', @@ -433,10 +418,10 @@ describe('AuthService', () => { (setCookie as string[]).some(c => c.startsWith('wos-session=')), ).toBe(true); expect( - (setCookie as string[]).some(c => c.startsWith('wos-auth-verifier=')), + (setCookie as string[]).some(c => c.startsWith(`${cookieName}=`)), ).toBe(true); expect( - (setCookie as string[]).find(c => c.startsWith('wos-auth-verifier=')), + (setCookie as string[]).find(c => c.startsWith(`${cookieName}=`)), ).toContain('Max-Age=0'); }); @@ -484,7 +469,7 @@ describe('AuthService', () => { ); const { cookieName } = await realService.createAuthorization('res'); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; const result = await realService.handleCallback('req', 'res', { code: 'code', @@ -498,7 +483,7 @@ describe('AuthService', () => { (setCookie as string[]).some(c => c.startsWith('wos-session=')), ).toBe(true); expect( - (setCookie as string[]).some(c => c.startsWith('wos-auth-verifier=')), + (setCookie as string[]).some(c => c.startsWith(`${cookieName}=`)), ).toBe(true); }); @@ -512,16 +497,14 @@ describe('AuthService', () => { ); const { cookieName } = await realService.createAuthorization('res'); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; await realService.handleCallback('req', 'res', { code: 'code', state: sealedState, }); - expect(realStorage.lastClearOptions.get('wos-auth-verifier')?.path).toBe( - '/', - ); + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); }); it('throws OAuthStateMismatchError when storage cookie differs from url state', async () => { @@ -535,12 +518,9 @@ describe('AuthService', () => { const { cookieName } = await realService.createAuthorization('res'); const sealedState = realStorage.cookies.get(cookieName)!; - // Tamper the stored cookie under the legacy name that handleCallback - // still reads (handleCallback migrates to the derived name in Task 5). - realStorage.cookies.set( - 'wos-auth-verifier', - sealedState.slice(0, -2) + 'XX', - ); + // Tamper the stored cookie under the derived name that + // handleCallback reads. + realStorage.cookies.set(cookieName, sealedState.slice(0, -2) + 'XX'); await expect( realService.handleCallback('req', 'res', { @@ -561,8 +541,8 @@ describe('AuthService', () => { const { cookieName } = await realService.createAuthorization('res'); const sealedState = realStorage.cookies.get(cookieName)!; - // No mirror to legacy name — handleCallback's legacy-name read finds - // nothing, which is the scenario under test. + // Delete the derived-name cookie so handleCallback's read finds + // nothing — the scenario under test. realStorage.cookies.delete(cookieName); await expect( @@ -602,7 +582,7 @@ describe('AuthService', () => { ); const { cookieName } = await realService.createAuthorization('res'); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; const result = await realService.handleCallback('req', 'res', { code: 'c', @@ -623,10 +603,7 @@ describe('AuthService', () => { const { cookieName } = await realService.createAuthorization('res'); const sealedState = realStorage.cookies.get(cookieName)!; - realStorage.cookies.set( - 'wos-auth-verifier', - sealedState.slice(0, -2) + 'XX', - ); + realStorage.cookies.set(cookieName, sealedState.slice(0, -2) + 'XX'); await expect( realService.handleCallback('req', 'res', { @@ -635,9 +612,7 @@ describe('AuthService', () => { }), ).rejects.toThrow(OAuthStateMismatchError); - expect(realStorage.lastClearOptions.get('wos-auth-verifier')?.path).toBe( - '/', - ); + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); }); it('emits a scheme-agnostic (secure=false, sameSite=lax) delete on pre-unseal failure', async () => { @@ -661,10 +636,7 @@ describe('AuthService', () => { // proving we can still clean up). expect(realStorage.lastSetOptions.get(cookieName)?.secure).toBe(false); const sealedState = realStorage.cookies.get(cookieName)!; - realStorage.cookies.set( - 'wos-auth-verifier', - sealedState.slice(0, -2) + 'XX', - ); + realStorage.cookies.set(cookieName, sealedState.slice(0, -2) + 'XX'); await expect( realService.handleCallback('req', 'res', { @@ -673,7 +645,7 @@ describe('AuthService', () => { }), ).rejects.toThrow(OAuthStateMismatchError); - const clearOpts = realStorage.lastClearOptions.get('wos-auth-verifier'); + const clearOpts = realStorage.lastClearOptions.get(cookieName); expect(clearOpts?.secure).toBe(false); expect(clearOpts?.sameSite).toBe('lax'); expect(clearOpts?.path).toBe('/'); @@ -690,10 +662,7 @@ describe('AuthService', () => { const { cookieName } = await realService.createAuthorization('res'); const sealedState = realStorage.cookies.get(cookieName)!; - realStorage.cookies.set( - 'wos-auth-verifier', - sealedState.slice(0, -2) + 'XX', - ); + realStorage.cookies.set(cookieName, sealedState.slice(0, -2) + 'XX'); await expect( realService.handleCallback('req', 'res', { @@ -702,7 +671,7 @@ describe('AuthService', () => { }), ).rejects.toThrow(OAuthStateMismatchError); - const clearOpts = realStorage.lastClearOptions.get('wos-auth-verifier'); + const clearOpts = realStorage.lastClearOptions.get(cookieName); expect(clearOpts?.secure).toBe(true); expect(clearOpts?.sameSite).toBe('none'); }); @@ -718,10 +687,8 @@ describe('AuthService', () => { const { cookieName } = await realService.createAuthorization('res'); const sealedState = realStorage.cookies.get(cookieName)!; - // Delete the derived-name cookie; the legacy name was never written - // (Task 4 only writes under the derived name), so handleCallback's - // legacy-name read still returns null and triggers the missing-cookie - // path under test. + // Delete the derived-name cookie so handleCallback's read returns + // null and triggers the missing-cookie path under test. realStorage.cookies.delete(cookieName); await expect( @@ -731,9 +698,7 @@ describe('AuthService', () => { }), ).rejects.toThrow(PKCECookieMissingError); - expect(realStorage.lastClearOptions.get('wos-auth-verifier')?.path).toBe( - '/', - ); + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); }); it('best-effort clears the verifier cookie when authenticateWithCode throws', async () => { @@ -750,7 +715,7 @@ describe('AuthService', () => { ); const { cookieName } = await realService.createAuthorization('res'); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; await expect( realService.handleCallback('req', 'res', { @@ -759,9 +724,7 @@ describe('AuthService', () => { }), ).rejects.toThrow('WorkOS exchange failed'); - expect(realStorage.lastClearOptions.get('wos-auth-verifier')?.path).toBe( - '/', - ); + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); }); it('best-effort clears the verifier cookie when saveSession throws', async () => { @@ -777,7 +740,7 @@ describe('AuthService', () => { ); const { cookieName } = await realService.createAuthorization('res'); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; await expect( realService.handleCallback('req', 'res', { @@ -786,9 +749,7 @@ describe('AuthService', () => { }), ).rejects.toThrow('storage write failed'); - expect(realStorage.lastClearOptions.get('wos-auth-verifier')?.path).toBe( - '/', - ); + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); }); it('swallows clearCookie errors so the original failure propagates', async () => { @@ -808,7 +769,7 @@ describe('AuthService', () => { ); const { cookieName } = await realService.createAuthorization('res'); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; await expect( realService.handleCallback('req', 'res', { @@ -832,7 +793,7 @@ describe('AuthService', () => { const { cookieName } = await realService.createAuthorization('res', { redirectUri: 'http://localhost:3000/callback', }); - const sealedState = mirrorToLegacyName(realStorage, cookieName); + const sealedState = realStorage.cookies.get(cookieName)!; expect(realStorage.lastSetOptions.get(cookieName)?.secure).toBe(false); @@ -841,9 +802,71 @@ describe('AuthService', () => { state: sealedState, }); - expect( - realStorage.lastClearOptions.get('wos-auth-verifier')?.secure, - ).toBe(false); + expect(realStorage.lastClearOptions.get(cookieName)?.secure).toBe(false); + }); + + describe('per-flow cookie isolation', () => { + it('reads and clears the cookie derived from URL state', async () => { + // Start a flow. createSignIn wrote under the derived name. + const realStorage = makeStorage(); + const authService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + const { cookieName } = await authService.createSignIn(undefined); + const sealedState = realStorage.cookies.get(cookieName)!; + + const result = await authService.handleCallback( + 'req' as never, + 'res' as never, + { code: 'abc', state: sealedState }, + ); + + const setCookies = Array.isArray(result.headers?.['Set-Cookie']) + ? (result.headers!['Set-Cookie'] as string[]) + : ([result.headers?.['Set-Cookie']].filter(Boolean) as string[]); + const deleteLine = setCookies.find(c => + c.startsWith(`${cookieName}=`), + ); + expect(deleteLine).toBeDefined(); + expect(deleteLine).toContain('Max-Age=0'); + }); + + it("does not touch another concurrent flow's cookie", async () => { + const realStorage = makeStorage(); + const authService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + const a = await authService.createSignIn(undefined, { + returnPathname: '/a', + }); + const b = await authService.createSignIn(undefined, { + returnPathname: '/b', + }); + const sealedA = realStorage.cookies.get(a.cookieName)!; + + const result = await authService.handleCallback( + 'req' as never, + 'res' as never, + { code: 'abc', state: sealedA }, + ); + + const setCookies = Array.isArray(result.headers?.['Set-Cookie']) + ? (result.headers!['Set-Cookie'] as string[]) + : ([result.headers?.['Set-Cookie']].filter(Boolean) as string[]); + // Flow A's cookie gets a delete. Flow B's cookie must NOT. + expect( + setCookies.some(c => c.startsWith(`${a.cookieName}=`)), + ).toBe(true); + expect( + setCookies.some(c => c.startsWith(`${b.cookieName}=`)), + ).toBe(false); + }); }); }); }); diff --git a/src/service/AuthService.ts b/src/service/AuthService.ts index 18c39b8..afb14e2 100644 --- a/src/service/AuthService.ts +++ b/src/service/AuthService.ts @@ -1,6 +1,7 @@ import type { WorkOS } from '@workos-inc/node'; import { AuthKitCore } from '../core/AuthKitCore.js'; import type { AuthKitConfig } from '../core/config/types.js'; +import { getPKCECookieNameForState } from '../core/pkce/cookieName.js'; import { getPKCECookieOptions, PKCE_COOKIE_NAME, @@ -321,7 +322,12 @@ export class AuthService { state: string | undefined; }, ) { - const cookieValue = await this.storage.getCookie(request, PKCE_COOKIE_NAME); + const cookieName = options.state + ? getPKCECookieNameForState(options.state) + : null; + const cookieValue = cookieName + ? await this.storage.getCookie(request, cookieName) + : null; let unsealed; try { @@ -337,7 +343,7 @@ export class AuthService { // case so the Set-Cookie is accepted over http:// callbacks too. // The `sameSite: 'none'` case already forces Secure on both set and // clear, so there's no scheme-mismatch risk there. - await this.bestEffortClearVerifier(response, undefined, { + await this.bestEffortClearVerifier(response, cookieName, undefined, { schemeAgnostic: true, }); throw err; @@ -363,11 +369,15 @@ export class AuthService { const encryptedSession = await this.core.encryptSession(session); const save = await this.storage.saveSession(response, encryptedSession); - const clear = await this.storage.clearCookie( - save.response ?? response, - PKCE_COOKIE_NAME, - clearOptions, - ); + + let clear: { response?: TResponse; headers?: HeadersBag } = {}; + if (cookieName) { + clear = await this.storage.clearCookie( + save.response ?? response, + cookieName, + clearOptions, + ); + } return { response: clear.response ?? save.response, @@ -377,7 +387,7 @@ export class AuthService { authResponse, }; } catch (err) { - await this.bestEffortClearVerifier(response, redirectUri); + await this.bestEffortClearVerifier(response, cookieName, redirectUri); throw err; } } @@ -397,15 +407,17 @@ export class AuthService { */ private async bestEffortClearVerifier( response: TResponse | undefined, + cookieName: string | null, redirectUri: string | undefined, { schemeAgnostic = false }: { schemeAgnostic?: boolean } = {}, ): Promise { + if (!cookieName) return; // nothing to clear — no state on the URL. const options = getPKCECookieOptions(this.config, redirectUri); if (schemeAgnostic && options.sameSite === 'lax') { options.secure = false; } try { - await this.storage.clearCookie(response, PKCE_COOKIE_NAME, options); + await this.storage.clearCookie(response, cookieName, options); } catch { // Swallow: cleanup is opportunistic; callers get the original error. } From abf45f29b8badfe53a06bce212f8ec045046ebf9 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 16:08:01 -0700 Subject: [PATCH 11/21] feat(service): add pure URL-generation methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getAuthorizationUrl, getSignInUrl, getSignUpUrl — same URL that createAuthorization/createSignIn/createSignUp produce, but no cookie write. Adapters with loop-prone paths (middleware hooks that fire on every request) can use these for non-document requests to avoid HTTP 431 cookie bloat. --- src/service/AuthService.spec.ts | 52 +++++++++++++++++++++++++++++++++ src/service/AuthService.ts | 30 +++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index 8a0bee2..1b3c886 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -325,6 +325,58 @@ describe('AuthService', () => { }); }); + describe('AuthService — pure URL-generation methods', () => { + it('getAuthorizationUrl returns { url, cookieName } without touching storage', async () => { + const realStorage = makeStorage(); + const authService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); + const result = await authService.getAuthorizationUrl({ + returnPathname: '/foo', + }); + + expect(result.url).toMatch(/^https:\/\//); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + expect(setCookieSpy).not.toHaveBeenCalled(); + expect(result).not.toHaveProperty('response'); + expect(result).not.toHaveProperty('headers'); + }); + + it('getSignInUrl returns { url, cookieName } with sign-in screen hint', async () => { + const realStorage = makeStorage(); + const authService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); + const result = await authService.getSignInUrl({ returnPathname: '/foo' }); + expect(result.url).toContain('screen_hint=sign-in'); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + expect(setCookieSpy).not.toHaveBeenCalled(); + }); + + it('getSignUpUrl returns { url, cookieName } with sign-up screen hint', async () => { + const realStorage = makeStorage(); + const authService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); + const result = await authService.getSignUpUrl(); + expect(result.url).toContain('screen_hint=sign-up'); + expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + expect(setCookieSpy).not.toHaveBeenCalled(); + }); + }); + describe('clearPendingVerifier()', () => { it('emits a delete cookie with Path=/', async () => { const realStorage = makeStorage(); diff --git a/src/service/AuthService.ts b/src/service/AuthService.ts index afb14e2..cd7f8aa 100644 --- a/src/service/AuthService.ts +++ b/src/service/AuthService.ts @@ -267,6 +267,36 @@ export class AuthService { }); } + /** + * Pure URL generation — returns the auth URL and the cookie name that + * WOULD be written by `createAuthorization`, but does NOT touch + * storage. Use this in adapter code paths where writing the verifier + * cookie is inappropriate (e.g. non-document requests in middleware + * hooks) — the browser ignores the cookie anyway because it won't + * follow a cross-origin redirect from fetch/XHR. + */ + async getAuthorizationUrl( + options: GetAuthorizationUrlOptions = {}, + ): Promise<{ url: string; cookieName: string }> { + const { url, cookieName } = + await this.operations.createAuthorization(options); + return { url, cookieName }; + } + + /** Pure variant of createSignIn — no cookie write. */ + async getSignInUrl( + options: Omit = {}, + ): Promise<{ url: string; cookieName: string }> { + return this.getAuthorizationUrl({ ...options, screenHint: 'sign-in' }); + } + + /** Pure variant of createSignUp — no cookie write. */ + async getSignUpUrl( + options: Omit = {}, + ): Promise<{ url: string; cookieName: string }> { + return this.getAuthorizationUrl({ ...options, screenHint: 'sign-up' }); + } + /** * Emit a `Set-Cookie` header that clears the PKCE verifier cookie. * From afc6144e6a8da9d2e77c17ec2fe21d8dfadfcc0d Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 16:10:19 -0700 Subject: [PATCH 12/21] feat(service)!: clearPendingVerifier requires state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: clearPendingVerifier now takes { state: string; redirectUri?: string } (state required). The old state-less form had no meaning in the per-flow cookie world — there is no single cookie name to clear without knowing which flow. Callers on callback paths have URL state in hand. Bailouts with no state should skip the call entirely; the 10-minute PKCE TTL handles orphans. --- src/service/AuthService.spec.ts | 62 +++++++++++++++++++++++++++++---- src/service/AuthService.ts | 27 +++++++------- 2 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index 1b3c886..f5cb1bd 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -387,11 +387,12 @@ describe('AuthService', () => { sessionEncryption, ); - await realService.clearPendingVerifier('res'); + const { cookieName } = await realService.createSignIn(undefined); + const sealedState = realStorage.cookies.get(cookieName)!; - expect(realStorage.lastClearOptions.get('wos-auth-verifier')?.path).toBe( - '/', - ); + await realService.clearPendingVerifier('res', { state: sealedState }); + + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); }); it('accepts undefined response for headers-only adapters', async () => { @@ -403,9 +404,58 @@ describe('AuthService', () => { sessionEncryption, ); - const result = await realService.clearPendingVerifier(undefined); + const { cookieName } = await realService.createSignIn(undefined); + const sealedState = realStorage.cookies.get(cookieName)!; + + const result = await realService.clearPendingVerifier(undefined, { + state: sealedState, + }); + + expect(result.headers?.['Set-Cookie']).toContain(`${cookieName}=`); + }); + + it('clears the flow-specific cookie derived from state', async () => { + const realStorage = makeStorage(); + const realService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + + const { cookieName } = await realService.createSignIn(undefined); + const sealedState = realStorage.cookies.get(cookieName)!; + + const clearCookieSpy = vi.spyOn(realStorage, 'clearCookie'); + await realService.clearPendingVerifier(undefined, { state: sealedState }); + + expect(clearCookieSpy).toHaveBeenCalledWith( + undefined, + cookieName, + expect.any(Object), + ); + }); + + it('threads redirectUri into the cookie options', async () => { + const realStorage = makeStorage(); + const realService = new AuthService( + mockConfig as any, + realStorage as any, + makeClient() as any, + sessionEncryption, + ); + + const { cookieName } = await realService.createSignIn(undefined, { + redirectUri: 'https://custom.example/cb', + }); + const sealedState = realStorage.cookies.get(cookieName)!; + + await realService.clearPendingVerifier(undefined, { + state: sealedState, + redirectUri: 'https://custom.example/cb', + }); - expect(result.headers?.['Set-Cookie']).toContain('wos-auth-verifier='); + expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); }); }); diff --git a/src/service/AuthService.ts b/src/service/AuthService.ts index cd7f8aa..1a2bae2 100644 --- a/src/service/AuthService.ts +++ b/src/service/AuthService.ts @@ -2,10 +2,7 @@ import type { WorkOS } from '@workos-inc/node'; import { AuthKitCore } from '../core/AuthKitCore.js'; import type { AuthKitConfig } from '../core/config/types.js'; import { getPKCECookieNameForState } from '../core/pkce/cookieName.js'; -import { - getPKCECookieOptions, - PKCE_COOKIE_NAME, -} from '../core/pkce/cookieOptions.js'; +import { getPKCECookieOptions } from '../core/pkce/cookieOptions.js'; import { AuthOperations } from '../operations/AuthOperations.js'; import type { AuthResult, @@ -298,24 +295,28 @@ export class AuthService { } /** - * Emit a `Set-Cookie` header that clears the PKCE verifier cookie. + * Emit a `Set-Cookie` header that clears the PKCE verifier cookie + * for the flow identified by `state`. * - * Use on any exit path where a sign-in was started (verifier cookie - * written) but `handleCallback` will not run to clear it — OAuth error - * responses, missing `code`, early bail-outs. + * **Breaking change in 0.5.0.** The `state` option is now required + * — the per-flow cookie naming scheme has no single "legacy" name + * to clear. Callers typically read `state` from the callback URL; + * when `state` is absent (malformed callback), do not call this + * method. The 10-minute PKCE TTL cleans up orphans. * * Pass `options.redirectUri` on requests that used a per-request - * `redirectUri` override at sign-in time, so the delete cookie's computed - * attributes (notably `secure`) match what was originally set. + * `redirectUri` override at sign-in time, so the delete cookie's + * computed attributes (notably `secure`) match the original set. */ async clearPendingVerifier( response: TResponse | undefined, - options?: Pick, + options: { state: string; redirectUri?: string }, ): Promise<{ response?: TResponse; headers?: HeadersBag }> { + const cookieName = getPKCECookieNameForState(options.state); return this.storage.clearCookie( response, - PKCE_COOKIE_NAME, - getPKCECookieOptions(this.config, options?.redirectUri), + cookieName, + getPKCECookieOptions(this.config, options.redirectUri), ); } From e46e4ccea7e7d86be4eae758cd0e67d23b663552 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 16:11:29 -0700 Subject: [PATCH 13/21] feat: export PKCE helpers from package root Exposes PKCE_COOKIE_PREFIX and getPKCECookieNameForState for custom adapters. PKCE_COOKIE_NAME remains as back-compat alias. --- src/index.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/index.ts b/src/index.ts index 5da4dc6..b4f16ed 100644 --- a/src/index.ts +++ b/src/index.ts @@ -37,6 +37,16 @@ export { AuthOperations } from './operations/AuthOperations.js'; // ============================================ export { CookieSessionStorage } from './core/session/CookieSessionStorage.js'; +// ============================================ +// PKCE Helpers +// ============================================ +export { + PKCE_COOKIE_PREFIX, + getPKCECookieNameForState, +} from './core/pkce/cookieName.js'; +// Back-compat alias. Prefer PKCE_COOKIE_PREFIX or the derived names via getPKCECookieNameForState. +export { PKCE_COOKIE_NAME } from './core/pkce/cookieOptions.js'; + // ============================================ // Encryption Fallback // ============================================ From 8b5938225fb239b2e6512869653c22f4900a80e7 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 16:14:01 -0700 Subject: [PATCH 14/21] docs: document per-flow PKCE cookies and clearPendingVerifier break Update README.md and MIGRATION.md to reflect the 0.5.0 signature change on clearPendingVerifier and introduce the new pure URL- generation methods. --- MIGRATION.md | 81 +++++++++++++++++++++++++++++++++++++++++++++------- README.md | 52 +++++++++++++++++++++++++++++---- 2 files changed, 118 insertions(+), 15 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index ac6ed92..4f7c053 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,5 +1,43 @@ # Migration Guide +## 0.5.0 — per-flow PKCE cookies + +PKCE verifier cookies now carry a per-flow suffix +(`wos-auth-verifier-`) so concurrent sign-ins from multiple +tabs no longer clobber each other. `clearPendingVerifier` now +**requires** `options.state`. + +### What consumers need to change + +| Before | After | +| -------------------------------------------------------- | ----------------------------------------------------------------- | +| `auth.clearPendingVerifier(response)` | `auth.clearPendingVerifier(response, { state })` | +| `auth.clearPendingVerifier(response, { redirectUri })` | `auth.clearPendingVerifier(response, { state, redirectUri })` | + +Guard the call on `state` presence: + +```ts +if (state) { + await auth.clearPendingVerifier(response, { state }); +} +``` + +Skip the call entirely when `state` is absent (malformed callback) — +the 10-minute PKCE TTL cleans up orphan cookies. + +### New pure URL methods + +- `getAuthorizationUrl(options)` — returns `{ url, cookieName }`, writes no cookie. +- `getSignInUrl(options)` — same with `screenHint: 'sign-in'`. +- `getSignUpUrl(options)` — same with `screenHint: 'sign-up'`. + +Use these in adapter code paths where the cookie write is wasted +(e.g. non-document requests in a SvelteKit `handle` hook). Browsers +don't follow cross-origin redirects from fetch/XHR, so the cookie +would never be used anyway. + +--- + ## 0.3.x → 0.4.0 0.4.0 introduces OAuth state binding via a PKCE verifier cookie and collapses @@ -24,7 +62,7 @@ adapter version. | No verifier cookie on the wire | New `wos-auth-verifier` cookie, `HttpOnly`, `Max-Age=600` | | `handleCallback` emits a single `Set-Cookie` string | Emits `string[]` — session cookie + verifier delete | | `state` = plaintext `{internal}.{userState}` | `state` = opaque sealed blob (custom `state` still round-trips) | -| No error-path cleanup helper | New: `clearPendingVerifier(response)` | +| No error-path cleanup helper | New: `clearPendingVerifier(response, { state })` | --- @@ -149,19 +187,27 @@ must also implement `setCookie` and `clearCookie`. See `src/core/session/types.t On paths where sign-in was initiated but `handleCallback` never runs (OAuth error responses, missing `code`, early bail-outs), the verifier cookie would -linger until `Max-Age` expires. Call `clearPendingVerifier` to emit a delete: +linger until `Max-Age` expires. Call `clearPendingVerifier` with the `state` +from the callback URL to emit a delete for the correct per-flow cookie: ```ts -const { headers } = await auth.clearPendingVerifier(response); -// Apply headers the same way you apply any storage output +if (state) { + const { headers } = await auth.clearPendingVerifier(response, { state }); + // Apply headers the same way you apply any storage output +} ``` -For headers-only adapters, pass `undefined`: +For headers-only adapters, pass `undefined` as the response: ```ts -const { headers } = await auth.clearPendingVerifier(undefined); +if (state) { + const { headers } = await auth.clearPendingVerifier(undefined, { state }); +} ``` +Skip the call entirely when `state` is absent from the callback URL +(malformed callback) — the 10-minute PKCE TTL cleans up orphan cookies. + (On callback success, the verifier is cleared automatically.) --- @@ -238,15 +284,30 @@ the cookie is read — state mismatch, tampered seal, exchange failure, or save failure — so response-mutating adapters don't need to call `clearPendingVerifier` manually. Headers-only adapters that can't observe the response mutation should still call `clearPendingVerifier` in the catch -block to capture the delete `Set-Cookie` headers. +block to capture the delete `Set-Cookie` headers — pass the `state` from +the callback URL so the correct per-flow cookie is cleared, and skip the +call when `state` is absent: + +```ts +try { + await auth.handleCallback(request, response, { code, state }); +} catch (err) { + if (state) { + await auth.clearPendingVerifier(response, { state }); + } + throw err; +} +``` --- ### 7. Verifier cookie on the wire -A `wos-auth-verifier` cookie is set during sign-in and read during callback. +A `wos-auth-verifier-` cookie is set during sign-in and read during +callback. As of 0.5.0 the cookie name carries a per-flow suffix so concurrent +sign-ins from multiple tabs don't clobber each other. -- **Name**: `wos-auth-verifier` +- **Name**: `wos-auth-verifier-` (per-flow; suffix derived from the sealed blob) - **HttpOnly**, **Secure** (unless explicitly `SameSite=None` without HTTPS) - **SameSite**: `Lax` (survives the cross-site return from WorkOS). `None` preserved for iframe/embed flows. @@ -257,7 +318,7 @@ A `wos-auth-verifier` cookie is set during sign-in and read during callback. **Checklist** - [ ] Edge/CDN/firewall allowlists pass the cookie through. -- [ ] Cookie-stripping proxies don't strip `wos-auth-verifier`. +- [ ] Cookie-stripping proxies don't strip `wos-auth-verifier-*`. - [ ] Multiple AuthKit apps on the same host use distinct `cookieDomain`s (path-based isolation is not available — the cookie path is always `/`). - [ ] CSP or cookie-policy banners don't interfere with setting an `HttpOnly` diff --git a/README.md b/README.md index 8fe04f4..8f96f9c 100644 --- a/README.md +++ b/README.md @@ -199,20 +199,52 @@ authService.createAuthorization(response, options) authService.createSignIn(response, options) authService.createSignUp(response, options) +// URL Generation — pure, return { url, cookieName } WITHOUT writing a cookie +authService.getAuthorizationUrl(options) +authService.getSignInUrl(options) +authService.getSignUpUrl(options) + // Error-path cleanup for the PKCE verifier cookie +// `state` is required (from the callback URL) — it identifies which per-flow +// verifier cookie to clear. Skip this call when `state` is absent from the +// callback URL (malformed callback); the 10-minute PKCE TTL handles the orphan. // (response may be `undefined` for headers-only adapters) -authService.clearPendingVerifier(response, { redirectUri? }) +authService.clearPendingVerifier(response, { state, redirectUri? }) +``` + +### Generating URLs without writing a cookie + +`getAuthorizationUrl`, `getSignInUrl`, and `getSignUpUrl` return +`{ url, cookieName }` WITHOUT writing the PKCE verifier cookie. Use +these in adapter code paths where the cookie write is wasted — for +example, on non-document requests in a middleware hook. Browsers +don't follow cross-origin redirects from fetch/XHR/RSC/prefetch, so +a cookie write on those requests is noise. + +```ts +const { url, cookieName } = await authService.getSignInUrl({ + returnPathname: '/dashboard', +}); +// No Set-Cookie emitted. Use createSignIn if you want the cookie written. ``` -### PKCE verifier cookie (`wos-auth-verifier`) +For regular user-initiated sign-in flows, keep using `createSignIn` / +`createSignUp` / `createAuthorization` — they write the cookie and +return it alongside the URL. + +### PKCE verifier cookie (`wos-auth-verifier-`) This library binds every OAuth sign-in to a PKCE code verifier, so a leaked `state` value on its own cannot be used to complete a session hijack. +Each in-flight sign-in gets its own per-flow verifier cookie with a +deterministic suffix derived from the sealed blob, so concurrent +sign-ins from multiple tabs no longer clobber each other. + The verifier is sealed into a single blob that serves two roles: 1. It is sent to WorkOS as the OAuth `state` query parameter. -2. It is set as a short-lived HTTP-only cookie (`wos-auth-verifier`, 10 min). +2. It is set as a short-lived HTTP-only cookie (`wos-auth-verifier-`, 10 min). The cookie is written and read through `SessionStorage`. Callers don't see sealed blobs or cookie options: @@ -253,8 +285,18 @@ if (setCookie) { Mismatched state and cookie raise `OAuthStateMismatchError`. A missing cookie (typical cause: Set-Cookie stripped by a proxy) raises `PKCECookieMissingError`. On either error path — or any early bail-out before -`handleCallback` runs — call `authService.clearPendingVerifier(response)` to -emit a delete header. +`handleCallback` runs — call +`authService.clearPendingVerifier(response, { state })` with the `state` from +the callback URL to emit a delete header for the correct per-flow cookie: + +```ts +if (state) { + await authService.clearPendingVerifier(response, { state }); +} +``` + +If the callback URL has no `state` (malformed callback), skip this call — the +10-minute PKCE TTL handles the orphan. ### Direct Access (Advanced) From c7672655963d17e970f3b7fcdcba25c449244c65 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 16:15:31 -0700 Subject: [PATCH 15/21] chore: release 0.5.0 Per-flow PKCE verifier cookies. See MIGRATION.md#050--per-flow-pkce-cookies and docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c5cf690..874ad12 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@workos/authkit-session", - "version": "0.4.0", + "version": "0.5.0", "description": "Framework-agnostic authentication library for WorkOS with pluggable storage adapters", "keywords": [], "license": "MIT", From 57ca49197a22235b2259d1ec51b83b3808a15a81 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 18:29:05 -0700 Subject: [PATCH 16/21] fix(factory): forward pure URL helpers through lazy proxy `createAuthService` returns a hand-rolled proxy cast to `AuthService`, which silently omitted `getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl`. Consumers saw them in autocomplete but hit `TypeError: not a function` at runtime. Add the forwarders and cover the factory surface with typeof checks + an end-to-end test. --- src/service/factory.spec.ts | 29 +++++++++++++++++++++++++++++ src/service/factory.ts | 3 +++ 2 files changed, 32 insertions(+) diff --git a/src/service/factory.spec.ts b/src/service/factory.spec.ts index 3563972..237ca4c 100644 --- a/src/service/factory.spec.ts +++ b/src/service/factory.spec.ts @@ -43,6 +43,9 @@ describe('createAuthService', () => { expect(typeof service.createAuthorization).toBe('function'); expect(typeof service.createSignIn).toBe('function'); expect(typeof service.createSignUp).toBe('function'); + expect(typeof service.getAuthorizationUrl).toBe('function'); + expect(typeof service.getSignInUrl).toBe('function'); + expect(typeof service.getSignUpUrl).toBe('function'); expect(typeof service.clearPendingVerifier).toBe('function'); expect(typeof service.getWorkOS).toBe('function'); expect(typeof service.handleCallback).toBe('function'); @@ -77,6 +80,32 @@ describe('createAuthService', () => { // Factory accepts the custom encryption - verify by checking the interface exists expect(typeof service.withAuth).toBe('function'); }); + + it('forwards pure URL helpers through the factory proxy', async () => { + const customClient = { + pkce: { + generate: async () => ({ + codeVerifier: 'verifier', + codeChallenge: 'challenge', + codeChallengeMethod: 'S256', + }), + }, + userManagement: { + getAuthorizationUrl: () => 'https://example.com/authorize', + getJwksUrl: () => 'https://custom.example.com/jwks', + }, + }; + + const service = createAuthService({ + sessionStorageFactory: () => mockStorage as any, + clientFactory: () => customClient as any, + }); + + const result = await service.getSignInUrl({ returnPathname: '/dashboard' }); + + expect(result.url).toBe('https://example.com/authorize'); + expect(result.cookieName).toMatch(/^wos-auth-verifier-/); + }); }); describe('lazy initialization', () => { diff --git a/src/service/factory.ts b/src/service/factory.ts index 90f3399..278994d 100644 --- a/src/service/factory.ts +++ b/src/service/factory.ts @@ -76,6 +76,9 @@ export function createAuthService(options: { getService().createAuthorization(response, opts), createSignIn: (response, opts) => getService().createSignIn(response, opts), createSignUp: (response, opts) => getService().createSignUp(response, opts), + getAuthorizationUrl: opts => getService().getAuthorizationUrl(opts), + getSignInUrl: opts => getService().getSignInUrl(opts), + getSignUpUrl: opts => getService().getSignUpUrl(opts), clearPendingVerifier: (response, opts) => getService().clearPendingVerifier(response, opts), getWorkOS: () => getService().getWorkOS(), From a8abf0acf28c9ca68de8fc593fcc7d636d63e662 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 18:31:02 -0700 Subject: [PATCH 17/21] chore: formatting --- src/core/pkce/cookieName.spec.ts | 12 +++++++++--- src/core/pkce/pkce.spec.ts | 4 +++- src/service/AuthService.spec.ts | 16 +++++++--------- src/service/factory.spec.ts | 4 +++- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/core/pkce/cookieName.spec.ts b/src/core/pkce/cookieName.spec.ts index dcfabd3..d96df0a 100644 --- a/src/core/pkce/cookieName.spec.ts +++ b/src/core/pkce/cookieName.spec.ts @@ -26,17 +26,23 @@ describe('fnv1a32Hex', () => { }); it('is deterministic', () => { - expect(fnv1a32Hex('some-sealed-state')).toBe(fnv1a32Hex('some-sealed-state')); + expect(fnv1a32Hex('some-sealed-state')).toBe( + fnv1a32Hex('some-sealed-state'), + ); }); }); describe('getPKCECookieNameForState', () => { it('prefixes with wos-auth-verifier and appends an 8-char hex hash', () => { - expect(getPKCECookieNameForState('any-state')).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); + expect(getPKCECookieNameForState('any-state')).toMatch( + /^wos-auth-verifier-[0-9a-f]{8}$/, + ); }); it('produces different names for different states', () => { - expect(getPKCECookieNameForState('state-a')).not.toBe(getPKCECookieNameForState('state-b')); + expect(getPKCECookieNameForState('state-a')).not.toBe( + getPKCECookieNameForState('state-b'), + ); }); it('is deterministic for the same input', () => { diff --git a/src/core/pkce/pkce.spec.ts b/src/core/pkce/pkce.spec.ts index a62b359..1d90a58 100644 --- a/src/core/pkce/pkce.spec.ts +++ b/src/core/pkce/pkce.spec.ts @@ -92,7 +92,9 @@ describe('PKCE end-to-end round-trip', () => { it('returns cookieName derived from the sealed state', async () => { const result = await generate(); - expect(result.cookieName).toBe(getPKCECookieNameForState(result.sealedState)); + expect(result.cookieName).toBe( + getPKCECookieNameForState(result.sealedState), + ); expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); }); diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index f5cb1bd..b7a453a 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -929,9 +929,7 @@ describe('AuthService', () => { const setCookies = Array.isArray(result.headers?.['Set-Cookie']) ? (result.headers!['Set-Cookie'] as string[]) : ([result.headers?.['Set-Cookie']].filter(Boolean) as string[]); - const deleteLine = setCookies.find(c => - c.startsWith(`${cookieName}=`), - ); + const deleteLine = setCookies.find(c => c.startsWith(`${cookieName}=`)); expect(deleteLine).toBeDefined(); expect(deleteLine).toContain('Max-Age=0'); }); @@ -962,12 +960,12 @@ describe('AuthService', () => { ? (result.headers!['Set-Cookie'] as string[]) : ([result.headers?.['Set-Cookie']].filter(Boolean) as string[]); // Flow A's cookie gets a delete. Flow B's cookie must NOT. - expect( - setCookies.some(c => c.startsWith(`${a.cookieName}=`)), - ).toBe(true); - expect( - setCookies.some(c => c.startsWith(`${b.cookieName}=`)), - ).toBe(false); + expect(setCookies.some(c => c.startsWith(`${a.cookieName}=`))).toBe( + true, + ); + expect(setCookies.some(c => c.startsWith(`${b.cookieName}=`))).toBe( + false, + ); }); }); }); diff --git a/src/service/factory.spec.ts b/src/service/factory.spec.ts index 237ca4c..ff9f421 100644 --- a/src/service/factory.spec.ts +++ b/src/service/factory.spec.ts @@ -101,7 +101,9 @@ describe('createAuthService', () => { clientFactory: () => customClient as any, }); - const result = await service.getSignInUrl({ returnPathname: '/dashboard' }); + const result = await service.getSignInUrl({ + returnPathname: '/dashboard', + }); expect(result.url).toBe('https://example.com/authorize'); expect(result.cookieName).toMatch(/^wos-auth-verifier-/); From dd4e91bfb1d89f7c851e9a9e8432aaab5bfd58bd Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 18:49:15 -0700 Subject: [PATCH 18/21] chore: remove superpowers plans --- .../plans/2026-04-22-per-flow-pkce-cookies.md | 1753 ----------------- ...2026-04-22-per-flow-pkce-cookies-design.md | 270 --- 2 files changed, 2023 deletions(-) delete mode 100644 docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md delete mode 100644 docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md diff --git a/docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md b/docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md deleted file mode 100644 index 9f5dabd..0000000 --- a/docs/superpowers/plans/2026-04-22-per-flow-pkce-cookies.md +++ /dev/null @@ -1,1753 +0,0 @@ -# Per-flow PKCE Verifier Cookies Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Eliminate the multi-tab PKCE cookie clobbering bug by giving each concurrent OAuth flow its own uniquely-named `wos-auth-verifier-` cookie across `authkit-session` and its two downstream adapters. - -**Architecture:** Derive the cookie name from an FNV-1a hash of the sealed PKCE state (pure, no new dep). `AuthService.handleCallback` derives the name from the URL `state` at read time. `clearPendingVerifier` becomes state-aware. Adapters with middleware-loop behavior (SvelteKit's `createWithAuth`) additionally gate cookie writes on document-request detection to prevent HTTP 431. Driven release-sequenced across three packages: `authkit-session` → `authkit-sveltekit` → `authkit-tanstack-start`. - -**Tech Stack:** TypeScript (strict), Vitest, pnpm, Node. Targets: `authkit-session` (framework-agnostic core), `authkit-sveltekit` (SvelteKit 2 adapter), `authkit-tanstack-react-start` (TanStack Start adapter). - -**Spec:** `docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md` - ---- - -## Working directories - -- `authkit-session`: `/Users/nicknisi/Developer/authkit-session` -- `authkit-sveltekit`: `/Users/nicknisi/Developer/authkit-sveltekit` -- `authkit-tanstack-start`: `/Users/nicknisi/Developer/authkit-tanstack-start` - -All three repos use pnpm. Run `pnpm install`, `pnpm test`, `pnpm build`, `pnpm typecheck` from each repo root. - -## File-structure map - -**`authkit-session` — creates:** -- `src/core/pkce/cookieName.ts` — FNV-1a + `getPKCECookieNameForState` -- `src/core/pkce/cookieName.spec.ts` - -**`authkit-session` — modifies:** -- `src/core/pkce/generateAuthorizationUrl.ts` — add `cookieName` to result -- `src/core/pkce/cookieOptions.ts` — re-export prefix; keep legacy name -- `src/service/AuthService.ts` — derive cookie name, new pure methods, require-state in `clearPendingVerifier` -- `src/service/AuthService.spec.ts` -- `src/core/session/types.ts` — extend `CreateAuthorizationResult` with `cookieName` -- `src/index.ts` — new exports -- `README.md`, `MIGRATION.md` — document signature change and new methods -- `package.json` — version `0.5.0` - -**`authkit-sveltekit` — creates:** -- `src/server/adapters/isDocumentRequest.ts` -- `src/server/adapters/isDocumentRequest.test.ts` - -**`authkit-sveltekit` — modifies:** -- `src/server/middleware.ts` — gate cookie writes in `createWithAuth` -- `src/server/auth.ts` — thread `state` into `clearPendingVerifier` -- `src/tests/get-sign-in-url.test.ts` — derive expected cookie names -- `src/tests/handle-callback.test.ts` — derive expected cookie names -- `package.json` — bump `@workos/authkit-session` dep + version `0.3.0` - -**`authkit-tanstack-start` — modifies:** -- `src/server/server.ts` — replace static fallback, thread `state`, fix comments -- `src/server/server.spec.ts` — update cookie-name assertions -- `src/server/server-functions.spec.ts` — update cookie-name assertions -- `package.json` — bump `@workos/authkit-session` dep + version `0.7.0` - -## Phases - -Three phases, sequenced strictly. Do not start Phase 2 until Phase 1 is merged/published. Do not start Phase 3 until Phase 2 is merged. - -- **Phase 1** — `authkit-session` (Tasks 1–10). Ends with a `0.5.0` release tag/commit. -- **Phase 2** — `authkit-sveltekit` (Tasks 11–15). Ends with a `0.3.0` release tag/commit. -- **Phase 3** — `authkit-tanstack-react-start` (Tasks 16–20). Ends with a `0.7.0` release tag/commit. - ---- - -## Phase 1 — `authkit-session` - -Start in `/Users/nicknisi/Developer/authkit-session`. On a fresh branch: `git checkout -b pkce-per-flow-cookies`. - -### Task 1: `cookieName.ts` — FNV-1a 32-bit hex + name derivation - -**Files:** -- Create: `src/core/pkce/cookieName.ts` -- Create: `src/core/pkce/cookieName.spec.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `src/core/pkce/cookieName.spec.ts`: - -```ts -import { describe, it, expect } from 'vitest'; -import { - PKCE_COOKIE_PREFIX, - getPKCECookieNameForState, - fnv1a32Hex, -} from './cookieName.js'; - -describe('fnv1a32Hex', () => { - // Known-answer tests against the reference FNV-1a 32-bit spec - // (http://www.isthe.com/chongo/tech/comp/fnv/). Empty string is the - // FNV offset basis 0x811c9dc5. - it('hashes the empty string to the FNV offset basis', () => { - expect(fnv1a32Hex('')).toBe('811c9dc5'); - }); - - it('hashes "a" to 0xe40c292c', () => { - expect(fnv1a32Hex('a')).toBe('e40c292c'); - }); - - it('hashes "foobar" to 0xbf9cf968', () => { - expect(fnv1a32Hex('foobar')).toBe('bf9cf968'); - }); - - it('returns a zero-padded 8-char hex string', () => { - // input chosen to produce a short hash that would need padding; - // the specific value doesn't matter — the pad-to-8 behavior does. - expect(fnv1a32Hex('x')).toMatch(/^[0-9a-f]{8}$/); - }); - - it('is deterministic', () => { - expect(fnv1a32Hex('some-sealed-state')).toBe(fnv1a32Hex('some-sealed-state')); - }); -}); - -describe('getPKCECookieNameForState', () => { - it('prefixes with wos-auth-verifier and appends an 8-char hex hash', () => { - expect(getPKCECookieNameForState('any-state')).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - }); - - it('produces different names for different states', () => { - expect(getPKCECookieNameForState('state-a')).not.toBe(getPKCECookieNameForState('state-b')); - }); - - it('is deterministic for the same input', () => { - const s = 'sealed-' + 'x'.repeat(200); - expect(getPKCECookieNameForState(s)).toBe(getPKCECookieNameForState(s)); - }); - - it('exports the prefix constant', () => { - expect(PKCE_COOKIE_PREFIX).toBe('wos-auth-verifier'); - }); -}); -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pnpm test src/core/pkce/cookieName.spec.ts -``` - -Expected: fails with `Cannot find module './cookieName.js'` (module not created yet). - -- [ ] **Step 3: Write the implementation** - -Create `src/core/pkce/cookieName.ts`: - -```ts -/** Stable prefix for all PKCE verifier cookies. */ -export const PKCE_COOKIE_PREFIX = 'wos-auth-verifier'; - -/** - * FNV-1a 32-bit hash of the input, returned as a zero-padded 8-char - * lowercase hex string. Used purely as a namespacing mechanism — not - * security-sensitive. Collision probability is ~1/4B per pair; a - * collision routes one flow's callback to the wrong cookie, which - * then fails byte-equality in `verifyCallbackState` (fail-closed). - */ -export function fnv1a32Hex(input: string): string { - let hash = 0x811c9dc5; - const bytes = new TextEncoder().encode(input); - for (const byte of bytes) { - hash = Math.imul(hash ^ byte, 0x01000193) >>> 0; - } - return hash.toString(16).padStart(8, '0'); -} - -/** - * Derive a flow-specific PKCE verifier cookie name from the sealed - * state blob. Each concurrent OAuth flow gets its own cookie so - * parallel sign-ins from multiple tabs don't clobber each other. - */ -export function getPKCECookieNameForState(state: string): string { - return `${PKCE_COOKIE_PREFIX}-${fnv1a32Hex(state)}`; -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pnpm test src/core/pkce/cookieName.spec.ts -``` - -Expected: all tests pass. - -- [ ] **Step 5: Commit** - -```bash -git add src/core/pkce/cookieName.ts src/core/pkce/cookieName.spec.ts -git commit -m "feat: add per-flow PKCE cookie name derivation - -Introduces PKCE_COOKIE_PREFIX and getPKCECookieNameForState(state), -backed by an inline FNV-1a 32-bit hash. Zero new dependencies." -``` - -### Task 2: Return `cookieName` from `generateAuthorizationUrl` - -**Files:** -- Modify: `src/core/pkce/generateAuthorizationUrl.ts` -- Modify: `src/core/pkce/cookieOptions.ts` - -- [ ] **Step 1: Update the tests** - -Open `src/core/pkce/pkce.spec.ts`. Any test that destructures the result of `generateAuthorizationUrl` will now also receive a `cookieName`. Add an assertion near the existing happy-path test: - -```ts -it('returns cookieName derived from the sealed state', async () => { - const result = await generateAuthorizationUrl({ - client: mockClient, - config: mockConfig, - encryption: mockEncryption, - options: {}, - }); - const { getPKCECookieNameForState } = await import('./cookieName.js'); - expect(result.cookieName).toBe(getPKCECookieNameForState(result.sealedState)); - expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); -}); -``` - -- [ ] **Step 2: Run test to verify it fails** - -```bash -pnpm test src/core/pkce/pkce.spec.ts -``` - -Expected: fails with `result.cookieName is undefined`. - -- [ ] **Step 3: Update `GeneratedAuthorizationUrl` interface and implementation** - -In `src/core/pkce/generateAuthorizationUrl.ts`: - -```ts -// Add import at top -import { getPKCECookieNameForState } from './cookieName.js'; - -// Update the interface -export interface GeneratedAuthorizationUrl { - url: string; - sealedState: string; - cookieName: string; - cookieOptions: CookieOptions; -} -``` - -Inside the function body, after `sealedState` is computed and the cookie byte-length guard passes, derive the cookie name and use it in the length check and the return: - -```ts -const cookieOptions = getPKCECookieOptions(config, redirectUri); -const cookieName = getPKCECookieNameForState(sealedState); -const serialized = serializeCookie(cookieName, sealedState, cookieOptions); -const cookieBytes = new TextEncoder().encode(serialized).byteLength; -if (cookieBytes > PKCE_MAX_COOKIE_BYTES) { - throw new PKCEPayloadTooLargeError( - `Sealed PKCE verifier cookie is ${cookieBytes} bytes, exceeds supported limit of ${PKCE_MAX_COOKIE_BYTES} bytes. ` + - `Reduce the size of options.state, options.returnPathname, or options.redirectUri.`, - ); -} - -// ... existing `url = client.userManagement.getAuthorizationUrl(...)` ... - -return { - url, - sealedState, - cookieName, - cookieOptions, -}; -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pnpm test src/core/pkce/pkce.spec.ts -``` - -Expected: all pass (existing + new). - -- [ ] **Step 5: Commit** - -```bash -git add src/core/pkce/generateAuthorizationUrl.ts src/core/pkce/pkce.spec.ts -git commit -m "feat(pkce): derive cookieName in generateAuthorizationUrl - -Adds cookieName to GeneratedAuthorizationUrl. Byte-length guard now -measures the actual (longer) on-wire name. Still well under the -3800-byte cap (worst case 26 chars vs 17)." -``` - -### Task 3: `CreateAuthorizationResult` gains `cookieName` - -**Files:** -- Modify: `src/core/session/types.ts:183` - -- [ ] **Step 1: Update the interface and ambient tests** - -In `src/core/session/types.ts`: - -```ts -export type CreateAuthorizationResult = GetAuthorizationUrlResult & { - response?: TResponse; - headers?: HeadersBag; - /** - * Name of the PKCE verifier cookie written during this call. Useful - * for assertion-in-tests and for adapters that want to log the flow - * identifier. NOT the shape `clearPendingVerifier` consumes — that - * method takes `state`, not `cookieName`. - */ - cookieName: string; -}; -``` - -- [ ] **Step 2: Run typecheck to see where it breaks** - -```bash -pnpm typecheck -``` - -Expected: compile errors in `src/service/AuthService.ts` where `createAuthorization` returns an object without `cookieName`. These get fixed in Task 4. - -- [ ] **Step 3: Commit the type change standalone** - -```bash -git add src/core/session/types.ts -git commit -m "feat(types): add cookieName to CreateAuthorizationResult - -Callers that destructure the result now see the PKCE verifier cookie -name that was written. Type-only change; runtime wiring lands next." -``` - -### Task 4: `AuthService.createAuthorization` uses and returns the derived name - -**Files:** -- Modify: `src/service/AuthService.ts:228-267` (createAuthorization + createSignIn + createSignUp) -- Modify: `src/service/AuthService.spec.ts` - -- [ ] **Step 1: Update the test** - -In `src/service/AuthService.spec.ts`, locate the test "returns url and writes the verifier cookie via storage.setCookie" (around line 227). Update it and add a new isolation test: - -```ts -it('returns url + cookieName and writes under the derived per-flow name', async () => { - const { url, cookieName, headers } = await authService.createSignIn(undefined, { - returnPathname: '/foo', - }); - expect(url).toMatch(/^https:\/\//); - expect(cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - - // Storage must have been called with the derived name, not the legacy name. - expect(realStorage.cookies.has('wos-auth-verifier')).toBe(false); - expect(realStorage.cookies.get(cookieName)).toBeTruthy(); - - // Set-Cookie header reflects the same name. - const setCookie = Array.isArray(headers?.['Set-Cookie']) - ? headers!['Set-Cookie'].join('\n') - : (headers?.['Set-Cookie'] ?? ''); - expect(setCookie).toContain(`${cookieName}=`); -}); - -it('isolates concurrent flows: two sign-ins produce two distinct cookies', async () => { - const a = await authService.createSignIn(undefined, { returnPathname: '/a' }); - const b = await authService.createSignIn(undefined, { returnPathname: '/b' }); - expect(a.cookieName).not.toBe(b.cookieName); - expect(realStorage.cookies.get(a.cookieName)).toBeTruthy(); - expect(realStorage.cookies.get(b.cookieName)).toBeTruthy(); -}); -``` - -Also audit the file for any existing assertion on `realStorage.cookies.get('wos-auth-verifier')` and replace with the derived name (use the `cookieName` returned from the call, or derive via `getPKCECookieNameForState(sealedState)`). - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pnpm test src/service/AuthService.spec.ts -``` - -Expected: new tests fail (`cookieName` undefined, storage write missing under derived name). - -- [ ] **Step 3: Update `createAuthorization`** - -In `src/service/AuthService.ts`, replace the body of `createAuthorization`: - -```ts -async createAuthorization( - response: TResponse | undefined, - options: GetAuthorizationUrlOptions = {}, -): Promise> { - const { url, sealedState, cookieName, cookieOptions } = - await this.operations.createAuthorization(options); - const write = await this.storage.setCookie( - response, - cookieName, - sealedState, - cookieOptions, - ); - return { url, cookieName, ...write }; -} -``` - -`createSignIn` and `createSignUp` already delegate to `createAuthorization`, so they need no code change — but double-check their return type picks up the new `cookieName` via inheritance. - -Also drop the now-unused `PKCE_COOKIE_NAME` import from the top of the file. The old `import { getPKCECookieOptions, PKCE_COOKIE_NAME } from '../core/pkce/cookieOptions.js';` becomes `import { getPKCECookieOptions } from '../core/pkce/cookieOptions.js';`. - -- [ ] **Step 4: Run tests to verify pass** - -```bash -pnpm test src/service/AuthService.spec.ts -``` - -Expected: all pass. - -- [ ] **Step 5: Commit** - -```bash -git add src/service/AuthService.ts src/service/AuthService.spec.ts -git commit -m "feat(service): write PKCE cookie under per-flow derived name - -createAuthorization, createSignIn, createSignUp now write under the -name returned by generateAuthorizationUrl. Concurrent flows no longer -clobber each other's cookies." -``` - -### Task 5: `handleCallback` derives cookie name from URL state - -**Files:** -- Modify: `src/service/AuthService.ts:316-412` (handleCallback + bestEffortClearVerifier) -- Modify: `src/service/AuthService.spec.ts` - -- [ ] **Step 1: Write the failing tests** - -Add to `src/service/AuthService.spec.ts`: - -```ts -describe('handleCallback — per-flow cookie isolation', () => { - it('reads and clears the cookie derived from URL state', async () => { - // Start a flow. createSignIn wrote under the derived name. - const { cookieName } = await authService.createSignIn(undefined); - const sealedState = realStorage.cookies.get(cookieName)!; - - mockClient.userManagement.authenticateWithCode.mockResolvedValue( - mockAuthenticationResponse(), - ); - - const request = new Request( - `https://app.example/callback?code=abc&state=${encodeURIComponent(sealedState)}`, - ); - - const result = await authService.handleCallback(request, undefined as never, { - code: 'abc', - state: sealedState, - }); - - const setCookies = Array.isArray(result.headers?.['Set-Cookie']) - ? result.headers!['Set-Cookie'] - : [result.headers?.['Set-Cookie']].filter(Boolean) as string[]; - const deleteLine = setCookies.find(c => c.startsWith(`${cookieName}=`)); - expect(deleteLine).toBeDefined(); - expect(deleteLine).toContain('Max-Age=0'); - }); - - it('does not touch another concurrent flow\'s cookie', async () => { - const a = await authService.createSignIn(undefined, { returnPathname: '/a' }); - const b = await authService.createSignIn(undefined, { returnPathname: '/b' }); - const sealedA = realStorage.cookies.get(a.cookieName)!; - - mockClient.userManagement.authenticateWithCode.mockResolvedValue( - mockAuthenticationResponse(), - ); - - const request = new Request( - `https://app.example/callback?code=abc&state=${encodeURIComponent(sealedA)}`, - ); - const result = await authService.handleCallback(request, undefined as never, { - code: 'abc', - state: sealedA, - }); - - const setCookies = Array.isArray(result.headers?.['Set-Cookie']) - ? result.headers!['Set-Cookie'] - : [result.headers?.['Set-Cookie']].filter(Boolean) as string[]; - // Flow A's cookie gets a delete. Flow B's cookie must NOT. - expect(setCookies.some(c => c.startsWith(`${a.cookieName}=`))).toBe(true); - expect(setCookies.some(c => c.startsWith(`${b.cookieName}=`))).toBe(false); - }); -}); -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pnpm test src/service/AuthService.spec.ts -t 'per-flow cookie isolation' -``` - -Expected: both tests fail (`handleCallback` still reads the legacy `wos-auth-verifier` name). - -- [ ] **Step 3: Rewrite `handleCallback`** - -In `src/service/AuthService.ts`, add the import at the top: - -```ts -import { getPKCECookieNameForState } from '../core/pkce/cookieName.js'; -``` - -Replace the body of `handleCallback`: - -```ts -async handleCallback( - request: TRequest, - response: TResponse, - options: { code: string; state: string | undefined }, -) { - const cookieName = options.state ? getPKCECookieNameForState(options.state) : null; - const cookieValue = cookieName - ? await this.storage.getCookie(request, cookieName) - : null; - - let unsealed; - try { - unsealed = await this.core.verifyCallbackState({ - stateFromUrl: options.state, - cookieValue: cookieValue ?? undefined, - }); - } catch (err) { - await this.bestEffortClearVerifier(response, cookieName, undefined, { - schemeAgnostic: true, - }); - throw err; - } - - const { codeVerifier, returnPathname, customState, redirectUri } = unsealed; - const clearOptions = getPKCECookieOptions(this.config, redirectUri); - - try { - const authResponse = await this.client.userManagement.authenticateWithCode({ - code: options.code, - clientId: this.config.clientId, - codeVerifier, - }); - - const session: Session = { - accessToken: authResponse.accessToken, - refreshToken: authResponse.refreshToken, - user: authResponse.user, - impersonator: authResponse.impersonator, - }; - - const encryptedSession = await this.core.encryptSession(session); - const save = await this.storage.saveSession(response, encryptedSession); - - let clear: { response?: TResponse; headers?: HeadersBag } = {}; - if (cookieName) { - clear = await this.storage.clearCookie( - save.response ?? response, - cookieName, - clearOptions, - ); - } - - return { - response: clear.response ?? save.response, - headers: mergeHeaderBags(save.headers, clear.headers), - returnPathname: returnPathname ?? '/', - state: customState, - authResponse, - }; - } catch (err) { - await this.bestEffortClearVerifier(response, cookieName, redirectUri); - throw err; - } -} -``` - -And update `bestEffortClearVerifier` to take the cookie name: - -```ts -private async bestEffortClearVerifier( - response: TResponse | undefined, - cookieName: string | null, - redirectUri: string | undefined, - { schemeAgnostic = false }: { schemeAgnostic?: boolean } = {}, -): Promise { - if (!cookieName) return; // nothing to clear — no state on the URL. - const options = getPKCECookieOptions(this.config, redirectUri); - if (schemeAgnostic && options.sameSite === 'lax') { - options.secure = false; - } - try { - await this.storage.clearCookie(response, cookieName, options); - } catch { - // Swallow: cleanup is opportunistic; callers get the original error. - } -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pnpm test src/service/AuthService.spec.ts -``` - -Expected: all pass, including legacy tests (which should already pass because the legacy `wos-auth-verifier` assertions were updated in Task 4). - -- [ ] **Step 5: Commit** - -```bash -git add src/service/AuthService.ts src/service/AuthService.spec.ts -git commit -m "feat(service): derive PKCE cookie name from URL state in callback - -handleCallback now reads and clears the flow-specific cookie identified -by the URL state parameter. Concurrent flows are fully isolated: -callback for flow A leaves flow B's cookie untouched." -``` - -### Task 6: Expose pure URL-generation methods - -**Files:** -- Modify: `src/service/AuthService.ts` (add three methods) -- Modify: `src/service/AuthService.spec.ts` - -- [ ] **Step 1: Write the failing tests** - -Add to `src/service/AuthService.spec.ts`: - -```ts -describe('AuthService — pure URL-generation methods', () => { - it('getAuthorizationUrl returns { url, cookieName } without touching storage', async () => { - const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); - const result = await authService.getAuthorizationUrl({ returnPathname: '/foo' }); - - expect(result.url).toMatch(/^https:\/\//); - expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - expect(setCookieSpy).not.toHaveBeenCalled(); - // Result must NOT include response or headers — pure URL generation. - expect(result).not.toHaveProperty('response'); - expect(result).not.toHaveProperty('headers'); - }); - - it('getSignInUrl returns { url, cookieName } with sign-in screen hint', async () => { - const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); - const result = await authService.getSignInUrl({ returnPathname: '/foo' }); - expect(result.url).toContain('screen_hint=sign-in'); - expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - expect(setCookieSpy).not.toHaveBeenCalled(); - }); - - it('getSignUpUrl returns { url, cookieName } with sign-up screen hint', async () => { - const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); - const result = await authService.getSignUpUrl(); - expect(result.url).toContain('screen_hint=sign-up'); - expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - expect(setCookieSpy).not.toHaveBeenCalled(); - }); -}); -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pnpm test src/service/AuthService.spec.ts -t 'pure URL-generation' -``` - -Expected: fail — methods don't exist. - -- [ ] **Step 3: Implement the three methods** - -In `src/service/AuthService.ts`, add after `clearPendingVerifier` (or near `createAuthorization`): - -```ts -/** - * Pure URL generation — returns the auth URL and the cookie name that - * WOULD be written by `createAuthorization`, but does NOT touch - * storage. Use this in adapter code paths where writing the verifier - * cookie is inappropriate (e.g. non-document requests in middleware - * hooks) — the browser ignores the cookie anyway because it won't - * follow a cross-origin redirect from fetch/XHR. - */ -async getAuthorizationUrl( - options: GetAuthorizationUrlOptions = {}, -): Promise<{ url: string; cookieName: string }> { - const { url, cookieName } = await this.operations.createAuthorization(options); - return { url, cookieName }; -} - -/** Pure variant of createSignIn — no cookie write. */ -async getSignInUrl( - options: Omit = {}, -): Promise<{ url: string; cookieName: string }> { - return this.getAuthorizationUrl({ ...options, screenHint: 'sign-in' }); -} - -/** Pure variant of createSignUp — no cookie write. */ -async getSignUpUrl( - options: Omit = {}, -): Promise<{ url: string; cookieName: string }> { - return this.getAuthorizationUrl({ ...options, screenHint: 'sign-up' }); -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pnpm test src/service/AuthService.spec.ts -t 'pure URL-generation' -``` - -Expected: all pass. - -- [ ] **Step 5: Commit** - -```bash -git add src/service/AuthService.ts src/service/AuthService.spec.ts -git commit -m "feat(service): add pure URL-generation methods - -getAuthorizationUrl, getSignInUrl, getSignUpUrl — same URL that -createAuthorization/createSignIn/createSignUp produce, but no cookie -write. Adapters with loop-prone paths (middleware hooks that fire -on every request) can use these for non-document requests to avoid -HTTP 431 cookie bloat." -``` - -### Task 7: `clearPendingVerifier` — breaking signature change - -**Files:** -- Modify: `src/service/AuthService.ts:280-289` -- Modify: `src/service/AuthService.spec.ts` - -- [ ] **Step 1: Update existing tests + add new tests** - -Find every `clearPendingVerifier` call in `src/service/AuthService.spec.ts` and update to pass `state`. New tests: - -```ts -describe('clearPendingVerifier — state-required', () => { - it('clears the flow-specific cookie derived from state', async () => { - const { cookieName } = await authService.createSignIn(undefined); - const sealedState = realStorage.cookies.get(cookieName)!; - - const clearCookieSpy = vi.spyOn(realStorage, 'clearCookie'); - await authService.clearPendingVerifier(undefined, { state: sealedState }); - - expect(clearCookieSpy).toHaveBeenCalledWith( - undefined, - cookieName, - expect.any(Object), - ); - }); - - it('threads redirectUri into the cookie options', async () => { - const { cookieName } = await authService.createSignIn(undefined, { - redirectUri: 'https://custom.example/cb', - }); - const sealedState = realStorage.cookies.get(cookieName)!; - - await authService.clearPendingVerifier(undefined, { - state: sealedState, - redirectUri: 'https://custom.example/cb', - }); - - expect(realStorage.lastClearOptions.get(cookieName)?.path).toBe('/'); - }); -}); -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pnpm test src/service/AuthService.spec.ts -t 'clearPendingVerifier' -``` - -Expected: fail — signature still takes only `{ redirectUri? }`. - -- [ ] **Step 3: Change the signature** - -In `src/service/AuthService.ts`, replace `clearPendingVerifier`: - -```ts -/** - * Emit a `Set-Cookie` header that clears the PKCE verifier cookie - * for the flow identified by `state`. - * - * **Breaking change in 0.5.0.** The `state` option is now required - * — the per-flow cookie naming scheme has no single "legacy" name - * to clear. Callers typically read `state` from the callback URL; - * when `state` is absent (malformed callback), do not call this - * method. The 10-minute PKCE TTL cleans up orphans. - * - * Pass `options.redirectUri` on requests that used a per-request - * `redirectUri` override at sign-in time, so the delete cookie's - * computed attributes (notably `secure`) match the original set. - */ -async clearPendingVerifier( - response: TResponse | undefined, - options: { state: string; redirectUri?: string }, -): Promise<{ response?: TResponse; headers?: HeadersBag }> { - const cookieName = getPKCECookieNameForState(options.state); - return this.storage.clearCookie( - response, - cookieName, - getPKCECookieOptions(this.config, options.redirectUri), - ); -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pnpm test src/service/AuthService.spec.ts -``` - -Expected: all pass. - -- [ ] **Step 5: Commit** - -```bash -git add src/service/AuthService.ts src/service/AuthService.spec.ts -git commit -m "feat(service)!: clearPendingVerifier requires state - -BREAKING CHANGE: clearPendingVerifier now takes -{ state: string; redirectUri?: string } (state required). The old -state-less form had no meaning in the per-flow cookie world — there -is no single cookie name to clear without knowing which flow. - -Callers on callback paths have URL state in hand. Bailouts with no -state should skip the call entirely; the 10-minute PKCE TTL handles -orphans." -``` - -### Task 8: Update exports in `src/index.ts` - -**Files:** -- Modify: `src/index.ts` - -- [ ] **Step 1: Add the new exports** - -At the bottom of `src/index.ts`, or near the `Storage Helpers` section, add: - -```ts -// ============================================ -// PKCE Helpers -// ============================================ -export { - PKCE_COOKIE_PREFIX, - getPKCECookieNameForState, -} from './core/pkce/cookieName.js'; -// Back-compat alias. Prefer PKCE_COOKIE_PREFIX or the derived names. -export { PKCE_COOKIE_NAME } from './core/pkce/cookieOptions.js'; -``` - -- [ ] **Step 2: Typecheck** - -```bash -pnpm typecheck -``` - -Expected: clean. - -- [ ] **Step 3: Build to verify bundle** - -```bash -pnpm build -``` - -Expected: clean. - -- [ ] **Step 4: Commit** - -```bash -git add src/index.ts -git commit -m "feat: export PKCE helpers from package root - -Exposes PKCE_COOKIE_PREFIX and getPKCECookieNameForState for custom -adapters. PKCE_COOKIE_NAME remains for back-compat." -``` - -### Task 9: Update `README.md` and `MIGRATION.md` - -**Files:** -- Modify: `README.md:197-260` -- Modify: `MIGRATION.md:27,148-162,239-240` - -- [ ] **Step 1: Update `README.md`** - -Locate the `clearPendingVerifier` example around line 197 and 256. Update each call signature to require `state`: - -In `README.md:197-210` (verifier section), replace the signature/example with: - -```ts -// After createSignIn/createSignUp has started a flow but handleCallback -// won't run (OAuth error response, missing code, early bailout), clear -// the pending verifier cookie. `state` is the sealed value returned on -// the callback URL — skip this call if the URL has no state. -authService.clearPendingVerifier(response, { - state, // required (from callback URL) - redirectUri?, // optional: match the per-call override used at sign-in -}); -``` - -At `README.md:256`, update the narrative example: - -```ts -if (state) { - await authService.clearPendingVerifier(response, { state }); -} -``` - -Also add a brief note near the URL-generation section documenting the new pure methods: - -```md -### Generate URLs without writing a cookie - -Use `getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl` when you need -the auth URL but don't want to write the PKCE verifier cookie (e.g. on -non-document requests in a middleware hook — browsers won't follow a -cross-origin redirect from XHR/fetch anyway). - -```ts -const { url, cookieName } = await authService.getSignInUrl({ returnPathname: '/dashboard' }); -// No Set-Cookie emitted. Use createSignIn if you want the cookie written. -``` -``` - -- [ ] **Step 2: Update `MIGRATION.md`** - -At `MIGRATION.md:27`, replace the `clearPendingVerifier(response)` callout with `clearPendingVerifier(response, { state })`. - -At `MIGRATION.md:148-162`, update the example block: - -```md -After `createSignIn`/`createSignUp` has started a flow but `handleCallback` -won't run to clear the verifier, call `clearPendingVerifier` with the -sealed `state` so the flow-specific cookie is deleted: - -```ts -if (state) { - const { headers } = await auth.clearPendingVerifier(response, { state }); - // ... -} -``` - -Skip the call when `state` is absent (malformed callback) — the -10-minute PKCE TTL handles orphans. -``` - -At `MIGRATION.md:239-240`, thread `state` into the example. - -Add a new top-level section near the top: - -```md -## 0.5.0 — per-flow PKCE cookies - -PKCE verifier cookies now carry a per-flow suffix -(`wos-auth-verifier-`) so concurrent sign-ins from multiple tabs -no longer clobber each other. `clearPendingVerifier` now **requires** -`options.state`. - -### What consumers need to change - -| Before | After | -| --- | --- | -| `auth.clearPendingVerifier(response)` | `auth.clearPendingVerifier(response, { state })` | -| `auth.clearPendingVerifier(response, { redirectUri })` | `auth.clearPendingVerifier(response, { state, redirectUri })` | - -Guard the call on `state` presence: - -```ts -if (state) { - await auth.clearPendingVerifier(response, { state }); -} -``` - -### New pure URL methods - -- `getAuthorizationUrl(options)` — returns `{ url, cookieName }`, writes no cookie. -- `getSignInUrl(options)` — same with `screenHint: 'sign-in'`. -- `getSignUpUrl(options)` — same with `screenHint: 'sign-up'`. - -Use these in adapter code paths where the cookie write is wasted (e.g. -non-document requests in a SvelteKit `handle` hook). Browsers don't -follow cross-origin redirects from fetch/XHR, so the cookie would never -be used anyway. -``` - -- [ ] **Step 3: Commit** - -```bash -git add README.md MIGRATION.md -git commit -m "docs: document per-flow PKCE cookies and clearPendingVerifier break - -Update README.md and MIGRATION.md to reflect the 0.5.0 signature -change on clearPendingVerifier and introduce the new pure URL- -generation methods." -``` - -### Task 10: Version bump and release commit - -**Files:** -- Modify: `package.json` -- Modify: `CHANGELOG.md` if present, else skip - -- [ ] **Step 1: Bump the version** - -Edit `package.json`: change `"version": "0.4.0"` to `"version": "0.5.0"`. - -- [ ] **Step 2: Run the full check** - -```bash -pnpm install -pnpm typecheck -pnpm test -pnpm build -``` - -All four must pass cleanly. If `build` produces a non-empty diff in `dist/`, include it in the commit. - -- [ ] **Step 3: Commit** - -```bash -git add package.json -git commit -m "chore: release 0.5.0 - -Per-flow PKCE verifier cookies. See docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md." -``` - -- [ ] **Step 4: Open PR and tag (user-driven)** - -After PR merges to `main`, tag: - -```bash -git tag v0.5.0 -git push origin v0.5.0 -``` - -Publish to npm per the repo's usual release process. Phase 1 complete. - ---- - -## Phase 2 — `authkit-sveltekit` - -Start in `/Users/nicknisi/Developer/authkit-sveltekit`. Fresh branch: `git checkout -b pkce-per-flow-cookies`. - -Phase 1 must be published to npm first so `pnpm install` resolves `@workos/authkit-session@^0.5.0`. - -### Task 11: Bump `@workos/authkit-session` dep - -**Files:** -- Modify: `package.json` - -- [ ] **Step 1: Bump the dependency** - -Edit `package.json`. Change the `@workos/authkit-session` entry under `dependencies` (or `peerDependencies`, whichever is present) from `^0.4.0` to `^0.5.0`. - -- [ ] **Step 2: Install** - -```bash -pnpm install -``` - -- [ ] **Step 3: Typecheck to see where the break hits** - -```bash -pnpm typecheck -``` - -Expected: a TS error at `src/server/auth.ts:118` about the `state` argument being required on `clearPendingVerifier`. This is the expected breakage — fixed in Task 14. - -- [ ] **Step 4: Commit the dep bump only** - -```bash -git add package.json pnpm-lock.yaml -git commit -m "chore(deps): bump @workos/authkit-session to ^0.5.0 - -Picks up per-flow PKCE cookies. Typecheck fails at -src/server/auth.ts:118 until clearPendingVerifier is updated to -pass { state }; follow-up commits wire it up." -``` - -### Task 12: `isDocumentRequest` helper - -**Files:** -- Create: `src/server/adapters/isDocumentRequest.ts` -- Create: `src/server/adapters/isDocumentRequest.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `src/server/adapters/isDocumentRequest.test.ts`: - -```ts -import { describe, it, expect } from 'vitest'; -import { isDocumentRequest } from './isDocumentRequest.js'; - -function h(entries: Record): Headers { - return new Headers(entries); -} - -describe('isDocumentRequest', () => { - it('returns true when Sec-Fetch-Dest is "document"', () => { - expect(isDocumentRequest(h({ 'sec-fetch-dest': 'document' }))).toBe(true); - }); - - it('returns false when Sec-Fetch-Dest is anything other than "document"', () => { - expect(isDocumentRequest(h({ 'sec-fetch-dest': 'empty' }))).toBe(false); - expect(isDocumentRequest(h({ 'sec-fetch-dest': 'script' }))).toBe(false); - expect(isDocumentRequest(h({ 'sec-fetch-dest': 'iframe' }))).toBe(false); - }); - - it('returns false for XMLHttpRequest even without Sec-Fetch-Dest', () => { - expect(isDocumentRequest(h({ 'x-requested-with': 'XMLHttpRequest' }))).toBe(false); - expect(isDocumentRequest(h({ 'x-requested-with': 'xmlhttprequest' }))).toBe(false); - }); - - it('returns false for prefetch requests', () => { - expect(isDocumentRequest(h({ purpose: 'prefetch' }))).toBe(false); - expect(isDocumentRequest(h({ purpose: 'Prefetch' }))).toBe(false); - }); - - it('returns false when Accept does not include text/html or */*', () => { - expect(isDocumentRequest(h({ accept: 'application/json' }))).toBe(false); - }); - - it('returns true when Accept includes text/html', () => { - expect( - isDocumentRequest(h({ accept: 'text/html,application/xhtml+xml' })), - ).toBe(true); - }); - - it('returns true when Accept is */*', () => { - expect(isDocumentRequest(h({ accept: '*/*' }))).toBe(true); - }); - - it('returns true for an empty header bag (fail-open)', () => { - expect(isDocumentRequest(h({}))).toBe(true); - }); -}); -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pnpm test src/server/adapters/isDocumentRequest.test.ts -``` - -Expected: module not found. - -- [ ] **Step 3: Implement** - -Create `src/server/adapters/isDocumentRequest.ts`: - -```ts -/** - * Best-effort detection of a top-level document navigation. - * - * Used by `createWithAuth` to decide whether to write a PKCE verifier - * cookie. Non-document requests (fetch/XHR/RSC/prefetch) can't follow - * a cross-origin redirect to WorkOS, so a cookie write on those - * requests is wasted and accumulates under the per-flow naming scheme - * — which can blow past browser per-host cookie budgets into HTTP 431. - * - * Fails open: when signals are ambiguous or absent, treat the request - * as a document. Worst case is one unneeded cookie bounded by the - * 10-minute PKCE TTL. - */ -export function isDocumentRequest(headers: Headers): boolean { - const dest = headers.get('sec-fetch-dest'); - if (dest) return dest === 'document'; - - if (headers.get('x-requested-with')?.toLowerCase() === 'xmlhttprequest') { - return false; - } - if (headers.get('purpose')?.toLowerCase() === 'prefetch') { - return false; - } - - const accept = headers.get('accept') ?? ''; - if (accept && !accept.includes('text/html') && !accept.includes('*/*')) { - return false; - } - - return true; -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pnpm test src/server/adapters/isDocumentRequest.test.ts -``` - -Expected: all pass. - -- [ ] **Step 5: Commit** - -```bash -git add src/server/adapters/isDocumentRequest.ts src/server/adapters/isDocumentRequest.test.ts -git commit -m "feat: add isDocumentRequest helper - -Header-based heuristic to detect top-level document navigations. -Fails open. Used next by createWithAuth to gate PKCE cookie writes." -``` - -### Task 13: `createWithAuth` gates PKCE cookie writes - -**Files:** -- Modify: `src/server/middleware.ts` -- Modify: existing `src/server/middleware.test.ts` if present — if missing, create it - -- [ ] **Step 1: Write the failing tests** - -Either append to an existing `middleware.test.ts` or create one. The test needs to spy on `authKitInstance.createSignIn` vs `getSignInUrl`: - -```ts -import { describe, it, expect, vi } from 'vitest'; -import { createWithAuth } from './middleware.js'; - -function buildAuthKit() { - return { - createSignIn: vi.fn().mockResolvedValue({ - url: 'https://auth.workos.com/sign-in', - cookieName: 'wos-auth-verifier-00000000', - response: new Response(), - headers: { 'Set-Cookie': ['wos-auth-verifier-00000000=abc; Path=/'] }, - }), - getSignInUrl: vi.fn().mockResolvedValue({ - url: 'https://auth.workos.com/sign-in', - cookieName: 'wos-auth-verifier-00000000', - }), - } as unknown as Parameters[0]; -} - -function event(headers: Record) { - return { - url: new URL('https://app.example/protected'), - request: new Request('https://app.example/protected', { headers }), - locals: { auth: { user: null } }, - } as unknown as Parameters>>[0]; -} - -describe('createWithAuth — document gating', () => { - it('calls createSignIn (with cookie) for document requests', async () => { - const ak = buildAuthKit(); - const withAuth = createWithAuth(ak); - const handler = withAuth(async () => ({ ok: true })); - - await handler(event({ 'sec-fetch-dest': 'document' })).catch(() => null); - expect(ak.createSignIn).toHaveBeenCalled(); - expect(ak.getSignInUrl).not.toHaveBeenCalled(); - }); - - it('calls getSignInUrl (no cookie) for XHR requests', async () => { - const ak = buildAuthKit(); - const withAuth = createWithAuth(ak); - const handler = withAuth(async () => ({ ok: true })); - - await handler(event({ 'sec-fetch-dest': 'empty' })).catch(() => null); - expect(ak.getSignInUrl).toHaveBeenCalled(); - expect(ak.createSignIn).not.toHaveBeenCalled(); - }); -}); -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -pnpm test src/server/middleware.test.ts -``` - -Expected: fail — `getSignInUrl` never called (current code always calls `createSignIn`). - -- [ ] **Step 3: Update `createWithAuth`** - -In `src/server/middleware.ts`, add the import: - -```ts -import { isDocumentRequest } from './adapters/isDocumentRequest.js'; -``` - -Replace the `!auth?.user` branch: - -```ts -if (!auth?.user) { - if (isDocumentRequest(event.request.headers)) { - const { url, response, headers } = await authKitInstance.createSignIn( - new Response(), - { returnPathname: event.url.pathname }, - ); - applyCookies(event, response, headers); - throw redirect(302, url); - } - - // Non-document request (fetch/XHR/RSC/prefetch). Browsers won't - // follow the cross-origin redirect to WorkOS from these, so a PKCE - // cookie write is wasted and — under per-flow naming — contributes - // to cookie-header bloat. The next real navigation from this client - // hits this branch with isDocumentRequest === true and gets the - // cookie then. - const { url } = await authKitInstance.getSignInUrl({ - returnPathname: event.url.pathname, - }); - throw redirect(302, url); -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -pnpm test src/server/middleware.test.ts -``` - -Expected: all pass. - -- [ ] **Step 5: Commit** - -```bash -git add src/server/middleware.ts src/server/middleware.test.ts -git commit -m "feat(middleware): gate PKCE cookie writes in createWithAuth - -Document requests take the createSignIn path (with cookie). Non- -document requests (fetch/XHR/RSC/prefetch) take getSignInUrl (no -cookie) — browsers don't follow cross-origin redirects from those -anyway, and under per-flow cookie naming, writing on every request -quickly overflows HTTP header limits (431)." -``` - -### Task 14: Thread `state` into `clearPendingVerifier` in `auth.ts` - -**Files:** -- Modify: `src/server/auth.ts:113-125` - -- [ ] **Step 1: Update the test fixture first (see Task 15)** - -Skip — Task 15 handles tests. Change code first. - -- [ ] **Step 2: Update the bail helper** - -In `src/server/auth.ts`, replace the bail function inside `createHandleCallback`: - -```ts -const bail = async (errCode: AuthErrorCode): Promise => { - const response = new Response(null, { - status: 302, - headers: { Location: `/auth/error?code=${errCode}` }, - }); - - // Only clear when we know which flow's cookie to delete. URL state - // is the flow key; if it's absent (malformed callback), skip — - // the 10-minute PKCE TTL handles orphans. - if (state) { - const { headers: deleteHeaders } = await authKitInstance.clearPendingVerifier( - new Response(), - { state }, - ); - appendHeaderBag(response.headers, deleteHeaders); - } - - return response; -}; -``` - -- [ ] **Step 3: Typecheck** - -```bash -pnpm typecheck -``` - -Expected: clean on auth.ts. (Tests still pending update — Task 15.) - -- [ ] **Step 4: Commit** - -```bash -git add src/server/auth.ts -git commit -m "fix(auth): thread state into clearPendingVerifier call site - -Required by authkit-session 0.5.0. Bailouts without state skip the -call — TTL-driven cleanup for the orphan." -``` - -### Task 15: Update test fixtures to use derived cookie names - -**Files:** -- Modify: `src/tests/get-sign-in-url.test.ts` -- Modify: `src/tests/handle-callback.test.ts` - -- [ ] **Step 1: Update `get-sign-in-url.test.ts`** - -At the top of the file, replace: - -```ts -const PKCE_COOKIE_NAME = 'wos-auth-verifier'; -``` - -with a helper import: - -```ts -import { getPKCECookieNameForState } from '@workos/authkit-session'; -``` - -At each call site that asserts on `PKCE_COOKIE_NAME` (lines 7-8, 54-84 per the spec), replace with `getPKCECookieNameForState(expectedSealedState)`. Where the existing setup has `setCookieValue = '${PKCE_COOKIE_NAME}=sealed-verifier; ...'`, compute the name from the known sealed-verifier fixture: - -```ts -const SEALED = 'sealed-verifier'; -const EXPECTED_NAME = getPKCECookieNameForState(SEALED); - -const setCookieValue = `${EXPECTED_NAME}=${SEALED}; Path=/; HttpOnly; Secure; SameSite=Lax; Max-Age=600`; -``` - -- [ ] **Step 2: Update `handle-callback.test.ts`** - -Same transformation. Replace: - -```ts -const PKCE_COOKIE_NAME = 'wos-auth-verifier'; -const VERIFIER_DELETE = `${PKCE_COOKIE_NAME}=; Path=/; Max-Age=0`; -``` - -with: - -```ts -import { getPKCECookieNameForState } from '@workos/authkit-session'; - -const SEALED = /* whatever sealed state the test sets up */; -const EXPECTED_NAME = getPKCECookieNameForState(SEALED); -const VERIFIER_DELETE = `${EXPECTED_NAME}=; Path=/; Max-Age=0`; -``` - -Some tests use different sealed-state fixtures per case; where that's true, compute the expected name inline per test. - -- [ ] **Step 3: Run the tests** - -```bash -pnpm test -``` - -Expected: all pass. - -- [ ] **Step 4: Typecheck and build** - -```bash -pnpm typecheck -pnpm build -``` - -Expected: clean. - -- [ ] **Step 5: Commit** - -```bash -git add src/tests/get-sign-in-url.test.ts src/tests/handle-callback.test.ts -git commit -m "test: derive expected PKCE cookie name from sealed state - -Fixtures previously hardcoded 'wos-auth-verifier' — they now derive -per-flow names via getPKCECookieNameForState to match the on-wire -reality post-0.5.0." -``` - -### Task 16: Release authkit-sveltekit 0.3.0 - -**Files:** -- Modify: `package.json` - -- [ ] **Step 1: Bump version** - -Edit `package.json`: `"version": "0.2.0"` → `"version": "0.3.0"`. - -- [ ] **Step 2: Full check** - -```bash -pnpm install -pnpm typecheck -pnpm test -pnpm build -``` - -- [ ] **Step 3: Commit** - -```bash -git add package.json -git commit -m "chore: release 0.3.0 - -Per-flow PKCE cookies via @workos/authkit-session@^0.5.0 + -document-request gating in createWithAuth." -``` - -- [ ] **Step 4: PR, merge, tag, publish** — user-driven, per the repo's normal process. - -Phase 2 complete. - ---- - -## Phase 3 — `authkit-tanstack-react-start` - -Start in `/Users/nicknisi/Developer/authkit-tanstack-start`. Fresh branch: `git checkout -b pkce-per-flow-cookies`. - -### Task 17: Bump dep and absorb the signature break - -**Files:** -- Modify: `package.json` - -- [ ] **Step 1: Bump the dep** - -Edit `package.json`. Change `@workos/authkit-session` from `^0.4.0` (or current) to `^0.5.0`. - -```bash -pnpm install -``` - -- [ ] **Step 2: Typecheck to list breakage** - -```bash -pnpm typecheck -``` - -Expected: TS errors at `src/server/server.ts:76-79` where `clearPendingVerifier` is called with `{ redirectUri? }` but now needs `state`. - -- [ ] **Step 3: Commit the dep bump** - -```bash -git add package.json pnpm-lock.yaml -git commit -m "chore(deps): bump @workos/authkit-session to ^0.5.0 - -Typecheck fails until clearPendingVerifier call sites are updated -and STATIC_FALLBACK_DELETE_HEADERS is replaced." -``` - -### Task 18: Thread `state` through `buildVerifierDeleteHeaders` and replace static fallback - -**Files:** -- Modify: `src/server/server.ts:7-10,73-88,100-108,144-176` - -- [ ] **Step 1: Write/update the failing tests** - -In `src/server/server.spec.ts`, find the tests that reference `STATIC_FALLBACK_DELETE_HEADERS` or that assert on `wos-auth-verifier=; Path=/; ...` deletes. Add: - -```ts -import { getPKCECookieNameForState } from '@workos/authkit-session'; - -describe('handleCallbackRoute — state-derived delete headers', () => { - it('emits a delete header whose cookie name matches getPKCECookieNameForState(state) when state is present', async () => { - const sealed = 'sealed-state-fixture'; - const expected = getPKCECookieNameForState(sealed); - const request = new Request( - `https://app.example/callback?code=bad&state=${encodeURIComponent(sealed)}`, - ); - // ... set up mocks so handleCallback throws, forcing errorResponse - const res = await handleCallbackRoute()({ request }); - const setCookies = res.headers.getSetCookie(); - expect(setCookies.some(c => c.startsWith(`${expected}=`))).toBe(true); - }); - - it('emits no Set-Cookie delete when state is absent', async () => { - const request = new Request('https://app.example/callback'); // no code, no state - const res = await handleCallbackRoute()({ request }); - const setCookies = res.headers.getSetCookie(); - expect(setCookies.some(c => c.includes('wos-auth-verifier'))).toBe(false); - }); -}); -``` - -Update any existing test that asserts the static header strings to instead compute via `getPKCECookieNameForState(sealedState)`. - -- [ ] **Step 2: Run tests to verify new ones fail** - -```bash -pnpm test src/server/server.spec.ts -``` - -Expected: new tests fail. - -- [ ] **Step 3: Refactor `src/server/server.ts`** - -Delete the `STATIC_FALLBACK_DELETE_HEADERS` const. Replace it and the two uses with a state-derived helper: - -```ts -import { - getPKCECookieNameForState, - type HeadersBag, -} from '@workos/authkit-session'; -import { getAuthkit } from './authkit-loader.js'; -import { getRedirectUriFromContext } from './auth-helpers.js'; -import { emitHeadersFrom } from './headers-bag.js'; -import type { HandleCallbackOptions } from './types.js'; - -/** - * Build Set-Cookie headers that delete the per-flow PKCE verifier - * cookie identified by `state`. When `state` is absent (malformed - * callback), return an empty list — the 10-minute TTL handles orphans. - */ -function deleteHeadersForState(state: string | null): readonly string[] { - if (!state) return []; - const name = getPKCECookieNameForState(state); - return [ - `${name}=; Path=/; HttpOnly; SameSite=Lax; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT`, - `${name}=; Path=/; HttpOnly; SameSite=None; Secure; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT`, - ]; -} -``` - -Refactor `buildVerifierDeleteHeaders` to take `state` and fall through to `deleteHeadersForState`: - -```ts -async function buildVerifierDeleteHeaders( - authkit: Awaited> | undefined, - state: string | null, -): Promise { - if (!state) return []; - if (!authkit) return deleteHeadersForState(state); - try { - const redirectUri = getRedirectUriFromContext(); - const { response, headers } = await authkit.clearPendingVerifier( - new Response(), - { state, ...(redirectUri ? { redirectUri } : {}) }, - ); - const fromResponse = response?.headers.getSetCookie?.() ?? []; - if (fromResponse.length > 0) return fromResponse; - const fromBag = headers?.['Set-Cookie']; - if (fromBag) return Array.isArray(fromBag) ? fromBag : [fromBag]; - return deleteHeadersForState(state); - } catch (error) { - console.error('[authkit-tanstack-react-start] clearPendingVerifier failed:', error); - return deleteHeadersForState(state); - } -} -``` - -Thread `state` through the call chain. In `handleCallbackInternal`: - -```ts -if (!code) { - return errorResponse(new Error('Missing authorization code'), request, options, authkit, state, 400); -} -``` - -And in `errorResponse`, add a `state: string | null` parameter and pass it to `buildVerifierDeleteHeaders`. - -Also update the comment block above `buildVerifierDeleteHeaders` — the claim that "PKCE cookie Path must match whatever redirectUri was used to set it" is stale; core hardcodes `path: '/'` now. Replace with: - -```ts -/** - * Extract the `Set-Cookie` header(s) produced by - * `authkit.clearPendingVerifier()` for the flow identified by `state`. - * - * Delete matching is on (name, domain, path); `path` is always `/` - * for PKCE cookies in authkit-session (see `getPKCECookieOptions`). - * When authkit setup itself failed, fall back to a state-derived - * header pair that covers both SameSite=Lax and SameSite=None set - * paths — browsers use (name, domain, path) for cookie replacement, - * not SameSite, so either variant deletes the original regardless of - * its original SameSite attribute. - */ -``` - -- [ ] **Step 4: Run tests** - -```bash -pnpm test -``` - -Expected: all pass. - -- [ ] **Step 5: Typecheck and build** - -```bash -pnpm typecheck -pnpm build -``` - -Expected: clean. - -- [ ] **Step 6: Commit** - -```bash -git add src/server/server.ts src/server/server.spec.ts -git commit -m "fix(server): state-derive PKCE delete headers - -Replaces STATIC_FALLBACK_DELETE_HEADERS with a dynamic helper that -computes cookie names from getPKCECookieNameForState(state). When -state is absent, emits no delete headers — the 10-minute TTL handles -orphans. Threads state through buildVerifierDeleteHeaders and -errorResponse. Also refreshes stale comment about Path tracking." -``` - -### Task 19: Update any other spec assertions on cookie names - -**Files:** -- Modify: `src/server/server-functions.spec.ts` (if it asserts on cookie names) - -- [ ] **Step 1: Grep for legacy cookie-name assertions** - -```bash -rg "wos-auth-verifier" src/ -``` - -- [ ] **Step 2: For each match in a test file, rewrite to derive via `getPKCECookieNameForState`** - -For example, if a test does `expect(setCookie).toContain('wos-auth-verifier=')`, change it to: - -```ts -import { getPKCECookieNameForState } from '@workos/authkit-session'; -// ... -const expected = getPKCECookieNameForState(sealedStateUsedInTest); -expect(setCookie).toContain(`${expected}=`); -``` - -- [ ] **Step 3: Run tests** - -```bash -pnpm test -``` - -Expected: all pass. - -- [ ] **Step 4: Commit (if any files changed)** - -```bash -git add src/ -git commit -m "test: derive expected PKCE cookie names from sealed state" -``` - -If the grep surfaced nothing, skip this commit. - -### Task 20: Release authkit-tanstack-react-start 0.7.0 - -**Files:** -- Modify: `package.json` - -- [ ] **Step 1: Bump version** - -Edit `package.json`: `"version": "0.6.0"` → `"version": "0.7.0"`. - -- [ ] **Step 2: Full check** - -```bash -pnpm install -pnpm typecheck -pnpm test -pnpm build -``` - -- [ ] **Step 3: Commit** - -```bash -git add package.json -git commit -m "chore: release 0.7.0 - -Per-flow PKCE cookies via @workos/authkit-session@^0.5.0. Static -fallback delete replaced with state-derived variant." -``` - -- [ ] **Step 4: PR, merge, tag, publish** — user-driven. - -Phase 3 complete. All three packages shipped with the per-flow cookie fix. - ---- - -## Self-review notes - -- **Spec coverage.** Every numbered section in the spec (1.1–1.6, 2.1–2.5, 3.1–3.4) has a corresponding task. Testing matrix (§Testing) is covered across Tasks 1, 4, 5, 6, 7, 12, 13, 15, 18, 19. -- **Placeholders.** None. Every code step includes the full code block. Test names are concrete; file paths are absolute or relative-from-repo-root with no ambiguity. -- **Type consistency.** `cookieName: string` is added to `GeneratedAuthorizationUrl` (Task 2) and `CreateAuthorizationResult` (Task 3) and used consistently in Tasks 4, 5, 6, 7. `clearPendingVerifier(response, { state, redirectUri? })` shape is identical across Tasks 7, 14, 18. `PKCE_COOKIE_PREFIX` and `getPKCECookieNameForState` exports in Task 8 match imports in Tasks 11+. -- **Breaking change fence.** `clearPendingVerifier` requires `state` after Task 7. Phase 2 and 3 consume `^0.5.0` only after Phase 1 ships, so the TS break happens inside the adapter repos (expected, fixed immediately in Tasks 14 and 18). diff --git a/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md b/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md deleted file mode 100644 index 7ddf014..0000000 --- a/docs/superpowers/specs/2026-04-22-per-flow-pkce-cookies-design.md +++ /dev/null @@ -1,270 +0,0 @@ -# Per-flow PKCE Verifier Cookies - -**Date:** 2026-04-22 -**Status:** Approved — ready for implementation plan -**Packages:** `authkit-session`, `authkit-sveltekit`, `authkit-tanstack-start` - -## Problem - -When a user has multiple tabs open against an app and the session expires (or multiple requests fire concurrently at an unauthenticated session), each request that hits the auth-url-minting code path generates a fresh PKCE verifier and writes it to the single shared `wos-auth-verifier` cookie. Last-write-wins: every callback except one fails with `OAuthStateMismatchError` or `PKCECookieMissingError`. - -This is the same bug `authkit-nextjs` fixed in PR [#403](https://github.com/workos/authkit-nextjs/pull/403). `authkit-session` and both downstream adapters (`authkit-sveltekit`, `authkit-tanstack-start`) currently share the bug because the cookie name is a module-level constant. - -### Evidence in code - -- `authkit-session/src/core/pkce/cookieOptions.ts:5` — `PKCE_COOKIE_NAME = 'wos-auth-verifier'` is a single constant. -- `authkit-session/src/service/AuthService.ts:234-239,324-325,366-369` — `createAuthorization` writes, `handleCallback` reads and clears, all against that one name. -- No concurrency isolation exists in the core or either adapter. - -## Goals - -1. Eliminate cross-flow clobbering by giving each concurrent PKCE flow its own cookie name. -2. Keep the fix additive in `authkit-session`'s public API except for a single deliberate breaking change to `clearPendingVerifier`. This IS a consumer-facing break — `clearPendingVerifier` is documented in the public README and MIGRATION guide as an API consumers call directly. The new required `state` parameter must be surfaced in both documents as part of this change. -3. Avoid HTTP 431 cookie bloat in middleware-loop-prone adapter paths. - -## Non-goals - -- No legacy cookie fallback. `authkit-session` has negligible real users mid-flow; the upgrade window isn't worth protecting. -- No orphan cleanup at `signOut`. `authkit-tanstack-start` hasn't shipped; `authkit-sveltekit` has no real users. The 10-minute PKCE TTL handles orphans. -- No shared document-request helper in the core. Header heuristics are adapter-specific. - -## Design - -### 1. `authkit-session` (the core library) - -#### 1.1 Cookie name derivation - -New file: `src/core/pkce/cookieName.ts`. - -```ts -export const PKCE_COOKIE_PREFIX = 'wos-auth-verifier'; - -function fnv1a32Hex(input: string): string { - let hash = 0x811c9dc5; - const bytes = new TextEncoder().encode(input); - for (const byte of bytes) { - hash = Math.imul(hash ^ byte, 0x01000193) >>> 0; - } - return hash.toString(16).padStart(8, '0'); -} - -export function getPKCECookieNameForState(state: string): string { - return `${PKCE_COOKIE_PREFIX}-${fnv1a32Hex(state)}`; -} -``` - -Inline FNV-1a 32-bit, no new dependency. Hash isn't security-sensitive — it's a namespacing mechanism. Collision probability is ~1/4B per pair; a collision would route one flow's callback to the wrong cookie, which then fails byte-equality against the URL state (fail-closed via existing `verifyCallbackState` logic). - -`PKCE_COOKIE_NAME` stays as an alias export for back-compat; internal code uses `PKCE_COOKIE_PREFIX`. - -#### 1.2 Return derived cookie name from URL generation - -`src/core/pkce/generateAuthorizationUrl.ts`: - -- `GeneratedAuthorizationUrl` gains `cookieName: string`, computed via `getPKCECookieNameForState(sealedState)`. -- The cookie byte-length guard uses the actual (longer) derived name — worst case 26 chars vs 17, still well under the 3800-byte cap. - -#### 1.3 Public pure URL-generation API - -Expose the side-effect-free path that `AuthOperations` already owns. Callers that don't want to write a cookie can bypass `storage.setCookie` entirely. - -`AuthService` gains three new methods: - -```ts -getAuthorizationUrl(options?): Promise<{ url: string; cookieName: string }> -getSignInUrl(options?): Promise<{ url: string; cookieName: string }> -getSignUpUrl(options?): Promise<{ url: string; cookieName: string }> -``` - -No response argument, no `Set-Cookie`, no `storage` touched. Thin wrappers around `AuthOperations.createAuthorization` / `createSignIn` / `createSignUp`. - -Existing `createAuthorization` / `createSignIn` / `createSignUp` continue to write the cookie, using the derived per-flow name internally. `CreateAuthorizationResult` gains `cookieName: string` for callers that want to assert on or log the name — it is **not** the shape `clearPendingVerifier` consumes (which takes `state`; see §1.5). - -No `writeCookie?: boolean` flag — rejected in favor of the cleaner URL-only methods. - -#### 1.4 `handleCallback` - -`AuthService.handleCallback` derives the cookie name from `options.state` (URL state) and uses it for both the cookie read and any subsequent clear. Pre-unseal error path uses the derived name. Post-unseal error path also uses the same derived name (cheaper than deriving from the unsealed blob, and equivalent by construction). - -Happy-path sketch: - -```ts -async handleCallback(request, response, options: { code, state }) { - const cookieName = options.state ? getPKCECookieNameForState(options.state) : null; - const cookieValue = cookieName - ? await this.storage.getCookie(request, cookieName) - : null; - - // verifyCallbackState throws OAuthStateMismatchError when state is missing - // and PKCECookieMissingError when cookieValue is null — existing behavior. - const unsealed = await this.core.verifyCallbackState({ - stateFromUrl: options.state, - cookieValue: cookieValue ?? undefined, - }); - // ... rest unchanged; clearCookie uses `cookieName` -} -``` - -Adapters require no change to their `handleCallback` call sites. - -#### 1.5 `clearPendingVerifier` — breaking signature change - -Today: - -```ts -clearPendingVerifier(response, options?: { redirectUri? }) -``` - -Becomes: - -```ts -clearPendingVerifier( - response, - options: { state: string; redirectUri?: string }, -) -``` - -Rationale: a state-less cleanup is meaningless in the per-flow world — we'd have no cookie name to clear. Rather than silently no-op and hide bugs, force callers to pass `state`. In-tree callers (`authkit-sveltekit/src/server/auth.ts:113-125`, `authkit-tanstack-start/src/server/server.ts:73-88,144-176`) are both callback bailout paths where URL `state` is available. - -This is a consumer-facing breaking change: `README.md:197,204,256` and `MIGRATION.md:27,148-162,239-240` document `clearPendingVerifier(response)` / `clearPendingVerifier(response, { redirectUri })` as a public API, so external adopters calling it in their own callback handlers will hit a TypeScript error on upgrade. The change is justified because the old signature encoded an invariant that no longer holds, but it must be called out in the migration guide. - -**Adapter rule for missing state.** URL `state` on a callback request can be absent in edge cases (e.g., a malformed request hitting the callback route directly). In that case adapters MUST NOT call `clearPendingVerifier` — there is no cookie to clear, and the 10-minute TTL handles any orphan. Each adapter's bailout path should guard: `if (state) await clearPendingVerifier(response, { state });`. - -#### 1.6 Exports - -New public exports from `src/index.ts`: - -- `getPKCECookieNameForState` -- `PKCE_COOKIE_PREFIX` - -Retained for back-compat: `PKCE_COOKIE_NAME` (aliased to the prefix). - -Not exported (deliberately): any document-request helper. Header heuristics are adapter-layer concerns. - -### 2. `authkit-sveltekit` - -#### 2.1 Adapter-local document-request helper - -Ship `src/server/adapters/isDocumentRequest.ts`: - -```ts -export function isDocumentRequest(headers: Headers): boolean { - const dest = headers.get('sec-fetch-dest'); - if (dest) return dest === 'document'; - if (headers.get('x-requested-with')?.toLowerCase() === 'xmlhttprequest') return false; - if (headers.get('purpose')?.toLowerCase() === 'prefetch') return false; - const accept = headers.get('accept') ?? ''; - if (accept && !accept.includes('text/html') && !accept.includes('*/*')) return false; - return true; -} -``` - -Fail-open: when ambiguous, assume document. Worst case is one extra cookie bounded by the 10-minute TTL. - -#### 2.2 `createWithAuth` — the loop-prone path - -`src/server/middleware.ts`: - -```ts -if (!auth?.user) { - if (isDocumentRequest(event.request.headers)) { - const { url, response, headers } = await authKitInstance.createSignIn( - new Response(), - { returnPathname: event.url.pathname }, - ); - applyCookies(event, response, headers); - throw redirect(302, url); - } - - // Non-document request (fetch/XHR/RSC/prefetch): browsers won't follow - // a cross-origin redirect to WorkOS anyway, so skip the cookie write to - // avoid bloat. The next real navigation from this client hits this - // branch with `isDocumentRequest === true`. - const { url } = await authKitInstance.getSignInUrl({ returnPathname: event.url.pathname }); - throw redirect(302, url); -} -``` - -#### 2.3 `createGetSignInUrl` / `createGetSignUpUrl` — no gating - -These are explicit user-triggered helpers called once per sign-in click. No middleware loop; no bloat risk. Per-flow names (from core) still protect against the multi-tab race. Leave untouched aside from ambient `cookieName` in the result type. - -#### 2.4 `clearPendingVerifier` call sites - -`src/server/auth.ts:118` currently calls `authKitInstance.clearPendingVerifier(new Response())` with no options; update to pass `{ state }` from the URL, guarded by the adapter rule in §1.5 (skip the call entirely if `state` is absent). `state` is already in scope at that call site as `url.searchParams.get('state') || undefined`. - -#### 2.5 Test fixture update - -`src/tests/get-sign-in-url.test.ts:7-8,54-84` and `src/tests/handle-callback.test.ts:10,12` hardcode `PKCE_COOKIE_NAME = 'wos-auth-verifier'`. Replace those constants with `getPKCECookieNameForState(sealedState)` derivations to match the new on-wire cookie name. - -### 3. `authkit-tanstack-start` - -#### 3.1 No gating - -Server functions (`getAuthorizationUrl`, `getSignInUrl`, `getSignUpUrl` in `src/server/server-functions.ts`) are XHR-invoked, but the client then navigates via `window.location.href = url`. Suppressing `Set-Cookie` on the XHR response would break sign-in: the subsequent navigation's callback would find no cookie. - -Always write the cookie. Per-flow names (from core) handle the concurrency correctness. Cookie bloat is self-limiting: one server-function call per user click. - -#### 3.2 `clearPendingVerifier` call sites - -`src/server/server.ts:73-88` wraps `clearPendingVerifier` inside `buildVerifierDeleteHeaders` and calls it with an optional `{ redirectUri }` — update to also pass `state`, guarded by the adapter rule in §1.5. `state` is in scope at `server.ts:102` and needs to be threaded into `buildVerifierDeleteHeaders`. - -#### 3.3 `STATIC_FALLBACK_DELETE_HEADERS` replacement - -`src/server/server.ts:7-10` hardcodes two static `Set-Cookie` delete headers for `wos-auth-verifier` (no suffix). Under per-flow names, those static deletes target a cookie that isn't set anymore. They're used in two branches: - -- `buildVerifierDeleteHeaders` failure path (`server.ts:84,87`) — when `clearPendingVerifier` throws. -- `errorResponse` when `getAuthkit()` itself fails before any authkit call (`server.ts:153`). - -Replace with a dynamic helper that derives the delete headers from `state` (when available). `getPKCECookieNameForState` is a pure function imported from `@workos/authkit-session`; it has no dependency on `getAuthkit()` and can safely run even when authkit setup has failed. - -When `state` is absent (malformed callback), emit **no** static delete headers. The 10-minute TTL handles orphans. - -#### 3.4 Stale comment cleanup - -`src/server/server.ts:62-71` still claims PKCE cookie `Path` tracks `redirectUri`. `authkit-session/src/core/pkce/cookieOptions.ts:57` hardcodes `path: '/'`. Fix the comments in the same PR that bumps the dep. - -## Testing - -### `authkit-session` - -- `getPKCECookieNameForState` — deterministic, different states produce different names, format matches `/^wos-auth-verifier-[0-9a-f]{8}$/`. -- `fnv1a32Hex` — known-answer tests against a reference implementation for at least 3 inputs. -- `AuthService.handleCallback` — two concurrent flows, two different cookies present, each callback picks its own cookie; the other remains untouched in the cleared Set-Cookie output. -- `AuthService.getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl` — return `{ url, cookieName }` and do **not** invoke `storage.setCookie` (spy on storage). -- `AuthService.createAuthorization` — returns `cookieName` in the result and writes under that exact name. -- `AuthService.clearPendingVerifier` — requires `state`; compile-time test via `expectTypeOf`; runtime test asserts the cleared cookie name matches `getPKCECookieNameForState(state)`. -- `handleCallback` pre-unseal error path clears the derived cookie, not the prefix-only name. - -### `authkit-sveltekit` - -- `isDocumentRequest` — matrix of header combinations (document, XHR, RSC, prefetch, bare, `*/*`, missing headers). -- `createWithAuth` — fires `createSignIn` (cookie write) for document requests; fires `getSignInUrl` (no cookie) for XHR. Spy on the instance methods. -- Callback-bailout sites correctly thread URL `state` into `clearPendingVerifier`. - -### `authkit-tanstack-start` - -- Existing tests continue to pass with per-flow cookie names (cookie name assertions need updating to derive via `getPKCECookieNameForState`). -- Callback-bailout sites correctly thread URL `state` into `clearPendingVerifier`. -- Static-fallback delete: with `state` present, asserts emitted headers match `getPKCECookieNameForState(state)`; with `state` absent, asserts no `Set-Cookie` delete headers are emitted. - -## Release - -Current versions (as of this spec): `authkit-session@0.4.0`, `authkit-sveltekit@0.2.0`, `authkit-tanstack-react-start@0.6.0`. - -Order: `authkit-session` → `authkit-sveltekit` → `authkit-tanstack-start`. - -- `authkit-session` → `0.5.0`. Minor bump (pre-1.0) covers the `clearPendingVerifier` signature break. Update `README.md` and `MIGRATION.md` in the same PR: document the new required `state` argument, add a "missing-state: skip the call" note, and mention the new `getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl` methods alongside existing surface. -- `authkit-sveltekit` → `0.3.0`. Minor bump: breaking behavior change inside `createWithAuth` (now gates cookie writes on document-request detection), and adapter-internal call-site updates for `clearPendingVerifier(state)`. -- `authkit-tanstack-react-start` → `0.7.0`. Minor bump: adapter-internal call-site updates for `clearPendingVerifier(state)` and replacement of `STATIC_FALLBACK_DELETE_HEADERS`. - -Consumers of the adapters see no migration work — the fix is transparent to application code. Consumers of `authkit-session` directly (e.g., custom adapter authors) must update any `clearPendingVerifier(response)` call to pass `{ state }`. - -## Rejected alternatives - -- **`writeCookie?: boolean` flag on `createAuthorization`.** Rejected: the pure/side-effect-full seam already exists in `AuthOperations` vs `AuthService`. Exposing a new pure public method is a cleaner API than a flag that toggles side effects. -- **Shared `isLikelyDocumentRequest` in `authkit-session`.** Rejected: header heuristics are adapter semantics, and the nextjs middleware model isn't shared by sveltekit or tanstack-start. Adapters implement whatever detection fits their request model. -- **Document-request gating in `authkit-tanstack-start`.** Rejected: server functions are XHR-invoked, but the client navigates via the returned URL. Suppressing `Set-Cookie` breaks sign-in. -- **Legacy cookie fallback in `handleCallback`.** Rejected: no meaningful at-risk user population; not worth the lingering code path. -- **`clearPendingVerifier` with optional `state` and warn+no-op.** Rejected: half-API that hides bugs. Require `state` or delete the method; we chose require. -- **Orphan cleanup on `signOut`.** Rejected: TTL (10 min) handles orphans; tanstack-start hasn't shipped and sveltekit has no real users. From eefa682f5e2999f978123e34d3e266c2026bdf58 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 18:50:00 -0700 Subject: [PATCH 19/21] chore: formatting: --- MIGRATION.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 4f7c053..61be15a 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -9,10 +9,10 @@ tabs no longer clobber each other. `clearPendingVerifier` now ### What consumers need to change -| Before | After | -| -------------------------------------------------------- | ----------------------------------------------------------------- | -| `auth.clearPendingVerifier(response)` | `auth.clearPendingVerifier(response, { state })` | -| `auth.clearPendingVerifier(response, { redirectUri })` | `auth.clearPendingVerifier(response, { state, redirectUri })` | +| Before | After | +| ------------------------------------------------------ | ------------------------------------------------------------- | +| `auth.clearPendingVerifier(response)` | `auth.clearPendingVerifier(response, { state })` | +| `auth.clearPendingVerifier(response, { redirectUri })` | `auth.clearPendingVerifier(response, { state, redirectUri })` | Guard the call on `state` presence: From ae3b78d86c14b4fe443ee43fce059bf6d5b14870 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 18:57:16 -0700 Subject: [PATCH 20/21] revert: drop pure URL-generation methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove `getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl`. The pure variants were added on speculation — no concrete consumer exists, and Greptile flagged a real footgun: a caller who uses them for a document-request redirect gets a broken callback with no surfaced `sealedState` to recover with. The "wasted cookie write on non-document requests" motivation is micro-optimization that adopters can solve via middleware hygiene (call `createSignIn` only on the actual sign-in route) rather than a parallel API surface. Per-flow cookie naming and the `clearPendingVerifier({ state })` breakage remain — those are the load-bearing fixes in 0.5.0. --- MIGRATION.md | 11 ------- README.md | 27 +---------------- src/service/AuthService.spec.ts | 52 --------------------------------- src/service/AuthService.ts | 30 ------------------- src/service/factory.spec.ts | 31 -------------------- src/service/factory.ts | 3 -- 6 files changed, 1 insertion(+), 153 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 61be15a..753f84b 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -25,17 +25,6 @@ if (state) { Skip the call entirely when `state` is absent (malformed callback) — the 10-minute PKCE TTL cleans up orphan cookies. -### New pure URL methods - -- `getAuthorizationUrl(options)` — returns `{ url, cookieName }`, writes no cookie. -- `getSignInUrl(options)` — same with `screenHint: 'sign-in'`. -- `getSignUpUrl(options)` — same with `screenHint: 'sign-up'`. - -Use these in adapter code paths where the cookie write is wasted -(e.g. non-document requests in a SvelteKit `handle` hook). Browsers -don't follow cross-origin redirects from fetch/XHR, so the cookie -would never be used anyway. - --- ## 0.3.x → 0.4.0 diff --git a/README.md b/README.md index 8f96f9c..f368dd2 100644 --- a/README.md +++ b/README.md @@ -194,16 +194,11 @@ authService.signOut(sessionId, { returnTo }) // → { logoutUrl, response?, authService.refreshSession(session, organizationId?) authService.switchOrganization(session, organizationId) -// URL Generation — write verifier cookie, return { url, response?, headers? } +// URL Generation — write verifier cookie, return { url, cookieName, response?, headers? } authService.createAuthorization(response, options) authService.createSignIn(response, options) authService.createSignUp(response, options) -// URL Generation — pure, return { url, cookieName } WITHOUT writing a cookie -authService.getAuthorizationUrl(options) -authService.getSignInUrl(options) -authService.getSignUpUrl(options) - // Error-path cleanup for the PKCE verifier cookie // `state` is required (from the callback URL) — it identifies which per-flow // verifier cookie to clear. Skip this call when `state` is absent from the @@ -212,26 +207,6 @@ authService.getSignUpUrl(options) authService.clearPendingVerifier(response, { state, redirectUri? }) ``` -### Generating URLs without writing a cookie - -`getAuthorizationUrl`, `getSignInUrl`, and `getSignUpUrl` return -`{ url, cookieName }` WITHOUT writing the PKCE verifier cookie. Use -these in adapter code paths where the cookie write is wasted — for -example, on non-document requests in a middleware hook. Browsers -don't follow cross-origin redirects from fetch/XHR/RSC/prefetch, so -a cookie write on those requests is noise. - -```ts -const { url, cookieName } = await authService.getSignInUrl({ - returnPathname: '/dashboard', -}); -// No Set-Cookie emitted. Use createSignIn if you want the cookie written. -``` - -For regular user-initiated sign-in flows, keep using `createSignIn` / -`createSignUp` / `createAuthorization` — they write the cookie and -return it alongside the URL. - ### PKCE verifier cookie (`wos-auth-verifier-`) This library binds every OAuth sign-in to a PKCE code verifier, so a leaked diff --git a/src/service/AuthService.spec.ts b/src/service/AuthService.spec.ts index b7a453a..c69146c 100644 --- a/src/service/AuthService.spec.ts +++ b/src/service/AuthService.spec.ts @@ -325,58 +325,6 @@ describe('AuthService', () => { }); }); - describe('AuthService — pure URL-generation methods', () => { - it('getAuthorizationUrl returns { url, cookieName } without touching storage', async () => { - const realStorage = makeStorage(); - const authService = new AuthService( - mockConfig as any, - realStorage as any, - makeClient() as any, - sessionEncryption, - ); - const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); - const result = await authService.getAuthorizationUrl({ - returnPathname: '/foo', - }); - - expect(result.url).toMatch(/^https:\/\//); - expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - expect(setCookieSpy).not.toHaveBeenCalled(); - expect(result).not.toHaveProperty('response'); - expect(result).not.toHaveProperty('headers'); - }); - - it('getSignInUrl returns { url, cookieName } with sign-in screen hint', async () => { - const realStorage = makeStorage(); - const authService = new AuthService( - mockConfig as any, - realStorage as any, - makeClient() as any, - sessionEncryption, - ); - const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); - const result = await authService.getSignInUrl({ returnPathname: '/foo' }); - expect(result.url).toContain('screen_hint=sign-in'); - expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - expect(setCookieSpy).not.toHaveBeenCalled(); - }); - - it('getSignUpUrl returns { url, cookieName } with sign-up screen hint', async () => { - const realStorage = makeStorage(); - const authService = new AuthService( - mockConfig as any, - realStorage as any, - makeClient() as any, - sessionEncryption, - ); - const setCookieSpy = vi.spyOn(realStorage, 'setCookie'); - const result = await authService.getSignUpUrl(); - expect(result.url).toContain('screen_hint=sign-up'); - expect(result.cookieName).toMatch(/^wos-auth-verifier-[0-9a-f]{8}$/); - expect(setCookieSpy).not.toHaveBeenCalled(); - }); - }); - describe('clearPendingVerifier()', () => { it('emits a delete cookie with Path=/', async () => { const realStorage = makeStorage(); diff --git a/src/service/AuthService.ts b/src/service/AuthService.ts index 1a2bae2..ad11ce4 100644 --- a/src/service/AuthService.ts +++ b/src/service/AuthService.ts @@ -264,36 +264,6 @@ export class AuthService { }); } - /** - * Pure URL generation — returns the auth URL and the cookie name that - * WOULD be written by `createAuthorization`, but does NOT touch - * storage. Use this in adapter code paths where writing the verifier - * cookie is inappropriate (e.g. non-document requests in middleware - * hooks) — the browser ignores the cookie anyway because it won't - * follow a cross-origin redirect from fetch/XHR. - */ - async getAuthorizationUrl( - options: GetAuthorizationUrlOptions = {}, - ): Promise<{ url: string; cookieName: string }> { - const { url, cookieName } = - await this.operations.createAuthorization(options); - return { url, cookieName }; - } - - /** Pure variant of createSignIn — no cookie write. */ - async getSignInUrl( - options: Omit = {}, - ): Promise<{ url: string; cookieName: string }> { - return this.getAuthorizationUrl({ ...options, screenHint: 'sign-in' }); - } - - /** Pure variant of createSignUp — no cookie write. */ - async getSignUpUrl( - options: Omit = {}, - ): Promise<{ url: string; cookieName: string }> { - return this.getAuthorizationUrl({ ...options, screenHint: 'sign-up' }); - } - /** * Emit a `Set-Cookie` header that clears the PKCE verifier cookie * for the flow identified by `state`. diff --git a/src/service/factory.spec.ts b/src/service/factory.spec.ts index ff9f421..3563972 100644 --- a/src/service/factory.spec.ts +++ b/src/service/factory.spec.ts @@ -43,9 +43,6 @@ describe('createAuthService', () => { expect(typeof service.createAuthorization).toBe('function'); expect(typeof service.createSignIn).toBe('function'); expect(typeof service.createSignUp).toBe('function'); - expect(typeof service.getAuthorizationUrl).toBe('function'); - expect(typeof service.getSignInUrl).toBe('function'); - expect(typeof service.getSignUpUrl).toBe('function'); expect(typeof service.clearPendingVerifier).toBe('function'); expect(typeof service.getWorkOS).toBe('function'); expect(typeof service.handleCallback).toBe('function'); @@ -80,34 +77,6 @@ describe('createAuthService', () => { // Factory accepts the custom encryption - verify by checking the interface exists expect(typeof service.withAuth).toBe('function'); }); - - it('forwards pure URL helpers through the factory proxy', async () => { - const customClient = { - pkce: { - generate: async () => ({ - codeVerifier: 'verifier', - codeChallenge: 'challenge', - codeChallengeMethod: 'S256', - }), - }, - userManagement: { - getAuthorizationUrl: () => 'https://example.com/authorize', - getJwksUrl: () => 'https://custom.example.com/jwks', - }, - }; - - const service = createAuthService({ - sessionStorageFactory: () => mockStorage as any, - clientFactory: () => customClient as any, - }); - - const result = await service.getSignInUrl({ - returnPathname: '/dashboard', - }); - - expect(result.url).toBe('https://example.com/authorize'); - expect(result.cookieName).toMatch(/^wos-auth-verifier-/); - }); }); describe('lazy initialization', () => { diff --git a/src/service/factory.ts b/src/service/factory.ts index 278994d..90f3399 100644 --- a/src/service/factory.ts +++ b/src/service/factory.ts @@ -76,9 +76,6 @@ export function createAuthService(options: { getService().createAuthorization(response, opts), createSignIn: (response, opts) => getService().createSignIn(response, opts), createSignUp: (response, opts) => getService().createSignUp(response, opts), - getAuthorizationUrl: opts => getService().getAuthorizationUrl(opts), - getSignInUrl: opts => getService().getSignInUrl(opts), - getSignUpUrl: opts => getService().getSignUpUrl(opts), clearPendingVerifier: (response, opts) => getService().clearPendingVerifier(response, opts), getWorkOS: () => getService().getWorkOS(), From 5659675262f5278bf2a88eebfdfcc9e02144eb62 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Wed, 22 Apr 2026 22:12:08 -0700 Subject: [PATCH 21/21] chore: drop PKCE_COOKIE_NAME and GetAuthorizationUrlResult Both are dead weight after the 0.5.0 changes: - `PKCE_COOKIE_NAME` pointed at `'wos-auth-verifier'`, which no cookie on the wire has anymore. Nothing in `src/` uses it. Verified neither authkit-tanstack-start nor authkit-sveltekit imports it. - `GetAuthorizationUrlResult` was a one-field interface that only existed as a shared base for the pure URL helpers we reverted. Inlined into `CreateAuthorizationResult`. Per-flow cookie name is still derivable via `PKCE_COOKIE_PREFIX` + `getPKCECookieNameForState(state)`. --- MIGRATION.md | 5 +++++ src/core/pkce/cookieOptions.ts | 3 --- src/core/session/types.ts | 5 +---- src/index.ts | 2 -- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 753f84b..51b8e4a 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -25,6 +25,11 @@ if (state) { Skip the call entirely when `state` is absent (malformed callback) — the 10-minute PKCE TTL cleans up orphan cookies. +### Removed exports + +- `PKCE_COOKIE_NAME` is gone. The wire cookie is now per-flow (`wos-auth-verifier-`), so a single static name no longer identifies anything. Use `PKCE_COOKIE_PREFIX` if you need the stable prefix, or `getPKCECookieNameForState(state)` to derive the per-flow name. +- `GetAuthorizationUrlResult` type is gone. The fields are inlined into `CreateAuthorizationResult`, which is what `createAuthorization` / `createSignIn` / `createSignUp` return. + --- ## 0.3.x → 0.4.0 diff --git a/src/core/pkce/cookieOptions.ts b/src/core/pkce/cookieOptions.ts index c3fcdd4..038387a 100644 --- a/src/core/pkce/cookieOptions.ts +++ b/src/core/pkce/cookieOptions.ts @@ -1,9 +1,6 @@ import type { AuthKitConfig } from '../config/types.js'; import type { CookieOptions } from '../session/types.js'; -/** Name of the PKCE verifier cookie on the wire. */ -export const PKCE_COOKIE_NAME = 'wos-auth-verifier'; - /** * PKCE verifier cookie lifetime (seconds). Matches the 10-minute convention * used by Arctic, openid-client, Clerk, and Okta for short-lived OAuth state diff --git a/src/core/session/types.ts b/src/core/session/types.ts index 511ef0d..9b4b932 100644 --- a/src/core/session/types.ts +++ b/src/core/session/types.ts @@ -176,11 +176,8 @@ export interface SessionEncryption { * The verifier cookie is written internally via `SessionStorage.setCookie` — callers * only need to redirect the browser to `url` and apply any returned `headers`/`response`. */ -export interface GetAuthorizationUrlResult { +export type CreateAuthorizationResult = { url: string; -} - -export type CreateAuthorizationResult = GetAuthorizationUrlResult & { response?: TResponse; headers?: HeadersBag; /** diff --git a/src/index.ts b/src/index.ts index b4f16ed..f0fa4e8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -44,8 +44,6 @@ export { PKCE_COOKIE_PREFIX, getPKCECookieNameForState, } from './core/pkce/cookieName.js'; -// Back-compat alias. Prefer PKCE_COOKIE_PREFIX or the derived names via getPKCECookieNameForState. -export { PKCE_COOKIE_NAME } from './core/pkce/cookieOptions.js'; // ============================================ // Encryption Fallback