diff --git a/src/backend/distributed/metadata/dependency.c b/src/backend/distributed/metadata/dependency.c index 6932c453c8f..3ee43d76126 100644 --- a/src/backend/distributed/metadata/dependency.c +++ b/src/backend/distributed/metadata/dependency.c @@ -186,10 +186,13 @@ static void ApplyAddCitusDependedObjectsToDependencyList( static List * GetViewRuleReferenceDependencyList(Oid relationId); static List * ExpandCitusSupportedTypes(ObjectAddressCollector *collector, ObjectAddress target); +static List * ExpandCitusSupportedTypesForNodeActivation(ObjectAddressCollector * + collector, + ObjectAddress target); static List * ExpandForPgVanilla(ObjectAddressCollector *collector, ObjectAddress target); static List * GetDependentRoleIdsFDW(Oid FDWOid); -static List * ExpandRolesToGroups(Oid roleid); +static List * ExpandRolesToGroups(Oid roleid, bool includeGrantors); static ViewDependencyNode * BuildViewDependencyGraph(Oid relationId, HTAB *nodeMap); static bool IsObjectAddressOwnedByExtension(const ObjectAddress *target, ObjectAddress *extensionAddress); @@ -343,7 +346,7 @@ OrderObjectAddressListInDependencyOrder(List *objectAddressList) } RecurseObjectDependencies(*objectAddress, - &ExpandCitusSupportedTypes, + &ExpandCitusSupportedTypesForNodeActivation, &FollowAllSupportedDependencies, &ApplyAddToDependencyList, &collector); @@ -1544,9 +1547,15 @@ ExpandCitusSupportedTypes(ObjectAddressCollector *collector, ObjectAddress targe { /* * Roles are members of other roles. These relations are not recorded directly - * but can be deduced from pg_auth_members + * but can be deduced from pg_auth_members. + * + * Note: we intentionally do NOT include grantors here. Grantors are + * only relevant for ordering role creation during node activation + * (see ExpandCitusSupportedTypesForNodeActivation). Including them + * in the generic dependency graph would produce false-positive + * circular-dependency errors for legitimate mutual GRANTs. */ - return ExpandRolesToGroups(target.objectId); + return ExpandRolesToGroups(target.objectId, false); } case ExtensionRelationId: @@ -1738,6 +1747,32 @@ ExpandCitusSupportedTypes(ObjectAddressCollector *collector, ObjectAddress targe } +/* + * ExpandCitusSupportedTypesForNodeActivation is a variant of + * ExpandCitusSupportedTypes used only when ordering distributed objects for + * propagation to a newly-activated node. For roles, it additionally treats + * grantors of membership tuples as dependencies so that a role used as a + * grantor is created on the new node before the grantee role. This is what + * fixes issue #8425. + * + * This expansion must NOT be used by the generic dependency-collection paths + * (creation, cycle-detection, DDL propagation) because mutually-granted roles + * would appear to form a cycle and be rejected by + * DeferErrorIfCircularDependencyExists. + */ +static List * +ExpandCitusSupportedTypesForNodeActivation(ObjectAddressCollector *collector, + ObjectAddress target) +{ + if (target.classId == AuthIdRelationId) + { + return ExpandRolesToGroups(target.objectId, true); + } + + return ExpandCitusSupportedTypes(collector, target); +} + + /* * ExpandForPgVanilla only expands only comosite types because other types * will find their dependencies in pg_depend. The method should only be called by @@ -1801,10 +1836,19 @@ GetDependentRoleIdsFDW(Oid FDWOid) /* * ExpandRolesToGroups returns a list of object addresses pointing to roles that roleid - * depends on. + * depends on. This always includes: + * 1. Roles that roleid is a member of (membership->roleid) + * + * When includeGrantors is true, it additionally includes: + * 2. Roles that are used as grantors for roleid's memberships (membership->grantor) + * + * The grantor dependency is only used for ordering role propagation during node + * activation (see ExpandCitusSupportedTypesForNodeActivation). It must NOT be used + * in the generic dependency graph because legitimate mutual GRANTs between roles + * would otherwise be reported as circular dependencies. */ static List * -ExpandRolesToGroups(Oid roleid) +ExpandRolesToGroups(Oid roleid, bool includeGrantors) { Relation pgAuthMembers = table_open(AuthMemRelationId, AccessShareLock); HeapTuple tuple = NULL; @@ -1820,15 +1864,47 @@ ExpandRolesToGroups(Oid roleid) true, NULL, scanKeyCount, scanKey); List *roles = NIL; + + /* + * Track all role OIDs we have already emitted as dependencies so that + * parent roles and grantors are de-duplicated through a single set. + * A role can appear multiple times in pg_auth_members for the same + * member (different grantors), and the same OID may show up as both a + * parent role and a grantor; one DependencyDefinition per OID is enough. + * + * Note: For roles with many memberships this O(n) membership check could + * be replaced with a hash set, but in practice the number of memberships + * per role is small. + */ + List *seenRoleIds = NIL; while ((tuple = systable_getnext(scanDescriptor)) != NULL) { Form_pg_auth_members membership = (Form_pg_auth_members) GETSTRUCT(tuple); - DependencyDefinition *definition = palloc0(sizeof(DependencyDefinition)); - definition->mode = DependencyObjectAddress; - ObjectAddressSet(definition->data.address, AuthIdRelationId, membership->roleid); + Oid candidates[2] = { membership->roleid, membership->grantor }; + int numCandidates = includeGrantors ? 2 : 1; + for (int i = 0; i < numCandidates; i++) + { + Oid candidateOid = candidates[i]; - roles = lappend(roles, definition); + /* + * Skip self-references: a role cannot depend on itself (the + * parent-role case cannot hit this because pg_auth_members does + * not allow roleid == member, but the grantor case can). + */ + if (candidateOid == roleid || + !OidIsValid(candidateOid) || + list_member_oid(seenRoleIds, candidateOid)) + { + continue; + } + + DependencyDefinition *definition = palloc0(sizeof(DependencyDefinition)); + definition->mode = DependencyObjectAddress; + ObjectAddressSet(definition->data.address, AuthIdRelationId, candidateOid); + roles = lappend(roles, definition); + seenRoleIds = lappend_oid(seenRoleIds, candidateOid); + } } systable_endscan(scanDescriptor); diff --git a/src/test/regress/expected/create_role_propagation.out b/src/test/regress/expected/create_role_propagation.out index 59f7948a115..67912ae6c9a 100644 --- a/src/test/regress/expected/create_role_propagation.out +++ b/src/test/regress/expected/create_role_propagation.out @@ -696,3 +696,61 @@ SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; (0 rows) \c - - - :master_port +-- test interdependent roles with grantor dependencies +-- This test recreates the issue where role1 is used as a grantor for role2's grants, +-- but role1 hasn't been granted admin option on the parent role yet. +SELECT master_remove_node('localhost', :worker_2_port); + master_remove_node +--------------------------------------------------------------------- + +(1 row) + +CREATE ROLE read_only_role; +CREATE ROLE interdep_role1; +CREATE ROLE interdep_role2; +-- Grant read_only_role to interdep_role1 WITH ADMIN OPTION +-- This allows interdep_role1 to grant read_only_role to other roles +GRANT read_only_role TO interdep_role1 WITH ADMIN OPTION; +-- Grant read_only_role to interdep_role2, using interdep_role1 as the grantor +-- Also make interdep_role1 a member of interdep_role2 with admin option +GRANT read_only_role TO interdep_role2 GRANTED BY interdep_role1; +GRANT interdep_role1 TO interdep_role2 WITH ADMIN OPTION; +-- Verify the grant relationships on coordinator +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option +FROM pg_auth_members +WHERE roleid::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') + OR member::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') +ORDER BY role, member; + role | member | grantor | admin_option +--------------------------------------------------------------------- + interdep_role1 | interdep_role2 | postgres | t + read_only_role | interdep_role1 | postgres | t + read_only_role | interdep_role2 | interdep_role1 | f +(3 rows) + +-- Add worker_2 back - this should succeed with our fix +-- Before the fix, this would fail because interdep_role2 would be propagated before +-- interdep_role1's admin option on read_only_role was set up +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +-- Verify the grants were properly propagated to worker_2 +\c - - - :worker_2_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option +FROM pg_auth_members +WHERE roleid::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') + OR member::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') +ORDER BY role, member; + role | member | grantor | admin_option +--------------------------------------------------------------------- + interdep_role1 | interdep_role2 | postgres | t + read_only_role | interdep_role1 | postgres | t + read_only_role | interdep_role2 | postgres | f +(3 rows) + +\c - - - :master_port +-- Clean up interdependent roles +DROP ROLE interdep_role2, interdep_role1, read_only_role; diff --git a/src/test/regress/expected/create_role_propagation_0.out b/src/test/regress/expected/create_role_propagation_0.out new file mode 100644 index 00000000000..835effdf888 --- /dev/null +++ b/src/test/regress/expected/create_role_propagation_0.out @@ -0,0 +1,757 @@ +SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, rolpassword, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; + rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | rolpassword | rolvaliduntil +--------------------------------------------------------------------- +(0 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- +(0 rows) + +\c - - - :worker_1_port +SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, rolpassword, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; + rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | rolpassword | rolvaliduntil +--------------------------------------------------------------------- +(0 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- +(0 rows) + +\c - - - :master_port +CREATE ROLE create_role; +CREATE ROLE create_role_2; +CREATE USER create_user; +CREATE USER create_user_2; +CREATE GROUP create_group; +CREATE GROUP create_group_2; +-- show that create role fails if sysid option is given as non-int +CREATE ROLE create_role_sysid SYSID "123"; +ERROR: syntax error at or near ""123"" +-- show that create role accepts sysid option as int +CREATE ROLE create_role_sysid SYSID 123; +NOTICE: SYSID can no longer be specified +SELECT master_remove_node('localhost', :worker_2_port); + master_remove_node +--------------------------------------------------------------------- + +(1 row) + +CREATE ROLE create_role_with_everything SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN REPLICATION BYPASSRLS CONNECTION LIMIT 105 PASSWORD 'strong_password123^' VALID UNTIL '2045-05-05 00:00:00.00+00' IN ROLE create_role, create_group ROLE create_user, create_group_2 ADMIN create_role_2, create_user_2; +CREATE ROLE create_role_with_nothing NOSUPERUSER NOCREATEDB NOCREATEROLE NOINHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 3 PASSWORD 'weakpassword' VALID UNTIL '2015-05-05 00:00:00.00+00'; +-- show that creating role from worker node is only allowed when create role +-- propagation is off +\c - - - :worker_1_port +CREATE ROLE role_on_worker; +ERROR: operation is not allowed on this node +HINT: Connect to the coordinator and run it again. +BEGIN; +SET citus.enable_create_role_propagation TO off; +CREATE ROLE role_on_worker; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +ROLLBACK; +\c - - - :master_port +-- edge case role names +CREATE ROLE "create_role'edge"; +CREATE ROLE "create_role""edge"; +-- test grant role +GRANT create_group TO create_role; +GRANT create_group TO create_role_2 WITH ADMIN OPTION; +SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, (rolpassword != '') as pass_not_empty, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; + rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | pass_not_empty | rolvaliduntil +--------------------------------------------------------------------- + create_group | f | t | f | f | f | f | f | -1 | | + create_group_2 | f | t | f | f | f | f | f | -1 | | + create_role | f | t | f | f | f | f | f | -1 | | + create_role"edge | f | t | f | f | f | f | f | -1 | | + create_role'edge | f | t | f | f | f | f | f | -1 | | + create_role_2 | f | t | f | f | f | f | f | -1 | | + create_role_sysid | f | t | f | f | f | f | f | -1 | | + create_role_with_everything | t | t | t | t | t | t | t | 105 | t | Thu May 04 17:00:00 2045 PDT + create_role_with_nothing | f | f | f | f | f | f | f | 3 | t | Mon May 04 17:00:00 2015 PDT + create_user | f | t | f | f | t | f | f | -1 | | + create_user_2 | f | t | f | f | t | f | f | -1 | | +(11 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + create_group | create_role | postgres | f + create_group | create_role_2 | postgres | t + create_group | create_role_with_everything | postgres | f + create_role | create_role_with_everything | postgres | f + create_role_with_everything | create_group_2 | postgres | f + create_role_with_everything | create_role_2 | postgres | t + create_role_with_everything | create_user | postgres | f + create_role_with_everything | create_user_2 | postgres | t +(8 rows) + +\c - - - :worker_1_port +SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, (rolpassword != '') as pass_not_empty, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; + rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | pass_not_empty | rolvaliduntil +--------------------------------------------------------------------- + create_group | f | t | f | f | f | f | f | -1 | | + create_group_2 | f | t | f | f | f | f | f | -1 | | + create_role | f | t | f | f | f | f | f | -1 | | + create_role"edge | f | t | f | f | f | f | f | -1 | | + create_role'edge | f | t | f | f | f | f | f | -1 | | + create_role_2 | f | t | f | f | f | f | f | -1 | | + create_role_sysid | f | t | f | f | f | f | f | -1 | | + create_role_with_everything | t | t | t | t | t | t | t | 105 | t | Thu May 04 17:00:00 2045 PDT + create_role_with_nothing | f | f | f | f | f | f | f | 3 | t | Mon May 04 17:00:00 2015 PDT + create_user | f | t | f | f | t | f | f | -1 | | + create_user_2 | f | t | f | f | t | f | f | -1 | | +(11 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + create_group | create_role | postgres | f + create_group | create_role_2 | postgres | t + create_group | create_role_with_everything | postgres | f + create_role | create_role_with_everything | postgres | f + create_role_with_everything | create_group_2 | postgres | f + create_role_with_everything | create_role_2 | postgres | t + create_role_with_everything | create_user | postgres | f + create_role_with_everything | create_user_2 | postgres | t +(8 rows) + +\c - - - :master_port +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +\c - - - :worker_2_port +SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, (rolpassword != '') as pass_not_empty, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; + rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | pass_not_empty | rolvaliduntil +--------------------------------------------------------------------- + create_group | f | t | f | f | f | f | f | -1 | | infinity + create_group_2 | f | t | f | f | f | f | f | -1 | | infinity + create_role | f | t | f | f | f | f | f | -1 | | infinity + create_role"edge | f | t | f | f | f | f | f | -1 | | infinity + create_role'edge | f | t | f | f | f | f | f | -1 | | infinity + create_role_2 | f | t | f | f | f | f | f | -1 | | infinity + create_role_sysid | f | t | f | f | f | f | f | -1 | | infinity + create_role_with_everything | t | t | t | t | t | t | t | 105 | t | Thu May 04 17:00:00 2045 PDT + create_role_with_nothing | f | f | f | f | f | f | f | 3 | t | Mon May 04 17:00:00 2015 PDT + create_user | f | t | f | f | t | f | f | -1 | | infinity + create_user_2 | f | t | f | f | t | f | f | -1 | | infinity +(11 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + create_group | create_role | postgres | f + create_group | create_role_2 | postgres | t + create_group | create_role_with_everything | postgres | f + create_role | create_role_with_everything | postgres | f + create_role_with_everything | create_group_2 | postgres | f + create_role_with_everything | create_role_2 | postgres | t + create_role_with_everything | create_user | postgres | f + create_role_with_everything | create_user_2 | postgres | t +(8 rows) + +\c - - - :master_port +DROP ROLE create_role_with_everything; +REVOKE create_group FROM create_role; +REVOKE ADMIN OPTION FOR create_group FROM create_role_2; +\c - - - :master_port +SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, (rolpassword != '') as pass_not_empty, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; + rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | pass_not_empty | rolvaliduntil +--------------------------------------------------------------------- + create_group | f | t | f | f | f | f | f | -1 | | + create_group_2 | f | t | f | f | f | f | f | -1 | | + create_role | f | t | f | f | f | f | f | -1 | | + create_role"edge | f | t | f | f | f | f | f | -1 | | + create_role'edge | f | t | f | f | f | f | f | -1 | | + create_role_2 | f | t | f | f | f | f | f | -1 | | + create_role_sysid | f | t | f | f | f | f | f | -1 | | + create_role_with_nothing | f | f | f | f | f | f | f | 3 | t | Mon May 04 17:00:00 2015 PDT + create_user | f | t | f | f | t | f | f | -1 | | + create_user_2 | f | t | f | f | t | f | f | -1 | | +(10 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + create_group | create_role_2 | postgres | f +(1 row) + +\c - - - :worker_1_port +SELECT rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, (rolpassword != '') as pass_not_empty, rolvaliduntil FROM pg_authid WHERE rolname LIKE 'create\_%' ORDER BY rolname; + rolname | rolsuper | rolinherit | rolcreaterole | rolcreatedb | rolcanlogin | rolreplication | rolbypassrls | rolconnlimit | pass_not_empty | rolvaliduntil +--------------------------------------------------------------------- + create_group | f | t | f | f | f | f | f | -1 | | + create_group_2 | f | t | f | f | f | f | f | -1 | | + create_role | f | t | f | f | f | f | f | -1 | | + create_role"edge | f | t | f | f | f | f | f | -1 | | + create_role'edge | f | t | f | f | f | f | f | -1 | | + create_role_2 | f | t | f | f | f | f | f | -1 | | + create_role_sysid | f | t | f | f | f | f | f | -1 | | + create_role_with_nothing | f | f | f | f | f | f | f | 3 | t | Mon May 04 17:00:00 2015 PDT + create_user | f | t | f | f | t | f | f | -1 | | + create_user_2 | f | t | f | f | t | f | f | -1 | | +(10 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE 'create\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + create_group | create_role_2 | postgres | f +(1 row) + +\c - - - :master_port +-- test grants with distributed and non-distributed roles +SELECT master_remove_node('localhost', :worker_2_port); + master_remove_node +--------------------------------------------------------------------- + +(1 row) + +CREATE ROLE dist_role_1 SUPERUSER; +CREATE ROLE dist_role_2; +CREATE ROLE dist_role_3; +CREATE ROLE dist_role_4; +SET citus.enable_create_role_propagation TO OFF; +CREATE ROLE non_dist_role_1 SUPERUSER; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +CREATE ROLE non_dist_role_2; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +CREATE ROLE non_dist_role_3; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +CREATE ROLE non_dist_role_4; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +SET citus.enable_create_role_propagation TO ON; +SET ROLE dist_role_1; +GRANT non_dist_role_1 TO non_dist_role_2; +SET citus.enable_create_role_propagation TO OFF; +SET ROLE non_dist_role_1; +GRANT dist_role_1 TO dist_role_2; +RESET ROLE; +SET citus.enable_create_role_propagation TO ON; +GRANT dist_role_3 TO non_dist_role_3; +GRANT non_dist_role_4 TO dist_role_4; +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +SELECT roleid::regrole::text AS role, member::regrole::text, (grantor::regrole::text IN ('postgres', 'non_dist_role_1', 'dist_role_1')) AS grantor, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + dist_role_1 | dist_role_2 | t | f + dist_role_3 | non_dist_role_3 | t | f + non_dist_role_1 | non_dist_role_2 | t | f + non_dist_role_4 | dist_role_4 | t | f +(4 rows) + +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist\_%' ORDER BY 1; + objid +--------------------------------------------------------------------- + dist_role_1 + dist_role_2 + dist_role_3 + dist_role_4 + non_dist_role_4 +(5 rows) + +\c - - - :worker_1_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + non_dist_role_4 | dist_role_4 | postgres | f +(1 row) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_role_1 + dist_role_2 + dist_role_3 + dist_role_4 + non_dist_role_4 +(5 rows) + +\c - - - :worker_2_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + dist_role_1 | dist_role_2 | postgres | f + non_dist_role_4 | dist_role_4 | postgres | f +(2 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_role_1 + dist_role_2 + dist_role_3 + dist_role_4 + non_dist_role_1 + non_dist_role_4 +(6 rows) + +\c - - - :master_port +DROP ROLE dist_role_3, non_dist_role_3, dist_role_4, non_dist_role_4; +-- test grant with multiple mixed roles +CREATE ROLE dist_mixed_1; +CREATE ROLE dist_mixed_2; +CREATE ROLE dist_mixed_3; +CREATE ROLE dist_mixed_4; +SET citus.enable_create_role_propagation TO OFF; +CREATE ROLE nondist_mixed_1; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +CREATE ROLE nondist_mixed_2; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_mixed%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- +(0 rows) + +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist\_mixed%' ORDER BY 1; + objid +--------------------------------------------------------------------- + dist_mixed_1 + dist_mixed_2 + dist_mixed_3 + dist_mixed_4 +(4 rows) + +\c - - - :worker_1_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_mixed%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- +(0 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_mixed%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_mixed_1 + dist_mixed_2 + dist_mixed_3 + dist_mixed_4 +(4 rows) + +\c - - - :master_port +SELECT master_remove_node('localhost', :worker_2_port); + master_remove_node +--------------------------------------------------------------------- + +(1 row) + +GRANT dist_mixed_1, dist_mixed_2, nondist_mixed_1 TO dist_mixed_3, dist_mixed_4, nondist_mixed_2; +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_mixed%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + dist_mixed_1 | dist_mixed_3 | postgres | f + dist_mixed_1 | dist_mixed_4 | postgres | f + dist_mixed_1 | nondist_mixed_2 | postgres | f + dist_mixed_2 | dist_mixed_3 | postgres | f + dist_mixed_2 | dist_mixed_4 | postgres | f + dist_mixed_2 | nondist_mixed_2 | postgres | f + nondist_mixed_1 | dist_mixed_3 | postgres | f + nondist_mixed_1 | dist_mixed_4 | postgres | f + nondist_mixed_1 | nondist_mixed_2 | postgres | f +(9 rows) + +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist\_mixed%' ORDER BY 1; + objid +--------------------------------------------------------------------- + dist_mixed_1 + dist_mixed_2 + dist_mixed_3 + dist_mixed_4 + nondist_mixed_1 +(5 rows) + +\c - - - :worker_1_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_mixed%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + dist_mixed_1 | dist_mixed_3 | postgres | f + dist_mixed_1 | dist_mixed_4 | postgres | f + dist_mixed_2 | dist_mixed_3 | postgres | f + dist_mixed_2 | dist_mixed_4 | postgres | f + nondist_mixed_1 | dist_mixed_3 | postgres | f + nondist_mixed_1 | dist_mixed_4 | postgres | f +(6 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_mixed%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_mixed_1 + dist_mixed_2 + dist_mixed_3 + dist_mixed_4 + nondist_mixed_1 +(5 rows) + +\c - - - :worker_2_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_mixed%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + dist_mixed_1 | dist_mixed_3 | postgres | f + dist_mixed_1 | dist_mixed_4 | postgres | f + dist_mixed_2 | dist_mixed_3 | postgres | f + dist_mixed_2 | dist_mixed_4 | postgres | f + nondist_mixed_1 | dist_mixed_3 | postgres | f + nondist_mixed_1 | dist_mixed_4 | postgres | f +(6 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_mixed%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_mixed_1 + dist_mixed_2 + dist_mixed_3 + dist_mixed_4 + nondist_mixed_1 +(5 rows) + +\c - - - :master_port +DROP ROLE dist_mixed_1, dist_mixed_2, dist_mixed_3, dist_mixed_4, nondist_mixed_1, nondist_mixed_2; +-- test drop multiple roles with non-distributed roles +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist%' ORDER BY 1; + objid +--------------------------------------------------------------------- + dist_role_1 + dist_role_2 +(2 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_role_1 + dist_role_2 + non_dist_role_1 + non_dist_role_2 +(4 rows) + +\c - - - :worker_1_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_role_1 + dist_role_2 +(2 rows) + +\c - - - :master_port +DROP ROLE dist_role_1, non_dist_role_1, dist_role_2, non_dist_role_2; +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist%' ORDER BY 1; + objid +--------------------------------------------------------------------- +(0 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist%' ORDER BY 1; + rolname +--------------------------------------------------------------------- +(0 rows) + +\c - - - :worker_1_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist%' ORDER BY 1; + rolname +--------------------------------------------------------------------- +(0 rows) + +\c - - - :master_port +-- test alter part of create or alter role +SELECT master_remove_node('localhost', :worker_2_port); + master_remove_node +--------------------------------------------------------------------- + +(1 row) + +DROP ROLE create_role, create_role_2; +\c - - - :worker_2_port +SELECT rolname, rolcanlogin FROM pg_authid WHERE rolname = 'create_role' OR rolname = 'create_role_2' ORDER BY rolname; + rolname | rolcanlogin +--------------------------------------------------------------------- + create_role | f + create_role_2 | f +(2 rows) + +\c - - - :master_port +CREATE ROLE create_role LOGIN; +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +CREATE ROLE create_role_2 LOGIN; +\c - - - :worker_2_port +SELECT rolname, rolcanlogin FROM pg_authid WHERE rolname = 'create_role' OR rolname = 'create_role_2' ORDER BY rolname; + rolname | rolcanlogin +--------------------------------------------------------------------- + create_role | t + create_role_2 | t +(2 rows) + +\c - - - :master_port +-- test cascading grants +SET citus.enable_create_role_propagation TO OFF; +CREATE ROLE nondist_cascade_1; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +CREATE ROLE nondist_cascade_2; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +CREATE ROLE nondist_cascade_3; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +SET citus.enable_create_role_propagation TO ON; +CREATE ROLE dist_cascade; +GRANT nondist_cascade_1 TO nondist_cascade_2; +GRANT nondist_cascade_2 TO nondist_cascade_3; +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%cascade%' ORDER BY 1; + objid +--------------------------------------------------------------------- + dist_cascade +(1 row) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%cascade%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + nondist_cascade_1 | nondist_cascade_2 | postgres | f + nondist_cascade_2 | nondist_cascade_3 | postgres | f +(2 rows) + +\c - - - :worker_1_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%cascade%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_cascade +(1 row) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%cascade%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- +(0 rows) + +\c - - - :master_port +SELECT master_remove_node('localhost', :worker_2_port); + master_remove_node +--------------------------------------------------------------------- + +(1 row) + +GRANT nondist_cascade_3 TO dist_cascade; +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%cascade%' ORDER BY 1; + objid +--------------------------------------------------------------------- + nondist_cascade_1 + nondist_cascade_2 + nondist_cascade_3 + dist_cascade +(4 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%cascade%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + nondist_cascade_1 | nondist_cascade_2 | postgres | f + nondist_cascade_2 | nondist_cascade_3 | postgres | f + nondist_cascade_3 | dist_cascade | postgres | f +(3 rows) + +\c - - - :worker_1_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%cascade%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_cascade + nondist_cascade_1 + nondist_cascade_2 + nondist_cascade_3 +(4 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%cascade%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + nondist_cascade_1 | nondist_cascade_2 | postgres | f + nondist_cascade_2 | nondist_cascade_3 | postgres | f + nondist_cascade_3 | dist_cascade | postgres | f +(3 rows) + +\c - - - :worker_2_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%cascade%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + dist_cascade + nondist_cascade_1 + nondist_cascade_2 + nondist_cascade_3 +(4 rows) + +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%cascade%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- + nondist_cascade_1 | nondist_cascade_2 | postgres | f + nondist_cascade_2 | nondist_cascade_3 | postgres | f + nondist_cascade_3 | dist_cascade | postgres | f +(3 rows) + +\c - - - :master_port +DROP ROLE create_role, create_role_2, create_group, create_group_2, create_user, create_user_2, create_role_with_nothing, create_role_sysid, "create_role'edge", "create_role""edge"; +-- test grant non-existing roles +CREATE ROLE existing_role_1; +CREATE ROLE existing_role_2; +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%existing%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- +(0 rows) + +GRANT existing_role_1, nonexisting_role_1 TO existing_role_2, nonexisting_role_2; +ERROR: role "nonexisting_role_2" does not exist +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%existing%' ORDER BY 1, 2; + role | member | grantor | admin_option +--------------------------------------------------------------------- +(0 rows) + +-- test drop non-existing roles +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%existing%' ORDER BY 1; + objid +--------------------------------------------------------------------- + existing_role_1 + existing_role_2 +(2 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + existing_role_1 + existing_role_2 +(2 rows) + +\c - - - :worker_1_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + existing_role_1 + existing_role_2 +(2 rows) + +\c - - - :master_port +DROP ROLE existing_role_1, existing_role_2, nonexisting_role_1, nonexisting_role_2; +ERROR: role "nonexisting_role_1" does not exist +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%existing%' ORDER BY 1; + objid +--------------------------------------------------------------------- + existing_role_1 + existing_role_2 +(2 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + existing_role_1 + existing_role_2 +(2 rows) + +\c - - - :worker_1_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; + rolname +--------------------------------------------------------------------- + existing_role_1 + existing_role_2 +(2 rows) + +\c - - - :master_port +DROP ROLE IF EXISTS existing_role_1, existing_role_2, nonexisting_role_1, nonexisting_role_2; +NOTICE: role "nonexisting_role_1" does not exist, skipping +NOTICE: role "nonexisting_role_2" does not exist, skipping +SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%existing%' ORDER BY 1; + objid +--------------------------------------------------------------------- +(0 rows) + +SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; + rolname +--------------------------------------------------------------------- +(0 rows) + +\c - - - :worker_1_port +SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; + rolname +--------------------------------------------------------------------- +(0 rows) + +\c - - - :master_port +-- test interdependent roles with grantor dependencies +-- This test recreates the issue where role1 is used as a grantor for role2's grants, +-- but role1 hasn't been granted admin option on the parent role yet. +SELECT master_remove_node('localhost', :worker_2_port); + master_remove_node +--------------------------------------------------------------------- + +(1 row) + +CREATE ROLE read_only_role; +CREATE ROLE interdep_role1; +CREATE ROLE interdep_role2; +-- Grant read_only_role to interdep_role1 WITH ADMIN OPTION +-- This allows interdep_role1 to grant read_only_role to other roles +GRANT read_only_role TO interdep_role1 WITH ADMIN OPTION; +-- Grant read_only_role to interdep_role2, using interdep_role1 as the grantor +-- Also make interdep_role1 a member of interdep_role2 with admin option +GRANT read_only_role TO interdep_role2 GRANTED BY interdep_role1; +GRANT interdep_role1 TO interdep_role2 WITH ADMIN OPTION; +-- Verify the grant relationships on coordinator +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option +FROM pg_auth_members +WHERE roleid::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') + OR member::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') +ORDER BY role, member; + role | member | grantor | admin_option +--------------------------------------------------------------------- + interdep_role1 | interdep_role2 | postgres | t + read_only_role | interdep_role1 | postgres | t + read_only_role | interdep_role2 | interdep_role1 | f +(3 rows) + +-- Add worker_2 back - this should succeed with our fix +-- Before the fix, this would fail because interdep_role2 would be propagated before +-- interdep_role1's admin option on read_only_role was set up +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +-- Verify the grants were properly propagated to worker_2 +\c - - - :worker_2_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option +FROM pg_auth_members +WHERE roleid::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') + OR member::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') +ORDER BY role, member; + role | member | grantor | admin_option +--------------------------------------------------------------------- + interdep_role1 | interdep_role2 | postgres | t + read_only_role | interdep_role1 | postgres | t + read_only_role | interdep_role2 | postgres | f +(3 rows) + +\c - - - :master_port +-- Clean up interdependent roles +DROP ROLE interdep_role2, interdep_role1, read_only_role; diff --git a/src/test/regress/sql/create_role_propagation.sql b/src/test/regress/sql/create_role_propagation.sql index 027e4f72e74..c892a1661f9 100644 --- a/src/test/regress/sql/create_role_propagation.sql +++ b/src/test/regress/sql/create_role_propagation.sql @@ -277,3 +277,47 @@ SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; \c - - - :worker_1_port SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1; \c - - - :master_port + +-- test interdependent roles with grantor dependencies +-- This test recreates the issue where role1 is used as a grantor for role2's grants, +-- but role1 hasn't been granted admin option on the parent role yet. + +SELECT master_remove_node('localhost', :worker_2_port); + +CREATE ROLE read_only_role; +CREATE ROLE interdep_role1; +CREATE ROLE interdep_role2; + +-- Grant read_only_role to interdep_role1 WITH ADMIN OPTION +-- This allows interdep_role1 to grant read_only_role to other roles +GRANT read_only_role TO interdep_role1 WITH ADMIN OPTION; + +-- Grant read_only_role to interdep_role2, using interdep_role1 as the grantor +-- Also make interdep_role1 a member of interdep_role2 with admin option +GRANT read_only_role TO interdep_role2 GRANTED BY interdep_role1; +GRANT interdep_role1 TO interdep_role2 WITH ADMIN OPTION; + +-- Verify the grant relationships on coordinator +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option +FROM pg_auth_members +WHERE roleid::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') + OR member::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') +ORDER BY role, member; + +-- Add worker_2 back - this should succeed with our fix +-- Before the fix, this would fail because interdep_role2 would be propagated before +-- interdep_role1's admin option on read_only_role was set up +SELECT 1 FROM master_add_node('localhost', :worker_2_port); + +-- Verify the grants were properly propagated to worker_2 +\c - - - :worker_2_port +SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option +FROM pg_auth_members +WHERE roleid::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') + OR member::regrole::text IN ('read_only_role', 'interdep_role1', 'interdep_role2') +ORDER BY role, member; + +\c - - - :master_port + +-- Clean up interdependent roles +DROP ROLE interdep_role2, interdep_role1, read_only_role;