From ea33903c1c79ce6bb149017f8203bee4eb359e2d Mon Sep 17 00:00:00 2001 From: ChangAo Chen Date: Tue, 23 Dec 2025 10:59:28 +0800 Subject: [PATCH v2] Use ROLERECURSE_PRIVS in is_admin_of_role(). Currently we use different role recurse methods in is_admin_of_role() and select_best_admin(), this could lead to an unexpected behavior. For example, when is_admin_of_role() return true, select_best_admin() can still return InvalidOid. To fix it, we use ROLERECURSE_PRIVS in is_admin_of_role(), making it consistent with select_best_admin(). --- src/backend/utils/adt/acl.c | 2 +- src/test/regress/expected/create_role.out | 7 +++++++ src/test/regress/sql/create_role.sql | 6 ++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 05d48412f82..d22e696a46a 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -5422,7 +5422,7 @@ is_admin_of_role(Oid member, Oid role) if (member == role) return false; - (void) roles_is_member_of(member, ROLERECURSE_MEMBERS, role, &admin_role); + (void) roles_is_member_of(member, ROLERECURSE_PRIVS, role, &admin_role); return OidIsValid(admin_role); } diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out index 46d4f9efe99..f3f9e647c9d 100644 --- a/src/test/regress/expected/create_role.out +++ b/src/test/regress/expected/create_role.out @@ -218,6 +218,13 @@ DROP ROLE regress_nosuch_recursive; ERROR: role "regress_nosuch_recursive" does not exist DROP ROLE regress_nosuch_admin_recursive; ERROR: role "regress_nosuch_admin_recursive" does not exist +-- fail, regress_role_admin is admin of regress_createrole, regress_createrole is admin +-- of regress_plainrole, but regress_role_admin is not admin of regress_plainrole +DROP ROLE regress_plainrole; +ERROR: permission denied to drop role +DETAIL: Only roles with the CREATEROLE attribute and the ADMIN option on role "regress_plainrole" may drop this role. +-- ok, now regress_role_admin is admin of regress_plainrole +GRANT regress_createrole TO regress_role_admin WITH INHERIT TRUE; DROP ROLE regress_plainrole; -- must revoke privileges before dropping role REVOKE CREATE ON DATABASE regression FROM regress_createrole CASCADE; diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql index 4491a28a8ae..bbeb86b29cd 100644 --- a/src/test/regress/sql/create_role.sql +++ b/src/test/regress/sql/create_role.sql @@ -174,6 +174,12 @@ DROP ROLE regress_nosuch_super; DROP ROLE regress_nosuch_dbowner; DROP ROLE regress_nosuch_recursive; DROP ROLE regress_nosuch_admin_recursive; + +-- fail, regress_role_admin is admin of regress_createrole, regress_createrole is admin +-- of regress_plainrole, but regress_role_admin is not admin of regress_plainrole +DROP ROLE regress_plainrole; +-- ok, now regress_role_admin is admin of regress_plainrole +GRANT regress_createrole TO regress_role_admin WITH INHERIT TRUE; DROP ROLE regress_plainrole; -- must revoke privileges before dropping role -- 2.34.1