Feat: [Pg] support for row level security

[Angelelz]

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()

[rphlmr]

For Supabase, we have other roles like authenticated, etc.

Maybe the type could be 'public' | 'current_role' | 'current_user' | 'session_user' | (string & {})

[rphlmr]

Oh you handle that with AnyPgRole, sorry.
But, how can we use existing roles not created by us in the schema?

[rphlmr]

.existing() like for views?

[Angelelz]

Maybe you could inline the pgRole in the config like this:

{ to: [pgRole("my_existing_role")] }

Because all it needs is the name

[rphlmr]

Sounds good

[rphlmr]

What about DROP or ALTER for policies/roles?

[rphlmr]

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).

[Angelelz]

What about DROP or ALTER for 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

[Angelelz]

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?

[rphlmr]

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.

[rphlmr]

What about DROP or ALTER for 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 😅

[rphlmr]

What about (suggestion):

set?: {
		configs?: {
			name: string;
			value: string;
			isLocal?: boolean;
		}[];
		role?: AnyPgRole;
	};

• set because postgres commands start with set or set_
• configs because of set_config(...)

And I think it is not only related to rls.
We can imagine wanting to use this to set_config, which is later used in a PG function that we called in our transaction.

Like https://www.postgresql.org/docs/current/sql-syntax-calling-funcs.html

[net]

Yes, set_config(...) is not just for policies. I like set: { configs: ..., role: ... } or set_configs: ..., set_role: ....

https://www.postgresql.org/docs/16/functions-admin.html

[net]

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().

[cayter]

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.

[net]

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.

[cayter]

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.

[net]

@cayter Good point, there are some restrictions that are implemented as query clauses but are still best built on the application side.

[rphlmr]

Just to don't forget that:
#594 (comment)

We should reset role at the end of the transaction.

 await tx.execute(sql`RESET ROLE;`)

[JonathonRP]

Any progress on this?

[julien-blanchon]

I really wish this feature will get merged soon

[cloudcreatr]

Can anyone tell when it's merging or any updates

[rphlmr]

👋 The team has been notified that there is some interest in this feature.

[AndriiSherman]

Starting the review and discussion of an API. After that, we will fix anything that needs to be changed before releasing it

[serhii-kucherenko]

Any updates on this one? Super important for me

[AntonioAngelino]

Any news? @AndriiSherman 🙏

[markoradinovic]

Will this be included in the next release?
Really hoping for it

[AntonioAngelino]

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

[Aaqu]

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

[serhii-kucherenko]

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?

[Angelelz]

This was implemented by the team.