diff --git a/internal/authtoken/immutable_fields_test.go b/internal/authtoken/immutable_fields_test.go new file mode 100644 index 000000000..9e2f6c099 --- /dev/null +++ b/internal/authtoken/immutable_fields_test.go @@ -0,0 +1,80 @@ +package authtoken + +import ( + "context" + "testing" + + "github.com/hashicorp/watchtower/internal/db" + "github.com/hashicorp/watchtower/internal/db/timestamp" + "github.com/hashicorp/watchtower/internal/iam" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" +) + +func TestAuthToken_ImmutableFields(t *testing.T) { + t.Parallel() + wrapper := db.TestWrapper(t) + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + + org, _ := iam.TestScopes(t, conn) + new := TestAuthToken(t, conn, wrapper, org.PublicId) + + var tests = []struct { + name string + update *AuthToken + fieldMask []string + }{ + { + name: "public_id", + update: func() *AuthToken { + c := new.clone() + c.PublicId = "o_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *AuthToken { + c := new.clone() + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "auth_account_id", + update: func() *AuthToken { + c := new.clone() + c.AuthAccountId = "aa_01234567890" + return c + }(), + fieldMask: []string{"AuthAccountId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.clone() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.clone() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig, after)) + + }) + } +} diff --git a/internal/db/domains_test.go b/internal/db/domains_test.go index d20963015..b697e9439 100644 --- a/internal/db/domains_test.go +++ b/internal/db/domains_test.go @@ -135,94 +135,6 @@ returning id; } } -func TestDomain_immutable_create_time_func(t *testing.T) { - assert := assert.New(t) - - const ( - createTable = ` -create table if not exists test_immutable_create_time ( - id bigint generated always as identity primary key, - name text, - create_time wt_timestamp -); -` - addTriggers = ` -create trigger - test_update_immutable_create_time -before -update on test_immutable_create_time - for each row execute procedure immutable_create_time_func(); -` - query = ` -select create_time from test_immutable_create_time where id = $1 -` - countQuery = ` -select count(*) from test_immutable_create_time -` - insert = ` -insert into test_immutable_create_time (name) -values ('alice') -returning id; -` - // no where clause intentionally, since it's not needed for the test - update = ` -update test_immutable_create_time -set name = 'bob'; -` - - // no where clause intentionally, since it's not needed for the test - bad_update = ` -update test_immutable_create_time -set create_time = now(); -` - // no where clause intentionally, since it's not needed for the test - bad_update_null = ` -update test_immutable_create_time -set create_time = null; -` - ) - - conn, _ := TestSetup(t, "postgres") - db := conn.DB() - _, err := db.Exec(createTable) - assert.NoError(err) - - _, err = db.Exec(addTriggers) - assert.NoError(err) - - var id int - err = db.QueryRow(insert).Scan(&id) - assert.NoError(err) - - var origTime time.Time - err = db.QueryRow(query, id).Scan(&origTime) - assert.NoError(err) - - var count int - err = db.QueryRow(countQuery).Scan(&count) - assert.NoError(err) - assert.Equal(1, count) - - _, err = db.Exec(update) - assert.NoError(err) - - _, err = db.Exec(bad_update) - assert.NoError(err) - - var updateTime time.Time - err = db.QueryRow(query, id).Scan(&updateTime) - assert.NoError(err) - assert.Equal(origTime, updateTime) - - _, err = db.Exec(bad_update_null) - assert.NoError(err) - - err = db.QueryRow(query, id).Scan(&updateTime) - assert.NoError(err) - assert.Equal(origTime, updateTime) - -} - func TestDomain_update_time_column(t *testing.T) { assert := assert.New(t) @@ -384,7 +296,7 @@ func TestDomain_DisallowDefaultRoleModification(t *testing.T) { // This actually tests not just default, but all roles _, err = db.Exec(`update iam_role set public_id = $1 where public_id = $2`, "r_something", "r_default") require.Error(t, err) - assert.Equal(t, `pq: update of role primary key not allowed`, err.Error()) + assert.Equal(t, `pq: immutable column: iam_role.public_id`, err.Error()) } func TestDomain_DefaultUsersExist(t *testing.T) { @@ -401,3 +313,178 @@ func TestDomain_DefaultUsersExist(t *testing.T) { assert.Equal(t, 1, count) } } + +func TestDomain_immutable_columns_func(t *testing.T) { + const ( + createTable = ` +create table if not exists test_immutable_columns ( + id bigint generated always as identity primary key, + name text not null, + create_time wt_timestamp not null, + updatable text not null +); +` + addTriggers = ` +create trigger + test_update_immutable_columns +before +update on test_immutable_columns + for each row execute procedure immutable_columns('id', 'name', 'create_time'); +` + dropTriggers = `drop trigger test_update_immutable_columns on test_immutable_columns` + + addBadTriggers = ` +create trigger + test_update_bad_immutable_columns +before +update on test_immutable_columns + for each row execute procedure immutable_columns('id', 'bad_column_name', 'create_time'); +` + dropBadTriggers = `drop trigger test_update_bad_immutable_columns on test_immutable_columns` + + query = ` +select id, name, create_time, updatable from test_immutable_columns where id = $1 +` + countQuery = ` +select count(*) from test_immutable_columns +` + insert = ` +insert into test_immutable_columns (name, updatable) +values ('alice', 'update alice') +returning id; +` + // no where clause intentionally, since it's not needed for the test + update_updatable = ` +update test_immutable_columns +set updatable = 'updated bob'; +` + + // no where clause intentionally, since it's not needed for the test + bad_update_create_time = ` +update test_immutable_columns +set create_time = now(); +` + // no where clause intentionally, since it's not needed for the test + bad_update_null_create_time = ` +update test_immutable_columns +set create_time = null; +` + + // no where clause intentionally, since it's not needed for the test + bad_update_id = ` +update test_immutable_columns +set id = 22; +` + // no where clause intentionally, since it's not needed for the test + bad_update_null_id = ` +update test_immutable_columns +set id = null; +` + ) + + type rowData struct { + Id int + CreateTime time.Time + Name string + Updatable string + } + + conn, _ := TestSetup(t, "postgres") + db := conn.DB() + _, err := db.Exec(createTable) + assert.NoError(t, err) + + _, err = db.Exec(addTriggers) + assert.NoError(t, err) + + var id int + err = db.QueryRow(insert).Scan(&id) + assert.NoError(t, err) + + var orig rowData + err = db.QueryRow(query, id).Scan(&orig.Id, &orig.Name, &orig.CreateTime, &orig.Updatable) + assert.NoError(t, err) + + var count int + err = db.QueryRow(countQuery).Scan(&count) + assert.NoError(t, err) + assert.Equal(t, 1, count) + + var tests = []struct { + name string + stmt string + }{ + { + name: "update create time", + stmt: bad_update_create_time, + }, + { + name: "null create time", + stmt: bad_update_null_create_time, + }, + { + name: "update id", + stmt: bad_update_id, + }, + { + name: "null id", + stmt: bad_update_null_id, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + _, err = db.Exec(tt.stmt) + assert.Error(err) + + var found rowData + err = db.QueryRow(query, id).Scan(&found.Id, &found.Name, &found.CreateTime, &found.Updatable) + assert.NoError(err) + assert.Equal(orig.Id, found.Id) + assert.Equal(orig.Name, found.Name) + assert.Equal(orig.CreateTime, found.CreateTime) + }) + } + t.Run("updatable", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + _, err = db.Exec(update_updatable) + require.NoError(err) + var found rowData + err = db.QueryRow(query, id).Scan(&found.Id, &found.Name, &found.CreateTime, &found.Updatable) + require.NoError(err) + assert.NotEqual(orig.Updatable, found.Updatable) + }) + + t.Run("bad column", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + t.Cleanup(func() { + // reset the update trigger to what they were before this test + // started. + _, err = db.Exec(dropBadTriggers) + require.NoError(err) + + _, err = db.Exec(addTriggers) + assert.NoError(err) + }) + _, err = db.Exec(dropTriggers) + require.NoError(err) + + _, err = db.Exec(addBadTriggers) + require.NoError(err) + + _, err = db.Exec(update_updatable) + require.Error(err) + assert.Containsf(err.Error(), `pq: column "bad_column_name" not found`, "unexpected error msg: %s", err.Error()) + + var found rowData + err = db.QueryRow(query, id).Scan(&found.Id, &found.Name, &found.CreateTime, &found.Updatable) + assert.NoError(err) + assert.Equal(orig.Id, found.Id) + assert.Equal(orig.Name, found.Name) + assert.Equal(orig.CreateTime, found.CreateTime) + + }) + +} diff --git a/internal/db/migrations/postgres.gen.go b/internal/db/migrations/postgres.gen.go index d65781dd9..f8c809ed6 100644 --- a/internal/db/migrations/postgres.gen.go +++ b/internal/db/migrations/postgres.gen.go @@ -18,9 +18,9 @@ drop domain wt_user_id; drop domain wt_version; drop function default_create_time; -drop function immutable_create_time_func; drop function update_time_column; drop function update_version_column; +drop function immutable_columns; commit; @@ -90,24 +90,6 @@ comment on function update_time_column() is 'function used in before update triggers to properly set update_time columns'; - -create or replace function - immutable_create_time_func() - returns trigger -as $$ -begin - if new.create_time is distinct from old.create_time then - raise warning 'create_time cannot be set to %', new.create_time; - new.create_time = old.create_time; - end if; - return new; -end; -$$ language plpgsql; - -comment on function - immutable_create_time_func() -is - 'function used in before update triggers to make create_time column immutable'; create or replace function default_create_time() @@ -160,6 +142,38 @@ comment on function is 'function used in after update triggers to properly set version columns'; +-- immutable_columns() will make the column names immutable which are passed as +-- parameters when the trigger is created. It raises error code 23601 which is a +-- class 23 integrity constraint violation: immutable column +create or replace function + immutable_columns() + returns trigger +as $$ +declare + col_name text; + new_value text; + old_value text; +begin + foreach col_name in array tg_argv loop + execute format('SELECT $1.%I', col_name) into new_value using new; + execute format('SELECT $1.%I', col_name) into old_value using old; + if new_value is distinct from old_value then + raise exception 'immutable column: %.%', tg_table_name, col_name using + errcode = '23601', + schema = tg_table_schema, + table = tg_table_name, + column = col_name; + end if; + end loop; + return new; +end; +$$ language plpgsql; + +comment on function + immutable_columns() +is + 'function used in before update triggers to make columns immutable'; + commit; `), @@ -182,6 +196,7 @@ commit; bytes: []byte(` begin; +-- TODO (jimlambrt 7/2020) remove update_time create table if not exists oplog_entry ( id bigint generated always as identity primary key, create_time wt_timestamp, @@ -197,18 +212,19 @@ before update on oplog_entry for each row execute procedure update_time_column(); -create trigger - create_time_column -before -update on oplog_entry - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on oplog_entry for each row execute procedure default_create_time(); +-- oplog_entry is immutable. +create trigger + immutable_columns +before +update on oplog_entry + for each row execute procedure immutable_columns('id','update_time','create_time','version','aggregate_name', 'data'); + create table if not exists oplog_ticket ( id bigint generated always as identity primary key, create_time wt_timestamp, @@ -223,18 +239,20 @@ before update on oplog_ticket for each row execute procedure update_time_column(); -create trigger - create_time_column -before -update on oplog_ticket - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on oplog_ticket for each row execute procedure default_create_time(); +-- oplog_ticket: only allow updates to: version and update_time +create trigger + immutable_columns +before +update on oplog_ticket + for each row execute procedure immutable_columns('id','create_time','name'); + +-- TODO (jimlambrt 7/2020) remove update_time create table if not exists oplog_metadata ( id bigint generated always as identity primary key, create_time wt_timestamp, @@ -250,18 +268,19 @@ before update on oplog_metadata for each row execute procedure update_time_column(); -create trigger - create_time_column -before -update on oplog_metadata - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on oplog_metadata for each row execute procedure default_create_time(); + -- oplog_metadata is immutable +create trigger + immutable_columns +before +update on oplog_metadata + for each row execute procedure immutable_columns('id','create_time','update_time','entry_id','key','value'); + create index if not exists idx_oplog_metatadata_key on oplog_metadata(key); create index if not exists idx_oplog_metatadata_value on oplog_metadata(value); @@ -331,11 +350,12 @@ before update on db_test_user for each row execute procedure update_time_column(); +-- define the immutable fields for db_test_user create trigger - create_time_column + immutable_columns before -update on db_test_user - for each row execute procedure immutable_create_time_func(); +update on db_test_user + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -364,11 +384,12 @@ before update on db_test_car for each row execute procedure update_time_column(); +-- define the immutable fields for db_test_car create trigger - create_time_column + immutable_columns before -update on db_test_car - for each row execute procedure immutable_create_time_func(); +update on db_test_car + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -392,11 +413,12 @@ before update on db_test_rental for each row execute procedure update_time_column(); +-- define the immutable fields for db_test_rental create trigger - create_time_column + immutable_columns before -update on db_test_rental - for each row execute procedure immutable_create_time_func(); +update on db_test_rental + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -421,11 +443,13 @@ before update on db_test_scooter for each row execute procedure update_time_column(); + +-- define the immutable fields for db_test_scooter create trigger - create_time_column + immutable_columns before -update on db_test_scooter - for each row execute procedure immutable_create_time_func(); +update on db_test_scooter + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -467,7 +491,6 @@ drop function iam_group_member_scope_check cascade; drop function iam_immutable_group_member cascade; drop function get_scoped_member_id cascade; drop function grant_scope_id_valid cascade; -drop function immutable_scope_id_func cascade; drop function disallow_global_scope_deletion cascade; drop function user_scope_id_valid cascade; drop function iam_immutable_role_grant cascade; @@ -493,23 +516,12 @@ values ('org'), ('project'); - -create or replace function - iam_immutable_scope_id_func() - returns trigger -as $$ -begin - if new.scope_id is distinct from old.scope_id then - raise exception 'scope_id cannot be set to %', new.scope_id; - end if; - return new; -end; -$$ language plpgsql; - -comment on function - iam_immutable_scope_id_func() -is - 'function used in before update triggers to make scope_id column is immutable'; + -- define the immutable fields of iam_scope_type_enm +create trigger + immutable_columns +before +update on iam_scope_type_enm + for each row execute procedure immutable_columns('string'); create table iam_scope ( public_id wt_scope_id primary key, @@ -619,35 +631,11 @@ before delete on iam_scope for each row execute procedure disallow_global_scope_deletion(); -create or replace function - iam_immutable_scope_type_func() - returns trigger -as $$ -declare parent_type int; -begin - if new.type != old.type then - raise exception 'scope type cannot be updated'; - end if; - return new; -end; -$$ language plpgsql; - -create trigger - iam_scope_update -before -update on iam_scope - for each row execute procedure iam_immutable_scope_type_func(); create trigger update_time_column before update on iam_scope for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_scope - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -656,6 +644,35 @@ insert on iam_scope for each row execute procedure default_create_time(); + -- define the immutable fields for iam_scope +create trigger + immutable_columns +before +update on iam_scope + for each row execute procedure immutable_columns('public_id', 'create_time', 'type', 'parent_id'); + + -- define the immutable fields of iam_scope_global +create trigger + immutable_columns +before +update on iam_scope_global + for each row execute procedure immutable_columns('scope_id'); + + -- define the immutable fields of iam_scope_org +create trigger + immutable_columns +before +update on iam_scope_org + for each row execute procedure immutable_columns('scope_id'); + + -- define the immutable fields of iam_scope_project +create trigger + immutable_columns +before +update on iam_scope_project + for each row execute procedure immutable_columns('scope_id'); + + -- iam_sub_names will allow us to enforce the different name constraints for -- orgs and projects via a before update trigger on the iam_scope -- table. @@ -789,12 +806,6 @@ create trigger update_time_column before update on iam_user for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_user - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -802,11 +813,6 @@ before insert on iam_user for each row execute procedure default_create_time(); -create trigger immutable_scope_id_user -before -update on iam_user - for each row execute procedure iam_immutable_scope_id_func(); - create trigger iam_user_disallow_anon_auth_deletion before @@ -820,6 +826,13 @@ insert into iam_user (public_id, name, description, scope_id) insert into iam_user (public_id, name, description, scope_id) values ('u_auth', 'authenticated', 'The authenticated user matches any user that has a valid token', 'global'); + -- define the immutable fields for iam_user +create trigger + immutable_columns +before +update on iam_user + for each row execute procedure immutable_columns('public_id', 'create_time', 'scope_id'); + create table iam_role ( public_id wt_role_id primary key, create_time wt_timestamp, @@ -880,12 +893,6 @@ before insert on iam_role_grant for each row execute procedure default_create_time(); --- prefix a_ to make this trigger run before ensure_grant_scope_id_valid -create trigger a_immutable_scope_id -before -update on iam_role - for each row execute procedure iam_immutable_scope_id_func(); - create trigger update_version_column after update on iam_role @@ -895,12 +902,6 @@ create trigger update_time_column before update on iam_role for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_role - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -934,24 +935,13 @@ before delete on iam_role for each row execute procedure disallow_default_role_deletion(); --- immutable_default_role_id prevents the a role's id from being updated. -create or replace function - immutable_role_id() - returns trigger -as $$ -begin - if new.public_id is distinct from old.public_id then - raise exception 'update of role primary key not allowed'; - end if; - return new; -end; -$$ language plpgsql; - +-- define the immutable fields for iam_role (started trigger name with "a_" so +-- it will run first) create trigger - immutable_role_id + a_immutable_columns before update on iam_role - for each row execute procedure immutable_role_id(); + for each row execute procedure immutable_columns('public_id', 'create_time', 'scope_id'); insert into iam_role (public_id, name, description, scope_id) values('r_default', 'default', 'default role', 'global'); @@ -985,12 +975,6 @@ create trigger update_time_column before update on iam_group for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_group - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -998,10 +982,12 @@ before insert on iam_group for each row execute procedure default_create_time(); -create trigger immutable_scope_id_group +-- define the immutable fields for iam_group +create trigger + immutable_columns before update on iam_group - for each row execute procedure iam_immutable_scope_id_func(); + for each row execute procedure immutable_columns('public_id', 'create_time', 'scope_id'); -- iam_user_role contains roles that have been assigned to users. Users can be -- from any scope. The rows in this table must be immutable after insert, which @@ -1106,12 +1092,6 @@ before update on iam_user_role for each row execute procedure iam_immutable_role_principal(); -create trigger - immutable_create_time -before -update on iam_user_role - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before @@ -1122,12 +1102,6 @@ create trigger immutable_role_principal before update on iam_group_role for each row execute procedure iam_immutable_role_principal(); - -create trigger - immutable_create_time -before -update on iam_group_role - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -1160,12 +1134,6 @@ before insert on iam_group_member_user for each row execute procedure default_create_time(); -create trigger - immutable_create_time -before -update on iam_group_member_user - for each row execute procedure immutable_create_time_func(); - create trigger iam_immutable_group_member before update on iam_group_member_user @@ -1382,18 +1350,18 @@ begin; before update on static_host_catalog for each row execute procedure update_time_column(); - create trigger - immutable_create_time - before - update on static_host_catalog - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on static_host_catalog for each row execute procedure default_create_time(); + create trigger + immutable_columns + before + update on static_host_catalog + for each row execute procedure immutable_columns('public_id', 'scope_id','create_time'); + create table static_host ( public_id wt_public_id primary key, static_host_catalog_id wt_public_id not null @@ -1418,18 +1386,18 @@ begin; before update on static_host for each row execute procedure update_time_column(); - create trigger - immutable_create_time - before - update on static_host - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on static_host for each row execute procedure default_create_time(); + create trigger + immutable_columns + before + update on static_host + for each row execute procedure immutable_columns('public_id', 'static_host_catalog_id','create_time'); + create table static_host_set ( public_id wt_public_id primary key, static_host_catalog_id wt_public_id not null @@ -1448,18 +1416,19 @@ begin; before update on static_host_set for each row execute procedure update_time_column(); - create trigger - immutable_create_time - before - update on static_host_set - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on static_host_set for each row execute procedure default_create_time(); + + create trigger + immutable_columns + before + update on static_host_set + for each row execute procedure immutable_columns('public_id', 'static_host_catalog_id','create_time'); + create table static_host_set_member ( static_host_set_id wt_public_id references static_host_set (public_id) @@ -1472,6 +1441,12 @@ begin; primary key(static_host_set_id, static_host_id) ); + create trigger + immutable_columns + before + update on static_host_set_member + for each row execute procedure immutable_columns('static_host_set_id', 'static_host_id'); + insert into oplog_ticket (name, version) values ('static_host_catalog', 1), @@ -1619,16 +1594,18 @@ begin; before update on auth_token for each row execute procedure update_last_access_time(); - create trigger - immutable_create_time - before update on auth_token - for each row execute procedure immutable_create_time_func(); create trigger immutable_auth_token_columns before update on auth_token for each row execute procedure immutable_auth_token_columns(); + create trigger + immutable_columns + before + update on auth_token + for each row execute procedure immutable_columns('public_id', 'auth_account_id', 'create_time'); + commit; `), @@ -1850,10 +1827,10 @@ begin; for each row execute procedure update_time_column(); create trigger - immutable_create_time + immutable_columns before update on auth_password_method - for each row execute procedure immutable_create_time_func(); + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -1868,10 +1845,10 @@ begin; for each row execute procedure update_time_column(); create trigger - immutable_create_time + immutable_columns before update on auth_password_account - for each row execute procedure immutable_create_time_func(); + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column diff --git a/internal/db/migrations/postgres/01_domain_types.down.sql b/internal/db/migrations/postgres/01_domain_types.down.sql index 4552416d9..08dab0bf7 100644 --- a/internal/db/migrations/postgres/01_domain_types.down.sql +++ b/internal/db/migrations/postgres/01_domain_types.down.sql @@ -8,8 +8,8 @@ drop domain wt_user_id; drop domain wt_version; drop function default_create_time; -drop function immutable_create_time_func; drop function update_time_column; drop function update_version_column; +drop function immutable_columns; commit; diff --git a/internal/db/migrations/postgres/01_domain_types.up.sql b/internal/db/migrations/postgres/01_domain_types.up.sql index 825f681cf..1acd44586 100644 --- a/internal/db/migrations/postgres/01_domain_types.up.sql +++ b/internal/db/migrations/postgres/01_domain_types.up.sql @@ -59,24 +59,6 @@ comment on function update_time_column() is 'function used in before update triggers to properly set update_time columns'; - -create or replace function - immutable_create_time_func() - returns trigger -as $$ -begin - if new.create_time is distinct from old.create_time then - raise warning 'create_time cannot be set to %', new.create_time; - new.create_time = old.create_time; - end if; - return new; -end; -$$ language plpgsql; - -comment on function - immutable_create_time_func() -is - 'function used in before update triggers to make create_time column immutable'; create or replace function default_create_time() @@ -129,4 +111,36 @@ comment on function is 'function used in after update triggers to properly set version columns'; +-- immutable_columns() will make the column names immutable which are passed as +-- parameters when the trigger is created. It raises error code 23601 which is a +-- class 23 integrity constraint violation: immutable column +create or replace function + immutable_columns() + returns trigger +as $$ +declare + col_name text; + new_value text; + old_value text; +begin + foreach col_name in array tg_argv loop + execute format('SELECT $1.%I', col_name) into new_value using new; + execute format('SELECT $1.%I', col_name) into old_value using old; + if new_value is distinct from old_value then + raise exception 'immutable column: %.%', tg_table_name, col_name using + errcode = '23601', + schema = tg_table_schema, + table = tg_table_name, + column = col_name; + end if; + end loop; + return new; +end; +$$ language plpgsql; + +comment on function + immutable_columns() +is + 'function used in before update triggers to make columns immutable'; + commit; diff --git a/internal/db/migrations/postgres/02_oplog.up.sql b/internal/db/migrations/postgres/02_oplog.up.sql index fed8b1314..930bff96b 100644 --- a/internal/db/migrations/postgres/02_oplog.up.sql +++ b/internal/db/migrations/postgres/02_oplog.up.sql @@ -1,5 +1,6 @@ begin; +-- TODO (jimlambrt 7/2020) remove update_time create table if not exists oplog_entry ( id bigint generated always as identity primary key, create_time wt_timestamp, @@ -15,18 +16,19 @@ before update on oplog_entry for each row execute procedure update_time_column(); -create trigger - create_time_column -before -update on oplog_entry - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on oplog_entry for each row execute procedure default_create_time(); +-- oplog_entry is immutable. +create trigger + immutable_columns +before +update on oplog_entry + for each row execute procedure immutable_columns('id','update_time','create_time','version','aggregate_name', 'data'); + create table if not exists oplog_ticket ( id bigint generated always as identity primary key, create_time wt_timestamp, @@ -41,18 +43,20 @@ before update on oplog_ticket for each row execute procedure update_time_column(); -create trigger - create_time_column -before -update on oplog_ticket - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on oplog_ticket for each row execute procedure default_create_time(); +-- oplog_ticket: only allow updates to: version and update_time +create trigger + immutable_columns +before +update on oplog_ticket + for each row execute procedure immutable_columns('id','create_time','name'); + +-- TODO (jimlambrt 7/2020) remove update_time create table if not exists oplog_metadata ( id bigint generated always as identity primary key, create_time wt_timestamp, @@ -68,18 +72,19 @@ before update on oplog_metadata for each row execute procedure update_time_column(); -create trigger - create_time_column -before -update on oplog_metadata - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on oplog_metadata for each row execute procedure default_create_time(); + -- oplog_metadata is immutable +create trigger + immutable_columns +before +update on oplog_metadata + for each row execute procedure immutable_columns('id','create_time','update_time','entry_id','key','value'); + create index if not exists idx_oplog_metatadata_key on oplog_metadata(key); create index if not exists idx_oplog_metatadata_value on oplog_metadata(value); diff --git a/internal/db/migrations/postgres/03_db.up.sql b/internal/db/migrations/postgres/03_db.up.sql index 57acbc01e..d4e754fe6 100644 --- a/internal/db/migrations/postgres/03_db.up.sql +++ b/internal/db/migrations/postgres/03_db.up.sql @@ -21,11 +21,12 @@ before update on db_test_user for each row execute procedure update_time_column(); +-- define the immutable fields for db_test_user create trigger - create_time_column + immutable_columns before -update on db_test_user - for each row execute procedure immutable_create_time_func(); +update on db_test_user + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -54,11 +55,12 @@ before update on db_test_car for each row execute procedure update_time_column(); +-- define the immutable fields for db_test_car create trigger - create_time_column + immutable_columns before -update on db_test_car - for each row execute procedure immutable_create_time_func(); +update on db_test_car + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -82,11 +84,12 @@ before update on db_test_rental for each row execute procedure update_time_column(); +-- define the immutable fields for db_test_rental create trigger - create_time_column + immutable_columns before -update on db_test_rental - for each row execute procedure immutable_create_time_func(); +update on db_test_rental + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -111,11 +114,13 @@ before update on db_test_scooter for each row execute procedure update_time_column(); + +-- define the immutable fields for db_test_scooter create trigger - create_time_column + immutable_columns before -update on db_test_scooter - for each row execute procedure immutable_create_time_func(); +update on db_test_scooter + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column diff --git a/internal/db/migrations/postgres/06_iam.down.sql b/internal/db/migrations/postgres/06_iam.down.sql index d25c2a7b5..a472ac02a 100644 --- a/internal/db/migrations/postgres/06_iam.down.sql +++ b/internal/db/migrations/postgres/06_iam.down.sql @@ -25,7 +25,6 @@ drop function iam_group_member_scope_check cascade; drop function iam_immutable_group_member cascade; drop function get_scoped_member_id cascade; drop function grant_scope_id_valid cascade; -drop function immutable_scope_id_func cascade; drop function disallow_global_scope_deletion cascade; drop function user_scope_id_valid cascade; drop function iam_immutable_role_grant cascade; diff --git a/internal/db/migrations/postgres/06_iam.up.sql b/internal/db/migrations/postgres/06_iam.up.sql index 04db35c26..fdbba194e 100644 --- a/internal/db/migrations/postgres/06_iam.up.sql +++ b/internal/db/migrations/postgres/06_iam.up.sql @@ -11,23 +11,12 @@ values ('org'), ('project'); - -create or replace function - iam_immutable_scope_id_func() - returns trigger -as $$ -begin - if new.scope_id is distinct from old.scope_id then - raise exception 'scope_id cannot be set to %', new.scope_id; - end if; - return new; -end; -$$ language plpgsql; - -comment on function - iam_immutable_scope_id_func() -is - 'function used in before update triggers to make scope_id column is immutable'; + -- define the immutable fields of iam_scope_type_enm +create trigger + immutable_columns +before +update on iam_scope_type_enm + for each row execute procedure immutable_columns('string'); create table iam_scope ( public_id wt_scope_id primary key, @@ -137,35 +126,11 @@ before delete on iam_scope for each row execute procedure disallow_global_scope_deletion(); -create or replace function - iam_immutable_scope_type_func() - returns trigger -as $$ -declare parent_type int; -begin - if new.type != old.type then - raise exception 'scope type cannot be updated'; - end if; - return new; -end; -$$ language plpgsql; - -create trigger - iam_scope_update -before -update on iam_scope - for each row execute procedure iam_immutable_scope_type_func(); create trigger update_time_column before update on iam_scope for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_scope - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -174,6 +139,35 @@ insert on iam_scope for each row execute procedure default_create_time(); + -- define the immutable fields for iam_scope +create trigger + immutable_columns +before +update on iam_scope + for each row execute procedure immutable_columns('public_id', 'create_time', 'type', 'parent_id'); + + -- define the immutable fields of iam_scope_global +create trigger + immutable_columns +before +update on iam_scope_global + for each row execute procedure immutable_columns('scope_id'); + + -- define the immutable fields of iam_scope_org +create trigger + immutable_columns +before +update on iam_scope_org + for each row execute procedure immutable_columns('scope_id'); + + -- define the immutable fields of iam_scope_project +create trigger + immutable_columns +before +update on iam_scope_project + for each row execute procedure immutable_columns('scope_id'); + + -- iam_sub_names will allow us to enforce the different name constraints for -- orgs and projects via a before update trigger on the iam_scope -- table. @@ -307,12 +301,6 @@ create trigger update_time_column before update on iam_user for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_user - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -320,11 +308,6 @@ before insert on iam_user for each row execute procedure default_create_time(); -create trigger immutable_scope_id_user -before -update on iam_user - for each row execute procedure iam_immutable_scope_id_func(); - create trigger iam_user_disallow_anon_auth_deletion before @@ -338,6 +321,13 @@ insert into iam_user (public_id, name, description, scope_id) insert into iam_user (public_id, name, description, scope_id) values ('u_auth', 'authenticated', 'The authenticated user matches any user that has a valid token', 'global'); + -- define the immutable fields for iam_user +create trigger + immutable_columns +before +update on iam_user + for each row execute procedure immutable_columns('public_id', 'create_time', 'scope_id'); + create table iam_role ( public_id wt_role_id primary key, create_time wt_timestamp, @@ -398,12 +388,6 @@ before insert on iam_role_grant for each row execute procedure default_create_time(); --- prefix a_ to make this trigger run before ensure_grant_scope_id_valid -create trigger a_immutable_scope_id -before -update on iam_role - for each row execute procedure iam_immutable_scope_id_func(); - create trigger update_version_column after update on iam_role @@ -413,12 +397,6 @@ create trigger update_time_column before update on iam_role for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_role - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -452,24 +430,13 @@ before delete on iam_role for each row execute procedure disallow_default_role_deletion(); --- immutable_default_role_id prevents the a role's id from being updated. -create or replace function - immutable_role_id() - returns trigger -as $$ -begin - if new.public_id is distinct from old.public_id then - raise exception 'update of role primary key not allowed'; - end if; - return new; -end; -$$ language plpgsql; - +-- define the immutable fields for iam_role (started trigger name with "a_" so +-- it will run first) create trigger - immutable_role_id + a_immutable_columns before update on iam_role - for each row execute procedure immutable_role_id(); + for each row execute procedure immutable_columns('public_id', 'create_time', 'scope_id'); insert into iam_role (public_id, name, description, scope_id) values('r_default', 'default', 'default role', 'global'); @@ -503,12 +470,6 @@ create trigger update_time_column before update on iam_group for each row execute procedure update_time_column(); - -create trigger - immutable_create_time -before -update on iam_group - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -516,10 +477,12 @@ before insert on iam_group for each row execute procedure default_create_time(); -create trigger immutable_scope_id_group +-- define the immutable fields for iam_group +create trigger + immutable_columns before update on iam_group - for each row execute procedure iam_immutable_scope_id_func(); + for each row execute procedure immutable_columns('public_id', 'create_time', 'scope_id'); -- iam_user_role contains roles that have been assigned to users. Users can be -- from any scope. The rows in this table must be immutable after insert, which @@ -624,12 +587,6 @@ before update on iam_user_role for each row execute procedure iam_immutable_role_principal(); -create trigger - immutable_create_time -before -update on iam_user_role - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before @@ -640,12 +597,6 @@ create trigger immutable_role_principal before update on iam_group_role for each row execute procedure iam_immutable_role_principal(); - -create trigger - immutable_create_time -before -update on iam_group_role - for each row execute procedure immutable_create_time_func(); create trigger default_create_time_column @@ -678,12 +629,6 @@ before insert on iam_group_member_user for each row execute procedure default_create_time(); -create trigger - immutable_create_time -before -update on iam_group_member_user - for each row execute procedure immutable_create_time_func(); - create trigger iam_immutable_group_member before update on iam_group_member_user diff --git a/internal/db/migrations/postgres/10_static_host.up.sql b/internal/db/migrations/postgres/10_static_host.up.sql index 11dc7b771..48aeac9b1 100644 --- a/internal/db/migrations/postgres/10_static_host.up.sql +++ b/internal/db/migrations/postgres/10_static_host.up.sql @@ -18,18 +18,18 @@ begin; before update on static_host_catalog for each row execute procedure update_time_column(); - create trigger - immutable_create_time - before - update on static_host_catalog - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on static_host_catalog for each row execute procedure default_create_time(); + create trigger + immutable_columns + before + update on static_host_catalog + for each row execute procedure immutable_columns('public_id', 'scope_id','create_time'); + create table static_host ( public_id wt_public_id primary key, static_host_catalog_id wt_public_id not null @@ -54,18 +54,18 @@ begin; before update on static_host for each row execute procedure update_time_column(); - create trigger - immutable_create_time - before - update on static_host - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on static_host for each row execute procedure default_create_time(); + create trigger + immutable_columns + before + update on static_host + for each row execute procedure immutable_columns('public_id', 'static_host_catalog_id','create_time'); + create table static_host_set ( public_id wt_public_id primary key, static_host_catalog_id wt_public_id not null @@ -84,18 +84,19 @@ begin; before update on static_host_set for each row execute procedure update_time_column(); - create trigger - immutable_create_time - before - update on static_host_set - for each row execute procedure immutable_create_time_func(); - create trigger default_create_time_column before insert on static_host_set for each row execute procedure default_create_time(); + + create trigger + immutable_columns + before + update on static_host_set + for each row execute procedure immutable_columns('public_id', 'static_host_catalog_id','create_time'); + create table static_host_set_member ( static_host_set_id wt_public_id references static_host_set (public_id) @@ -108,6 +109,12 @@ begin; primary key(static_host_set_id, static_host_id) ); + create trigger + immutable_columns + before + update on static_host_set_member + for each row execute procedure immutable_columns('static_host_set_id', 'static_host_id'); + insert into oplog_ticket (name, version) values ('static_host_catalog', 1), diff --git a/internal/db/migrations/postgres/11_auth_token.up.sql b/internal/db/migrations/postgres/11_auth_token.up.sql index 67b642add..bf09dd04c 100644 --- a/internal/db/migrations/postgres/11_auth_token.up.sql +++ b/internal/db/migrations/postgres/11_auth_token.up.sql @@ -115,14 +115,16 @@ begin; before update on auth_token for each row execute procedure update_last_access_time(); - create trigger - immutable_create_time - before update on auth_token - for each row execute procedure immutable_create_time_func(); create trigger immutable_auth_token_columns before update on auth_token for each row execute procedure immutable_auth_token_columns(); + create trigger + immutable_columns + before + update on auth_token + for each row execute procedure immutable_columns('public_id', 'auth_account_id', 'create_time'); + commit; diff --git a/internal/db/migrations/postgres/12_auth_password.up.sql b/internal/db/migrations/postgres/12_auth_password.up.sql index 1c8ce5eb0..80d76cda6 100644 --- a/internal/db/migrations/postgres/12_auth_password.up.sql +++ b/internal/db/migrations/postgres/12_auth_password.up.sql @@ -195,10 +195,10 @@ begin; for each row execute procedure update_time_column(); create trigger - immutable_create_time + immutable_columns before update on auth_password_method - for each row execute procedure immutable_create_time_func(); + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column @@ -213,10 +213,10 @@ begin; for each row execute procedure update_time_column(); create trigger - immutable_create_time + immutable_columns before update on auth_password_account - for each row execute procedure immutable_create_time_func(); + for each row execute procedure immutable_columns('create_time'); create trigger default_create_time_column diff --git a/internal/host/static/immutable_fields_test.go b/internal/host/static/immutable_fields_test.go new file mode 100644 index 000000000..b11211d17 --- /dev/null +++ b/internal/host/static/immutable_fields_test.go @@ -0,0 +1,292 @@ +package static + +import ( + "context" + "testing" + + "github.com/hashicorp/watchtower/internal/db" + "github.com/hashicorp/watchtower/internal/db/timestamp" + "github.com/hashicorp/watchtower/internal/host/static/store" + "github.com/hashicorp/watchtower/internal/iam" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" +) + +func TestHostCatalog_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + _, proj := iam.TestScopes(t, conn) + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + new := testCatalog(t, conn) + + var tests = []struct { + name string + update *HostCatalog + fieldMask []string + }{ + { + name: "public_id", + update: func() *HostCatalog { + c := new.clone() + c.PublicId = "hc_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *HostCatalog { + c := new.clone() + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "scope id", + update: func() *HostCatalog { + c := new.clone() + c.ScopeId = proj.PublicId + return c + }(), + fieldMask: []string{"ScopeId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.clone() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.clone() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig, after)) + + }) + } +} + +func TestStaticHost_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + cat := testCatalog(t, conn) + hosts := testHosts(t, conn, cat.GetPublicId(), 1) + + new := hosts[0] + + var tests = []struct { + name string + update *Host + fieldMask []string + }{ + { + name: "public_id", + update: func() *Host { + c := new.testCloneHost() + c.PublicId = "hc_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *Host { + c := new.testCloneHost() + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "static_host_catalog_id", + update: func() *Host { + c := new.testCloneHost() + c.StaticHostCatalogId = "stc_01234567890" + return c + }(), + fieldMask: []string{"StaticHostCatalogId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.testCloneHost() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.testCloneHost() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig, after)) + + }) + } +} + +func (c *Host) testCloneHost() *Host { + cp := proto.Clone(c.Host) + return &Host{ + Host: cp.(*store.Host), + } +} + +func TestStaticHostSet_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + cat := testCatalog(t, conn) + sets := testSets(t, conn, cat.GetPublicId(), 1) + + new := sets[0] + + var tests = []struct { + name string + update *HostSet + fieldMask []string + }{ + { + name: "public_id", + update: func() *HostSet { + c := new.testCloneHostSet() + c.PublicId = "hc_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *HostSet { + c := new.testCloneHostSet() + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "static_host_catalog_id", + update: func() *HostSet { + c := new.testCloneHostSet() + c.StaticHostCatalogId = "stc_01234567890" + return c + }(), + fieldMask: []string{"StaticHostCatalogId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.testCloneHostSet() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.testCloneHostSet() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig, after)) + + }) + } +} + +func (c *HostSet) testCloneHostSet() *HostSet { + cp := proto.Clone(c.HostSet) + return &HostSet{ + HostSet: cp.(*store.HostSet), + } +} + +func TestStaticHostSetMember_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + cat := testCatalog(t, conn) + sets := testSets(t, conn, cat.GetPublicId(), 1) + hosts := testHosts(t, conn, cat.GetPublicId(), 1) + + new, err := NewHostSetMember(sets[0].PublicId, hosts[0].PublicId) + require.NoError(t, err) + err = w.Create(context.Background(), new) + assert.NoError(t, err) + + var tests = []struct { + name string + update *HostSetMember + fieldMask []string + }{ + { + name: "static_host_set_id", + update: func() *HostSetMember { + c := new.testCloneHostSetMember() + c.StaticHostId = "shs_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"StaticHostSetId"}, + }, + { + name: "static_host_id", + update: func() *HostSetMember { + c := new.testCloneHostSetMember() + c.StaticHostId = "sth_01234567890" + return c + }(), + fieldMask: []string{"StaticHostId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.testCloneHostSetMember() + err = w.LookupWhere(context.Background(), orig, "static_host_id = ? and static_host_set_id = ?", orig.StaticHostId, orig.StaticHostSetId) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.testCloneHostSetMember() + err = w.LookupWhere(context.Background(), after, "static_host_id = ? and static_host_set_id = ?", after.StaticHostId, after.StaticHostSetId) + require.NoError(err) + + assert.True(proto.Equal(orig, after)) + + }) + } +} + +func (c *HostSetMember) testCloneHostSetMember() *HostSetMember { + cp := proto.Clone(c.HostSetMember) + return &HostSetMember{ + HostSetMember: cp.(*store.HostSetMember), + } +} diff --git a/internal/iam/immutable_fields_test.go b/internal/iam/immutable_fields_test.go new file mode 100644 index 000000000..b113689fe --- /dev/null +++ b/internal/iam/immutable_fields_test.go @@ -0,0 +1,335 @@ +package iam + +import ( + "context" + "strings" + "testing" + + "github.com/hashicorp/watchtower/internal/db" + "github.com/hashicorp/watchtower/internal/db/timestamp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" +) + +func TestScope_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + + new, _ := TestScopes(t, conn) + var tests = []struct { + name string + update *Scope + fieldMask []string + }{ + { + name: "public_id", + update: func() *Scope { + c := new.Clone().(*Scope) + c.PublicId = "o_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *Scope { + c := new.Clone().(*Scope) + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "type", + update: func() *Scope { + c := new.Clone().(*Scope) + c.Type = "project" + return c + }(), + fieldMask: []string{"Type"}, + }, + { + name: "parent_id", + update: func() *Scope { + u := new.Clone().(*Scope) + u.PublicId = "p_thisIsNotAValidId" + return u + }(), + fieldMask: []string{"ParentId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.Clone() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.Clone() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig.(*Scope), after.(*Scope))) + + }) + } +} + +func TestConcreteScope_ImmutableFields(t *testing.T) { + const ( + query = `select scope_id from {{rep}} where scope_id = $1` + update = `update {{rep}} set scope_id = $1 where scope_id = $2;` + ) + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + db := conn.DB() + + org, proj := TestScopes(t, conn) + + var tests = []struct { + name string + scopeId string + updateTo string + tableName string + }{ + { + name: "global", + scopeId: "global", + updateTo: "o_12345678901", + tableName: "iam_scope_global", + }, + { + name: "org", + scopeId: org.PublicId, + updateTo: "o_12345678901", + tableName: "iam_scope_org", + }, + { + name: "project", + scopeId: proj.PublicId, + updateTo: "o_12345678901", + tableName: "iam_scope_project", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + updateStmt := strings.Replace(update, "{{rep}}", tt.tableName, -1) + queryStmt := strings.Replace(query, "{{rep}}", tt.tableName, -1) + + _, err := db.Exec(updateStmt, tt.updateTo, tt.scopeId) + assert.Error(err) + + var scopeId string + err = db.QueryRow(queryStmt, tt.scopeId).Scan(&scopeId) + assert.NoError(err) + assert.Equal(tt.scopeId, scopeId) + }) + } +} + +func TestUser_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + + org, proj := TestScopes(t, conn) + new := TestUser(t, conn, org.PublicId) + + var tests = []struct { + name string + update *User + fieldMask []string + }{ + { + name: "public_id", + update: func() *User { + c := new.Clone().(*User) + c.PublicId = "o_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *User { + c := new.Clone().(*User) + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "scope id", + update: func() *User { + c := new.Clone().(*User) + c.ScopeId = proj.PublicId + return c + }(), + fieldMask: []string{"ScopeId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.Clone() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.Clone() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig.(*User), after.(*User))) + + }) + } +} + +func TestRole_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + + org, proj := TestScopes(t, conn) + new := TestRole(t, conn, org.PublicId) + + var tests = []struct { + name string + update *Role + fieldMask []string + }{ + { + name: "public_id", + update: func() *Role { + c := new.Clone().(*Role) + c.PublicId = "r_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *Role { + c := new.Clone().(*Role) + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "scope id", + update: func() *Role { + c := new.Clone().(*Role) + c.ScopeId = proj.PublicId + return c + }(), + fieldMask: []string{"ScopeId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.Clone() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.Clone() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig.(*Role), after.(*Role))) + + }) + } +} + +func TestGroup_ImmutableFields(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + w := db.New(conn) + + ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} + + org, proj := TestScopes(t, conn) + new := TestGroup(t, conn, org.PublicId) + + var tests = []struct { + name string + update *Group + fieldMask []string + }{ + { + name: "public_id", + update: func() *Group { + c := new.Clone().(*Group) + c.PublicId = "g_thisIsNotAValidId" + return c + }(), + fieldMask: []string{"PublicId"}, + }, + { + name: "create time", + update: func() *Group { + c := new.Clone().(*Group) + c.CreateTime = &ts + return c + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "scope id", + update: func() *Group { + c := new.Clone().(*Group) + c.ScopeId = proj.PublicId + return c + }(), + fieldMask: []string{"ScopeId"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := new.Clone() + err := w.LookupById(context.Background(), orig) + require.NoError(err) + + rowsUpdated, err := w.Update(context.Background(), tt.update, tt.fieldMask, nil) + require.Error(err) + assert.Equal(0, rowsUpdated) + + after := new.Clone() + err = w.LookupById(context.Background(), after) + require.NoError(err) + + assert.True(proto.Equal(orig.(*Group), after.(*Group))) + + }) + } +} diff --git a/internal/iam/repository_group_test.go b/internal/iam/repository_group_test.go index 8600d529c..65a59168a 100644 --- a/internal/iam/repository_group_test.go +++ b/internal/iam/repository_group_test.go @@ -390,7 +390,7 @@ func TestRepository_UpdateGroup(t *testing.T) { }, newScopeId: org.PublicId, wantErr: true, - wantErrMsg: `update: failed: pq: scope_id cannot be set`, + wantErrMsg: `update: failed: pq: immutable column: iam_group.scope_id`, directUpdate: true, }, } diff --git a/internal/iam/repository_user_test.go b/internal/iam/repository_user_test.go index c3a67634f..b28e802f3 100644 --- a/internal/iam/repository_user_test.go +++ b/internal/iam/repository_user_test.go @@ -290,7 +290,7 @@ func TestRepository_UpdateUser(t *testing.T) { opt: []Option{WithSkipVetForWrite(true)}, }, wantErr: true, - wantErrMsg: `update: failed: pq: scope_id cannot be set`, + wantErrMsg: `update: failed: pq: immutable column: iam_user.scope_id`, directUpdate: true, }, } diff --git a/internal/iam/role_test.go b/internal/iam/role_test.go index 08f48cf56..70bdfde1d 100644 --- a/internal/iam/role_test.go +++ b/internal/iam/role_test.go @@ -467,7 +467,7 @@ func Test_RoleUpdate(t *testing.T) { updatedRows, err := rw.Update(context.Background(), &updateRole, []string{"ScopeId"}, nil, db.WithSkipVetForWrite(true)) require.Error(err) assert.Equal(0, updatedRows) - assert.Equal("update: failed: pq: scope_id cannot be set to "+proj.PublicId, err.Error()) + assert.Equal("update: failed: pq: immutable column: iam_role.scope_id", err.Error()) }) } diff --git a/internal/oplog/immutable_fields_test.go b/internal/oplog/immutable_fields_test.go new file mode 100644 index 000000000..883b415d5 --- /dev/null +++ b/internal/oplog/immutable_fields_test.go @@ -0,0 +1,144 @@ +package oplog + +import ( + "context" + "testing" + "time" + + "github.com/golang/protobuf/ptypes" + "github.com/hashicorp/watchtower/internal/db/timestamp" + "github.com/hashicorp/watchtower/internal/oplog/oplog_test" + "github.com/hashicorp/watchtower/internal/oplog/store" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +func Test_ImmutableFields(t *testing.T) { + + cleanup, db := setup(t) + defer testCleanup(t, cleanup, db) + cipherer := testWrapper(t) + + writer := &GormWriter{db} + id := testId(t) + u := oplog_test.TestUser{ + Name: "foo-" + id, + } + t.Log(&u) + + id2 := testId(t) + u2 := oplog_test.TestUser{ + Name: "foo-" + id2, + } + t.Log(&u2) + + ticketer, err := NewGormTicketer(db, WithAggregateNames(true)) + require.NoError(t, err) + + ticket, err := ticketer.GetTicket("default") + require.NoError(t, err) + + new, err := NewEntry( + "test-users", + Metadata{ + "key-only": nil, + "deployment": []string{"amex"}, + "project": []string{"central-info-systems", "local-info-systems"}, + }, + cipherer, + ticketer, + ) + require.NoError(t, err) + err = new.WriteEntryWith(context.Background(), writer, ticket, + &Message{Message: &u, TypeName: "user", OpType: OpType_OP_TYPE_CREATE}, + &Message{Message: &u2, TypeName: "user", OpType: OpType_OP_TYPE_CREATE}) + require.NoError(t, err) + + future, err := ptypes.TimestampProto(time.Now().Add(time.Hour)) + require.NoError(t, err) + + var tests = []struct { + name string + update *Entry + fieldMask []string + }{ + { + name: "update id", + update: func() *Entry { e := testCloneEntry(new); e.Id = 11111111; return e }(), + fieldMask: []string{"Id"}, + }, + { + name: "update update_time", + update: func() *Entry { + e := testCloneEntry(new) + e.UpdateTime = ×tamp.Timestamp{Timestamp: future} + return e + }(), + fieldMask: []string{"UpdateTime"}, + }, + { + name: "update create_time", + update: func() *Entry { + e := testCloneEntry(new) + e.CreateTime = ×tamp.Timestamp{Timestamp: future} + return e + }(), + fieldMask: []string{"CreateTime"}, + }, + { + name: "update create_time", + update: func() *Entry { + e := testCloneEntry(new) + e.Version = "867-5309" + return e + }(), + fieldMask: []string{"Version"}, + }, + { + name: "update create_time", + update: func() *Entry { + e := testCloneEntry(new) + e.AggregateName = "Jenny" + return e + }(), + fieldMask: []string{"AggregateName"}, + }, + { + name: "update data", + update: func() *Entry { + e := testCloneEntry(new) + // CtData is the field sent to the db. + e.CtData = []byte("Lorem Ipsum") + return e + }(), + fieldMask: []string{"CtData"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + orig := testCloneEntry(new) + err = db.First(orig).Error + require.NoError(err) + + err := writer.Update(tt.update, tt.fieldMask, nil) + require.Error(err) + + after := testCloneEntry(new) + err = db.First(&after).Error + require.NoError(err) + + assert.True(proto.Equal(orig, after)) + + }) + } +} + +func testCloneEntry(e *Entry) *Entry { + cp := proto.Clone(e.Entry) + return &Entry{ + Entry: cp.(*store.Entry), + } +}