supabase-tenant-rbac icon indicating copy to clipboard operation
supabase-tenant-rbac copied to clipboard

looks like db_pre_request is not running on sturage upload request

Open matart15 opened this issue 1 year ago • 12 comments

create or replace function get_group() 
    returns text language sql stable 
as $function$
    select current_setting('request.groups', true)
$function$;

this function returns value if i write to database. but return null if is upload image

matart15 avatar Oct 24 '24 21:10 matart15

Oh that's an interesting discovery. How are you calling get_group()? Are you triggering it on insert of one of the storage related tables? I will attempt to replicate in the local dev environment.

point-source avatar Oct 28 '24 21:10 point-source

-- Step 2: Override the db_pre_request function
create or REPLACE FUNCTION public.db_pre_request () returns void language plpgsql stable security definer
set
  search_path = public as $$
declare
    groups jsonb;
    default_group_id uuid;
begin
    -- get current groups from auth.users
    select raw_app_meta_data->'groups' from auth.users into groups where id = auth.uid();
    -- store it in the request object
    perform set_config('request.groups'::text, groups::text, false /* applies to transaction if true, session if false */);

    -- custom logic
    SELECT id FROM public.groups into default_group_id WHERE metadata ->> 'groupName' = 'default_group';
    RAISE LOG 'group_id: %', default_group_id;
    perform set_config('request.default_group_id'::text, default_group_id::text, false /* applies to transaction if true, session if false */);
end;
$$;

create or replace function get_default_group_id() 
  returns uuid language sql stable 
as $function$
  select current_setting('request.default_group_id', true)::uuid
$function$;


CREATE OR REPLACE FUNCTION root_admin_check()
  RETURNS boolean
  AS $$
DECLARE
  user_roles jsonb;
BEGIN
  RAISE LOG 'root_admin_check: current_user %', to_jsonb(current_user);
  -- Extract the roles from the user's 'groups' using the default_group_id function
  user_roles := (auth.jwt() -> 'app_metadata' -> 'groups' -> get_default_group_id()::text -> 'roles');
  RAISE LOG 'root_admin_check: User roles %', to_jsonb(user_roles);

  -- Check if the user_roles is NULL or if 'root_admin' is not in the list of roles
  IF user_roles IS NULL OR NOT (user_roles ? 'root_admin') THEN
    RAISE LOG 'root_admin_check: Not a root admin %', to_jsonb(auth.jwt());
    RETURN FALSE;
  END IF;
  RETURN TRUE;
END;
$$
LANGUAGE plpgsql;


CREATE POLICY authorize_groups_all ON groups AS PERMISSIVE FOR ALL TO authenticated 
using (
  root_admin_check()
);

currently I am doing something like this

It works when i do db actions. But same function fail on Storage.

I use ts to upload images

supabase.storage
      .from("u_profiles")
      .upload(...)

I assume it is trying to write something to "storage"."objects" and failed. because db_pre_request is set to run for public only not storage

matart15 avatar Oct 29 '24 08:10 matart15

I added a few

RAISE LOG

in db_pre_request function and it was not called when i upload image

matart15 avatar Oct 29 '24 08:10 matart15

I asked in the Supabase Discord and got the following reply:

db_pre_request is only for PostgREST calls to the database which storage does not use.

For anyone trying to get around it by using a trigger, the trigger uses the same session so the authorization functions (e.g. user_is_group_member) functions will still fail.

So the following will not work:

-- THIS WON'T WORK!

create table
  files (
    id uuid primary key default gen_random_uuid (),
    group_id uuid not null references groups (id)
    ...
  );

alter table files enable row level security;

create policy "Admins can CRUD files" on files as permissive for all to authenticated
using (user_is_group_member(group_id))
with
  check (user_is_group_member(group_id));

create function handle_storage_update() returns trigger language plpgsql as $$
declare
  file_id uuid;
  result int;
begin
  insert into files (group_id, ...)
    values (uuid_or_null(new.path_tokens[1]), ...)
    returning id into file_id;

  return null;
end;
$$;

create trigger on_file_upload
  after insert on storage.objects
  for each row
  execute procedure private.handle_storage_update();

danielbank avatar Nov 04 '24 20:11 danielbank

@danielbank thank you.

If i understand right, we can't use any kind of caching methond in storage RLS. We should use select with join and it will be run every file in bucket? ( or all files in all bucket )

Should we create storage trigger feature request on supabase repo?

matart15 avatar Nov 05 '24 03:11 matart15

My take on it is that you can only check authentication (who are you?) in Storage calls via auth.uid(). You cannot immediately check authorization as part of any storage CRUD. You'll have to add another layer of abstraction to guarantee authorization to files in storage. In my case, I'm going to have a user action to assign their files to the groups they are a member of. I'm assuming an edge function could also be utilized here

danielbank avatar Nov 05 '24 05:11 danielbank

Thank you since it is not problem of this plugin, closing the issue

created feature request here : https://github.com/orgs/supabase/discussions/30286

matart15 avatar Nov 05 '24 05:11 matart15

Just getting up to speed on this, is the conclusion that even contents of the user's jwt are inaccessible for storage requests with rls?

point-source avatar Nov 05 '24 07:11 point-source

auth.jwt() should work in storage.

GaryAustin1 avatar Nov 05 '24 14:11 GaryAustin1

auth.jwt() should work in storage.

If that's the case then I think this issue should be reopened, @matart15. There definitely seems to be a discrepancy with the following from the README:

The difference comes in when a query is performed. This library uses the postgrest db_pre_request hook to trigger on all incoming api calls to postgrest, fetch the current state of the claims cache (not from the JWT), and inject it into the request context. The included RLS policy convenience functions check this request context first, before falling back to the JWT method.

The experience we are seeing with storage is that the db_pre_request function is not called and the RLS policy convenience functions are not successfully falling back to the JWT claims

danielbank avatar Nov 05 '24 18:11 danielbank

@danielbank I am not an employee of Supabase, but I believe this is the storage code that sets the claims... https://github.com/supabase/storage/blob/6b052d85b9f0c7eb2442a0377b49ca058a5809af/src/internal/database/connection.ts#L231

And unless something broke auth.jwt(), auth.uid() and auth.role() all work for storage requests. BUT PostgREST is not involved with storage. It goes directly to the database, so db_pre_request would never fire. If that function is used to do some sort of security it would not apply to storage operations.

GaryAustin1 avatar Nov 05 '24 18:11 GaryAustin1

I was able to get around the issue by not using the helper functions and instead accessing the auth.jwt() directly. It's less than ideal because you lose the freshness guarantee, but it's better than nothing:

Note in my example below, I am assuming the top-level folder (path_tokens[1]) indicates the group_id.

create policy "Admins can CRUD storage objects in the `files` bucket" on storage.objects as permissive for all to authenticated using (
  coalesce(
    (
      select
        auth.jwt ()->'app_metadata'->'groups'->(path_tokens[1])
    )?'admin',
    false
  )
)
with
  check (
    bucket_id='files'
    and private.uuid_or_null (path_tokens[1]) is not null
    and coalesce(
      (
        select
          auth.jwt ()->'app_metadata'->'groups'->(path_tokens[1])
      )?'admin',
      false
    )
  );

create policy "Members can read storage objects in the `files` bucket" on storage.objects for
select
  to authenticated using (
    coalesce(
      (
        select
          auth.jwt ()->'app_metadata'->'groups'
      )?(path_tokens[1]),
      false
    )
  );

danielbank avatar Nov 10 '24 04:11 danielbank