Re: fixing CREATEROLE

On Tue, Nov 22, 2022 at 5:48 PM Mark Dilger
<mark(dot)dilger(at)enterprisedb(dot)com> wrote:
> Whatever behavior is to happen in the CREATE ROLE statement should be spelled out in that statement. "CREATE ROLE bob WITH INHERIT false WITH SET false" doesn't seem too unwieldy, and has the merit that it can be read and understood without reference to hidden parameters. Forcing this to be explicit should be safer if these statements ultimately make their way into dump/restore scripts, or into logical replication.
>
> That's not to say that I wouldn't rather that it always work one way or always the other. It's just to say that I don't want it to work differently based on some poorly advertised property of the role executing the command.

That seems rather pejorative. If we stipulate from the outset that the
property is poorly advertised, then obviously anything at all
depending on it is not going to seem like a very good idea. But why
should we assume that it will be poorly-advertised? I clearly said
that the documentation needs a bunch of work, and that I planned to
work on it. As an initial matter, the documentation is where we
advertise new features, so I think you ought to take it on faith that
this will be well-advertised, unless you think that I'm completely
hopeless at writing documentation or something.

On the actual issue, I think that one key question is who should
control what happens when a role is created. Is that the superuser's
decision, or the CREATEROLE user's decision? I think it's better for
it to be the superuser's decision. Consider first the use case where
you want to set up a user who "feels like a superuser" i.e. inherits
the privileges of users they create. You don't want them to have to
specify anything when they create a role for that to happen. You just
want it to happen. So you want to set up their account so that it will
happen automatically, not throw the complexity back on them. In the
reverse scenario where you don't want the privileges inherited, I
think it's a little less clear, possibly just because I haven't
thought about that scenario as much, but I think it's very reasonable
here too to want the superuser to set up a configuration for the
CREATEROLE user that does what the superuser wants, rather than what
the CREATEROLE user wants.

Even aside from the question of who controls what, I think it is far
better from a usability perspective to have ways of setting up good
defaults. That is why we have the default_tablespace GUC, for example.
We could have made the CREATE TABLE command always use the database's
default tablespace, or we could have made it always use the main
tablespace. Then it would not be dependent on (poorly advertised?)
settings elsewhere. But it would also be really inconvenient, because
if someone is creating a lot of tables and wants them all to end up in
the same place, they don't want to have to specify the name of that
tablespace each time. They want to set a default and have that get
used by each command.

There's another, subtler consideration here, too. Since
ce6b672e4455820a0348214be0da1a024c3f619f, there are constraints on who
can validly be recorded as the grantor of a particular role grant,
just as we have always done for other types of grants. The grants have
to form a tree, with each grant having a grantor that was themselves
granted ADMIN OPTION by someone else, until eventually you get back to
the bootstrap superuser who is the source of all privileges. Thus,
today, when a CREATEROLE user grants membership in a role, the grantor
is recorded as the bootstrap superuser, because they might not
actually possess ADMIN OPTION on the role at all, and so we can't
necessarily record them as the grantor. But this patch changes that,
which I think is a significant improvement. The implicit grant that is
created when CREATE ROLE is issued has the bootstrap superuser as
grantor, because there is no other legal option, but then any further
grants performed by the CREATE ROLE user rely on that user having that
grant, and thus record the OID of the CREATEROLE user as the grantor,
not the bootstrap superuser.

That, in turn, has a number of rather nice consequences. It means in
particular that the CREATEROLE user can't remove the implicit grant,
nor can they alter it. They are, after all, not the grantor, who is
the bootstrap superuser, nor do they any longer have the authority to
act as the bootstrap superuser. Thus, if you have two or more
CREATEROLE users running around doing stuff, you can tell who did
what. Every role that those users created is linked back to the
creating role in a way that the creator can't alter. A CREATEROLE user
is unable to contrive a situation where they no longer control a role
that they created. That also means that the superuser, if desired, can
revoke all membership grants performed by any particular CREATEROLE
user by revoking the implicit grants with CASCADE.

But since this implicit grant has, and must have, the bootstrap
superuser as grantor, it is also only reasonable that superusers get
to determine what options are used when creating that grant, rather
than leaving that up to the CREATEROLE user.

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