Re: role self-revocation - Mailing list pgsql-hackers

On Thu, Mar 10, 2022 at 3:41 PM Stephen Frost <sfrost@snowman.net> wrote:
> > Gosh, I feel like I've spelled that out approximately 463,121 times
> > already. That estimate might be slightly off though; I've been known
> > to make mistakes from time to time....
>
> If there's a specific message that details it closely on the lists
> somewhere, I'm happy to go review it.  I admit that I didn't go back and
> look for such.

Probably easier to just say it again: I want to have users that can
create roles and then have superuser-like powers with respect to those
roles. They can freely exercise the privileges of those roles, and
they can do all the things that a superuser can do but only with
respect to those roles. They cannot break out to the OS. I think it's
pretty similar to what you are describing, with a couple of possible
exceptions. For example, would you imagine that being an admin of a
login role would let you change that user's password? Because that
would be desirable behavior from where I sit.

> Errr, just to be clear, ALTER ROLE doesn't exist *in the spec*.  I
> wasn't suggesting that we get rid of it, just that it doesn't exist in
> the spec and therefore the spec doesn't have anything to say about it.

Oh, OK.

> > Basically, in this sketch, ADMIN OPTION on a role involves the ability
> > to DROP it, which means we don't need a separate role owner concept.
>
> Right.  The above doesn't include any specifics about what to do with
> ALTER ROLE, but my thought would be to have it also be under ADMIN
> OPTION rather than under CREATEROLE, as I tried to outline (though not
> very well, I'll admit) below.

This sentence really confused me at first, but I think you're saying
that the right to alter a role would be dependent on having ADMIN
OPTION on the role rather than on having the CREATEROLE attribute.
That seems like a reasonable idea to me.

> > It also involves membership, meaning that you can freely exercise the
> > privileges of the role without SET ROLE. While I'm totally down with
> > having other possible behaviors as options, that particular behavior
> > seems very useful to me, so, sounds great.
>
> Well, yes and no- by default you're right, presuming everything is set
> as inheirited, but I'd wish for us to keep the option of creating roles
> which are noinherit and having that work just as it does today.

Hmm, so if I have membership WITH ADMIN OPTION in a role, but my role
is marked NOINHERIT, that means I can't exercise the privileges of
that role without SET ROLE. But, can I still do other things to that
role, such as dropping it? Given the current coding of
roles_is_member_of(), it seems like I can't. I don't like that, but
then I don't like much of anything about NOINHERIT. Do you have any
suggestions for how this could be improved?

To make this more concrete, suppose the superuser does "CREATE USER
alice CREATEROLE". Alice will have INHERIT, so she'll have control
over any roles she creates. But if she does "CREATE USER bob
CREATEROLE NOINHERIT" then neither she nor Bob will be able to control
the roles bob creates. I'd like to have a way to make it so that
neither Alice nor any other CREATEROLE users she spins up can create
roles over which they no longer have control. Because otherwise people
will do dumb stuff like that and then have to call the superuser to
sort it out, and the superuser won't like that because s/he is a super
busy person.

> > What do you mean by the 'drop role' exception?
>
> 'ability' was probably a better word there.  What I'm talking about is
> changing in DropRole:
>
> to be, more or less:
>
>     if (!is_admin_of_role(role))
>         ereport(ERROR,
>                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>                  errmsg("permission denied to drop role")));

Sounds good.

> > I don't like removing 'alter role'.
>
> Ditto above but for AlterRole.  Taking it away from users with
> CREATEROLE being able to run those commands on $anyrole and instead
> making it so that the role running DROP ROLE or ALTER ROLE needs to have
> ADMIN on the role they're messing with.  I do think we may also need to
> make some adjustments in terms of what a regular user WITH ADMIN on a
> given role is able to do when it comes to ALTER ROLE, in particular, I
> don't think we'll want to remove the existing is-superuser checks
> against a user settings bypassrls or replication or superuser on some
> other role.  Maybe we can provide a way for a non-superuser to be given
> the ability to set those attributes for roles they create, but that
> would be a separate thing.

This too.

> > > Step #2 is also in-line with the spec: track GRANTORs and care about
> > > them, for everything.  We really should have been doing this all along.
> >
> > There are details to work out here, but in general, I like it.
>
> Cool.  Note that superusers would still be able to do $anything,
> including removing someone's ADMIN rights on a role even if that
> superuser didn't GRANT it (at least, that's my thinking on this).

Agree. I also think that it would be a good idea to attribute grants
performed by any superuser to the bootstrap superuser, or leave them
unattributed somehow. Because otherwise dropping superusers becomes a
pain in the tail for no good reason.

We might also need to think carefully about what happens if for
example the table owner is changed. If bob owns the table and we
change the owner to mary, but bob's previous grants are still
attributed to bob, I'm not sure that's going to be very convenient.
Possibly if the table owner changes we also change the owner of all
grants attributed to the old table owner to be attributed to the new
table owner?

-- 
Robert Haas
EDB: http://www.enterprisedb.com