-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
OpenAPI output depends on the user making the request. Is this correct? #954
Comments
I'm indifferent, as long as it's consistent (both or neither of CRUD endpoints/models and RPCs are authorized). I ended up using nginx to add the token to requests, with a script that generates the token given a user name and password, replaces the current one in nginx.conf and sends a |
I think that B) would be the right choice because: I believe that Obscurity is a Valid Security Layer and in more conservative environments, like government IT departments with data governance managers and so, it may be a hard requirement not to show frontend devs all the available tables/views/functions in a schema for "security reasons"(high turnover, contractors that do a few features). Of course a workaround would be creating new restricted schemas but sometimes designs have lots of tables and copying them is harder than just creating a ROLE and assigning GRANTs. Aside from security a bigger reason I see is that having metadata tailored for the user enables devs to build dynamic forms that vary with user permissions, I built something like that but with previous PostgREST OPTIONS output, don't know how feasible is to do something like that with |
@steve-chavez good points, |
I think that's something that'll most likely be needed on a local development environment, if the developer wants to use the full Swagger UI output while doing features then he could run PostgREST with a maximum privileged anonymous role like If something like that is needed for a testing/production environment then a role like |
Actually, i am leaning towards B, as the front-end dev on my team is finding it useful to have the OpenAPI spec restricted according to a specific user's privileges. |
* Fix PostgREST#933, update externals docs url to current version * Fix PostgREST#962, openApi don't err on nonexistent schema * Fix PostgREST#954, make OpenAPI rpc output dependent on user privileges
TL;DR
While for tables, the swagger output considers the privileges of the user making the request
https://github.com/begriffs/postgrest/blob/master/src/PostgREST/DbStructure.hs#L208-L210
when it comes to stored procedures, it does not
https://github.com/begriffs/postgrest/blob/master/src/PostgREST/DbStructure.hs#L156-L170
This is inconsistent and should be fixed.
This thing came up in gitter.
@litriv asked this:
Then, i mistakenly told him that the output does not depend on the user (which was probably a few days wasted for him tracking the problem, i am sorry). I am not sure why i thought that but the fact that we have different logic in queries for tables and procs shows that this wasn't taught through.
Also note how for tables, a live query is always issued, while for stored proc, the cached version is used. The
accessibleTables
is used only for openApi butaccessibleProcs
is used here https://github.com/begriffs/postgrest/blob/master/src/PostgREST/DbStructure.hs#L38 and cached, and that cached version is used in other places (when rpc's are called), i.e. we can't change the function to request only the procs accessible by the current user (like for tables) because it will break other things.So, for tables, accessible means accessible to the user, and for rpc it means accessible in general through the api.
There are two ways of making this consistent:
I think option A) is the correct one (and simpler to implement).
Everyone i saw using this output is to hook up swaggerui, i.e. it's a way to document the api for frontend devs, and for documentation, you don't want it to depend on the logged in user, you want to see all the possible calls.
I saw arguments like "i want to show different endpoints for different users for security reasons" but i think this is a useless thing, "security through obscurity", not likely to be an obstacle in the path of an attacker. Event PostgreSQL (and probably all other databases) in a way went with option A (so did the GraphQL ecosystem, in a schema you see everything possible).
You can see all the entities in a database (you can query pg_catalog) but it does nto mean you can access them.
We should do the same thing (since this is the most useful scenario also), openapi should display all the available endpoint, which does not mean anyone can query them.
Anyone interested in this, vote with thumbs up for option A (the way i sugested) and thumbd down for B)
let's see other opinions.
If you vote for B, give arguments and things like "it is more secure" and "i don't want to ... x,y,z" are not arguments :)
The text was updated successfully, but these errors were encountered: