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 }