[ruggi99]

What kind of change does this PR introduce?

Feature.
Add CRUD table and column permission to postgres-meta.
Should allow an admin to list, grant and revoke permissions on single tables and columns of a table.
Needed to add in future column permission to Supabase Dashboard.
This requires a little discussion on variables naming, routes and so on.

I took the sql from this SELECT definition FROM pg_views WHERE viewname = 'column_privileges' and adapted it.

TODO list:

• Add CRUD table permission to lib
• Add column permission to lib
• Add CRUD table permission to server
• Add column permission to server
• Eventually add read-only merged permissions

Waiting for feedback

What is the current behavior?

Please link any relevant issues here.

What is the new behavior?

Feel free to include screenshots if it includes visual changes.

Additional context

Closes #489

[ruggi99]

@sweatybridge @soedirgo what are your initial thoughts?

[soedirgo]

Sorry @ruggi99, currently occupied with other stuff - will come back to this once I have some bandwidth.

[ruggi99]

Yeah thanks.
I think it needs some other work that I will do in my free time

[ruggi99]

@soedirgo any ETA on this for at least a first feedback?
I don't like having opened PRs that are pending due to others bandwidth constraints or other problems related.
If not I will temporary close this PR.

[kiwicopple]

hey @ruggi99 - this is a great PR, so I'm going to re-open it. We're a bit blocked, but we definitely need this functionality

If you prefer, I can migrate the PR over to myself

[kiwicopple]

one thing that could be useful is to add tests. this repo is well-tested, and we'd love to keep the coverage high given the function this is serving

[ruggi99]

Hi @kiwicopple ,
no problem in keeping it open, at least I know it will be reviewed one day.

Yeah I know that this PR is lacking tests but I'm not so good at writing them.
At least I can try in the spare time and push if I reach a good point.
But I think one of the team should take care of writing tests when reviewing this.
Sorry about that.

[kiwicopple]

not so good at writing them

Use ChatGPT 😛 . Seriously though - no obligation, it's awesome that you've gone to the effort of doing this.

We'll merge this if after we have some passing tests 👍

[jdgamble555]

Hey guys, anyone looking at accepting this? This would be a huge feature to help with security!

Thanks!

J

[alaister]

@jdgamble555 yep, these are features that are great for pg-meta! We'll aim to get this in after launch week.

[MentalGear]

This is a much needed feature, e.g. for letting users update certain aspects of their table row, but not all.
ATM I'm not sure how to disallow a user to update his "role" with RLS/supabase only - is this something that can just be done in middleware/custom backend code? (maybe there is a supabase guide no this, but searching the docs there isn't)

[jdgamble555]

Hey @alaister, hopefully you guys are working on it this week 😃 I was thinking due to the way the grants will be, it is probably best to release this so that it works on local and cloud.

J

[jdgamble555]

Hey @alaister, are you guys able to get to this this week?

Thanks!

J

[ibanks42]

Any update on this?

[daKmoR]

wooowwww I never knew I needed this 😅

just saw https://dev.to/jdgamble555/supabase-needs-column-level-security-170a and now I want it 🤣

[ruggi99]

Hi all,
I simply closed (again) this PR because there's no interest from Supabase team in reviewing this PR, even the first time. No discussion, no asking, no review.
I only heard things like promises but not even a useful comment by Supabase team.
One could argue that there were conflicts to be resolved but I did not resolve them because it would have been a waste of my time.
There are open PRs that are trying to do what I've done. Hope they review and merge them in the next few years (or more?)

@jdgamble555 sorry for not being active on comments but now you know why I did not interact with this PR too much

[jdgamble555]

Got it. Sorry about that. I updated the article. I didn't realize you weren't from the Supabase Team. Hopefully they still add this feature in one form or another.

J

[soedirgo]

Sorry @ruggi99, we should've got back to you on this PR. We still want the feature, but there are differences on what we want the API to look like & the SQL queries used.

As you said, there are open PRs now that implement this, which are currently being reviewed:

• feat: table privileges #588
• feat: column privileges #589

[ruggi99]

And that's why I'm not contributing back to Supabase anymore.
At least until you change the bad approach you have with external open-source contributors.

[kiwicopple]

Hey @ruggi99 - sorry for this, it looks like a comms problem from our side. As @soedirgo mentioned, we want this and we're using your PR as a baseline to implement the functionality

As you mentioned above, the supabase team still had some work to do with creating tests ("one of the team should take care of writing tests when reviewing this"). That's 100% understandable, but please also be understanding if we create separate PRs to implement the missing functionality or decide to make some tweaks along the way

This is a great contribution and we're appreciate, even if we did a poor job communicating that.

[ruggi99]

Agree on what you've said. I'm pleased that you are taking my work to continue working on that and moving forward, but as stated now it's better to continue on separate PRs that include tests and other stuff.

There are open PRs that are trying to do what I've done

Written like this may sound a little bad but I only wanted to let know people that Supabase Team has taken in account the task by creating PRs that implement the same thing I wanted to implement. I'm not saying that you are stealing my code, ideas, ecc No problem on my side

[lt692]

I wonder what's the progress with this feature and when is it planned to be released? 🤔

[soedirgo]

Hey @lt692! This is now available as a feature preview: supabase/supabase#13745 (comment)

[lt692]

Already trying it out! 🫡 People will love it!