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

Migrations do not allow adding storage policies #96

Closed
everzet opened this issue Dec 6, 2021 · 10 comments
Closed

Migrations do not allow adding storage policies #96

everzet opened this issue Dec 6, 2021 · 10 comments
Labels
bug Something isn't working

Comments

@everzet
Copy link
Contributor

everzet commented Dec 6, 2021

Bug report

Describe the bug

One of the biggest benefits I see in Suapabase is its ability to be nearly 100% controlled via SQL.

This to me means that I should be able to describe most of my database, storage and authentication requirements through migrations.

Currently, if migration includes a change of a policy on sotrage.objects - it fails with ERROR: must be owner of ... (e.g.: ERROR: must be owner of table objects).

Short term fix for me was to make postgres user a superuser with (via Supabase UI):

ALTER USER postgres WITH SUPERUSER;

But that feels less than ideal for a variety of reasons.

To Reproduce

Create a migration that includes addition of RLS policy onto a sotrage.objects table:

create policy "Avatars are accessible to authenticated users"
  on storage.objects for select
  using ( bucket_id = 'avatars' and auth.role() = 'authenticated' );

Expected behavior

Migrations should just be applied without an error with supabase db push.

Instead, they only execute (without an error) against the local version.

@everzet everzet added the bug Something isn't working label Dec 6, 2021
@everzet everzet changed the title Migrations do not allow adding storage policies or enabling extensions Migrations do not allow adding storage policies Dec 6, 2021
@soedirgo
Copy link
Member

soedirgo commented Dec 7, 2021

The CLI doesn't manage schemas like auth, storage, etc. since it uses the postgres role on the remote db. The CLI's postgres user is superuser (thus allows creating the policy above), but the remote db's is not - the storage schema is instead managed by the supabase_storage_admin user.

In general, we don't give full access to schemas like auth, storage, realtime which are managed by Supabase to postgres. You may not agree with this design - if so please create a Discussion thread on supabase/supabase to prompt a discussion.

@bnjmnt4n
Copy link

In general, we don't give full access to schemas like auth, storage, realtime which are managed by Supabase to postgres.

I'd think that this severe limits the potential of using the Supabase CLI for handling migrations and syncing them across local development environments and production deploys on the Supabase platform. Although access to the Supabase-managed schemas is discouraged, there are some things that can only be done with access to the schemas: eg. creating triggers on auth.users, and adding policies to storage (both of which are common and crucial usecases, and can benefit greatly from transactional migrations).

It'd be great if the CLI actually ran everything with the same kind of permissions locally and on the remote, as this is otherwise a major blocker against adoption of the CLI.

You may not agree with this design - if so please create a Discussion thread on supabase/supabase to prompt a discussion.

I think we could probably use this issue to discuss, but I can also open a discussion there if preferred.

@soedirgo
Copy link
Member

I agree - the CLI use case is sort of an afterthought, so the permissions model used for Supabase-managed stuff has unintended implications that is only now discovered. In general I think whatever users can do on the dashboard they should be able to do programmatically (either with the postgres role or some other way).

We're prioritizing rethinking the permissions next year, but unfortunately it's difficult to get this right without breaking current projects, so this will take a while.

@RinyVT
Copy link

RinyVT commented Dec 24, 2021

Agreed that the CLI should also be able to handle these cases, or is there another way to automate/migrate these changes I'm not aware of?

Currently, it's not possible to do CI/CD with Supabase as there's no option to do something related to authorization (tables) / storage within a deployment pipeline.

Probably the only thing (at the moment) holding me back to use Supabase in my project, as the rest of the Supbase looks awesome and exactly what I need!

@isaiahdahl
Copy link

Was about to create a separate issue but figure it's close enough to this one.

Been playing around with the CLI for a few days and came across the Error: must be owner of... Except for me it was on a simple table with nothing but a primary key ID column in a table I created on supabase, then pulled down into a local following the example/tour, and then tried altering the table in the local UI and pushing.

Running this in Supabase UI worked as mentioned above.

ALTER USER postgres WITH SUPERUSER;

But I'm not really sure what the consequences of doing that are.

Just wanted to pipe my two cents in that I agree with this.

It'd be great if the CLI actually ran everything with the same kind of permissions locally and on the remote, as this is otherwise a major blocker against adoption of the CLI.

Seems to make sense to me that my whole development workflow should be able to start from local and then pushed up.

@soedirgo
Copy link
Member

But I'm not really sure what the consequences of doing that are.

We will be taking out superuser role access in the future, so it's better not to lean on it.

Your case seems to be similar to #128, in which case changing the owner to postgres should suffice.

@madeleineostoja
Copy link

Just wanted to echo that this seems like a huge oversight. It makes managing migrations properly with the cli very limited, since there's no longer a single source of truth for the whole project. Your table migrations are tracked, but anything to do with any other schema have to be kept in the supabase UI which feels very fragile, especially for things like triggers and RLS policies.

@noeleom
Copy link

noeleom commented May 6, 2022

Was able to work around this issue via

GRANT supabase_storage_admin TO postgres; /* migrations */ REVOKE supabase_storage_admin FROM postgres;

Hopefully that helps, but still acts as a workaround and not expected functionality.

Good luck 👍

@sweatybridge
Copy link
Contributor

Related to #287

@soedirgo
Copy link
Member

This is now supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants