Skip to content

feat: The OIDC logout flow is supported - #205

Draft
mrudatsprint wants to merge 64 commits into
parent/dpop-in-the-javascript-sdkfrom
miker/eng-4802/start-logout
Draft

feat: The OIDC logout flow is supported#205
mrudatsprint wants to merge 64 commits into
parent/dpop-in-the-javascript-sdkfrom
miker/eng-4802/start-logout

Conversation

@mrudatsprint

@mrudatsprint mrudatsprint commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Issue:

Description:

The OIDC logout flow will clear the key pair, stored tokens and in-memory nonces.

Note, added an accessor to return the access token.

- 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
…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).
… 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.
@mrudatsprint
mrudatsprint changed the base branch from main to miker/eng-4800/code-grant-finish July 23, 2026 19:54
@mrudatsprint mrudatsprint changed the title feat: SDKCore: implement startLogout() and getAccessToken() (ENG-4802) feat: The OIDC logout flow is supported Jul 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for the OIDC logout flow in @fusionauth-sdk/core, ensuring DPoP-mode logout clears local DPoP state (key pair, tokens, nonces) before redirecting, and introduces an accessor for reading the DPoP-bound access token (with tests).

Changes:

  • Implement DPoP-aware startLogout() which clears DPoP state prior to redirecting to the logout URL.
  • Add getAccessToken() to SDKCore and DPoPManager, with corresponding unit tests.
  • Extend the DPoP e2e smoke suite to validate logout state clearing + redirect behavior.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
yarn.lock Dependency lock updates related to the repo’s toolchain/dev dependencies.
packages/core/src/SDKCore/SDKCore.ts Adds DPoP-aware logout flow and a getAccessToken() accessor on SDKCore.
packages/core/src/SDKCore/SDKCore.test.ts Unit tests for SDKCore.getAccessToken() behavior in DPoP vs cookie mode.
packages/core/src/DPoP/DPoPManager.ts Adds DPoPManager.getAccessToken() to expose the stored access token.
packages/core/src/DPoP/DPoPManager.test.ts Unit tests for DPoPManager.getAccessToken() and post-clear behavior.
e2e/tests/dpop-smoke.test.ts Adds an e2e test ensuring startLogout() clears DPoP state before redirect.
Comments suppressed due to low confidence (1)

packages/core/src/SDKCore/SDKCore.ts:109

  • In DPoP mode, if DPoPManager.clear() rejects, the .catch(...) handler prevents the redirect from happening at all, leaving the app stuck in a partially-logged-out state. Consider making the redirect unconditional (e.g., in a finally) and treat clearing local state as best-effort.
  startLogout(): void {
    if (this.dpopManager) {
      this.startDpopLogout().catch(error => {
        console.error('FusionAuth SDK: startLogout failed', error);
      });
      return;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/SDKCore/SDKCore.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/core/src/SDKCore/SDKCore.ts:101

  • The JSDoc has a grammatical error: “starting an asynchronously flow” should be “starting an asynchronous flow” (or similar).
   * In DPoP mode, this synchronously returns after starting an
   * asynchronously flow.
   *

Comment thread packages/core/src/SDKCore/SDKCore.ts

@mrudatsprint mrudatsprint left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self-review

/**
* Returns the stored access token string, or `null` if none is stored.
*/
getAccessToken(): string | null {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exposed so the application can call a resource server and is not able to use fetch. In this case the application needs to call generateProof and would need the accessToken in order to add it to the Authorization: DPoP header.

Base automatically changed from miker/eng-4800/code-grant-finish to parent/dpop-in-the-javascript-sdk July 28, 2026 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants