feat: additional end to end tests - #208
Closed
mrudatsprint wants to merge 117 commits into
Closed
Conversation
- Add `dpop` v2.1.1 runtime dependency and `fake-indexeddb` devDependency to packages/core/package.json - SDKConfig: add optional `useDpop` and `dpopTokenStorage` fields - UrlHelper: add `getAuthorizeUrl(state?, dpopJkt?, codeChallenge?)` targeting FusionAuth /oauth2/authorize directly; update UrlHelperTypes to include response_type, code_challenge, code_challenge_method, dpop_jkt - DPoPStorage: IndexedDB abstraction for ES256 CryptoKeyPair persistence (db: fusionauth-sdk:dpop, store: keypair, keyed by clientId) - DPoPTokenStore: localStorage/memory token storage for DPoP-bound tokens (key: fusionauth-sdk:tokens:<clientId>); includes getAccessToken() and isExpired getter - packages/core/src/DPoP/index.ts re-exports both classes - 54 tests passing (21 DPoPTokenStore, 6 DPoPStorage, 16 UrlHelper, 7 SDKCore, 4 CookieHelpers)
- 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)
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.
…84/central-coordinator' of github.com:FusionAuth/fusionauth-javascript-sdk into miker/eng-4784/central-coordinator
…g object - Export DEFAULT_DPOP_DB_NAME, DEFAULT_DPOP_DB_VERSION, DEFAULT_DPOP_STORE_NAME as named constants (no hardcoded magic strings anywhere in the codebase) - Add DPoPStorageConfig interface with clientId (required) and optional dbName, dbVersion, storeName fields — each defaults to the exported constant - Refactor DPoPStorage constructor from positional (clientId: string) to config object, matching the UrlHelperConfig convention in this monorepo - openDb() and all three public methods now reference instance fields (this.dbName, this.dbVersion, this.storeName) instead of module constants - Add tests: defaults apply when no config overrides provided; custom dbName and storeName land data in the right database; two instances with different dbNames but the same clientId do not share keys; dbVersion downgrade produces a clean rejection (VersionError) - Update AGENTS.md: note the config-object constructor convention and the IndexedDB dbVersion must-only-increase constraint
…central-coordinator
…ENG-4782 refactor
…t (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.
…edirect 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.
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.
…ormat 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).
…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.
… 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.
…mption (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.
…opilot 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.
…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: 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.
…-4790) Extends dpop-endpoints.test.ts with three new tests covering the remaining ENG-4790 scenarios not yet exercised end-to-end: - dpopFetch() sets Authorization: DPoP and DPoP proof headers, with the proof's ath claim verified against base64url(SHA-256(accessToken)) - dpopFetch() retries exactly once with the server-provided nonce after a 401 use_dpop_nonce challenge - dpopFetch() does not retry a second time when the retry also returns 401 use_dpop_nonce These tests inject a second, independent SDKCore instance built directly from packages/core/dist/index.js (see injectDpopSdkCore()). No changes to the consuming quickstart application are required: DPoP key pairs (IndexedDB, keyed by clientId) and tokens (localStorage, keyed by fusionauth-sdk:tokens:<clientId>) are namespaced by clientId/origin rather than by JS object identity, so the injected instance transparently reuses state the quickstart's own React app already created via a normal login.
…n 401s The nonce-retry tests mock https://api.example.com, which is a different origin from the quickstart app. For cross-origin cors-mode fetches, the browser filters Response.headers down to the CORS-safelisted set unless Access-Control-Expose-Headers is present. DPoPManager.fetch() reads WWW-Authenticate and DPoP-Nonce off the response to detect a nonce challenge, so without exposing them the SDK silently saw no challenge and returned the first 401 without retrying (requestCount stayed at 1 instead of 2). Add Access-Control-Allow-Origin and Access-Control-Expose-Headers to the mocked 401 responses in both nonce-retry tests, and document the gotcha in the file header for future edits to these mocked routes.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds additional DPoP-focused end-to-end coverage (Playwright) and refines existing DPoP-related test wording to better reflect the flow being tested.
Changes:
- Add new Playwright e2e tests validating
dpopFetch()headers/athclaim and nonce-challenge retry behavior. - Update Vue SDK unit test descriptions/comments from “settles” to “completes” for clearer intent.
- Remove a now-unneeded JSDoc block for a private DPoP refresh method in
SDKCore.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/sdk-vue/src/createFusionAuth/createFusionAuth.test.ts | Wording tweaks in DPoP redirect-related unit tests for clarity. |
| packages/core/src/SDKCore/SDKCore.ts | Removes a doc comment on a private DPoP refresh helper. |
| e2e/tests/dpop-endpoints.test.ts | Adds new Playwright e2e coverage around dpopFetch() header/proof behavior and nonce retry logic. |
Suppressed comments (2)
e2e/tests/dpop-endpoints.test.ts:396
- Same as the other dpopFetch() route handlers: without handling OPTIONS preflight separately, the browser may never make it to the intended request, and requestCount will not reflect the actual GET attempts. The fulfilled responses should also include Access-Control-Allow-Origin so the browser doesn't block the fetch.
await page.route('https://api.example.com/always-nonce', route => {
e2e/tests/dpop-endpoints.test.ts:352
- This route handler likely intercepts the CORS preflight (OPTIONS) request as well as the actual GET, which will inflate requestCount and can cause the browser to block the response if the 200 retry doesn't include Access-Control-Allow-Origin. Handle OPTIONS separately and only increment requestCount for the real request method.
await page.route('https://api.example.com/nonce-protected', route => {
requestCount += 1;
if (requestCount === 1) {
route.fulfill({
status: 401,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix typos in injectDpopSdkCore() doc comment (dropFetch -> dpopFetch, getAcessToken -> getAccessToken). - Handle CORS preflight OPTIONS requests separately in all three mocked https://api.example.com routes via a new handleCorsPreflight() helper. dpopFetch() sends non-safelisted Authorization/DPoP headers, so cross-origin requests trigger a browser preflight OPTIONS request first. Since page.route() matches by URL regardless of method, the preflight was previously falling through to the same handler as the real request, which could inflate request counters and, when answered with a non-2xx status (e.g. the mocked 401), fail the preflight and block the real request from ever being sent.
Addresses the third (unaddressed) Copilot comment on PR #208: these tests require packages/core/dist/index.js to exist (injectDpopSdkCore imports it directly), but test:e2e:dpop-endpoints didn't build @fusionauth-sdk/core first, so a clean checkout would fail. Chain yarn build:core into test:e2e:dpop-endpoints, matching the existing build:sdk-react/build:sdk-vue convention of building core before consuming it. Update the file header's run instructions and note the manual yarn build:core step needed when running via npx playwright test directly.
mrudatsprint
commented
Aug 3, 2026
| } | ||
|
|
||
| /** | ||
| * Injects a second, independent `SDKCore` instance into the page. |
Collaborator
Author
There was a problem hiding this comment.
I added three dropFetch() tests. I believe they provide coverage for when a Resource Server API is not directly available.
| let requestCount = 0; | ||
| let retryDpopHeader: string | undefined; | ||
|
|
||
| await page.route('https://api.example.com/nonce-protected', route => { |
Collaborator
Author
There was a problem hiding this comment.
Note, the re-try logic is in dropFetch()
mrudatsprint
commented
Aug 3, 2026
mrudatsprint
left a comment
mrudatsprint
left a comment
Collaborator
Author
There was a problem hiding this comment.
self-review completed
mrudatsprint
marked this pull request as ready for review
August 3, 2026 15:16
* 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.
Base automatically changed from
mike/eng-4787/dpop-sdks
to
parent/dpop-in-the-javascript-sdk
August 3, 2026 22:01
mrudatsprint
commented
Aug 3, 2026
mrudatsprint
left a comment
mrudatsprint
left a comment
Collaborator
Author
There was a problem hiding this comment.
self-review completed
mrudatsprint
changed the title
mrudatsprint
requested review from
lyleschemmerling
and removed request for
wied03
August 3, 2026 23:04
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:
Add the testing of dropFetch() which is responsible for making resource server API calls and implementing re-try logic. These tests are not using a live resource server. They are using a second SDKCore instance to provide the functionality.