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

Add RLS for new Supabase tables (Github, Quotes) #3433

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

joelazwar
Copy link
Contributor

@joelazwar joelazwar commented Apr 8, 2022

Issue This PR Addresses

fix #3408

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

Adds read level functionality, and restricts altering and updating for all users (github-* tables, and quotes table)

Steps to test the PR

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@joelazwar joelazwar requested a review from JerryHue April 8, 2022 16:23
@gitpod-io
Copy link

gitpod-io bot commented Apr 8, 2022

@joelazwar
Copy link
Contributor Author

joelazwar commented Apr 8, 2022

@JerryHue @DukeManh could you guys help me test this? I can't get the init scripts to work. (even the github tables arent showing up in my local supabase)

@JerryHue
Copy link
Contributor

JerryHue commented Apr 8, 2022

@JerryHue @DukeManh could you guys help me test this? I can't get the init scripts to work. (even the github tables arent showing up in my local supabase)

You need to delete the whole volume for the init scripts to actually execute. That means that you need to delete the folder docker/supabase/volumes/db/data. You might need to use admin privileges in your computer because docker created the volume itself.

It is kind of cumbersome, but hey, it is what it is 😅

If you want, you can open an issue that could address this situation. We would need to discuss on how to actually approach this.

@DukeManh
Copy link
Contributor

DukeManh commented Apr 8, 2022

@joelazwar, we have a meeting at 1 on Teams about migrations with Prisma to talk about how to edit the existing schema. To add RLS with it, you would make a migration file and put your RLS into it and apply the migration.

The init scripts won't be run again because the DB has already been initialized. I doubt that the volumes data can be deleted.

@joelazwar
Copy link
Contributor Author

joelazwar commented Apr 8, 2022

Sorry, I missed the meeting, I was afk. So should we just add the RLS policies manually in the our dev Supabase environments? Also reading up on the prisms stuff, currently

@joelazwar
Copy link
Contributor Author

joelazwar commented Apr 8, 2022

Going to close this in favor of #3418

@joelazwar joelazwar closed this Apr 8, 2022
@humphd
Copy link
Contributor

humphd commented Apr 9, 2022

Why did we close this? We still need to run this migration I think, right?

@joelazwar
Copy link
Contributor Author

Reopened with migration script for Prisma

Copy link
Contributor

@DukeManh DukeManh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We introduce _prisma_migrations table used by Prisma in #3418, I think you can enable RLS on that too with no select and update for all because only Prisma uses it and it has a password to access the DB.

@joelazwar
Copy link
Contributor Author

@DukeManh Seems like it doesn't exist or maybe hidden.

https://github.com/Seneca-CDOT/telescope/runs/5991956564?check_suite_focus=true

Run pnpm migrate

> @senecacdot/[email protected] migrate /home/runner/work/telescope/telescope
> pnpm migrate --prefix src/db


> [email protected] migrate /home/runner/work/telescope/telescope/src/db
> prisma migrate dev

Prisma schema loaded from prisma/schema.prisma
Datasource "db": PostgreSQL database "postgres", schema "public" at "localhost:5432"

Error: P3006

Migration `20220409140003_add_rls_policy_new_tables` failed to apply cleanly to the shadow database. 
Error code: P[10](https://github.com/Seneca-CDOT/telescope/runs/5991956564?check_suite_focus=true#step:10:10)[14](https://github.com/Seneca-CDOT/telescope/runs/5991956564?check_suite_focus=true#step:10:14)
Error:
The underlying table for model `_prisma_migrations` does not exist.

 ELIFECYCLE  Command failed with exit code 1.
 ELIFECYCLE  Command failed with exit code 1.
Error: Process completed with exit code 1.

DukeManh
DukeManh previously approved these changes Apr 12, 2022
Copy link
Contributor

@DukeManh DukeManh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joelazwar, I see, there is no underlying _migrations table to enable RLS on. We can enable RLS manually using the studio on staging/production if we need to.

DukeManh
DukeManh previously approved these changes Apr 12, 2022
fix enable RLS query

trailing space

reformated solution to migrate db with prisma

fix missing semicolons

missed one

edit db-migration container script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add RLS for existing Postgres tables
4 participants