Conversation
Every sign-in now generates a PKCE verifier, seals it alongside
returnPathname + customState + a nonce into a single blob, and uses that
blob as BOTH the OAuth `state` query param AND the value of a short-lived
`wos-auth-verifier` cookie. On callback, the adapter passes the cookie
value; the library byte-compares it against the URL state before
decrypting and then threads `codeVerifier` into `authenticateWithCode`.
A leaked state value on its own can no longer be used to complete a
session hijack.
Breaking changes (minor bump, pre-1.0):
- `getAuthorizationUrl` / `getSignInUrl` / `getSignUpUrl` now return
`{ url, sealedState, cookieOptions }` instead of a bare URL string.
- `handleCallback` requires a new `cookieValue: string | undefined` field.
- `SessionEncryption.unsealData` interface gains optional `ttl`.
- The `{internal}.{userState}` split-state format is removed; customState
now lives inside the sealed blob and round-trips byte-identically.
New module: `src/core/pkce/` — constants, state schema, seal/unseal,
cookie options, and orchestrator. New errors: `OAuthStateMismatchError`,
`PKCECookieMissingError`. New AuthService delegators:
`getPKCECookieOptions`, `buildPKCEDeleteCookieHeader`.
Coverage lifted to 90/82/85/90 (lines/branches/functions/statements) with
new tests across pkce.spec, state.spec, cookieOptions.spec, and extended
ironWebcryptoEncryption.spec + AuthService.spec + AuthOperations.spec.
- `getPKCECookieOptions`: collapse redundant three-way ternary. Since `configuredSameSite ?? 'lax'` already defaults non-'none' values to 'lax', the explicit `strict → lax` branch was dead. Replaced with a single `none ? 'none' : 'lax'` check and an inline comment documenting the `strict → lax` downgrade intent. - `AuthService.getPKCECookieOptions`: strip JSDoc paragraph that restated what the method name and one-line delegator body already convey. No behavior change. 194/194 tests pass.
…-level TTL Two defense-in-depth fixes for the PKCE verifier cookie: 1. Scope `wos-auth-verifier` cookie `Path` to the redirect URI's pathname instead of `/`. Multiple AuthKit apps on the same host under different subpaths no longer collide on a shared cookie slot — starting sign-in in one app no longer overwrites another's in-flight verifier. Falls back to `/` if the redirect URI is missing or invalid. 2. Embed `issuedAt` in the sealed PKCE state payload and enforce age in `unsealState` independently of the `SessionEncryption` adapter's `ttl` handling. Custom encryptors may legally ignore the optional `ttl` option; the payload-level check is the authoritative 600s guard. No breaking changes to the public surface: `PKCECookieOptions.path` is widened from the literal `'/'` to `string`; `PKCEState` gains `issuedAt` (consumers destructure the fields they need).
Strict `issuedAt > Date.now()` rejection broke multi-node deployments
where sign-in on node A and callback on node B could see node B's clock
slightly behind, producing a negative `ageMs` and a spurious
`SessionEncryptionError('PKCE state expired')`.
Allow up to 60s of negative skew — matches iron-webcrypto's default
`timestampSkewSec`. Payloads meaningfully in the future (> 60s) still
fail closed.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the OAuth flow by binding the OAuth state parameter to a short-lived PKCE verifier cookie, so callbacks must present both the URL state and the matching cookie before the code exchange proceeds.
Changes:
- Introduces PKCE state sealing/unsealing with TTL enforcement and new typed errors for state mismatch / missing cookie.
- Updates URL-generation APIs to return
{ url, sealedState, cookieOptions }and updates callback handling to requirecookieValue. - Adds PKCE cookie helpers (options + delete header builder) and updates public exports + docs/tests accordingly.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/service/factory.ts | Exposes new PKCE cookie helper methods on the service factory output. |
| src/service/factory.spec.ts | Verifies the factory output includes the new helper functions. |
| src/service/AuthService.ts | Adopts the new URL-generation result shape and enforces cookie-bound callback verification. |
| src/service/AuthService.spec.ts | Updates tests for the new URL result shape + cookie-bound callback flow and errors. |
| src/operations/AuthOperations.ts | Switches authorization URL generation to PKCE-backed generateAuthorizationUrl and returns the new triple shape. |
| src/operations/AuthOperations.spec.ts | Adds assertions for sealed state, cookie options, and PKCE params in the generated URL. |
| src/index.ts | Exports new PKCE helpers/constants and new error types as part of the public API. |
| src/core/session/types.ts | Extends SessionEncryption to support optional ttl and adds PKCE-related public types. |
| src/core/pkce/state.ts | Adds sealed PKCE state format with runtime validation + TTL enforcement. |
| src/core/pkce/state.spec.ts | Adds coverage for state round-tripping, tamper detection, schema validation, and TTL behavior. |
| src/core/pkce/pkce.spec.ts | Adds end-to-end PKCE round-trip coverage through generateAuthorizationUrl and verifyCallbackState. |
| src/core/pkce/index.ts | Creates a PKCE submodule export surface for internal/public consumption. |
| src/core/pkce/generateAuthorizationUrl.ts | Implements PKCE URL generation returning { url, sealedState, cookieOptions }. |
| src/core/pkce/cookieOptions.ts | Implements PKCE cookie option derivation + Set-Cookie serialization helper. |
| src/core/pkce/cookieOptions.spec.ts | Adds unit tests for cookie option derivation and Set-Cookie serialization. |
| src/core/pkce/constants.ts | Introduces PKCE cookie name and TTL constants. |
| src/core/errors.ts | Adds OAuthStateMismatchError and PKCECookieMissingError. |
| src/core/errors.spec.ts | Adds tests validating new error types and inheritance. |
| src/core/encryption/ironWebcryptoEncryption.ts | Adds TTL support to unsealData (and already supports TTL in sealing). |
| src/core/encryption/ironWebcryptoEncryption.spec.ts | Adds tests validating TTL enforcement behavior. |
| src/core/AuthKitCore.ts | Adds verifyCallbackState that checks state vs cookie before decrypting/unsealing. |
| src/core/AuthKitCore.spec.ts | Adds tests for verifyCallbackState error cases and refresh behaviors. |
| pnpm-lock.yaml | Locks the new valibot dependency. |
| package.json | Bumps version to 0.4.0 and adds valibot. |
| README.md | Documents the new PKCE cookie-bound flow, updated APIs, and adapter responsibilities. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…w intent - Use crypto.timingSafeEqual for state-vs-cookie compare to avoid timing side-channels (with length gate). - Document cookieOptions `secure` forcing when sameSite=none. - Expand PKCE_CLOCK_SKEW_MS docs: asymmetric by design (negative skew for multi-node drift, strict upper bound keeps payload check authoritative). - Fix misleading state.spec test comment: assert the real 601s boundary, not a fictional 660s grace window.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Generalizes the OAuth sign-in flow to mint a sealed PKCE state blob per request and bind it to a short-lived
wos-auth-verifiercookie. The callback verifies the URLstateagainst the cookie before decrypting and exchanging the code.getAuthorizationUrl/getSignInUrl/getSignUpUrlreturn{ url, sealedState, cookieOptions }— adapters set the cookie and redirect.handleCallbackaccepts acookieValueand performs the byte-compare + unseal internally.OAuthStateMismatchError,PKCECookieMissingError(in addition to the existingSessionEncryptionError).wos-auth-verifiercookie isHttpOnly,SameSite=Lax(downgraded fromStrict),Max-Age=600,Pathscoped to the redirect URI's pathname so multiple AuthKit apps on the same host under different subpaths don't collide.issuedAt;unsealStateenforces the 600s TTL independently of theSessionEncryptionadapter'sttlhandling (with 60s clock-skew tolerance for multi-node deploys).decodeStatehelper — state is no longer a cleartext return-pathname carrier.Bumps to
0.4.0.Breaking changes
getAuthorizationUrl/getSignInUrl/getSignUpUrlreturn shape changed fromstringto{ url, sealedState, cookieOptions }. Adapters must set the cookie and redirect tourl.handleCallbacknow requirescookieValue: string | undefined. Adapters must readwos-auth-verifierfrom the request cookies and pass it through.decodeStateis removed. Return-pathname handling moves intohandleCallback's return value.PKCEStategainsissuedAt: number. Destructured consumers are unaffected.Test plan
pnpm test— 203/203 pass across core, service, and PKCE round-trip specspnpm run typecheckcleanpnpm run lintclean (oxlint)pnpm run format:checkclean (oxfmt)authkit-tanstack-react-startexample app: happy path + missing-cookie drill