Re: [PATCH v2] use has_privs_for_role for predefined roles

On Mon, Feb 7, 2022 at 12:09 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Mon, Feb 7, 2022 at 11:13 AM Joe Conway <mail@joeconway.com> wrote:
> > Easily worked around with one additional level of role:
>
> Interesting.
>
> > > But in the absence of that, it seems clearly better for predefined
> > > roles to disregard INHERIT and just always grant the rights they are
> > > intended to give. Because if we don't do that, then we end up with
> > > people having to SET ROLE to the predefined role and perform actions
> > > directly as that role, which seems like it can't be what we want. I
> > > almost feel like we ought to be looking for ways of preventing people
> > > from doing SET ROLE to a predefined role altogether, not encouraging
> > > them to do it.
> > I disagree with this though.
> >
> > It is confusing and IMHO dangerous that the predefined roles currently
> > work differently than regular roles eith respect to privilege inheritance.
>
> I feel like that's kind of a conclusory statement, as opposed to
> making an argument. I mean that this tells me something about how you
> feel, but it doesn't really help me understand why you feel that way.
>
> I suppose one argument in favor of your position is that if it
> happened to be sri who was granted a predefined role, sunita would
> inherit the rest of sr's privileges only with SET ROLE, but the
> predefined role either way (IIUC, which I might not). If that's so,
> then I guess I see the point, but I'm still sort of inclined to think
> we're just trading one set of problems in for a different set. I just
> have such a hard time imaging anyone using NOINHERIT in anger and
> being happy with the result....
>

IMO this is inarguably a plain bug. The inheritance system works one
way for pre-defined roles and another way for other roles - and the
difference isn't even documented.

The question is whether there is a security issue warranting back
patching, which is a bit of a tossup I think. According to git history
it's always worked this way, and the possible breakage of pre-existing
clusters seems maybe not worth it.