PostgREST / postgrest-docs

This repo is archived and will be merged into postgrest/postgrest soon.
http://postgrest.org
MIT License
365 stars 164 forks source link

Incorrect instructions to alter default privileges on functions #334

Closed tad-lispy closed 4 years ago

tad-lispy commented 4 years ago

In the Schema structure document the following is prescribed:

ALTER DEFAULT PRIVILEGES IN SCHEMA api REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC;

but the PostgreSQL documentation specifically informs that this will have no effect. From https://www.postgresql.org/docs/12/sql-alterdefaultprivileges.html#SQL-ALTERDEFAULTPRIVILEGES-EXAMPLES:

Note however that you cannot accomplish that effect with a command limited to a single schema. This command has no effect, unless it is undoing a matching GRANT:

ALTER DEFAULT PRIVILEGES IN SCHEMA public REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC;

That's because per-schema default privileges can only add privileges to the global setting, not remove privileges granted by it.

To make it work as expected the IN SCHEMA api fragment should be removed. The possible unintended side effect is that this will change default privileges in all schemas. The only way to deal with it I can think of is to order the statements.

Shall I prepare a PR to improve this?

Thanks for all the excellent work on PostgREST!

steve-chavez commented 4 years ago

@tad-lispy Whoa, thanks a lot for catching that one. It wasn't obvious at all.

Shall I prepare a PR to improve this?

Sure! Would love your help on this :)

dpopowich commented 4 years ago

I discovered this problem and found this issue. A few things I discovered:

A subtlety I hadn't realized: ALTER DEFAULT PRIVILEGES affects functions created in the future. From the docs:

ALTER DEFAULT PRIVILEGES allows you to set the privileges that will be applied to objects created in the > future. (It does not affect privileges assigned to already-existing objects.)

So my solution was to place the following just before creating my api functions in my sql script:

alter default privileges revoke execute on functions from public;

Then after creating the functions, the following:

-- grant execute on functions in api schema to role auth_user
grant execute on all functions in schema api to auth_user;
-- grant execute to the login function to anonymous users
grant execute on function api.login to web_anon;
tad-lispy commented 4 years ago

The way @dpopowich is granting execute on all functions is nice. I didn't include it in my PR because I wanted to make minimal changes just to fix the mistake. But maybe we should mention it? What do you think @steve-chavez?

steve-chavez commented 4 years ago

@tad-lispy Sure! Go ahead.

I think @dpopowich solution is the way to go forward. We enforce being explicit about function privileges.

tad-lispy commented 4 years ago

PR updated.