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

Configurable query grammar #2805

Open
steve-chavez opened this issue May 28, 2023 · 7 comments
Open

Configurable query grammar #2805

steve-chavez opened this issue May 28, 2023 · 7 comments
Labels
config related to the configuration options idea Needs of discussion to become an enhancement, not ready for implementation

Comments

@steve-chavez
Copy link
Member

steve-chavez commented May 28, 2023

Problem

Solution

Make the query grammar configurable. This can be paired up with pre-config.

Say I want to only allow eq operators and only on primary keys:

-- pgrst.query_grammar is the config prefix 
-- '*' is the default "allow all"
-- ' ' empty is "disallow all"
select set_config('pgrst.query_grammar.op', ' ', true);

-- op.eq is the allowlist for columns
-- here we add all the pks 
select set_config('pgrst.query_grammar.op.eq', array_agg(c.table_name || '.' || c.column_name)::text, true) 
from information_schema.table_constraints tc
join information_schema.constraint_column_usage as ccu using (constraint_schema, constraint_name)
join information_schema.columns as c
  on c.table_schema = tc.constraint_schema and tc.table_name = c.table_name and ccu.column_name = c.column_name
where constraint_type = 'PRIMARY KEY' and c.table_schema = 'test';

-- obtain all the pks allowed
select current_setting('pgrst.query_grammar.op.eq') as eq_allowed;

eq_allowed | {items.id,items2.id,items3.id,clients.id,compound_pk.k1,compound_pk.k2,..

A similar config can be used for order too:

select set_config('pgrst.query_grammar.order', 'table.col ', true);

Notes

@steve-chavez steve-chavez added config related to the configuration options idea Needs of discussion to become an enhancement, not ready for implementation labels May 28, 2023
@steve-chavez

This comment was marked as outdated.

@mitch-lbw
Copy link

mitch-lbw commented Oct 12, 2023

Hi @steve-chavez ,
i recently stumbled upon the same requirement to restrict the type of allowed operators and order criteria to a predefined set of columns, which IMHO would be a very enriching feature. Is the solution you proposed as pre_config function configuring restrictions for operations and order like this
select set_config('pgrst.query_grammar.op', '*', true);
already working or is this a proposal/idea? At least when adding the line above to a preconfig functionit wasn't able to get my postgrest API delivering any data. Also if added a line to restrict ordering to one column like

select set_config('pgrst.query_grammar.order', 'table.col', true);
select set_config('pgrst.query_grammar.order', 'table.col2', false);

i was not able to get a restriction on ordering.
If it is already working, i most probably i made a mistake on configuring and would be thankful for a complete working example :-)
Is there also any documentation available?
Thanks in advance!

@steve-chavez
Copy link
Member Author

@mitch-lbw No, this is not implemented. There's no consensus on the feature design yet.

@steve-chavez
Copy link
Member Author

A related discussion. User asking about restricting operand values #3355.

@gc-ft
Copy link

gc-ft commented Aug 20, 2024

Want to simply re-iterate how important this kind of a feature is to a lot of users in my opinion, at least bigger production setups using PostgREST would benefit from more hardening than is currently available.

Just a tiny quick-fix-suggestion that might not be "great", but still a first step solution while a final one is discussed and decided: publishing just the raw query in request.query (no need to store it parsed or anything on the postgREST end, raw is fine I believe) would be a first step. It would allow making functions on our own which look for certain things in the query string in order to stop the query using RLS.

For example, if the query string is exposed as a simple string in settings, we could write a function which returns true if a certain regex pattern matches request.query ... for example postgrest_query_has('(^|&)id=eq\.') - would return true if an eq filter on the field id is present in the query.

We then simply setup a RLS policy like this, which would automatically filter out all results and return nothing if id filter is missing in the request.

CREATE POLICY select_if_pattern_found
ON your_table_name
FOR SELECT
TO authenticated
USING (
    (SELECT postgrest_query_has('(^|&)id=eq\.'))
);

Of course, this would use the db however it should optimize most of the query away if the match is false. At least it is better than nothing and seems a very easy thing to add to expose as a variable in settings just like you do for path right now.

@steve-chavez
Copy link
Member Author

@gc-ft Right, I think that's the simplest solution, this PR had a similar idea #1710

Now I wouldn't recommend doing that in RLS since it can get really complicated as it is. But doing the filtering on a pre-request should work fine.

@steve-chavez
Copy link
Member Author

Some thoughts about exposing the HTTP query string as a transactional variable.

  • Ideally we pass a single tx var. e.g. a current_setting('request.query_string') not several current_setting('request.query_string.select), current_setting('request.query_string.order), etc. Since adding each added tx var makes the transaction slower.

  • current_setting('request.query_string') shouldn't contain the values of the query string parameters (no X in col=eq.x). If we do this, it could lead to the bad practice of filtering values using pre-request (leading to an "anemic domain model"). We have constraints, domain representations, etc. for these cases.

    • This will also have the advantage of reducing the size of the tx var.
  • current_setting('request.query_string') then should contain a representation of our AST in JSON (to maintain consistency with our other tx vars).

    • For starters, we could have the columns +operators and the top order inside this JSON.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config related to the configuration options idea Needs of discussion to become an enhancement, not ready for implementation
Development

No branches or pull requests

3 participants