Re: [PATCH] remove is_member_of_role() from header, add can_set_role()

On Wed, Oct 27, 2021 at 1:12 PM Mark Dilger
<mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>
>
>
> > On Oct 27, 2021, at 9:26 AM, Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com> wrote:
> >
> > As a follow-on to Conflation of member/privs for predefined roles,
> > this removes is_member_of_role from the header to dissuade it's use
> > for privilege checking. Since SET ROLE must use membership rather than
> > privileges a new, explicitly named can_set_role() function is
> > exported.
> >
> > is_member_of_role_nosuper() still exists for the following purposes:
> > - membership loop checking in user.c
> > - membership matching for pg_hba.conf in hba.c
> >
> > Other uses of is_member_of_role_nosuper() should be avoided.
> > <0001-unexport-is_member_of_role-add-can_set_role.patch>
>
> I don't understand the purpose of this. You are defining can_set_role(member,role) as a simple wrapper around is_member_of_role(member,role). Couldn't the comment:
>
> + *
> + * Do not use this for privilege checking, instead use has_privs_of_role()
>
> be added to the header for is_member_of_role() without needing the new wrapper function?

It could be, but the intent is to dissuade it from being used, so
getting rid of it and making an explicit version that has a sole use
seemed useful.

It's possible that it's being used inappropriately out-of-tree so this
would also prevent that.