Add OIDC RP-Initiated Logout support to signOut()#36
Conversation
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (4)
📝 WalkthroughWalkthroughOIDC RP-initiated logout is configurable, logout URL parameter selection is updated, and browser sign-out now clears sessions before navigating to the provider logout endpoint when available. ChangesRP-initiated logout
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BrowserClient
participant JavaScriptClient
participant AuthHelper
participant OP
BrowserClient->>JavaScriptClient: Build sign-out URL
BrowserClient->>AuthHelper: Clear session asynchronously
BrowserClient->>OP: Navigate to end-session URL
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
🤖 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 392-416: Update the RP-initiated logout success branch in the
sign-out method so the optional afterSignOut callback is invoked after
clearSessionAsync and before navigating or returning. Preserve the existing
callback behavior in the local-only fallback and ensure the callback receives
the same session/logout context expected by callers.
🪄 Autofix (Beta)
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: 261b3f58-261e-4f5d-bd31-714c22098cc9
📒 Files selected for processing (4)
packages/browser/src/ThunderIDBrowserClient.tspackages/javascript/src/ThunderIDJavaScriptClient.tspackages/javascript/src/__tests__/ThunderIDJavaScriptClient.test.tspackages/javascript/src/models/config.ts
signOut() performs OIDC RP-Initiated Logout by default: it resolves the sign-out URL (post_logout_redirect_uri, plus id_token_hint when an ID token is available, else client_id) before clearing the local session, then redirects to the OP's end_session_endpoint. It falls back to a local-only sign out when no end_session_endpoint is advertised or the URL cannot be built. Set rpInitiatedLogout: false to force a local-only sign out, or sendIdTokenInLogoutRequest: false to always send client_id. Add clearSessionAsync so the local clear can complete before the redirect.
e2d884b to
bd78aff
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Purpose
signOut()in the browser client previously only cleared the local session (a// TEMPORARYlocal sign out), leaving the OP's session intact. This PR implements OIDC RP-Initiated Logout sosignOut()also terminates the session at the provider'send_session_endpoint, then returns the user to the app.It is enabled by default and degrades gracefully: when the OP advertises no
end_session_endpoint(or a sign-out URL cannot be built), it falls back to the existing local-only sign out, so consumers without an end-session endpoint are unaffected.🔧 Summary of Breaking Changes
signOut()now performs RP-Initiated Logout by default (redirecting to theend_session_endpoint) instead of only clearing the local session.id_token_hintis now sent by default when an ID token is available (sendIdTokenInLogoutRequestdefault is effectivelytrue), falling back toclient_idotherwise.💥 Impact
Consumers whose OP advertises an
end_session_endpointwill now seesignOut()redirect the browser to it (previously local-only). Consumers with noend_session_endpointsee no change.🔄 Migration Guide
rpInitiatedLogout: false.client_idinstead ofid_token_hint: setsendIdTokenInLogoutRequest: false.Approach
ThunderIDBrowserClient.signOut()resolves the sign-out URL (a cached URL built at token exchange, or freshly built for a targeted session) before clearing the local session, so the ID token used forid_token_hintis still available. It awaits the clear, then redirects to theend_session_endpoint. Gated byconfig.rpInitiatedLogout !== false(on by default) with a local-only fallback when no URL can be built.ThunderIDJavaScriptClient.clearSessionAsync()(new,protected) is an awaitable variant of the fire-and-forgetclearSession(), so the local clear completes before the full-page redirect.clearSession()is unchanged.getSignOutUrl()now setsid_token_hintwhen an ID token is available andsendIdTokenInLogoutRequestis notfalse, otherwiseclient_id(no longer throws when the ID token is absent).config.ts:rpInitiatedLogout(new) andsendIdTokenInLogoutRequest(existing) documented with@default true.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
id_token_hintandclient_idhandling.Bug Fixes