Implement db_pre_request

[bangdragon]

If implement db_pre_request, rbac system will be fast and secure for production. I think so.
https://github.com/burggraf/postgrest-request-processing/tree/main

[fnimick]

I personally would recommend not doing this. I have supabase RLS set up to authenticate both requests coming in via postgrest, as well as requests made in the context of a user from my backend, using the same RLS policies. I do this by setting custom transaction variables with the JWT of the user request. This won't work if db_pre_request is used because that is postgrest specific.

I believe this will also fail with Supabase GraphQL, where currently we can use the same policies for that and the REST API.

[point-source]

@fnimick could you elaborate on how implementing this would affect your use-case? From what I understand, it would be possible for me to use db_pre_request as a way to ensure that the claims in the incoming JWT are still correct compared to the claims in the group_users table. Beyond that, nothing should really change operationally.

Perhaps you can describe how you are anticipating the change to function and how it would break your current setup? Thanks

[fnimick]

@point-source ah it wasn't clear that this would only be for JWT verification, not to actually load any other information into the request context.

I would still encourage caution, since it would mean that identical table RLS policies would have different levels of security and therefore different behavior based on how the table is accessed - e.g. via the supabase API (postgrest) vs supabase graphql vs a standard database connection with the JWT included in the transaction variables.

[point-source]

@fnimick Yeah my plan currently is to just have the pre request hook overwrite the claims in the jwt exactly as they are. The only time this wouldn't be overwriting it with identical information is if the user's claims changed but their jwt had not yet been refreshed. In these cases, the pre request hook is fixing what is currently a vulnerability in this system, but only fixing it for REST requests, as you pointed out.

It seems to me that even if I can't patch the entire hole, I should patch as much as I can. Thoughts?

[point-source]

I now have this implemented but am waiting on this doc issue and this bug so I can figure out how to roll it out.

[point-source]

@fnimick After further research into the way this works, it became clear to me that adding information to the request context is likely the better approach. The way I currently have this configured is as follows:

• When a claim is changed, the associated user's app_metadata field is updated to include the current state of claims (all group memberships for that user + roles)
• When a JWT is generated (due to a user signing in, usually) then that JWT includes the contents of the app_metadata field as it as at the time of JWT generation
• If a claim is modified after that, then the existing JWT would need to be manually refreshed to include the change. Otherwise, it will get updated once it expires
• As of this update, when a request comes in to postgrest, the db_pre_request hook will perform a query to get the current state of the app_metadata field. This gets stored in a new key in the request context called request.groups
• Functions which previously relied solely on the JWT will now check to see if request.groups exists. If it does, it will use that to determine access. If not, it will fall back to the data encoded into the JWT
• If you are using the JWT for authorization outside of postgrest, then the behavior after this update is identical to the way it was before. You are still relying on claim data that was correct at the time the JWT was made. The difference is that postgrest will now make a best-effort attempt to use the most recent/true claim data.

Is this an acceptable solution for you? If not, could you explain why and perhaps propose an alternative that doesn't reduce the security of the postgrest side? Thanks!

[fnimick]

I still am worried about potentially different behavior between postgrest and standard queries.

Could you add a function to populate request.groups from the JWT, and then call that function in db_pre_request? If so, then we could manually call the same function when setting up a transaction context using the JWT in order to duplicate the db_pre_request behavior.

To give a concrete example, this is what I do to set up a RLS transaction using the JWT when accessing the database from a SQL client. drizzle-team/drizzle-orm#594 (comment) . My thought is that in my transaction setup here, I can use your new JWT lookup function rather than simply copying the JWT into the request claims config, which would duplicate the behavior of db_pre_request.

[point-source]

I still am worried about potentially different behavior between postgrest and standard queries.

Well yes, there would be a chance of different behavior but in the case where it differs, it differs in favor of security. So postgrest would be sometimes more secure than standard queries. I don't think it makes sense to keep postgrest on the less secure JWT method in order to maintain consistency.

Could you add a function to populate request.groups from the JWT, and then call that function in db_pre_request? If so, then we could manually call the same function when setting up a transaction context using the JWT in order to duplicate the db_pre_request behavior.

Populating request.groups from the JWT would defeat the purpose since the goal is to not use claims that are in the JWT at all (since they might be wrong / out of date). That said, if you manually call db_pre_request(), it will do the same thing it does when triggered by postgrest. That is, it will query auth.users to get the current claims and inject them into request.groups.

Alternatively, you can call get_req_groups() to directly get the claim groups from auth.users. This is the function that db_pre_request() uses to get them before putting them inside request.groups.

You can then access those by either checking request.groups directly or calling get_req_groups() which will return the claims in request.groups OR an empty jsonb object if request.groups is null.

Ultimately, I want to deprecate and remove claims from the JWT entirely now that there are better methods to get claim data for both postgrest and standard transactions.

My thought is that in my transaction setup here, I can use your new JWT lookup function rather than simply copying the JWT into the request claims config, which would duplicate the behavior of db_pre_request.

Yes, this sounds very nearly like the right approach. The exception being that the function will populate from the auth.users.raw_app_meta_data field instead of the JWT. But this should operate the same/better than before and will be consistent with the new db_pre_request behavior.

[fnimick]

Ultimately, I want to deprecate and remove claims from the JWT entirely now that there are better methods to get claim data for both postgrest and standard transactions.

I still like having claim data in the JWT since I parse that claim data on the frontend and use it for client-side control of routing to allowed UI elements. The data loaded for those elements is still protected, so there's no harm in gating access to a specific frontend URL behind a JWT claim, and then failing at the load step if the claim is invalid.

[point-source]

That's definitely valid. I'll make sure that that remains a possibility then, if not the default.

As for the rest, do those new db functions I mentioned serve your needs for the current implementation?

[fnimick]

@point-source I assume you're referring to the functions as implemented on the develop branch?

I have a few questions:

• is there a reason the actual auth functions (has_group_role, etc) aren't marked as stable? This would avoid the 'single row lookup to each request rather than each row of each request' problem.
• the docs for the JWT functions don't indicate that they use the settings set in db-pre-request and only fallback to the JWT if those are not available
• get_req_groups is documented as performing a query against the auth table, but it appears to return either the request settings or an empty object?

[point-source]

is there a reason the actual auth functions (has_group_role, etc) aren't marked as stable? This would avoid the 'single row lookup to each request rather than each row of each request' problem.

Good call. No reason. Will fix in the next release

the docs for the JWT functions don't indicate that they use the settings set in db-pre-request and only fallback to the JWT if those are not available

Yep, good point. In the next release (1.0.0, breaking changes), I am actually removing the JWT functions in favor of having the primary functions (now marked stable as well) check the request context first (in case of db_pre_request) and then fallback to JWT. This should improve performance enough to consolidate the functions. The only time it would impact security is if the user was previously relying on the non-jwt functions AND is not using postgrest (thus skipping db_pre_request) AND does not trust the claims within the JWT.

get_req_groups is documented as performing a query against the auth table, but it appears to return either the request settings or an empty object?

Got me again, I mixed that up. It will be fixed in 1.0.0 (or replaced)

You can review the current unfinished state of 1.0.0 here.

[point-source]

This has been implemented as part of the 1.0.0 release