Update test cases and improve account actioning
This commit refactors Supabase test cases to reflect the updated account actioning mechanism. The "makerkit.get_user_id" function calls were replaced with the new "tests.get_supabase_uid" function, aligning with the testing structure update. It also introduces new policies which further refine user role actions with more precise checks, replacing the old 'delete' policy with the more comprehensive 'can_action_account_member' function. New test cases for updating memberships and deleting memberships have also been added.
This commit is contained in:
@@ -686,10 +686,11 @@ create policy roles_read on public.roles
|
||||
or public.has_role_on_account(account_id)
|
||||
);
|
||||
|
||||
|
||||
-- Function to check if a user can perform management actions on an account member
|
||||
create or replace function
|
||||
public.can_action_account_member(target_team_account_id uuid,
|
||||
user_id uuid)
|
||||
target_user_id uuid)
|
||||
returns boolean
|
||||
as $$
|
||||
declare
|
||||
@@ -697,64 +698,83 @@ declare
|
||||
target_user_hierarchy_level int;
|
||||
current_user_hierarchy_level int;
|
||||
is_account_owner boolean;
|
||||
target_user_role varchar(50);
|
||||
begin
|
||||
select public.is_account_owner(target_team_account_id) into is_account_owner;
|
||||
|
||||
-- a primary owner of the account can never be actioned
|
||||
if is_account_owner and user_id = auth.uid() then
|
||||
raise exception 'You cannot action the primary owner of the account';
|
||||
if target_user_id = auth.uid() then
|
||||
raise exception 'You cannot update your own account membership with this function';
|
||||
end if;
|
||||
|
||||
-- an account owner can action any member of the account
|
||||
if is_account_owner then
|
||||
return true;
|
||||
if public.is_account_owner(target_team_account_id) then
|
||||
return true;
|
||||
end if;
|
||||
|
||||
-- check the target user is the primary owner of the account
|
||||
select
|
||||
exists (
|
||||
select
|
||||
1
|
||||
from
|
||||
public.accounts
|
||||
where
|
||||
id = target_team_account_id
|
||||
and primary_owner_user_id = target_user_id) into is_account_owner;
|
||||
|
||||
|
||||
if is_account_owner then
|
||||
raise exception 'The primary account owner cannot be actioned';
|
||||
end if;
|
||||
|
||||
|
||||
-- validate the auth user has the required permission on the account
|
||||
-- to manage members of the account
|
||||
-- to manage members of the account
|
||||
select
|
||||
public.has_permission(auth.uid(), target_team_account_id,
|
||||
'members.manage'::app_permissions) into
|
||||
permission_granted;
|
||||
public.has_permission(auth.uid(), target_team_account_id,
|
||||
'members.manage'::app_permissions) into
|
||||
permission_granted;
|
||||
|
||||
-- if the user does not have the required permission, raise an exception
|
||||
if not permission_granted then
|
||||
raise exception 'You do not have permission to action a member from this account';
|
||||
|
||||
end if;
|
||||
-- users cannot remove themselves from the account with this function
|
||||
if can_action_account_member.user_id = auth.uid() then
|
||||
raise exception 'You cannot update your own account membership with this function';
|
||||
raise exception 'You do not have permission to action a member from this account';
|
||||
end if;
|
||||
|
||||
-- get the role of the target user
|
||||
select
|
||||
hierarchy_level into target_user_hierarchy_level
|
||||
am.account_role,
|
||||
r.hierarchy_level
|
||||
from
|
||||
public.roles
|
||||
public.accounts_memberships as am
|
||||
join
|
||||
public.roles as r on am.account_role = r.name
|
||||
where
|
||||
name = target_user_role;
|
||||
am.account_id = target_team_account_id
|
||||
and am.user_id = target_user_id
|
||||
into target_user_role, target_user_hierarchy_level;
|
||||
|
||||
-- get the hierarchy level of the current user
|
||||
select
|
||||
hierarchy_level into current_user_hierarchy_level
|
||||
r.hierarchy_level into current_user_hierarchy_level
|
||||
from
|
||||
public.roles
|
||||
public.roles as r
|
||||
join
|
||||
public.accounts_memberships as am on r.name = am.account_role
|
||||
where
|
||||
name =(
|
||||
select
|
||||
account_role
|
||||
from
|
||||
public.accounts_memberships
|
||||
where
|
||||
account_id = target_team_account_id
|
||||
and user_id = auth.uid());
|
||||
|
||||
-- check if the current user has a higher hierarchy level than the
|
||||
-- target user. Lower hierarchy levels have higher permissions than higher hierarchy levels
|
||||
if current_user_hierarchy_level <= target_user_hierarchy_level then
|
||||
raise exception 'You do not have permission to action this user';
|
||||
am.account_id = target_team_account_id
|
||||
and am.user_id = auth.uid();
|
||||
|
||||
if target_user_role is null then
|
||||
raise exception 'The target user does not have a role on the account';
|
||||
end if;
|
||||
|
||||
if current_user_hierarchy_level is null then
|
||||
raise exception 'The current user does not have a role on the account';
|
||||
end if;
|
||||
|
||||
-- check the current user has a higher role than the target user
|
||||
if current_user_hierarchy_level >= target_user_hierarchy_level then
|
||||
raise exception 'You do not have permission to action a member from this account';
|
||||
end if;
|
||||
|
||||
-- return true if the user has the required permission
|
||||
return true;
|
||||
|
||||
end;
|
||||
@@ -782,14 +802,16 @@ create policy accounts_read_team on public.accounts
|
||||
for select to authenticated
|
||||
using (has_role_on_account(id));
|
||||
|
||||
-- DELETE: Users can remove themselves from an account
|
||||
-- DELETE: Users can remove themselves from an account unless they are the primary owner
|
||||
create policy accounts_memberships_delete_self on public.accounts_memberships
|
||||
for delete to authenticated
|
||||
for delete
|
||||
to authenticated
|
||||
using (user_id = auth.uid());
|
||||
|
||||
-- DELETE: Users with the required role can remove members from an account
|
||||
create policy accounts_memberships_delete on public.accounts_memberships
|
||||
for delete to authenticated
|
||||
for delete
|
||||
to authenticated
|
||||
using (public.can_action_account_member(account_id, user_id));
|
||||
|
||||
-- SELECT (public.accounts): Team members can read accounts of the team
|
||||
|
||||
@@ -15,6 +15,8 @@ create or replace function makerkit.set_identifier(
|
||||
user_email text
|
||||
)
|
||||
returns text
|
||||
security definer
|
||||
set search_path = auth, pg_temp
|
||||
as $$
|
||||
begin
|
||||
update auth.users set raw_user_meta_data = jsonb_build_object('test_identifier', identifier)
|
||||
@@ -65,25 +67,6 @@ end;
|
||||
|
||||
$$ language PLPGSQL;
|
||||
|
||||
create or replace function makerkit.get_user_id(
|
||||
user_email text
|
||||
)
|
||||
returns uuid
|
||||
as $$
|
||||
begin
|
||||
|
||||
return
|
||||
(select
|
||||
primary_owner_user_id
|
||||
from
|
||||
accounts
|
||||
where
|
||||
email = user_email);
|
||||
|
||||
end;
|
||||
|
||||
$$ language PLPGSQL;
|
||||
|
||||
begin;
|
||||
|
||||
select plan(1);
|
||||
|
||||
@@ -75,7 +75,7 @@ select throws_ok(
|
||||
update public.accounts_memberships
|
||||
set account_role = 'custom-role'
|
||||
where account_id = makerkit.get_account_id_by_slug('test')
|
||||
and user_id = makerkit.get_user_id('test1@test.com');
|
||||
and user_id = tests.get_supabase_uid('test1');
|
||||
|
||||
set local role postgres;
|
||||
|
||||
|
||||
94
apps/web/supabase/tests/database/delete-membership.test.sql
Normal file
94
apps/web/supabase/tests/database/delete-membership.test.sql
Normal file
@@ -0,0 +1,94 @@
|
||||
begin;
|
||||
create extension "basejump-supabase_test_helpers" version '0.0.6';
|
||||
|
||||
select no_plan();
|
||||
|
||||
select makerkit.set_identifier('primary_owner', 'test@makerkit.dev');
|
||||
select makerkit.set_identifier('owner', 'owner@makerkit.dev');
|
||||
select makerkit.set_identifier('member', 'member@makerkit.dev');
|
||||
select makerkit.set_identifier('custom', 'custom@makerkit.dev');
|
||||
|
||||
-- another user not in the team
|
||||
select tests.create_supabase_user('test', 'test@supabase.com');
|
||||
|
||||
-- an owner cannot remove the primary owner
|
||||
select tests.authenticate_as('owner');
|
||||
|
||||
select throws_ok(
|
||||
$$ delete from public.accounts_memberships
|
||||
where account_id = makerkit.get_account_id_by_slug('makerkit')
|
||||
and user_id = '31a03e74-1639-45b6-bfa7-77447f1a4762' $$,
|
||||
'The primary account owner cannot be actioned'
|
||||
);
|
||||
|
||||
-- an owner can remove accounts with lower roles
|
||||
select lives_ok(
|
||||
$$ delete from public.accounts_memberships
|
||||
where account_id = makerkit.get_account_id_by_slug('makerkit')
|
||||
and user_id = '6b83d656-e4ab-48e3-a062-c0c54a427368' $$,
|
||||
'Owner should be able to remove a member'
|
||||
);
|
||||
|
||||
-- a member cannot remove a member with a higher role
|
||||
select tests.authenticate_as('member');
|
||||
|
||||
-- delete a membership record where the user is a higher role than the current user
|
||||
select throws_ok(
|
||||
$$ delete from public.accounts_memberships
|
||||
where account_id = makerkit.get_account_id_by_slug('makerkit')
|
||||
and user_id = '5c064f1b-78ee-4e1c-ac3b-e99aa97c99bf' $$,
|
||||
'You do not have permission to action a member from this account'
|
||||
);
|
||||
|
||||
-- an primary_owner cannot remove themselves
|
||||
select tests.authenticate_as('primary_owner');
|
||||
|
||||
select throws_ok(
|
||||
$$ delete from public.accounts_memberships
|
||||
where account_id = makerkit.get_account_id_by_slug('makerkit')
|
||||
and user_id = '31a03e74-1639-45b6-bfa7-77447f1a4762' $$,
|
||||
'The primary account owner cannot be removed from the account membership list'
|
||||
);
|
||||
|
||||
-- a primary_owner can remove another member
|
||||
select lives_ok(
|
||||
$$ delete from public.accounts_memberships
|
||||
where account_id = makerkit.get_account_id_by_slug('makerkit')
|
||||
and user_id = 'b73eb03e-fb7a-424d-84ff-18e2791ce0b4'; $$,
|
||||
'Primary owner should be able to remove another member'
|
||||
);
|
||||
|
||||
-- foreigners
|
||||
|
||||
-- a user not in the account cannot remove a member
|
||||
|
||||
select tests.authenticate_as('test');
|
||||
|
||||
select throws_ok(
|
||||
$$ delete from public.accounts_memberships
|
||||
where account_id = '5deaa894-2094-4da3-b4fd-1fada0809d1c'
|
||||
and user_id = tests.get_supabase_uid('owner'); $$,
|
||||
'You do not have permission to action a member from this account'
|
||||
);
|
||||
|
||||
select tests.authenticate_as('owner');
|
||||
|
||||
select isnt_empty(
|
||||
$$ select 1 from public.accounts_memberships
|
||||
where account_id = '5deaa894-2094-4da3-b4fd-1fada0809d1c'
|
||||
and user_id = tests.get_supabase_uid('owner'); $$,
|
||||
'Foreigners should not be able to remove members');
|
||||
|
||||
select tests.authenticate_as('test');
|
||||
|
||||
-- a user not in the account cannot remove themselves
|
||||
select throws_ok(
|
||||
$$ delete from public.accounts_memberships
|
||||
where account_id = makerkit.get_account_id_by_slug('makerkit')
|
||||
and user_id = auth.uid(); $$,
|
||||
'You do not have permission to action a member from this account'
|
||||
);
|
||||
|
||||
select * from finish();
|
||||
|
||||
rollback;
|
||||
@@ -1,10 +0,0 @@
|
||||
begin;
|
||||
create extension "basejump-supabase_test_helpers" version '0.0.6';
|
||||
|
||||
select no_plan();
|
||||
|
||||
-- test
|
||||
|
||||
select * from finish();
|
||||
|
||||
rollback;
|
||||
@@ -18,7 +18,7 @@ select tests.authenticate_as('primary_owner');
|
||||
select throws_ok(
|
||||
$$ select public.transfer_team_account_ownership(
|
||||
makerkit.get_account_id_by_slug('makerkit'),
|
||||
makerkit.get_user_id('custom@makerkit.dev')
|
||||
tests.get_supabase_uid('custom')
|
||||
) $$,
|
||||
'permission denied for function transfer_team_account_ownership'
|
||||
);
|
||||
@@ -29,7 +29,7 @@ set local role service_role;
|
||||
select throws_ok(
|
||||
$$ select public.transfer_team_account_ownership(
|
||||
makerkit.get_account_id_by_slug('makerkit'),
|
||||
makerkit.get_user_id('test@supabase.com')
|
||||
tests.get_supabase_uid('test')
|
||||
) $$,
|
||||
'The new owner must be a member of the account'
|
||||
);
|
||||
@@ -38,14 +38,14 @@ select throws_ok(
|
||||
select lives_ok(
|
||||
$$ select public.transfer_team_account_ownership(
|
||||
makerkit.get_account_id_by_slug('makerkit'),
|
||||
makerkit.get_user_id('owner@makerkit.dev')
|
||||
tests.get_supabase_uid('owner')
|
||||
) $$
|
||||
);
|
||||
|
||||
-- check the account owner has been updated
|
||||
select row_eq(
|
||||
$$ select primary_owner_user_id from public.accounts where id = makerkit.get_account_id_by_slug('makerkit') $$,
|
||||
row(makerkit.get_user_id('owner@makerkit.dev')),
|
||||
row(tests.get_supabase_uid('owner')),
|
||||
'The account owner should be updated'
|
||||
);
|
||||
|
||||
@@ -54,7 +54,7 @@ select row_eq(
|
||||
select lives_ok(
|
||||
$$ select public.transfer_team_account_ownership(
|
||||
makerkit.get_account_id_by_slug('makerkit'),
|
||||
makerkit.get_user_id('member@makerkit.dev')
|
||||
tests.get_supabase_uid('member')
|
||||
) $$
|
||||
);
|
||||
|
||||
@@ -62,20 +62,12 @@ select lives_ok(
|
||||
select row_eq(
|
||||
$$ select account_role from public.accounts_memberships
|
||||
where account_id = makerkit.get_account_id_by_slug('makerkit')
|
||||
and user_id = makerkit.get_user_id('member@makerkit.dev');
|
||||
and user_id = tests.get_supabase_uid('member');
|
||||
$$,
|
||||
row('owner'::varchar),
|
||||
'The account owner should be updated'
|
||||
);
|
||||
|
||||
-- rollback
|
||||
select lives_ok(
|
||||
$$ select public.transfer_team_account_ownership(
|
||||
makerkit.get_account_id_by_slug('makerkit'),
|
||||
makerkit.get_user_id('test@makerkit.dev')
|
||||
) $$
|
||||
);
|
||||
|
||||
select * from finish();
|
||||
|
||||
rollback;
|
||||
27
apps/web/supabase/tests/database/update-membership.test.sql
Normal file
27
apps/web/supabase/tests/database/update-membership.test.sql
Normal file
@@ -0,0 +1,27 @@
|
||||
begin;
|
||||
create extension "basejump-supabase_test_helpers" version '0.0.6';
|
||||
|
||||
select no_plan();
|
||||
|
||||
select makerkit.set_identifier('primary_owner', 'test@makerkit.dev');
|
||||
select makerkit.set_identifier('owner', 'owner@makerkit.dev');
|
||||
select makerkit.set_identifier('member', 'member@makerkit.dev');
|
||||
select makerkit.set_identifier('custom', 'custom@makerkit.dev');
|
||||
|
||||
-- another user not in the team
|
||||
select tests.create_supabase_user('test', 'test@supabase.com');
|
||||
|
||||
select tests.authenticate_as('member');
|
||||
|
||||
-- run an update query
|
||||
update public.accounts_memberships set account_role = 'owner' where user_id = auth.uid() and account_id = makerkit.get_account_id_by_slug('makerkit');
|
||||
|
||||
select row_eq(
|
||||
$$ select account_role from public.accounts_memberships where user_id = auth.uid() and account_id = makerkit.get_account_id_by_slug('makerkit'); $$,
|
||||
row('member'::varchar),
|
||||
'Updates fail silently to any field of the accounts_membership table'
|
||||
);
|
||||
|
||||
select * from finish();
|
||||
|
||||
rollback;
|
||||
@@ -1,10 +0,0 @@
|
||||
begin;
|
||||
create extension "basejump-supabase_test_helpers" version '0.0.6';
|
||||
|
||||
select no_plan();
|
||||
|
||||
-- test
|
||||
|
||||
select * from finish();
|
||||
|
||||
rollback;
|
||||
Reference in New Issue
Block a user