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

Unable to insert into table using a write-only role #399

Closed
angerman opened this issue Dec 2, 2015 · 12 comments
Closed

Unable to insert into table using a write-only role #399

angerman opened this issue Dec 2, 2015 · 12 comments

Comments

@angerman
Copy link

angerman commented Dec 2, 2015

Assuming I want public writability, but secured readability.

Say

CREATE TABLE test (id INTEGER, doc JSON);
GRANT INSERT ON test TO anonymous;
GRANT ALL ON test TO admin;

running with

$ postgrest postgres://admin@localhost:5432/db --port 3000 --schema public --anonymous anonymous --pool 50 --jwt-secret secret

I would expect that

$ curl -X POST -H "Content-Type: application/json" -d '{"id":2,"doc":{"foo":"bar"}}' 'http://host:3000/test'

returns success instead of

{"hint":null,"details":null,"code":"42501","message":"permission denied for relation logs"}

What am I doing wrong?

@begriffs
Copy link
Member

begriffs commented Dec 2, 2015

You're hitting a bug actually. I suspect this is happening because the generated SQL has a RETURNING * at the end in case someone asks for the results of the insert. To verify can you include SQL logs from when you run this action? To obtain logs first enable logging all statements, then find your logs.

@ruslantalpa
Copy link
Contributor

@begriffs It can't be a bug in postgrest, if the db is returning the data then it means the role has access to that data.
The "returning *" can be a problem when the role has only insert rights, there is no prefer header, but still we are doing a "returning *". We could make the correction when there is no prefer header, the query does not contain "returning *".

@angerman
can you provide a "full" sql file where you create the database, tables, the users, grant them access?
I suspect that somehow when you created the anonymous role, you granted it full access to all tables in the database.

@begriffs
Copy link
Member

begriffs commented Dec 2, 2015

@ruslantalpa it's not a bug per se, just that postgrest asks for the inserted row back in order to create the Location response header. In a write-only table it's not permitted.

Here's a possible solution. Just like we support the Prefer: return=representation request header to return the entire inserted row(s), we could support Prefer: return=minimal to return nothing, not even a Location header. This behavior is covered in RFC7240. This would allow an insert into a write-only table.

@angerman
Copy link
Author

angerman commented Dec 3, 2015

I will try to cook up a complete example when I'm back home. My expectation would be that, the system adapts to the role, and hence would result in the INSERT and then say automatically imply returning=minimal, as the role doesn't allow for anything else.

@angerman
Copy link
Author

angerman commented Dec 3, 2015

Alright here we go:
Setup two roles (admin and anon)

$ createuser -d -I -l admin
$ createuser -D -I -l anon

and the database

$ createdb -O admin test "PostgREST Test Database"

Create a table with a key and a jsonb document.

$ psql -U admin test
test=> CREATE TABLE test (key varchar(64) primary key, doc jsonb);

Add permissions (admin can do anything as owner) to anon to be able to INSERT.

test=> GRANT INSERT ON test TO anon;

Ensure that the permissions work

$ psql -U anon test
test=> INSERT INTO test (key, doc) VALUES ('foo', '{"x":"1"}');
INSERT 0 1
test=> SELECT * FROM test;
ERROR:  permission denied for relation test

So far so good.

Let's allow admin to set role anon. Log in as super user to database test.

$ psql test
test=# GRANT anon to admin;

Let's try PostgREST

$ ./postgrest postgres://admin@localhost:5432/test -a anon -s public

And try to POST data.

$ curl -X POST -H "Content-Type: application/json" -d '{"key":"quux","doc":{"x":"2"}}' 'http://localhost:3000/test'
{"hint":null,"details":null,"code":"42501","message":"permission denied for relation test"}

The executed query looks like this:

STATEMENT:
WITH pg_source AS (INSERT INTO  public.test  (doc, key)
                   SELECT doc, key
                   FROM json_populate_recordset(null:: public.test , $1)
                   RETURNING public.test.*)
SELECT null, pg_catalog.count(t),
  (WITH s AS (SELECT row_to_json(ss) as r
              from pg_source as ss
              limit 1)
   SELECT string_agg(json_data.key || '=' || coalesce( 'eq.' || json_data.value, 'is.null'), '&')
   FROM s, json_each_text(s.r) AS son_data
   WHERE json_data.key IN ('key')),
  null
  FROM (SELECT pg_source.*
        FROM   pg_source
        LIMIT ALL OFFSET 0) t

Edit: Formatted Statement for better readability on GitHub

@ruslantalpa
Copy link
Contributor

@angerman @begriffs now that i read the question again i realise my previous comment was wrong.
I read it like "i am inserting stuff and it should fail but it somehow gets through" and it actually is "i am trying to insert stuff, i can do that in pure sql, but in postgrest it fails".

Yes, the problem is that we are using "RETURNING *" regardless of the fact if we need the inserted result back (and in this case we don't want it back since we can't actually have it because we do not have SELECT rights).

I will look at implementing this case of prefer header but i would actually call it "return=nothing", we can debate this when the PR is ready.

@angerman
Copy link
Author

angerman commented Dec 3, 2015

@ruslantalpa May I add, that I would expect that header to be implicit if the SELECT permission is not GRANTed?

@ruslantalpa
Copy link
Contributor

That is not possible since there is no way to know (at least in the introspection we currently do) if there is a SELECT granted.
I don't think its a problem, i actually think it's good to fail "loud", it will make you (the api caller) aware that you can not get any results back from this endpoint and force you to make explicit in your call that you do not expect any response beyond the status code.

I view your "requirement/suggestion" when translated into sql as something like this:
when doing "INSERT into .... RETURNING *" the output should be "null" by default if there are no SELECT privileges.
I think you can agree that it would be a bad design choice for SQL for a lot of reasons. That's why i am leaning toward the style "Allow you to execute the query but if you do not have enough privileges, it will fail". This way we do not have to check/mess with who and what privileges has.

@begriffs
Copy link
Member

begriffs commented Dec 3, 2015

@angerman I can understand how you'd rather not have to specify a special header to make things work on a write-only table. We might be able to modify the sql to include the RETURNING * only when this returns true:

SELECT has_table_privilege('myschema.mytable', 'select');

Thus we could gracefully omit the Location response header when we're unable to read the table.

@angerman
Copy link
Author

angerman commented Dec 3, 2015

@ruslantalpa I see your point about being explicit, maybe I got a wrong impression of PostgREST initially. To me the idea that the REST interface adapts to the database kind of left me to believe that the interface should not return anything if the privilege is not given. And technically I can execute the query. Just the translation from REST to SQL, adds the select requirement. I'd likely translate it as:
If I can SELECT: INSERT into ... RETURNING *
else INSERT into ...

@begriffs if that works, that would be awesome. Yet I certainly understand @ruslantalpa concert about adding too much complexity.

@angerman
Copy link
Author

angerman commented Dec 3, 2015

I'm not very vested in SQL, so please excuse if my suggestion makes no sense.

@angerman
Copy link
Author

angerman commented Dec 4, 2015

@ruslantalpa let me know how I can help.

@begriffs begriffs changed the title What roles to set for write only? Unable to insert into table using a write-only role Dec 4, 2015
begriffs added a commit that referenced this issue Dec 16, 2015
Fix #399 insert records in tables with no SELECT 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