perf(session): Change list behavior around terminated sessions

It was determined that it is a more common use case for users to request
non-terminated sessions (i.e. sessions in `pending`, `active`, or
`canceling` when requesting sessions. Notably, end users would need
non-terminated sessions to:

- Establish a new connection to a session.
- Terminate a session

So now by default a list request to `/v1/sessions` will *not* include
terminated sessions.

To still enable use cases where terminated sessions are needed, this
adds a new query parameter to include the terminated session:
`/v1/sessions?include_terminated=true`.

Note that this query parameter is considered experimental and subject to
change.

internal/daemon/controller/handlers/sessions/session_service.go

@@ -152,7 +152,7 @@ func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest)
 			RootScopeId:                  req.GetScopeId(),
 			Type:                         resource.Session,
 			Recursive:                    req.GetRecursive(),
-			AuthzProtectedEntityProvider: repo,
+			AuthzProtectedEntityProvider: session.ListForAuthzCheck(repo, session.WithTerminated(req.IncludeTerminated)),
 			ActionSet:                    IdActions,
 		},
 	)

internal/daemon/controller/handlers/sessions/session_service_test.go

@@ -232,6 +232,7 @@ func TestList(t *testing.T) {
 	conn, _ := db.TestSetup(t, "postgres")
 	wrap := db.TestWrapper(t)
 	kms := kms.TestKms(t, conn, wrap)
+	ctx := context.Background()
 
 	iamRepo := iam.TestRepo(t, conn, wrap)
 
@@ -260,16 +261,17 @@ func TestList(t *testing.T) {
 	hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0]
 	h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0]
 	static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h})
-	tar := tcp.TestTarget(context.Background(), t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()}))
+	tar := tcp.TestTarget(ctx, t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()}))
 
 	hcOther := static.TestCatalogs(t, conn, pWithOtherSessions.GetPublicId(), 1)[0]
 	hsOther := static.TestSets(t, conn, hcOther.GetPublicId(), 1)[0]
 	hOther := static.TestHosts(t, conn, hcOther.GetPublicId(), 1)[0]
 	static.TestSetMembers(t, conn, hsOther.GetPublicId(), []*static.Host{hOther})
-	tarOther := tcp.TestTarget(context.Background(), t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()}))
+	tarOther := tcp.TestTarget(ctx, t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()}))
 
 	var wantSession []*pb.Session
 	var totalSession []*pb.Session
+	var wantIncludeTerminatedSessions []*pb.Session
 	for i := 0; i < 10; i++ {
 		sess := session.TestSession(t, conn, wrap, session.ComposedOf{
 			UserId:      uId,
@@ -308,6 +310,7 @@ func TestList(t *testing.T) {
 		})
 
 		totalSession = append(totalSession, wantSession[i])
+		wantIncludeTerminatedSessions = append(wantIncludeTerminatedSessions, wantSession[i])
 
 		sess = session.TestSession(t, conn, wrap, session.ComposedOf{
 			UserId:      uIdOther,
@@ -346,6 +349,53 @@ func TestList(t *testing.T) {
 		})
 	}
 
+	{
+		sess := session.TestSession(t, conn, wrap, session.ComposedOf{
+			UserId:      uId,
+			HostId:      h.GetPublicId(),
+			TargetId:    tar.GetPublicId(),
+			HostSetId:   hs.GetPublicId(),
+			AuthTokenId: at.GetPublicId(),
+			ScopeId:     pWithSessions.GetPublicId(),
+			Endpoint:    "tcp://127.0.0.1:22",
+		})
+
+		sess, err := sessRepo.CancelSession(ctx, sess.PublicId, sess.Version)
+		require.NoError(t, err)
+		terminated, err := sessRepo.TerminateCompletedSessions(ctx)
+		require.NoError(t, err)
+		require.Equal(t, 1, terminated)
+
+		sess, _, err = sessRepo.LookupSession(ctx, sess.PublicId)
+		require.NoError(t, err)
+		status, states := convertStates(sess.States)
+
+		expected := &pb.Session{
+			Id:                sess.GetPublicId(),
+			ScopeId:           pWithSessions.GetPublicId(),
+			AuthTokenId:       at.GetPublicId(),
+			UserId:            at.GetIamUserId(),
+			TargetId:          sess.TargetId,
+			Endpoint:          sess.Endpoint,
+			HostSetId:         sess.HostSetId,
+			HostId:            sess.HostId,
+			Version:           sess.Version,
+			UpdatedTime:       sess.UpdateTime.GetTimestamp(),
+			CreatedTime:       sess.CreateTime.GetTimestamp(),
+			ExpirationTime:    sess.ExpirationTime.GetTimestamp(),
+			Scope:             &scopes.ScopeInfo{Id: pWithSessions.GetPublicId(), Type: scope.Project.String(), ParentScopeId: o.GetPublicId()},
+			Status:            status,
+			States:            states,
+			Certificate:       sess.Certificate,
+			TerminationReason: sess.TerminationReason,
+			Type:              tcp.Subtype.String(),
+			AuthorizedActions: testAuthorizedActions,
+			Connections:       []*pb.Connection{}, // connections should not be returned for list
+		}
+
+		wantIncludeTerminatedSessions = append(wantIncludeTerminatedSessions, expected)
+	}
+
 	cases := []struct {
 		name string
 		req  *pbs.ListSessionsRequest
@@ -357,6 +407,11 @@ func TestList(t *testing.T) {
 			req:  &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId()},
 			res:  &pbs.ListSessionsResponse{Items: wantSession},
 		},
+		{
+			name: "List Many Include Terminated",
+			req:  &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId(), IncludeTerminated: true},
+			res:  &pbs.ListSessionsResponse{Items: wantIncludeTerminatedSessions},
+		},
 		{
 			name: "List No Sessions",
 			req:  &pbs.ListSessionsRequest{ScopeId: pNoSessions.GetPublicId()},

internal/session/options.go

@@ -32,6 +32,7 @@ type options struct {
 	withServerId          string
 	withDbOpts            []db.Option
 	withWorkerStateDelay  time.Duration
+	withTerminated        bool
 }
 
 func getDefaultOptions() options {
@@ -121,3 +122,10 @@ func WithWorkerStateDelay(d time.Duration) Option {
 		o.withWorkerStateDelay = d
 	}
 }
+
+// WithTerminated is used to include terminated sessions in a list request.
+func WithTerminated(withTerminated bool) Option {
+	return func(o *options) {
+		o.withTerminated = withTerminated
+	}
+}

internal/session/query.go

@@ -117,11 +117,20 @@ session_connection_limit(expiration_time, connection_limit) as (
 select expiration_time, connection_limit, current_connection_count
 from
 	session_connection_limit, session_connection_count;
+`
+	nonTerminatedSessionPublicIdList = `
+select public_id, scope_id, user_id from session
+where
+	session.termination_reason is null
+and
+	scope_id = any(@scope_ids)
+;
 `
 
 	sessionPublicIdList = `
 select public_id, scope_id, user_id from session
-%s
+where
+	scope_id = any(@scope_ids)
 ;
 `
 

internal/session/repository_session.go

@@ -207,32 +207,31 @@ func (r *Repository) LookupSession(ctx context.Context, sessionId string, _ ...O
 	return &session, authzSummary, nil
 }
 
-// FetchAuthzProtectedEntitiesByScope implements boundary.AuthzProtectedEntityProvider
-func (r *Repository) FetchAuthzProtectedEntitiesByScope(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) {
-	const op = "session.(Repository).FetchAuthzProtectedEntityInfo"
+// fetchAuthzProtectedSessionsByScope fetches sessions for the given scopes.
+// Note that the sessions are not fully populated, and only contain the
+// necessary information to implement the boundary.AuthzProtectedEntity
+// interface. Supports the WithTerminated option.
+func (r *Repository) fetchAuthzProtectedSessionsByScope(
+	ctx context.Context, scopeIds []string, opt ...Option,
+) (map[string][]boundary.AuthzProtectedEntity, error) {
+	const op = "session.(Repository).fetchAuthzProtectedSessionsByScope"
 
-	var where string
-	var args []interface{}
+	opts := getOpts(opt...)
 
-	inClauseCnt := 0
-
-	switch len(scopeIds) {
-	case 0:
+	if len(scopeIds) == 0 {
 		return nil, errors.New(ctx, errors.InvalidParameter, op, "no scopes given")
-	case 1:
-		inClauseCnt += 1
-		where, args = fmt.Sprintf("where scope_id = @%d", inClauseCnt), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), scopeIds[0]))
-	default:
-		idsInClause := make([]string, 0, len(scopeIds))
-		for _, id := range scopeIds {
-			inClauseCnt += 1
-			idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id))
-		}
-		where = fmt.Sprintf("where scope_id in (%s)", strings.Join(idsInClause, ","))
 	}
 
-	q := sessionPublicIdList
-	query := fmt.Sprintf(q, where)
+	args := []interface{}{
+		sql.Named("scope_ids", "{"+strings.Join(scopeIds, ",")+"}"),
+	}
+
+	var query string
+	if opts.withTerminated {
+		query = sessionPublicIdList
+	} else {
+		query = nonTerminatedSessionPublicIdList
+	}
 
 	rows, err := r.reader.Query(ctx, query, args)
 	if err != nil {

internal/session/repository_session_test.go

@@ -1592,3 +1592,232 @@ func TestRepository_deleteTerminated(t *testing.T) {
 		})
 	}
 }
+
+func TestFetchAuthzProtectedSessionsByScopes(t *testing.T) {
+	conn, _ := db.TestSetup(t, "postgres")
+	ctx := context.Background()
+	wrapper := db.TestWrapper(t)
+	iamRepo := iam.TestRepo(t, conn, wrapper)
+	rw := db.New(conn)
+	kms := kms.TestKms(t, conn, wrapper)
+	repo, err := NewRepository(rw, rw, kms)
+	require.NoError(t, err)
+	composedOf := TestSessionParams(t, conn, wrapper, iamRepo)
+
+	_, pWithOtherSessions := iam.TestScopes(t, iamRepo)
+
+	hcOther := static.TestCatalogs(t, conn, pWithOtherSessions.GetPublicId(), 1)[0]
+	hsOther := static.TestSets(t, conn, hcOther.GetPublicId(), 1)[0]
+	hOther := static.TestHosts(t, conn, hcOther.GetPublicId(), 1)[0]
+	static.TestSetMembers(t, conn, hsOther.GetPublicId(), []*static.Host{hOther})
+	tarOther := tcp.TestTarget(ctx, t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()}))
+
+	composedOfOther := ComposedOf{
+		UserId:      composedOf.UserId,
+		HostId:      hOther.GetPublicId(),
+		TargetId:    tarOther.GetPublicId(),
+		HostSetId:   hsOther.GetPublicId(),
+		AuthTokenId: composedOf.AuthTokenId,
+		ScopeId:     pWithOtherSessions.GetPublicId(),
+		Endpoint:    "tcp://127.0.0.1:22",
+	}
+
+	type testCase struct {
+		name              string
+		createCnt         int
+		terminateCnt      int
+		otherCnt          int
+		otherTerminateCnt int
+		opts              []Option
+		reqScopes         []string
+		wantCnt           int
+		wantOtherCnt      int
+		wantErr           bool
+	}
+
+	cases := []testCase{
+		{
+			name:      "NonTerminated/none",
+			createCnt: 0,
+			reqScopes: []string{composedOf.ScopeId},
+			wantCnt:   0,
+			wantErr:   false,
+		},
+		{
+			name:      "NonTerminated/one",
+			createCnt: 1,
+			reqScopes: []string{composedOf.ScopeId},
+			wantCnt:   1,
+			wantErr:   false,
+		},
+		{
+			name:      "NonTerminated/many",
+			createCnt: 5,
+			reqScopes: []string{composedOf.ScopeId},
+			wantCnt:   5,
+			wantErr:   false,
+		},
+		{
+			name:         "NonTerminated/many one terminated",
+			createCnt:    5,
+			terminateCnt: 1,
+			reqScopes:    []string{composedOf.ScopeId},
+			wantCnt:      4,
+			wantErr:      false,
+		},
+		{
+			name:         "NonTerminated/many terminated",
+			createCnt:    5,
+			terminateCnt: 3,
+			reqScopes:    []string{composedOf.ScopeId},
+			wantCnt:      2,
+			wantErr:      false,
+		},
+		{
+			name:         "NonTerminated/many multiple scopes",
+			createCnt:    5,
+			terminateCnt: 3,
+			reqScopes:    []string{composedOf.ScopeId, composedOfOther.ScopeId},
+			wantCnt:      2,
+			wantErr:      false,
+		},
+		{
+			name:              "NonTerminated/many multiple scopes",
+			createCnt:         5,
+			terminateCnt:      3,
+			otherCnt:          3,
+			otherTerminateCnt: 1,
+			reqScopes:         []string{composedOf.ScopeId, composedOfOther.ScopeId},
+			wantCnt:           2,
+			wantOtherCnt:      2,
+			wantErr:           false,
+		},
+		{
+			name:              "NonTerminated/no scopes",
+			createCnt:         2,
+			terminateCnt:      1,
+			otherCnt:          2,
+			otherTerminateCnt: 1,
+			reqScopes:         []string{},
+			wantErr:           true,
+		},
+		{
+			name:      "none",
+			opts:      []Option{WithTerminated(true)},
+			createCnt: 0,
+			reqScopes: []string{composedOf.ScopeId},
+			wantCnt:   0,
+			wantErr:   false,
+		},
+		{
+			name:      "one",
+			opts:      []Option{WithTerminated(true)},
+			createCnt: 1,
+			reqScopes: []string{composedOf.ScopeId},
+			wantCnt:   1,
+			wantErr:   false,
+		},
+		{
+			name:      "many",
+			opts:      []Option{WithTerminated(true)},
+			createCnt: 5,
+			reqScopes: []string{composedOf.ScopeId},
+			wantCnt:   5,
+			wantErr:   false,
+		},
+		{
+			name:         "many one terminated",
+			opts:         []Option{WithTerminated(true)},
+			createCnt:    5,
+			terminateCnt: 1,
+			reqScopes:    []string{composedOf.ScopeId},
+			wantCnt:      5,
+			wantErr:      false,
+		},
+		{
+			name:         "many terminated",
+			opts:         []Option{WithTerminated(true)},
+			createCnt:    5,
+			terminateCnt: 3,
+			reqScopes:    []string{composedOf.ScopeId},
+			wantCnt:      5,
+			wantErr:      false,
+		},
+		{
+			name:         "many multiple scopes",
+			opts:         []Option{WithTerminated(true)},
+			createCnt:    5,
+			terminateCnt: 3,
+			reqScopes:    []string{composedOf.ScopeId, composedOfOther.ScopeId},
+			wantCnt:      5,
+			wantErr:      false,
+		},
+		{
+			name:              "many multiple scopes",
+			opts:              []Option{WithTerminated(true)},
+			createCnt:         5,
+			terminateCnt:      3,
+			otherCnt:          3,
+			otherTerminateCnt: 1,
+			reqScopes:         []string{composedOf.ScopeId, composedOfOther.ScopeId},
+			wantCnt:           5,
+			wantOtherCnt:      3,
+			wantErr:           false,
+		},
+		{
+			name:              "no scopes",
+			opts:              []Option{WithTerminated(true)},
+			createCnt:         2,
+			terminateCnt:      1,
+			otherCnt:          2,
+			otherTerminateCnt: 1,
+			reqScopes:         []string{},
+			wantErr:           true,
+		},
+	}
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			assert, require := assert.New(t), require.New(t)
+			db.TestDeleteWhere(t, conn, func() interface{} { i := AllocSession(); return &i }(), "1=1")
+
+			testSessions := []*Session{}
+			for i := 0; i < tt.createCnt; i++ {
+				s := TestSession(t, conn, wrapper, composedOf)
+				_ = TestState(t, conn, s.PublicId, StatusActive)
+				testSessions = append(testSessions, s)
+			}
+			for i := 0; i < tt.terminateCnt; i++ {
+				_, err := repo.CancelSession(ctx, testSessions[i].PublicId, testSessions[i].Version)
+				require.NoError(err)
+			}
+			terminated, err := repo.TerminateCompletedSessions(ctx)
+			require.NoError(err)
+			require.Equal(tt.terminateCnt, terminated)
+
+			otherTestSessions := []*Session{}
+			for i := 0; i < tt.otherCnt; i++ {
+				s := TestSession(t, conn, wrapper, composedOfOther)
+				_ = TestState(t, conn, s.PublicId, StatusActive)
+				otherTestSessions = append(otherTestSessions, s)
+			}
+			for i := 0; i < tt.otherTerminateCnt; i++ {
+				_, err := repo.CancelSession(ctx, otherTestSessions[i].PublicId, otherTestSessions[i].Version)
+				require.NoError(err)
+			}
+			terminated, err = repo.TerminateCompletedSessions(ctx)
+			require.NoError(err)
+			require.Equal(tt.otherTerminateCnt, terminated)
+
+			assert.Equal(tt.otherCnt, len(otherTestSessions))
+
+			got, err := repo.fetchAuthzProtectedSessionsByScope(ctx, tt.reqScopes, tt.opts...)
+			if tt.wantErr {
+				require.Error(err)
+				return
+			}
+			require.NoError(err)
+			assert.Equal(tt.wantCnt, len(got[composedOf.ScopeId]))
+			assert.Equal(tt.wantOtherCnt, len(got[composedOfOther.ScopeId]))
+		})
+	}
+}

internal/session/service_list_for_authz_check.go

@@ -0,0 +1,23 @@
+package session
+
+import (
+	"context"
+
+	"github.com/hashicorp/boundary/internal/boundary"
+)
+
+type listForAuthzCheckFunc func(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error)
+
+func (a listForAuthzCheckFunc) FetchAuthzProtectedEntitiesByScope(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) {
+	return a(ctx, scopeIds)
+}
+
+// ListForAuthzCheck returns a functions that fetches sessions for the given
+// scopes. Note that the sessions are not fully populated, and only contain the
+// necessary information to implement the boundary.AuthzProtectedEntity
+// interface. Supports the WithTerminated option.
+func ListForAuthzCheck(repo *Repository, opt ...Option) listForAuthzCheckFunc {
+	return func(ctx context.Context, scopeIds []string) (map[string][]boundary.AuthzProtectedEntity, error) {
+		return repo.fetchAuthzProtectedSessionsByScope(ctx, scopeIds, opt...)
+	}
+}