Greetings, * gkokola...@pm.me (gkokola...@pm.me) wrote: > On Monday, November 23, 2020 11:31 PM, Stephen Frost <sfr...@snowman.net> > wrote: > > - Anastasia Lubennikova (a.lubennik...@postgrespro.ru) wrote: > > > > > On 29.10.2020 17:19, Stephen Frost wrote: > > > > > > > - Georgios Kokolatos (gkokola...@protonmail.com) wrote: > > > > > > > > > this patch is in "Ready for committer" state and the Cfbot is happy. > > > > > Glad that's still the case. :) > > > > > > > > > Is there any committer that is available for taking a look at it? > > > > > If there aren't any objections or further comments, I'll take another > > > > > look through it and will commit it during the upcoming CF. > > > > > > CFM reminder. Just in case you forgot about this thread) > > > The commitfest is heading to the end. And there was a plenty of time for > > > anyone to object. > > > > Thanks, I've not forgotten, but it's a bit complicated given that I've > > another patch in progress to rename default roles to be predefined > > roles which conflicts with this one. Hopefully we'll have a few > > comments on that and I can get it committed and this one updated with > > the new naming. I'd rather not commit this one and then immediately > > commit changes over top of it. > > May I enquire about the status of the current?
The patch to rename default roles to predefined roles for v14 has gone in, and so I've come back to this patch to add the pg_read/write_all_data roles. Having contemplated a bit further, I ended up deciding that it made more sense for these predefined roles to *not* have BYPASSRLS, which gives admins the flexibilty to choose if they actually want RLS to be bypassed, or not, on the roles who they GRANT these roles to (if we just always had bypassrls set, then they wouldn't have any choice but to accept that, which doesn't seem very kind). I've updated the documentation to make a note of that and to encourage admins who use these roles to consider if they want to set BYPASSRLS on the actual login role which they'll have to create in order to use these roles (since they can't be used to login directly). Updated patch attached. Will be playing with it a bit more but generally feel like it's in pretty good shape. Unless there's anything further on this, I'll likely commit it over the weekend. Thanks! Stephen
From 3032fcf79d162c8e170d648af26d9230609c7b29 Mon Sep 17 00:00:00 2001 From: Stephen Frost <sfr...@snowman.net> Date: Fri, 26 Mar 2021 17:24:18 -0400 Subject: [PATCH] Add pg_read_all_data and pg_write_all_data roles A commonly requested use-case is to have a role who can run an unfettered pg_dump without having to explicitly GRANT that user access to all tables, schemas, et al, without that role being a superuser. This address that by adding a "pg_read_all_data" role which implicitly gives any member of this role SELECT rights on all tables, views and sequences, and USAGE rights on all schemas. As there may be cases where it's also useful to have a role who has write access to all objects, pg_write_all_data is also introduced and gives users implicit INSERT, UPDATE and DELETE rights on all tables, views and sequences. These roles can not be logged into directly but instead should be GRANT'd to a role which is able to log in. As noted in the documentation, if RLS is being used then an administrator may (or may not) wish to set BYPASSRLS on the login role which these predefined roles are GRANT'd to. Reviewed-by: Georgios Kokolatos Discussion: https://postgr.es/m/20200828003023.gu29...@tamriel.snowman.net --- doc/src/sgml/user-manag.sgml | 18 +++++++++++ src/backend/catalog/aclchk.c | 30 +++++++++++++++++++ src/include/catalog/pg_authid.dat | 10 +++++++ src/test/regress/expected/privileges.out | 38 +++++++++++++++++++++++- src/test/regress/sql/privileges.sql | 20 +++++++++++++ 5 files changed, 115 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index d171b13236..fe0bdb7599 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -518,6 +518,24 @@ DROP ROLE doomed_role; </row> </thead> <tbody> + <row> + <entry>pg_read_all_data</entry> + <entry>Read all data (tables, views, sequences), as if having SELECT + rights on those objects, and USAGE rights on all schemas, even without + having it explicitly. This role does not have the role attribute + <literal>BYPASSRLS</literal> set. If RLS is being used, an administrator + may wish to set <literal>BYPASSRLS</literal> on roles which this role is + GRANTed to.</entry> + </row> + <row> + <entry>pg_write_all_data</entry> + <entry>Write all data (tables, views, sequences), as if having INSERT, + UPDATE, and DELETE rights on those objects, and USAGE rights on all + schemas, even without having it explicitly. This role does not have the + role attribute <literal>BYPASSRLS</literal> set. If RLS is being used, + an administrator may wish to set <literal>BYPASSRLS</literal> on roles + which this role is GRANTed to.</entry> + </row> <row> <entry>pg_read_all_settings</entry> <entry>Read all configuration variables, even those normally visible only to diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 4b2afffb8f..6a0da06d9f 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -3925,6 +3925,26 @@ pg_class_aclmask_ext(Oid table_oid, Oid roleid, AclMode mask, ReleaseSysCache(tuple); + /* + * Check if ACL_SELECT is being checked and, if so, and not set already + * as part of the result, then check if the user is a member of the + * pg_read_all_data role, which allows read access to all relations. + */ + if (mask & ACL_SELECT && !(result & ACL_SELECT) && + has_privs_of_role(roleid, ROLE_READ_ALL_DATA)) + result |= ACL_SELECT; + + /* + * Check if ACL_INSERT, ACL_UPDATE, or ACL_DELETE is being checked + * and, if so, and not set already as part of the result, then check + * if the user is a member of the pg_write_all_data role, which + * allows INSERT/UPDATE/DELETE access to all relations. + */ + if (mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE) && + !(result & (ACL_INSERT | ACL_UPDATE | ACL_DELETE)) && + has_privs_of_role(roleid, ROLE_WRITE_ALL_DATA)) + result |= (mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE)); + return result; } @@ -4251,6 +4271,16 @@ pg_namespace_aclmask(Oid nsp_oid, Oid roleid, ReleaseSysCache(tuple); + /* + * Check if ACL_USAGE is being checked and, if so, and not set already + * as part of the result, then check if the user is a member of the + * pg_read_all_data or pg_write_all_data roles, which allow usage + * access to all schemas. + */ + if (mask & ACL_USAGE && !(result & ACL_USAGE) && + (has_privs_of_role(roleid, ROLE_READ_ALL_DATA) || + has_privs_of_role(roleid, ROLE_WRITE_ALL_DATA))) + result |= ACL_USAGE; return result; } diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index 65795a965b..f78802e41f 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -29,6 +29,16 @@ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '9274', oid_symbol => 'ROLE_READ_ALL_DATA', + rolname => 'pg_read_all_data', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '9275', oid_symbol => 'ROLE_WRITE_ALL_DATA', + rolname => 'pg_write_all_data', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, { oid => '3373', oid_symbol => 'ROLE_PG_MONITOR', rolname => 'pg_monitor', rolsuper => 'f', rolinherit => 't', rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 89f3d5da46..baa65ce324 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -12,6 +12,7 @@ DROP ROLE IF EXISTS regress_priv_user3; DROP ROLE IF EXISTS regress_priv_user4; DROP ROLE IF EXISTS regress_priv_user5; DROP ROLE IF EXISTS regress_priv_user6; +DROP ROLE IF EXISTS regress_priv_user7; SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid; lo_unlink ----------- @@ -26,6 +27,10 @@ CREATE USER regress_priv_user4; CREATE USER regress_priv_user5; CREATE USER regress_priv_user5; -- duplicate ERROR: role "regress_priv_user5" already exists +CREATE USER regress_priv_user6; +CREATE USER regress_priv_user7; +GRANT pg_read_all_data TO regress_priv_user6; +GRANT pg_write_all_data TO regress_priv_user7; CREATE GROUP regress_priv_group1; CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2; ALTER GROUP regress_priv_group1 ADD USER regress_priv_user4; @@ -131,6 +136,29 @@ SELECT * FROM atest2 WHERE ( col1 IN ( SELECT b FROM atest1 ) ); ------+------ (0 rows) +SET SESSION AUTHORIZATION regress_priv_user6; +SELECT * FROM atest1; -- ok + a | b +---+----- + 1 | two + 1 | two +(2 rows) + +SELECT * FROM atest2; -- ok + col1 | col2 +------+------ +(0 rows) + +INSERT INTO atest2 VALUES ('foo', true); -- fail +ERROR: permission denied for table atest2 +SET SESSION AUTHORIZATION regress_priv_user7; +SELECT * FROM atest1; -- fail +ERROR: permission denied for table atest1 +SELECT * FROM atest2; -- fail +ERROR: permission denied for table atest2 +INSERT INTO atest2 VALUES ('foo', true); -- ok +UPDATE atest2 SET col2 = true; -- ok +DELETE FROM atest2; -- ok SET SESSION AUTHORIZATION regress_priv_user3; SELECT session_user, current_user; session_user | current_user @@ -1884,6 +1912,12 @@ SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'USAGE'); -- yes t (1 row) +SELECT has_schema_privilege('regress_priv_user6', 'testns2', 'USAGE'); -- yes + has_schema_privilege +---------------------- + t +(1 row) + SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'CREATE'); -- no has_schema_privilege ---------------------- @@ -2284,7 +2318,9 @@ DROP USER regress_priv_user3; DROP USER regress_priv_user4; DROP USER regress_priv_user5; DROP USER regress_priv_user6; -ERROR: role "regress_priv_user6" does not exist +DROP USER regress_priv_user7; +DROP USER regress_priv_user8; -- does not exist +ERROR: role "regress_priv_user8" does not exist -- permissions with LOCK TABLE CREATE USER regress_locktable_user; CREATE TABLE lock_table (a int); diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 22337310af..41e9f12517 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -16,6 +16,7 @@ DROP ROLE IF EXISTS regress_priv_user3; DROP ROLE IF EXISTS regress_priv_user4; DROP ROLE IF EXISTS regress_priv_user5; DROP ROLE IF EXISTS regress_priv_user6; +DROP ROLE IF EXISTS regress_priv_user7; SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid; @@ -29,6 +30,11 @@ CREATE USER regress_priv_user3; CREATE USER regress_priv_user4; CREATE USER regress_priv_user5; CREATE USER regress_priv_user5; -- duplicate +CREATE USER regress_priv_user6; +CREATE USER regress_priv_user7; + +GRANT pg_read_all_data TO regress_priv_user6; +GRANT pg_write_all_data TO regress_priv_user7; CREATE GROUP regress_priv_group1; CREATE GROUP regress_priv_group2 WITH USER regress_priv_user1, regress_priv_user2; @@ -96,6 +102,17 @@ GRANT ALL ON atest1 TO PUBLIC; -- fail SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) ); SELECT * FROM atest2 WHERE ( col1 IN ( SELECT b FROM atest1 ) ); +SET SESSION AUTHORIZATION regress_priv_user6; +SELECT * FROM atest1; -- ok +SELECT * FROM atest2; -- ok +INSERT INTO atest2 VALUES ('foo', true); -- fail + +SET SESSION AUTHORIZATION regress_priv_user7; +SELECT * FROM atest1; -- fail +SELECT * FROM atest2; -- fail +INSERT INTO atest2 VALUES ('foo', true); -- ok +UPDATE atest2 SET col2 = true; -- ok +DELETE FROM atest2; -- ok SET SESSION AUTHORIZATION regress_priv_user3; SELECT session_user, current_user; @@ -1121,6 +1138,7 @@ ALTER DEFAULT PRIVILEGES GRANT USAGE ON SCHEMAS TO regress_priv_user2; CREATE SCHEMA testns2; SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'USAGE'); -- yes +SELECT has_schema_privilege('regress_priv_user6', 'testns2', 'USAGE'); -- yes SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'CREATE'); -- no ALTER DEFAULT PRIVILEGES REVOKE USAGE ON SCHEMAS FROM regress_priv_user2; @@ -1364,6 +1382,8 @@ DROP USER regress_priv_user3; DROP USER regress_priv_user4; DROP USER regress_priv_user5; DROP USER regress_priv_user6; +DROP USER regress_priv_user7; +DROP USER regress_priv_user8; -- does not exist -- permissions with LOCK TABLE -- 2.27.0
signature.asc
Description: PGP signature
