Implement session sign-out - #3973
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds configurable sign-out flows, RP-initiated OAuth2 logout, session termination and cookie clearing, post-logout redirect URI support, application persistence and APIs, and console and gate UI flows. ChangesSign-out flow and application persistence
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
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 |
65c9504 to
b58bad1
Compare
63b1d6a to
1fa257a
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
7fa07a7 to
dd5ddc5
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx (1)
201-217: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
commitUriscan persist a malformed URI from a sibling row.
commitUrisonly filters out empty strings (uri.trim() !== ''), not values that failisValidUriFormat.handleUriBlur/handlePostLogoutUriBlurgate the commit on the blurred row being valid, butcommitUriswrites the entireredirectUris/postLogoutRedirectUrisarrays each time. So if row A has a non-empty, malformed URI (error shown), blurring a different, valid row B still commits row A's bad value intoapplication.inboundAuthConfig.🐛 Proposed fix: filter invalid (not just empty) entries before committing
const commitUris = (nextRedirect: string[], nextPostLogout: string[]) => { if (!oauth2Config) return; const updatedConfig = { ...oauth2Config, - redirectUris: nextRedirect.filter((uri) => uri.trim() !== ''), - postLogoutRedirectUris: nextPostLogout.filter((uri) => uri.trim() !== ''), + redirectUris: nextRedirect.filter((uri) => uri.trim() !== '' && isValidUriFormat(uri)), + postLogoutRedirectUris: nextPostLogout.filter((uri) => uri.trim() !== '' && isValidUriFormat(uri)), };Also applies to: 248-253, 284-289
🤖 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 `@frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx` around lines 201 - 217, Update commitUris to filter both redirectUris and postLogoutRedirectUris through isValidUriFormat, excluding malformed and empty values before constructing updatedConfig. Ensure the same validation is applied when handleUriBlur or handlePostLogoutUriBlur commits sibling-row arrays, while preserving valid URI entries.
🧹 Nitpick comments (4)
frontend/apps/gate/src/components/SignOut/SignOutBox.tsx (2)
107-114: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSilent blank screen when
COMPLETEarrives without a landing URI.If the server ever returns
flowStatus === 'COMPLETE'withoutadditionalData.postLogoutRedirectUri, the function just returns —isLoadingbecomes false, no components render, and no error is shown, leaving a blank card.🤖 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 `@frontend/apps/gate/src/components/SignOut/SignOutBox.tsx` around lines 107 - 114, Update the COMPLETE branch in the sign-out handler to handle a missing postLogoutRedirectUri explicitly instead of returning silently. Preserve the redirect through landingWithState when the URI exists; otherwise set the appropriate error state or fallback UI so the SignOutBox does not render a blank card.
92-126: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout/abort on the
/flow/executefetch.If the server hangs, the user is stuck on the loading spinner indefinitely with no recovery path.
♻️ Proposed fix: add an abort timeout
- const response = await fetch(`${baseUrl}/flow/execute`, { - method: 'POST', - headers: {'Content-Type': 'application/json', Accept: 'application/json'}, - credentials: 'include', - body: JSON.stringify({...payload, verbose: true}), - }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 15000); + const response = await fetch(`${baseUrl}/flow/execute`, { + method: 'POST', + headers: {'Content-Type': 'application/json', Accept: 'application/json'}, + credentials: 'include', + body: JSON.stringify({...payload, verbose: true}), + signal: controller.signal, + }); + clearTimeout(timeoutId);🤖 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 `@frontend/apps/gate/src/components/SignOut/SignOutBox.tsx` around lines 92 - 126, Update the run function’s /flow/execute fetch to use an AbortController with a timeout, aborting requests that exceed the configured limit so the existing catch path clears loading and displays the failure message. Ensure the timeout is cleaned up when the request completes, including error and abort cases.frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx (2)
81-97: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winValidation state isn't surfaced to the parent for Save gating.
AccessSectionnow maintains two error maps (uriErrors,postLogoutUriErrors) but doesn't accept/call anonValidationChangecallback, soApplicationEditPage'shasValidationErrors/Save-button gating won't reflect these fields' invalid state.Based on learnings, "ensure client-side validation participates in disabling the page Save button... accept an optional
onValidationChange?: (hasErrors: boolean) => voidprop... and call it whenever its validation error state changes."🤖 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 `@frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx` around lines 81 - 97, Update AccessSection to accept an optional onValidationChange callback and notify the parent whenever uriErrors or postLogoutUriErrors changes, passing whether either error map contains validation errors. Ensure ApplicationEditPage can use this state to disable Save while either redirect URI field remains invalid.Source: Learnings
395-405: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueBoth "Add URI" buttons share the same accessible name.
Redirect and post-logout sections both render a button whose accessible text resolves to "Add URI", which could be ambiguous for screen-reader users navigating by role/name.
Also applies to: 453-463
🤖 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 `@frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx` around lines 395 - 405, Update the button labels used by handleAddUri in the redirect-URI and post-logout sections so each resolves to a distinct, section-specific accessible name, such as identifying whether it adds a redirect URI or a post-logout URI. Preserve the existing button behavior and translation structure while adding or reusing appropriate localized keys.
🤖 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 `@backend/internal/flow/flowexec/service_test.go`:
- Around line 2240-2254: Update docs/content/apis.mdx to document the
OAuth2/OIDC /oauth2/logout endpoint, end_session_endpoint discovery, and native
logout through /flow/execute; also update the relevant sign-out flow and
post-logout redirect URI configuration documentation under docs/content/guides/
or related config pages.
In `@backend/internal/flow/flowexec/service.go`:
- Around line 214-229: Update the relevant documentation to cover the new
RP-initiated logout endpoint and its id_token_hint, post_logout_redirect_uri,
and state parameters, the SIGNOUT flow type and direct initiation requirements
for /flow/execute, the OIDC discovery end_session_endpoint, and console
configuration for sign-out flows and post-logout redirect URIs. Use the existing
API reference and guide documentation locations, including docs/content/apis.mdx
and docs/content/guides/.
In `@backend/internal/inboundclient/store_constants.go`:
- Around line 124-138: Update the relevant documentation under docs/ to cover
all sign-out changes: add GET/POST /oauth2/logout parameters and
post_logout_redirect_uri details to apis.mdx with a guide walkthrough; document
the SIGNOUT flow type, SessionSignOutExecutor, and /flow/execute resume pattern
in the guides; document postLogoutRedirectUris and the console Sign Out settings
in apis.mdx and the applications configuration guide; and document OIDC
discovery’s end_session_endpoint for SDK consumers.
In `@backend/internal/oauth/oauth2/dcr/model.go`:
- Line 39: Update the documentation for all four affected sites: in
backend/internal/oauth/oauth2/dcr/model.go:39 document post_logout_redirect_uris
in the OAuth 2.0 DCR request and response schemas; in
backend/internal/application/model/application.go:180-181 document signOutFlowId
and isSignOutFlowEnabled in the Application REST API schema; in
backend/internal/application/declarative_resource.go:186-188 document the
declarative YAML sign-out flow options; and in
frontend/apps/console/src/features/login-flow/data/executors.json:895-913
document the Session Sign Out Executor and its sign-out UI behavior, using the
relevant docs/content/apis.mdx and docs/content/guides/ sections.
In `@backend/internal/oauth/oauth2/logout/handler_test.go`:
- Around line 86-87: Update the relevant documentation under docs/ to cover the
GET/POST /oauth2/logout endpoint and its parameters, including id_token_hint and
post_logout_redirect_uri; add a guides section describing sign-out flow
configuration in the console; and update OIDC discovery documentation to list
the available end_session_endpoint.
In `@backend/internal/oauth/oauth2/logout/handler.go`:
- Around line 59-60: Update the documentation to cover the new RP-initiated
logout flow implemented by logoutHandler.HandleLogout: document GET/POST
/oauth2/logout and its id_token_hint, client_id, post_logout_redirect_uri, and
state parameters in docs/content/apis.mdx; document the console sign-out
settings and post-logout redirect URI configuration under docs/content/guides/;
and add end_session_endpoint to the OIDC discovery API reference.
In `@backend/internal/oauth/oauth2/logout/service.go`:
- Around line 136-150: Update audienceClientID to inspect the azp claim when aud
contains multiple audience strings, returning the string azp value as the exact
client ID; only fall back to the existing audience handling when azp is absent
or unusable, while preserving single-audience behavior.
- Around line 81-117: Update logoutService.Resolve to require a non-empty
req.IDTokenHint whenever req.PostLogoutRedirectURI is provided; reject the
request before client resolution when that requirement is not met, and preserve
the existing client-ID and token-hint validation for valid requests.
In `@backend/internal/system/importer/service.go`:
- Around line 861-863: Update the DTO conversion mapping for SignOutFlowID to
resolve it through flowIDAliases, matching the existing AuthFlowID and
RegistrationFlowID alias handling. Keep SignOutFlowHandle and
IsSignOutFlowEnabled unchanged.
In `@frontend/apps/gate/src/components/SignOut/__tests__/SignOutBox.test.tsx`:
- Around line 204-213: Rename the fallback environment variable from
VITE_THUNDER_BASE_URL to the approved ThunderID name (or the
file-type-appropriate template placeholder) consistently in the SignOutBox test
title, fetch URL assertion, and every corresponding definition or usage.
---
Outside diff comments:
In
`@frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx`:
- Around line 201-217: Update commitUris to filter both redirectUris and
postLogoutRedirectUris through isValidUriFormat, excluding malformed and empty
values before constructing updatedConfig. Ensure the same validation is applied
when handleUriBlur or handlePostLogoutUriBlur commits sibling-row arrays, while
preserving valid URI entries.
---
Nitpick comments:
In
`@frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx`:
- Around line 81-97: Update AccessSection to accept an optional
onValidationChange callback and notify the parent whenever uriErrors or
postLogoutUriErrors changes, passing whether either error map contains
validation errors. Ensure ApplicationEditPage can use this state to disable Save
while either redirect URI field remains invalid.
- Around line 395-405: Update the button labels used by handleAddUri in the
redirect-URI and post-logout sections so each resolves to a distinct,
section-specific accessible name, such as identifying whether it adds a redirect
URI or a post-logout URI. Preserve the existing button behavior and translation
structure while adding or reusing appropriate localized keys.
In `@frontend/apps/gate/src/components/SignOut/SignOutBox.tsx`:
- Around line 107-114: Update the COMPLETE branch in the sign-out handler to
handle a missing postLogoutRedirectUri explicitly instead of returning silently.
Preserve the redirect through landingWithState when the URI exists; otherwise
set the appropriate error state or fallback UI so the SignOutBox does not render
a blank card.
- Around line 92-126: Update the run function’s /flow/execute fetch to use an
AbortController with a timeout, aborting requests that exceed the configured
limit so the existing catch path clears loading and displays the failure
message. Ensure the timeout is cleaned up when the request completes, including
error and abort cases.
🪄 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: 38a04507-4cd0-4b93-84a3-d1b4371fc60d
⛔ Files ignored due to path filters (1)
backend/tests/mocks/flow/sessionmock/Service_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (73)
backend/dbscripts/configdb/postgres.sqlbackend/dbscripts/configdb/sqlite.sqlbackend/internal/actorprovider/service.gobackend/internal/actorprovider/utils.gobackend/internal/actorprovider/utils_test.gobackend/internal/agent/service.gobackend/internal/application/declarative_resource.gobackend/internal/application/handler.gobackend/internal/application/model/application.gobackend/internal/application/service.gobackend/internal/flow/common/constants.gobackend/internal/flow/executor/constants.gobackend/internal/flow/executor/register.gobackend/internal/flow/executor/session_signout_executor.gobackend/internal/flow/executor/session_signout_executor_test.gobackend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/flow/flowexec/handler.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/session/Service_mock_test.gobackend/internal/flow/session/service.gobackend/internal/flow/session/service_test.gobackend/internal/inboundclient/error_constants.gobackend/internal/inboundclient/model/oauth.gobackend/internal/inboundclient/service.gobackend/internal/inboundclient/store.gobackend/internal/inboundclient/store_constants.gobackend/internal/inboundclient/store_test.gobackend/internal/oauth/init.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/oauth/oauth2/dcr/model.gobackend/internal/oauth/oauth2/dcr/service.gobackend/internal/oauth/oauth2/discovery/service.gobackend/internal/oauth/oauth2/logout/handler.gobackend/internal/oauth/oauth2/logout/handler_test.gobackend/internal/oauth/oauth2/logout/init.gobackend/internal/oauth/oauth2/logout/service.gobackend/internal/oauth/oauth2/logout/service_test.gobackend/internal/system/config/config.gobackend/internal/system/i18n/core/defaults.gobackend/internal/system/importer/service.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/providers/constants.gobackend/pkg/thunderidengine/providers/model.gobackend/pkg/thunderidengine/providers/oauth_client.gofrontend/apps/console/src/App.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/EditFlowsSettings.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/SignOutFlowSection.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/__tests__/EditFlowsSettings.test.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/__tests__/SignOutFlowSection.test.tsxfrontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsxfrontend/apps/console/src/features/applications/components/edit-application/general-settings/__tests__/AccessSection.test.tsxfrontend/apps/console/src/features/applications/models/application.tsfrontend/apps/console/src/features/applications/models/oauth.tsfrontend/apps/console/src/features/flows/components/create-flow/SelectFlowType.tsxfrontend/apps/console/src/features/flows/components/create-flow/__tests__/SelectFlowType.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsxfrontend/apps/console/src/features/flows/data/templates.jsonfrontend/apps/console/src/features/flows/models/flows.tsfrontend/apps/console/src/features/login-flow/data/executors.jsonfrontend/apps/gate/src/App.tsxfrontend/apps/gate/src/components/SignOut/SignOut.tsxfrontend/apps/gate/src/components/SignOut/SignOutBox.tsxfrontend/apps/gate/src/components/SignOut/__tests__/SignOut.test.tsxfrontend/apps/gate/src/components/SignOut/__tests__/SignOutBox.test.tsxfrontend/apps/gate/src/constants/__tests__/routes.test.tsfrontend/apps/gate/src/constants/routes.tsfrontend/apps/gate/src/pages/SignOutPage.tsxfrontend/apps/gate/src/pages/__tests__/SignOutPage.test.tsxfrontend/packages/i18n/src/locales/en-US.tstests/integration/oauth/discovery/discovery_test.go
| flowType providers.FlowType | ||
| grantTypes []string | ||
| flowSecret string | ||
| wantCode string | ||
| }{ | ||
| {"redirect-based app blocked", []string{"authorization_code"}, "", | ||
| {"redirect-based app blocked", providers.FlowTypeAuthentication, []string{"authorization_code"}, "", | ||
| ErrorDirectFlowInitiationNotPermitted.Code}, | ||
| {"m2m app blocked", []string{"client_credentials"}, "", | ||
| {"m2m app blocked", providers.FlowTypeAuthentication, []string{"client_credentials"}, "", | ||
| ErrorDirectFlowInitiationNotPermitted.Code}, | ||
| {"flow-native app without secret", | ||
| {"flow-native app without secret", providers.FlowTypeAuthentication, | ||
| []string{"client_credentials", "urn:ietf:params:oauth:grant-type:token-exchange"}, "", | ||
| ErrorFlowSecretRequired.Code}, | ||
| {"sign-out redirect-based app blocked", providers.FlowTypeSignOut, []string{"authorization_code"}, "", | ||
| ErrorDirectFlowInitiationNotPermitted.Code}, | ||
| {"sign-out flow-native app without secret", providers.FlowTypeSignOut, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | 🏗️ Heavy lift
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
- OAuth2/OIDC
/oauth2/logoutendpoint andend_session_endpointdiscovery: Updatedocs/content/apis.mdx. - Sign-out flows and post-logout redirect URIs configuration: Update
docs/content/guides/or related config pages. - Native logout through
/flow/execute: Updatedocs/content/apis.mdx.
🤖 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/flow/flowexec/service_test.go` around lines 2240 - 2254,
Update docs/content/apis.mdx to document the OAuth2/OIDC /oauth2/logout
endpoint, end_session_endpoint discovery, and native logout through
/flow/execute; also update the relevant sign-out flow and post-logout redirect
URI configuration documentation under docs/content/guides/ or related config
pages.
Source: Path instructions
| // checkDirectFlowInitiationAllowed governs which applications may initiate an authentication or | ||
| // sign-out flow directly over HTTP, based on how the application is classified: | ||
| // - RedirectOnly — the application signs users in through a redirect-based protocol component | ||
| // (currently OAuth 2.0 authorization_code apps) and must have its flows initiated by that | ||
| // component, not via a direct HTTP call. | ||
| // - FlowSecret — a backend / server-side application (including embedded apps with no protocol | ||
| // profile) that must authenticate at flow initiation by presenting its Flow Secret. | ||
| // | ||
| // Other flow types (registration, recovery, user onboarding) are not restricted. The classification | ||
| // is derived from neutral actor data resolved through the actor layer; Flow Secret verification is | ||
| // the only credential check that remains here. | ||
| // Sign-out is guarded like authentication so a native caller must prove its identity before ending a | ||
| // session; a redirect-based app is pushed to the RP-initiated /oauth2/logout endpoint instead. Other | ||
| // flow types (registration, recovery, user onboarding) are not restricted. The classification is | ||
| // derived from neutral actor data resolved through the actor layer; Flow Secret verification is the | ||
| // only credential check that remains here. | ||
| func (s *flowExecService) checkDirectFlowInitiationAllowed(ctx context.Context, appID string, | ||
| flowType providers.FlowType, flowSecret string, logger *log.Logger) *tidcommon.ServiceError { | ||
| if flowType != providers.FlowTypeAuthentication { | ||
| if flowType != providers.FlowTypeAuthentication && flowType != providers.FlowTypeSignOut { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
- RP-initiated logout (
/oauth2/logout): Document the new endpoint, parameters (id_token_hint,post_logout_redirect_uri,state), and behavior indocs/content/apis.mdxor relevant guide. - Native sign-out flow execution (
/flow/execute): Document theSIGNOUTflow type and direct initiation requirements in the flow execution API reference. - OIDC discovery updates: Mention the inclusion of
end_session_endpointin the well-known configuration. - Console sign-out configuration: Add a guide in
docs/content/guides/for setting up sign-out flows and post-logout redirect URIs in the console.
As per path instructions, changes introducing new public endpoints, configuration options, and auth flows require documentation updates.
🤖 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/flow/flowexec/service.go` around lines 214 - 229, Update the
relevant documentation to cover the new RP-initiated logout endpoint and its
id_token_hint, post_logout_redirect_uri, and state parameters, the SIGNOUT flow
type and direct initiation requirements for /flow/execute, the OIDC discovery
end_session_endpoint, and console configuration for sign-out flows and
post-logout redirect URIs. Use the existing API reference and guide
documentation locations, including docs/content/apis.mdx and
docs/content/guides/.
Source: Path instructions
| // flow through any of the authentication, registration, recovery, or sign-out flow slots. | ||
| queryGetEntityIDsByFlowID = dbmodel.DBQuery{ | ||
| ID: "ASQ-INBC_MGT-15", | ||
| Query: `SELECT ENTITY_ID FROM "INBOUND_CLIENT" WHERE ` + | ||
| `(AUTH_FLOW_ID = $1 OR REGISTRATION_FLOW_ID = $2 OR RECOVERY_FLOW_ID = $3) AND DEPLOYMENT_ID = $4 ` + | ||
| `ORDER BY ENTITY_ID ASC LIMIT $5 OFFSET $6`, | ||
| `(AUTH_FLOW_ID = $1 OR REGISTRATION_FLOW_ID = $2 OR RECOVERY_FLOW_ID = $3 OR SIGNOUT_FLOW_ID = $4) ` + | ||
| `AND DEPLOYMENT_ID = $5 ORDER BY ENTITY_ID ASC LIMIT $6 OFFSET $7`, | ||
| } | ||
|
|
||
| // queryGetEntityIDsByFlowIDCount retrieves the total count of inbound clients referencing a specific flow. | ||
| queryGetEntityIDsByFlowIDCount = dbmodel.DBQuery{ | ||
| ID: "ASQ-INBC_MGT-16", | ||
| Query: `SELECT COUNT(*) as total FROM "INBOUND_CLIENT" WHERE ` + | ||
| `(AUTH_FLOW_ID = $1 OR REGISTRATION_FLOW_ID = $2 OR RECOVERY_FLOW_ID = $3) AND DEPLOYMENT_ID = $4`, | ||
| `(AUTH_FLOW_ID = $1 OR REGISTRATION_FLOW_ID = $2 OR RECOVERY_FLOW_ID = $3 OR SIGNOUT_FLOW_ID = $4) ` + | ||
| `AND DEPLOYMENT_ID = $5`, | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
- RP-initiated sign-out endpoint: document the new
GET/POST /oauth2/logoutendpoint (request/response params includingpost_logout_redirect_uri) indocs/content/apis.mdx, plus a walkthrough indocs/content/guides/. - Native sign-out flow: document the new first-class
SIGNOUTflow type,SessionSignOutExecutor, and the/flow/executesign-out resume pattern indocs/content/guides/. postLogoutRedirectUrisapplication config: document the new OAuth2 config field and console "Sign Out" settings panel (flow selection, redirect URI allow-list) indocs/content/apis.mdxand the applications configuration guide.- OIDC discovery
end_session_endpoint: document the new discovery metadata field, relevant to SDK consumers, indocs/content/sdks/ordocs/content/apis.mdx.
As per path instructions: "If ANY of the above are detected and the PR does NOT include corresponding updates under docs/... post a single consolidated PR-level comment."
🤖 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/inboundclient/store_constants.go` around lines 124 - 138,
Update the relevant documentation under docs/ to cover all sign-out changes: add
GET/POST /oauth2/logout parameters and post_logout_redirect_uri details to
apis.mdx with a guide walkthrough; document the SIGNOUT flow type,
SessionSignOutExecutor, and /flow/execute resume pattern in the guides; document
postLogoutRedirectUris and the console Sign Out settings in apis.mdx and the
applications configuration guide; and document OIDC discovery’s
end_session_endpoint for SDK consumers.
Source: Path instructions
| type DCRRegistrationRequest struct { | ||
| OUID string `json:"ou_id,omitempty"` | ||
| RedirectURIs []string `json:"redirect_uris"` | ||
| PostLogoutRedirectURIs []string `json:"post_logout_redirect_uris,omitempty"` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚖️ Poor tradeoff
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
backend/internal/oauth/oauth2/dcr/model.go#L39-L39: Document the newpost_logout_redirect_urisfield in the OAuth 2.0 DCR request and response schemas (e.g., indocs/content/apis.mdx).backend/internal/application/model/application.go#L180-L181: Document the newsignOutFlowIdandisSignOutFlowEnabledfields in the Application REST API schema (e.g., indocs/content/apis.mdx).backend/internal/application/declarative_resource.go#L186-L188: Document the new declarative YAML configuration options for sign-out flows (e.g., indocs/content/guides/).frontend/apps/console/src/features/login-flow/data/executors.json#L895-L913: Document the new user-facing "Session Sign Out Executor" login flow step and sign-out UI behavior (e.g., indocs/content/guides/).
📍 Affects 4 files
backend/internal/oauth/oauth2/dcr/model.go#L39-L39(this comment)backend/internal/application/model/application.go#L180-L181backend/internal/application/declarative_resource.go#L186-L188frontend/apps/console/src/features/login-flow/data/executors.json#L895-L913
🤖 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/dcr/model.go` at line 39, Update the
documentation for all four affected sites: in
backend/internal/oauth/oauth2/dcr/model.go:39 document post_logout_redirect_uris
in the OAuth 2.0 DCR request and response schemas; in
backend/internal/application/model/application.go:180-181 document signOutFlowId
and isSignOutFlowEnabled in the Application REST API schema; in
backend/internal/application/declarative_resource.go:186-188 document the
declarative YAML sign-out flow options; and in
frontend/apps/console/src/features/login-flow/data/executors.json:895-913
document the Session Sign Out Executor and its sign-out UI behavior, using the
relevant docs/content/apis.mdx and docs/content/guides/ sections.
Source: Path instructions
| req := httptest.NewRequest(http.MethodGet, | ||
| "/oauth2/logout?client_id=client-x&post_logout_redirect_uri=https://rp.example/after&state=xyz", nil) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
GET/POST /oauth2/logoutendpoint: Needs documentation indocs/content/apis.mdxor the relevant API reference covering parameters (e.g.,id_token_hint,post_logout_redirect_uri).- Sign-out flows and settings: Needs documentation in
docs/content/guides/detailing how users can configure sign-out flows in the console. end_session_endpointdiscovery: Update the OIDC discovery documentation to reflect the availability of the new endpoint.
(As per path instructions, this is a consolidated PR-level comment regarding missing documentation.)
🤖 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/handler_test.go` around lines 86 - 87,
Update the relevant documentation under docs/ to cover the GET/POST
/oauth2/logout endpoint and its parameters, including id_token_hint and
post_logout_redirect_uri; add a guides section describing sign-out flow
configuration in the console; and update OIDC discovery documentation to list
the available end_session_endpoint.
Source: Path instructions
| func (h *logoutHandler) HandleLogout(w http.ResponseWriter, r *http.Request) { | ||
| if err := r.ParseForm(); err != nil { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚖️ Poor tradeoff
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
GET/POST /oauth2/logout: Document the new RP-initiated logout endpoint and its request parameters (id_token_hint,client_id,post_logout_redirect_uri,state) in the API references underdocs/content/apis.mdx.- Console Configuration for Sign-out Flows: Document the new sign-out settings UI and the configuration of post-logout redirect URIs in the user guides under
docs/content/guides/. - OIDC Discovery Endpoint Updates: Update the OIDC discovery documentation to include the new
end_session_endpointin the API references.
Based on path instructions for **/*.go, any PR introducing new APIs, configuration options, or auth flows must include corresponding documentation updates.
backend/internal/oauth/oauth2/logout/handler.go#L59-L60: The/oauth2/logoutHTTP handler requires API documentation updates.
🤖 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/handler.go` around lines 59 - 60, Update
the documentation to cover the new RP-initiated logout flow implemented by
logoutHandler.HandleLogout: document GET/POST /oauth2/logout and its
id_token_hint, client_id, post_logout_redirect_uri, and state parameters in
docs/content/apis.mdx; document the console sign-out settings and post-logout
redirect URI configuration under docs/content/guides/; and add
end_session_endpoint to the OIDC discovery API reference.
Source: Path instructions
| it('falls back to VITE_THUNDER_BASE_URL when getServerUrl returns null', async () => { | ||
| mockGetServerUrl.mockReturnValue(null); | ||
| render(<SignOutBox />); | ||
| await waitFor(() => { | ||
| expect(fetch).toHaveBeenCalledWith( | ||
| `${import.meta.env.VITE_THUNDER_BASE_URL as string}/flow/execute`, | ||
| expect.objectContaining({method: 'POST'}) as RequestInit, | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether VITE_THUNDER_BASE_URL is used elsewhere (pre-existing convention) or newly introduced.
rg -n 'VITE_THUNDER_BASE_URL' --type ts --type tsx -g '*.env*'
rg -n 'VITE_THUNDER_BASE_URL' frontendRepository: thunder-id/thunderid
Length of output: 2316
🔴 Incorrect product name: VITE_THUNDER_BASE_URL must be ThunderID (or the appropriate template placeholder for the file type). Bare THUNDER is not an accepted short form of the product name. This appears in both the test title and the fetch URL assertion, so the env name needs to change consistently wherever it is defined and used.
🤖 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 `@frontend/apps/gate/src/components/SignOut/__tests__/SignOutBox.test.tsx`
around lines 204 - 213, Rename the fallback environment variable from
VITE_THUNDER_BASE_URL to the approved ThunderID name (or the
file-type-appropriate template placeholder) consistently in the SignOutBox test
title, fetch URL assertion, and every corresponding definition or usage.
Source: Path instructions
dd5ddc5 to
82d7bd1
Compare
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 `@backend/internal/oauth/oauth2/constants/constants.go`:
- Around line 69-70: Remove the unused RequestParamLogoutHint constant from the
OAuth constants unless logout_hint is being implemented; if supporting it,
thread the parameter through LogoutRequest and the logout handler/service
parsing and add coverage for requests containing only logout_hint.
🪄 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: c8b9fc7c-4c7d-4769-a24e-151d61df09a9
⛔ Files ignored due to path filters (1)
backend/tests/mocks/flow/sessionmock/Service_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (73)
backend/dbscripts/configdb/postgres.sqlbackend/dbscripts/configdb/sqlite.sqlbackend/internal/actorprovider/service.gobackend/internal/actorprovider/utils.gobackend/internal/actorprovider/utils_test.gobackend/internal/agent/service.gobackend/internal/application/declarative_resource.gobackend/internal/application/handler.gobackend/internal/application/model/application.gobackend/internal/application/service.gobackend/internal/flow/common/constants.gobackend/internal/flow/executor/constants.gobackend/internal/flow/executor/register.gobackend/internal/flow/executor/session_signout_executor.gobackend/internal/flow/executor/session_signout_executor_test.gobackend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/flow/flowexec/handler.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/session/Service_mock_test.gobackend/internal/flow/session/service.gobackend/internal/flow/session/service_test.gobackend/internal/inboundclient/error_constants.gobackend/internal/inboundclient/model/oauth.gobackend/internal/inboundclient/service.gobackend/internal/inboundclient/store.gobackend/internal/inboundclient/store_constants.gobackend/internal/inboundclient/store_test.gobackend/internal/oauth/init.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/oauth/oauth2/dcr/model.gobackend/internal/oauth/oauth2/dcr/service.gobackend/internal/oauth/oauth2/discovery/service.gobackend/internal/oauth/oauth2/logout/handler.gobackend/internal/oauth/oauth2/logout/handler_test.gobackend/internal/oauth/oauth2/logout/init.gobackend/internal/oauth/oauth2/logout/service.gobackend/internal/oauth/oauth2/logout/service_test.gobackend/internal/system/config/config.gobackend/internal/system/i18n/core/defaults.gobackend/internal/system/importer/service.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/providers/constants.gobackend/pkg/thunderidengine/providers/model.gobackend/pkg/thunderidengine/providers/oauth_client.gofrontend/apps/console/src/App.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/EditFlowsSettings.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/SignOutFlowSection.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/__tests__/EditFlowsSettings.test.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/__tests__/SignOutFlowSection.test.tsxfrontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsxfrontend/apps/console/src/features/applications/components/edit-application/general-settings/__tests__/AccessSection.test.tsxfrontend/apps/console/src/features/applications/models/application.tsfrontend/apps/console/src/features/applications/models/oauth.tsfrontend/apps/console/src/features/flows/components/create-flow/SelectFlowType.tsxfrontend/apps/console/src/features/flows/components/create-flow/__tests__/SelectFlowType.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsxfrontend/apps/console/src/features/flows/data/templates.jsonfrontend/apps/console/src/features/flows/models/flows.tsfrontend/apps/console/src/features/login-flow/data/executors.jsonfrontend/apps/gate/src/App.tsxfrontend/apps/gate/src/components/SignOut/SignOut.tsxfrontend/apps/gate/src/components/SignOut/SignOutBox.tsxfrontend/apps/gate/src/components/SignOut/__tests__/SignOut.test.tsxfrontend/apps/gate/src/components/SignOut/__tests__/SignOutBox.test.tsxfrontend/apps/gate/src/constants/__tests__/routes.test.tsfrontend/apps/gate/src/constants/routes.tsfrontend/apps/gate/src/pages/SignOutPage.tsxfrontend/apps/gate/src/pages/__tests__/SignOutPage.test.tsxfrontend/packages/i18n/src/locales/en-US.tstests/integration/oauth/discovery/discovery_test.go
🚧 Files skipped from review as they are similar to previous changes (61)
- frontend/apps/gate/src/pages/tests/SignOutPage.test.tsx
- frontend/apps/gate/src/components/SignOut/SignOut.tsx
- frontend/apps/gate/src/pages/SignOutPage.tsx
- backend/internal/actorprovider/utils.go
- backend/internal/flow/executor/constants.go
- backend/internal/flow/common/constants.go
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/EditFlowsSettings.tsx
- frontend/apps/console/src/features/applications/models/application.ts
- frontend/apps/console/src/features/flows/components/create-flow/tests/SelectFlowType.test.tsx
- frontend/apps/gate/src/constants/routes.ts
- backend/internal/application/model/application.go
- frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx
- frontend/apps/console/src/features/flows/models/flows.ts
- backend/dbscripts/configdb/postgres.sql
- frontend/apps/gate/src/components/SignOut/tests/SignOut.test.tsx
- backend/dbscripts/configdb/sqlite.sql
- frontend/apps/gate/src/App.tsx
- backend/internal/actorprovider/service.go
- backend/internal/flow/flowexec/engine.go
- backend/pkg/thunderidengine/providers/constants.go
- backend/internal/oauth/oauth2/logout/init.go
- backend/internal/inboundclient/error_constants.go
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/SignOutFlowSection.tsx
- backend/internal/oauth/oauth2/dcr/service.go
- backend/internal/flow/executor/register.go
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/tests/SignOutFlowSection.test.tsx
- frontend/apps/gate/src/constants/tests/routes.test.ts
- tests/integration/oauth/discovery/discovery_test.go
- backend/internal/system/config/config.go
- backend/pkg/thunderidengine/providers/oauth_client.go
- backend/internal/flow/flowexec/model.go
- backend/internal/flow/flowexec/handler.go
- frontend/apps/gate/src/components/SignOut/tests/SignOutBox.test.tsx
- frontend/apps/console/src/features/flows/data/templates.json
- backend/internal/flow/flowexec/service_test.go
- backend/internal/actorprovider/utils_test.go
- backend/internal/oauth/oauth2/logout/handler.go
- backend/pkg/thunderidengine/providers/model.go
- backend/internal/agent/service.go
- backend/internal/flow/session/service.go
- frontend/apps/console/src/features/login-flow/data/executors.json
- backend/internal/oauth/init.go
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/tests/EditFlowsSettings.test.tsx
- frontend/packages/i18n/src/locales/en-US.ts
- backend/internal/inboundclient/store_test.go
- backend/internal/application/handler.go
- backend/internal/application/declarative_resource.go
- backend/internal/oauth/oauth2/logout/service.go
- frontend/apps/console/src/features/applications/components/edit-application/general-settings/tests/AccessSection.test.tsx
- frontend/apps/gate/src/components/SignOut/SignOutBox.tsx
- backend/internal/oauth/oauth2/logout/handler_test.go
- backend/internal/oauth/oauth2/discovery/service.go
- frontend/apps/console/src/features/applications/models/oauth.ts
- backend/internal/inboundclient/store_constants.go
- backend/internal/inboundclient/store.go
- backend/internal/flow/executor/session_signout_executor_test.go
- backend/internal/flow/session/service_test.go
- backend/internal/flow/flowexec/service.go
- backend/internal/inboundclient/service.go
- frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx
- backend/internal/application/service.go
| RequestParamLogoutHint string = "logout_hint" | ||
| RequestParamPostLogoutRedirect string = "post_logout_redirect_uri" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- constants.go ---'
sed -n '1,140p' backend/internal/oauth/oauth2/constants/constants.go
echo
echo '--- search logout_hint / post_logout_redirect_uri / id_token_hint ---'
rg -n --hidden --glob '!**/.git/**' 'logout_hint|post_logout_redirect_uri|id_token_hint|LogoutRequest|post logout|logout request' backend/internal/oauth -SRepository: thunder-id/thunderid
Length of output: 12745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' backend/internal/oauth/oauth2/logout/handler.go
echo
sed -n '1,220p' backend/internal/oauth/oauth2/logout/service.go
echo
sed -n '1,220p' backend/internal/oauth/oauth2/logout/handler_test.goRepository: thunder-id/thunderid
Length of output: 15614
Remove the unused logout_hint constant or wire it into logout parsing.
RequestParamLogoutHint is defined in backend/internal/oauth/oauth2/constants/constants.go, but the logout handler and service never read it. Requests that send only logout_hint will be ignored, so either thread it through LogoutRequest and add coverage or omit the constant until it is supported.
🤖 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/constants/constants.go` around lines 69 - 70,
Remove the unused RequestParamLogoutHint constant from the OAuth constants
unless logout_hint is being implemented; if supporting it, thread the parameter
through LogoutRequest and the logout handler/service parsing and add coverage
for requests containing only logout_hint.
82d7bd1 to
ab3d183
Compare
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 `@backend/internal/actorprovider/utils.go`:
- Around line 57-59: Update the documentation for all three affected sites: in
docs/content/guides/ or the relevant configuration pages, document the sign-out
flow, sign-out enablement, and post-logout redirect URI settings represented by
actorprovider configuration; in docs/content/apis.mdx, document the OAuth2/OIDC
/oauth2/logout endpoint, post_logout_redirect_uri parameter, and
end_session_endpoint discovery; and in the same API documentation, describe
native logout through /flow/execute using sign-out flows.
🪄 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: 054a6471-af13-4f8c-aa07-f9773b0ee2fd
⛔ Files ignored due to path filters (1)
backend/tests/mocks/flow/sessionmock/Service_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (73)
backend/dbscripts/configdb/postgres.sqlbackend/dbscripts/configdb/sqlite.sqlbackend/internal/actorprovider/service.gobackend/internal/actorprovider/utils.gobackend/internal/actorprovider/utils_test.gobackend/internal/agent/service.gobackend/internal/application/declarative_resource.gobackend/internal/application/handler.gobackend/internal/application/model/application.gobackend/internal/application/service.gobackend/internal/flow/common/constants.gobackend/internal/flow/executor/constants.gobackend/internal/flow/executor/register.gobackend/internal/flow/executor/session_signout_executor.gobackend/internal/flow/executor/session_signout_executor_test.gobackend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/flow/flowexec/handler.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/session/Service_mock_test.gobackend/internal/flow/session/service.gobackend/internal/flow/session/service_test.gobackend/internal/inboundclient/error_constants.gobackend/internal/inboundclient/model/oauth.gobackend/internal/inboundclient/service.gobackend/internal/inboundclient/store.gobackend/internal/inboundclient/store_constants.gobackend/internal/inboundclient/store_test.gobackend/internal/oauth/init.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/oauth/oauth2/dcr/model.gobackend/internal/oauth/oauth2/dcr/service.gobackend/internal/oauth/oauth2/discovery/service.gobackend/internal/oauth/oauth2/logout/handler.gobackend/internal/oauth/oauth2/logout/handler_test.gobackend/internal/oauth/oauth2/logout/init.gobackend/internal/oauth/oauth2/logout/service.gobackend/internal/oauth/oauth2/logout/service_test.gobackend/internal/system/config/config.gobackend/internal/system/i18n/core/defaults.gobackend/internal/system/importer/service.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/providers/constants.gobackend/pkg/thunderidengine/providers/model.gobackend/pkg/thunderidengine/providers/oauth_client.gofrontend/apps/console/src/App.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/EditFlowsSettings.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/SignOutFlowSection.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/__tests__/EditFlowsSettings.test.tsxfrontend/apps/console/src/features/applications/components/edit-application/flows-settings/__tests__/SignOutFlowSection.test.tsxfrontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsxfrontend/apps/console/src/features/applications/components/edit-application/general-settings/__tests__/AccessSection.test.tsxfrontend/apps/console/src/features/applications/models/application.tsfrontend/apps/console/src/features/applications/models/oauth.tsfrontend/apps/console/src/features/flows/components/create-flow/SelectFlowType.tsxfrontend/apps/console/src/features/flows/components/create-flow/__tests__/SelectFlowType.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsxfrontend/apps/console/src/features/flows/data/templates.jsonfrontend/apps/console/src/features/flows/models/flows.tsfrontend/apps/console/src/features/login-flow/data/executors.jsonfrontend/apps/gate/src/App.tsxfrontend/apps/gate/src/components/SignOut/SignOut.tsxfrontend/apps/gate/src/components/SignOut/SignOutBox.tsxfrontend/apps/gate/src/components/SignOut/__tests__/SignOut.test.tsxfrontend/apps/gate/src/components/SignOut/__tests__/SignOutBox.test.tsxfrontend/apps/gate/src/constants/__tests__/routes.test.tsfrontend/apps/gate/src/constants/routes.tsfrontend/apps/gate/src/pages/SignOutPage.tsxfrontend/apps/gate/src/pages/__tests__/SignOutPage.test.tsxfrontend/packages/i18n/src/locales/en-US.tstests/integration/oauth/discovery/discovery_test.go
🚧 Files skipped from review as they are similar to previous changes (66)
- backend/internal/system/i18n/core/defaults.go
- frontend/apps/gate/src/pages/tests/SignOutPage.test.tsx
- frontend/apps/console/src/App.tsx
- frontend/apps/gate/src/components/SignOut/SignOut.tsx
- frontend/apps/gate/src/constants/tests/routes.test.ts
- backend/internal/flow/executor/constants.go
- frontend/apps/console/src/features/flows/models/flows.ts
- backend/dbscripts/configdb/sqlite.sql
- frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/SignOutFlowSection.tsx
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/EditFlowsSettings.tsx
- frontend/apps/gate/src/pages/SignOutPage.tsx
- backend/internal/actorprovider/service.go
- frontend/apps/console/src/features/flows/components/create-flow/SelectFlowType.tsx
- frontend/apps/console/src/features/applications/models/oauth.ts
- frontend/apps/gate/src/constants/routes.ts
- frontend/apps/gate/src/components/SignOut/tests/SignOut.test.tsx
- backend/internal/inboundclient/error_constants.go
- backend/internal/oauth/oauth2/logout/handler.go
- backend/internal/oauth/init.go
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/tests/EditFlowsSettings.test.tsx
- backend/dbscripts/configdb/postgres.sql
- frontend/apps/console/src/features/flows/components/create-flow/tests/SelectFlowType.test.tsx
- backend/internal/flow/flowexec/error_constants.go
- backend/internal/oauth/oauth2/logout/init.go
- backend/internal/oauth/oauth2/dcr/model.go
- backend/internal/flow/flowexec/handler.go
- backend/internal/application/declarative_resource.go
- backend/pkg/thunderidengine/config/config.go
- frontend/packages/i18n/src/locales/en-US.ts
- backend/internal/system/importer/service.go
- backend/internal/oauth/oauth2/discovery/service.go
- backend/pkg/thunderidengine/providers/oauth_client.go
- frontend/apps/console/src/features/login-flow/data/executors.json
- backend/internal/oauth/oauth2/logout/service.go
- backend/internal/flow/session/Service_mock_test.go
- backend/internal/flow/common/constants.go
- backend/internal/inboundclient/model/oauth.go
- backend/pkg/thunderidengine/providers/model.go
- backend/pkg/thunderidengine/providers/constants.go
- backend/internal/system/config/config.go
- backend/internal/flow/executor/register.go
- backend/internal/oauth/oauth2/dcr/service.go
- backend/internal/flow/flowexec/engine.go
- backend/internal/agent/service.go
- frontend/apps/console/src/features/applications/models/application.ts
- frontend/apps/console/src/features/applications/components/edit-application/flows-settings/tests/SignOutFlowSection.test.tsx
- frontend/apps/console/src/features/applications/components/edit-application/general-settings/tests/AccessSection.test.tsx
- backend/internal/inboundclient/store_test.go
- backend/internal/application/model/application.go
- backend/internal/oauth/oauth2/logout/service_test.go
- backend/internal/flow/executor/session_signout_executor.go
- frontend/apps/gate/src/components/SignOut/tests/SignOutBox.test.tsx
- backend/internal/flow/executor/session_signout_executor_test.go
- backend/internal/flow/session/service.go
- backend/internal/inboundclient/store_constants.go
- backend/internal/inboundclient/service.go
- backend/internal/oauth/oauth2/logout/handler_test.go
- backend/internal/flow/flowexec/service.go
- backend/internal/inboundclient/store.go
- backend/internal/application/handler.go
- frontend/apps/gate/src/components/SignOut/SignOutBox.tsx
- backend/internal/flow/session/service_test.go
- frontend/apps/console/src/features/applications/components/edit-application/general-settings/AccessSection.tsx
- frontend/apps/console/src/features/flows/data/templates.json
- backend/internal/application/service.go
| AuthFlowID: client.AuthFlowID, | ||
| SignOutFlowID: client.SignOutFlowID, | ||
| IsSignOutFlowEnabled: client.IsSignOutFlowEnabled, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/. Please update the relevant documentation before merging.
Missing documentation:
backend/internal/actorprovider/utils.go#L57-L59: Document the sign-out flows and post-logout redirect URIs configuration indocs/content/guides/or related config pages.backend/internal/oauth/oauth2/constants/constants.go#L69-L69: Document the OAuth2/OIDC/oauth2/logoutendpoint,post_logout_redirect_uriparameter, andend_session_endpointdiscovery indocs/content/apis.mdx.backend/internal/flow/flowexec/service_test.go#L2240-L2254: Document the native logout through/flow/execute(sign-out flows) indocs/content/apis.mdx.
📍 Affects 3 files
backend/internal/actorprovider/utils.go#L57-L59(this comment)backend/internal/oauth/oauth2/constants/constants.go#L69-L69backend/internal/flow/flowexec/service_test.go#L2240-L2254
🤖 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/actorprovider/utils.go` around lines 57 - 59, Update the
documentation for all three affected sites: in docs/content/guides/ or the
relevant configuration pages, document the sign-out flow, sign-out enablement,
and post-logout redirect URI settings represented by actorprovider
configuration; in docs/content/apis.mdx, document the OAuth2/OIDC /oauth2/logout
endpoint, post_logout_redirect_uri parameter, and end_session_endpoint
discovery; and in the same API documentation, describe native logout through
/flow/execute using sign-out flows.
Source: Path instructions
| // lands the browser on the validated post-logout redirect URI once the flow completes. | ||
| type logoutHandler struct { | ||
| service LogoutServiceInterface | ||
| flowExecService flowexec.FlowExecServiceInterface |
There was a problem hiding this comment.
Do we need a direct dependency to flowExec service? Can we proxy this through the logout service?
| } | ||
| // The handle must belong to the expected flow: never end a session grouped under a different flow. | ||
| if flowID != "" && sess.FlowID != flowID { | ||
| s.logger.Debug(ctx, "Session handle belongs to a different flow; not terminating") |
There was a problem hiding this comment.
Shouldn't this be a error scenario?
8f8618d to
6082d48
Compare
6082d48 to
4fc4ee0
Compare
4fc4ee0 to
d154c29
Compare
Add session sign-out authored and configured per flow, on the same flow-graph model login/SSO use, rather than a hardcoded path in the OAuth2 layer. Two entry points converge on one termination machinery: - RP-initiated logout via GET/POST /oauth2/logout (end_session_endpoint) - Native, API-driven sign-out via the existing /flow/execute machinery Both resolve the target session, revoke it and its stored context, clear the per-flow SSO cookie, and (browser path) confirm with the user and land them on a validated post_logout_redirect_uri. Key design: sign-out separates two resolutions. The flow to run is the client's SignOutFlowID (first-class SIGNOUT flow type, resolved like recovery); the session to end belongs to the login flow, so AuthFlowID is threaded onto the engine context as SessionFlowID and used by applyInboundSSO to read the correct login-flow cookie. SessionSignOutExecutor calls session.Terminate (State=ENDED, bump version, delete contexts and participants; idempotent) and signals a cookie clear applied via the existing ssoTransport.Clear seam. The gate sign-out page resumes the flow over /flow/execute and runs on the published @thunderid/react SDK. OIDC discovery advertises end_session_endpoint. Console gains the SIGNOUT flow type, a sign-out flow selector, templates, post-logout redirect URI settings, and the sign-out executor in the flow editor. OIDC spec terms (/oauth2/logout, post_logout_redirect_uri, end_session_endpoint, logout_hint) are preserved; user-facing text and internal identifiers use "Sign Out". Refs thunder-id#3916
d154c29 to
fe8e815
Compare
Purpose
Implements session sign-out on the flow-graph model, closing the loop on SSO: #3672 / #3779 established and reused flow-centric sessions — this ends them.
Direction is set by discussion #3916: sign-out is authored and configured per flow, on the same flow-graph model login/SSO use, rather than as a hardcoded path in the OAuth2 layer.
Two entry points converge on one termination machinery:
Both resolve the target session, revoke it and its stored context, clear the per-flow SSO cookie, and — on the browser path — confirm with the user and land them on a validated post_logout_redirect_uri.
Approach
Two separate resolutions — the crux. A session is keyed by the login flow's ID (cookie tid_sso_<hash(loginFlowID)>), but the flow we run on sign-out is a different flow (the client's sign-out flow). These are kept distinct:
Termination machinery. SessionSignOutExecutor reads the inbound handle + target flow id, calls session.Terminate (resolve by handle → State=ENDED, bump version, delete contexts + participants; idempotent on absent/ended sessions), and emits a cookie-clear signal the flow handler applies via the existing ssoTransport.Clear seam with CookieName(session.FlowID).
RP-initiated endpoint. New oauth2/logout package mirrors authz/: validates id_token_hint → aud → client → AuthFlowID, reads the per-flow cookie, and redirects the browser to the gate sign-out page with the flow executionId. post_logout_redirect_uri is validated against the client's allow-list (PostLogoutRedirectURIs) or falls back to a default. end_session_endpoint is advertised in OIDC discovery.
Gate. A flow-type-agnostic SignOutBox resumes the execution via a plain /flow/execute fetch (credentials: 'include'), rendering any confirmation step with FlowComponentRenderer and echoing the per-step challengeToken on submit. It runs on the published @thunderid/react 0.4.0 — no bespoke SDK sign-out surface, so this is a single-repo change (no SDK PR).
Terminology. User-facing text and internal identifiers use "Sign Out"; OIDC spec terms are preserved as-is (/oauth2/logout, post_logout_redirect_uri, end_session_endpoint, logout_hint).
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
id_token_hint+ optionalstate), including validation and redirecting to the configured post-logout target.