feat: add DPoP core storage layer (ENG-4782) - #201
Merged
mrudatsprint merged 8 commits intoJul 20, 2026
Merged
Conversation
- Add `dpop` v2.1.1 runtime dependency and `fake-indexeddb` devDependency to packages/core/package.json - SDKConfig: add optional `useDpop` and `dpopTokenStorage` fields - UrlHelper: add `getAuthorizeUrl(state?, dpopJkt?, codeChallenge?)` targeting FusionAuth /oauth2/authorize directly; update UrlHelperTypes to include response_type, code_challenge, code_challenge_method, dpop_jkt - DPoPStorage: IndexedDB abstraction for ES256 CryptoKeyPair persistence (db: fusionauth-sdk:dpop, store: keypair, keyed by clientId) - DPoPTokenStore: localStorage/memory token storage for DPoP-bound tokens (key: fusionauth-sdk:tokens:<clientId>); includes getAccessToken() and isExpired getter - packages/core/src/DPoP/index.ts re-exports both classes - 54 tests passing (21 DPoPTokenStore, 6 DPoPStorage, 16 UrlHelper, 7 SDKCore, 4 CookieHelpers)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the core building blocks needed to support DPoP mode in @fusionauth-sdk/core, adding a persistent keypair store (IndexedDB), a token store (localStorage/memory), and an authorize URL helper for DPoP + PKCE flows.
Changes:
- Added DPoP keypair persistence via IndexedDB (
DPoPStorage) and corresponding tests. - Added a DPoP token storage abstraction (
DPoPTokenStore) with localStorage/memory backends and tests. - Extended
UrlHelperto generate a direct/oauth2/authorizeURL for DPoP/PKCE, plus type/test updates and public exports.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/UrlHelper/UrlHelperTypes.ts | Expands query param typing to include DPoP + PKCE authorize parameters. |
| packages/core/src/UrlHelper/UrlHelper.ts | Adds getAuthorizeUrl for direct FusionAuth /oauth2/authorize URL generation. |
| packages/core/src/UrlHelper/UrlHelper.test.ts | Adds coverage for getAuthorizeUrl query string construction. |
| packages/core/src/SDKConfig/SDKConfig.ts | Introduces DPoP opt-in configuration (useDpop, dpopTokenStorage). |
| packages/core/src/index.ts | Exposes the new DPoP module from the package entrypoint. |
| packages/core/src/DPoP/index.ts | Barrel exports for DPoP storage/token store APIs. |
| packages/core/src/DPoP/DPoPTokenStore.ts | Implements token persistence abstraction for DPoP mode. |
| packages/core/src/DPoP/DPoPTokenStore.test.ts | Adds tests for localStorage and memory token persistence behavior. |
| packages/core/src/DPoP/DPoPStorage.ts | Implements IndexedDB-backed persistence for DPoP keypairs. |
| packages/core/src/DPoP/DPoPStorage.test.ts | Adds IndexedDB persistence tests using fake-indexeddb. |
| packages/core/package.json | Adds dpop runtime dependency and fake-indexeddb dev dependency. |
| AGENTS.md | Adds repo agent guidance (workspace layout, testing notes, and conventions). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove 'as any' cast in catch block — reject() accepts unknown directly - Add tests for indexedDB unavailable (SSR/non-browser): all three public methods (getKeyPair, setKeyPair, clearKeyPair) reject with a descriptive error - Add test for indexedDB.open() throwing synchronously (e.g. security policy block)
mrudatsprint
marked this pull request as ready for review
July 16, 2026 01:22
mrudatsprint
changed the base branch from
miker/dpop-in-the-javascript-sdk
to
main
July 16, 2026 01:22
mrudatsprint
changed the base branch from
main
to
miker/dpop-in-the-javascript-sdk
July 16, 2026 01:24
All three DPoPStorage methods (getKeyPair, setKeyPair, clearKeyPair) now resolve on tx.oncomplete and reject on tx.onerror / tx.onabort. Previously, resolving on req.onsuccess meant the caller was told 'success' before the transaction had fully committed — a transaction abort occurring after the request succeeded (e.g. quota exceeded) would go undetected. Applies the same fix consistently to all three methods, including getKeyPair (readonly, lower risk, but now consistent) and setKeyPair (readwrite, same durability concern as clearKeyPair). Adds a test that aborts a clearKeyPair transaction synchronously inside the request onsuccess handler and verifies the promise rejects and the key pair is still present in IndexedDB.
mrudatsprint
marked this pull request as draft
July 16, 2026 01:58
mrudatsprint
changed the base branch from
miker/dpop-in-the-javascript-sdk
to
main
July 16, 2026 01:58
mrudatsprint
marked this pull request as ready for review
July 16, 2026 14:40
mrudatsprint
commented
Jul 16, 2026
mrudatsprint
left a comment
mrudatsprint
left a comment
Collaborator
Author
There was a problem hiding this comment.
self-review completed
mrudatsprint
changed the base branch from
main
to
miker/dpop-in-the-javascript-sdk
July 16, 2026 15:54
lyleschemmerling
left a comment
lyleschemmerling
left a comment
There was a problem hiding this comment.
The use of the indexedDB looks good. I am still not sold that this DPoP library is worth adding the dependency. Have you looked at what it would take to do without it?
Collaborator
Author
The functionality to be implemented if we didn't use the library:
I believe this can be implemented in 1-2 days. Would we want to perform cross-validation testing against the panva/dpop library as a sanity check that we're building the thumbprint and proof correctly? Also, integration testing against FusionAuth will give us the best validation. Thus, end to end tests would be key. The RFC has an example to validate against. |
…g object - Export DEFAULT_DPOP_DB_NAME, DEFAULT_DPOP_DB_VERSION, DEFAULT_DPOP_STORE_NAME as named constants (no hardcoded magic strings anywhere in the codebase) - Add DPoPStorageConfig interface with clientId (required) and optional dbName, dbVersion, storeName fields — each defaults to the exported constant - Refactor DPoPStorage constructor from positional (clientId: string) to config object, matching the UrlHelperConfig convention in this monorepo - openDb() and all three public methods now reference instance fields (this.dbName, this.dbVersion, this.storeName) instead of module constants - Add tests: defaults apply when no config overrides provided; custom dbName and storeName land data in the right database; two instances with different dbNames but the same clientId do not share keys; dbVersion downgrade produces a clean rejection (VersionError) - Update AGENTS.md: note the config-object constructor convention and the IndexedDB dbVersion must-only-increase constraint
lyleschemmerling
approved these changes
Jul 20, 2026
mrudatsprint
merged commit Jul 20, 2026
f5b1143
into
parent/dpop-in-the-javascript-sdk
4 checks passed
mrudatsprint
added a commit
that referenced
this pull request
Aug 5, 2026
* feat: add DPoP core storage layer (ENG-4782) (#201)
* feat: add DPoP core storage layer (ENG-4782)
- Add `dpop` v2.1.1 runtime dependency and `fake-indexeddb` devDependency
to packages/core/package.json
- SDKConfig: add optional `useDpop` and `dpopTokenStorage` fields
- UrlHelper: add `getAuthorizeUrl(state?, dpopJkt?, codeChallenge?)`
targeting FusionAuth /oauth2/authorize directly; update UrlHelperTypes
to include response_type, code_challenge, code_challenge_method, dpop_jkt
- DPoPStorage: IndexedDB abstraction for ES256 CryptoKeyPair persistence
(db: fusionauth-sdk:dpop, store: keypair, keyed by clientId)
- DPoPTokenStore: localStorage/memory token storage for DPoP-bound tokens
(key: fusionauth-sdk:tokens:<clientId>); includes getAccessToken() and
isExpired getter
- packages/core/src/DPoP/index.ts re-exports both classes
- 54 tests passing (21 DPoPTokenStore, 6 DPoPStorage, 16 UrlHelper, 7 SDKCore,
4 CookieHelpers)
* feat: fix file formatting.
* fix: DPoPStorage openDb() error handling and test coverage
- Remove 'as any' cast in catch block — reject() accepts unknown directly
- Add tests for indexedDB unavailable (SSR/non-browser): all three public
methods (getKeyPair, setKeyPair, clearKeyPair) reject with a descriptive error
- Add test for indexedDB.open() throwing synchronously (e.g. security policy block)
* fix: fix copilot warnings.
* fix: resolve DPoP transactions on tx.oncomplete, not req.onsuccess
All three DPoPStorage methods (getKeyPair, setKeyPair, clearKeyPair) now
resolve on tx.oncomplete and reject on tx.onerror / tx.onabort.
Previously, resolving on req.onsuccess meant the caller was told 'success'
before the transaction had fully committed — a transaction abort occurring
after the request succeeded (e.g. quota exceeded) would go undetected.
Applies the same fix consistently to all three methods, including getKeyPair
(readonly, lower risk, but now consistent) and setKeyPair (readwrite, same
durability concern as clearKeyPair).
Adds a test that aborts a clearKeyPair transaction synchronously inside the
request onsuccess handler and verifies the promise rejects and the key pair
is still present in IndexedDB.
* feat: the workflow will run regardless of the branch being merged into.
* refactor: make DPoPStorage IndexedDB constants configurable via config object
- Export DEFAULT_DPOP_DB_NAME, DEFAULT_DPOP_DB_VERSION, DEFAULT_DPOP_STORE_NAME
as named constants (no hardcoded magic strings anywhere in the codebase)
- Add DPoPStorageConfig interface with clientId (required) and optional
dbName, dbVersion, storeName fields — each defaults to the exported constant
- Refactor DPoPStorage constructor from positional (clientId: string) to
config object, matching the UrlHelperConfig convention in this monorepo
- openDb() and all three public methods now reference instance fields
(this.dbName, this.dbVersion, this.storeName) instead of module constants
- Add tests: defaults apply when no config overrides provided; custom dbName
and storeName land data in the right database; two instances with different
dbNames but the same clientId do not share keys; dbVersion downgrade
produces a clean rejection (VersionError)
- Update AGENTS.md: note the config-object constructor convention and the
IndexedDB dbVersion must-only-increase constraint
* feature: delete contrived test to intercept a successful even and then abort.
* feat: Implement DPoPManager - central coordinator (ENG-4784) (#202)
* feat: add DPoP core storage layer (ENG-4782)
- Add `dpop` v2.1.1 runtime dependency and `fake-indexeddb` devDependency
to packages/core/package.json
- SDKConfig: add optional `useDpop` and `dpopTokenStorage` fields
- UrlHelper: add `getAuthorizeUrl(state?, dpopJkt?, codeChallenge?)`
targeting FusionAuth /oauth2/authorize directly; update UrlHelperTypes
to include response_type, code_challenge, code_challenge_method, dpop_jkt
- DPoPStorage: IndexedDB abstraction for ES256 CryptoKeyPair persistence
(db: fusionauth-sdk:dpop, store: keypair, keyed by clientId)
- DPoPTokenStore: localStorage/memory token storage for DPoP-bound tokens
(key: fusionauth-sdk:tokens:<clientId>); includes getAccessToken() and
isExpired getter
- packages/core/src/DPoP/index.ts re-exports both classes
- 54 tests passing (21 DPoPTokenStore, 6 DPoPStorage, 16 UrlHelper, 7 SDKCore,
4 CookieHelpers)
* feat: fix file formatting.
* fix: DPoPStorage openDb() error handling and test coverage
- Remove 'as any' cast in catch block — reject() accepts unknown directly
- Add tests for indexedDB unavailable (SSR/non-browser): all three public
methods (getKeyPair, setKeyPair, clearKeyPair) reject with a descriptive error
- Add test for indexedDB.open() throwing synchronously (e.g. security policy block)
* fix: fix copilot warnings.
* fix: resolve DPoP transactions on tx.oncomplete, not req.onsuccess
All three DPoPStorage methods (getKeyPair, setKeyPair, clearKeyPair) now
resolve on tx.oncomplete and reject on tx.onerror / tx.onabort.
Previously, resolving on req.onsuccess meant the caller was told 'success'
before the transaction had fully committed — a transaction abort occurring
after the request succeeded (e.g. quota exceeded) would go undetected.
Applies the same fix consistently to all three methods, including getKeyPair
(readonly, lower risk, but now consistent) and setKeyPair (readwrite, same
durability concern as clearKeyPair).
Adds a test that aborts a clearKeyPair transaction synchronously inside the
request onsuccess handler and verifies the promise rejects and the key pair
is still present in IndexedDB.
* feat: the workflow will run regardless of the branch being merged into.
* feat: implement DPoPManager central coordinator (ENG-4784)
* feat: re-generate lock file.
* feat: re-generate lock file.
* feat: update lock file.
* test: add DPoP smoke tests against real FusionAuth instance (pre-SDKCore)
* feat: fix format and lint errors.
* refactor: make DPoPStorage IndexedDB constants configurable via config object
- Export DEFAULT_DPOP_DB_NAME, DEFAULT_DPOP_DB_VERSION, DEFAULT_DPOP_STORE_NAME
as named constants (no hardcoded magic strings anywhere in the codebase)
- Add DPoPStorageConfig interface with clientId (required) and optional
dbName, dbVersion, storeName fields — each defaults to the exported constant
- Refactor DPoPStorage constructor from positional (clientId: string) to
config object, matching the UrlHelperConfig convention in this monorepo
- openDb() and all three public methods now reference instance fields
(this.dbName, this.dbVersion, this.storeName) instead of module constants
- Add tests: defaults apply when no config overrides provided; custom dbName
and storeName land data in the right database; two instances with different
dbNames but the same clientId do not share keys; dbVersion downgrade
produces a clean rejection (VersionError)
- Update AGENTS.md: note the config-object constructor convention and the
IndexedDB dbVersion must-only-increase constraint
* feature: delete contrived test to intercept a successful even and then abort.
* fix: update DPoPStorage constructor calls to use config object after ENG-4782 refactor
* feature: update lock file
* fix: merge Request and init headers in DPoPManager.fetch() instead of dropping one (PR #202 review)
_resolveHeaders() previously returned early with init.headers whenever it
was present, silently discarding any headers already set on a Request
object passed as input. This contradicted the _doFetch documentation's
promise to never drop caller headers.
_resolveHeaders() now returns a merged Headers object: init.headers is
the base, and Request.headers are layered on top, winning on any
conflicting header name.
* feat: fix angular and vue tests.
* fix: clone Request before retry in fetch() to avoid double body consumption (PR #202 Copilot review)
fetch() previously passed the same input reference to both the initial
attempt and the nonce-triggered retry. If input was a Request with a
body, the first attempt consumed it, and the retry would throw a
'body already used' error instead of succeeding.
- Clone the Request twice up front (before either is read from) so each
attempt gets an independent, unconsumed body. Request.clone() safely
tees any internal streaming body per spec, so this also covers a
Request built with a ReadableStream body.
- A raw ReadableStream passed via init.body (not wrapped in a Request)
cannot be cloned this way. On retry, this now throws a clear,
actionable error instead of letting native fetch throw an opaque one.
* fix: normalize htu and htm in generateProof() per RFC 9449 (PR #202 Copilot review)
generateProof() documented htu as being 'without query/fragment' but
passed it through unmodified. fetch() supplies Request.url, which can
include a query string, so proofs generated via dpopFetch() could
carry an htu that includes query parameters — a subtle interop bug
with strict DPoP verifiers.
htm was also not normalised to uppercase, which most DPoP verifiers
require.
Both are now normalised inside generateProof() itself, so this is
correct regardless of whether callers go through fetch() or call
generateProof() directly with arbitrary casing/query strings.
* fix: remove dead captured header variables and misleading comment in dpop-smoke.test.ts (PR #202 Copilot review)
T2-1 declared capturedAuthHeader/capturedDpopHeader that were never
assigned and only suppressed via void, alongside a comment claiming
Playwright route interception captures DPoPManager.fetch()'s headers
— no such interception exists since fetch() runs in the Node test
process, not the browser page.
Removed the dead variables and replaced the comment with an accurate
explanation of how correctness is actually validated (end-to-end via
FusionAuth's server-side verification, plus T2-2's direct proof
decoding).
* feat: update approvers.
* feat: remove redundant comments.
* feat: The Authorization Code Grant can be started (#203)
* feat: add DPoP core storage layer (ENG-4782)
- Add `dpop` v2.1.1 runtime dependency and `fake-indexeddb` devDependency
to packages/core/package.json
- SDKConfig: add optional `useDpop` and `dpopTokenStorage` fields
- UrlHelper: add `getAuthorizeUrl(state?, dpopJkt?, codeChallenge?)`
targeting FusionAuth /oauth2/authorize directly; update UrlHelperTypes
to include response_type, code_challenge, code_challenge_method, dpop_jkt
- DPoPStorage: IndexedDB abstraction for ES256 CryptoKeyPair persistence
(db: fusionauth-sdk:dpop, store: keypair, keyed by clientId)
- DPoPTokenStore: localStorage/memory token storage for DPoP-bound tokens
(key: fusionauth-sdk:tokens:<clientId>); includes getAccessToken() and
isExpired getter
- packages/core/src/DPoP/index.ts re-exports both classes
- 54 tests passing (21 DPoPTokenStore, 6 DPoPStorage, 16 UrlHelper, 7 SDKCore,
4 CookieHelpers)
* feat: fix file formatting.
* fix: DPoPStorage openDb() error handling and test coverage
- Remove 'as any' cast in catch block — reject() accepts unknown directly
- Add tests for indexedDB unavailable (SSR/non-browser): all three public
methods (getKeyPair, setKeyPair, clearKeyPair) reject with a descriptive error
- Add test for indexedDB.open() throwing synchronously (e.g. security policy block)
* fix: fix copilot warnings.
* fix: resolve DPoP transactions on tx.oncomplete, not req.onsuccess
All three DPoPStorage methods (getKeyPair, setKeyPair, clearKeyPair) now
resolve on tx.oncomplete and reject on tx.onerror / tx.onabort.
Previously, resolving on req.onsuccess meant the caller was told 'success'
before the transaction had fully committed — a transaction abort occurring
after the request succeeded (e.g. quota exceeded) would go undetected.
Applies the same fix consistently to all three methods, including getKeyPair
(readonly, lower risk, but now consistent) and setKeyPair (readwrite, same
durability concern as clearKeyPair).
Adds a test that aborts a clearKeyPair transaction synchronously inside the
request onsuccess handler and verifies the promise rejects and the key pair
is still present in IndexedDB.
* feat: the workflow will run regardless of the branch being merged into.
* feat: implement DPoPManager central coordinator (ENG-4784)
* feat: re-generate lock file.
* feat: re-generate lock file.
* feat: update lock file.
* test: add DPoP smoke tests against real FusionAuth instance (pre-SDKCore)
* feat: fix format and lint errors.
* refactor: make DPoPStorage IndexedDB constants configurable via config object
- Export DEFAULT_DPOP_DB_NAME, DEFAULT_DPOP_DB_VERSION, DEFAULT_DPOP_STORE_NAME
as named constants (no hardcoded magic strings anywhere in the codebase)
- Add DPoPStorageConfig interface with clientId (required) and optional
dbName, dbVersion, storeName fields — each defaults to the exported constant
- Refactor DPoPStorage constructor from positional (clientId: string) to
config object, matching the UrlHelperConfig convention in this monorepo
- openDb() and all three public methods now reference instance fields
(this.dbName, this.dbVersion, this.storeName) instead of module constants
- Add tests: defaults apply when no config overrides provided; custom dbName
and storeName land data in the right database; two instances with different
dbNames but the same clientId do not share keys; dbVersion downgrade
produces a clean rejection (VersionError)
- Update AGENTS.md: note the config-object constructor convention and the
IndexedDB dbVersion must-only-increase constraint
* feature: delete contrived test to intercept a successful even and then abort.
* fix: update DPoPStorage constructor calls to use config object after ENG-4782 refactor
* feature: update lock file
* feat: implement SDKCore.startLogin() for DPoP authorization code grant (ENG-4786)
- Add Pkce module (generateCodeVerifier, generateCodeChallenge) with RFC 7636
Appendix B test vector coverage; runs under @vitest-environment node
- Extend RedirectHelper to persist code_verifier as a second colon-delimited
segment alongside state; add public getCodeVerifier() getter; add test file
- SDKCore: construct DPoPManager when config.useDpop is true; startLogin() is
now async — DPoP branch calls getOrCreateKeyPair()/getThumbprint() and
generates PKCE params then redirects to /oauth2/authorize directly; isLoggedIn
delegates to DPoPManager.isLoggedIn in DPoP mode (not app.at_exp cookie)
- SDKCore.test.ts: add DPoP-mode describe block with mocked DPoPManager and Pkce
(jsdom lacks crypto.subtle); all existing cookie-mode tests unaffected
- e2e/dpop-smoke.test.ts: replace local generatePkce() helper with shared Pkce
module; add Tier 0 tests exercising SDKCore.startLogin() in DPoP mode
end-to-end (no live FusionAuth required for Tier 0)
- Export Pkce from packages/core/src/index.ts
Note: yarn test:core cannot run in this sandbox environment due to a missing
@rollup/rollup-linux-arm64-gnu native binary (arch mismatch); TypeScript
compilation (tsc --noEmit) and ESLint/Prettier are clean.
* fix: add @vitest-environment jsdom to SDKCore.test.ts; fix handlePreRedirect assertion
Without the explicit jsdom annotation, vitest inherits the 'node' environment
from DPoPManager.test.ts when the full suite runs, causing 'document is not
defined' and 'window is not defined' failures in all SDKCore tests.
Also corrects the handlePreRedirect spy assertion: cookie-mode startLogin()
passes one argument (state), not two — the codeVerifier arg is only added in
DPoP mode.
* fix: suppress cookie console.error noise in Tier 0 e2e tests
SDKCore's constructor calls scheduleTokenExpiration() which calls
getAccessTokenExpirationMoment(). In a Node/Playwright process document
doesn't exist, so CookieHelpers catches the ReferenceError and logs
'Error accessing cookies...' to console.error. The tests still pass, but the
stderr noise is confusing.
Fix: extract a shared DPOP_CONFIG constant in the Tier 0 describe block that
includes a no-op cookieAdapter ({ at_exp: () => undefined }). This causes
getAccessTokenExpirationMoment() to take the adapter path and skip
document.cookie entirely, eliminating the noise.
Also fixes T0-1 where the await core.startLogin() call was accidentally
dropped during the previous config refactor.
* feat: update lock file.
* fix: update Angular onRedirect test to use 3-segment redirect-value format
RedirectHelper now stores nonce:codeVerifier:state (three colon-delimited
segments) instead of the previous nonce:state (two segments). The Angular
sdkcore/ directory is generated by 'yarn get-sdk-core' which copies
packages/core/src/ verbatim — so in CI the Angular RedirectHelper picks up
the updated parser automatically.
The test was writing the old two-segment format 'abc123:/welcome-page',
which the new parser splits as [nonce='abc123', codeVerifier='/welcome-page',
state=''] — returning undefined for state instead of '/welcome-page'.
Fix: write 'abc123::/welcome-page' (empty codeVerifier segment, matching
cookie mode where no verifier is stored).
* fix: update Vue and React onRedirect tests to use 3-segment redirect-value format
RedirectHelper now stores nonce:codeVerifier:state (three colon-delimited
segments) instead of nonce:state (two segments). Both sdk-vue and sdk-react
import SDKCore directly from @fusionauth-sdk/core (via the @fusionauth-sdk/*
tsconfig path alias), so their tests exercise the live, current
RedirectHelper — same root cause as the earlier Angular fix.
- packages/sdk-vue/src/createFusionAuth/createFusionAuth.test.ts: was seeding
the old 2-segment format ('rAnd0mStR1ng:<state>'), causing the new state
getter to return undefined instead of the expected state value. Fixed to
'rAnd0mStR1ng::<state>' (empty codeVerifier segment).
- packages/sdk-react/src/components/providers/FusionAuthProvider.test.tsx:
had the same stale 2-segment seed, but wasn't caught by CI because the
assertion only checked toHaveBeenCalled() (no argument check). Fixed the
seed format and strengthened the assertion to toHaveBeenCalledWith(stateValue)
to restore real coverage of the callback argument.
* fix: merge Request and init headers in DPoPManager.fetch() instead of dropping one (PR #202 review)
_resolveHeaders() previously returned early with init.headers whenever it
was present, silently discarding any headers already set on a Request
object passed as input. This contradicted the _doFetch documentation's
promise to never drop caller headers.
_resolveHeaders() now returns a merged Headers object: init.headers is
the base, and Request.headers are layered on top, winning on any
conflicting header name.
* feat: fix angular and vue tests.
* fix: clone Request before retry in fetch() to avoid double body consumption (PR #202 Copilot review)
fetch() previously passed the same input reference to both the initial
attempt and the nonce-triggered retry. If input was a Request with a
body, the first attempt consumed it, and the retry would throw a
'body already used' error instead of succeeding.
- Clone the Request twice up front (before either is read from) so each
attempt gets an independent, unconsumed body. Request.clone() safely
tees any internal streaming body per spec, so this also covers a
Request built with a ReadableStream body.
- A raw ReadableStream passed via init.body (not wrapped in a Request)
cannot be cloned this way. On retry, this now throws a clear,
actionable error instead of letting native fetch throw an opaque one.
* fix: normalize htu and htm in generateProof() per RFC 9449 (PR #202 Copilot review)
generateProof() documented htu as being 'without query/fragment' but
passed it through unmodified. fetch() supplies Request.url, which can
include a query string, so proofs generated via dpopFetch() could
carry an htu that includes query parameters — a subtle interop bug
with strict DPoP verifiers.
htm was also not normalised to uppercase, which most DPoP verifiers
require.
Both are now normalised inside generateProof() itself, so this is
correct regardless of whether callers go through fetch() or call
generateProof() directly with arbitrary casing/query strings.
* fix: remove dead captured header variables and misleading comment in dpop-smoke.test.ts (PR #202 Copilot review)
T2-1 declared capturedAuthHeader/capturedDpopHeader that were never
assigned and only suppressed via void, alongside a comment claiming
Playwright route interception captures DPoPManager.fetch()'s headers
— no such interception exists since fetch() runs in the Node test
process, not the browser page.
Removed the dead variables and replaced the comment with an accurate
explanation of how correctness is actually validated (end-to-end via
FusionAuth's server-side verification, plus T2-2's direct proof
decoding).
* feat: update approvers.
* test: add deterministic nonce-retry smoke test (T2-4)
FusionAuth (as the Authorization Server) never issues a use_dpop_nonce
challenge itself — per FusionAuth's DPoP docs, nonce enforcement is a
Resource Server responsibility implemented by your own APIs, not something
FusionAuth's own endpoints (e.g. /oauth2/userinfo) do. This is why the
existing T2-3 test can only assert structurally ('either outcome is a pass')
against a real FusionAuth instance.
T2-4 adds a self-contained, deterministic test that mocks globalThis.fetch
to simulate a Resource Server 401 response with a use_dpop_nonce challenge
(WWW-Authenticate + DPoP-Nonce headers), then verifies:
- exactly one retry occurs (not zero, not more than one)
- the first proof has no nonce claim
- the retried proof carries the exact server-issued nonce claim
- both proofs target the same htu/htm
Uses its own fresh DPoPManager (via the existing makeManager() helper) so it
does not depend on shared state/order from the Tier 1 tests, and requires no
live FusionAuth instance.
* fix: revert startLogin() to void, address Copilot PR review comment (ENG-4786)
Reverts SDKCore.startLogin()'s signature from 'async ... Promise<void>' back
to plain 'void', matching the public SDKContext/framework-wrapper types
exactly (SDKContext.ts, FusionAuthProviderContext.ts, Vue's FusionAuth<T>,
Angular's SDKContext.ts all still declare startLogin: (state?) => void).
Although tsc --noEmit already reported zero errors thanks to TypeScript's
void-returning-function compatibility rule, the underlying concern was real:
none of the three framework wrappers (React's useRedirecting, Vue's login(),
Angular's startLogin()) awaited or caught the promise, so a DPoP async
failure (e.g. crypto.subtle unavailable, IndexedDB blocked) would surface as
an unhandled promise rejection.
- SDKCore.ts: startLogin() is synchronous again. In DPoP mode it fires a new
private async startDpopLogin() and catches failures via the new optional
SDKConfig.onLoginFailure callback (falls back to console.error), following
the existing onAutoRefreshFailure convention. Cookie mode is unchanged.
- SDKConfig.ts: add onLoginFailure?: (error: Error) => void.
- SDKCore.test.ts: DPoP startLogin() tests now call startLogin() without
awaiting it and use vi.waitFor() to wait for window.location.assign
before asserting. Added two new tests covering onLoginFailure and the
console.error fallback.
- e2e/tests/dpop-smoke.test.ts: added a createAssignWaiter() helper (a
deferred promise resolved when window.location.assign is called) and
reworked T0-1/T0-2/T0-3 to use it instead of awaiting startLogin()
directly. T0-3 now explicitly waits for core1's redirect before swapping
IndexedDB for core2, preserving the original sequential-completion
guarantee that awaiting startLogin() used to provide implicitly.
No changes needed to SDKContext.ts, FusionAuthProviderContext.ts, Vue's
types, Angular's types/service, or any framework wrapper implementation —
zero blast radius outside packages/core, as intended.
* docs: fix stale/ambiguous state-reconstruction description in RedirectHelper.ts
Addresses a Copilot PR review comment. The class-level doc comment said
state is retrieved by joining segments 'after index 1 (skipping the verifier
segment)' — phrasing left over from before the codeVerifier segment existed.
The storage format is nonce:codeVerifier:state (3 segments), and the actual
implementation (line 85) skips both the nonce (index 0) and codeVerifier
(index 1) segments, with state starting at index 2 — not just 'the verifier
segment' as the old wording implied.
Doc-only change; no logic or test changes needed.
* fix: preserve state from legacy 2-segment redirect values (Copilot PR review)
RedirectHelper.state and getCodeVerifier() always assumed the current
3-segment storage format (nonce:codeVerifier:state). If a user initiates a
login redirect on a pre-DPoP SDK version (which wrote the legacy 2-segment
nonce:state format) and the app is upgraded to a newer SDK version before
they land back — e.g. a deploy that happens while they're on FusionAuth's
hosted login page — the leftover legacy value would be misparsed: state
would resolve to undefined instead of the real value.
Fix: detect the legacy format unambiguously. The current writer
(handlePreRedirect) always includes a codeVerifier segment, even when empty,
so any value it produces has at least two colons. A stored value with
exactly one colon can therefore only be the legacy format.
- state getter: if there are exactly 2 segments (1 colon), treat the second
segment as the legacy state directly, instead of destructuring past index 1
(which only works for the 3-segment format).
- getCodeVerifier(): same legacy-format guard, since a 2-segment value never
carried a code_verifier — prevents misreading a fragment of a legacy state
value as a verifier.
- Documented (as a comment, not a test) the known acceptable limitation: a
legacy state value that itself contained a colon is indistinguishable from
a current-format value with a non-empty codeVerifier — an inherent
ambiguity in a delimiter-based format without a version marker, accepted
given the narrow redirect-round-trip window.
- RedirectHelper.test.ts: added 4 tests seeding localStorage directly with
the legacy format, covering handlePostRedirect's callback value (including
empty legacy state), marker cleanup, and getCodeVerifier()'s undefined
result.
* feat: update comments.
* feat: remove redundant comments.
* feat: remove file not needed until adding end to end tests.
* feat: remove lengthy comment.
* feat: remote unnecessary comments.
* fix: store DPoP redirect data as JSON, leave hosted backend format untouched
Per PR review discussion: instead of patching around the ambiguity between
the legacy 2-segment (nonce:state) and current 3-segment
(nonce:codeVerifier:state) colon-delimited formats, eliminate the ambiguity
entirely by using two structurally distinct, non-overlapping formats under
the same fa-sdk-redirect-value key:
- Hosted backend mode (no codeVerifier passed to handlePreRedirect): plain
string `${randomNonce}:${state ?? ''}` — exactly the format every
published SDK version has always written, byte-for-byte unchanged. There
is no 'legacy format' to handle anymore because this format itself never
changed.
- DPoP mode (codeVerifier passed): JSON object { codeVerifier, state }.
JSON.parse deterministically throws on the hosted-backend plain string (it
never starts with '{', and a bare 'nonce:state' string can never be valid
JSON on its own — even an all-digit nonce fails because JSON.parse requires
the entire string to be one valid value, and trailing ':state' content after
a parsed number is rejected). This makes the two formats provably
non-ambiguous, unlike the previous 'count the colons' heuristic which had an
acknowledged edge case (a legacy state value containing its own colon was
indistinguishable from a new-format value with a real codeVerifier).
This also matches the existing convention in DPoPTokenStore, which already
uses JSON.stringify/JSON.parse for its persisted data rather than a
delimited string.
RedirectHelper.ts:
- handlePreRedirect/handlePostRedirect/getCodeVerifier signatures unchanged
(no SDKCore.ts changes needed) — only the internal storage format changed.
- Replaced the private "state" getter with a "parseState(raw)" method that
takes the already-fetched raw value (handlePostRedirect previously read
the value from storage twice per call; now once).
- Removed the now-obsolete 'exactly one colon = legacy' segment-counting
logic from getCodeVerifier() and the old state getter.
RedirectHelper.test.ts:
- Removed the 'legacy 2-segment format backward-compatibility' describe
block (4 tests) — no longer applicable, since hosted backend mode's format
never changes.
- Added a new 'storage format' describe block: hosted backend mode's raw
value is verified to still be a plain non-JSON string; DPoP mode's raw
value is verified to be the expected JSON shape; switching modes on the
same helper instance in either direction doesn't leak data from the
previous format; an empty-string codeVerifier still takes the JSON path
but getCodeVerifier() correctly returns undefined for it.
* feat: remove unnecessary tests and comments.
* feat: validate state in the redirect callback in hosted backend mode.
* feat: remove verbose comments.
* feat: remove verbose comments.
* feat: re-organize comments.
* refactor: consolidate DPoP/hosted-backend format parsing into parseStoredValue()
Follow-up to PR #203 review discussion about relying on a caught
JSON.parse() SyntaxError as the primary discriminator between DPoP mode's
JSON format and hosted backend mode's plain string format. Although the
performance concern doesn't really apply here (this is called at most once
or twice per completed redirect, not a hot path, and modern JS engines no
longer deoptimize functions containing try/catch), the underlying code
smell was worth addressing: hosted backend mode calls hit the catch branch
on 100% of calls, not as a rare/exceptional case.
Changes:
- handlePreRedirect(): named the write-side mode check `isDpopMode =
codeVerifier !== undefined` instead of an inline anonymous condition in
the ternary.
- Replaced parseState() and the duplicated try/catch logic inside
getCodeVerifier() with a single parseStoredValue(raw) method that returns
both `codeVerifier` and `state` in one pass. The DPoP-vs-hosted-backend
distinction is now expressed in exactly one place in the class, via a
cheap, deterministic `raw.startsWith('{')` sniff rather than a caught
exception — DPoP mode's JSON values always start with `{`; hosted backend
mode's plain `nonce:state` strings never do (the nonce is always hex
digits), so this is a provably correct discriminator with no exception
construction/throwing on the common hosted-backend path at all.
- getCodeVerifier() and handlePostRedirect() now both delegate to
parseStoredValue() instead of having their own separate parsing logic.
Pure internal refactor — no public API or behavior change. All 18 existing
RedirectHelper.test.ts tests pass unchanged, confirming no external behavior
was affected.
* feat: no flow control using a try...catch
* feat: In the Authorization Code Grant flow, exchange the code for tokens (#204)
* feat: add DPoP core storage layer (ENG-4782)
- Add `dpop` v2.1.1 runtime dependency and `fake-indexeddb` devDependency
to packages/core/package.json
- SDKConfig: add optional `useDpop` and `dpopTokenStorage` fields
- UrlHelper: add `getAuthorizeUrl(state?, dpopJkt?, codeChallenge?)`
targeting FusionAuth /oauth2/authorize directly; update UrlHelperTypes
to include response_type, code_challenge, code_challenge_method, dpop_jkt
- DPoPStorage: IndexedDB abstraction for ES256 CryptoKeyPair persistence
(db: fusionauth-sdk:dpop, store: keypair, keyed by clientId)
- DPoPTokenStore: localStorage/memory token storage for DPoP-bound tokens
(key: fusionauth-sdk:tokens:<clientId>); includes getAccessToken() and
isExpired getter
- packages/core/src/DPoP/index.ts re-exports both classes
- 54 tests passing (21 DPoPTokenStore, 6 DPoPStorage, 16 UrlHelper, 7 SDKCore,
4 CookieHelpers)
* feat: fix file formatting.
* fix: DPoPStorage openDb() error handling and test coverage
- Remove 'as any' cast in catch block — reject() accepts unknown directly
- Add tests for indexedDB unavailable (SSR/non-browser): all three public
methods (getKeyPair, setKeyPair, clearKeyPair) reject with a descriptive error
- Add test for indexedDB.open() throwing synchronously (e.g. security policy block)
* fix: fix copilot warnings.
* fix: resolve DPoP transactions on tx.oncomplete, not req.onsuccess
All three DPoPStorage methods (getKeyPair, setKeyPair, clearKeyPair) now
resolve on tx.oncomplete and reject on tx.onerror / tx.onabort.
Previously, resolving on req.onsuccess meant the caller was told 'success'
before the transaction had fully committed — a transaction abort occurring
after the request succeeded (e.g. quota exceeded) would go undetected.
Applies the same fix consistently to all three methods, including getKeyPair
(readonly, lower risk, but now consistent) and setKeyPair (readwrite, same
durability concern as clearKeyPair).
Adds a test that aborts a clearKeyPair transaction synchronously inside the
request onsuccess handler and verifies the promise rejects and the key pair
is still present in IndexedDB.
* feat: the workflow will run regardless of the branch being merged into.
* feat: implement DPoPManager central coordinator (ENG-4784)
* feat: re-generate lock file.
* feat: re-generate lock file.
* feat: update lock file.
* test: add DPoP smoke tests against real FusionAuth instance (pre-SDKCore)
* feat: fix format and lint errors.
* refactor: make DPoPStorage IndexedDB constants configurable via config object
- Export DEFAULT_DPOP_DB_NAME, DEFAULT_DPOP_DB_VERSION, DEFAULT_DPOP_STORE_NAME
as named constants (no hardcoded magic strings anywhere in the codebase)
- Add DPoPStorageConfig interface with clientId (required) and optional
dbName, dbVersion, storeName fields — each defaults to the exported constant
- Refactor DPoPStorage constructor from positional (clientId: string) to
config object, matching the UrlHelperConfig convention in this monorepo
- openDb() and all three public methods now reference instance fields
(this.dbName, this.dbVersion, this.storeName) instead of module constants
- Add tests: defaults apply when no config overrides provided; custom dbName
and storeName land data in the right database; two instances with different
dbNames but the same clientId do not share keys; dbVersion downgrade
produces a clean rejection (VersionError)
- Update AGENTS.md: note the config-object constructor convention and the
IndexedDB dbVersion must-only-increase constraint
* feature: delete contrived test to intercept a successful even and then abort.
* fix: update DPoPStorage constructor calls to use config object after ENG-4782 refactor
* feature: update lock file
* feat: implement SDKCore.startLogin() for DPoP authorization code grant (ENG-4786)
- Add Pkce module (generateCodeVerifier, generateCodeChallenge) with RFC 7636
Appendix B test vector coverage; runs under @vitest-environment node
- Extend RedirectHelper to persist code_verifier as a second colon-delimited
segment alongside state; add public getCodeVerifier() getter; add test file
- SDKCore: construct DPoPManager when config.useDpop is true; startLogin() is
now async — DPoP branch calls getOrCreateKeyPair()/getThumbprint() and
generates PKCE params then redirects to /oauth2/authorize directly; isLoggedIn
delegates to DPoPManager.isLoggedIn in DPoP mode (not app.at_exp cookie)
- SDKCore.test.ts: add DPoP-mode describe block with mocked DPoPManager and Pkce
(jsdom lacks crypto.subtle); all existing cookie-mode tests unaffected
- e2e/dpop-smoke.test.ts: replace local generatePkce() helper with shared Pkce
module; add Tier 0 tests exercising SDKCore.startLogin() in DPoP mode
end-to-end (no live FusionAuth required for Tier 0)
- Export Pkce from packages/core/src/index.ts
Note: yarn test:core cannot run in this sandbox environment due to a missing
@rollup/rollup-linux-arm64-gnu native binary (arch mismatch); TypeScript
compilation (tsc --noEmit) and ESLint/Prettier are clean.
* fix: add @vitest-environment jsdom to SDKCore.test.ts; fix handlePreRedirect assertion
Without the explicit jsdom annotation, vitest inherits the 'node' environment
from DPoPManager.test.ts when the full suite runs, causing 'document is not
defined' and 'window is not defined' failures in all SDKCore tests.
Also corrects the handlePreRedirect spy assertion: cookie-mode startLogin()
passes one argument (state), not two — the codeVerifier arg is only added in
DPoP mode.
* fix: suppress cookie console.error noise in Tier 0 e2e tests
SDKCore's constructor calls scheduleTokenExpiration() which calls
getAccessTokenExpirationMoment(). In a Node/Playwright process document
doesn't exist, so CookieHelpers catches the ReferenceError and logs
'Error accessing cookies...' to console.error. The tests still pass, but the
stderr noise is confusing.
Fix: extract a shared DPOP_CONFIG constant in the Tier 0 describe block that
includes a no-op cookieAdapter ({ at_exp: () => undefined }). This causes
getAccessTokenExpirationMoment() to take the adapter path and skip
document.cookie entirely, eliminating the noise.
Also fixes T0-1 where the await core.startLogin() call was accidentally
dropped during the previous config refactor.
* feat: update lock file.
* fix: update Angular onRedirect test to use 3-segment redirect-value format
RedirectHelper now stores nonce:codeVerifier:state (three colon-delimited
segments) instead of the previous nonce:state (two segments). The Angular
sdkcore/ directory is generated by 'yarn get-sdk-core' which copies
packages/core/src/ verbatim — so in CI the Angular RedirectHelper picks up
the updated parser automatically.
The test was writing the old two-segment format 'abc123:/welcome-page',
which the new parser splits as [nonce='abc123', codeVerifier='/welcome-page',
state=''] — returning undefined for state instead of '/welcome-page'.
Fix: write 'abc123::/welcome-page' (empty codeVerifier segment, matching
cookie mode where no verifier is stored).
* fix: update Vue and React onRedirect tests to use 3-segment redirect-value format
RedirectHelper now stores nonce:codeVerifier:state (three colon-delimited
segments) instead of nonce:state (two segments). Both sdk-vue and sdk-react
import SDKCore directly from @fusionauth-sdk/core (via the @fusionauth-sdk/*
tsconfig path alias), so their tests exercise the live, current
RedirectHelper — same root cause as the earlier Angular fix.
- packages/sdk-vue/src/createFusionAuth/createFusionAuth.test.ts: was seeding
the old 2-segment format ('rAnd0mStR1ng:<state>'), causing the new state
getter to return undefined instead of the expected state value. Fixed to
'rAnd0mStR1ng::<state>' (empty codeVerifier segment).
- packages/sdk-react/src/components/providers/FusionAuthProvider.test.tsx:
had the same stale 2-segment seed, but wasn't caught by CI because the
assertion only checked toHaveBeenCalled() (no argument check). Fixed the
seed format and strengthened the assertion to toHaveBeenCalledWith(stateValue)
to restore real coverage of the callback argument.
* fix: merge Request and init headers in DPoPManager.fetch() instead of dropping one (PR #202 review)
_resolveHeaders() previously returned early with init.headers whenever it
was present, silently discarding any headers already set on a Request
object passed as input. This contradicted the _doFetch documentation's
promise to never drop caller headers.
_resolveHeaders() now returns a merged Headers object: init.headers is
the base, and Request.headers are layered on top, winning on any
conflicting header name.
* feat: fix angular and vue tests.
* fix: clone Request before retry in fetch() to avoid double body consumption (PR #202 Copilot review)
fetch() previously passed the same input reference to both the initial
attempt and the nonce-triggered retry. If input was a Request with a
body, the first attempt consumed it, and the retry would throw a
'body already used' error instead of succeeding.
- Clone the Request twice up front (before either is read from) so each
attempt gets an independent, unconsumed body. Request.clone() safely
tees any internal streaming body per spec, so this also covers a
Request built with a ReadableStream body.
- A raw ReadableStream passed via init.body (not wrapped in a Request)
cannot be cloned this way. On retry, this now throws a clear,
actionable error instead of letting native fetch throw an opaque one.
* fix: normalize htu and htm in generateProof() per RFC 9449 (PR #202 Copilot review)
generateProof() documented htu as being 'without query/fragment' but
passed it through unmodified. fetch() supplies Request.url, which can
include a query string, so proofs generated via dpopFetch() could
carry an htu that includes query parameters — a subtle interop bug
with strict DPoP verifiers.
htm was also not normalised to uppercase, which most DPoP verifiers
require.
Both are now normalised inside generateProof() itself, so this is
correct regardless of whether callers go through fetch() or call
generateProof() directly with arbitrary casing/query strings.
* fix: remove dead captured header variables and misleading comment in dpop-smoke.test.ts (PR #202 Copilot review)
T2-1 declared capturedAuthHeader/capturedDpopHeader that were never
assigned and only suppressed via void, alongside a comment claiming
Playwright route interception captures DPoPManager.fetch()'s headers
— no such interception exists since fetch() runs in the Node test
process, not the browser page.
Removed the dead variables and replaced the comment with an accurate
explanation of how correctness is actually validated (end-to-end via
FusionAuth's server-side verification, plus T2-2's direct proof
decoding).
* feat: update approvers.
* test: add deterministic nonce-retry smoke test (T2-4)
FusionAuth (as the Authorization Server) never issues a use_dpop_nonce
challenge itself — per FusionAuth's DPoP docs, nonce enforcement is a
Resource Server responsibility implemented by your own APIs, not something
FusionAuth's own endpoints (e.g. /oauth2/userinfo) do. This is why the
existing T2-3 test can only assert structurally ('either outcome is a pass')
against a real FusionAuth instance.
T2-4 adds a self-contained, deterministic test that mocks globalThis.fetch
to simulate a Resource Server 401 response with a use_dpop_nonce challenge
(WWW-Authenticate + DPoP-Nonce headers), then verifies:
- exactly one retry occurs (not zero, not more than one)
- the first proof has no nonce claim
- the retried proof carries the exact server-issued nonce claim
- both proofs target the same htu/htm
Uses its own fresh DPoPManager (via the existing makeManager() helper) so it
does not depend on shared state/order from the Tier 1 tests, and requires no
live FusionAuth instance.
* fix: revert startLogin() to void, address Copilot PR review comment (ENG-4786)
Reverts SDKCore.startLogin()'s signature from 'async ... Promise<void>' back
to plain 'void', matching the public SDKContext/framework-wrapper types
exactly (SDKContext.ts, FusionAuthProviderContext.ts, Vue's FusionAuth<T>,
Angular's SDKContext.ts all still declare startLogin: (state?) => void).
Although tsc --noEmit already reported zero errors thanks to TypeScript's
void-returning-function compatibility rule, the underlying concern was real:
none of the three framework wrappers (React's useRedirecting, Vue's login(),
Angular's startLogin()) awaited or caught the promise, so a DPoP async
failure (e.g. crypto.subtle unavailable, IndexedDB blocked) would surface as
an unhandled promise rejection.
- SDKCore.ts: startLogin() is synchronous again. In DPoP mode it fires a new
private async startDpopLogin() and catches failures via the new optional
SDKConfig.onLoginFailure callback (falls back to console.error), following
the existing onAutoRefreshFailure convention. Cookie mode is unchanged.
- SDKConfig.ts: add onLoginFailure?: (error: Error) => void.
- SDKCore.test.ts: DPoP startLogin() tests now call startLogin() without
awaiting it and use vi.waitFor() to wait for window.location.assign
before asserting. Added two new tests covering onLoginFailure and the
console.error fallback.
- e2e/tests/dpop-smoke.test.ts: added a createAssignWaiter() helper (a
deferred promise resolved when window.location.assign is called) and
reworked T0-1/T0-2/T0-3 to use it instead of awaiting startLogin()
directly. T0-3 now explicitly waits for core1's redirect before swapping
IndexedDB for core2, preserving the original sequential-completion
guarantee that awaiting startLogin() used to provide implicitly.
No changes needed to SDKContext.ts, FusionAuthProviderContext.ts, Vue's
types, Angular's types/service, or any framework wrapper implementation —
zero blast radius outside packages/core, as intended.
* docs: fix stale/ambiguous state-reconstruction description in RedirectHelper.ts
Addresses a Copilot PR review comment. The class-level doc comment said
state is retrieved by joining segments 'after index 1 (skipping the verifier
segment)' — phrasing left over from before the codeVerifier segment existed.
The storage format is nonce:codeVerifier:state (3 segments), and the actual
implementation (line 85) skips both the nonce (index 0) and codeVerifier
(index 1) segments, with state starting at index 2 — not just 'the verifier
segment' as the old wording implied.
Doc-only change; no logic or test changes needed.
* fix: preserve state from legacy 2-segment redirect values (Copilot PR review)
RedirectHelper.state and getCodeVerifier() always assumed the current
3-segment storage format (nonce:codeVerifier:state). If a user initiates a
login redirect on a pre-DPoP SDK version (which wrote the legacy 2-segment
nonce:state format) and the app is upgraded to a newer SDK version before
they land back — e.g. a deploy that happens while they're on FusionAuth's
hosted login page — the leftover legacy value would be misparsed: state
would resolve to undefined instead of the real value.
Fix: detect the legacy format unambiguously. The current writer
(handlePreRedirect) always includes a codeVerifier segment, even when empty,
so any value it produces has at least two colons. A stored value with
exactly one colon can therefore only be the legacy format.
- state getter: if there are exactly 2 segments (1 colon), treat the second
segment as the legacy state directly, instead of destructuring past index 1
(which only works for the 3-segment format).
- getCodeVerifier(): same legacy-format guard, since a 2-segment value never
carried a code_verifier — prevents misreading a fragment of a legacy state
value as a verifier.
- Documented (as a comment, not a test) the known acceptable limitation: a
legacy state value that itself contained a colon is indistinguishable from
a current-format value with a non-empty codeVerifier — an inherent
ambiguity in a delimiter-based format without a version marker, accepted
given the narrow redirect-round-trip window.
- RedirectHelper.test.ts: added 4 tests seeding localStorage directly with
the legacy format, covering handlePostRedirect's callback value (including
empty legacy state), marker cleanup, and getCodeVerifier()'s undefined
result.
* feat: update comments.
* feat: SDKCore: implement handlePostRedirect() authorization code exchange (ENG-4800)
- UrlHelper.getTokenUrl() targets FusionAuth's /oauth2/token directly.
- DPoPManager.getExpiresAt() exposes the stored token's expiry (-1 when
none), mirroring CookieHelpers' convention.
- SDKCore.handlePostRedirect() branches into handleDpopPostRedirect() in
DPoP mode: detects the `code` query param, retrieves the persisted PKCE
code_verifier, signs a DPoP proof for the token endpoint (no ath),
POSTs the authorization_code grant, stores the returned tokens, and
schedules token expiration + (when shouldAutoRefresh) auto-refresh from
expiresAt. No-ops silently when code/code_verifier is missing (e.g. a
second invocation after a successful exchange). Failures report via
onLoginFailure/console.error, mirroring startLogin().
- SDKCore.at_exp generalized to delegate to DPoPManager.getExpiresAt() in
DPoP mode so scheduling logic is shared between cookie and DPoP modes.
- Unit tests for all of the above; mockWindowLocation extended to accept
a search override for simulating the post-redirect landing.
- e2e/tests/dpop-smoke.test.ts: extracted shared ensureNodeBrowserPolyfills()
helper; updated T1-2 to drive the full authorization code grant through
the real SDKCore.startLogin() + handlePostRedirect() against a live
FusionAuth instance instead of replicating the exchange manually.
* feat: remove references to ENG- linear issues.
* feat: remove redundant comments.
* feat: remove file not needed until adding end to end tests.
* feat: remove lengthy comment.
* feat: remote unnecessary comments.
* feat: remove verbose comment.
* feat: copilot review warnings.
* feat: failing smoke test.
* feat: minimize verbose comments.
* feat: clean up comments.
* feat: minimize verbose comments.
* feat: copilot recommendation .
* feat: cleanup comments.
* feat: reduce commenting.
* feat: remove verbose comment and non-browser functionality.
* feat: only remove the code query string parameter from the DPoP mode callback after calling /oauth2/authorize. This matches the behavior in Hosted Backend mode.
* feat: The OIDC logout flow is supported (#205)
* feat: add DPoP core storage layer (ENG-4782)
- Add `dpop` v2.1.1 runtime dependency and `fake-indexeddb` devDependency
to packages/core/package.json
- SDKConfig: add optional `useDpop` and `dpopTokenStorage` fields
- UrlHelper: add `getAuthorizeUrl(state?, dpopJkt?, codeChallenge?)`
targeting FusionAuth /oauth2/authorize directly; update UrlHelperTypes
to include response_type, code_challenge, code_challenge_method, dpop_jkt
- DPoPStorage: IndexedDB abstraction for ES256 CryptoKeyPair persistence
(db: fusionauth-sdk:dpop, store: keypair, keyed by clientId)
- DPoPTokenStore: localStorage/memory token storage for DPoP-bound tokens
(key: fusionauth-sdk:tokens:<clientId>); includes getAccessToken() and
isExpired getter
- packages/core/src/DPoP/index.ts re-exports both classes
- 54 tests passing (21 DPoPTokenStore, 6 DPoPStorage, 16 UrlHelper, 7 SDKCore,
4 CookieHelpers)
* feat: fix file formatting.
* fix: DPoPStorage openDb() error handling and test coverage
- Remove 'as any' cast in catch block — reject() accepts unknown directly
- Add tests for indexedDB unavailable (SSR/non-browser): all three public
methods (getKeyPair, setKeyPair, clearKeyPair) reject with a descriptive error
- Add test for indexedDB.open() throwing synchronously (e.g. security policy block)
* fix: fix copilot warnings.
* fix: resolve DPoP transactions on tx.oncomplete, not req.onsuccess
All three DPoPStorage methods (getKeyPair, setKeyPair, clearKeyPair) now
resolve on tx.oncomplete and reject on tx.onerror / tx.onabort.
Previously, resolving on req.onsuccess meant the caller was told 'success'
before the transaction had fully committed — a transaction abort occurring
after the request succeeded (e.g. quota exceeded) would go undetected.
Applies the same fix consistently to all three methods, including getKeyPair
(readonly, lower risk, but now consistent) and setKeyPair (readwrite, same
durability concern as clearKeyPair).
Adds a test that aborts a clearKeyPair transaction synchronously inside the
request onsuccess handler and verifies the promise rejects and the key pair
is still present in IndexedDB.
* feat: the workflow will run regardless of the branch being merged into.
* feat: implement DPoPManager central coordinator (ENG-4784)
* feat: re-generate lock file.
* feat: re-generate lock file.
* feat: update lock file.
* test: add DPoP smoke tests against real FusionAuth instance (pre-SDKCore)
* feat: fix format and lint errors.
* refactor: make DPoPStorage IndexedDB constants configurable via config object
- Export DEFAULT_DPOP_DB_NAME, DEFAULT_DPOP_DB_VERSION, DEFAULT_DPOP_STORE_NAME
as named constants (no hardcoded magic strings anywhere in the codebase)
- Add DPoPStorageConfig interface with clientId (required) and optional
dbName, dbVersion, storeName fields — each defaults to the exported constant
- Refactor DPoPStorage constructor from positional (clientId: string) to
config object, matching the UrlHelperConfig convention in this monorepo
- openDb() and all three public methods now reference instance fields
(this.dbName, this.dbVersion, this.storeName) instead of module constants
- Add tests: defaults apply when no config overrides provided; custom dbName
and storeName land data in the right database; two instances with different
dbNames but the same clientId do not share keys; dbVersion downgrade
produces a clean rejection (VersionError)
- Update AGENTS.md: note the config-object constructor convention and the
IndexedDB dbVersion must-only-increase constraint
* feature: delete contrived test to intercept a successful even and then abort.
* fix: update DPoPStorage constructor calls to use config object after ENG-4782 refactor
* feature: update lock file
* feat: implement SDKCore.startLogin() for DPoP authorization code grant (ENG-4786)
- Add Pkce module (generateCodeVerifier, generateCodeChallenge) with RFC 7636
Appendix B test vector coverage; runs under @vitest-environment node
- Extend RedirectHelper to persist code_verifier as a second colon-delimited
segment alongside state; add public getCodeVerifier() getter; add test file
- SDKCore: construct DPoPManager when config.useDpop is true; startLogin() is
now async — DPoP branch calls getOrCreateKeyPair()/getThumbprint() and
generates PKCE params then redirects to /oauth2/authorize directly; isLoggedIn
delegates to DPoPManager.isLoggedIn in DPoP mode (not app.at_exp cookie)
- SDKCore.test.ts: add DPoP-mode describe block with mocked DPoPManager and Pkce
(jsdom lacks crypto.subtle); all existing cookie-mode tests unaffected
- e2e/dpop-smoke.test.ts: replace local generatePkce() helper with shared Pkce
module; add Tier 0 tests exercising SDKCore.startLogin() in DPoP mode
end-to-end (no live FusionAuth required for Tier 0)
- Export Pkce from packages/core/src/index.ts
Note: yarn test:core cannot run in this sandbox environment due to a missing
@rollup/rollup-linux-arm64-gnu native binary (arch mismatch); TypeScript
compilation (tsc --noEmit) and ESLint/Prettier are clean.
* fix: add @vitest-environment jsdom to SDKCore.test.ts; fix handlePreRedirect assertion
Without the explicit jsdom annotation, vitest inherits the 'node' environment
from DPoPManager.test.ts when the full suite runs, causing 'document is not
defined' and 'window is not defined' failures in all SDKCore tests.
Also corrects the handlePreRedirect spy assertion: cookie-mode startLogin()
passes one argument (state), not two — the codeVerifier arg is only added in
DPoP mode.
* fix: suppress cookie console.error noise in Tier 0 e2e tests
SDKCore's constructor calls scheduleTokenExpiration() which calls
getAccessTokenExpirationMoment(). In a Node/Playwright process document
doesn't exist, so CookieHelpers catches the ReferenceError and logs
'Error accessing cookies...' to console.error. The tests still pass, but the
stderr noise is confusing.
Fix: extract a shared DPOP_CONFIG constant in the Tier 0 describe block that
includes a no-op cookieAdapter ({ at_exp: () => undefined }). This causes
getAccessTokenExpirationMoment() to take the adapter path and skip
document.cookie entirely, eliminating the noise.
Also fixes T0-1 where the await core.startLogin() call was accidentally
dropped during the previous config refactor.
* feat: update lock file.
* fix: update Angular onRedirect test to use 3-segment redirect-value format
RedirectHelper now stores nonce:codeVerifier:state (three colon-delimited
segments) instead of the previous nonce:state (two segments). The Angular
sdkcore/ directory is generated by 'yarn get-sdk-core' which copies
packages/core/src/ verbatim — so in CI the Angular RedirectHelper picks up
the updated parser automatically.
The test was writing the old two-segment format 'abc123:/welcome-page',
which the new parser splits as [nonce='abc123', codeVerifier='/welcome-page',
state=''] — returning undefined for state instead of '/welcome-page'.
Fix: write 'abc123::/welcome-page' (empty codeVerifier segment, matching
cookie mode where no verifier is stored).
* fix: update Vue and React onRedirect tests to use 3-segment redirect-value format
RedirectHelper now stores nonce:codeVerifier:state (three colon-delimited
segments) instead of nonce:state (two segments). Both sdk-vue and sdk-react
import SDKCore directly from @fusionauth-sdk/core (via the @fusionauth-sdk/*
tsconfig path alias), so their tests exercise the live, current
RedirectHelper — same root cause as the earlier Angular fix.
- packages/sdk-vue/src/createFusionAuth/createFusionAuth.test.ts: was seeding
the old 2-segment format ('rAnd0mStR1ng:<state>'), causing the new state
getter to return undefined instead of the expected state value. Fixed to
'rAnd0mStR1ng::<state>' (empty codeVerifier segment).
- packages/sdk-react/src/components/providers/FusionAuthProvider.test.tsx:
had the same stale 2-segment seed, but wasn't caught by CI because the
assertion only checked toHaveBeenCalled() (no argument check). Fixed the
seed format and strengthened the assertion to toHaveBeenCalledWith(stateValue)
to restore real coverage of the callback argument.
* fix: merge Request and init headers in DPoPManager.fetch() instead of dropping one (PR #202 review)
_resolveHeaders() previously returned early with init.headers whenever it
was present, silently discarding any headers already set on a Request
object passed as input. This contradicted the _doFetch documentation's
promise to never drop caller headers.
_resolveHeaders() now returns a merged Headers object: init.headers is
the base, and Request.headers are layered on top, winning on any
conflicting header name.
* feat: fix angular and vue tests.
* fix: clone Request before retry in fetch() to avoid double body consumption (PR #202 Copilot review)
fetch() previously passed the same input reference to both the initial
attempt and the nonce-triggered retry. If input was a Request with a
body, the first attempt consumed it, and the retry would throw a
'body already used' error instead of succeeding.
- Clone the Request twice up front (before either is read from) so each
attempt gets an independent, unconsumed body. Request.clone() safely
tees any internal streaming body per spec, so this also covers a
Request built with a ReadableStream body.
- A raw ReadableStream passed via init.body (not wrapped in a Request)
cannot be cloned this way. On retry, this now throws a clear,
actionable error instead of letting native fetch throw an opaque one.
* fix: normalize htu and htm in generateProof() per RFC 9449 (PR #202 Copilot review)
generateProof() documented htu as being 'without query/fragment' but
passed it through unmodified. fetch() supplies Request.url, which can
include a query string, so proofs generated via dpopFetch() could
carry an htu that includes query parameters — a subtle interop bug
with strict DPoP verifiers.
htm was also not normalised to uppercase, which most DPoP verifiers
require.
Both are now normalised inside generateProof() itself, so this is
correct regardless of whether callers go through fetch() or call
generateProof() directly with arbitrary casing/query strings.
* fix: remove dead captured header variables and misleading comment in dpop-smoke.test.ts (PR #202 Copilot review)
T2-1 declared capturedAuthHeader/capturedDpopHeader that were never
assigned and only suppressed via void, alongside a comment claiming
Playwright route interception captures DPoPManager.fetch()'s headers
— no such interception exists since fetch() runs in the Node test
process, not the browser page.
Removed the dead variables and replaced the comment with an accurate
explanation of how correctness is actually validated (end-to-end via
FusionAuth's server-side verification, plus T2-2's direct proof
decoding).
* feat: update approvers.
* test: add deterministic nonce-retry smoke test (T2-4)
FusionAuth (as the Authorization Server) never issues a use_dpop_nonce
challenge itself — per FusionAuth's DPoP docs, nonce enforcement is a
Resource Server responsibility implemented by your own APIs, not something
FusionAuth's own endpoints (e.g. /oauth2/userinfo) do. This is why the
existing T2-3 test can only assert structurally ('either outcome is a pass')
against a real FusionAuth instance.
T2-4 adds a self-contained, deterministic test that mocks globalThis.fetch
to simulate a Resource Server 401 response with a use_dpop_nonce challenge
(WWW-Authenticate + DPoP-Nonce headers), then verifies:
- exactly one retry occurs (not zero, not more than one)
- the first proof has no nonce claim
- the retried proof carries the exact server-issued nonce claim
- both proofs target the same htu/htm
Uses its own fresh DPoPManager (via the existing makeManager() helper) so it
does not depend on shared state/order from the Tier 1 tests, and requires no
live FusionAuth instance.
* fix: revert startLogin() to void, address Copilot PR review comment (ENG-4786)
Reverts SDKCore.startLogin()'s signature from 'async ... Promise<void>' back
to plain 'void', matching the public SDKContext/framework-wrapper types
exactly (SDKContext.ts, FusionAuthProviderContext.ts, Vue's FusionAuth<T>,
Angular's SDKContext.ts all still declare startLogin: (state?) => void).
Although tsc --noEmit already reported zero errors thanks to TypeScript's
void-returning-function compatibility rule, the underlying concern was real:
none of the three framework wrappers (React's useRedirecting, Vue's login(),
Angular's startLogin()) awaited or caught the promise, so a DPoP async
failure (e.g. crypto.subtle unavailable, IndexedDB blocked) would surface as
an unhandled promise rejection.
- SDKCore.ts: startLogin() is synchronous again. In DPoP mode it fires a new
private async startDpopLogin() and catches failures via the new optional
SDKConfig.onLoginFailure callback (falls back to console.error), following
the existing onAutoRefreshFailure convention. Cookie mode is unchanged.
- SDKConfig.ts: add onLoginFailure?: (error: Error) => void.
- SDKCore.test.ts: DPoP startLogin() tests now call startLogin() without
awaiting it and use vi.waitFor() to wait for window.location.assign
before asserting. Added two new tests covering onLoginFailure and the
console.error fallback.
- e2e/tests/dpop-smoke.test.ts: added a createAssignWaiter() helper (a
deferred promise resolved when window.location.assign is called) and
reworked T0-1/T0-2/T0-3 to use it instead of awaiting startLogin()
directly. T0-3 now explicitly waits for core1's redirect before swapping
IndexedDB for core2, preserving the original sequential-completion
guarantee that awaiting startLogin() used to provide implicitly.
No changes needed to SDKContext.ts, FusionAuthProviderContext.ts, Vue's
types, Angular's types/service, or any framework wrapper implementation —
zero blast radius outside packages/core, as intended.
* docs: fix stale/ambiguous state-reconstruction description in RedirectHelper.ts
Addresses a Copilot PR review comment. The class-level doc comment said
state is retrieved by joining segments 'after index 1 (skipping the verifier
segment)' — phrasing left over from before the codeVerifier segment existed.
The storage format is nonce:codeVerifier:state (3 segments), and the actual
implementation (line 85) skips both the nonce (index 0) and codeVerifier
(index 1) segments, with state starting at index 2 — not just 'the verifier
segment' as the old wording implied.
Doc-only change; no logic or test changes needed.
* fix: preserve state from legacy 2-segment redirect values (Copilot PR review)
RedirectHelper.state and getCodeVerifier() always assumed the current
3-segment storage format (nonce:codeVerifier:state). If a user initiates a
login redirect on a pre-DPoP SDK version (which wrote the legacy 2-segment
nonce:state format) and the app is upgraded to a newer SDK version before
they land back — e.g. a deploy that happens while they're on FusionAuth's
hosted login page — the leftover legacy value would be misparsed: state
would resolve to undefined instead of the real value.
Fix: detect the legacy format unambiguously. The current writer
(handlePreRedirect) always includes a codeVerifier segment, even when empty,
so any value it produces has at least two colons. A stored value with
exactly one colon can therefore only be the legacy format.
- state getter: if there are exac…
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.
Issue:
Description:
Implement the storage for the Cryptographic Key Pair (DPoPStorage) and the storage for the tokens (DPoPTokenStore)