Revoke the access token before completing sign out - #70
Conversation
Adds tokenLifecycle.revokeToken.revokeOnSignOut config (default true) so signOut() revokes the access token at the OP's revocation_endpoint before clearing the local session. Revocation is best-effort: failures don't block sign out since the local session is cleared regardless.
|
Warning Review limit reached
Next review available in: 21 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe configuration model adds optional sign-out token revocation. The browser client revokes the access token before logout by default, logs revocation failures, and continues sign-out. ChangesSign-out token revocation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/javascript/src/models/config.ts (1)
432-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect the
revokeOnSignOutdocumentation.When
revokeOnSignOutisfalse,signOut()still performs RP-Initiated Logout by default. State thatrpInitiatedLogout: falseis also required for local-only sign-out.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/javascript/src/models/config.ts` around lines 432 - 449, Update the revokeToken.revokeOnSignOut documentation to clarify that setting it to false only skips token revocation; local-only sign-out also requires configuring rpInitiatedLogout: false.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/browser/src/ThunderIDBrowserClient.ts`:
- Around line 377-382: Update signOut() around revokeAccessToken() to bound the
revocation wait with a timeout, aborting or rejecting the request after a finite
duration so stalled endpoints cannot block local cleanup. Preserve the existing
debug logging and ensure sign-out continues after timeout or other revocation
failures.
- Around line 372-384: Separate token revocation from session cleanup by adding
a core-client method that only sends the revocation request and does not clear
the session. Update signOut() to call this request-only method before resolving
the RP-Initiated Logout URL, while preserving the existing cleanup after URL
resolution; do not use the public revokeAccessToken() override in this flow.
- Around line 372-384: The signOut flow must build the logout URL before
revokeAccessToken clears the session, then use that precomputed URL during
sign-out. Also bound the revocation request inside revokeAccessToken with the
existing timeout/AbortSignal pattern so a stalled fetch cannot block signOut
indefinitely.
In `@packages/javascript/src/models/config.ts`:
- Around line 433-449: Update the JSDoc for revokeToken.revokeOnSignOut in the
configuration definition to state that disabling revocation skips token
revocation while sign-out continues according to rpInitiatedLogout, rather than
claiming it only clears the local session.
---
Nitpick comments:
In `@packages/javascript/src/models/config.ts`:
- Around line 432-449: Update the revokeToken.revokeOnSignOut documentation to
clarify that setting it to false only skips token revocation; local-only
sign-out also requires configuring rpInitiatedLogout: false.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 06c16f9b-1a71-4a56-b880-36e9ea01a545
📒 Files selected for processing (2)
packages/browser/src/ThunderIDBrowserClient.tspackages/javascript/src/models/config.ts
…ix doc Extracts a revocation-request-only core method so signOut() can resolve the RP-Initiated Logout URL (needs the ID token) before revoking the access token, instead of the public revokeAccessToken() override clearing the session prematurely. Bounds the revocation fetch with a timeout so a stalled revocation_endpoint can't block signOut() indefinitely, and corrects the revokeOnSignOut JSDoc to describe the actual fallback behavior.
3 participants
Purpose
signOut()only cleared the local session and, when RP-Initiated Logout is enabled, redirected to the OP'send_session_endpoint. It never revoked the access token itself, so a client-side sign out could leave the token valid at the OP.Approach
Adds a
tokenLifecycle.revokeToken.revokeOnSignOutconfig (defaulttrue) and hassignOut()revoke the access token at the OP'srevocation_endpointfirst, before the existing RP-Initiated Logout / local-only sign-out logic runs.Revocation is best-effort: if it fails (no
revocation_endpointadvertised, network error, non-200 response — e.g. for non-OAuth sessions), the error is caught and logged at debug level, and sign out proceeds exactly as it did before, since the local session must be cleared regardless.Set
tokenLifecycle: { revokeToken: { revokeOnSignOut: false } }to opt out and skip revocation entirely.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Documentation