drizzle-orm
drizzle-orm copied to clipboard
drizzle-team
Feat: [Pg] support for row level security
This PR will close #594.
The changes in this PR will not do anything on their own, as this is mostly a migration/push feature from drizzle-kit. Upon merging this, the user will be able to define roles and policies in the schema like this:
// schema.ts
import { pgRole, pgPolicy, pgTable, serial, text } from "drizzle-orm/pg-core";
export const users = pgTable("users", {
id: serial("id"),
name: text("name")
})
export const userRole = pgRole("user_role", { // all these config options are optional, and so is the parameter
superuser: true,
inherit: true
});
export const newPolicy = pgPolicy("new_policy", users, { // all these config options are optional, and so is the parameter
as: "permissive",
for: "update",
to: userRole
})
This is a WIP PR. Still looking into how to run enable row level security on the tables.
Maybe we can do this?
export const users = pgTable("users", {
id: serial("id"),
name: text("name")
}).enableRLS()
We should ask the community, but I think RLS should be enable by default or at least print a warning during migration. A table without RLS is exposed (if publicly accessible, like in Supabase. You only need the public anon to read/write tables).
What about
DROPorALTERfor policies/roles?
Those should work the same way it works for tables. Drizzle-kit should diff your schema against what's on the DB and generate alter/drop/create accordingly
We should ask the community, but I think RLS should be enable by default or at least print a warning during migration. A table without RLS is exposed (if publicly accessible, like in Supabase. You only need the public anon to read/write tables).
I think drizzle should do the same as postgres, you have to enable it. Aren't all tables protected by the connection password?
We should ask the community, but I think RLS should be enable by default or at least print a warning during migration. A table without RLS is exposed (if publicly accessible, like in Supabase. You only need the public anon to read/write tables).
I think drizzle should do the same as postgres, you have to enable it. Aren't all tables protected by the connection password?
Not when using Supabase (DB is exposed through a web api). You have a big warning. But you are right, Drizzle have to follow postgres.
What about
DROPorALTERfor policies/roles?Those should work the same way it works for tables. Drizzle-kit should diff your schema against what's on the DB and generate alter/drop/create accordingly
Right 😅
I use RLS policies without Supabase, and this looks good to me! It's key that using and withCheck take raw SQL instead of some neutered JS abstraction.
Something to consider is that naming policies can be tediously repetitive if you tend to stick to one policy per table per operation. I've found the convention <role>_<inserts|updates|deletes>_<table>—for example, user_updates_posts—to work well. It might be nice to autogenerate a name like that if no name is provided to pgPolicy().
We should ask the community, but I think RLS should be enable by default or at least print a warning during migration. A table without RLS is exposed (if publicly accessible, like in Supabase. You only need the public anon to read/write tables).
This would be a bad idea, just like how NextJS 13 started enabling caching by default. If Postgres didn't enable RLS by default, there's a reason to that. But at least to us, we don't buy into RLS is the answer to auth check everything in Postgres itself for 4 reasons:
- roles & permissions might differ on different products, some are fine with RLS and some are not
- RLS can cause performance headache if 1 isn't careful enough and aware of what they are doing
- our app servers can scale much better than a single Postgres cluster (even with read replicas, we'd still need to take care of replica lag and whatnot)
- unit testing with RLS requires more tedious setup unless your product has simple roles/permissions set
In addition, if your database provider isn't providing a private network and secure connection with long random password (or even better, something like IAM access policy) + SSL, you should definitely push them to do so. All the production workloads I had worked on that touches sensitive data including money movement are all putting their database inside VPC with restricted port at the very least.
This would be a bad idea, just like how NextJS 13 started enabling caching by default. If Postgres didn't enable RLS by default, there's a reason to that.
Ideally warning on tables without policies would be a configurable option. If RLS is being used in a project, one probably wants policies on all tables.
- RLS can cause performance headache if 1 isn't careful enough and aware of what they are doing
- our app servers can scale much better than a single Postgres cluster (even with read replicas, we'd still need to take care of replica lag and whatnot)
You probably are pushing access control logic into your queries and thus into your DB anyways (SELECT FROM users WHERE company_id = ${current_user.company_id}). If you're not, then your app servers must fetch far more data than they need, and that will almost always have a far larger performance impact than filtering in the DB and will absolutely not scale.
All RLS does is automatically add the same query clauses to every query (for a role) that you would've otherwise had to manually add in your application code every time you wrote a restricted query.
Ideally warning on tables without policies would be a configurable option. If RLS is being used in a project, one probably wants policies on all tables.
If this warning is only showing up because of you choose to opt-in to use RLS in new Drizzle(), I think it's fine.
You probably are pushing access control logic into your queries and thus into your DB anyways (SELECT FROM users WHERE company_id = ${current_user.company_id}). If you're not, then your app servers must fetch far more data than they need, and that will almost always have a far larger performance impact than filtering in the DB and will absolutely not scale.
Yes, definitely.
As I mentioned, RLS might work on some products where the product users don't get to define the roles/permissions although I'm not sure how one can easily unit test the different roles on different code paths when the roles set grow tremendously. Also, note that not every engineer has direct access to production database due to compliance/regulatory needs which would make the maintenance/debugging with RLS policies tedious.
In our current case, we do have some roles/permissions that are defined by the users themselves. If we're to use RLS for this, this would mean we need to implement additional validation/review logic to ensure the RLS policies that users manage won't be causing security issues which itself can become an additional layer of complexity that defeats the purpose of using RLS in the first place.
@cayter Good point, there are some restrictions that are implemented as query clauses but are still best built on the application side.
Just to don't forget that: https://github.com/drizzle-team/drizzle-orm/issues/594#issuecomment-1917830225
We should reset role at the end of the transaction.
await tx.execute(sql`RESET ROLE;`)
Starting the review and discussion of an API. After that, we will fix anything that needs to be changed before releasing it
I'm checking the commits/changelogs in the beta branch once per week to see if the RLS support has been added. We had to wrote a custom script to create the RLS policies, but I'd love to define them whit drizzle constructs to use the standard migration system
Will this be included in the next release? Really hoping for it
If I understand correctly, it's close to beta Update: finishing pull and push logic for policies and roles and preparing for beta release
I'm checking the commits/changelogs in the beta branch once per week to see if the RLS support has been added.
We had to wrote a custom script to create the RLS policies, but I'd love to define them whit drizzle constructs to use the standard migration system
Can you please share it, please?