From 5d6aa3b72940104e199a08495965b83409199fc5 Mon Sep 17 00:00:00 2001 From: Irena Rindos Date: Tue, 15 Feb 2022 08:55:25 -0500 Subject: [PATCH] refactor(session): Remove dead session termination code --- .../vault/repository_credentials_test.go | 8 -- internal/session/query.go | 34 -------- internal/session/repository_session.go | 51 ----------- internal/session/repository_session_test.go | 84 ------------------- 4 files changed, 177 deletions(-) diff --git a/internal/credential/vault/repository_credentials_test.go b/internal/credential/vault/repository_credentials_test.go index 978663109..5dc45294d 100644 --- a/internal/credential/vault/repository_credentials_test.go +++ b/internal/credential/vault/repository_credentials_test.go @@ -593,14 +593,6 @@ group by session_id, status; assert.Equal(1, count) assertCreds(cc) - // call TerminateSession - id = sessionIds[1] - _, err = sessionRepo.TerminateSession(ctx, id, 1, session.ClosedByUser) - assert.NoError(err) - cc[id].revoke = cc[id].active - cc[id].active = 0 - assertCreds(cc) - count, err = sessionRepo.TerminateCompletedSessions(ctx) assert.NoError(err) assert.Zero(count) diff --git a/internal/session/query.go b/internal/session/query.go index 0da27a89a..0cbf1fe23 100644 --- a/internal/session/query.go +++ b/internal/session/query.go @@ -56,40 +56,6 @@ where state = 'terminated' ) ) -` - - terminateSessionCte = ` -insert into session_state -with terminated as ( - select s.public_id, 'terminated' as state - from - session s - where - s.version = @version and - s.public_id in ( - -- sessions without any connections - select s.public_id - from - session s - left join session_connection sc on sc.session_id = s.public_id - where - sc.session_id is null - and s.public_id = @session_id - union - -- sessions where all connections are closed - select s.public_id - from - session s, - session_connection c, - session_connection_state cs - where - s.public_id = c.session_id and - c.public_id = cs.connection_id and - cs.state = 'closed' and - s.public_id = @session_id - ) -) -select * from terminated; ` authorizeConnectionCte = ` insert into session_connection ( diff --git a/internal/session/repository_session.go b/internal/session/repository_session.go index a2ca92e43..69b9d3202 100644 --- a/internal/session/repository_session.go +++ b/internal/session/repository_session.go @@ -349,57 +349,6 @@ func (r *Repository) CancelSession(ctx context.Context, sessionId string, sessio return s, nil } -// TerminateSession sets a session's termination reason and it's state to -// "terminated" Sessions cannot be terminated which still have connections that -// are not closed. -func (r *Repository) TerminateSession(ctx context.Context, sessionId string, sessionVersion uint32, reason TerminationReason) (*Session, error) { - const op = "session.(Repository).TerminateSession" - if sessionId == "" { - return nil, errors.New(ctx, errors.InvalidParameter, op, "missing session id") - } - if sessionVersion == 0 { - return nil, errors.New(ctx, errors.InvalidParameter, op, "missing session version") - } - - updatedSession := AllocSession() - updatedSession.PublicId = sessionId - updatedSession.TerminationReason = reason.String() - _, err := r.writer.DoTx( - ctx, - db.StdRetryCnt, - db.ExpBackoff{}, - func(reader db.Reader, w db.Writer) error { - rowsAffected, err := w.Exec(ctx, terminateSessionCte, []interface{}{ - sql.Named("version", sessionVersion), - sql.Named("session_id", sessionId), - }) - if err != nil { - return errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("unable to terminate session %s", sessionId))) - } - if rowsAffected == 0 { - return errors.New(ctx, errors.InvalidSessionState, op, fmt.Sprintf("unable to terminate session %s", sessionId)) - } - rowsUpdated, err := w.Update(ctx, &updatedSession, []string{"TerminationReason"}, nil, db.WithVersion(&sessionVersion)) - if err != nil { - return errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("failed for %s", sessionId))) - } - if rowsUpdated != 1 { - return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("update to session %s would have updated %d session", updatedSession.PublicId, rowsUpdated)) - } - states, err := fetchStates(ctx, reader, sessionId, db.WithOrder("start_time desc")) - if err != nil { - return errors.Wrap(ctx, err, op) - } - updatedSession.States = states - return nil - }, - ) - if err != nil { - return nil, errors.Wrap(ctx, err, op) - } - return &updatedSession, nil -} - // TerminateCompletedSessions will terminate sessions in the repo based on: // * sessions that have exhausted their connection limit and all their connections are closed. // * sessions that are expired and all their connections are closed. diff --git a/internal/session/repository_session_test.go b/internal/session/repository_session_test.go index b042df6ac..b04475cab 100644 --- a/internal/session/repository_session_test.go +++ b/internal/session/repository_session_test.go @@ -758,90 +758,6 @@ func TestRepository_ConnectConnection(t *testing.T) { } } -func TestRepository_TerminateSession(t *testing.T) { - t.Parallel() - conn, _ := db.TestSetup(t, "postgres") - rw := db.New(conn) - wrapper := db.TestWrapper(t) - iamRepo := iam.TestRepo(t, conn, wrapper) - kms := kms.TestKms(t, conn, wrapper) - repo, err := NewRepository(rw, rw, kms) - require.NoError(t, err) - - setupFn := func() *Session { - s := TestDefaultSession(t, conn, wrapper, iamRepo) - srv := TestWorker(t, conn, wrapper) - tofu := TestTofu(t) - s, _, err := repo.ActivateSession(context.Background(), s.PublicId, s.Version, srv.PrivateId, srv.Type, tofu) - require.NoError(t, err) - return s - } - tests := []struct { - name string - session *Session - reason TerminationReason - wantErr bool - wantIsError errors.Code - }{ - { - name: "valid-active-session", - session: setupFn(), - reason: ClosedByUser, - }, - { - name: "valid-pending-session", - session: TestDefaultSession(t, conn, wrapper, iamRepo), - reason: ClosedByUser, - }, - { - name: "empty-session-id", - session: func() *Session { - s := setupFn() - s.PublicId = "" - return s - }(), - reason: ClosedByUser, - wantErr: true, - wantIsError: errors.InvalidParameter, - }, - { - name: "empty-session-version", - session: func() *Session { - s := setupFn() - s.Version = 0 - return s - }(), - reason: ClosedByUser, - wantErr: true, - wantIsError: errors.InvalidParameter, - }, - { - name: "open-connection", - session: func() *Session { - s := setupFn() - _ = TestConnection(t, conn, s.PublicId, "127.0.0.1", 22, "127.0.0.1", 222, "127.0.0.1") - return s - }(), - reason: ClosedByUser, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert, require := assert.New(t), require.New(t) - s, err := repo.TerminateSession(context.Background(), tt.session.PublicId, tt.session.Version, tt.reason) - if tt.wantErr { - require.Error(err) - assert.Truef(errors.Match(errors.T(tt.wantIsError), err), "unexpected error %s", err.Error()) - return - } - require.NoError(err) - assert.Equal(tt.reason.String(), s.TerminationReason) - assert.Equal(StatusTerminated, s.States[0].Status) - }) - } -} - func TestRepository_TerminateCompletedSessions(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres")