diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index fde986dc2..ec7e1430b 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -937,28 +937,6 @@ GenerateGrantRoleStmts() { Form_pg_auth_members membership = (Form_pg_auth_members) GETSTRUCT(tuple); - /* we only propagate the grant if the grantor is - * member and role are distributed since all are required - * to be distributed for the grant to be propagated - */ - - - bool isAuthMemberDistributed = IsAnyObjectDistributed(list_make1( - GetRoleObjectAddressFromOid( - membership->grantor))) - && - IsAnyObjectDistributed(list_make1( - GetRoleObjectAddressFromOid( - membership->member))) - && - IsAnyObjectDistributed(list_make1( - GetRoleObjectAddressFromOid( - membership->roleid))); - if (!isAuthMemberDistributed) - { - /* we only need to propagate the grant if the grantor is distributed */ - continue; - } GrantRoleStmt *grantRoleStmt = GetGrantRoleStmtFromAuthMemberRecord(membership); if (grantRoleStmt == NULL) @@ -1009,11 +987,6 @@ GetGrantRoleStmtFromAuthMemberRecord(Form_pg_auth_members membership) { ObjectAddress *roleAddress = palloc0(sizeof(ObjectAddress)); ObjectAddressSet(*roleAddress, AuthIdRelationId, membership->grantor); - if (!IsAnyObjectDistributed(list_make1(roleAddress))) - { - /* we only need to propagate the grant if the grantor is distributed */ - return NULL; - } GrantRoleStmt *grantRoleStmt = makeNode(GrantRoleStmt); grantRoleStmt->is_grant = true; @@ -1392,24 +1365,8 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString, EnsurePropagationToCoordinator(); GrantRoleStmt *stmt = castNode(GrantRoleStmt, node); - List *allGranteeRoles = stmt->grantee_roles; - List *allGrantedRoles = stmt->granted_roles; - RoleSpec *grantor = stmt->grantor; - DistributedRolesInGrantRoleStmt *distributedRolesInGrantStmt = - ExtractDistributedRolesInGrantRoleStmt(stmt); - - if (!distributedRolesInGrantStmt->isGrantRoleStmtValid) - { - return NIL; - } - - stmt->grantee_roles = distributedRolesInGrantStmt->distributedGrantees; - stmt->granted_roles = distributedRolesInGrantStmt->distributedGrantedRoles; char *sql = DeparseTreeNode((Node *) stmt); - stmt->grantee_roles = allGranteeRoles; - stmt->granted_roles = allGrantedRoles; - stmt->grantor = grantor; List *commands = list_make3(DISABLE_DDL_PROPAGATION, sql, diff --git a/src/test/regress/expected/granted_by_support.out b/src/test/regress/expected/granted_by_support.out index 4928f5516..2ab0e6c7b 100644 --- a/src/test/regress/expected/granted_by_support.out +++ b/src/test/regress/expected/granted_by_support.out @@ -36,13 +36,13 @@ SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid':: (0 rows) grant dist_role2 to non_dist_role1 with admin option; -NOTICE: not propagating GRANT command to other nodes -HINT: Since no grantees are distributed, the GRANT command will not be propagated to other nodes. +ERROR: role "non_dist_role1" does not exist +CONTEXT: while executing command on localhost:xxxxx grant dist_role3 to "dist_role5'_test" granted by dist_role4; grant dist_role2 to "dist_role5'_test" granted by dist_role3; grant dist_role2 to dist_role4 granted by non_dist_role1 ;--will not be propagated since grantor is non-distributed -NOTICE: not propagating GRANT command to other nodes -HINT: Since grantor is not distributed, the GRANT command will not be propagated to other nodes. +ERROR: permission denied to grant privileges as role "non_dist_role1" +DETAIL: The grantor must have the ADMIN option on role "dist_role2". grant dist_role4 to "dist_role5'_test" with admin option; --below command propagates the non_dist_role1 since non_dist_role1 is already granted to dist_role1 --and citus sees granted roles as a dependency and citus propagates the dependent roles @@ -77,9 +77,9 @@ select result FROM run_command_on_all_nodes( ) t $$ ); - result + result --------------------------------------------------------------------- - [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role1","role":"non_dist_role1","grantor":"postgres","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role2","grantor":"non_dist_role1","admin_option":false},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] + [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role1","role":"non_dist_role1","grantor":"postgres","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] (2 rows) @@ -101,11 +101,11 @@ select result FROM run_command_on_all_nodes( ) t $$ ); - result + result --------------------------------------------------------------------- - [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role1","role":"non_dist_role1","grantor":"postgres","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role2","grantor":"non_dist_role1","admin_option":false},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] + [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role1","role":"non_dist_role1","grantor":"postgres","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] - [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role1","role":"non_dist_role1","grantor":"postgres","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role2","grantor":"non_dist_role1","admin_option":false},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] + [{"member":"dist_role1","role":"\"dist_role5'_test\"","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role2","grantor":"postgres","admin_option":true},{"member":"dist_role1","role":"dist_role3","grantor":"dist_role4","admin_option":true},{"member":"dist_role1","role":"dist_role4","grantor":"\"dist_role5'_test\"","admin_option":true},{"member":"dist_role1","role":"non_dist_role1","grantor":"postgres","admin_option":true},{"member":"dist_role3","role":"dist_role2","grantor":"dist_role1","admin_option":true},{"member":"dist_role4","role":"dist_role3","grantor":"postgres","admin_option":true}] (3 rows) --clean all resources diff --git a/src/test/regress/sql/granted_by_support.sql b/src/test/regress/sql/granted_by_support.sql index 1261e78d4..afd3e416c 100644 --- a/src/test/regress/sql/granted_by_support.sql +++ b/src/test/regress/sql/granted_by_support.sql @@ -22,23 +22,52 @@ grant dist_role3 to dist_role4 with admin option; -- To test non-distributed grantor, set this option off for some roles. set citus.enable_create_role_propagation to off; grant non_dist_role1 to dist_role1 with admin option; +grant dist_role2 to non_dist_role1 with admin option; +grant dist_role2 to dist_role4 granted by non_dist_role1 ; reset citus.enable_create_role_propagation; -SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; +grant dist_role2 to "dist_role5'_test" granted by non_dist_role1;--will fail since non_dist_role1 does not exist on worker_1 - -grant dist_role2 to non_dist_role1 with admin option; - +\c - - - :master_port grant dist_role3 to "dist_role5'_test" granted by dist_role4; grant dist_role2 to "dist_role5'_test" granted by dist_role3; -grant dist_role2 to dist_role4 granted by non_dist_role1 ;--will not be propagated since grantor is non-distributed + + +--will fail since non_dist_role2 does not exist in worker_1 +grant dist_role2 to non_dist_role2 with admin option; +grant dist_role2 to dist_role4 granted by non_dist_role2 ; +grant non_dist_role2 to "dist_role5'_test"; + + +\c - - - :worker_1_port +create role non_dist_role2; + +\c - - - :master_port +--will be successful since non_dist_role has been created on worker_1 +grant dist_role2 to non_dist_role2 with admin option; +grant dist_role2 to dist_role4 granted by non_dist_role2 ; +grant non_dist_role2 to "dist_role5'_test"; grant dist_role4 to "dist_role5'_test" with admin option; +select result FROM run_command_on_all_nodes( + $$ + SELECT array_to_json(array_agg(row_to_json(t))) + FROM ( + SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option + FROM pg_auth_members + WHERE member::regrole::text in + ('dist_role1','non_dist_role1') + order by member::regrole::text, roleid::regrole::text + ) t + $$ +); + --below command propagates the non_dist_role1 since non_dist_role1 is already granted to dist_role1 --and citus sees granted roles as a dependency and citus propagates the dependent roles + grant dist_role4 to dist_role1 with admin option GRANTED BY "dist_role5'_test"; SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; @@ -54,6 +83,18 @@ 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" + + +set citus.enable_create_role_propagation to off; +create role non_dist_role_for_mds; + +grant dist_role3 to non_dist_role_for_mds with admin option; +grant non_dist_role_for_mds to dist_role1 with admin option; + +grant dist_role3 to dist_role4 with admin option GRANTED BY non_dist_role_for_mds; +reset citus.enable_create_role_propagation; + + select result FROM run_command_on_all_nodes( $$ SELECT array_to_json(array_agg(row_to_json(t))) @@ -61,7 +102,7 @@ select result FROM run_command_on_all_nodes( SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option FROM pg_auth_members WHERE member::regrole::text in - ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"') + ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"', 'non_dist_role_for_mds','non_dist_role1','non_dist_role2') order by member::regrole::text, roleid::regrole::text ) t $$ @@ -76,7 +117,7 @@ select result FROM run_command_on_all_nodes( SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option FROM pg_auth_members WHERE member::regrole::text in - ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"') + ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"','non_dist_role_for_mds','non_dist_role1','non_dist_role2') order by member::regrole::text, roleid::regrole::text ) t $$ @@ -84,7 +125,7 @@ select result FROM run_command_on_all_nodes( --clean all resources drop role dist_role1,dist_role2,dist_role3,dist_role4,"dist_role5'_test"; -drop role non_dist_role1; +drop role non_dist_role1,non_dist_role2,non_dist_role_for_mds; SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; reset citus.enable_create_role_propagation; @@ -96,143 +137,11 @@ select result FROM run_command_on_all_nodes( SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option FROM pg_auth_members WHERE member::regrole::text in - ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"') + ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"','non_dist_role_for_mds','non_dist_role1','non_dist_role2') order by member::regrole::text, roleid::regrole::text ) t $$ ); ---- Test 2: Tests from non-main database -set citus.enable_create_database_propagation to on; -create database test_granted_by_support; - -select 1 from citus_remove_node ('localhost',:worker_2_port); - -SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; - -\c test_granted_by_support ---here in below block since 'citus.enable_create_role_propagation to off ' is not effective, ---non_dist_role1 is being propagated to dist_role1 unlike main db scenario ---non_dist_role1 will be used for the test scenarios in this section -set citus.enable_create_role_propagation to off; -create role non_dist_role1; -reset citus.enable_create_role_propagation; - ---dropping since it isn't non-distributed as intended -drop role non_dist_role1; - ---creating non_dist_role1 again in main database ---This is actually non-distributed role -\c regression -set citus.enable_create_role_propagation to off; -create role non_dist_role1; -reset citus.enable_create_role_propagation; - -\c test_granted_by_support -create role dist_role1; -create role dist_role1; -create role dist_role2; -create role dist_role3; -create role dist_role4; -create role "dist_role5'_test"; -\c regression - - :master_port -SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; -\c test_granted_by_support -grant dist_role2 to dist_role1 with admin option; -grant dist_role2 to dist_role3 with admin option granted by dist_role1; -grant dist_role3 to dist_role4 with admin option; - --- With enable_create_role_propagation on, all grantees are propagated. --- To test non-distributed grantor, set this option off for some roles. - -\c regression -set citus.enable_create_role_propagation to off; -grant non_dist_role1 to dist_role1 with admin option; -reset citus.enable_create_role_propagation; - -\c test_granted_by_support -grant dist_role2 to non_dist_role1 with admin option; - -\c test_granted_by_support - - :worker_1_port -grant dist_role3 to "dist_role5'_test" granted by dist_role4; -grant dist_role2 to "dist_role5'_test" granted by dist_role3; -grant dist_role2 to dist_role4 granted by non_dist_role1 ;--will not be propagated since grantor is non-distributed -\c regression - - :master_port -SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; -\c test_granted_by_support - - :worker_1_port -grant dist_role4 to "dist_role5'_test" with admin option; - -\c regression - - :master_port -SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; - -\c test_granted_by_support - --- Unlike maindb scenario, non-maindb scenario doesn't propagate 'create non_dist_role1' to ---workers as it doesn't create dependency objects for non-distributed roles. -grant dist_role4 to dist_role1 with admin option GRANTED BY "dist_role5'_test"; - -\c regression - - :master_port -SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text= 'non_dist_role1' ORDER BY 1; - - -\c test_granted_by_support - - :worker_1_port -grant dist_role4 to dist_role3 with admin option GRANTED BY dist_role1; --fails since already dist_role3 granted to dist_role4 -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" - -\c regression - - :master_port - -select result FROM run_command_on_all_nodes( - $$ - SELECT array_to_json(array_agg(row_to_json(t))) - FROM ( - SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option - FROM pg_auth_members - WHERE member::regrole::text in - ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"') - order by member::regrole::text, roleid::regrole::text - ) t - $$ -); -select 1 from citus_add_node ('localhost',:worker_2_port); - -select result FROM run_command_on_all_nodes( - $$ - SELECT array_to_json(array_agg(row_to_json(t))) - FROM ( - SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option - FROM pg_auth_members - WHERE member::regrole::text in - ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"') - order by member::regrole::text, roleid::regrole::text - ) t - $$ -); - ---clean all resources - -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))) - FROM ( - SELECT member::regrole, roleid::regrole as role, grantor::regrole, admin_option - FROM pg_auth_members - WHERE member::regrole::text in - ('dist_role1','dist_role2','dist_role3','dist_role4','"role5''_test"') - order by member::regrole::text, roleid::regrole::text - ) t - $$ -); -reset citus.enable_create_database_propagation;