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

OpenAPI output depends on the user making the request. Is this correct? #954

Closed
ruslantalpa opened this issue Aug 26, 2017 · 5 comments
Closed
Assignees

Comments

@ruslantalpa
Copy link
Contributor

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:

Jaco Esterhuizen @litriv Aug 17 14:23
Hi, I'm using postgREST for the first time and loving it - great work ! I just have a quick question about Swagger and authentication. I'm using JWT authentication as per examples and so without an Authorization header with token, I'm only getting the login function in the OpenAPI spec when GETting with empty path, as expected. So the question is: how to get the OpenAPI specs for when logged in as other roles? Do I need to implement a mechanism for passing an Authorization header in Swagger itself? It looks like the OpenAPI 2.0 spec does not support this use case, but that OpenAPI 3.0.0 may have a sufficiently enhanced Security Requirements Object for being able to deal with this. I'm just wondering how to currently solve my problem.
This is mainly just for having Swagger UI exposed to front-end devs. I guess I could create a route in nginx for each role, that passes the token for that role in the header for all requests. I could then add those routes, to the swagger configuration, so it'll display in the top bar for being able to switch between them. Does that sound like a reasonable solution or am i missing something about this entirely? Is there a simpler way perhaps?

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 but accessibleProcs 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:

  • A) make tables output not consider the user.
  • B) fix the output for rpc to consider users, but we'll need a completely different sql and fn for openapi in order to not break other things.

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

@litriv
Copy link

litriv commented Aug 26, 2017

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 reload signal to nginx. That way it's easy to 'log in' as a different user for retrieving the OpenAPI spec.

@steve-chavez
Copy link
Member

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 openapi+json output but if we go with option A) we would not support this valid use case.

@steve-chavez steve-chavez self-assigned this Sep 12, 2017
steve-chavez added a commit to steve-chavez/postgrest that referenced this issue Sep 13, 2017
@ruslantalpa
Copy link
Contributor Author

@steve-chavez good points,
if we go with option B (as I see you've already implemented) how would you configure a system so that all endpoints are visible (auto docs) regardless of who is requesting /

@steve-chavez
Copy link
Member

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 db-anon-role="postgres/${whoami}" then for his tests switch the db-anon-role to the only login role anonymous.

If something like that is needed for a testing/production environment then a role like master_dev and GRANTs would be needed, and for the dev to see the full Swagger UI output it would have to set the JWT token.

steve-chavez added a commit to steve-chavez/postgrest that referenced this issue Sep 18, 2017
@litriv
Copy link

litriv commented Sep 21, 2017

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.

monacoremo pushed a commit to monacoremo/postgrest that referenced this issue Jul 17, 2021
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants