From d965e2867bda594e33dbd4a62cb56a9fb353387d Mon Sep 17 00:00:00 2001 From: ChangAo Chen Date: Wed, 29 Apr 2026 16:16:35 +0800 Subject: [PATCH v4] Fix error "no possible grantors". 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 introduce has_admin_option_on_role() which is consistent with select_best_admin(). This function is for privilege checking when doing membership changes to role so that we can report a detailed error message rather than "no possible grantors". For non-membership changes, we still use is_admin_of_role() so that we don't break existing scripts. --- src/backend/commands/user.c | 4 ++-- src/backend/utils/adt/acl.c | 24 ++++++++++++++++++++++++ src/include/utils/acl.h | 1 + 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index be11c49f919..d5e22b24e60 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -825,7 +825,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) } /* To add or drop members, you need ADMIN OPTION. */ - if (drolemembers && !is_admin_of_role(currentUserId, roleid)) + if (drolemembers && !has_admin_option_on_role(currentUserId, roleid)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to alter role"), @@ -2165,7 +2165,7 @@ check_role_membership_authorization(Oid currentUserId, Oid roleid, /* * Otherwise, must have admin option on the role to be changed. */ - if (!is_admin_of_role(currentUserId, roleid)) + if (!has_admin_option_on_role(currentUserId, roleid)) { if (is_grant) ereport(ERROR, diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 01caa12eca7..c5a750453df 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -5456,6 +5456,30 @@ is_admin_of_role(Oid member, Oid role) return OidIsValid(admin_role); } +/* + * Does member have ADMIN OPTION on role (directly or indirectly)? + * + * Like is_admin_of_role(), but we use ROLERECURSE_PRIVS rather than + * ROLERECURSE_MEMBERS. This function is for privilege checking when + * doing membership changes to role. For non-membership changes, use + * is_admin_of_role() instead. + */ +bool +has_admin_option_on_role(Oid member, Oid role) +{ + Oid admin_role; + + if (superuser_arg(member)) + return true; + + /* By policy, a role cannot have WITH ADMIN OPTION on itself. */ + if (member == role) + return false; + + (void) roles_is_member_of(member, ROLERECURSE_PRIVS, role, &admin_role); + return OidIsValid(admin_role); +} + /* * Find a role whose privileges "member" inherits which has ADMIN OPTION * on "role", ignoring super-userness. diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 0b9b04e78ee..198637aef77 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -216,6 +216,7 @@ extern void check_can_set_role(Oid member, Oid role); extern bool is_member_of_role(Oid member, Oid role); extern bool is_member_of_role_nosuper(Oid member, Oid role); extern bool is_admin_of_role(Oid member, Oid role); +extern bool has_admin_option_on_role(Oid member, Oid role); extern Oid select_best_admin(Oid member, Oid role); extern Oid get_role_oid(const char *rolname, bool missing_ok); extern Oid get_role_oid_or_public(const char *rolname); -- 2.34.1