CyberCoveLLC/threshold
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bug(session): Allow only valid state transitions

Browse Source
This commit is contained in:
Irena Rindos committed 2022-01-12 16:01:49 -05:00
1 parent f8b2e99e11
commit edae7c899f
15 files changed
+441 -11
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

CHANGELOG.md
+3
View File
@@ -31,6 +31,9 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
* plugins/aws: AWS plugin based hosts now include DNS names in addition to the
IP addresses they already provide.
### Bug Fixes
* session: Fix duplicate sessions and invalid session state transitions. ([PR](https://github.com/hashicorp/boundary/pull/1793))
## 0.7.3 (2021/12/16)
### Bug Fixes
internal/db/schema/migrations/oss/postgres/24/01_session_valid_state.up.sql
+92
View File
@@ -0,0 +1,92 @@
begin;
-- Remove invalid (states with no prior end time) rows
delete from session_state where previous_end_time is null and state != 'pending';
-- In the case of dupes that are otherwise valid, identify first valid session state and remove others
with cte_session_state_delete
as (select session_id, min(start_time) as st_t, state from session_state ou
where (select count(*) from session_state inr
where inr.session_id = ou.session_id and inr.state = ou.state) > 1
group by session_id, state)
delete from session_state t1
using cte_session_state_delete t2 where t1.session_id =t2.session_id and t1.start_time > t2.st_t and t1.state=t2.state;
-- Close all open session_connections
update session_connection
set closed_reason='canceled' where closed_reason is null;
update session
set termination_reason='canceled' where termination_reason is null;
-- End of data cleanup; start of table modifications
-- session_valid_state table creation and related constraints
create table session_valid_state(
prior_state text
references session_state_enm(name)
on delete restrict
on update cascade,
constraint prior_state_session_state_enm_fkey
check (
prior_state in ('pending', 'active', 'canceling')
),
current_state text
references session_state_enm(name)
on delete restrict
on update cascade,
constraint current_state_session_state_enm_fkey
check (
current_state in ('pending', 'active', 'canceling', 'terminated')
),
primary key (prior_state, current_state)
);
comment on table session_valid_state is
'session_valid_state entries define valid prior_state and current_state pairs to define valid state transitions';
insert into session_valid_state (prior_state, current_state)
values
('pending','pending'),
('pending','active'),
('pending','terminated'),
('pending','canceling'),
('active','canceling'),
('active','terminated'),
('canceling','terminated');
alter table session_state
add column prior_state text not null default 'pending'
references session_state_enm(name)
on delete restrict
on update cascade;
alter table session_state
add constraint session_valid_state_enm_fkey
foreign key (prior_state, state)
references session_valid_state (prior_state,current_state);
alter table session_state
add unique (session_id, state);
create function
update_prior_session_state()
returns trigger
as $$
begin
-- Prior state is the most recent valid prior state entry for this session_id
new.prior_state = query.state from(
select state from session_state where session_id=new.session_id and state in(
select prior_state from session_valid_state where current_state=new.state )
order by start_time desc limit 1) as query;
if new.prior_state is null then
new.prior_state='pending';
end if;
return new;
end;
$$ language plpgsql;
create trigger update_session_state before insert on session_state
for each row execute procedure update_prior_session_state();
commit;
internal/db/schema/migrations/oss/postgres_24_01_test.go
+165
View File
@@ -0,0 +1,165 @@
package oss_test
import (
"context"
"testing"
"github.com/hashicorp/boundary/internal/authtoken"
"github.com/hashicorp/boundary/internal/db"
"github.com/hashicorp/boundary/internal/db/common"
"github.com/hashicorp/boundary/internal/db/schema"
"github.com/hashicorp/boundary/internal/host/static"
"github.com/hashicorp/boundary/internal/iam"
"github.com/hashicorp/boundary/internal/kms"
"github.com/hashicorp/boundary/internal/session"
"github.com/hashicorp/boundary/internal/target"
"github.com/hashicorp/boundary/internal/target/tcp"
"github.com/hashicorp/boundary/testing/dbtest"
"github.com/stretchr/testify/require"
)
func TestMigrations_SessionState(t *testing.T) {
t.Parallel()
require := require.New(t)
const (
priorMigration = 21002
currentMigration = 24001
)
dialect := dbtest.Postgres
ctx := context.Background()
c, u, _, err := dbtest.StartUsingTemplate(dialect, dbtest.WithTemplate(dbtest.Template1))
require.NoError(err)
t.Cleanup(func() {
require.NoError(c())
})
d, err := common.SqlOpen(dialect, u)
require.NoError(err)
// migration to the prior migration (before the one we want to test)
m, err := schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": priorMigration}),
))
require.NoError(err)
require.NoError(m.ApplyMigrations(ctx))
state, err := m.CurrentState(ctx)
require.NoError(err)
want := &schema.State{
Initialized: true,
Editions: []schema.EditionState{
{
Name: "oss",
BinarySchemaVersion: priorMigration,
DatabaseSchemaVersion: priorMigration,
DatabaseSchemaState: schema.Equal,
},
},
}
require.Equal(want, state)
// Seed the database with test data
dbType, err := db.StringToDbType(dialect)
require.NoError(err)
conn, err := db.Open(dbType, u)
require.NoError(err)
rw := db.New(conn)
wrapper := db.TestWrapper(t)
org, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper))
require.NotNil(prj)
hc := static.TestCatalogs(t, conn, prj.GetPublicId(), 1)[0]
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(ctx, t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()}))
kmsCache := kms.TestKms(t, conn, wrapper)
{
at := authtoken.TestAuthToken(t, conn, kmsCache, org.GetPublicId())
uId := at.GetIamUserId()
sess := session.TestSession(t, conn, wrapper, session.ComposedOf{
UserId: uId,
HostId: h.GetPublicId(),
TargetId: tar.GetPublicId(),
HostSetId: hs.GetPublicId(),
AuthTokenId: at.GetPublicId(),
ScopeId: prj.GetPublicId(),
Endpoint: "tcp://127.0.0.1:22",
ConnectionLimit: -1,
})
session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22,
"127.0.0.2", 23, "127.0.0.1")
}
// now we're ready for the migration we want to test.
m, err = schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": currentMigration}),
))
require.NoError(err)
require.NoError(m.ApplyMigrations(ctx))
state, err = m.CurrentState(ctx)
require.NoError(err)
want = &schema.State{
Initialized: true,
Editions: []schema.EditionState{
{
Name: "oss",
BinarySchemaVersion: currentMigration,
DatabaseSchemaVersion: currentMigration,
DatabaseSchemaState: schema.Equal,
},
},
}
require.Equal(want, state)
sessionRepo, err := session.NewRepository(rw, rw, kmsCache)
require.NoError(err)
// Ensure session is cancelled
repoSessions, err := sessionRepo.ListSessions(ctx)
require.NoError(err)
var sessionTermReason []string
var repoSessionId string
for i := 0; i < len(repoSessions); i++ {
s := repoSessions[i]
repoSessionId = s.PublicId
sessionTermReason = append(sessionTermReason, s.TerminationReason)
}
require.Equal([]string{"canceled"}, sessionTermReason)
// Ensure connection is also cancelled
connections, err := sessionRepo.ListConnectionsBySessionId(ctx, repoSessionId)
require.NoError(err)
var connTermReason []string
for i := 0; i < len(connections); i++ {
c := connections[i]
connTermReason = append(connTermReason, c.ClosedReason)
}
require.Equal([]string{"canceled"}, connTermReason)
// Validate new table contents
rows, err := d.QueryContext(ctx, "select * from session_valid_state")
require.NoError(err)
var validStates []string
for rows.Next() {
var a, b string
require.NoError(rows.Scan(&a, &b))
validStates = append(validStates, []string{a, b}...)
}
require.Equal([]string{
"pending", "pending", "pending", "active", "pending", "terminated",
"pending", "canceling", "active", "canceling", "active", "terminated", "canceling", "terminated",
}, validStates)
}
internal/db/sqltest/initdb.d/01_colors_persona.sql
+8 -2
View File
@@ -120,7 +120,10 @@ begin;
insert into auth_token
(key_id, auth_account_id, public_id, token)
values
('key', 'apa____clare', 'tok____clare', 'tok____clare'::bytea);
('key', 'apa____clare', 'tok____clare', 'tok____clare'::bytea),
('key', 'apa____cindy', 'tok____cindy', 'tok____cindy'::bytea),
('key', 'apa____ciara', 'tok____ciara', 'tok____ciara'::bytea),
('key', 'apa____carly', 'tok____carly', 'tok____carly'::bytea);
insert into static_host
(catalog_id, public_id, address)
@@ -194,7 +197,10 @@ begin;
insert into session
( scope_id, target_id, host_set_id, host_id, user_id, auth_token_id, certificate, endpoint, public_id)
values
('p____bcolors', 't_________cb', 's___1cb-sths', 'h_____cb__01', 'u______clare', 'tok____clare', 'abc'::bytea, 'ep1', 's1_____clare');
('p____bcolors', 't_________cb', 's___1cb-sths', 'h_____cb__01', 'u______clare', 'tok____clare', 'abc'::bytea, 'ep1', 's1_____clare'),
('p____bcolors', 't_________cb', 's___1cb-sths', 'h_____cb__01', 'u______cindy', 'tok____cindy', 'abc'::bytea, 'ep1', 's1_____cindy'),
('p____bcolors', 't_________cb', 's___1cb-sths', 'h_____cb__01', 'u______cindy', 'tok____cindy', 'abc'::bytea, 'ep1', 's1_____ciara'),
('p____bcolors', 't_________cb', 's___1cb-sths', 'h_____cb__01', 'u______carly', 'tok____carly', 'abc'::bytea, 'ep1', 's1_____carly');
insert into session_connection
(session_id, public_id)
internal/db/sqltest/initdb.d/02_colors_persona.sql
+11
View File
@@ -0,0 +1,11 @@
begin;
-- For use in testing session_state transitions
insert into session_state
(session_id, state)
values
('s1_____cindy','terminated'),
('s1_____ciara','canceling'),
('s1_____carly','active');
commit;
internal/db/sqltest/initdb.d/03_widgets_persona.sql
+12
View File
@@ -358,4 +358,16 @@ begin;
('t_________wb', 'vl______wvl3', 'egress');
end;
$$ language plpgsql;
create function _wtt_load_widgets_sessions()
returns void
as $$
begin
insert into session
( scope_id , target_id , host_set_id , host_id , user_id , auth_token_id , certificate , endpoint , public_id)
values
('p____bwidget' , 't_________wb' , 's___1wb-sths' , 'h_____wb__01' , 'u_____warren' , 'tok___warren' , 'abc'::bytea , 'ep1' , 's1____warren');
end;
$$ language plpgsql;
commit;
internal/db/sqltest/tests/session/invalid_state_transition.sql
+43
View File
@@ -0,0 +1,43 @@
begin;
select plan(12);
-- Ensure session state table is populated
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='pending';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____cindy' and state='terminated';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____ciara' and state='canceling';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____carly' and state='active';
-- Invalid duplicate state
select throws_ok($$ insert into session_state ( session_id, state )
values ('s1_____clare','pending')$$);
-- Invalid state transition from terminated to pending
select throws_ok($$ insert into session_state ( session_id, state)
values ('s1______cindy','pending')$$);
-- Invalid state transition from terminated to active
select throws_ok($$ insert into session_state ( session_id, state)
values ('s1______cindy','active')$$);
-- Invalid state transition from terminated to canceling
select throws_ok($$ insert into session_state ( session_id, state)
values ('s1______cindy','canceling')$$);
-- Invalid state transition from terminated to terminated
select throws_ok($$ insert into session_state ( session_id, state)
values ('s1______cindy','terminated')$$);
-- Invalid state transition from active to pending
select throws_ok($$ insert into session_state ( session_id, state)
values ('s1______carly','pending')$$);
-- Invalid state transition from canceling to pending
select throws_ok($$ insert into session_state ( session_id, state)
values ('s1______ciara','pending')$$);
-- Invalid state transition from canceling to active
select throws_ok($$ insert into session_state ( session_id, state)
values ('s1______ciara','active')$$);
select * from finish();
rollback;
internal/db/sqltest/tests/session/valid_state_transition_canceling.sql
+28
View File
@@ -0,0 +1,28 @@
begin;
select plan(7);
-- Ensure session state table is populated
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='pending';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____carly' and state='active';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____ciara' and state='canceling';
-- Valid state transition from pending to canceling
insert into session_state
( session_id, state)
values
('s1_____clare','canceling');
select is(count(*), 2::bigint) from session_state where session_id = 's1_____clare';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='canceling';
-- Valid state transition from active to canceling
insert into session_state
( session_id, state)
values
('s1_____carly','canceling');
select is(count(*), 3::bigint) from session_state where session_id = 's1_____carly';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____carly' and state='canceling';
select * from finish();
rollback;
internal/db/sqltest/tests/session/valid_state_transition_pending_active.sql
+17
View File
@@ -0,0 +1,17 @@
begin;
select plan(3);
-- Ensure session state table is populated
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='pending';
-- Valid state transition from pending to active
insert into session_state
( session_id, state)
values
('s1_____clare','active');
select is(count(*), 2::bigint) from session_state where session_id = 's1_____clare';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='active';
select * from finish();
rollback;
internal/db/sqltest/tests/session/valid_state_transition_pending_canceling.sql
+17
View File
@@ -0,0 +1,17 @@
begin;
select plan(3);
-- Ensure session state table is populated
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='pending';
-- Valid state transition from pending to canceling
insert into session_state
( session_id, state)
values
('s1_____clare','canceling');
select is(count(*), 2::bigint) from session_state where session_id = 's1_____clare';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='canceling';
select * from finish();
rollback;
internal/db/sqltest/tests/session/valid_state_transition_terminated.sql
+35
View File
@@ -0,0 +1,35 @@
begin;
select plan(9);
-- Ensure session state table is populated
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='pending';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____carly' and state='active';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____ciara' and state='canceling';
-- Valid state transition from pending to terminated
insert into session_state
( session_id, state)
values
('s1_____clare','terminated');
select is(count(*), 2::bigint) from session_state where session_id = 's1_____clare';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='terminated';
-- Valid state transition from active to terminated
insert into session_state
( session_id, state)
values
('s1_____carly','terminated');
select is(count(*), 3::bigint) from session_state where session_id = 's1_____carly';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____carly' and state='terminated';
-- Valid state transition from canceling to terminated
insert into session_state
( session_id, state)
values
('s1_____ciara','terminated');
select is(count(*), 3::bigint) from session_state where session_id = 's1_____ciara';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____ciara' and state='terminated';
select * from finish();
rollback;
internal/db/sqltest/tests/wh/session/update_state.sql
+1 -1
View File
@@ -15,7 +15,7 @@ begin;
termination_reason = 'closed by end-user'
where public_id = 's1_____clare';
select is(count(*), 1::bigint) from wh_session_accumulating_fact;
select is(count(*), 4::bigint) from wh_session_accumulating_fact;
select is(total_bytes_up, 10::wh_bytes_transmitted) from wh_session_accumulating_fact where session_id = 's1_____clare';
select is(total_bytes_down, 5::wh_bytes_transmitted) from wh_session_accumulating_fact where session_id = 's1_____clare';
select is(session_terminated_time, now()::wh_timestamp) from wh_session_accumulating_fact where session_id = 's1_____clare';
internal/session/immutable_fields_test.go
+2 -2
View File
@@ -99,7 +99,7 @@ func TestState_ImmutableFields(t *testing.T) {
_, _ = iam.TestScopes(t, iam.TestRepo(t, conn, wrapper))
session := TestDefaultSession(t, conn, wrapper, iamRepo)
state := TestState(t, conn, session.PublicId, StatusPending)
state := TestState(t, conn, session.PublicId, StatusActive)
var new State
err := rw.LookupWhere(context.Background(), &new, "session_id = ? and state = ?", []interface{}{state.SessionId, state.Status})
@@ -123,7 +123,7 @@ func TestState_ImmutableFields(t *testing.T) {
name: "status",
update: func() *State {
s := new.Clone().(*State)
s.Status = "active"
s.Status = "canceling"
return s
}(),
fieldMask: []string{"Status"},
internal/session/state_test.go
+6 -5
View File
@@ -22,6 +22,7 @@ func TestState_Create(t *testing.T) {
sessionId string
status Status
}
tests := []struct {
name string
args args
@@ -35,11 +36,11 @@ func TestState_Create(t *testing.T) {
name: "valid",
args: args{
sessionId: session.PublicId,
status: StatusPending,
status: StatusActive,
},
want: &State{
SessionId: session.PublicId,
Status: StatusPending,
Status: StatusActive,
},
create: true,
},
@@ -161,7 +162,7 @@ func TestState_Clone(t *testing.T) {
t.Run("valid", func(t *testing.T) {
assert := assert.New(t)
s := TestDefaultSession(t, conn, wrapper, iamRepo)
state := TestState(t, conn, s.PublicId, StatusPending)
state := TestState(t, conn, s.PublicId, StatusActive)
cp := state.Clone()
assert.Equal(cp.(*State), state)
})
@@ -169,8 +170,8 @@ func TestState_Clone(t *testing.T) {
assert := assert.New(t)
s := TestDefaultSession(t, conn, wrapper, iamRepo)
s2 := TestDefaultSession(t, conn, wrapper, iamRepo)
state := TestState(t, conn, s.PublicId, StatusPending)
state2 := TestState(t, conn, s2.PublicId, StatusPending)
state := TestState(t, conn, s.PublicId, StatusActive)
state2 := TestState(t, conn, s2.PublicId, StatusActive)
cp := state.Clone()
assert.NotEqual(cp.(*State), state2)
internal/session/testing_test.go
+1 -1
View File
@@ -29,7 +29,7 @@ func Test_TestState(t *testing.T) {
require.NotNil(s)
assert.NotEmpty(s.PublicId)
state := TestState(t, conn, s.PublicId, StatusPending)
state := TestState(t, conn, s.PublicId, StatusActive)
require.NotNil(state)
}