Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions backend/internal/flow/common/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,14 @@ const (
// from the SSO checkpoint snapshot so each flow execution mints a fresh tfid rather than reusing a
// prior one on SSO reuse.
RuntimeKeyTokenFamilyID = "tokenFamilyId"
// RuntimeKeyLogoutPromptRequired is set by the OAuth RP-initiated logout layer when a logout was
// requested without a valid id_token_hint. A sign-out flow's session sign-out node reads it to
// decide whether the End-User must confirm the logout before the session is terminated.
RuntimeKeyLogoutPromptRequired = "logoutPromptRequired"
// 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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

)

// SSOCheckpointKey scopes a per-checkpoint SSO control key (RuntimeKeySSOSessionPresent,
Expand Down
4 changes: 4 additions & 0 deletions backend/internal/flow/executor/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ const (
propertyKeyCallbackType = "callbackType"
propertyKeyLoginHintAttribute = "loginHintAttribute"
propertyKeyMaxOTPAttempts = "maxAttempts"
// propertyKeyPromptOnSignOut, when set to boolean true on a session sign-out node, makes the executor
// confirm the logout with the End-User (via the node's onIncomplete prompt) whenever the RP-initiated
// logout was not accompanied by a valid id_token_hint (RuntimeKeyLogoutPromptRequired).
propertyKeyPromptOnSignOut = "promptOnSignOut"
)

// nonSearchableInputs contains the list of user inputs/ attributes that are non-searchable.
Expand Down
31 changes: 31 additions & 0 deletions backend/internal/flow/executor/session_signout_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ func newSessionSignOutExecutor(flowFactory core.FlowFactoryInterface, sso sessio
// cookie-clear signal. Terminate is idempotent, so a missing or already-ended session is not an
// error; the cookie is cleared regardless so the browser drops any stale handle. It routes to the
// success outcome — sign-out completes even when there was nothing to end.
//
// When the node opts in with the promptOnSignOut property and the RP-initiated logout arrived without
// a valid id_token_hint (RuntimeKeyLogoutPromptRequired), the executor first routes to the node's
// onIncomplete confirmation prompt and only terminates the session once the End-User confirms. This
// keeps the confirmation logic in the executor rather than a node condition the flow editor cannot
// represent.
func (e *sessionSignOutExecutor) Execute(ctx *providers.NodeContext) (*providers.ExecutorResponse, error) {
logger := e.logger.With(log.String(log.LoggerKeyExecutionID, ctx.ExecutionID))

Expand All @@ -69,6 +75,16 @@ func (e *sessionSignOutExecutor) Execute(ctx *providers.NodeContext) (*providers
EngineData: make(map[string]string),
}

// Ask the End-User to confirm before terminating when the node requests it and no valid
// id_token_hint established the request's legitimacy. The prompt is shown once: the marker is
// persisted in RuntimeData so the re-run (after confirmation) proceeds to terminate the session.
if e.confirmationRequired(ctx) {
execResp.RuntimeData[common.RuntimeKeyLogoutPromptShown] = dataValueTrue
execResp.Status = providers.ExecUserInputRequired
logger.Debug(ctx.Context, "Routing to sign-out confirmation prompt")
return execResp, nil
}

in := session.SSOInputsFrom(ctx.Context)
if _, err := e.sso.Terminate(ctx.Context, in.Handle, in.FlowID); err != nil {
return execResp, err
Expand All @@ -82,3 +98,18 @@ func (e *sessionSignOutExecutor) Execute(ctx *providers.NodeContext) (*providers
logger.Debug(ctx.Context, "Terminated SSO session on sign-out", log.String("flowId", in.FlowID))
return execResp, nil
}

// confirmationRequired reports whether the executor should route to its onIncomplete confirmation
// prompt before terminating the session. It is true only when the node opts in (promptOnSignOut),
// the RP-initiated logout requires a prompt (no valid id_token_hint), and the prompt has not already
// been shown in this flow run.
func (e *sessionSignOutExecutor) confirmationRequired(ctx *providers.NodeContext) bool {
promptEnabled, _ := ctx.NodeProperties[propertyKeyPromptOnSignOut].(bool)
if !promptEnabled {
return false
}
if ctx.RuntimeData[common.RuntimeKeyLogoutPromptRequired] != dataValueTrue {
return false
}
return ctx.RuntimeData[common.RuntimeKeyLogoutPromptShown] != dataValueTrue
}
77 changes: 77 additions & 0 deletions backend/internal/flow/executor/session_signout_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,80 @@ func (suite *SessionSignOutExecutorTestSuite) TestTerminateError() {
suite.Require().Error(err)
suite.Empty(resp.EngineData[common.RuntimeKeySSOSessionCleared])
}

// TestPromptsWhenConfirmationRequired covers a prompt-enabled node whose logout arrived without a
// valid id_token_hint: the executor routes to the confirmation prompt (incomplete), marks the prompt
// as shown, and does not terminate the session.
func (suite *SessionSignOutExecutorTestSuite) TestPromptsWhenConfirmationRequired() {
sso := sessionmock.NewServiceMock(suite.T())
exec := suite.newExecutor(sso)

ctx := signOutNodeContext()
ctx.NodeProperties = map[string]interface{}{propertyKeyPromptOnSignOut: true}
ctx.RuntimeData = map[string]string{common.RuntimeKeyLogoutPromptRequired: dataValueTrue}

resp, err := exec.Execute(ctx)

suite.Require().NoError(err)
suite.Equal(providers.ExecUserInputRequired, resp.Status)
suite.Equal(dataValueTrue, resp.RuntimeData[common.RuntimeKeyLogoutPromptShown])
suite.Empty(resp.EngineData[common.RuntimeKeySSOSessionCleared])
sso.AssertNotCalled(suite.T(), "Terminate", mock.Anything, mock.Anything, mock.Anything)
}

// TestTerminatesAfterConfirmation covers the re-run once the prompt has been shown: the guard marker
// is present, so the executor terminates the session instead of prompting again.
func (suite *SessionSignOutExecutorTestSuite) TestTerminatesAfterConfirmation() {
sso := sessionmock.NewServiceMock(suite.T())
sso.EXPECT().Terminate(mock.Anything, "handle-abc", "flow-1").
Return(&session.Session{SessionID: "sess-1", State: session.StateEnded}, nil)
exec := suite.newExecutor(sso)

ctx := signOutNodeContext()
ctx.NodeProperties = map[string]interface{}{propertyKeyPromptOnSignOut: true}
ctx.RuntimeData = map[string]string{
common.RuntimeKeyLogoutPromptRequired: dataValueTrue,
common.RuntimeKeyLogoutPromptShown: dataValueTrue,
}

resp, err := exec.Execute(ctx)

suite.Require().NoError(err)
suite.Equal(providers.ExecComplete, resp.Status)
suite.Equal(dataValueTrue, resp.EngineData[common.RuntimeKeySSOSessionCleared])
}

// TestTerminatesWhenHintProvided covers a prompt-enabled node whose logout carried a valid
// id_token_hint (no prompt flag): the executor terminates directly without confirming.
func (suite *SessionSignOutExecutorTestSuite) TestTerminatesWhenHintProvided() {
sso := sessionmock.NewServiceMock(suite.T())
sso.EXPECT().Terminate(mock.Anything, "handle-abc", "flow-1").Return(nil, nil)
exec := suite.newExecutor(sso)

ctx := signOutNodeContext()
ctx.NodeProperties = map[string]interface{}{propertyKeyPromptOnSignOut: true}

resp, err := exec.Execute(ctx)

suite.Require().NoError(err)
suite.Equal(providers.ExecComplete, resp.Status)
suite.Equal(dataValueTrue, resp.EngineData[common.RuntimeKeySSOSessionCleared])
}

// TestTerminatesWhenNodeDoesNotOptIn covers a node without the promptOnSignOut property (e.g. the
// always-prompt default flow, where a separate prompt node precedes this one): even when a prompt was
// requested, the executor terminates rather than emitting a second, unhandled prompt.
func (suite *SessionSignOutExecutorTestSuite) TestTerminatesWhenNodeDoesNotOptIn() {
sso := sessionmock.NewServiceMock(suite.T())
sso.EXPECT().Terminate(mock.Anything, "handle-abc", "flow-1").Return(nil, nil)
exec := suite.newExecutor(sso)

ctx := signOutNodeContext()
ctx.RuntimeData = map[string]string{common.RuntimeKeyLogoutPromptRequired: dataValueTrue}

resp, err := exec.Execute(ctx)

suite.Require().NoError(err)
suite.Equal(providers.ExecComplete, resp.Status)
suite.Equal(dataValueTrue, resp.EngineData[common.RuntimeKeySSOSessionCleared])
}
28 changes: 19 additions & 9 deletions backend/internal/oauth/oauth2/logout/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"context"
"errors"

flowcommon "github.com/thunder-id/thunderid/internal/flow/common"
"github.com/thunder-id/thunderid/internal/flow/flowexec"
"github.com/thunder-id/thunderid/internal/oauth/oauth2/constants"
oauth2utils "github.com/thunder-id/thunderid/internal/oauth/oauth2/utils"
Expand All @@ -42,7 +43,6 @@ var (
errClientRequired = errors.New("id_token_hint or client_id is required")
errInvalidClient = errors.New("invalid client")
errInvalidPostLogoutRedirectURI = errors.New("invalid post_logout_redirect_uri")
errIDTokenHintRequired = errors.New("id_token_hint is required when post_logout_redirect_uri is provided")
)

// LogoutRequest holds the RP-initiated logout parameters received from the request.
Expand All @@ -62,6 +62,10 @@ type LogoutResolution struct {
State string
Headers map[string][]string
QueryParams map[string][]string
// PromptRequired reports whether the sign-out flow must confirm the logout with the End-User.
// Per OIDC RP-Initiated Logout the OP MUST ask when no id_token_hint was supplied; it is set
// true in that case so a conditional sign-out flow can decide whether to render its prompt.
PromptRequired bool
}

// SignOutInitiation is the result of starting an RP-initiated sign-out: the stored logout-request id
Expand Down Expand Up @@ -127,14 +131,19 @@ func (s *logoutService) InitiateSignOutFlow(
// JWT with user identity claims into the flow context store.
forwardedQueryParams := filterQueryParams(resolution.QueryParams, constants.RequestParamIDTokenHint)

executionID, svcErr := s.flowExecService.InitiateFlow(ctx, &flowexec.FlowInitContext{
initContext := &flowexec.FlowInitContext{
ApplicationID: resolution.AppID,
FlowType: string(providers.FlowTypeSignOut),
InitiatorRequest: &providers.InitiatorRequest{
Headers: sysutils.FilterSensitiveHeaders(resolution.Headers),
QueryParams: forwardedQueryParams,
},
})
}
if resolution.PromptRequired {
initContext.RuntimeData = map[string]string{flowcommon.RuntimeKeyLogoutPromptRequired: "true"}
}

executionID, svcErr := s.flowExecService.InitiateFlow(ctx, initContext)
if svcErr != nil {
return nil, svcErr
}
Expand Down Expand Up @@ -175,13 +184,13 @@ func (s *logoutService) CompleteSignOut(ctx context.Context, logoutID string) (s

// Resolve identifies the client from id_token_hint (preferred) or the client_id parameter, validates
// any post_logout_redirect_uri against the client's registered list, and returns the logout target.
//
// id_token_hint is not required: a request carrying only client_id is accepted, and any
// post_logout_redirect_uri is still confirmed legitimate by matching the client's registered list
// (the OP's "other means" of confirming the redirection target per OIDC RP-Initiated Logout). When no
// id_token_hint is supplied the resolution is marked PromptRequired so the sign-out flow can confirm
// the logout with the End-User, as the spec requires in that case.
func (s *logoutService) Resolve(ctx context.Context, req LogoutRequest) (*LogoutResolution, error) {
// Per OIDC RP-Initiated Logout, if post_logout_redirect_uri is supplied the id_token_hint MUST be
// supplied too; the OP must not redirect to the URI without a valid hint.
if req.PostLogoutRedirectURI != "" && req.IDTokenHint == "" {
return nil, errIDTokenHintRequired
}

clientID := req.ClientID
if req.IDTokenHint != "" {
hintClientID, err := s.clientIDFromIDTokenHint(ctx, req.IDTokenHint)
Expand Down Expand Up @@ -223,6 +232,7 @@ func (s *logoutService) Resolve(ctx context.Context, req LogoutRequest) (*Logout
State: req.State,
Headers: req.Headers,
QueryParams: req.QueryParams,
PromptRequired: req.IDTokenHint == "",
}, nil
}

Expand Down
55 changes: 51 additions & 4 deletions backend/internal/oauth/oauth2/logout/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"

flowcommon "github.com/thunder-id/thunderid/internal/flow/common"
"github.com/thunder-id/thunderid/internal/flow/flowexec"
"github.com/thunder-id/thunderid/internal/system/config"
tidcommon "github.com/thunder-id/thunderid/pkg/thunderidengine/common"
Expand Down Expand Up @@ -104,6 +105,28 @@ func (suite *LogoutServiceTestSuite) TestInitiateSignOutFlow_StoresContextAndIni
suite.Empty(captured.RuntimeData)
}

func (suite *LogoutServiceTestSuite) TestInitiateSignOutFlow_PromptRequiredSetsRuntimeData() {
store := newLogoutRequestStoreInterfaceMock(suite.T())
store.EXPECT().AddRequest(mock.Anything, mock.Anything).Return("logout-1", nil)
flowSvc := flowexecmock.NewFlowExecServiceInterfaceMock(suite.T())
var captured *flowexec.FlowInitContext
flowSvc.EXPECT().InitiateFlow(mock.Anything, mock.Anything).RunAndReturn(
func(_ context.Context, ic *flowexec.FlowInitContext) (string, *tidcommon.ServiceError) {
captured = ic
return "exec-1", nil
})
svc := suite.newServiceWithStore(store, flowSvc)

_, svcErr := svc.InitiateSignOutFlow(context.Background(), &LogoutResolution{
AppID: "app-1", PromptRequired: true,
})

suite.Nil(svcErr)
// The confirmation requirement is carried to the flow so a conditional prompt node can read it.
suite.Require().NotNil(captured)
suite.Equal("true", captured.RuntimeData[flowcommon.RuntimeKeyLogoutPromptRequired])
}

func (suite *LogoutServiceTestSuite) TestCompleteSignOut_ReturnsRedirectWithStateAndConsumes() {
store := newLogoutRequestStoreInterfaceMock(suite.T())
store.EXPECT().GetRequest(mock.Anything, "logout-1").Return(true, logoutRequestContext{
Expand Down Expand Up @@ -301,14 +324,34 @@ func makeIDTokenMultiAud(iss string, aud []string, azp string) string {
enc(map[string]interface{}{"iss": iss, "aud": aud, "azp": azp}) + ".sig"
}

func (suite *LogoutServiceTestSuite) TestResolve_RedirectWithoutIDTokenHintRejected() {
svc, _, _ := suite.newService()
func (suite *LogoutServiceTestSuite) TestResolve_ClientIDWithRedirectWithoutIDTokenHint() {
svc, _, actor := suite.newService()
actor.EXPECT().GetOAuthClientByClientID(mock.Anything, "client-x").
Return(clientWithPostLogout("https://rp.example/after"), nil)

_, err := svc.Resolve(context.Background(), LogoutRequest{
res, err := svc.Resolve(context.Background(), LogoutRequest{
ClientID: "client-x", PostLogoutRedirectURI: "https://rp.example/after",
})

suite.Require().ErrorIs(err, errIDTokenHintRequired)
// id_token_hint is not required: the redirect is confirmed by the client's registered
// allow-list, and the missing hint marks the logout for End-User confirmation.
suite.Require().NoError(err)
suite.Equal("app-1", res.AppID)
suite.Equal("https://rp.example/after", res.PostLogoutRedirectURI)
suite.True(res.PromptRequired)
}

func (suite *LogoutServiceTestSuite) TestResolve_UnregisteredRedirectWithoutIDTokenHintRejected() {
svc, _, actor := suite.newService()
actor.EXPECT().GetOAuthClientByClientID(mock.Anything, "client-x").
Return(clientWithPostLogout("https://rp.example/after"), nil)

// Dropping the mandatory id_token_hint must not weaken redirect-target validation.
_, err := svc.Resolve(context.Background(), LogoutRequest{
ClientID: "client-x", PostLogoutRedirectURI: "https://evil.example/steal",
})

suite.Require().ErrorIs(err, errInvalidPostLogoutRedirectURI)
}

func (suite *LogoutServiceTestSuite) TestResolve_UnregisteredRedirectRejected() {
Expand All @@ -335,6 +378,8 @@ func (suite *LogoutServiceTestSuite) TestResolve_ClientIDWithoutRedirect() {
suite.Require().NoError(err)
suite.Equal("app-1", res.AppID)
suite.Empty(res.PostLogoutRedirectURI)
// No id_token_hint supplied, so the logout must be confirmed with the End-User.
suite.True(res.PromptRequired)
}

func (suite *LogoutServiceTestSuite) TestResolve_NoClientReference() {
Expand Down Expand Up @@ -370,6 +415,8 @@ func (suite *LogoutServiceTestSuite) TestResolve_IDTokenHintIdentifiesClient() {
suite.Equal("app-1", res.AppID)
suite.Equal("https://rp.example/after", res.PostLogoutRedirectURI)
suite.Equal("xyz", res.State)
// A valid id_token_hint establishes legitimacy, so no confirmation prompt is forced.
suite.False(res.PromptRequired)
}

func (suite *LogoutServiceTestSuite) TestResolve_IDTokenHintPrefersAzpForMultiAudience() {
Expand Down
Loading
Loading