Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

looks like db_pre_request is not running on sturage upload request #34

Open
matart15 opened this issue Oct 24, 2024 · 12 comments
Open

looks like db_pre_request is not running on sturage upload request #34

matart15 opened this issue Oct 24, 2024 · 12 comments

Comments

@matart15
Copy link

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

@point-source
Copy link
Owner

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.

@matart15
Copy link
Author

-- 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
Copy link
Author

I added a few

RAISE LOG

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

@danielbank
Copy link

danielbank commented Nov 4, 2024

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();

@matart15
Copy link
Author

matart15 commented Nov 5, 2024

@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?

@danielbank
Copy link

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

@matart15
Copy link
Author

matart15 commented Nov 5, 2024

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 matart15 closed this as completed Nov 5, 2024
@point-source
Copy link
Owner

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?

@GaryAustin1
Copy link

auth.jwt() should work in storage.

@danielbank
Copy link

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

@GaryAustin1
Copy link

@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.

@matart15 matart15 reopened this Nov 5, 2024
@danielbank
Copy link

danielbank commented Nov 10, 2024

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
    )
  );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants