feat: JavaScript SDK supports DPoP - #211
Open
mrudatsprint wants to merge 21 commits into
Open
Conversation
* 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: 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: 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
…ens (#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: 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: SDKCore - implement startLogout() and getAccessToken() for DPoP mode (ENG-4802)
- startLogout() in DPoP mode now awaits DPoPManager.clear() (key pair,
tokens, nonces) before redirecting, mirroring startLogin()'s
fire-and-forget async pattern. Cookie mode is unchanged.
- New DPoPManager.getAccessToken() delegate (mirrors getRefreshToken()).
- New public SDKCore.getAccessToken(): returns the stored DPoP access
token, or throws in cookie mode.
- Unit tests for both in SDKCore.test.ts and DPoPManager.test.ts.
- e2e dpop-smoke.test.ts: new startLogout() smoke test reusing the
logged-in SDKCore from the authorization code grant test.
* feat: rebuild the lock file.
* feat: remove verbose comments.
* feat: remove verbose comments.
* feat: copilot recommendation .
* feat: cleanup comments.
* feat: reduce commenting.
* feat: insure logout url is called.
* feat: use hosted backend mode versus cookie mode in comments.
* feat: remove verbose comments.
* 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: SDKCore - implement startLogout() and getAccessToken() for DPoP mode (ENG-4802)
- startLogout() in DPoP mode now awaits DPoPManager.clear() (key pair,
tokens, nonces) before redirecting, mirroring startLogin()'s
fire-and-forget async pattern. Cookie mode is unchanged.
- New DPoPManager.getAccessToken() delegate (mirrors getRefreshToken()).
- New public SDKCore.getAccessToken(): returns the stored DPoP access
token, or throws in cookie mode.
- Unit tests for both in SDKCore.test.ts and DPoPManager.test.ts.
- e2e dpop-smoke.test.ts: new startLogout() smoke test reusing the
logged-in SDKCore from the authorization code grant test.
* feat: rebuild the lock file.
* feat: remove verbose comments.
* feat: remove verbose comments.
* feat: SDKCore - implement refreshToken() for DPoP mode (ENG-4801)
- refreshToken() branches to a new refreshDpopToken() when useDpop is
enabled: reads the stored refresh token from DPoPManager, generates a
DPoP proof for the token endpoint (no ath), POSTs grant_type=refresh_token
to /oauth2/token with a DPoP header, updates DPoPManager's stored tokens
on success, and reschedules token expiration / auto-refresh (gated on
shouldAutoRefresh) from the new expiresAt.
- Throws a descriptive error if no refresh token is stored.
- Cookie-mode refreshToken() behavior is unchanged.
- Adds unit tests covering the DPoP request shape, token update, error
paths, and expiration/auto-refresh rescheduling.
- Replaces the pre-SDKCore raw refresh-token-grant e2e smoke test with one
that exercises SDKCore.refreshToken() directly against a live FusionAuth
instance.
* feat: refresh token grant
* feat: remove verbose comments.
* feat: copilot recommendation .
* feat: cleanup comments.
* feat: reduce commenting.
* feat: insure logout url is called.
* feat: use hosted backend mode versus cookie mode in comments.
* feat: remove verbose comments.
* feat: update from the last merge.
* feat: remove comment verbosity.
* feat: remove comment verbosity.
* fix: address Copilot review comments on refreshDpopToken() (PR #206)
- Preserve the existing refresh token when FusionAuth's refresh response
omits refresh_token (no rotation), instead of clearing it out and
breaking future refreshes.
- Read the token response via response.clone().json() so the Response
returned to callers still has an unconsumed body.
- Test: mockTokenResponse() now returns a fresh Response per fetch() call
via mockImplementation, avoiding a 'body already used' error when
refreshToken() is invoked more than once in a test (e.g. explicit call +
auto-refresh timer firing).
- Test: add coverage for the no-rotation case, asserting the original
refresh token is still used on a subsequent refresh.
* feat: preserve the existing refresh token, if needed.
* fix: dpop-smoke.test.ts refresh token test — undefined var + wrong order
'refresh token grant — issues new DPoP-bound tokens' had two compounding
bugs after being resurrected via a merge:
1. test.skip(!refreshToken, ...) referenced a variable that was never
declared in this file (ReferenceError). Every other test in the file
uses the !accessToken skip-guard convention -- switch to that.
2. The test requires core to still be logged in (asserts core.isLoggedIn
and calls core.refreshToken()), but it ran *after*
'startLogout() clears DPoP state...', which already logs core out.
Move it back to run right after the authorization code grant test and
before startLogout(), matching its actual dependency.
* 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: SDKCore - implement startLogout() and getAccessToken() for DPoP mode (ENG-4802)
- startLogout() in DPoP mode now awaits DPoPManager.clear() (key pair,
tokens, nonces) before redirecting, mirroring startLogin()'s
fire-and-forget async pattern. Cookie mode is unchanged.
- New DPoPManager.getAccessToken() delegate (mirrors getRefreshToken()).
- New public SDKCore.getAccessToken(): returns the stored DPoP access
token, or throws in cookie mode.
- Unit tests for both in SDKCore.test.ts and DPoPManager.test.ts.
- e2e dpop-smoke.test.ts: new startLogout() smoke test reusing the
logged-in SDKCore from the authorization code grant test.
* feat: rebuild the lock file.
* feat: remove verbose comments.
* feat: remove verbose comments.
* feat: SDKCore - implement refreshToken() for DPoP mode (ENG-4801)
- refreshToken() branches to a new refreshDpopToken() when useDpop is
enabled: reads the stored refresh token from DPoPManager, generates a
DPoP proof for the token endpoint (no ath), POSTs grant_type=refresh_token
to /oauth2/token with a DPoP header, updates DPoPManager's stored tokens
on success, and reschedules token expiration / auto-refresh (gated on
shouldAutoRefresh) from the new expiresAt.
- Throws a descriptive error if no refresh token is stored.
- Cookie-mode refreshToken() behavior is unchanged.
- Adds unit tests covering the DPoP request shape, token update, error
paths, and expiration/auto-refresh rescheduling.
- Replaces the pre-SDKCore raw refresh-token-grant e2e smoke test with one
that exercises SDKCore.refreshToken() directly against a live FusionAuth
instance.
* feat: refresh token grant
* feat: remove verbose comments.
* chore: upgrade vitest to v3.2.6 for core, lexicon, and sdk-react
Matches the version already used by sdk-vue. These three packages were
still on vitest v1.x (released Feb 2025). vite stays unchanged since
each package's current vite version already satisfies vitest v3's
peer range (^5.0.0 || ^6.0.0 || ^7.0.0-0). sdk-angular is unaffected
(already on v4).
All test suites, lint, format, and builds pass unchanged.
* feat: add DPoP to the React SDK, sync the version of vitest being used by the SDKs
* feat: copilot recommendation .
* feat: cleanup comments.
* feat: reduce commenting.
* feat: insure logout url is called.
* feat: use hosted backend mode versus cookie mode in comments.
* feat: remove verbose comments.
* feat: update from the last merge.
* feat: remove comment verbosity.
* feat: remove comment verbosity.
* feat: reduce verbose commenting.
* fix: address Copilot review comments on refreshDpopToken() (PR #206)
- Preserve the existing refresh token when FusionAuth's refresh response
omits refresh_token (no rotation), instead of clearing it out and
breaking future refreshes.
- Read the token response via response.clone().json() so the Response
returned to callers still has an unconsumed body.
- Test: mockTokenResponse() now returns a fresh Response per fetch() call
via mockImplementation, avoiding a 'body already used' error when
refreshToken() is invoked more than once in a test (e.g. explicit call +
auto-refresh timer firing).
- Test: add coverage for the no-rotation case, asserting the original
refresh token is still used on a subsequent refresh.
* feat: preserve the existing refresh token, if needed.
* fix: reapply refreshDpopToken() Copilot fixes lost in the eng-4801 merge
The merge of miker/eng-4801/refresh-token into this branch reintroduced the
pre-fix version of refreshDpopToken(), silently dropping the two Copilot
review fixes from PR #206:
- Preserve the existing refresh token when FusionAuth's refresh response
omits refresh_token (no rotation), instead of clearing it out.
- Read the token response via response.clone().json() so the Response
returned to callers still has an unconsumed body.
Also updates the unit test's mockTokenResponse() to return a fresh Response
per fetch() call (mockImplementation instead of mockResolvedValue), and
restores the regression test for the no-rotation case.
* fix: DPoP mode startLogout() targets /oauth2/logout directly
DPoP mode has no hosted backend to proxy through, so startLogout() should
never target /app/logout/ (a hosted-backend-only path that does not exist
on a raw FusionAuth server). Add UrlHelper.getOAuth2LogoutUrl(), which
builds the direct FusionAuth /oauth2/logout URL (client_id +
post_logout_redirect_uri), and switch SDKCore.startDpopLogout() to use it.
Cookie-mode getLogoutUrl()/startLogout() are unchanged.
Update dpop-smoke.test.ts's logout assertion accordingly, and add unit
coverage in UrlHelper.test.ts and SDKCore.test.ts.
* test: add DPoP endpoint e2e tests (dpop-endpoints.test.ts)
Mirrors endpoints.test.ts for DPoP mode, driving a consuming quickstart
application (useDpop: true) through its UI and validating the direct
FusionAuth calls SDKCore makes instead of the hosted backend API:
- Login: /oauth2/authorize (dpop_jkt, code_challenge/S256) followed by the
direct /oauth2/token authorization_code exchange (DPoP header, tokens
landing in localStorage instead of app.* cookies).
- Refresh: waits for the configured auto-refresh window to fire a direct
/oauth2/token refresh_token grant (DPoP header, new access token).
- Logout: direct /oauth2/logout navigation and local DPoP state cleared.
Register and fetching user info remain known gaps (not yet DPoP-aware in
SDKCore) and are documented as such in the file header.
Also:
- Add playwright.dpop-endpoints.config.ts (mirrors the main config, scoped
to this file via testMatch, keeps webServer/SERVER_COMMAND/PORT support).
- Exclude both DPoP e2e files (dpop-smoke.test.ts, dpop-endpoints.test.ts)
from the main playwright.config.ts via testIgnore, since they require
different FusionAuth Application configs than the hosted-backend-mode
quickstart used by endpoints.test.ts/cookies.test.ts.
- Add a test:e2e:dpop-endpoints root script.
- Update README's DPoP E2E tests section accordingly.
* feat: SDKCore.fetchUserInfo() is DPoP aware (ENG-4931)
- Add UrlHelper.getUserInfoUrl(), building the direct FusionAuth
/oauth2/userinfo URL, matching the getTokenUrl()/getOAuth2LogoutUrl()
pattern.
- fetchUserInfo() now branches to a new fetchDpopUserInfo() in DPoP mode:
reads the stored access token from DPoPManager, throws a descriptive
error if not logged in, and calls DPoPManager.fetch() against
/oauth2/userinfo directly instead of the hosted backend's /app/me.
DPoPManager.fetch() already handles the DPoP proof (with ath),
Authorization/DPoP headers, and nonce-retry dance.
- Cookie-mode fetchUserInfo()/getMeUrl() behavior is unchanged.
- No changes needed to React/Vue/Angular SDKs -- all three call
core.fetchUserInfo() generically and get this transparently.
- Add unit tests for UrlHelper.getUserInfoUrl() and
SDKCore.fetchUserInfo() DPoP mode (success, no-access-token error,
non-OK error, cookie-mode unaffected).
- dpop-endpoints.test.ts: add an e2e test asserting fetchUserInfo() hits
/oauth2/userinfo directly (with a DPoP header) and that the fetched
claims surface correctly in the app's UI. Update the file's "known
gaps" header comment -- only Register remains.
- README: update the DPoP E2E tests section accordingly.
* feat: cleanup
* feat: test user info endpoint.
* fix: React SDK auto-fetches userInfo after async DPoP login, not just at mount
useUserInfo()'s auto-fetch previously checked core.isLoggedIn synchronously,
exactly once, inside a ref-guarded effect. This works in cookie mode (the
post-redirect callback does a full page reload, so isLoggedIn is already
true by the time the app re-mounts), but breaks DPoP mode: the code
exchange happens asynchronously within the same already-mounted app, so
isLoggedIn is still false the one time the guard checks it, and
fetchUserInfo() is never called -- shouldAutoFetchUserInfo silently does
nothing for the rest of the session.
- useUserInfo() now accepts isLoggedIn as an explicit reactive parameter
instead of reading core.isLoggedIn directly, so the auto-fetch effect
re-evaluates when isLoggedIn transitions to true (still only fetching
once, via the existing ref guard).
- FusionAuthProvider passes its own isLoggedIn state through.
- Add a regression test simulating the DPoP flow (isLoggedIn starts
false, flips true once the post-redirect token exchange settles) and
asserting userInfo is fetched once that happens.
Vue's createFusionAuth() has the same synchronous-check pattern (and
Angular's auto-refresh check does too), but neither exposes useDpop yet
(ENG-4788/ENG-4789), so it's currently dormant there -- worth revisiting
once DPoP wiring lands for those SDKs.
* test: merge userinfo check into the auto-refresh test
Moves the standalone 'User info is fetched directly from /oauth2/userinfo'
test's assertions into 'Access token auto-refreshes...' (renamed to 'User
info is fetched after login, and the access token auto-refreshes via a
direct /oauth2/token refresh_token grant'), right after authenticate()
and before the refresh-window wait.
Both are automatic, no-user-interaction behaviors that fire post-login
(shouldAutoFetchUserInfo and shouldAutoRefresh), so combining them saves
one login/authenticate cycle. File now has 3 tests: Login, this merged
one, and Logout.
* debug: temporary diagnostics for the /oauth2/userinfo investigation
Logs every request/response/requestfailed to/from FusionAuth (localhost:9011),
plus browser console messages and page errors, to the Playwright terminal
output. Should be reverted once the root cause of the userinfo test
timeout is found.
* Revert "debug: temporary diagnostics for the /oauth2/userinfo investigation"
This reverts commit 02ed28a.
* docs: document the FusionAuth CORS prerequisite for /oauth2/userinfo
Root-caused the userinfo test timeout: the browser's CORS preflight for
/oauth2/userinfo fails with 'No Access-Control-Allow-Origin header is
present', so the request never reaches FusionAuth at all. Confirmed via
live diagnostics that /oauth2/authorize and /oauth2/token succeed
cross-origin (handled independently of the System CORS filter), but
/oauth2/userinfo does not -- this is a FusionAuth Application/System CORS
configuration gap, not a bug in SDKCore/DPoPManager.
Document the required CORS configuration (Settings -> System -> CORS:
enable the filter, add the quickstart's origin, add DPoP and Authorization
to Allowed headers) in the file's Prerequisites section.
* feat: remove verbose comments.
* fix: prevent duplicate DPoP authorization code exchange on concurrent handlePostRedirect() calls
SDKCore.handlePostRedirect() only cleared the pending `code` query
param and persisted `code_verifier` *after* a successful /oauth2/token
exchange. A second call arriving while the first was still in flight
(e.g. React StrictMode's mount -> cleanup -> mount double-invoke of
effects, or an un-memoized onRedirect prop retriggering the effect)
would read the same still-pending code/verifier and re-POST to
/oauth2/token, exchanging the same authorization code twice — visible
as two 'exchange authorization code' debug entries in the FusionAuth
event log.
Fix: memoize handlePostRedirect()'s promise (single-flight, mirroring
DPoPManager.getOrCreateKeyPair()'s keyPairPromise pattern) so repeated
or concurrent calls on the same SDKCore instance always return the
same in-flight/settled promise instead of re-entering
handleDpopPostRedirect().
- packages/core/src/SDKCore/SDKCore.ts: add postRedirectPromise guard.
- packages/core/src/SDKCore/SDKCore.test.ts: add regression tests
(verified both fail without the fix).
- e2e/tests/dpop-endpoints.test.ts: track every authorization_code
exchange request during login and assert exactly one occurs.
sdk-angular's vendored SDKCore.ts copy is gitignored and generated via
`yarn get-sdk-core` (packages/sdk-angular/getSDKCore.js), so it picks
up this fix automatically — verified locally, nothing to commit there.
sdk-vue consumes @fusionauth-sdk/core directly and gets the fix for
free too.
* feat: cleanup duplicate code grant exchanges being tracket.
* fix: dpop-smoke.test.ts refresh token test — undefined var + wrong order
'refresh token grant — issues new DPoP-bound tokens' had two compounding
bugs after being resurrected via a merge:
1. test.skip(!refreshToken, ...) referenced a variable that was never
declared in this file (ReferenceError). Every other test in the file
uses the !accessToken skip-guard convention -- switch to that.
2. The test requires core to still be logged in (asserts core.isLoggedIn
and calls core.refreshToken()), but it ran *after*
'startLogout() clears DPoP state...', which already logs core out.
Move it back to run right after the authorization code grant test and
before startLogout(), matching its actual dependency.
* feat: The Angular Framework has is using the DPoP functionality in the Core Package.
* feat: The Vue Framework using DPoP.
* feat: remove verbose comments.
* feat: remove verbose comments.
* feat: remove test duplication.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds DPoP (RFC 9449) support end-to-end across the monorepo, with the core package implementing DPoP key/token management + PKCE and the React/Angular/Vue SDKs exposing DPoP-aware APIs and updated post-redirect behavior. This is primarily implemented in packages/core and then surfaced via framework-specific wrappers, docs, and E2E coverage.
Changes:
- Introduces core DPoP primitives (
DPoPManager, IndexedDB key storage, token store) plus PKCE utilities and direct OAuth2 URL helpers. - Exposes DPoP mode configuration (
useDpop,dpopTokenStorage) and optional DPoP helpers (dpopFetch,generateProof,getAccessToken) in React/Vue contexts and Angular service APIs. - Adds unit + E2E test coverage and documentation for DPoP mode, including a dedicated Playwright config for DPoP endpoint tests.
Reviewed changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents DPoP E2E tests |
| playwright.dpop-endpoints.config.ts | New Playwright config for DPoP endpoints |
| playwright.config.ts | Ignores DPoP endpoints test in default run |
| packages/sdk-vue/web-types.json | Bumps web-types version metadata |
| packages/sdk-vue/src/types.ts | Adds DPoP config + optional helpers types |
| packages/sdk-vue/src/createFusionAuth/createFusionAuth.ts | Exposes DPoP helpers; improves post-redirect syncing |
| packages/sdk-vue/src/createFusionAuth/createFusionAuth.test.ts | Adds DPoP-mode behavior tests |
| packages/sdk-vue/README.md | Adds DPoP mode docs and examples |
| packages/sdk-vue/package.json | Version bump to 1.4.0 |
| packages/sdk-vue/CHANGES.md | Changelog entry for DPoP mode |
| packages/sdk-react/src/testing-tools/mocks/createContextMock.ts | Adds DPoP fields to context mock |
| packages/sdk-react/src/components/providers/hooks/useUserInfo.ts | Auto-fetch gated by isLoggedIn |
| packages/sdk-react/src/components/providers/hooks/useRedirecting.ts | Awaits post-redirect settling hook |
| packages/sdk-react/src/components/providers/hooks/useDpop.ts | New hook to expose DPoP helpers |
| packages/sdk-react/src/components/providers/hooks/index.ts | Exports new useDpop hook |
| packages/sdk-react/src/components/providers/FusionAuthProviderContext.ts | Adds optional DPoP helper APIs |
| packages/sdk-react/src/components/providers/FusionAuthProviderConfig.ts | Adds DPoP config options |
| packages/sdk-react/src/components/providers/FusionAuthProvider.tsx | Wires DPoP + post-redirect state sync |
| packages/sdk-react/src/components/providers/FusionAuthProvider.test.tsx | Adds DPoP-mode provider tests |
| packages/sdk-react/README.md | Adds DPoP mode docs and examples |
| packages/sdk-react/package.json | Version bump; vitest version bump |
| packages/sdk-react/CHANGES.md | Changelog entry for DPoP mode |
| packages/sdk-angular/README.md | Adds DPoP mode docs and examples |
| packages/sdk-angular/projects/fusionauth-angular-sdk/src/lib/types.ts | Adds DPoP config options |
| packages/sdk-angular/projects/fusionauth-angular-sdk/src/lib/fusion-auth.service.ts | Adds Signal; ensures zone/tick updates post-redirect |
| packages/sdk-angular/projects/fusionauth-angular-sdk/src/lib/fusion-auth.service.spec.ts | Adds DPoP-mode service tests |
| packages/sdk-angular/projects/fusionauth-angular-sdk/package.json | Version bump; adds dpop dependency |
| packages/sdk-angular/projects/fusionauth-angular-sdk/ng-package.json | Allows dpop non-peer dependency |
| packages/sdk-angular/CHANGES.md | Changelog entry for DPoP mode |
| packages/lexicon/package.json | Vitest version bump |
| packages/core/src/UrlHelper/UrlHelperTypes.ts | Adds direct OAuth2 query params types |
| packages/core/src/UrlHelper/UrlHelper.ts | Adds direct OAuth2 authorize/token/userinfo/logout URL builders |
| packages/core/src/UrlHelper/UrlHelper.test.ts | Adds tests for new direct OAuth2 URL helpers |
| packages/core/src/testUtils/mockWindowLocation.ts | Allows mocking location.search |
| packages/core/src/SDKCore/SDKCore.ts | Implements DPoP mode flows + exposes DPoP helpers |
| packages/core/src/SDKCore/SDKCore.test.ts | Adds extensive DPoP mode test suite |
| packages/core/src/SDKContext/SDKContext.ts | Adds optional DPoP helper APIs to context |
| packages/core/src/SDKConfig/SDKConfig.ts | Adds DPoP config + onLoginFailure hook |
| packages/core/src/RedirectHelper/RedirectHelper.ts | Stores state + PKCE verifier (JSON) for DPoP mode |
| packages/core/src/RedirectHelper/RedirectHelper.test.ts | Adds tests for both redirect storage formats |
| packages/core/src/Pkce/Pkce.ts | New PKCE verifier/challenge utilities |
| packages/core/src/Pkce/Pkce.test.ts | Adds RFC 7636 test vectors |
| packages/core/src/Pkce/index.ts | Barrel export for PKCE |
| packages/core/src/index.ts | Exports new DPoP + PKCE modules |
| packages/core/src/DPoP/index.ts | Barrel export for DPoP module |
| packages/core/src/DPoP/DPoPTokenStore.ts | Implements localStorage/memory token storage |
| packages/core/src/DPoP/DPoPTokenStore.test.ts | Tests for token store backends |
| packages/core/src/DPoP/DPoPStorage.ts | IndexedDB-backed keypair storage |
| packages/core/src/DPoP/DPoPStorage.test.ts | Tests IndexedDB key storage + error paths |
| packages/core/src/DPoP/DPoPManager.ts | Implements DPoP proof generation + fetch wrapper |
| packages/core/src/DPoP/DPoPManager.test.ts | Tests proof generation, nonce retry, clear behavior |
| packages/core/package.json | Adds dpop; adds fake-indexeddb; bumps vitest |
| package.json | Adds script for DPoP endpoint E2E run |
| e2e/tests/dpop-endpoints.test.ts | New DPoP endpoint E2E test suite |
| e2e/pages/common.page.ts | Stabilizes login/logout navigation waits |
| AGENTS.md | Adds repo dev/testing conventions doc |
| .github/workflows/run-tests.yml | Runs on all PR branches |
| .github/workflows/lint-and-format.yml | Runs on all PR branches |
| .github/CODEOWNERS | Adds @fusionauth/sdk-owners for .github/ |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mrudatsprint
marked this pull request as ready for review
August 3, 2026 22:29
lyleschemmerling
requested changes
Aug 4, 2026
| callback?: (state?: string) => void, | ||
| ): Promise<void> { | ||
| const code = new URLSearchParams(window.location.search).get('code'); | ||
| const codeVerifier = this.redirectHelper.getCodeVerifier(); |
There was a problem hiding this comment.
We should also be checking the state parameter here. Basically to prevent CSRF we should check the state after the redirect before the code exchange.
We discuss that here: https://fusionauth.io/articles/oauth/modern-guide-to-oauth#authorize-endpoint-parameters
and is called out in the OAuth spec here:
Collaborator
Author
There was a problem hiding this comment.
now checks the
state parameter.
| throw new Error(JSON.stringify(errorDetails)); | ||
| } | ||
|
|
||
| const tokenResponse = await response.clone().json(); |
There was a problem hiding this comment.
you should check here that the token response actually has "token_type": "DPoP". We could have also returned Bearer
Collaborator
Author
There was a problem hiding this comment.
The token_type is now checked in
The function is called during both grant's.
| this.dpopManager!.setTokens({ | ||
| accessToken: tokenResponse.access_token, | ||
| refreshToken: tokenResponse.refresh_token, | ||
| expiresAt: Date.now() + tokenResponse.expires_in * 1000, |
There was a problem hiding this comment.
DPoPTokenStore says expired is tokens.expiresAt <= Date.now(). If expires_in is not a positive integer than you could return true for expired tokens
Collaborator
Author
There was a problem hiding this comment.
This is also fixed in toDpopTokens
| } | ||
|
|
||
| if (this.dpopManager) { | ||
| this.postRedirectPromise = this.handleDpopPostRedirect(callback).catch( |
There was a problem hiding this comment.
this method is already async, prefer using try/catch and await instead of Promise.catch
Collaborator
Author
There was a problem hiding this comment.
Fixed in
| * Backend Mode. | ||
| */ | ||
| handlePostRedirect(callback?: (state?: string) => void): Promise<void> { | ||
| if (this.postRedirectPromise) { |
There was a problem hiding this comment.
I think what this is trying to do is prevent concurrent authorization requests, but the use of this promise as some kind of sdk state is strange. I recommend just removing this.postRedirectPromise altogether and labeling this method as properly async. That should shake out the weirdness.
Collaborator
Author
There was a problem hiding this comment.
Fixed in
| */ | ||
| async getUserInfo<T>(): Promise<T> { | ||
| return await this.core.fetchUserInfo<T>(); | ||
| return this.core.fetchUserInfo<T>().then(userInfo => { |
| * headers. | ||
| * @throws {Error} if called when `useDpop` is not enabled. | ||
| */ | ||
| async dpopFetch( |
There was a problem hiding this comment.
we offer isLoggedIn as an Observable and a Signal but this fetch method returns a promise. This feels inconsistent.
Collaborator
Author
There was a problem hiding this comment.
The thought is the actions such as dpopFetch, generateProof, refreshToken and getUserInfo are actions whereas isLoggedIn$ and isLoggedInSignal maintain state changes.
| useEffect(() => { | ||
| core.handlePostRedirect(onRedirect); | ||
| }, [core, onRedirect]); | ||
| core.handlePostRedirect(onRedirect).then(() => { |
| if ( | ||
| response.status === 401 && | ||
| this._isUseNonceError(response) && | ||
| response.headers.has('DPoP-Nonce') |
There was a problem hiding this comment.
feels like this line should go in _isUseNonceHeader
| // attempt and a potential retry each get an independent, unconsumed body. | ||
| // Request.clone() safely tees any internal streaming body per spec, so | ||
| // this also covers a Request constructed with a ReadableStream body. | ||
| const primaryInput = input instanceof Request ? input.clone() : input; |
There was a problem hiding this comment.
There are several instanceof calls in this method but input is only required to meet the contract of the interface and not provide concrete instances of an object, which will fail silently. Consider duck typing or type predicates instead of instanceof
Collaborator
Author
There was a problem hiding this comment.
Resolved using the function
lyleschemmerling
requested changes
Aug 4, 2026
lyleschemmerling
left a comment
lyleschemmerling
left a comment
There was a problem hiding this comment.
some more things that came out of review, a couple that look problematic
| this.stopAutoRefresh(); | ||
|
|
||
| if (this.dpopManager) { | ||
| this.startDpopLogout().catch(error => { |
| clearTimeout(this.tokenExpirationTimeout); | ||
| this.stopAutoRefresh(); | ||
|
|
||
| if (this.dpopManager) { |
There was a problem hiding this comment.
there are cases here where the presence of the dpopManager means "we are in dpop mode", and there are other places, like in startDpopLogout or refreshDpopToken that assume the dpopManager will be there. I think the methods where we do the latter are actually probably more appropriate in the DPoPManager
Collaborator
Author
There was a problem hiding this comment.
The commit 8e52bb8 moves the methods to DPoPManager
and next wire them into SDKCore
Collaborator
Author
There was a problem hiding this comment.
commit 1becf41 delegates DPoP flows to the DPoPManager
| */ | ||
| startLogin(state?: string): void { | ||
| if (this.dpopManager) { | ||
| this.startDpopLogin(state).catch(error => { |
| ); | ||
| } | ||
|
|
||
| startRegister(state?: string) { |
There was a problem hiding this comment.
it doesn't look like the register pathway got touched at all. Does it need to?
Collaborator
Author
There was a problem hiding this comment.
I didn't believe it needed to be touched. Now, that means /app/register will be called which in turn redirects to /oauth2/register. If that's confusing to the application programmer, we can directly call /oauth2/register if in DPoP mode?
Collaborator
Author
There was a problem hiding this comment.
We should call /oauth2/register otherwise/app/callback will be called
|
|
||
| const { manageAccount, startLogin, startLogout, startRegister } = | ||
| useRedirecting(core, config.onRedirect); | ||
| useRedirecting(core, config.onRedirect, syncIsLoggedIn); |
There was a problem hiding this comment.
onRedirect runs before syncIsLoggedIn sets the react state. There is a race there.
| this.isLoggedIn$ = toObservable(this.isLoggedInState); | ||
|
|
||
| this.core.handlePostRedirect(config.onRedirect); | ||
| this.core.handlePostRedirect(config.onRedirect).then(() => { |
There was a problem hiding this comment.
await. same thing as react, you could have login out of order. Vue handles this correctly
|
|
||
| core.handlePostRedirect(config.onRedirect); | ||
| core.handlePostRedirect(state => { | ||
| syncIsLoggedIn(); |
There was a problem hiding this comment.
this correctly handles syncIsLoggedIn before onRedirect
| } | ||
|
|
||
| async refreshToken(): Promise<Response> { | ||
| if (this.dpopManager) { |
There was a problem hiding this comment.
Just going to give the full context that I got here:
Each framework publishes false when the access token expires:
onTokenExpiration: () => {
isLoggedIn = false;
}But their public refresh wrappers only delegate to core.
React:
const refreshToken = useCallback(
async () => await core.refreshToken(),
[core],
);Vue:
async function refreshToken() {
return await core.refreshToken();
}Angular:
async refreshToken(): Promise<Response> {
return await this.core.refreshToken();
}Core successfully stores the new token and now reports core.isLoggedIn === true, but none of the frameworks republish that value.
Result:
- React context remains false.
- Vue ref remains false.
- Angular signal/observable remains false, even though its direct isLoggedIn() method reads true from core.
This is easiest to reproduce by letting the access token expire, retaining a valid refresh token, then invoking the public refreshToken() API.
| * the error via `console.error` if not provided. Only relevant when | ||
| * `useDpop: true`. | ||
| */ | ||
| onLoginFailure?: (error: Error) => void; |
There was a problem hiding this comment.
Core defines:
onLoginFailure?: (error: Error) => void;This is important because startLogin() returns void while key generation, IndexedDB, Web Crypto, and PKCE work asynchronously. It is also used for token-exchange failures.
But React, Vue, and Angular public config types omit it.
React is definitively blocked because FusionAuthProvider.tsx:26–48 manually copies supported properties and does not forward onLoginFailure.
Vue and Angular spread their configs into core, so an untyped runtime property might pass through, but TypeScript consumers cannot configure it through the supported API.
Consequences:
- Applications cannot show login failures.
- They cannot recover from denied IndexedDB/Web Crypto.
- Exchange failures generally fall back to console.error.
Core tests cover the callback, including the exchange failure test at SDKCore.test.ts:733–750. Framework tests do not cover forwarding it.
| // attempt and a potential retry each get an independent, unconsumed body. | ||
| // Request.clone() safely tees any internal streaming body per spec, so | ||
| // this also covers a Request constructed with a ReadableStream body. | ||
| const primaryInput = isRequestLike(input) ? input.clone() : input; |
There was a problem hiding this comment.
Suggested change
| const primaryInput = isRequestLike(input) ? input.clone() : input; | |
| const primaryInput = new Request(input, init); |
| // Request.clone() safely tees any internal streaming body per spec, so | ||
| // this also covers a Request constructed with a ReadableStream body. | ||
| const primaryInput = isRequestLike(input) ? input.clone() : input; | ||
| const retryInput = isRequestLike(input) ? input.clone() : input; |
There was a problem hiding this comment.
Suggested change
| const retryInput = isRequestLike(input) ? input.clone() : input; | |
| const retryInput = primaryInput.clone(); |
the issue is you are trying to mimick the native fetch api and handle a retry without knowing what object they gave you. This will do a better job ensuring that
Contributor
|
reviewed — no security concerns. |
| * @param codeVerifier Optional PKCE `code_verifier` (DPoP mode only). | ||
| */ | ||
| handlePreRedirect(state?: string, codeVerifier?: string) { | ||
| const isDpopMode = codeVerifier !== undefined; |
There was a problem hiding this comment.
handlePreRedirect handles two distinct transaction formats and infers the mode from whether codeVerifier !== undefined. That is brittle: an empty string—or null from JavaScript despite the TypeScript signature—selects DPoP mode while producing an unusable verifier. Consider separate methods or a discriminated configuration object so mode is explicit and the verifier can be validated.
More importantly, caller-provided application state, OAuth transaction state, and the PKCE verifier are currently conflated. The hosted-mode random value is only a local marker; it is never echoed or compared. In DPoP mode, state is optional and may not be unpredictable. The PKCE verifier protects code exchange, but it is not the callback state value.
Could the SDK generate a distinct random OAuth transaction state, persist it with the verifier and optional application state, send it in the authorization request, and require an exact callback match?
…review, batch 2 - 2/7)
… (PR #211 review, batch 2 - 2/7)
…Auth/fusionauth-javascript-sdk into parent/dpop-in-the-javascript-sdk
…, batch 2 - 4/7) # Conflicts: # packages/core/src/SDKCore/SDKCore.ts
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.
Issues:
Description:
The JavaScript SDK supports DPoP in the React, Angular and Vue Framework's.