From 1023c907294b947e6d45987b2593597c74adaf61 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Sun, 21 Jun 2020 13:16:03 -0400 Subject: [PATCH] Enforce auth_account to iam_user plus iam_scope constraints (#137) --- internal/auth/db_test.go | 37 ++++- internal/db/migrations/postgres.gen.go | 156 +++++++++++------- .../db/migrations/postgres/04_auth.up.sql | 39 ----- internal/db/migrations/postgres/06_iam.up.sql | 7 +- .../{04_auth.down.sql => 07_auth.down.sql} | 0 .../db/migrations/postgres/07_auth.up.sql | 68 ++++++++ 6 files changed, 198 insertions(+), 109 deletions(-) delete mode 100644 internal/db/migrations/postgres/04_auth.up.sql rename internal/db/migrations/postgres/{04_auth.down.sql => 07_auth.down.sql} (100%) create mode 100644 internal/db/migrations/postgres/07_auth.up.sql diff --git a/internal/auth/db_test.go b/internal/auth/db_test.go index 1f23d5ab4..98c69584b 100644 --- a/internal/auth/db_test.go +++ b/internal/auth/db_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/watchtower/internal/db" + "github.com/hashicorp/watchtower/internal/iam" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -12,12 +13,15 @@ func TestDB_AuthMethodIDTrigger(t *testing.T) { const ( createTable = ` create table if not exists test_auth_method ( - auth_method_id wt_public_id primary key + auth_method_id wt_public_id primary key, + iam_scope_id wt_public_id not null references iam_scope(public_id) ); ` insert = ` -insert into test_auth_method (auth_method_id) -values ($1); +insert into test_auth_method + (auth_method_id, iam_scope_id) +values + ($1, $2); ` addTriggers = ` create trigger @@ -55,8 +59,10 @@ select count(*) from test_auth_method where auth_method_id = $1; _, err = db.Exec(addTriggers) require.NoError(err) + org, _ := iam.TestScopes(t, conn) + id := "l1Ocw0TpHn800CekIxIXlmQqRDgFDfYl" - _, err = db.Query(insert, id) + _, err = db.Query(insert, id, org.GetPublicId()) require.NoError(err) var count int @@ -75,12 +81,16 @@ func TestDB_AuthAccountIDTrigger(t *testing.T) { const ( createTable = ` create table if not exists test_auth_account ( - auth_account_id wt_public_id primary key + auth_account_id wt_public_id primary key, + auth_method_id wt_public_id not null, + iam_scope_id wt_public_id not null ); ` insert = ` -insert into test_auth_account (auth_account_id) -values ($1); +insert into test_auth_account + (auth_account_id, auth_method_id, iam_scope_id) +values + ($1, $2, $3); ` addTriggers = ` create trigger @@ -94,6 +104,12 @@ select count(*) from auth_account where auth_account_id = $1; ` testTableQuery = ` select count(*) from test_auth_account where auth_account_id = $1; +` + insertAuthMethod = ` +insert into auth_method + (auth_method_id, iam_scope_id) +values + ($1, $2); ` ) @@ -118,8 +134,13 @@ select count(*) from test_auth_account where auth_account_id = $1; _, err = db.Exec(addTriggers) require.NoError(err) + org, _ := iam.TestScopes(t, conn) + meth_id := "31Ocw0TpHn800CekIxIXlmQqRDgFDfYl" + _, err = db.Query(insertAuthMethod, meth_id, org.GetPublicId()) + require.NoError(err) + id := "l1Ocw0TpHn800CekIxIXlmQqRDgFDfYl" - _, err = db.Query(insert, id) + _, err = db.Query(insert, id, meth_id, org.GetPublicId()) require.NoError(err) var count int diff --git a/internal/db/migrations/postgres.gen.go b/internal/db/migrations/postgres.gen.go index 7918451a1..e28232fb4 100644 --- a/internal/db/migrations/postgres.gen.go +++ b/internal/db/migrations/postgres.gen.go @@ -330,66 +330,6 @@ insert on db_test_rental commit; -`), - }, - "migrations/04_auth.down.sql": { - name: "04_auth.down.sql", - bytes: []byte(` -begin; - - drop function insert_auth_account_subtype; - drop function insert_auth_method_subtype; - - drop table auth_account cascade; - drop table auth_method cascade; - -commit; - -`), - }, - "migrations/04_auth.up.sql": { - name: "04_auth.up.sql", - bytes: []byte(` -begin; - - -- base table for auth methods - create table auth_method ( - auth_method_id wt_public_id primary key - ); - - - -- base table for auth accounts - create table auth_account ( - auth_account_id wt_public_id primary key - ); - - - create or replace function - insert_auth_method_subtype() - returns trigger - as $$ - begin - insert into auth_method (auth_method_id) - values - (new.auth_method_id); - return new; - end; - $$ language plpgsql; - - create or replace function - insert_auth_account_subtype() - returns trigger - as $$ - begin - insert into auth_account (auth_account_id) - values - (new.auth_account_id); - return new; - end; - $$ language plpgsql; - -commit; - `), }, "migrations/06_iam.down.sql": { @@ -568,7 +508,12 @@ create table iam_user ( description text, scope_id wt_public_id not null references iam_scope_organization(scope_id) on delete cascade on update cascade, unique(name, scope_id), - disabled boolean not null default false + disabled boolean not null default false, + + -- The order of columns is important for performance. See: + -- https://dba.stackexchange.com/questions/58970/enforcing-constraints-two-tables-away/58972#58972 + -- https://dba.stackexchange.com/questions/27481/is-a-composite-index-also-good-for-queries-on-the-first-field + unique(scope_id, public_id) ); create trigger @@ -646,6 +591,95 @@ insert on iam_group commit; +`), + }, + "migrations/07_auth.down.sql": { + name: "07_auth.down.sql", + bytes: []byte(` +begin; + + drop function insert_auth_account_subtype; + drop function insert_auth_method_subtype; + + drop table auth_account cascade; + drop table auth_method cascade; + +commit; + +`), + }, + "migrations/07_auth.up.sql": { + name: "07_auth.up.sql", + bytes: []byte(` +begin; + + -- Design influenced by: + -- https://dba.stackexchange.com/questions/58970/enforcing-constraints-two-tables-away/58972#58972 + -- + -- iam_scope ←───── auth_method + -- ↑ ↑ + -- iam_user ←───── auth_account + + -- base table for auth methods + create table auth_method ( + auth_method_id wt_public_id primary key, + iam_scope_id wt_public_id not null + references iam_scope(public_id) + on delete cascade + on update cascade, + + -- The order of columns is important for performance. See: + -- https://dba.stackexchange.com/questions/58970/enforcing-constraints-two-tables-away/58972#58972 + -- https://dba.stackexchange.com/questions/27481/is-a-composite-index-also-good-for-queries-on-the-first-field + unique(iam_scope_id, auth_method_id) + ); + + + -- base table for auth accounts + create table auth_account ( + auth_account_id wt_public_id primary key, + auth_method_id wt_public_id not null, + iam_scope_id wt_public_id not null, + iam_user_id wt_public_id, + foreign key (iam_scope_id, auth_method_id) + references auth_method (iam_scope_id, auth_method_id) + on delete cascade + on update cascade, + foreign key (iam_scope_id, iam_user_id) + references iam_user (scope_id, public_id) + on delete set null + on update cascade + ); + + + create or replace function + insert_auth_method_subtype() + returns trigger + as $$ + begin + insert into auth_method + (auth_method_id, iam_scope_id) + values + (new.auth_method_id, new.iam_scope_id); + return new; + end; + $$ language plpgsql; + + create or replace function + insert_auth_account_subtype() + returns trigger + as $$ + begin + insert into auth_account + (auth_account_id, auth_method_id, iam_scope_id) + values + (new.auth_account_id, new.auth_method_id, new.iam_scope_id); + return new; + end; + $$ language plpgsql; + +commit; + `), }, "migrations/10_static_host.down.sql": { diff --git a/internal/db/migrations/postgres/04_auth.up.sql b/internal/db/migrations/postgres/04_auth.up.sql deleted file mode 100644 index 0fc955a8a..000000000 --- a/internal/db/migrations/postgres/04_auth.up.sql +++ /dev/null @@ -1,39 +0,0 @@ -begin; - - -- base table for auth methods - create table auth_method ( - auth_method_id wt_public_id primary key - ); - - - -- base table for auth accounts - create table auth_account ( - auth_account_id wt_public_id primary key - ); - - - create or replace function - insert_auth_method_subtype() - returns trigger - as $$ - begin - insert into auth_method (auth_method_id) - values - (new.auth_method_id); - return new; - end; - $$ language plpgsql; - - create or replace function - insert_auth_account_subtype() - returns trigger - as $$ - begin - insert into auth_account (auth_account_id) - values - (new.auth_account_id); - return new; - end; - $$ language plpgsql; - -commit; diff --git a/internal/db/migrations/postgres/06_iam.up.sql b/internal/db/migrations/postgres/06_iam.up.sql index 9c888c06f..34b7c4346 100644 --- a/internal/db/migrations/postgres/06_iam.up.sql +++ b/internal/db/migrations/postgres/06_iam.up.sql @@ -149,7 +149,12 @@ create table iam_user ( description text, scope_id wt_public_id not null references iam_scope_organization(scope_id) on delete cascade on update cascade, unique(name, scope_id), - disabled boolean not null default false + disabled boolean not null default false, + + -- The order of columns is important for performance. See: + -- https://dba.stackexchange.com/questions/58970/enforcing-constraints-two-tables-away/58972#58972 + -- https://dba.stackexchange.com/questions/27481/is-a-composite-index-also-good-for-queries-on-the-first-field + unique(scope_id, public_id) ); create trigger diff --git a/internal/db/migrations/postgres/04_auth.down.sql b/internal/db/migrations/postgres/07_auth.down.sql similarity index 100% rename from internal/db/migrations/postgres/04_auth.down.sql rename to internal/db/migrations/postgres/07_auth.down.sql diff --git a/internal/db/migrations/postgres/07_auth.up.sql b/internal/db/migrations/postgres/07_auth.up.sql new file mode 100644 index 000000000..bd2655726 --- /dev/null +++ b/internal/db/migrations/postgres/07_auth.up.sql @@ -0,0 +1,68 @@ +begin; + + -- Design influenced by: + -- https://dba.stackexchange.com/questions/58970/enforcing-constraints-two-tables-away/58972#58972 + -- + -- iam_scope ←───── auth_method + -- ↑ ↑ + -- iam_user ←───── auth_account + + -- base table for auth methods + create table auth_method ( + auth_method_id wt_public_id primary key, + iam_scope_id wt_public_id not null + references iam_scope(public_id) + on delete cascade + on update cascade, + + -- The order of columns is important for performance. See: + -- https://dba.stackexchange.com/questions/58970/enforcing-constraints-two-tables-away/58972#58972 + -- https://dba.stackexchange.com/questions/27481/is-a-composite-index-also-good-for-queries-on-the-first-field + unique(iam_scope_id, auth_method_id) + ); + + + -- base table for auth accounts + create table auth_account ( + auth_account_id wt_public_id primary key, + auth_method_id wt_public_id not null, + iam_scope_id wt_public_id not null, + iam_user_id wt_public_id, + foreign key (iam_scope_id, auth_method_id) + references auth_method (iam_scope_id, auth_method_id) + on delete cascade + on update cascade, + foreign key (iam_scope_id, iam_user_id) + references iam_user (scope_id, public_id) + on delete set null + on update cascade + ); + + + create or replace function + insert_auth_method_subtype() + returns trigger + as $$ + begin + insert into auth_method + (auth_method_id, iam_scope_id) + values + (new.auth_method_id, new.iam_scope_id); + return new; + end; + $$ language plpgsql; + + create or replace function + insert_auth_account_subtype() + returns trigger + as $$ + begin + insert into auth_account + (auth_account_id, auth_method_id, iam_scope_id) + values + (new.auth_account_id, new.auth_method_id, new.iam_scope_id); + return new; + end; + $$ language plpgsql; + +commit;