Skip to content

fix(token-vault): reliably end the AM SSO session on logout (SDKS-5448) - #595

Merged
ancheetah merged 3 commits into
developfrom
SDKS-5448-composed-urls
Oct 14, 2026
Merged

ancheetah merged 3 commits into
developfrom
SDKS-5448-composed-urls

Conversation

@vatsalparikh

@vatsalparikh vatsalparikh commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The token vault app's logout did not actually end the user's AM SSO session. The user appeared logged out, but the AM session survived — the next login silently re-authenticated via prompt=none with no login form.

Root cause

In the vault environment, FRUser.logout() (default path) fires endSession last — after TokenManager.deleteTokens() has already emptied the vault's token storage via TVP_REMOVE_TOKENS. The vault's endSession branch staples id_token_hint from its own storage, so by the time endSession fires, the only copy of the hint is gone → AM returns 400 and the SSO session survives.

Meanwhile the sessions?_action=logout leg is a page-direct fetch with no service-worker route, so with 5823 removed from AM's CORS policy (the mitigation for the third token-vault vulnerability), it is refused preflight and never reaches AM.

Two solutions

Solution A — fix the app (commit 1): Reorder logout: end the SSO session first, while the vault still holds the idToken, then revoke the grant, then clear vault storage:

await OAuth2Client.endSession();     // vault staples its stored id_token_hint → AM kills SSO session
await OAuth2Client.revokeToken();    // vault appends its stored token → AM kills the OAuth grant
await TokenManager.deleteTokens();   // TVP_REMOVE_TOKENS → vault clears its storage

No vault library changes needed — the existing endSession, revoke, and remove branches in the proxy already handle this correctly when called in the right order.

Solution B — fix the vault (commit 2): Route the AM session logout through the vault so stock FRUser.logout() works as-is:

  • generateAmUrls now includes the AM sessions endpoint URL
  • The service worker intercept list gains …/json/…/sessions/*
  • The proxy gains a sessions branch that relays _action=logout to AM with credentials: 'include', so the SSO cookie authenticates it (runs before the tokens-required gate, as it needs no tokens)

With this, FRUser.logout()'s first leg (session kill) succeeds through the vault; its trailing endSession 400s harmlessly on an already-dead session, and revoke + cleanup proceed as before.

Deploying either solution alone produces a complete logout; both together are also fine.

Testing

Verified against a live AM tenant:

Solution A (before commit 2):

  • Before: revoke returns 200 but next login silently re-authenticates (session survived); endSession 400s after vault storage is cleared.
  • After: endSession goes out armed with the real hint (204), revoke succeeds (200), vault storage clears, and the next login shows the AM login form instead of silently re-logging in.

Solution B (commit 2, live probe through the vault proxy):

  • POST …/json/realms/root/realms/alpha/sessions/?_action=logout relayed through the vault with a live session → 200 "Successfully logged out"
  • Same probe with a dead session → 401 (correct: nothing to end), proving the relay itself works and responses reach the page.

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 11ac633

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@forgerock/token-vault Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added the test label Oct 9, 2026
@nx-cloud

nx-cloud Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 11ac633

Command Status Duration Result
nx affected -t build lint test docs e2e-ci ✅ Succeeded <1s View ↗
nx-cloud record -- nx format:check --verbose ✅ Succeeded 4s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-10-14 13:21:02 UTC

@pkg-pr-new

pkg-pr-new Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/ForgeRock/forgerock-javascript-sdk/@forgerock/javascript-sdk@595
pnpm add https://pkg.pr.new/ForgeRock/forgerock-javascript-sdk/@forgerock/ping-protect@595
pnpm add https://pkg.pr.new/ForgeRock/forgerock-javascript-sdk/@forgerock/token-vault@595

commit: 11ac633

@vatsalparikh vatsalparikh changed the title fix(token-vault): end SSO session before clearing vault tokens (SDKS-5448) fix(token-vault): reliably end the AM SSO session on logout (SDKS-5448) Oct 9, 2026
@codecov-commenter

codecov-commenter commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 13.33333% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.24%. Comparing base (87775ec) to head (11ac633).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
packages/token-vault/src/lib/proxy.ts 0.00% 26 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop     #595       +/-   ##
============================================
- Coverage    56.72%   27.24%   -29.48%     
============================================
  Files          105       11       -94     
  Lines        28457     1567    -26890     
  Branches      1644       49     -1595     
============================================
- Hits         16143      427    -15716     
+ Misses       12314     1140    -11174     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vatsalparikh
vatsalparikh force-pushed the SDKS-5448-composed-urls branch from 3a20558 to 718aef5 Compare October 9, 2026 23:48

@ryanbas21 ryanbas21 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.

Approving solution 2, i'm assuming we will drop commit 1?

@SteinGabriel SteinGabriel 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.

Also in favour of solution 2.

@ancheetah
ancheetah merged commit 0feeda6 into develop Oct 14, 2026
7 checks passed
@ancheetah
ancheetah deleted the SDKS-5448-composed-urls branch October 14, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

6 participants