Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

Setup RLS and CLS for supabase #919

Closed
rndquu opened this issue Feb 26, 2024 · 12 comments · Fixed by #923
Closed

Setup RLS and CLS for supabase #919

rndquu opened this issue Feb 26, 2024 · 12 comments · Fixed by #923

Comments

@rndquu
Copy link
Member

rndquu commented Feb 26, 2024

Right now we're using a pavlovcik's supabase DB instance with disabled RLS

What should be done:

  1. Make sure that RLS is enabled and the bot acts as expected
  2. Enable CLS and make sure that permits.transaction field can only be updated by an authorized user who matches permits.beneficiary_id (related comment)
@0x4007
Copy link
Member

0x4007 commented Feb 26, 2024

Can you do a time estimate on this?

@gentlementlegen
Copy link
Member

/start

Copy link

ubiquibot bot commented Mar 12, 2024

DeadlineTue, Mar 12, 11:24 AM UTC
Registered Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED
Tips:
  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@gentlementlegen
Copy link
Member

Important notes:

The column rights features is still in Alpha through the Supabase Dashboard, so to be manipulated with caution. Secondly:

Changes to column privileges will not be reflected in migrations when running supabase db diff.
Column privileges are not supported in the current version of the Supabase CLI.
You will need to manually apply these changes to your database.

Third, the changes also impact audit.ubq project, since it also pulls data from the db (reads only).

@rndquu
Copy link
Member Author

rndquu commented Mar 12, 2024

You will need to manually apply these changes to your database.

By "manually" it is meant to set priviliges via supabase's dashboard UI, right?

Third, the changes also impact audit.ubq project, since it also pulls data from the db (reads only).

As far as I remember https://audit.ubq.fi/ needs access only to the permits table so it seems to be ok to allow https://audit.ubq.fi/ read access to the permits table.

@gentlementlegen
Copy link
Member

Yes audit is good with read only permissions on permits so that's all good, if you can think of any project that is also linked to the db that we should test please let me know.

Manually means that if we run a diff these changes won't be contained in the migration file. So any change done through the UI will be lost when it comes to CLS and migrations, which I mention to avoid mistakes later. Otherwise it works fine so far.

@gentlementlegen
Copy link
Member

@pavlovcik @rndquu Is the pavlovcik's supabase DB also used for the bot in production? Or is there a different instance for this? Also asking because the migrations in this repo are not matching.

@rndquu
Copy link
Member Author

rndquu commented Mar 13, 2024

So any change done through the UI will be lost when it comes to CLS and migrations, which I mention to avoid mistakes later.

The best thing we can do in this case is to describe the required RLS/CLS setup in the README file

Is the pavlovcik's supabase DB also used for the bot in production?

Yes

Also asking because the migrations in this repo are not matching.

Yes, migrations in the current repo are deprecated. Migrations for the kernel are not yet created and I'm in doubt if we need ones since all of the kernel plugins will have their own DBs so I'm not sure what exactly we need to store in the kernel's DB.

@gentlementlegen
Copy link
Member

gentlementlegen commented Mar 13, 2024

So the wfzpewmlyiozupulbuur Supabase project has been updated as such:

  • locations: read-only for public
  • permits: read-only for public
  • tokens: read-only for public
  • users: read-only for public
  • wallets: read-only for public

All the other tables have no rules so are accessible only by service key / admin.

The reason why all these table need the read-only is to satisfy audit that calls

    const { data: permits } = await supabase
      .from("permits")
      .select("*, locations(*), users(*, wallets(*)), tokens(*)")
      .in("locations.issue_id", issueIds)
      .not("locations", "is", null);

To retrieve permits and their associated data. If we don't allow the read, all the data returned is empty within the permit.

For updates, the column transaction is the only one that can be modified by a user
image

Copy link

ubiquibot bot commented Apr 11, 2024

! action has an uncaught error

@0x4007 0x4007 reopened this Apr 15, 2024
@0x4007 0x4007 closed this as completed Apr 15, 2024
Copy link

ubiquibot bot commented Apr 15, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented Apr 15, 2024

[ 2.7 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment10.8
ReviewComment21.9
Conversation Incentives
CommentFormattingRelevanceReward
Can you do a time estimate on this?...
0.80.270.8
I don't know enough about migrations to understand this pull wel...
1.30.21.3
> Since we gonna have one DB per module it shall be irrelevant s...
0.60.710.6

[ 272.2 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueTask1200
IssueComment40
IssueComment445.2
ReviewComment213.5
ReviewComment213.5
Conversation Incentives
CommentFormattingRelevanceReward
Important notes:

The column rights features is still in Alpha...

-

code:
  count: 2
  score: "0"
  words: 2
0.56-
Yes audit is good with read only permissions on `permits` so tha...
-
code:
  count: 2
  score: "0"
  words: 2
0.61-
@pavlovcik @rndquu Is the `pavlovcik's supabase DB` also used fo...
-
code:
  count: 1
  score: "0"
  words: 4
0.7-
So the `wfzpewmlyiozupulbuur` Supabase project has been updated ...
-
li:
  count: 5
  score: "0"
  words: 25
code:
  count: 3
  score: "0"
  words: 2
0.8-
Important notes:

The column rights features is still in Alpha...

9.8

code:
  count: 2
  score: "2"
  words: 2
0.569.8
Yes audit is good with read only permissions on `permits` so tha...
10.5
code:
  count: 2
  score: "2"
  words: 2
0.6110.5
@pavlovcik @rndquu Is the `pavlovcik's supabase DB` also used fo...
4.4
code:
  count: 1
  score: "1"
  words: 4
0.74.4
So the `wfzpewmlyiozupulbuur` Supabase project has been updated ...
20.5
li:
  count: 5
  score: "5"
  words: 25
code:
  count: 3
  score: "3"
  words: 2
0.820.5
It is a literal db dump as I didn't know myself what functions w...
5.40.35.4
@whilefoo I think it would be important to get this in because m...
8.1
a:
  count: 1
  score: "1"
  words: 1
0.28.1
It is a literal db dump as I didn't know myself what functions w...
5.40.35.4
@whilefoo I think it would be important to get this in because m...
8.1
a:
  count: 1
  score: "1"
  words: 1
0.28.1

[ 46 WXDAI ]

@rndquu
Contributions Overview
ViewContributionCountReward
IssueSpecification118.6
IssueComment227.4
Conversation Incentives
CommentFormattingRelevanceReward
Right now we're using a pavlovcik's supabase DB instance with di...
18.6
a:
  count: 4
  score: "4"
  words: 4
li:
  count: 2
  score: "2"
  words: 76
code:
  count: 2
  score: "2"
  words: 4
118.6
> You will need to manually apply these changes to your database...
11.6
code:
  count: 2
  score: "2"
  words: 2
0.6611.6
> So any change done through the UI will be lost when it comes t...
15.8
a:
  count: 1
  score: "1"
  words: 1
0.7415.8

[ 1 WXDAI ]

@whilefoo
Contributions Overview
ViewContributionCountReward
ReviewComment11
Conversation Incentives
CommentFormattingRelevanceReward
Sorry, I didn't know you were waiting for me...
10.121

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.