Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,4 @@ lib*.pc

# added output when modifying check_gucs_are_alphabetically_sorted.sh
guc.out
_codeql_detected_source_root
35 changes: 30 additions & 5 deletions src/backend/distributed/metadata/dependency.c
Original file line number Diff line number Diff line change
Expand Up @@ -1800,7 +1800,13 @@ GetDependentRoleIdsFDW(Oid FDWOid)

/*
* ExpandRolesToGroups returns a list of object addresses pointing to roles that roleid
* depends on.
* depends on. This includes:
* 1. Roles that roleid is a member of (membership->roleid)
* 2. Roles that are used as grantors for roleid's memberships (membership->grantor)
*
* The second dependency is critical for proper role propagation order when adding nodes.
* If role A is used as a grantor when granting role B to role C, then role A must be
* propagated before role C to ensure role A has the necessary admin option on role B.
*/
static List *
ExpandRolesToGroups(Oid roleid)
Expand All @@ -1819,15 +1825,34 @@ ExpandRolesToGroups(Oid roleid)
true, NULL, scanKeyCount, scanKey);

List *roles = NIL;
List *seenGrantors = 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);
/* Add the role being granted as a dependency */
DependencyDefinition *roleDefinition = palloc0(sizeof(DependencyDefinition));
roleDefinition->mode = DependencyObjectAddress;
ObjectAddressSet(roleDefinition->data.address, AuthIdRelationId, membership->roleid);
roles = lappend(roles, roleDefinition);

roles = lappend(roles, definition);
/*
* Add the grantor as a dependency if it's not the same as the current role.
* We track grantors separately to avoid adding duplicates.
* Note: For roles with many memberships, this O(n) duplicate check could be
* optimized with a hash table, but given that most roles have relatively few
* memberships, the list-based approach is simpler and sufficient.
*/
if (membership->grantor != roleid &&
!list_member_oid(seenGrantors, membership->grantor))
{
DependencyDefinition *grantorDefinition = palloc0(sizeof(DependencyDefinition));
grantorDefinition->mode = DependencyObjectAddress;
ObjectAddressSet(grantorDefinition->data.address, AuthIdRelationId,
membership->grantor);
roles = lappend(roles, grantorDefinition);
seenGrantors = lappend_oid(seenGrantors, membership->grantor);
}
}

systable_endscan(scanDescriptor);
Expand Down
44 changes: 44 additions & 0 deletions src/test/regress/sql/create_role_propagation.sql
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,48 @@ SELECT rolname FROM pg_authid WHERE rolname LIKE '%existing%' ORDER BY 1;
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;

DROP ROLE nondist_cascade_1, nondist_cascade_2, nondist_cascade_3, dist_cascade;
Loading