From b151c41a1338c7a0663774dde777c3aee3f1f3a9 Mon Sep 17 00:00:00 2001 From: gurkanindibay Date: Mon, 11 Mar 2024 15:09:05 +0300 Subject: [PATCH] Fixes system role filters --- src/backend/distributed/commands/role.c | 18 ++++++++++-------- .../regress/expected/granted_by_support.out | 2 ++ src/test/regress/sql/granted_by_support.sql | 7 +++---- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index ea2486d96..fde986dc2 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -1317,7 +1317,7 @@ UnmarkRolesDistributed(List *roles) List * FilterDistributedRoles(List *roles) { - List *distributedRoles = NIL; + List *validRoles = NIL; Node *roleNode = NULL; foreach_ptr(roleNode, roles) { @@ -1333,12 +1333,13 @@ FilterDistributedRoles(List *roles) } ObjectAddress *roleAddress = palloc0(sizeof(ObjectAddress)); ObjectAddressSet(*roleAddress, AuthIdRelationId, roleOid); - if (IsAnyObjectDistributed(list_make1(roleAddress))) + bool isSystemRole = IsReservedName(role->rolename); + if (IsAnyObjectDistributed(list_make1(roleAddress)) || isSystemRole) { - distributedRoles = lappend(distributedRoles, role); + validRoles = lappend(validRoles, role); } } - return distributedRoles; + return validRoles; } @@ -1349,7 +1350,7 @@ FilterDistributedRoles(List *roles) List * FilterDistributedGrantedRoles(List *roles) { - List *distributedRoles = NIL; + List *validRoles = NIL; Node *roleNode = NULL; foreach_ptr(roleNode, roles) { @@ -1365,12 +1366,13 @@ FilterDistributedGrantedRoles(List *roles) } ObjectAddress *roleAddress = palloc0(sizeof(ObjectAddress)); ObjectAddressSet(*roleAddress, AuthIdRelationId, roleOid); - if (IsAnyObjectDistributed(list_make1(roleAddress))) + bool isSystemRole = IsReservedName(role->priv_name); + if (IsAnyObjectDistributed(list_make1(roleAddress)) || isSystemRole) { - distributedRoles = lappend(distributedRoles, role); + validRoles = lappend(validRoles, role); } } - return distributedRoles; + return validRoles; } diff --git a/src/test/regress/expected/granted_by_support.out b/src/test/regress/expected/granted_by_support.out index 4e44b4c7d..4928f5516 100644 --- a/src/test/regress/expected/granted_by_support.out +++ b/src/test/regress/expected/granted_by_support.out @@ -55,6 +55,8 @@ SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid':: grant dist_role4 to dist_role3 with admin option GRANTED BY dist_role1; --fails since already dist_role3 granted to dist_role4 ERROR: role "dist_role4" is a member of role "dist_role3" +--Below command will not be successful since non_dist_role1 is propagated with the dependency resolution above +--however, ADMIN OPTION is not propagated for non_dist_role1 to worker 1 because the citus.enable_create_role_propagation is off grant non_dist_role1 to dist_role4 granted by dist_role1; ERROR: permission denied to grant privileges as role "dist_role1" DETAIL: The grantor must have the ADMIN option on role "non_dist_role1". diff --git a/src/test/regress/sql/granted_by_support.sql b/src/test/regress/sql/granted_by_support.sql index fcec183c5..1261e78d4 100644 --- a/src/test/regress/sql/granted_by_support.sql +++ b/src/test/regress/sql/granted_by_support.sql @@ -45,9 +45,10 @@ SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid':: grant dist_role4 to dist_role3 with admin option GRANTED BY dist_role1; --fails since already dist_role3 granted to dist_role4 +--Below command will not be successful since non_dist_role1 is propagated with the dependency resolution above +--however, ADMIN OPTION is not propagated for non_dist_role1 to worker 1 because the citus.enable_create_role_propagation is off grant non_dist_role1 to dist_role4 granted by dist_role1; - grant dist_role3 to dist_role1 with admin option GRANTED BY dist_role4; grant "dist_role5'_test" to dist_role1 with admin option; grant "dist_role5'_test" to dist_role3 with admin option GRANTED BY dist_role1;--fails since already dist_role3 granted to "dist_role5'_test" @@ -218,12 +219,10 @@ set citus.enable_create_database_propagation to on; drop database test_granted_by_support; drop role dist_role1,dist_role2,dist_role3,dist_role4,"dist_role5'_test"; drop role non_dist_role1; + drop role if exists non_dist_role1; - - - select result FROM run_command_on_all_nodes( $$ SELECT array_to_json(array_agg(row_to_json(t)))