Обсуждение: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

Поиск
Список
Период
Сортировка

Why is_admin_of_role() use ROLERECURSE_MEMBERS rather than ROLERECURSE_PRIVS?

От
"cca5507"
Дата:
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?

--
Regards,
ChangAo Chen

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

Hi,

I attach a small patch for this.

Looking forward to your review.

--
Regards,
ChangAo Chen

Вложения

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

От
"cca5507"
Дата:
Hi,

Fix "create_role.sql" in v2.

--
Regards,
ChangAo Chen

Вложения

> 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
tou3”, 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
testscript, 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/







Re: Why is_admin_of_role() use ROLERECURSE_MEMBERS rather thanROLERECURSE_PRIVS?

От
"cca5507"
Дата:
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

Вложения
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