ubiquity / ubiquibot

Putting the 'A' in 'DAO'
https://github.com/marketplace/ubiquibot
MIT License
16 stars 59 forks source link

Setup RLS and CLS for supabase #919

Closed rndquu closed 2 months ago

rndquu commented 4 months ago

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 commented 4 months ago

Can you do a time estimate on this?

gentlementlegen commented 3 months ago

/start

ubiquibot[bot] commented 3 months ago

DeadlineTue, Mar 12, 11:24 AM UTC
Registered Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED
Tips:
gentlementlegen commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

So the wfzpewmlyiozupulbuur Supabase project has been updated as such:

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

ubiquibot[bot] commented 2 months ago
! action has an uncaught error
ubiquibot[bot] commented 2 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 2 months ago

[ 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