From 7c81869b56fcebe627ce5cc42113f1752dd6a3c3 Mon Sep 17 00:00:00 2001 From: Maduranga Siriwardena Date: Tue, 28 Jul 2026 14:05:13 +0530 Subject: [PATCH] Forward the SSO-Check node's session and checkpoint reads to the Session node An SSO reuse issued six SSO_SESSION/SSO_SESSION_CONTEXT statements for four rows' worth of work. The SSO-Check node resolved the session and answered "does this checkpoint exist" by listing every checkpoint id, then the paired Session node re-read the same session by the same handle and fetched the checkpoint context, both microseconds later in the same request. Neither repeat read validated anything the resolver had not already checked. Hand both rows to the Session node through ForwardedData, the engine's existing node-to-node channel, so the load path uses what the check node already read. LoadCheckpoint takes them on its input struct and validates each before use: the session is rejected unless its handle matches the one being loaded, the context unless its session id and checkpoint id both match. Anything rejected or absent is read from the store, so a Session node reached without the handover still works and a partial handover costs a query rather than correctness. ForwardedData reaches the immediate next node only, so the rows are forwarded on the Skip outcome alone. The Authenticate outcome prompts and suspends the flow, and ForwardedData is a serialized field, so data left there could be persisted with the flow context. HasCheckpoint becomes FindCheckpoint, returning the checkpoint context instead of a bool. Fetching the row by its full primary key answers the availability question by itself and gives the check node something to forward, so ListCheckpointIDs and SSO-SESS-08 have no callers left and are removed. Measured on PostgreSQL with pg_stat_statements: the reuse leg drops from six statements to four (one SSO_SESSION read instead of two, one SSO_SESSION_CONTEXT read instead of two), removing two round trips and the 0.121 ms the checkpoint listing cost, which was a prefix range scan against the 0.024 ms full-key probe that replaces it. Verified end to end: the whole reuse leg falls from 20 statements to 18 with SSO-SESS-08 absent from pg_stat_statements, and 250 concurrent reuses across 25 workers complete with no failures. No SQL was added, so nothing is engine-specific. --- backend/internal/flow/common/constants.go | 6 + .../flow/executor/session_executor.go | 31 +++- .../flow/executor/session_executor_test.go | 53 ++++++- .../flow/executor/sso_check_executor.go | 14 +- .../flow/executor/sso_check_executor_test.go | 24 +++- .../flow/session/Service_mock_test.go | 86 +++++------- backend/internal/flow/session/interface.go | 3 - backend/internal/flow/session/service.go | 125 +++++++++++------ backend/internal/flow/session/service_test.go | 132 +++++++++++++++--- .../flow/session/sessionStore_mock_test.go | 68 --------- .../session/session_context_store_test.go | 37 ----- backend/internal/flow/session/store.go | 24 ---- .../internal/flow/session/store_constants.go | 8 -- .../mocks/flow/sessionmock/Service_mock.go | 86 +++++------- 14 files changed, 373 insertions(+), 324 deletions(-) diff --git a/backend/internal/flow/common/constants.go b/backend/internal/flow/common/constants.go index acb0967c0f..373361d9ed 100644 --- a/backend/internal/flow/common/constants.go +++ b/backend/internal/flow/common/constants.go @@ -309,6 +309,12 @@ const ( // ForwardedDataKeyExpiryMinutes is the key for the OTP expiry duration (in minutes) inside the // ForwardedData[ForwardedDataKeyTemplateData] map forwarded by OTPExecutor to sender executors. ForwardedDataKeyExpiryMinutes = "expiryMinutes" + // ForwardedDataKeySSOSession holds the SSO session the SSO-Check node resolved, forwarded to the + // paired Session node so it restores the checkpoint without reading the same row again. + ForwardedDataKeySSOSession = "ssoSession" + // ForwardedDataKeySSOSessionContext holds the checkpoint context the SSO-Check node fetched, + // forwarded alongside ForwardedDataKeySSOSession. + ForwardedDataKeySSOSessionContext = "ssoSessionContext" ) // InterceptorStatus represents the outcome of an interceptor execution. diff --git a/backend/internal/flow/executor/session_executor.go b/backend/internal/flow/executor/session_executor.go index 299eb83bed..c726819806 100644 --- a/backend/internal/flow/executor/session_executor.go +++ b/backend/internal/flow/executor/session_executor.go @@ -224,6 +224,18 @@ func setHandleOut(execResp *providers.ExecutorResponse, handle string) { execResp.EngineData[common.RuntimeKeySSOSessionHandle] = handle } +// readForwardedSSOData returns the session and checkpoint context the paired SSO-Check node read and +// forwarded, or nils when this node was not reached directly from it. The service re-reads whatever it +// is not given, so a missing or partial handover costs a query rather than correctness. +func readForwardedSSOData(ctx *providers.NodeContext) (*session.Session, *session.SessionContext) { + if ctx.ForwardedData == nil { + return nil, nil + } + forwardedSession, _ := ctx.ForwardedData[common.ForwardedDataKeySSOSession].(*session.Session) + forwardedContext, _ := ctx.ForwardedData[common.ForwardedDataKeySSOSessionContext].(*session.SessionContext) + return forwardedSession, forwardedContext +} + // loadCheckpoint loads a checkpoint's saved flow state into the execution context so downstream // nodes continue with the authenticated subject and claims. The SSO session service fetches the // session and its checkpoint context (and refreshes the session's activity); this executor @@ -234,7 +246,15 @@ func (e *sessionExecutor) loadCheckpoint(ctx *providers.NodeContext, execResp *p // An SSO reuse still issues a fresh grant, so mint a new token family id and record it against the // joining participant. It is published onto RuntimeData after the snapshot replay below. tokenFamilyID := e.resolveTokenFamilyID(ctx, logger) - sess, sc, err := e.sso.LoadCheckpoint(ctx.Context, handle, checkpoint, ctx.Application.ID, tokenFamilyID) + forwardedSession, forwardedContext := readForwardedSSOData(ctx) + ssoSession, snapshot, err := e.sso.LoadCheckpoint(ctx.Context, session.LoadCheckpointInput{ + Handle: handle, + Checkpoint: checkpoint, + AppID: ctx.Application.ID, + TokenFamilyID: tokenFamilyID, + Session: forwardedSession, + Context: forwardedContext, + }) if err != nil { return err } @@ -242,20 +262,21 @@ func (e *sessionExecutor) loadCheckpoint(ctx *providers.NodeContext, execResp *p // Rehydrate the AuthUser from the snapshot verbatim — it was stored as-is — so downstream nodes // continue with the same subject and attributes this session resolved when the checkpoint was saved. var authUser providers.AuthUser - if err := json.Unmarshal(sc.AuthUser, &authUser); err != nil { + if err := json.Unmarshal(snapshot.AuthUser, &authUser); err != nil { return fmt.Errorf("failed to rehydrate subject reference from snapshot: %w", err) } execResp.AuthUser = authUser // Replay the snapshotted RuntimeData (the effective attribute set captured at save) so downstream // nodes see the same attributes the fresh path produced. - for k, v := range sc.RuntimeData { + for k, v := range snapshot.RuntimeData { execResp.RuntimeData[k] = v } // auth_time comes from the lean session, not the context. Set it after the RuntimeData replay so // the live, session-derived value wins over any stale snapshot copy. - if !sess.AuthenticatedAt.IsZero() { - execResp.RuntimeData[common.RuntimeKeyAuthTime] = strconv.FormatInt(sess.AuthenticatedAt.Unix(), 10) + if !ssoSession.AuthenticatedAt.IsZero() { + execResp.RuntimeData[common.RuntimeKeyAuthTime] = + strconv.FormatInt(ssoSession.AuthenticatedAt.Unix(), 10) } // Publish the freshly minted token family id after the snapshot replay so it is never shadowed by // a stale copy (the tfid is excluded from the snapshot, so this is the only source). diff --git a/backend/internal/flow/executor/session_executor_test.go b/backend/internal/flow/executor/session_executor_test.go index 6ae387160a..d23f3a6519 100644 --- a/backend/internal/flow/executor/session_executor_test.go +++ b/backend/internal/flow/executor/session_executor_test.go @@ -331,7 +331,9 @@ func (suite *SessionExecutorTestSuite) TestSSOLoad() { snapAuthUser := `{"default":{"entityReference":{"entityId":"user-2","ouId":"ou-9","type":"person"},` + `"attributes":{"attributes":{"email":{"value":"bob@example.com"}}}}}` sso := sessionmock.NewServiceMock(suite.T()) - sso.EXPECT().LoadCheckpoint(mock.Anything, "handle-abc", "session", "app-456", mock.Anything).Return( + sso.EXPECT().LoadCheckpoint(mock.Anything, mock.MatchedBy(func(in session.LoadCheckpointInput) bool { + return in.Handle == "handle-abc" && in.Checkpoint == "session" && in.AppID == "app-456" + })).Return( &session.Session{ SessionID: "sess-1", SubjectID: "user-2", HandleID: "handle-abc", AuthenticatedAt: time.Unix(1700000000, 0).UTC(), @@ -368,11 +370,56 @@ func (suite *SessionExecutorTestSuite) TestSSOLoad() { suite.Equal("1700000000", resp.RuntimeData[common.RuntimeKeyAuthTime]) } +// TestSSOLoad_PassesForwardedReadsToService is the executor half of the reuse-path read reduction: the +// rows the SSO-Check node put on ForwardedData must reach the service, which then skips both reads. +func (suite *SessionExecutorTestSuite) TestSSOLoad_PassesForwardedReadsToService() { + forwardedSession := &session.Session{SessionID: "sess-1", HandleID: "handle-abc"} + forwardedContext := &session.SessionContext{ + SessionID: "sess-1", CheckpointID: "session", AuthUser: json.RawMessage("{}"), + } + + sso := sessionmock.NewServiceMock(suite.T()) + sso.EXPECT().LoadCheckpoint(mock.Anything, mock.MatchedBy(func(in session.LoadCheckpointInput) bool { + return in.Session == forwardedSession && in.Context == forwardedContext + })).Return(forwardedSession, forwardedContext, nil) + exec := suite.newExecutor(sso, managermock.NewAuthnProviderManagerMock(suite.T())) + + ctx := ssoLoadCtx() + ctx.ForwardedData = map[string]interface{}{ + common.ForwardedDataKeySSOSession: forwardedSession, + common.ForwardedDataKeySSOSessionContext: forwardedContext, + } + + _, err := exec.Execute(ctx) + + suite.Require().NoError(err) +} + +// TestSSOLoad_ForwardsNothingWhenAbsent covers a Session node reached without the paired SSO-Check +// node's handover: the service is asked to read both rows itself rather than being handed junk. +func (suite *SessionExecutorTestSuite) TestSSOLoad_ForwardsNothingWhenAbsent() { + sso := sessionmock.NewServiceMock(suite.T()) + sso.EXPECT().LoadCheckpoint(mock.Anything, mock.MatchedBy(func(in session.LoadCheckpointInput) bool { + return in.Session == nil && in.Context == nil + })).Return( + &session.Session{SessionID: "sess-1", HandleID: "handle-abc"}, + &session.SessionContext{SessionID: "sess-1", CheckpointID: "session", AuthUser: json.RawMessage("{}")}, + nil) + exec := suite.newExecutor(sso, managermock.NewAuthnProviderManagerMock(suite.T())) + + ctx := ssoLoadCtx() + ctx.ForwardedData = map[string]interface{}{"unrelated": 42} + + _, err := exec.Execute(ctx) + + suite.Require().NoError(err) +} + // TestSSOLoad_ErrorFailsFlow covers a load failure surfacing as a server error so the task-execution // node fails the flow (the credential steps were already skipped). func (suite *SessionExecutorTestSuite) TestSSOLoad_ErrorFailsFlow() { sso := sessionmock.NewServiceMock(suite.T()) - sso.EXPECT().LoadCheckpoint(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + sso.EXPECT().LoadCheckpoint(mock.Anything, mock.Anything). Return(nil, nil, errors.New("resolved session no longer exists")) exec := suite.newExecutor(sso, managermock.NewAuthnProviderManagerMock(suite.T())) @@ -386,7 +433,7 @@ func (suite *SessionExecutorTestSuite) TestSSOLoad_ErrorFailsFlow() { // reconstruct the subject, so the flow fails. func (suite *SessionExecutorTestSuite) TestSSOLoad_RehydrateErrorFailsFlow() { sso := sessionmock.NewServiceMock(suite.T()) - sso.EXPECT().LoadCheckpoint(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return( + sso.EXPECT().LoadCheckpoint(mock.Anything, mock.Anything).Return( &session.Session{SessionID: "sess-1", HandleID: "handle-abc"}, &session.SessionContext{SessionID: "sess-1", AuthUser: json.RawMessage("not-json")}, nil) exec := suite.newExecutor(sso, managermock.NewAuthnProviderManagerMock(suite.T())) diff --git a/backend/internal/flow/executor/sso_check_executor.go b/backend/internal/flow/executor/sso_check_executor.go index 6660d2bafa..a0238eed8c 100644 --- a/backend/internal/flow/executor/sso_check_executor.go +++ b/backend/internal/flow/executor/sso_check_executor.go @@ -89,16 +89,24 @@ func (e *ssoCheckExecutor) Execute(ctx *providers.NodeContext) (*providers.Execu execResp.RuntimeData[common.RuntimeKeySSOSessionHandle] = resolved.HandleID } - present := false + var snapshot *session.SessionContext if resolved != nil && checkpoint != "" { - if present, err = e.sso.HasCheckpoint(ctx.Context, resolved.SessionID, checkpoint); err != nil { + if snapshot, err = e.sso.FindCheckpoint(ctx.Context, resolved.SessionID, checkpoint); err != nil { return execResp, err } } - if present { + if snapshot != nil { execResp.Status = providers.ExecComplete execResp.RuntimeData[presentKey] = dataValueTrue + // Hand the two rows this node just read to the paired Session node, which needs the same ones + // to restore the checkpoint. ForwardedData reaches the immediate next node only, so this is + // set on the Skip outcome alone: on the Authenticate outcome the flow prompts and suspends, + // and forwarded data that outlived the node would be persisted with the flow context. + execResp.ForwardedData = map[string]interface{}{ + common.ForwardedDataKeySSOSession: resolved, + common.ForwardedDataKeySSOSessionContext: snapshot, + } logger.Debug(ctx.Context, "Live SSO checkpoint present; routing to the Skip outcome", log.String("flowId", in.FlowID), log.String("checkpoint", checkpoint)) diff --git a/backend/internal/flow/executor/sso_check_executor_test.go b/backend/internal/flow/executor/sso_check_executor_test.go index 36b1747518..a124226aa3 100644 --- a/backend/internal/flow/executor/sso_check_executor_test.go +++ b/backend/internal/flow/executor/sso_check_executor_test.go @@ -87,6 +87,9 @@ func (suite *SSOCheckExecutorTestSuite) assertAbsent(resp *providers.ExecutorRes suite.Equal("false", resp.RuntimeData[common.SSOCheckpointKey(common.RuntimeKeySSOSessionPresent, "session")]) suite.Empty(resp.RuntimeData[common.RuntimeKeySSOSessionHandle]) + // The Authenticate outcome prompts and suspends the flow, and forwarded data that outlived the node + // would be persisted with the flow context, so nothing is forwarded on this path. + suite.Empty(resp.ForwardedData) } // TestPresent covers a live session that already holds this checkpoint: routes to Skip and shares the @@ -94,7 +97,8 @@ func (suite *SSOCheckExecutorTestSuite) assertAbsent(resp *providers.ExecutorRes func (suite *SSOCheckExecutorTestSuite) TestPresent() { sso := sessionmock.NewServiceMock(suite.T()) sso.EXPECT().Resolve(mock.Anything, "handle-abc", "flow-1", 3, mock.Anything).Return(liveSession(), nil) - sso.EXPECT().HasCheckpoint(mock.Anything, "sess-1", "session").Return(true, nil) + sso.EXPECT().FindCheckpoint(mock.Anything, "sess-1", "session"). + Return(&session.SessionContext{SessionID: "sess-1", CheckpointID: "session"}, nil) exec := suite.newExecutor(sso) resp, err := exec.Execute(ssoNodeContext()) @@ -104,6 +108,14 @@ func (suite *SSOCheckExecutorTestSuite) TestPresent() { suite.Equal(dataValueTrue, resp.RuntimeData[common.SSOCheckpointKey(common.RuntimeKeySSOSessionPresent, "session")]) suite.Equal("handle-abc", resp.RuntimeData[common.RuntimeKeySSOSessionHandle]) + // Both rows this node read are forwarded to the paired Session node so the load path does not read + // them again. Losing this makes the reuse correct but two queries more expensive. + forwardedSession, ok := resp.ForwardedData[common.ForwardedDataKeySSOSession].(*session.Session) + suite.Require().True(ok, "the resolved session must be forwarded to the Session node") + suite.Equal("sess-1", forwardedSession.SessionID) + forwardedContext, ok := resp.ForwardedData[common.ForwardedDataKeySSOSessionContext].(*session.SessionContext) + suite.Require().True(ok, "the checkpoint context must be forwarded to the Session node") + suite.Equal("session", forwardedContext.CheckpointID) } // TestAbsentCheckpointNotPresent covers a live session that lacks this checkpoint: the node routes to @@ -113,7 +125,7 @@ func (suite *SSOCheckExecutorTestSuite) TestAbsentCheckpointNotPresent() { sso := sessionmock.NewServiceMock(suite.T()) sso.EXPECT().Resolve(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(liveSession(), nil) - sso.EXPECT().HasCheckpoint(mock.Anything, "sess-1", "session").Return(false, nil) + sso.EXPECT().FindCheckpoint(mock.Anything, "sess-1", "session").Return(nil, nil) exec := suite.newExecutor(sso) resp, err := exec.Execute(ssoNodeContext()) @@ -170,14 +182,14 @@ func (suite *SSOCheckExecutorTestSuite) TestResolverErrorFailsFlow() { suite.Contains(err.Error(), "store down") } -// TestCheckpointListErrorFailsFlow covers a checkpoint-existence lookup failure on a live session: +// TestCheckpointLookupErrorFailsFlow covers a checkpoint lookup failure on a live session: // Execute returns a Go error rather than skipping on incomplete information. -func (suite *SSOCheckExecutorTestSuite) TestCheckpointListErrorFailsFlow() { +func (suite *SSOCheckExecutorTestSuite) TestCheckpointLookupErrorFailsFlow() { sso := sessionmock.NewServiceMock(suite.T()) sso.EXPECT().Resolve(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(liveSession(), nil) - sso.EXPECT().HasCheckpoint(mock.Anything, mock.Anything, mock.Anything). - Return(false, errors.New("store down")) + sso.EXPECT().FindCheckpoint(mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.New("store down")) exec := suite.newExecutor(sso) _, err := exec.Execute(ssoNodeContext()) diff --git a/backend/internal/flow/session/Service_mock_test.go b/backend/internal/flow/session/Service_mock_test.go index 2cb24b17a2..10101cb30a 100644 --- a/backend/internal/flow/session/Service_mock_test.go +++ b/backend/internal/flow/session/Service_mock_test.go @@ -38,23 +38,25 @@ func (_m *ServiceMock) EXPECT() *ServiceMock_Expecter { return &ServiceMock_Expecter{mock: &_m.Mock} } -// HasCheckpoint provides a mock function for the type ServiceMock -func (_mock *ServiceMock) HasCheckpoint(ctx context.Context, sessionID string, checkpoint string) (bool, error) { +// FindCheckpoint provides a mock function for the type ServiceMock +func (_mock *ServiceMock) FindCheckpoint(ctx context.Context, sessionID string, checkpoint string) (*SessionContext, error) { ret := _mock.Called(ctx, sessionID, checkpoint) if len(ret) == 0 { - panic("no return value specified for HasCheckpoint") + panic("no return value specified for FindCheckpoint") } - var r0 bool + var r0 *SessionContext var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) (bool, error)); ok { + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) (*SessionContext, error)); ok { return returnFunc(ctx, sessionID, checkpoint) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) bool); ok { + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) *SessionContext); ok { r0 = returnFunc(ctx, sessionID, checkpoint) } else { - r0 = ret.Get(0).(bool) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*SessionContext) + } } if returnFunc, ok := ret.Get(1).(func(context.Context, string, string) error); ok { r1 = returnFunc(ctx, sessionID, checkpoint) @@ -64,20 +66,20 @@ func (_mock *ServiceMock) HasCheckpoint(ctx context.Context, sessionID string, c return r0, r1 } -// ServiceMock_HasCheckpoint_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'HasCheckpoint' -type ServiceMock_HasCheckpoint_Call struct { +// ServiceMock_FindCheckpoint_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'FindCheckpoint' +type ServiceMock_FindCheckpoint_Call struct { *mock.Call } -// HasCheckpoint is a helper method to define mock.On call +// FindCheckpoint is a helper method to define mock.On call // - ctx context.Context // - sessionID string // - checkpoint string -func (_e *ServiceMock_Expecter) HasCheckpoint(ctx interface{}, sessionID interface{}, checkpoint interface{}) *ServiceMock_HasCheckpoint_Call { - return &ServiceMock_HasCheckpoint_Call{Call: _e.mock.On("HasCheckpoint", ctx, sessionID, checkpoint)} +func (_e *ServiceMock_Expecter) FindCheckpoint(ctx interface{}, sessionID interface{}, checkpoint interface{}) *ServiceMock_FindCheckpoint_Call { + return &ServiceMock_FindCheckpoint_Call{Call: _e.mock.On("FindCheckpoint", ctx, sessionID, checkpoint)} } -func (_c *ServiceMock_HasCheckpoint_Call) Run(run func(ctx context.Context, sessionID string, checkpoint string)) *ServiceMock_HasCheckpoint_Call { +func (_c *ServiceMock_FindCheckpoint_Call) Run(run func(ctx context.Context, sessionID string, checkpoint string)) *ServiceMock_FindCheckpoint_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -100,19 +102,19 @@ func (_c *ServiceMock_HasCheckpoint_Call) Run(run func(ctx context.Context, sess return _c } -func (_c *ServiceMock_HasCheckpoint_Call) Return(b bool, err error) *ServiceMock_HasCheckpoint_Call { - _c.Call.Return(b, err) +func (_c *ServiceMock_FindCheckpoint_Call) Return(sessionContext *SessionContext, err error) *ServiceMock_FindCheckpoint_Call { + _c.Call.Return(sessionContext, err) return _c } -func (_c *ServiceMock_HasCheckpoint_Call) RunAndReturn(run func(ctx context.Context, sessionID string, checkpoint string) (bool, error)) *ServiceMock_HasCheckpoint_Call { +func (_c *ServiceMock_FindCheckpoint_Call) RunAndReturn(run func(ctx context.Context, sessionID string, checkpoint string) (*SessionContext, error)) *ServiceMock_FindCheckpoint_Call { _c.Call.Return(run) return _c } // LoadCheckpoint provides a mock function for the type ServiceMock -func (_mock *ServiceMock) LoadCheckpoint(ctx context.Context, handle string, checkpoint string, appID string, tokenFamilyID string) (*Session, *SessionContext, error) { - ret := _mock.Called(ctx, handle, checkpoint, appID, tokenFamilyID) +func (_mock *ServiceMock) LoadCheckpoint(ctx context.Context, in LoadCheckpointInput) (*Session, *SessionContext, error) { + ret := _mock.Called(ctx, in) if len(ret) == 0 { panic("no return value specified for LoadCheckpoint") @@ -121,25 +123,25 @@ func (_mock *ServiceMock) LoadCheckpoint(ctx context.Context, handle string, che var r0 *Session var r1 *SessionContext var r2 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string, string, string) (*Session, *SessionContext, error)); ok { - return returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(0).(func(context.Context, LoadCheckpointInput) (*Session, *SessionContext, error)); ok { + return returnFunc(ctx, in) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string, string, string) *Session); ok { - r0 = returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(0).(func(context.Context, LoadCheckpointInput) *Session); ok { + r0 = returnFunc(ctx, in) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*Session) } } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, string, string, string) *SessionContext); ok { - r1 = returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(1).(func(context.Context, LoadCheckpointInput) *SessionContext); ok { + r1 = returnFunc(ctx, in) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*SessionContext) } } - if returnFunc, ok := ret.Get(2).(func(context.Context, string, string, string, string) error); ok { - r2 = returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(2).(func(context.Context, LoadCheckpointInput) error); ok { + r2 = returnFunc(ctx, in) } else { r2 = ret.Error(2) } @@ -153,42 +155,24 @@ type ServiceMock_LoadCheckpoint_Call struct { // LoadCheckpoint is a helper method to define mock.On call // - ctx context.Context -// - handle string -// - checkpoint string -// - appID string -// - tokenFamilyID string -func (_e *ServiceMock_Expecter) LoadCheckpoint(ctx interface{}, handle interface{}, checkpoint interface{}, appID interface{}, tokenFamilyID interface{}) *ServiceMock_LoadCheckpoint_Call { - return &ServiceMock_LoadCheckpoint_Call{Call: _e.mock.On("LoadCheckpoint", ctx, handle, checkpoint, appID, tokenFamilyID)} +// - in LoadCheckpointInput +func (_e *ServiceMock_Expecter) LoadCheckpoint(ctx interface{}, in interface{}) *ServiceMock_LoadCheckpoint_Call { + return &ServiceMock_LoadCheckpoint_Call{Call: _e.mock.On("LoadCheckpoint", ctx, in)} } -func (_c *ServiceMock_LoadCheckpoint_Call) Run(run func(ctx context.Context, handle string, checkpoint string, appID string, tokenFamilyID string)) *ServiceMock_LoadCheckpoint_Call { +func (_c *ServiceMock_LoadCheckpoint_Call) Run(run func(ctx context.Context, in LoadCheckpointInput)) *ServiceMock_LoadCheckpoint_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { arg0 = args[0].(context.Context) } - var arg1 string + var arg1 LoadCheckpointInput if args[1] != nil { - arg1 = args[1].(string) - } - var arg2 string - if args[2] != nil { - arg2 = args[2].(string) - } - var arg3 string - if args[3] != nil { - arg3 = args[3].(string) - } - var arg4 string - if args[4] != nil { - arg4 = args[4].(string) + arg1 = args[1].(LoadCheckpointInput) } run( arg0, arg1, - arg2, - arg3, - arg4, ) }) return _c @@ -199,7 +183,7 @@ func (_c *ServiceMock_LoadCheckpoint_Call) Return(session *Session, sessionConte return _c } -func (_c *ServiceMock_LoadCheckpoint_Call) RunAndReturn(run func(ctx context.Context, handle string, checkpoint string, appID string, tokenFamilyID string) (*Session, *SessionContext, error)) *ServiceMock_LoadCheckpoint_Call { +func (_c *ServiceMock_LoadCheckpoint_Call) RunAndReturn(run func(ctx context.Context, in LoadCheckpointInput) (*Session, *SessionContext, error)) *ServiceMock_LoadCheckpoint_Call { _c.Call.Return(run) return _c } diff --git a/backend/internal/flow/session/interface.go b/backend/internal/flow/session/interface.go index 5706daa842..62b1e2fbe6 100644 --- a/backend/internal/flow/session/interface.go +++ b/backend/internal/flow/session/interface.go @@ -46,9 +46,6 @@ type sessionStore interface { Delete(ctx context.Context, sessionID string) error // DeleteSession removes the session row itself. DeleteSession(ctx context.Context, sessionID string) error - // ListCheckpointIDs returns the checkpoint ids a session has saved, without loading any context - // payload — the existence check the SSO-Check node uses to decide checkpoint availability. - ListCheckpointIDs(ctx context.Context, sessionID string) ([]string, error) // Record inserts the participant, or refreshes its LAST_ACTIVE_AT (preserving FIRST_JOINED_AT) // when the application has already joined the session. diff --git a/backend/internal/flow/session/service.go b/backend/internal/flow/session/service.go index 7079e9d92d..461e41632e 100644 --- a/backend/internal/flow/session/service.go +++ b/backend/internal/flow/session/service.go @@ -45,9 +45,11 @@ type Service interface { // session, a session from a different flow, or one established at an incompatible flow version. Resolve(ctx context.Context, handle, flowID string, flowVersion int, now time.Time) (*Session, error) - // HasCheckpoint reports whether the resolved session already holds a snapshot for the checkpoint, - // using the decrypt-free checkpoint listing. - HasCheckpoint(ctx context.Context, sessionID, checkpoint string) (bool, error) + // FindCheckpoint returns the resolved session's snapshot for the checkpoint, or (nil, nil) when + // the session holds none. Callers deciding only whether a checkpoint is available compare the + // result against nil; the SSO-Check node forwards it to the paired Session node so the load path + // does not fetch the same row again. + FindCheckpoint(ctx context.Context, sessionID, checkpoint string) (*SessionContext, error) // SaveCheckpoint attaches the checkpoint to this flow execution's session — the one already // resolved (via HandleHint), one an earlier join minted, or a freshly established one — writing @@ -55,23 +57,44 @@ type Service interface { // true when the authenticated subject conflicts with the existing session's subject. SaveCheckpoint(ctx context.Context, in SaveCheckpointInput) (SaveCheckpointResult, error) - // LoadCheckpoint fetches the session referenced by handle and its checkpoint context, refreshes - // the session's last-active timestamp and idle deadline (throttled: skipped when the last refresh - // is within the activity-refresh window), and records the joining participant with the grant's - // token family id (all best-effort). It errors when the session or its checkpoint context no - // longer exists. - LoadCheckpoint(ctx context.Context, handle, checkpoint, appID, tokenFamilyID string) ( - *Session, *SessionContext, error) - - // Terminate ends the session referenced by handle: it marks the session ENDED (so it can no - // longer back SSO) and removes its checkpoint contexts and participants, all in one transaction. - // When flowID is non-empty the handle must belong to that flow, guarding against ending a - // session grouped under a different flow. It is idempotent — a no-op returning (nil, nil) when - // no session matches the handle, and the unchanged session when it is already ended — and - // returns the ended session on success. + // LoadCheckpoint returns the session referenced by in.Handle and the checkpoint's context, refreshes + // the session's last-active timestamp and idle deadline (throttled: skipped when the last refresh is + // within the activity-refresh window), and records the joining application as a participant. It uses + // the rows the SSO-Check node forwarded when they match what is being loaded, and reads whatever it + // was not given. The activity refresh is best-effort, but recording the participant is not when + // in.TokenFamilyID is set: a token family with no persisted mapping would be unrevocable, so that + // failure aborts the load. It errors when a row it has to read no longer exists. + LoadCheckpoint(ctx context.Context, in LoadCheckpointInput) (*Session, *SessionContext, error) + + // Terminate ends the session referenced by handle: it revokes the token families of every + // participating application (when a revoker is wired) and hard-deletes the session along with its + // checkpoint contexts and participants, all in one transaction, so nothing is left that could back + // SSO or hold live grants. When flowID is non-empty the handle must belong to that flow, guarding + // against ending a session grouped under a different flow. It is idempotent, returning (nil, nil) + // when no session matches the handle, and returns the deleted session on success. Terminate(ctx context.Context, handle, flowID string) (*Session, error) } +// LoadCheckpointInput carries what a Session join needs to restore a checkpoint. Session and Context +// are the rows the SSO-Check node already read for this checkpoint, handed over so the load path does +// not repeat those two queries; either may be nil, in which case it is read from the store. +type LoadCheckpointInput struct { + // Handle is the resolved session handle. It identifies the session to load when Session is nil, + // and guards the handed-over Session against belonging to a different handle. + Handle string + // Checkpoint is the checkpoint id whose snapshot is being restored. + Checkpoint string + // AppID is the joining application, recorded as a session participant. + AppID string + // TokenFamilyID is the token family id (tfid) minted for this grant. Recording it is required + // when non-empty: a tfid with no persisted mapping would be unrevocable. + TokenFamilyID string + // Session is the session the SSO-Check node resolved, or nil to read it by Handle. + Session *Session + // Context is the checkpoint context the SSO-Check node fetched, or nil to read it. + Context *SessionContext +} + // SaveCheckpointInput carries the data a Session join needs to persist. The caller resolves the // subject and builds the (already sanitized) snapshot; the service only stores it. type SaveCheckpointInput struct { @@ -143,18 +166,15 @@ func (s *service) Resolve(ctx context.Context, handle, flowID string, flowVersio return sess, nil } -// HasCheckpoint implements Service. -func (s *service) HasCheckpoint(ctx context.Context, sessionID, checkpoint string) (bool, error) { - ids, err := s.store.ListCheckpointIDs(ctx, sessionID) +// FindCheckpoint implements Service. Fetching the checkpoint context by its full primary key answers +// the availability question by itself, so this replaces listing every checkpoint id and matching in +// Go, and it returns the row for the caller to hand to LoadCheckpoint. +func (s *service) FindCheckpoint(ctx context.Context, sessionID, checkpoint string) (*SessionContext, error) { + snapshot, err := s.store.GetByCheckpoint(ctx, sessionID, checkpoint) if err != nil { - return false, fmt.Errorf("failed to list SSO session checkpoints: %w", err) + return nil, fmt.Errorf("failed to read SSO session checkpoint: %w", err) } - for _, id := range ids { - if id == checkpoint { - return true, nil - } - } - return false, nil + return snapshot, nil } // SaveCheckpoint implements Service. @@ -192,26 +212,47 @@ func (s *service) SaveCheckpoint(ctx context.Context, in SaveCheckpointInput) (S } // LoadCheckpoint implements Service. -func (s *service) LoadCheckpoint(ctx context.Context, handle, checkpoint, appID, tokenFamilyID string) ( +func (s *service) LoadCheckpoint(ctx context.Context, in LoadCheckpointInput) ( *Session, *SessionContext, error) { - if handle == "" { + if in.Handle == "" { return nil, nil, fmt.Errorf("no resolved session handle to load") } - sess, err := s.store.GetByHandle(ctx, handle) - if err != nil { - return nil, nil, err + // The SSO-Check node resolved the session and read this checkpoint's context to decide routing, and + // forwards both here, so this path re-reads neither. The resolver has already applied every liveness, + // flow-identity and flow-version check. What the handover gives up is noticing that a concurrent + // sign-out deleted the session in between: the idle slide below then matches no row and logs, and the + // reuse still completes. That is accepted, since the check node had already committed to skipping. + // ForwardedData reaches the immediate next node only, so anything not handed over is read here and a + // load still works when the handover did not survive to this node. + sess := in.Session + if sess != nil && sess.HandleID != in.Handle { + sess = nil // forwarded from a different handle; do not trust it } if sess == nil { - return nil, nil, fmt.Errorf("resolved session no longer exists") + loaded, err := s.store.GetByHandle(ctx, in.Handle) + if err != nil { + return nil, nil, err + } + if loaded == nil { + return nil, nil, fmt.Errorf("resolved session no longer exists") + } + sess = loaded } - // Lazily load this checkpoint's durable session context (only the load path reads it). - sc, err := s.store.GetByCheckpoint(ctx, sess.SessionID, checkpoint) - if err != nil { - return nil, nil, err + // This checkpoint's durable context, read here only when the check node's row did not reach us. + snapshot := in.Context + if snapshot != nil && (snapshot.SessionID != sess.SessionID || snapshot.CheckpointID != in.Checkpoint) { + snapshot = nil // forwarded for a different session or checkpoint; do not trust it } - if sc == nil { - return nil, nil, fmt.Errorf("session context for checkpoint %q no longer exists", checkpoint) + if snapshot == nil { + loaded, err := s.store.GetByCheckpoint(ctx, sess.SessionID, in.Checkpoint) + if err != nil { + return nil, nil, err + } + if loaded == nil { + return nil, nil, fmt.Errorf("session context for checkpoint %q no longer exists", in.Checkpoint) + } + snapshot = loaded } // Refresh last-active and slide the idle deadline under the optimistic-lock guard — touches @@ -239,14 +280,14 @@ func (s *service) LoadCheckpoint(ctx context.Context, handle, checkpoint, appID, // load before publishing the tfid, forcing full re-authentication). Without a tfid there is nothing // to revoke, so the write stays best-effort. Either way it is not throttled with the activity // refresh above: the upsert also registers an application joining the session for the first time. - if partErr := s.recordParticipant(ctx, sess.SessionID, appID, tokenFamilyID, now); partErr != nil { - if tokenFamilyID != "" { + if partErr := s.recordParticipant(ctx, sess.SessionID, in.AppID, in.TokenFamilyID, now); partErr != nil { + if in.TokenFamilyID != "" { return nil, nil, fmt.Errorf("failed to record SSO session participant for token family: %w", partErr) } s.logger.Warn(ctx, "Failed to record SSO session participant", log.Error(partErr)) } - return sess, sc, nil + return sess, snapshot, nil } // Terminate implements Service. diff --git a/backend/internal/flow/session/service_test.go b/backend/internal/flow/session/service_test.go index 6dbcd80013..6fe72e69f4 100644 --- a/backend/internal/flow/session/service_test.go +++ b/backend/internal/flow/session/service_test.go @@ -33,6 +33,10 @@ import ( "github.com/thunder-id/thunderid/tests/mocks/transactionmock" ) +// testOtherFlowID is a flow other than the one under test, used to assert that a session grouped +// under a different flow is never reused. +const testOtherFlowID = "other-flow" + type ServiceTestSuite struct { suite.Suite } @@ -110,7 +114,7 @@ func (suite *ServiceTestSuite) TestResolve_NoHandle() { func (suite *ServiceTestSuite) TestResolve_DifferentFlow() { svc, m := suite.newService() s := liveStoreSession() - s.FlowID = "other-flow" + s.FlowID = testOtherFlowID m.store.EXPECT().GetByHandle(mock.Anything, mock.Anything).Return(s, nil) got, err := svc.Resolve(context.Background(), "handle-abc", "flow-1", 3, time.Now().UTC()) @@ -141,34 +145,38 @@ func (suite *ServiceTestSuite) TestResolve_StoreError() { suite.Contains(err.Error(), "failed to resolve SSO session") } -// --- HasCheckpoint --- +// --- FindCheckpoint --- -func (suite *ServiceTestSuite) TestHasCheckpoint_Present() { +// TestFindCheckpoint_Present also pins that the row itself is returned, not just its existence: the +// SSO-Check node forwards it to the Session node so the load path does not fetch it again. +func (suite *ServiceTestSuite) TestFindCheckpoint_Present() { svc, m := suite.newService() - m.store.EXPECT().ListCheckpointIDs(mock.Anything, "sess-1").Return([]string{"password", "session"}, nil) + sc := &SessionContext{SessionID: "sess-1", CheckpointID: "session"} + m.store.EXPECT().GetByCheckpoint(mock.Anything, "sess-1", "session").Return(sc, nil) - present, err := svc.HasCheckpoint(context.Background(), "sess-1", "session") + got, err := svc.FindCheckpoint(context.Background(), "sess-1", "session") suite.Require().NoError(err) - suite.True(present) + suite.Same(sc, got) } -func (suite *ServiceTestSuite) TestHasCheckpoint_Absent() { +func (suite *ServiceTestSuite) TestFindCheckpoint_Absent() { svc, m := suite.newService() - m.store.EXPECT().ListCheckpointIDs(mock.Anything, "sess-1").Return([]string{"password"}, nil) + m.store.EXPECT().GetByCheckpoint(mock.Anything, "sess-1", "session").Return(nil, nil) - present, err := svc.HasCheckpoint(context.Background(), "sess-1", "session") + got, err := svc.FindCheckpoint(context.Background(), "sess-1", "session") suite.Require().NoError(err) - suite.False(present) + suite.Nil(got) } -func (suite *ServiceTestSuite) TestHasCheckpoint_ListError() { +func (suite *ServiceTestSuite) TestFindCheckpoint_ReadError() { svc, m := suite.newService() - m.store.EXPECT().ListCheckpointIDs(mock.Anything, mock.Anything).Return(nil, errors.New("store down")) + m.store.EXPECT().GetByCheckpoint(mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.New("store down")) - _, err := svc.HasCheckpoint(context.Background(), "sess-1", "session") + _, err := svc.FindCheckpoint(context.Background(), "sess-1", "session") suite.Require().Error(err) - suite.Contains(err.Error(), "failed to list SSO session checkpoints") + suite.Contains(err.Error(), "failed to read SSO session checkpoint") } // --- SaveCheckpoint --- @@ -272,6 +280,21 @@ func (suite *ServiceTestSuite) TestSaveCheckpoint_ContextWriteError() { // --- LoadCheckpoint --- +// loadInput is the baseline load input: a handle to resolve and nothing forwarded, so the service +// reads both rows itself. Tests override the fields they are exercising. +func loadInput() LoadCheckpointInput { + return LoadCheckpointInput{ + Handle: "handle-abc", Checkpoint: "session", AppID: "app-456", TokenFamilyID: "tfid-1", + } +} + +// noTFIDLoadInput is a load with no token family minted, where the participant write is best-effort. +func noTFIDLoadInput() LoadCheckpointInput { + in := loadInput() + in.TokenFamilyID = "" + return in +} + func (suite *ServiceTestSuite) TestLoadCheckpoint_Success() { originalIdle := time.Unix(1700000600, 0).UTC() originalAbsolute := time.Unix(1700050000, 0).UTC() @@ -289,7 +312,7 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_Success() { m.store.EXPECT().Record(mock.Anything, mock.Anything).RunAndReturn( func(_ context.Context, p Participant) error { recorded = p; return nil }) - sess, sc, err := svc.LoadCheckpoint(context.Background(), "handle-abc", "session", "app-456", "tfid-1") + sess, sc, err := svc.LoadCheckpoint(context.Background(), loadInput()) suite.Require().NoError(err) suite.Require().NotNil(sess) @@ -303,10 +326,73 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_Success() { suite.Equal("app-456", recorded.AppID) } +// TestLoadCheckpoint_UsesForwardedReads is the guard for the reuse-path read reduction: given the rows +// the SSO-Check node forwarded, the load path must issue neither read. No GetByHandle or +// GetByCheckpoint expectation is registered, so the mock fails the test if either query comes back. +func (suite *ServiceTestSuite) TestLoadCheckpoint_UsesForwardedReads() { + svc, m := suite.newService() + m.store.EXPECT().Update(mock.Anything, mock.Anything).Return(nil) + m.store.EXPECT().Record(mock.Anything, mock.Anything).Return(nil) + + in := loadInput() + in.Session = liveStoreSession() + in.Context = &SessionContext{SessionID: "sess-1", CheckpointID: "session"} + + sess, sc, err := svc.LoadCheckpoint(context.Background(), in) + + suite.Require().NoError(err) + suite.Same(in.Session, sess) + suite.Same(in.Context, sc) +} + +// TestLoadCheckpoint_ReadsWhenForwardedSessionIsAnotherHandle rejects a forwarded session that does +// not belong to the handle being loaded, rather than trusting whatever arrived. +func (suite *ServiceTestSuite) TestLoadCheckpoint_ReadsWhenForwardedSessionIsAnotherHandle() { + svc, m := suite.newService() + m.store.EXPECT().GetByHandle(mock.Anything, "handle-xyz").Return(&Session{ + SessionID: "sess-2", HandleID: "handle-xyz", State: StateActive, + }, nil) + m.store.EXPECT().GetByCheckpoint(mock.Anything, "sess-2", "session"). + Return(&SessionContext{SessionID: "sess-2", CheckpointID: "session"}, nil) + m.store.EXPECT().Update(mock.Anything, mock.Anything).Return(nil) + m.store.EXPECT().Record(mock.Anything, mock.Anything).Return(nil) + + in := loadInput() + in.Handle = "handle-xyz" + in.Session = liveStoreSession() // handle-abc, not the handle being loaded + in.Context = &SessionContext{SessionID: "sess-1", CheckpointID: "session"} + + sess, sc, err := svc.LoadCheckpoint(context.Background(), in) + + suite.Require().NoError(err) + suite.Equal("sess-2", sess.SessionID) + suite.Equal("sess-2", sc.SessionID) +} + +// TestLoadCheckpoint_ReadsWhenForwardedContextIsAnotherCheckpoint rejects a forwarded context that +// describes a different checkpoint, so a flow holding several checkpoints cannot cross-load them. +func (suite *ServiceTestSuite) TestLoadCheckpoint_ReadsWhenForwardedContextIsAnotherCheckpoint() { + svc, m := suite.newService() + m.store.EXPECT().GetByCheckpoint(mock.Anything, "sess-1", "step_up"). + Return(&SessionContext{SessionID: "sess-1", CheckpointID: "step_up"}, nil) + m.store.EXPECT().Update(mock.Anything, mock.Anything).Return(nil) + m.store.EXPECT().Record(mock.Anything, mock.Anything).Return(nil) + + in := loadInput() + in.Checkpoint = "step_up" + in.Session = liveStoreSession() + in.Context = &SessionContext{SessionID: "sess-1", CheckpointID: "session"} + + _, sc, err := svc.LoadCheckpoint(context.Background(), in) + + suite.Require().NoError(err) + suite.Equal("step_up", sc.CheckpointID) +} + func (suite *ServiceTestSuite) TestLoadCheckpoint_NoHandle() { svc, _ := suite.newService() - _, _, err := svc.LoadCheckpoint(context.Background(), "", "session", "app-456", "tfid-1") + _, _, err := svc.LoadCheckpoint(context.Background(), LoadCheckpointInput{Checkpoint: "session", AppID: "app-456"}) suite.Require().Error(err) suite.Contains(err.Error(), "no resolved session handle") @@ -316,7 +402,7 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_MissingSession() { svc, m := suite.newService() m.store.EXPECT().GetByHandle(mock.Anything, mock.Anything).Return(nil, nil) - _, _, err := svc.LoadCheckpoint(context.Background(), "handle-abc", "session", "app-456", "tfid-1") + _, _, err := svc.LoadCheckpoint(context.Background(), loadInput()) suite.Require().Error(err) suite.Contains(err.Error(), "resolved session no longer exists") @@ -328,7 +414,7 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_MissingContext() { Return(&Session{SessionID: "sess-1", HandleID: "handle-abc", State: StateActive}, nil) m.store.EXPECT().GetByCheckpoint(mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) - _, _, err := svc.LoadCheckpoint(context.Background(), "handle-abc", "session", "app-456", "tfid-1") + _, _, err := svc.LoadCheckpoint(context.Background(), loadInput()) suite.Require().Error(err) suite.Contains(err.Error(), "session context for checkpoint") @@ -343,7 +429,7 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_ParticipantErrorWithTokenFamil m.store.EXPECT().Update(mock.Anything, mock.Anything).Return(nil) m.store.EXPECT().Record(mock.Anything, mock.Anything).Return(errors.New("db down")) - sess, sc, err := svc.LoadCheckpoint(context.Background(), "handle-abc", "session", "app-456", "tfid-1") + sess, sc, err := svc.LoadCheckpoint(context.Background(), loadInput()) suite.Require().Error(err, "issuing a token family whose mapping cannot persist must fail the load") suite.Contains(err.Error(), "token family") @@ -360,7 +446,7 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_ParticipantErrorWithoutTokenFa m.store.EXPECT().Update(mock.Anything, mock.Anything).Return(nil) m.store.EXPECT().Record(mock.Anything, mock.Anything).Return(errors.New("db down")) - sess, sc, err := svc.LoadCheckpoint(context.Background(), "handle-abc", "session", "app-456", "") + sess, sc, err := svc.LoadCheckpoint(context.Background(), noTFIDLoadInput()) suite.Require().NoError(err, "with no token family there is nothing to revoke, so the load survives") suite.NotNil(sess) @@ -380,7 +466,7 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_ThrottledRefreshSkipsUpdate() Return(&SessionContext{SessionID: "sess-1"}, nil) m.store.EXPECT().Record(mock.Anything, mock.Anything).Return(nil) - sess, sc, err := svc.LoadCheckpoint(context.Background(), "handle-abc", "session", "app-456", "") + sess, sc, err := svc.LoadCheckpoint(context.Background(), noTFIDLoadInput()) suite.Require().NoError(err) suite.Require().NotNil(sess) @@ -404,7 +490,7 @@ func (suite *ServiceTestSuite) TestLoadCheckpoint_WritesAfterRefreshWindow() { func(_ context.Context, s *Session) error { updated = s; return nil }) m.store.EXPECT().Record(mock.Anything, mock.Anything).Return(nil) - _, _, err := svc.LoadCheckpoint(context.Background(), "handle-abc", "session", "app-456", "") + _, _, err := svc.LoadCheckpoint(context.Background(), noTFIDLoadInput()) suite.Require().NoError(err) suite.Require().NotNil(updated, "an activity refresh past the throttle window must persist") @@ -486,7 +572,7 @@ func (suite *ServiceTestSuite) TestTerminate_MissingSessionIsNoOp() { func (suite *ServiceTestSuite) TestTerminate_DifferentFlowErrors() { svc, m := suite.newService() s := liveStoreSession() - s.FlowID = "other-flow" + s.FlowID = testOtherFlowID m.store.EXPECT().GetByHandle(mock.Anything, "handle-abc").Return(s, nil) got, err := svc.Terminate(context.Background(), "handle-abc", "flow-1") diff --git a/backend/internal/flow/session/sessionStore_mock_test.go b/backend/internal/flow/session/sessionStore_mock_test.go index 9ab5cff40a..7d1d57ba91 100644 --- a/backend/internal/flow/session/sessionStore_mock_test.go +++ b/backend/internal/flow/session/sessionStore_mock_test.go @@ -600,74 +600,6 @@ func (_c *sessionStoreMock_ListBySessionID_Call) RunAndReturn(run func(ctx conte return _c } -// ListCheckpointIDs provides a mock function for the type sessionStoreMock -func (_mock *sessionStoreMock) ListCheckpointIDs(ctx context.Context, sessionID string) ([]string, error) { - ret := _mock.Called(ctx, sessionID) - - if len(ret) == 0 { - panic("no return value specified for ListCheckpointIDs") - } - - var r0 []string - var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string) ([]string, error)); ok { - return returnFunc(ctx, sessionID) - } - if returnFunc, ok := ret.Get(0).(func(context.Context, string) []string); ok { - r0 = returnFunc(ctx, sessionID) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]string) - } - } - if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = returnFunc(ctx, sessionID) - } else { - r1 = ret.Error(1) - } - return r0, r1 -} - -// sessionStoreMock_ListCheckpointIDs_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListCheckpointIDs' -type sessionStoreMock_ListCheckpointIDs_Call struct { - *mock.Call -} - -// ListCheckpointIDs is a helper method to define mock.On call -// - ctx context.Context -// - sessionID string -func (_e *sessionStoreMock_Expecter) ListCheckpointIDs(ctx interface{}, sessionID interface{}) *sessionStoreMock_ListCheckpointIDs_Call { - return &sessionStoreMock_ListCheckpointIDs_Call{Call: _e.mock.On("ListCheckpointIDs", ctx, sessionID)} -} - -func (_c *sessionStoreMock_ListCheckpointIDs_Call) Run(run func(ctx context.Context, sessionID string)) *sessionStoreMock_ListCheckpointIDs_Call { - _c.Call.Run(func(args mock.Arguments) { - var arg0 context.Context - if args[0] != nil { - arg0 = args[0].(context.Context) - } - var arg1 string - if args[1] != nil { - arg1 = args[1].(string) - } - run( - arg0, - arg1, - ) - }) - return _c -} - -func (_c *sessionStoreMock_ListCheckpointIDs_Call) Return(strings []string, err error) *sessionStoreMock_ListCheckpointIDs_Call { - _c.Call.Return(strings, err) - return _c -} - -func (_c *sessionStoreMock_ListCheckpointIDs_Call) RunAndReturn(run func(ctx context.Context, sessionID string) ([]string, error)) *sessionStoreMock_ListCheckpointIDs_Call { - _c.Call.Return(run) - return _c -} - // Record provides a mock function for the type sessionStoreMock func (_mock *sessionStoreMock) Record(ctx context.Context, p Participant) error { ret := _mock.Called(ctx, p) diff --git a/backend/internal/flow/session/session_context_store_test.go b/backend/internal/flow/session/session_context_store_test.go index 2c34d27600..97967b9ff6 100644 --- a/backend/internal/flow/session/session_context_store_test.go +++ b/backend/internal/flow/session/session_context_store_test.go @@ -141,21 +141,6 @@ func (s *SessionContextStoreTestSuite) TestGetByCheckpoint_Miss() { s.Nil(got) } -func (s *SessionContextStoreTestSuite) TestListCheckpointIDs() { - s.mockDBProvider.On("GetRuntimePersistentDBClient").Return(s.mockDBClient, nil) - s.mockDBClient.On("QueryContext", context.Background(), queryListCheckpointsBySessionID, - "sess-1", testDeploymentID). - Return([]map[string]interface{}{ - {"checkpoint_id": "password"}, - {"checkpoint_id": "step_up"}, - }, nil) - - ids, listErr := s.store.ListCheckpointIDs(context.Background(), "sess-1") - - s.NoError(listErr) - s.Equal([]string{"password", "step_up"}, ids) -} - func (s *SessionContextStoreTestSuite) TestDelete() { s.mockDBProvider.On("GetRuntimePersistentDBClient").Return(s.mockDBClient, nil) s.mockDBClient.On("ExecuteContext", context.Background(), queryDeleteSessionContext, @@ -238,28 +223,6 @@ func (s *SessionContextStoreTestSuite) TestGetByCheckpoint_BadPayload() { s.Nil(got) } -func (s *SessionContextStoreTestSuite) TestListCheckpointIDs_QueryError() { - s.mockDBProvider.On("GetRuntimePersistentDBClient").Return(s.mockDBClient, nil) - s.mockDBClient.On("QueryContext", context.Background(), queryListCheckpointsBySessionID, - "sess-1", testDeploymentID). - Return(nil, errors.New("query failed")) - - got, err := s.store.ListCheckpointIDs(context.Background(), "sess-1") - s.Error(err) - s.Nil(got) -} - -func (s *SessionContextStoreTestSuite) TestListCheckpointIDs_ParseError() { - s.mockDBProvider.On("GetRuntimePersistentDBClient").Return(s.mockDBClient, nil) - s.mockDBClient.On("QueryContext", context.Background(), queryListCheckpointsBySessionID, - "sess-1", testDeploymentID). - Return([]map[string]interface{}{{"checkpoint_id": 42}}, nil) // non-string fails parseString - - got, err := s.store.ListCheckpointIDs(context.Background(), "sess-1") - s.Error(err) - s.Nil(got) -} - func (s *SessionContextStoreTestSuite) TestDelete_DBError() { s.mockDBProvider.On("GetRuntimePersistentDBClient").Return(s.mockDBClient, nil) s.mockDBClient.On("ExecuteContext", context.Background(), queryDeleteSessionContext, diff --git a/backend/internal/flow/session/store.go b/backend/internal/flow/session/store.go index 93ed0a4a02..a8f7254edd 100644 --- a/backend/internal/flow/session/store.go +++ b/backend/internal/flow/session/store.go @@ -173,30 +173,6 @@ func (st *store) GetByCheckpoint(ctx context.Context, sessionID, return result, nil } -// ListCheckpointIDs returns the checkpoint ids a session has saved, without decrypting any payload. -func (st *store) ListCheckpointIDs(ctx context.Context, sessionID string) ([]string, error) { - var ids []string - - err := withRuntimePersistentDBClient(st.dbProvider, func(dbClient provider.DBClientInterface) error { - results, queryErr := dbClient.QueryContext(ctx, queryListCheckpointsBySessionID, sessionID, st.deploymentID) - if queryErr != nil { - return fmt.Errorf("failed to execute query: %w", queryErr) - } - for _, row := range results { - id, parseErr := parseString(row["checkpoint_id"], "checkpoint_id") - if parseErr != nil { - return parseErr - } - ids = append(ids, id) - } - return nil - }) - if err != nil { - return nil, err - } - return ids, nil -} - // Delete removes a session's session context. func (st *store) Delete(ctx context.Context, sessionID string) error { return withRuntimePersistentDBClient(st.dbProvider, func(dbClient provider.DBClientInterface) error { diff --git a/backend/internal/flow/session/store_constants.go b/backend/internal/flow/session/store_constants.go index 7433e65fb1..07bea03442 100644 --- a/backend/internal/flow/session/store_constants.go +++ b/backend/internal/flow/session/store_constants.go @@ -90,14 +90,6 @@ var ( Query: `DELETE FROM "SSO_SESSION_CONTEXT" WHERE SESSION_ID = $1 AND DEPLOYMENT_ID = $2`, } - // queryListCheckpointsBySessionID returns the checkpoint ids a session has saved. It is the - // existence check the SSO-Check node uses to decide availability without decrypting any context. - queryListCheckpointsBySessionID = model.DBQuery{ - ID: "SSO-SESS-08", - Query: `SELECT CHECKPOINT_ID FROM "SSO_SESSION_CONTEXT" ` + - `WHERE SESSION_ID = $1 AND DEPLOYMENT_ID = $2`, - } - // queryUpsertParticipant records an application as a participant of a session, refreshing // LAST_ACTIVE_AT and the current-grant TFID (but preserving FIRST_JOINED_AT) when the application // has already joined. TFID moves to the latest grant so logout revokes the most recent family. diff --git a/backend/tests/mocks/flow/sessionmock/Service_mock.go b/backend/tests/mocks/flow/sessionmock/Service_mock.go index 62338f95ce..5546ebddb0 100644 --- a/backend/tests/mocks/flow/sessionmock/Service_mock.go +++ b/backend/tests/mocks/flow/sessionmock/Service_mock.go @@ -39,23 +39,25 @@ func (_m *ServiceMock) EXPECT() *ServiceMock_Expecter { return &ServiceMock_Expecter{mock: &_m.Mock} } -// HasCheckpoint provides a mock function for the type ServiceMock -func (_mock *ServiceMock) HasCheckpoint(ctx context.Context, sessionID string, checkpoint string) (bool, error) { +// FindCheckpoint provides a mock function for the type ServiceMock +func (_mock *ServiceMock) FindCheckpoint(ctx context.Context, sessionID string, checkpoint string) (*session.SessionContext, error) { ret := _mock.Called(ctx, sessionID, checkpoint) if len(ret) == 0 { - panic("no return value specified for HasCheckpoint") + panic("no return value specified for FindCheckpoint") } - var r0 bool + var r0 *session.SessionContext var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) (bool, error)); ok { + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) (*session.SessionContext, error)); ok { return returnFunc(ctx, sessionID, checkpoint) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) bool); ok { + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) *session.SessionContext); ok { r0 = returnFunc(ctx, sessionID, checkpoint) } else { - r0 = ret.Get(0).(bool) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*session.SessionContext) + } } if returnFunc, ok := ret.Get(1).(func(context.Context, string, string) error); ok { r1 = returnFunc(ctx, sessionID, checkpoint) @@ -65,20 +67,20 @@ func (_mock *ServiceMock) HasCheckpoint(ctx context.Context, sessionID string, c return r0, r1 } -// ServiceMock_HasCheckpoint_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'HasCheckpoint' -type ServiceMock_HasCheckpoint_Call struct { +// ServiceMock_FindCheckpoint_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'FindCheckpoint' +type ServiceMock_FindCheckpoint_Call struct { *mock.Call } -// HasCheckpoint is a helper method to define mock.On call +// FindCheckpoint is a helper method to define mock.On call // - ctx context.Context // - sessionID string // - checkpoint string -func (_e *ServiceMock_Expecter) HasCheckpoint(ctx interface{}, sessionID interface{}, checkpoint interface{}) *ServiceMock_HasCheckpoint_Call { - return &ServiceMock_HasCheckpoint_Call{Call: _e.mock.On("HasCheckpoint", ctx, sessionID, checkpoint)} +func (_e *ServiceMock_Expecter) FindCheckpoint(ctx interface{}, sessionID interface{}, checkpoint interface{}) *ServiceMock_FindCheckpoint_Call { + return &ServiceMock_FindCheckpoint_Call{Call: _e.mock.On("FindCheckpoint", ctx, sessionID, checkpoint)} } -func (_c *ServiceMock_HasCheckpoint_Call) Run(run func(ctx context.Context, sessionID string, checkpoint string)) *ServiceMock_HasCheckpoint_Call { +func (_c *ServiceMock_FindCheckpoint_Call) Run(run func(ctx context.Context, sessionID string, checkpoint string)) *ServiceMock_FindCheckpoint_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -101,19 +103,19 @@ func (_c *ServiceMock_HasCheckpoint_Call) Run(run func(ctx context.Context, sess return _c } -func (_c *ServiceMock_HasCheckpoint_Call) Return(b bool, err error) *ServiceMock_HasCheckpoint_Call { - _c.Call.Return(b, err) +func (_c *ServiceMock_FindCheckpoint_Call) Return(sessionContext *session.SessionContext, err error) *ServiceMock_FindCheckpoint_Call { + _c.Call.Return(sessionContext, err) return _c } -func (_c *ServiceMock_HasCheckpoint_Call) RunAndReturn(run func(ctx context.Context, sessionID string, checkpoint string) (bool, error)) *ServiceMock_HasCheckpoint_Call { +func (_c *ServiceMock_FindCheckpoint_Call) RunAndReturn(run func(ctx context.Context, sessionID string, checkpoint string) (*session.SessionContext, error)) *ServiceMock_FindCheckpoint_Call { _c.Call.Return(run) return _c } // LoadCheckpoint provides a mock function for the type ServiceMock -func (_mock *ServiceMock) LoadCheckpoint(ctx context.Context, handle string, checkpoint string, appID string, tokenFamilyID string) (*session.Session, *session.SessionContext, error) { - ret := _mock.Called(ctx, handle, checkpoint, appID, tokenFamilyID) +func (_mock *ServiceMock) LoadCheckpoint(ctx context.Context, in session.LoadCheckpointInput) (*session.Session, *session.SessionContext, error) { + ret := _mock.Called(ctx, in) if len(ret) == 0 { panic("no return value specified for LoadCheckpoint") @@ -122,25 +124,25 @@ func (_mock *ServiceMock) LoadCheckpoint(ctx context.Context, handle string, che var r0 *session.Session var r1 *session.SessionContext var r2 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string, string, string) (*session.Session, *session.SessionContext, error)); ok { - return returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(0).(func(context.Context, session.LoadCheckpointInput) (*session.Session, *session.SessionContext, error)); ok { + return returnFunc(ctx, in) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string, string, string) *session.Session); ok { - r0 = returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(0).(func(context.Context, session.LoadCheckpointInput) *session.Session); ok { + r0 = returnFunc(ctx, in) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*session.Session) } } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, string, string, string) *session.SessionContext); ok { - r1 = returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(1).(func(context.Context, session.LoadCheckpointInput) *session.SessionContext); ok { + r1 = returnFunc(ctx, in) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*session.SessionContext) } } - if returnFunc, ok := ret.Get(2).(func(context.Context, string, string, string, string) error); ok { - r2 = returnFunc(ctx, handle, checkpoint, appID, tokenFamilyID) + if returnFunc, ok := ret.Get(2).(func(context.Context, session.LoadCheckpointInput) error); ok { + r2 = returnFunc(ctx, in) } else { r2 = ret.Error(2) } @@ -154,42 +156,24 @@ type ServiceMock_LoadCheckpoint_Call struct { // LoadCheckpoint is a helper method to define mock.On call // - ctx context.Context -// - handle string -// - checkpoint string -// - appID string -// - tokenFamilyID string -func (_e *ServiceMock_Expecter) LoadCheckpoint(ctx interface{}, handle interface{}, checkpoint interface{}, appID interface{}, tokenFamilyID interface{}) *ServiceMock_LoadCheckpoint_Call { - return &ServiceMock_LoadCheckpoint_Call{Call: _e.mock.On("LoadCheckpoint", ctx, handle, checkpoint, appID, tokenFamilyID)} +// - in session.LoadCheckpointInput +func (_e *ServiceMock_Expecter) LoadCheckpoint(ctx interface{}, in interface{}) *ServiceMock_LoadCheckpoint_Call { + return &ServiceMock_LoadCheckpoint_Call{Call: _e.mock.On("LoadCheckpoint", ctx, in)} } -func (_c *ServiceMock_LoadCheckpoint_Call) Run(run func(ctx context.Context, handle string, checkpoint string, appID string, tokenFamilyID string)) *ServiceMock_LoadCheckpoint_Call { +func (_c *ServiceMock_LoadCheckpoint_Call) Run(run func(ctx context.Context, in session.LoadCheckpointInput)) *ServiceMock_LoadCheckpoint_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { arg0 = args[0].(context.Context) } - var arg1 string + var arg1 session.LoadCheckpointInput if args[1] != nil { - arg1 = args[1].(string) - } - var arg2 string - if args[2] != nil { - arg2 = args[2].(string) - } - var arg3 string - if args[3] != nil { - arg3 = args[3].(string) - } - var arg4 string - if args[4] != nil { - arg4 = args[4].(string) + arg1 = args[1].(session.LoadCheckpointInput) } run( arg0, arg1, - arg2, - arg3, - arg4, ) }) return _c @@ -200,7 +184,7 @@ func (_c *ServiceMock_LoadCheckpoint_Call) Return(session1 *session.Session, ses return _c } -func (_c *ServiceMock_LoadCheckpoint_Call) RunAndReturn(run func(ctx context.Context, handle string, checkpoint string, appID string, tokenFamilyID string) (*session.Session, *session.SessionContext, error)) *ServiceMock_LoadCheckpoint_Call { +func (_c *ServiceMock_LoadCheckpoint_Call) RunAndReturn(run func(ctx context.Context, in session.LoadCheckpointInput) (*session.Session, *session.SessionContext, error)) *ServiceMock_LoadCheckpoint_Call { _c.Call.Return(run) return _c }