From e2248c1360736332c07bf7d7a9eb487d488c7e94 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Fri, 13 Jan 2023 11:24:32 -0500 Subject: [PATCH] fix(wh): Associate session fact with credential dimensions (#2787) The wh_upsert_credential_group function was incorrectly attempting to update the session connection fact table. There are a couple problems with this: 1. At the time that this runs, there would be no session connection facts for this session. 2. This function is meant to update the session fact table, there is a separate function that associates the credential group with the session connection facts. This also fixes another bug that was causing expired rows of the credential dimension to be included in the group membership. And finally it fixes a typo in the function name. --- CHANGELOG.md | 2 + .../16/03_wh_credential_dimension.up.sql | 1 + .../01_fix_wh_upsert_credential_group.up.sql | 80 ++++++++++++++++++ .../three_credentials_one_change.sql | 81 +++++++++++++++++-- 4 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 internal/db/schema/migrations/oss/postgres/61/01_fix_wh_upsert_credential_group.up.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 568877277..e9a8e9fde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. * plugins: Ignore `SIGHUP` sent to parent process; some init systems, notably `dumb-init`, would pass them along to the child processes and cause the plugin to exit ([PR](https://github.com/hashicorp/boundary/pull/2677)) +* data warehouse: Fix bug that caused credential dimensions to not get + associated with session facts ([PR](https://github.com/hashicorp/boundary/pull/2787)). ## 0.11.2 (2022/12/09) diff --git a/internal/db/schema/migrations/oss/postgres/16/03_wh_credential_dimension.up.sql b/internal/db/schema/migrations/oss/postgres/16/03_wh_credential_dimension.up.sql index e2792b40b..24bedceaf 100644 --- a/internal/db/schema/migrations/oss/postgres/16/03_wh_credential_dimension.up.sql +++ b/internal/db/schema/migrations/oss/postgres/16/03_wh_credential_dimension.up.sql @@ -90,6 +90,7 @@ begin; -- wh_upsert_credentail_group determines if a new wh_credential_group needs to be -- created due to changes to the coresponding wh_credential_dimensions. It then -- updates the wh_session_accumulating_fact to associate it with the correct wh_credential_group. + -- Replaced in 61/01_fix_wh_upsert_credential_group create function wh_upsert_credentail_group() returns trigger as $$ declare diff --git a/internal/db/schema/migrations/oss/postgres/61/01_fix_wh_upsert_credential_group.up.sql b/internal/db/schema/migrations/oss/postgres/61/01_fix_wh_upsert_credential_group.up.sql new file mode 100644 index 000000000..653360743 --- /dev/null +++ b/internal/db/schema/migrations/oss/postgres/61/01_fix_wh_upsert_credential_group.up.sql @@ -0,0 +1,80 @@ +begin; + + drop trigger wh_insert_stmt_session_credential_dynamic on session_credential_dynamic; + drop function wh_upsert_credentail_group; + + -- wh_upsert_credential_group determines if a new wh_credential_group needs to be + -- created due to changes to the coresponding wh_credential_dimensions. It then + -- updates the wh_session_accumulating_fact to associate it with the correct wh_credential_group. + -- Replaces function in 16/03_wh_credential_dimension + create function wh_upsert_credential_group() returns trigger + as $$ + declare + cg_key wh_dim_key; + t_id wt_public_id; + s_id wt_public_id; + c_key wh_dim_key; + begin + select distinct scd.session_id into strict s_id + from new_table as scd; + + select distinct s.target_id into strict t_id + from new_table as scd + left join session as s on s.public_id = scd.session_id; + + -- based on query written by Michele Gaffney + with + credential_list (key) as ( + select key + from wh_credential_dimension + where target_id = t_id + and credential_library_id in (select credential_library_id from new_table) + and current_row_indicator = 'Current' + ) + select distinct credential_group_key into cg_key + from wh_credential_group_membership a + where a.credential_key in (select key from credential_list) + and (select count(key) from credential_list) = + ( + select count(b.credential_key) + from wh_credential_group_membership b + where a.credential_key = b.credential_key + and b.credential_key in (select key from credential_list) + ) + and not exists + ( + select 1 + from wh_credential_group_membership b + where a.credential_key = b.credential_key + and b.credential_key not in (select key from credential_list) + ) + ; + if cg_key is null then + insert into wh_credential_group default values returning key into cg_key; + for c_key in + select key + from wh_credential_dimension + where target_id = t_id + and credential_library_id in (select credential_library_id from new_table) + and current_row_indicator = 'Current' + loop + insert into wh_credential_group_membership + (credential_group_key, credential_key) + values + (cg_key, c_key); + end loop; + end if; + + update wh_session_accumulating_fact + set credential_group_key = cg_key + where session_id = s_id; + + return null; + end; + $$ language plpgsql; + + create trigger wh_insert_stmt_session_credential_dynamic after insert on session_credential_dynamic + referencing new table as new_table + for each statement execute function wh_upsert_credential_group(); + +commit; diff --git a/internal/db/sqltest/tests/wh/credential_dimension/three_credentials_one_change.sql b/internal/db/sqltest/tests/wh/credential_dimension/three_credentials_one_change.sql index f0f00201d..b617c89f5 100644 --- a/internal/db/sqltest/tests/wh/credential_dimension/three_credentials_one_change.sql +++ b/internal/db/sqltest/tests/wh/credential_dimension/three_credentials_one_change.sql @@ -5,7 +5,7 @@ -- and a new session is created -- only one of the wh_credential_dimensions is updated begin; - select plan(4); + select plan(12); select wtt_load('widgets', 'iam', 'kms', 'auth', 'hosts', 'targets', 'credentials'); @@ -30,6 +30,42 @@ begin; select is(count(*), 3::bigint) from wh_credential_dimension where organization_id = 'o_____widget'; + prepare select_session_credential_group as + select credential_library_id::text, credential_store_id::text, target_id::text, credential_purpose::text, credential_library_vault_path::text + from wh_credential_dimension as wh_cd + join wh_credential_group_membership as wh_cgm on + wh_cd.key = wh_cgm.credential_key + join wh_credential_group as wh_cg on + wh_cg.key = wh_cgm.credential_group_key + join wh_session_accumulating_fact as wh_saf on + wh_saf.credential_group_key = wh_cg.key + where wh_saf.session_id = 's1____walter'; + select results_eq( + 'select_session_credential_group', + $$VALUES + ('vl______wvl1', 'vs_______wvs', 't_________wb', 'brokered', '/secrets'), + ('vl______wvl2', 'vs_______wvs', 't_________wb', 'brokered', '/secrets/ssh/admin'), + ('vl______wvl3', 'vs_______wvs', 't_________wb', 'brokered', '/secrets/kv/one')$$ + ); + + select isnt(wh_session_accumulating_fact.credential_group_key::text, 'no credentials') + from wh_session_accumulating_fact + where session_id = 's1____walter'; + + insert into session_connection + (session_id, public_id) + values + ('s1____walter', 'sc1____walter'); + + select isnt(wh_session_connection_accumulating_fact.credential_group_key::text, 'no credentials') + from wh_session_connection_accumulating_fact + where session_id = 's1____walter'; + + select is(wh_session_accumulating_fact.credential_group_key::text, wh_session_connection_accumulating_fact.credential_group_key::text, 'session fact and connection fact should have same credential group') + from wh_session_connection_accumulating_fact + join wh_session_accumulating_fact on wh_session_accumulating_fact.session_id = wh_session_connection_accumulating_fact.session_id + where wh_session_connection_accumulating_fact.session_id = 's1____walter'; + update credential_vault_library set vault_path = '/secrets/tcp/user' where public_id = 'vl______wvl2'; insert into session @@ -41,15 +77,50 @@ begin; values ('s2____walter', 's___1wb-sths', 'h_____wb__01'); insert into session_credential_dynamic - ( session_id, library_id, credential_id, credential_purpose) + ( session_id, library_id, credential_id, credential_purpose) values - ('s2____walter', 'vl______wvl1', null, 'brokered'), - ('s2____walter', 'vl______wvl2', null, 'brokered'), - ('s2____walter', 'vl______wvl3', null, 'brokered'); + ('s2____walter', 'vl______wvl1', null, 'brokered'), + ('s2____walter', 'vl______wvl2', null, 'brokered'), + ('s2____walter', 'vl______wvl3', null, 'brokered'); select is(count(*), 4::bigint) from wh_credential_dimension where organization_id = 'o_____widget'; select is(count(*), 3::bigint) from wh_credential_dimension where organization_id = 'o_____widget' and current_row_indicator = 'Current'; + select is(count(distinct(wh_session_accumulating_fact.credential_group_key)), 2::bigint) + from wh_session_accumulating_fact + where session_id = any(array['s1____walter', 's2____walter']); + + prepare select_updated_credential_dimension as + select credential_library_vault_path::text + from wh_credential_dimension + where current_row_indicator = 'Current' + and credential_library_id = 'vl______wvl2'; + select results_eq( + 'select_updated_credential_dimension', + $$VALUES + ('/secrets/tcp/user')$$ + ); + + select is(count(*), 3::bigint) from wh_credential_dimension where organization_id = 'o_____widget' and current_row_indicator = 'Current'; + + prepare select_second_session_credential_group as + select credential_library_id::text, credential_store_id::text, target_id::text, credential_purpose::text, credential_library_vault_path::text + from wh_credential_dimension as wh_cd + join wh_credential_group_membership as wh_cgm on + wh_cd.key = wh_cgm.credential_key + join wh_credential_group as wh_cg on + wh_cg.key = wh_cgm.credential_group_key + join wh_session_accumulating_fact as wh_saf on + wh_saf.credential_group_key = wh_cg.key + where wh_saf.session_id = 's2____walter'; + select results_eq( + 'select_second_session_credential_group', + $$VALUES + ('vl______wvl1', 'vs_______wvs', 't_________wb', 'brokered','/secrets'), + ('vl______wvl3', 'vs_______wvs', 't_________wb', 'brokered','/secrets/kv/one'), + ('vl______wvl2', 'vs_______wvs', 't_________wb', 'brokered','/secrets/tcp/user')$$ + ); + select * from finish(); rollback;