From 4195697b54e71fc5a41fcea72f71010a36f83973 Mon Sep 17 00:00:00 2001 From: giancarlo Date: Sat, 20 Apr 2024 19:37:39 +0800 Subject: [PATCH] Add new tests and update schema.sql and account permissions New test files for database functionalities like transfer of ownership, schema conditions, and updating roles have been added. Changes have also been made in the schema.sql file for checking the role hierarchy levels and updating rules for permissions. Modifications in account permissions test have also been performed for more accuracy. --- .../migrations/20221215192558_schema.sql | 100 ++++++++++++++++-- .../tests/database/00000-makerkit-helpers.sql | 16 +++ .../database/account-permissions.test.sql | 5 + .../tests/database/billing-orders.test.sql | 10 ++ .../database/billing-subscriptions.test.sql | 10 ++ .../tests/database/invitations.test.sql | 63 +++++++++++ .../tests/database/remove-membership.test.sql | 10 ++ .../tests/database/schema-conditions.test.sql | 57 ++++++++++ .../supabase/tests/database/schema.test.sql | 18 +++- .../database/transfer-ownership.test.sql | 10 ++ .../tests/database/update-role.test.sql | 10 ++ .../services/account-invitations.service.ts | 2 +- 12 files changed, 300 insertions(+), 11 deletions(-) create mode 100644 apps/web/supabase/tests/database/billing-orders.test.sql create mode 100644 apps/web/supabase/tests/database/billing-subscriptions.test.sql create mode 100644 apps/web/supabase/tests/database/invitations.test.sql create mode 100644 apps/web/supabase/tests/database/remove-membership.test.sql create mode 100644 apps/web/supabase/tests/database/schema-conditions.test.sql create mode 100644 apps/web/supabase/tests/database/transfer-ownership.test.sql create mode 100644 apps/web/supabase/tests/database/update-role.test.sql diff --git a/apps/web/supabase/migrations/20221215192558_schema.sql b/apps/web/supabase/migrations/20221215192558_schema.sql index e7a696a30..9258004d0 100644 --- a/apps/web/supabase/migrations/20221215192558_schema.sql +++ b/apps/web/supabase/migrations/20221215192558_schema.sql @@ -631,8 +631,8 @@ create or replace function public.has_role_on_account(account_id where membership.user_id = auth.uid() and membership.account_id = has_role_on_account.account_id - and(membership.account_role = has_role_on_account.account_role - or has_role_on_account.account_role is null)); + and((membership.account_role = has_role_on_account.account_role + or has_role_on_account.account_role is null))); $$; grant execute on function public.has_role_on_account(uuid, varchar) @@ -840,6 +840,7 @@ language plpgsql; grant execute on function public.has_permission(uuid, uuid, public.app_permissions) to authenticated, service_role; +-- Function: Check if a user has a more elevated role than the target role create or replace function public.has_more_elevated_role(target_user_id uuid, target_account_id uuid, role_name varchar) @@ -850,6 +851,7 @@ declare user_role_hierarchy_level int; target_role_hierarchy_level int; begin + -- Check if the user is the primary owner of the account select exists ( select @@ -864,9 +866,9 @@ begin -- perform any action if is_primary_owner then return true; - end if; + -- Get the hierarchy level of the user's role within the account select hierarchy_level into user_role_hierarchy_level from @@ -881,16 +883,26 @@ begin account_id = target_account_id and target_user_id = user_id); + if user_role_hierarchy_level is null then + return false; + end if; + + -- Get the hierarchy level of the target role select hierarchy_level into target_role_hierarchy_level from public.roles where name = role_name - and account_id = target_account_id or account_id is null; + and (account_id = target_account_id or account_id is null); + + -- If the target role does not exist, the user cannot perform the action + if target_role_hierarchy_level is null then + return false; + end if; + -- If the user's role is higher than the target role, they can perform -- the action - return user_role_hierarchy_level < target_role_hierarchy_level; end; @@ -901,6 +913,77 @@ language plpgsql; grant execute on function public.has_more_elevated_role(uuid, uuid, varchar) to authenticated, service_role; +-- Function: Check if a user has the same role hierarchy level as the target role +create or replace function + public.has_same_role_hierarchy_level(target_user_id uuid, + target_account_id uuid, role_name varchar) + returns boolean + as $$ +declare + is_primary_owner boolean; + user_role_hierarchy_level int; + target_role_hierarchy_level int; +begin + -- Check if the user is the primary owner of the account + select + exists ( + select + 1 + from + public.accounts + where + id = target_account_id + and primary_owner_user_id = target_user_id) into is_primary_owner; + + -- If the user is the primary owner, they have the highest role and can perform any action + if is_primary_owner then + return true; + end if; + + -- Get the hierarchy level of the user's role within the account + select + hierarchy_level into user_role_hierarchy_level + from + public.roles + where + name =( + select + account_role + from + public.accounts_memberships + where + account_id = target_account_id + and target_user_id = user_id); + + if user_role_hierarchy_level is null then + return false; + end if; + + -- Get the hierarchy level of the target role + select + hierarchy_level into target_role_hierarchy_level + from + public.roles + where + name = role_name + and (account_id = target_account_id or account_id is null); + + -- If the target role does not exist, the user cannot perform the action + if target_role_hierarchy_level is null then + return false; + end if; + + -- check the user's role hierarchy level is the same as the target role + return user_role_hierarchy_level = target_role_hierarchy_level; + +end; + +$$ +language plpgsql; + +grant execute on function public.has_same_role_hierarchy_level(uuid, uuid, + varchar) to authenticated, service_role; + -- Enable RLS on the role_permissions table alter table public.role_permissions enable row level security; @@ -910,7 +993,6 @@ create policy role_permissions_read on public.role_permissions for select to authenticated using (true); - /* * ------------------------------------------------------- * Section: Invitations @@ -948,7 +1030,7 @@ comment on column public.invitations.email is 'The email of the user being invit -- Open up access to invitations table for authenticated users and -- service_role grant select, insert, update, delete on table public.invitations to - service_role; + authenticated, service_role; -- Enable RLS on the invitations table alter table public.invitations enable row level security; @@ -996,8 +1078,8 @@ create policy invitations_create_self on public.invitations for insert to authenticated with check ( public.is_set('enable_team_accounts') and - public.has_permission(auth.uid(), account_id, 'invites.manage' ::app_permissions) - and public.has_more_elevated_role( + public.has_permission(auth.uid(), account_id, 'invites.manage'::app_permissions) + and public.has_same_role_hierarchy_level( auth.uid(), account_id, role)); -- UPDATE: Users can update invitations to users of an account they are diff --git a/apps/web/supabase/tests/database/00000-makerkit-helpers.sql b/apps/web/supabase/tests/database/00000-makerkit-helpers.sql index b582a41bf..981e1775f 100644 --- a/apps/web/supabase/tests/database/00000-makerkit-helpers.sql +++ b/apps/web/supabase/tests/database/00000-makerkit-helpers.sql @@ -10,6 +10,22 @@ alter default PRIVILEGES in schema makerkit revoke execute on FUNCTIONS from pub alter default PRIVILEGES in schema makerkit grant execute on FUNCTIONS to anon, authenticated, service_role; +create or replace function makerkit.set_identifier( + identifier text, + user_email text +) + returns text + as $$ +begin + update auth.users set raw_user_meta_data = jsonb_build_object('test_identifier', identifier) + where email = user_email; + + return identifier; + +end; + +$$ language PLPGSQL; + create or replace function makerkit.get_account_by_slug( account_slug text ) diff --git a/apps/web/supabase/tests/database/account-permissions.test.sql b/apps/web/supabase/tests/database/account-permissions.test.sql index d8de8fd24..5543d644f 100644 --- a/apps/web/supabase/tests/database/account-permissions.test.sql +++ b/apps/web/supabase/tests/database/account-permissions.test.sql @@ -77,6 +77,11 @@ update public.accounts_memberships where account_id = makerkit.get_account_id_by_slug('test') and user_id = makerkit.get_user_id('test1@test.com'); +set local role postgres; + +-- insert permissions for the custom role +insert into public.role_permissions (role, permission) values ('custom-role', 'members.manage'); + select tests.authenticate_as('test1'); -- the custom role does not have permissions to manage billing diff --git a/apps/web/supabase/tests/database/billing-orders.test.sql b/apps/web/supabase/tests/database/billing-orders.test.sql new file mode 100644 index 000000000..cfd705036 --- /dev/null +++ b/apps/web/supabase/tests/database/billing-orders.test.sql @@ -0,0 +1,10 @@ +begin; +create extension "basejump-supabase_test_helpers" version '0.0.6'; + +select no_plan(); + +-- test + +select * from finish(); + +rollback; \ No newline at end of file diff --git a/apps/web/supabase/tests/database/billing-subscriptions.test.sql b/apps/web/supabase/tests/database/billing-subscriptions.test.sql new file mode 100644 index 000000000..cfd705036 --- /dev/null +++ b/apps/web/supabase/tests/database/billing-subscriptions.test.sql @@ -0,0 +1,10 @@ +begin; +create extension "basejump-supabase_test_helpers" version '0.0.6'; + +select no_plan(); + +-- test + +select * from finish(); + +rollback; \ No newline at end of file diff --git a/apps/web/supabase/tests/database/invitations.test.sql b/apps/web/supabase/tests/database/invitations.test.sql new file mode 100644 index 000000000..ea15000d0 --- /dev/null +++ b/apps/web/supabase/tests/database/invitations.test.sql @@ -0,0 +1,63 @@ +begin; +create extension "basejump-supabase_test_helpers" version '0.0.6'; + +select no_plan(); + +-- test + +select makerkit.set_identifier('test', 'test@makerkit.dev'); +select makerkit.set_identifier('member', 'member@makerkit.dev'); +select makerkit.set_identifier('custom', 'custom@makerkit.dev'); + +select tests.authenticate_as('test'); + +select lives_ok( + $$ insert into public.invitations (email, invited_by, account_id, role, invite_token) values ('invite1@makerkit.dev', auth.uid(), makerkit.get_account_id_by_slug('makerkit'), 'member', gen_random_uuid()); $$, +'owner should be able to create invitations' +); + +-- check two invitations to the same email/account are not allowed +select throws_ok( + $$ insert into public.invitations (email, invited_by, account_id, role, invite_token) values ('invite1@makerkit.dev', auth.uid(), makerkit.get_account_id_by_slug('makerkit'), 'member', gen_random_uuid()) $$, + 'duplicate key value violates unique constraint "invitations_email_account_id_key"' +); + +select tests.authenticate_as('member'); + +-- check a member cannot invite members with higher roles +select throws_ok( + $$ insert into public.invitations (email, invited_by, account_id, role, invite_token) values ('invite2@makerkit.dev', auth.uid(), makerkit.get_account_id_by_slug('makerkit'), 'owner', gen_random_uuid()) $$, + 'new row violates row-level security policy for table "invitations"' +); + +-- check a member can invite members with the same or lower roles +select lives_ok( + $$ insert into public.invitations (email, invited_by, account_id, role, invite_token) values ('invite2@makerkit.dev', auth.uid(), makerkit.get_account_id_by_slug('makerkit'), 'member', gen_random_uuid()) $$, + 'member should be able to create invitations for members or lower roles' +); + +-- authenticate_as the custom role +select tests.authenticate_as('custom'); + +-- it will fail because the custom role does not have the invites.manage permission +select throws_ok( + $$ insert into public.invitations (email, invited_by, account_id, role, invite_token) values ('invite3@makerkit.dev', auth.uid(), makerkit.get_account_id_by_slug('makerkit'), 'custom-role', gen_random_uuid()) $$, + 'new row violates row-level security policy for table "invitations"' +); + +set local role postgres; + +-- add permissions to invite members to the custom role +insert into public.role_permissions (role, permission) values ('custom-role', 'invites.manage'); + +-- authenticate_as the custom role +select tests.authenticate_as('custom'); + +select lives_ok( + $$ insert into public.invitations (email, invited_by, account_id, role, invite_token) values ('invite3@makerkit.dev', auth.uid(), makerkit.get_account_id_by_slug('makerkit'), 'custom-role', gen_random_uuid()) $$, + 'custom role should be able to create invitations' +); + +select * from finish(); + +rollback; \ No newline at end of file diff --git a/apps/web/supabase/tests/database/remove-membership.test.sql b/apps/web/supabase/tests/database/remove-membership.test.sql new file mode 100644 index 000000000..cfd705036 --- /dev/null +++ b/apps/web/supabase/tests/database/remove-membership.test.sql @@ -0,0 +1,10 @@ +begin; +create extension "basejump-supabase_test_helpers" version '0.0.6'; + +select no_plan(); + +-- test + +select * from finish(); + +rollback; \ No newline at end of file diff --git a/apps/web/supabase/tests/database/schema-conditions.test.sql b/apps/web/supabase/tests/database/schema-conditions.test.sql new file mode 100644 index 000000000..bb2ffd766 --- /dev/null +++ b/apps/web/supabase/tests/database/schema-conditions.test.sql @@ -0,0 +1,57 @@ +begin; + +create extension "basejump-supabase_test_helpers" version '0.0.6'; + +select + no_plan(); + +CREATE OR REPLACE FUNCTION check_schema_conditions() +RETURNS void AS +$$ +DECLARE + _table RECORD; + _column RECORD; + columnCheckCount INTEGER; +BEGIN + FOR _table IN (SELECT tablename FROM pg_tables WHERE schemaname = 'public') + LOOP + -- 1. Check if every table has RLS enabled + IF ( + SELECT relrowsecurity FROM pg_class + INNER JOIN pg_namespace n ON n.oid = pg_class.relnamespace + WHERE n.nspname = 'public' AND relname = _table.tablename + ) IS FALSE THEN + RAISE EXCEPTION 'Table "%" does not have RLS enabled.', _table.tablename; + END IF; + + -- 2. Check that every text column in the current table has a constraint + FOR _column IN (SELECT column_name FROM information_schema.columns WHERE table_schema = 'public' AND table_name = _table.tablename AND data_type = 'text') + LOOP + SELECT COUNT(*) + INTO columnCheckCount + FROM information_schema.constraint_column_usage + WHERE table_schema = 'public' AND table_name = _table.tablename AND column_name = _column.column_name; + + IF columnCheckCount = 0 THEN + RAISE NOTICE 'Text column "%.%" does not have a constraint + .', + _table.tablename, _column.column_name; + END IF; + END LOOP; + END LOOP; + + RAISE NOTICE 'Schema check completed.'; +END +$$ LANGUAGE plpgsql; + +select lives_ok($$ + select + check_schema_conditions(); +$$, 'check_schema_conditions()'); + +select + * +from + finish(); + +rollback; diff --git a/apps/web/supabase/tests/database/schema.test.sql b/apps/web/supabase/tests/database/schema.test.sql index f706dee31..b7b0fddce 100644 --- a/apps/web/supabase/tests/database/schema.test.sql +++ b/apps/web/supabase/tests/database/schema.test.sql @@ -8,9 +8,25 @@ select has_table('public', 'accounts', 'Makerkit accounts table should exist'); select has_table('public', 'accounts_memberships', 'Makerkit account_users table should exist'); select has_table('public', 'invitations', 'Makerkit invitations table should exist'); select has_table('public', 'billing_customers', 'Makerkit billing_customers table should exist'); -select has_table('public', 'subscriptions', 'Makerkit billing_subscriptions table should exist'); +select has_table('public', 'subscriptions', 'Makerkit subscriptions table should exist'); +select has_table('public', 'subscription_items', 'Makerkit subscription_items table should exist'); +select has_table('public', 'orders', 'Makerkit orders table should exist'); +select has_table('public', 'order_items', 'Makerkit order_items table should exist'); +select has_table('public', 'roles', 'Makerkit roles table should exist'); select has_table('public', 'role_permissions', 'Makerkit roles_permissions table should exist'); +select tests.rls_enabled('public', 'config'); +select tests.rls_enabled('public', 'accounts'); +select tests.rls_enabled('public', 'accounts_memberships'); +select tests.rls_enabled('public', 'invitations'); +select tests.rls_enabled('public', 'billing_customers'); +select tests.rls_enabled('public', 'subscriptions'); +select tests.rls_enabled('public', 'subscription_items'); +select tests.rls_enabled('public', 'orders'); +select tests.rls_enabled('public', 'order_items'); +select tests.rls_enabled('public', 'roles'); +select tests.rls_enabled('public', 'role_permissions'); + SELECT schema_privs_are('public', 'anon', Array [NULL], 'Anon should not have access to public schema'); -- set the role to anonymous for verifying access tests diff --git a/apps/web/supabase/tests/database/transfer-ownership.test.sql b/apps/web/supabase/tests/database/transfer-ownership.test.sql new file mode 100644 index 000000000..cfd705036 --- /dev/null +++ b/apps/web/supabase/tests/database/transfer-ownership.test.sql @@ -0,0 +1,10 @@ +begin; +create extension "basejump-supabase_test_helpers" version '0.0.6'; + +select no_plan(); + +-- test + +select * from finish(); + +rollback; \ No newline at end of file diff --git a/apps/web/supabase/tests/database/update-role.test.sql b/apps/web/supabase/tests/database/update-role.test.sql new file mode 100644 index 000000000..cfd705036 --- /dev/null +++ b/apps/web/supabase/tests/database/update-role.test.sql @@ -0,0 +1,10 @@ +begin; +create extension "basejump-supabase_test_helpers" version '0.0.6'; + +select no_plan(); + +-- test + +select * from finish(); + +rollback; \ No newline at end of file diff --git a/packages/features/team-accounts/src/server/services/account-invitations.service.ts b/packages/features/team-accounts/src/server/services/account-invitations.service.ts index 62f9c6cc8..6ab580c7a 100644 --- a/packages/features/team-accounts/src/server/services/account-invitations.service.ts +++ b/packages/features/team-accounts/src/server/services/account-invitations.service.ts @@ -30,7 +30,7 @@ export class AccountInvitationsService { ...params, }; - logger.info(ctx, 'Removing invitation'); + logger.info(ctx, 'Removing invitation...'); const { data, error } = await this.client .from('invitations')