Make id_token_hint optional for RP-initiated logout - #4299
Conversation
|
Warning Review limit reached
Next review available in: 1 minute 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 (16)
📝 WalkthroughWalkthroughAdds RP-initiated logout confirmation when ChangesLogout confirmation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RP
participant LogoutService
participant SessionSignOutExecutor
participant ConfirmationPrompt
participant SSO
RP->>LogoutService: Submit logout request without id_token_hint
LogoutService->>SessionSignOutExecutor: Start flow with prompt required
SessionSignOutExecutor->>ConfirmationPrompt: Request user confirmation
ConfirmationPrompt->>SessionSignOutExecutor: Confirm sign-out
SessionSignOutExecutor->>SSO: Terminate session
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1de09e8 to
633c296
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/oauth/oauth2/logout/service.go (1)
187-236: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates underdocs/.
Please update the relevant documentation before merging.Missing documentation:
backend/internal/oauth/oauth2/logout/service.go#L187-L236: Document optionalid_token_hint,client_id-based logout resolution, confirmation behavior, and redirect validation indocs/content/apis.mdx.frontend/apps/console/src/features/login-flow/data/executors.json#L948-L950: DocumentpromptOnSignOutand the conditional sign-out flow/template in an appropriatedocs/content/guides/page.🤖 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 `@backend/internal/oauth/oauth2/logout/service.go` around lines 187 - 236, Document the new sign-out behavior in docs/content/apis.mdx for logoutService.Resolve: explain that id_token_hint is optional, client_id can resolve the client, missing hints require user confirmation, and post_logout_redirect_uri must match the client’s registered redirects. Also document promptOnSignOut and its conditional sign-out flow/template in an appropriate docs/content/guides/ page, covering the executor configuration at frontend/apps/console/src/features/login-flow/data/executors.json lines 948-950.Source: Path instructions
🤖 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.
Outside diff comments:
In `@backend/internal/oauth/oauth2/logout/service.go`:
- Around line 187-236: Document the new sign-out behavior in
docs/content/apis.mdx for logoutService.Resolve: explain that id_token_hint is
optional, client_id can resolve the client, missing hints require user
confirmation, and post_logout_redirect_uri must match the client’s registered
redirects. Also document promptOnSignOut and its conditional sign-out
flow/template in an appropriate docs/content/guides/ page, covering the executor
configuration at
frontend/apps/console/src/features/login-flow/data/executors.json lines 948-950.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a8438114-bf6d-4084-85f9-5adcfa0b930f
📒 Files selected for processing (15)
backend/internal/flow/common/constants.gobackend/internal/flow/executor/constants.gobackend/internal/flow/executor/session_signout_executor.gobackend/internal/flow/executor/session_signout_executor_test.gobackend/internal/oauth/oauth2/logout/service.gobackend/internal/oauth/oauth2/logout/service_test.gofrontend/apps/console/src/features/flows/data/templates.jsonfrontend/apps/console/src/features/flows/models/steps.tsfrontend/apps/console/src/features/flows/utils/__tests__/resolveStepMetadata.test.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/ExecutionExtendedProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/ExecutionExtendedProperties.test.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/SessionSignOutProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/constants.tsfrontend/apps/console/src/features/login-flow/data/executors.jsonfrontend/packages/i18n/src/locales/en-US.ts
| // RuntimeKeyLogoutPromptShown is the session sign-out node's own guard, set when it routes to the | ||
| // confirmation prompt so that on re-run (after the user confirms) it terminates instead of | ||
| // prompting again. | ||
| RuntimeKeyLogoutPromptShown = "logoutPromptShown" |
There was a problem hiding this comment.
We have a type field in the prompt node action that can be used to indicate a metadata about the action edge. We should be able to improve this logic and get rid of this runtime data key by utilizing that.
Let's do it as a followup
633c296 to
4c26526
Compare
Drop the requirement that id_token_hint accompany post_logout_redirect_uri. A logout carrying only client_id (with or without a registered post_logout_redirect_uri) now resolves; the redirect target is still confirmed against the client's registered allow-list, satisfying the OIDC RP-Initiated Logout requirement for confirming the redirection target. When no valid id_token_hint is supplied, the OAuth layer marks the sign-out flow with logoutPromptRequired. SessionSignOutExecutor reads it and, when its node opts in via the promptOnSignOut property, routes to the node's onIncomplete confirmation prompt before terminating the session, prompting once per flow run. Nodes that do not opt in terminate as before, leaving the always-prompt default sign-out flow unchanged. Add a Confirm Sign Out Without a Hint flow-builder template that wires this behavior with builder-native constructs (an onIncomplete edge and the promptOnSignOut property) so it renders and round-trips in the flow editor. Expose promptOnSignOut as an editable checkbox on the End Session executor in the Console flow editor, mirroring how other executors surface node properties. The property is a boolean, consistent with the provisioning executor's options.
4c26526 to
35dfe14
Compare
Replace the RuntimeKeyLogoutPromptShown runtime-data guard on the session sign-out executor with the confirmation prompt's action type. When the End-User confirms, the prompt forwards its SIGN_OUT_CONFIRM action type to the executor on the re-run, so a confirmed request is told apart from the initial one without persisting a marker. Mark the confirmation button of the Conditional Sign Out Flow template with that type, both on the prompt action the engine reads and on the element itself, so the flow builder's Action selector shows Sign out for it and the type survives editing the flow. Refs thunder-id#4299
Replace the RuntimeKeyLogoutPromptShown runtime-data guard on the session sign-out executor with the confirmation prompt's action type. When the End-User confirms, the prompt forwards its SIGN_OUT_CONFIRM action type to the executor on the re-run, so a confirmed request is told apart from the initial one without persisting a marker. Mark the confirmation button of the Conditional Sign Out Flow template with that type, both on the prompt action the engine reads and on the element itself, so the flow builder's Action selector shows Sign out for it and the type survives editing the flow. Document SIGN_OUT_CONFIRM alongside the existing SUBMIT and REJECT action types in the flow management and flow execution API specs and in the flow configuration guide. Refs thunder-id#4299
Replace the RuntimeKeyLogoutPromptShown runtime-data guard on the session sign-out executor with the confirmation prompt's action type. When the End-User confirms, the prompt forwards its SIGN_OUT_CONFIRM action type to the executor on the re-run, so a confirmed request is told apart from the initial one without persisting a marker. Mark the confirmation button of the Conditional Sign Out Flow template with that type, both on the prompt action the engine reads and on the element itself, so the flow builder's Action selector shows Sign out for it and the type survives editing the flow. Describe prompts[].action.type as an opaque string owned by the consuming executor instead of enumerating its values in the flow management and flow execution API specs and in the flow configuration guide. The field already accepted arbitrary strings, so the enumerations were never the contract, and they would need editing in three places for every action type added. Refs thunder-id#4299
Replace the RuntimeKeyLogoutPromptShown runtime-data guard on the session sign-out executor with the confirmation prompt's action type. When the End-User confirms, the prompt forwards its SIGN_OUT_CONFIRM action type to the executor on the re-run, so a confirmed request is told apart from the initial one without persisting a marker. Resolve that into an outcome by dispatching on the forwarded action type rather than comparing against the confirm type alone, so the confirmation prompt can grow further actions: each action type the prompt raises maps to one signOutOutcome, and an unrecognized type is treated as no consent and prompts again rather than falling through to termination. Mark the confirmation button of the Conditional Sign Out Flow template with that type, both on the prompt action the engine reads and on the element itself, so the flow builder's Action selector shows Sign out for it and the type survives editing the flow. Describe prompts[].action.type as an opaque string owned by the consuming executor instead of enumerating its values in the flow management and flow execution API specs and in the flow configuration guide. The field already accepted arbitrary strings, so the enumerations were never the contract, and they would need editing in three places for every action type added. Refs thunder-id#4299
The prompt action's type was write-only in the console: serialization emitted prompts[].action.type from the button element's actionType, but the load path never read it back and FlowNodeAction did not even declare the field. A flow definition authored outside the builder therefore lost its action type on the first save, and the Action selector showed the wrong option for it. Restore the type onto the element when a definition is loaded, so the property panel can display it and serialization emits it again. The element wins when it already carries a type, because an unwired button holds the only copy: a prompt action is emitted only once the button has a nextNode. Clear only the type the selector owns when the author picks a plain action. SUBMIT and REJECT are documented prompt action types with no option in the dropdown, and blanket clearing discarded them. Key the Sign out option on SIGN_OUT_CONFIRM rather than a UI-only SIGN_OUT sentinel, so the value selected in the editor and the value in the flow definition are one vocabulary, and label it Sign Out Action to match the Submit Form and Trigger Action options beside it. Refs thunder-id#4299
The prompt action's type was write-only in the console: serialization emitted prompts[].action.type from the button element's actionType, but the load path never read it back and FlowNodeAction did not even declare the field. A flow definition authored outside the builder therefore lost its action type on the first save, and the Action selector showed the wrong option for it. Restore the type onto the element when a definition is loaded, so the property panel can display it and serialization emits it again. The element wins when it already carries a type, because an unwired button holds the only copy: a prompt action is emitted only once the button has a nextNode. Clear only the type the selector owns when the author picks a plain action. SUBMIT and REJECT are documented prompt action types with no option in the dropdown, and blanket clearing discarded them. Key the Sign out option on SIGN_OUT_CONFIRM rather than a UI-only SIGN_OUT sentinel, so the value selected in the editor and the value in the flow definition are one vocabulary, and label it Sign Out Action to match the Submit Form and Trigger Action options beside it. Refs thunder-id#4299
Rename the prompt action type the session sign-out executor reads from SIGN_OUT_CONFIRM to CONFIRM, in the backend constant and in the console's PromptActionTypes, and update the Conditional Sign Out Flow template. Forwarding a prompt action's type to the next node's executor is a generic mechanism, so naming the value after one use case would push every executor that routes to a confirmation prompt to add a near-identical constant of its own. CONFIRM is the vocabulary rather than SIGN_OUT_CONFIRM; the session sign-out executor is its first consumer, not its only possible one. Rename the Action selector's option to match, from Sign Out Action under core.buttonExtendedProperties.action.signOut to Confirm Action under core.buttonExtendedProperties.action.confirm. Leaving it named for signing out while it writes the generic type would offer no option an author could pick to confirm anything else. The sign-out validation rule keeps its own notification identifiers and its sign-out wording: it checks that a confirmation button leads to a session sign-out step, which stays specific to sign-out flows even though the action type it looks for no longer is. Refs thunder-id#4299
Purpose
Makes id_token_hint optional for OIDC RP-Initiated Logout. Previously the logout endpoint rejected any request that supplied post_logout_redirect_uri without an id_token_hint.
Per the RP-Initiated Logout spec, the OP may perform post-logout redirection without an id_token_hint as long as it confirms the legitimacy of the redirection target by other means, and per RPLogout it MUST ask the End-User to confirm the logout when no id_token_hint was provided. This PR implements both: a logout carrying only client_id now resolves (the redirect target is still validated against the client's registered allow-list), and when no valid hint is present the sign-out flow can prompt the user to confirm before the session is terminated.
A new "Confirm Sign Out Without a Hint" flow-builder template wires up this conditional-confirmation behavior.
Approach
Prompt logout consent always

Prompt logout consent when id_token_hint is not provided

Available flow templates

Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit