From 91803c6d089d64e8fdd91f2e6308d24dbc8a1e8a Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Wed, 7 Dec 2022 16:54:35 -0800 Subject: [PATCH v1 2/2] add predefined roles for cluster, refresh matview, and reindex --- doc/src/sgml/ref/cluster.sgml | 4 ++- .../sgml/ref/refresh_materialized_view.sgml | 4 ++- doc/src/sgml/ref/reindex.sgml | 10 ++++-- doc/src/sgml/user-manag.sgml | 18 +++++++++++ src/backend/catalog/aclchk.c | 31 +++++++++++++++++++ src/backend/commands/indexcmds.c | 7 +++-- src/include/catalog/pg_authid.dat | 15 +++++++++ src/test/regress/expected/privileges.out | 23 ++++++++++++++ src/test/regress/sql/privileges.sql | 24 ++++++++++++++ 9 files changed, 129 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 24eccab251..1a13e2116b 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -70,7 +70,9 @@ CLUSTER [VERBOSE] CLUSTER without any parameter reclusters all the previously-clustered tables in the current database that the calling user owns or has the CLUSTER privilege for, or all such tables - if called by a superuser. This form of CLUSTER cannot be + if called by a superuser or a role with privileges of the + pg_cluster_all_tables + role. This form of CLUSTER cannot be executed inside a transaction block. diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml index 90b3de5274..5af0f83be2 100644 --- a/doc/src/sgml/ref/refresh_materialized_view.sgml +++ b/doc/src/sgml/ref/refresh_materialized_view.sgml @@ -32,7 +32,9 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] name REFRESH MATERIALIZED VIEW completely replaces the contents of a materialized view. To execute this command you must be the - owner of the materialized view or have the REFRESH + owner of the materialized view, have privileges of the + pg_cluster_all_tables + role, or have the REFRESH privilege on the materialized view. The old contents are discarded. If WITH DATA is specified (or defaults) the backing query is executed to provide the new data, and the materialized view is left in a diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 560e536b3c..01a23061a0 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -293,15 +293,19 @@ REINDEX [ ( option [, ...] ) ] { DA Reindexing a single index or table requires being the owner of that - index or table or having the REINDEX privilege on the + index or table, having privileges of the + pg_reindex_all_indexes + role, or having the REINDEX privilege on the table. Reindexing a schema or database requires being the - owner of that schema or database. Note specifically that it's thus + owner of that schema or database or having privileges of the + pg_reindex_all_indexes role. Note specifically that it's thus possible for non-superusers to rebuild indexes of tables owned by other users. However, as a special exception, when REINDEX DATABASE, REINDEX SCHEMA or REINDEX SYSTEM is issued by a non-superuser, indexes on shared catalogs will be skipped unless the user owns the - catalog (which typically won't be the case) or has the + catalog (which typically won't be the case), has privileges of the + pg_reindex_all_indexes role, or has the REINDEX privilege on the catalog. Of course, superusers can always reindex anything. diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index 2bff4e47d0..2722e624b4 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -647,6 +647,24 @@ DROP ROLE doomed_role; ANALYZE command on all tables. + + pg_cluster_all_tables + Allow executing the + CLUSTER command on + all tables. + + + pg_refresh_all_matviews + Allow executing the + REFRESH MATERIALIZED VIEW + command on all materialized views. + + + pg_reindex_all_indexes + Allow executing the + REINDEX command on + all indexes. + diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index b0b44b3fb2..9be457fe3c 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -4234,6 +4234,37 @@ pg_class_aclmask_ext(Oid table_oid, Oid roleid, AclMode mask, has_privs_of_role(roleid, ROLE_PG_ANALYZE_ALL_TABLES)) result |= ACL_ANALYZE; + /* + * Check if ACL_CLUSTER is being checked and, if so, and not already set as + * part of the result, then check if the user is a member of the + * pg_cluster_all_tables role, which allows CLUSTER on all relations. + */ + if (mask & ACL_CLUSTER && + !(result & ACL_CLUSTER) && + has_privs_of_role(roleid, ROLE_PG_CLUSTER_ALL_TABLES)) + result |= ACL_CLUSTER; + + /* + * Check if ACL_REFRESH is being checked and, if so, and not already set as + * part of the result, then check if the user is a member of the + * pg_refresh_all_matviews role, which allows REFRESH MATERIALIZED VIEW on + * all relations. + */ + if (mask & ACL_REFRESH && + !(result & ACL_REFRESH) && + has_privs_of_role(roleid, ROLE_PG_REFRESH_ALL_MATVIEWS)) + result |= ACL_REFRESH; + + /* + * Check if ACL_REINDEX is being checked and, if so, and not already set as + * part of the result, then check if the user is a member of the + * pg_reindex_all_indexes role, which allows REINDEX on all relations. + */ + if (mask & ACL_REINDEX && + !(result & ACL_REINDEX) && + has_privs_of_role(roleid, ROLE_PG_REINDEX_ALL_INDEXES)) + result |= ACL_REINDEX; + return result; } diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index e30e2ef844..c471e8510d 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -26,6 +26,7 @@ #include "catalog/index.h" #include "catalog/indexing.h" #include "catalog/pg_am.h" +#include "catalog/pg_authid.h" #include "catalog/pg_constraint.h" #include "catalog/pg_database.h" #include "catalog/pg_inherits.h" @@ -2922,7 +2923,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, { objectOid = get_namespace_oid(objectName, false); - if (!object_ownercheck(NamespaceRelationId, objectOid, GetUserId())) + if (!object_ownercheck(NamespaceRelationId, objectOid, GetUserId()) && + !has_privs_of_role(GetUserId(), ROLE_PG_REINDEX_ALL_INDEXES)) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SCHEMA, objectName); } @@ -2934,7 +2936,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("can only reindex the currently open database"))); - if (!object_ownercheck(DatabaseRelationId, objectOid, GetUserId())) + if (!object_ownercheck(DatabaseRelationId, objectOid, GetUserId()) && + !has_privs_of_role(GetUserId(), ROLE_PG_REINDEX_ALL_INDEXES)) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_DATABASE, get_database_name(objectOid)); } diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index 2574e2906d..07bd3b2cae 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -94,5 +94,20 @@ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '4551', oid_symbol => 'ROLE_PG_CLUSTER_ALL_TABLES', + rolname => 'pg_cluster_all_tables', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '4552', oid_symbol => 'ROLE_PG_REFRESH_ALL_MATVIEWS', + rolname => 'pg_refresh_all_matviews', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '4553', oid_symbol => 'ROLE_PG_REINDEX_ALL_INDEXES', + rolname => 'pg_reindex_all_indexes', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, ] diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 1f7050438f..02da69bcad 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -2917,6 +2917,7 @@ DROP ROLE regress_both_all; -- CLUSTER CREATE ROLE regress_no_cluster; CREATE ROLE regress_cluster; +CREATE ROLE regress_cluster_all IN ROLE pg_cluster_all_tables; CREATE TABLE cluster_test (a INT); CREATE INDEX ON cluster_test (a); GRANT CLUSTER ON cluster_test TO regress_cluster; @@ -2927,12 +2928,17 @@ RESET ROLE; SET ROLE regress_cluster; CLUSTER cluster_test USING cluster_test_a_idx; RESET ROLE; +SET ROLE regress_cluster_all; +CLUSTER cluster_test USING cluster_test_a_idx; +RESET ROLE; DROP TABLE cluster_test; DROP ROLE regress_no_cluster; DROP ROLE regress_cluster; +DROP ROLE regress_cluster_all; -- REFRESH MATERIALIZED VIEW CREATE ROLE regress_no_refresh; CREATE ROLE regress_refresh; +CREATE ROLE regress_refresh_all IN ROLE pg_refresh_all_matviews; CREATE MATERIALIZED VIEW refresh_test AS SELECT 1; GRANT REFRESH ON refresh_test TO regress_refresh; SET ROLE regress_no_refresh; @@ -2942,25 +2948,42 @@ RESET ROLE; SET ROLE regress_refresh; REFRESH MATERIALIZED VIEW refresh_test; RESET ROLE; +SET ROLE regress_refresh_all; +REFRESH MATERIALIZED VIEW refresh_test; +RESET ROLE; DROP MATERIALIZED VIEW refresh_test; DROP ROLE regress_no_refresh; DROP ROLE regress_refresh; +DROP ROLE regress_refresh_all; -- REINDEX CREATE ROLE regress_no_reindex; CREATE ROLE regress_reindex; +CREATE ROLE regress_reindex_all IN ROLE pg_reindex_all_indexes; CREATE TABLE reindex_test (a INT); CREATE INDEX ON reindex_test (a); GRANT REINDEX ON reindex_test TO regress_reindex; +CREATE SCHEMA reindex_schema; SET ROLE regress_no_reindex; REINDEX TABLE reindex_test; ERROR: permission denied for "reindex_test" REINDEX INDEX reindex_test_a_idx; ERROR: permission denied for "reindex_test_a_idx" +REINDEX SCHEMA reindex_schema; +ERROR: must be owner of schema reindex_schema RESET ROLE; SET ROLE regress_reindex; REINDEX TABLE reindex_test; REINDEX INDEX reindex_test_a_idx; +REINDEX SCHEMA reindex_schema; +ERROR: must be owner of schema reindex_schema +RESET ROLE; +SET ROLE regress_reindex_all; +REINDEX TABLE reindex_test; +REINDEX INDEX reindex_test_a_idx; +REINDEX SCHEMA reindex_schema; RESET ROLE; DROP TABLE reindex_test; +DROP SCHEMA reindex_schema; DROP ROLE regress_no_reindex; DROP ROLE regress_reindex; +DROP ROLE regress_reindex_all; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 98eb7f05be..6ffc0c71e6 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -1920,6 +1920,7 @@ DROP ROLE regress_both_all; -- CLUSTER CREATE ROLE regress_no_cluster; CREATE ROLE regress_cluster; +CREATE ROLE regress_cluster_all IN ROLE pg_cluster_all_tables; CREATE TABLE cluster_test (a INT); CREATE INDEX ON cluster_test (a); @@ -1933,13 +1934,19 @@ SET ROLE regress_cluster; CLUSTER cluster_test USING cluster_test_a_idx; RESET ROLE; +SET ROLE regress_cluster_all; +CLUSTER cluster_test USING cluster_test_a_idx; +RESET ROLE; + DROP TABLE cluster_test; DROP ROLE regress_no_cluster; DROP ROLE regress_cluster; +DROP ROLE regress_cluster_all; -- REFRESH MATERIALIZED VIEW CREATE ROLE regress_no_refresh; CREATE ROLE regress_refresh; +CREATE ROLE regress_refresh_all IN ROLE pg_refresh_all_matviews; CREATE MATERIALIZED VIEW refresh_test AS SELECT 1; GRANT REFRESH ON refresh_test TO regress_refresh; @@ -1952,28 +1959,45 @@ SET ROLE regress_refresh; REFRESH MATERIALIZED VIEW refresh_test; RESET ROLE; +SET ROLE regress_refresh_all; +REFRESH MATERIALIZED VIEW refresh_test; +RESET ROLE; + DROP MATERIALIZED VIEW refresh_test; DROP ROLE regress_no_refresh; DROP ROLE regress_refresh; +DROP ROLE regress_refresh_all; -- REINDEX CREATE ROLE regress_no_reindex; CREATE ROLE regress_reindex; +CREATE ROLE regress_reindex_all IN ROLE pg_reindex_all_indexes; CREATE TABLE reindex_test (a INT); CREATE INDEX ON reindex_test (a); GRANT REINDEX ON reindex_test TO regress_reindex; +CREATE SCHEMA reindex_schema; SET ROLE regress_no_reindex; REINDEX TABLE reindex_test; REINDEX INDEX reindex_test_a_idx; +REINDEX SCHEMA reindex_schema; RESET ROLE; SET ROLE regress_reindex; REINDEX TABLE reindex_test; REINDEX INDEX reindex_test_a_idx; +REINDEX SCHEMA reindex_schema; +RESET ROLE; + +SET ROLE regress_reindex_all; +REINDEX TABLE reindex_test; +REINDEX INDEX reindex_test_a_idx; +REINDEX SCHEMA reindex_schema; RESET ROLE; DROP TABLE reindex_test; +DROP SCHEMA reindex_schema; DROP ROLE regress_no_reindex; DROP ROLE regress_reindex; +DROP ROLE regress_reindex_all; -- 2.25.1