cli icon indicating copy to clipboard operation
cli copied to clipboard

Migrations do not allow adding storage policies

Open everzet opened this issue 3 years ago • 8 comments

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 avatar Dec 06 '21 10:12 everzet

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.

soedirgo avatar Dec 07 '21 09:12 soedirgo

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.

bnjmnt4n avatar Dec 23 '21 11:12 bnjmnt4n

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.

soedirgo avatar Dec 23 '21 12:12 soedirgo

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!

RinyVT avatar Dec 24 '21 11:12 RinyVT

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.

isaiahdahl avatar Jan 18 '22 05:01 isaiahdahl

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 https://github.com/supabase/cli/issues/128, in which case changing the owner to postgres should suffice.

soedirgo avatar Jan 18 '22 06:01 soedirgo

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.

madeleineostoja avatar May 04 '22 23:05 madeleineostoja

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 :+1:

noeleom avatar May 06 '22 19:05 noeleom

Related to #287

sweatybridge avatar Aug 17 '22 10:08 sweatybridge

This is now supported.

soedirgo avatar Sep 29 '22 10:09 soedirgo