Horton

Thread

  1. Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    cca5507 <cca5507@qq.com> — 2025-11-18T08:41:45Z

    Hi,


When reading the code, I find is_admin_of_role()&nbsp;use ROLERECURSE_MEMBERS while select_best_admin()&nbsp;use ROLERECURSE_PRIVS.


Why they are dismatch?


The following case will have is_admin_of_role() return true and select_best_admin() return InvalidOid:


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 ;
set session authorization u1;
grant u3 to u4;


The "grant u3 to u4;" will report error "no possible grantors" rather than "permission denied to grant role".


Is this the expected behavior?


--
Regards,
ChangAo Chen

  2. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    cca5507 <cca5507@qq.com> — 2025-11-19T03:42:59Z

    Hi,


According to the comment in check_role_grantor():


            /*
             * Otherwise, the grantor must either have ADMIN OPTION on the role or
             * inherit the privileges of a role which does. In the former case,
             * record the grantor as the current user; in the latter, pick one of
             * the roles that is "most directly" inherited by the current role
             * (i.e. fewest "hops").
             *
             * (We shouldn't fail to find a best grantor, because we've already
             * established that the current user has permission to perform the
             * operation.)
             */
            grantorId = select_best_admin(currentUserId, roleid);
            if (!OidIsValid(grantorId))
                  elog(ERROR, "no possible grantors");


But the "no possible grantors" error can happen in my test case.


The main reason is that is_admin_of_role() and select_best_admin() use different role recurse methods.


I think they should keep consistent, maybe both use ROLERECURSE_PRIVS? Thoughts?


--
Regards,
ChangAo Chen

  3. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    cca5507 <cca5507@qq.com> — 2025-11-24T03:22:08Z

    Hi,


I attach a small patch for this.


Looking forward to your review.


--
Regards,
ChangAo Chen

  4. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    cca5507 <cca5507@qq.com> — 2025-12-23T03:04:55Z

    Hi,

Fix "create_role.sql" in v2.

--
Regards,
ChangAo Chen

  5. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    Chao Li <li.evan.chao@gmail.com> — 2025-12-23T04:05:11Z

    
> On Nov 18, 2025, at 16:41, cca5507 <cca5507@qq.com> wrote:
> 
> Hi,
> 
> When reading the code, I find is_admin_of_role() use ROLERECURSE_MEMBERS while select_best_admin() use ROLERECURSE_PRIVS.
> 
> Why they are dismatch?
> 
> The following case will have is_admin_of_role() return true and select_best_admin() return InvalidOid:
> 
> 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 ;
> set session authorization u1;
> grant u3 to u4;
> 
> The "grant u3 to u4;" will report error "no possible grantors" rather than "permission denied to grant role".
> 
> Is this the expected behavior?
> 

Let’s do a simpler test:
```
create user u1;
create user u2;
create user u3;
set session authorization u1;
grant u2 to u3;
```

In this test, u1 doesn’t administer u2, so when u1 runs “grant u2 to u3”, the error is “permission denied to grant role u2”. 

Then back to ChangAo’s test, after revoking u2 from u1, u1 no longer can administer u3, so that when u1 runs “grant u2 to u3”, the error should also be “permission denied”. From this perspective, the current error “no possible grantors” is unexpected.

Reviewing v2, overall LGTM, my only nitpick is:
```
+-- ok, now regress_role_admin is admin of regress_plainrole
```

In this test comment, “now” is not needed. I think “now” is just from this patch’s perspective, but in the scope of the test script, this test case is just one test step. None of other comments in the same file have wordings of “now”, “then” or so.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  6. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather thanROLERECURSE_PRIVS?

    cca5507 <cca5507@qq.com> — 2025-12-23T06:21:51Z

    Hi,

Update some comments in v3.

(CC Pretham, we discuss it in [1])

[1]: https://www.postgresql.org/message-id/flat/CAJUn_kN%2BMhbb8fYP5xxQCq1KEziOinM6HgYx4ts_pPDnQ2y1nQ%40mail.gmail.com

--
Regards,
ChangAo Chen

  7. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather thanROLERECURSE_PRIVS?

    preTham <prezza672@gmail.com> — 2025-12-24T07:05:31Z

    Hi,
the "permission denied" error does make sense from a user perspective as it
details that.only roles with admin option for u2 can grant it. the patch
also LGTM.

Regards,
Pretham

On Tue, Dec 23, 2025 at 11:52 AM cca5507 <cca5507@qq.com> wrote:

> Hi,
>
> Update some comments in v3.
>
> (CC Pretham, we discuss it in [1])
>
> [1]:
> https://www.postgresql.org/message-id/flat/CAJUn_kN%2BMhbb8fYP5xxQCq1KEziOinM6HgYx4ts_pPDnQ2y1nQ%40mail.gmail.com
>
> --
> Regards,
> ChangAo Chen
>

  8. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    Nathan Bossart <nathandbossart@gmail.com> — 2026-05-06T16:48:27Z

    On Wed, Apr 29, 2026 at 04:46:05PM +0800, cca5507 wrote:
>> I'm pretty strongly disinclined to change the meaning of
>> is_admin_of_role() in released code. That affects more than this call
>> site. When this code was under development, one of the use cases that
>> was booted was a user management bot who should be able to run ALTER
>> ROLE but should not automatically exercise the privilege of any
>> created roles. If we standardize on ROLERECURSE_PRIVS, that use case
>> doesn't work any more. You now have to inherit a role's privileges or
>> AlterRole() will fail.
> 
> This use case makes sense to me.
> 
>> One idea could be that non-membership changes to roles continue to
>> work as they do today, but membership changes use ROLERECURSE_PRIVS.
>> So we'd have is_admin_of_role() and inherits_admin_privs_for_role()
>> and be careful to use the right one in each case. This seems a little
>> weird, but I'm not sure what would be better.
> 
> Attach a patch done like this.

The patch seems to resolve the reported case.  I don't like how the new
function is named "has_admin_option_on_role()" because it sounds like it
means the exact same thing as "is_admin_of_role()".  IMHO Robert's
suggestion of inherits_admin_privs_of_role() would be better.

I don't have any better ideas for how to solve it, but I also fear for the
day when I have to explain these subtle differences in behavior to a casual
user...

-- 
nathan

  9. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-05-06T18:56:35Z

    On Wed, May 6, 2026 at 9:48 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
> I don't have any better ideas for how to solve it, but I also fear for the
> day when I have to explain these subtle differences in behavior to a casual
> user...

(spitballing)

Would it help to name the functions according to what they intend to
let the caller do? is_admin_of_role() -> can_administer_role();
inherits_admin_privs_of_role() -> is_administering_role()?

Alternatively, we have the is_member_* vs has_privs_* division
already, so has_admin_privs_over_role(), or something?

--Jacob

  10. Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

    cca5507 <cca5507@qq.com> — 2026-05-07T04:04:44Z

    > I'm pretty strongly disinclined to change the meaning of
> is_admin_of_role() in released code. That affects more than this call
> site. When this code was under development, one of the use cases that
> was booted was a user management bot who should be able to run ALTER
> ROLE but should not automatically exercise the privilege of any
> created roles. If we standardize on ROLERECURSE_PRIVS, that use case
> doesn't work any more. You now have to inherit a role's privileges or
> AlterRole() will fail.

After thinking more about this case, it seems still work even if we use
ROLERECURSE_PRIVS in is_admin_of_role():

```
postgres=# create role bot createrole;
CREATE ROLE
postgres=# set session authorization bot;
SET
postgres=> create role user1;
CREATE ROLE
postgres=> reset session authorization;
RESET
postgres=# select pg_has_role('bot', 'user1', 'USAGE');
 pg_has_role 
-------------
 f
(1 row)

postgres=# select pg_has_role('bot', 'user1', 'MEMBER');
 pg_has_role 
-------------
 t
(1 row)

postgres=# select pg_has_role('bot', 'user1', 'MEMBER WITH ADMIN OPTION');
 pg_has_role 
-------------
 t
(1 row)

postgres=# select * from pg_auth_members where member = (select oid from pg_authid where rolname = 'bot');
  oid  | roleid | member | grantor | admin_option | inherit_option | set_option 
-------+--------+--------+---------+--------------+----------------+------------
 16393 |  16392 |  16391 |      10 | t            | f              | f
(1 row)

```

The bot doesn't have privs of user1 by default, but is admin of user1. Changing
is_admin_of_role() to use ROLERECURSE_PRIVS only affects the case of indirect
inheritance. Do I miss something?

--
Regards,
ChangAo Chen