Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

First seen: 2025-11-18 08:41:45+00:00 · Messages: 12 · Participants: 6

Latest Update

2026-05-07 · opus 4.7

Core Problem: Inconsistent Role Recursion Semantics in ADMIN Checks

This thread exposes a subtle but real semantic inconsistency introduced (or at least surfaced) by commit ce6b672e44 (v16), which overhauled role membership and the GRANTED BY machinery. The bug report centers on a mismatch between two internal helpers that are meant to cooperate when a user issues GRANT <role> TO <role>:

Because the gate is more permissive than the selector, a user can pass the authorization check but then fail to produce any valid grantor, triggering the internal elog(ERROR, "no possible grantors") — a message that was explicitly commented as "shouldn't happen" ("We shouldn't fail to find a best grantor, because we've already established that the current user has permission to perform the operation.").

Reproducer

CREATE USER u1; CREATE USER u2; CREATE USER u3; CREATE USER u4;
GRANT u2 TO u1 WITH ADMIN TRUE;
GRANT u3 TO u2 WITH ADMIN TRUE;
REVOKE INHERIT OPTION FOR u2 FROM u1;   -- u1 still a "member" of u2, but no longer inherits
SET SESSION AUTHORIZATION u1;
GRANT u3 TO u4;                          -- ERROR: no possible grantors

Semantically, once INHERIT OPTION is revoked, u1 should no longer be able to exercise u2's admin rights over u3. The expected user-visible error is permission denied to grant role u3, not an internal elog.

Why The Two Modes Exist

The three recursion modes (ROLERECURSE_MEMBERS, ROLERECURSE_PRIVS, ROLERECURSE_SETROLE) were introduced as part of Robert Haas's redesign to disentangle three orthogonal concepts:

  1. Membership — an administrative/organizational relationship (who is "in" a role).
  2. Privilege inheritance — whether the member automatically exercises the role's privileges (the INHERIT option).
  3. SET ROLE capability — whether the member can switch to that identity.

Before v16, these were fused. After the redesign, each grant carries independent INHERIT, SET, and ADMIN options. The three ROLERECURSE_* modes let callers ask the right question.

Robert confirms in the thread that his original mental model treated the MEMBERS-vs-PRIVS axis as orthogonal to the ADMIN-vs-ordinary-privilege axis. pg_role_aclcheck() reinforces this: it uses is_member_of_role() (MEMBERS) for ACL_CREATE, so using MEMBERS for ACL_GRANT_OPTION_FOR(ACL_CREATE) is internally consistent. The inconsistency arises only because check_role_grantor() uses has_privs_of_role() (PRIVS) and then select_best_admin() (PRIVS), while the upstream gate check_role_membership_authorization() uses is_admin_of_role() (MEMBERS).

Design Tension: The "User-Management Bot" Use Case

Robert reveals a design constraint that is not obvious from the code: during v16 development, a deliberate use case was preserved where an administrative bot role can manage other roles (create, alter, drop, grant membership) without automatically inheriting those roles' privileges. This requires is_admin_of_role() to follow MEMBERS edges — otherwise the bot would have to be given INHERIT on every role it manages, defeating the point.

Therefore Robert is "pretty strongly disinclined" to change is_admin_of_role() globally to use ROLERECURSE_PRIVS (which was OP's first proposed fix, and which Nathan also flagged as too restrictive — e.g., it would break DROP ROLE cases in existing deployments).

Proposed Resolution: Split the Concept

Robert proposes bifurcating the helper:

ChangAo's v4 patch implements this split (named has_admin_option_on_role() in the patch). The behavior change: in the reproducer, GRANT u3 TO u4 now raises the user-facing permission denied to grant role instead of the internal "no possible grantors".

Naming Debate

Nathan objects that has_admin_option_on_role() is indistinguishable in English from is_admin_of_role(). Jacob Champion proposes intent-based naming:

Or mirror the existing is_member_of_role() / has_privs_of_role() split with has_admin_privs_over_role(). This naming question matters because, as Nathan notes, "I fear for the day when I have to explain these subtle differences to a casual user" — the function names are the primary documentation of these semantics for future hackers.

Architectural Implications

  1. The elog(ERROR, "no possible grantors") was load-bearing documentation — its comment asserted an invariant that the code did not actually enforce. The fix tightens the invariant by making the gate and the selector use the same recursion mode.

  2. Three-axis role model is powerful but confusing. The fact that a committer (Robert) acknowledges his own "mental model here is actually a bit incoherent for ADMIN" is telling. The patch adds a fourth helper to an already dense family (is_member_of_role, has_privs_of_role, is_admin_of_role, now plus an admin-privs variant).

  3. Backward compatibility constrains the fix. A naive "just use PRIVS everywhere" fix would break the intentional management-bot pattern and cause silent permission regressions on upgrade. The split-function approach is narrowly targeted at the GRANT path where the inconsistency produces a genuinely broken error.

  4. This is a v16+ bug. Pre-v16 code simply checked is_admin_of_role() and recorded the current user as grantor; the concept of select_best_admin() choosing among inherited admin-capable roles is new. So the fix is unlikely to be back-patched beyond v16.

Whose Opinions Carry Weight