Re: fixing CREATEROLE

On Thu, Nov 24, 2022 at 2:41 AM <walther(at)technowledgy(dot)de> wrote:
> INHERITCREATEDROLES and SETCREATEDROLES behave much like DEFAULT
> PRIVILEGES. What about something like:
>
> ALTER DEFAULT PRIVILEGES FOR alice
> GRANT TO alice WITH INHERIT FALSE, SET TRUE, ADMIN TRUE
>
> The "abbreviated grant" is very much abbreviated, because the original
> syntax GRANT a TO b is already quite short to begin with, i.e. there is
> no ON ROLE or something like that in it.
>
> The initial DEFAULT privilege would be INHERIT FALSE, SET FALSE, ADMIN
> TRUE, I guess?

I don't know if changing the syntax from A to B is really getting us
anywhere. I generally agree that the ALTER DEFAULT PRIVILEGES syntax
looks nicer than the CREATE/ALTER ROLE syntax, but I'm not sure that's
a sufficient reason to move the control over this behavior to ALTER
DEFAULT PRIVILEGES. One thing to consider is that, as I've designed
this, whether or not ADMIN is included in the grant is non-negotiable.
I am, at least at present, inclined to think that was the right call,
partly because Mark Dilger expressed a lot of concern about the
CREATEROLE user losing control over the role they'd just created, and
allowing ADMIN to be turned off would have exactly that effect. Plus a
grant with INHERIT FALSE, SET FALSE, ADMIN FALSE would end up being
almost identical to no great at all, which seems pointless. Basically,
without ADMIN, the implicit GRANT fails to accomplish its intended
purpose, so I don't like having that as a possibility.

The other thing that's a little weird about the syntax which you
propose is that it's not obviously related to CREATE ROLE. The intent
of the patch as implemented is to allow control over only the implicit
GRANT that is created when a new role is created, not all grants that
might be created by or to a particular user. Setting defaults for all
grants doesn't seem like a particularly good idea to me, but it's
definitely a different idea than what the patch proposes to do.

I did spend some time thinking about trying to tie this to the
CREATEROLE syntax itself. For example, instead of CREATE ROLE alice
CREATEROLE INHERITCREATEDROLES SETCREATEDROLES you could write CREATE
ROLE alice CREATEROLE WITH (INHERIT TRUE, SET TRUE) or something like
this. That would avoid introducing new, lengthy keywords that are just
concatenations of other English words, a kind of syntax that doesn't
look particularly nice to me and probably is less friendly to
non-English speakers as well. I didn't do it that way because the
parser support would be more complex, but I could. CREATEROLE would
have to become a keyword again, but that's not a catastrophe.

Another idea would be to break the CREATEROLE stuff off from CREATE
ROLE entirely and put it all into GRANT. You could imagine syntax like
GRANT CREATEROLE (or CREATE ROLE?) TO role_specification WITH (INHERIT
TRUE/FALSE, SET TRUE/FALSE). There are a few potential issues with
this direction. One, if we did this, then CREATEROLE probably ought to
become inheritable, because that's the way grants work in general, and
this likely shouldn't be an exception, but this would be a behavior
change. However, if it is the consensus that such a behavior change
would be an improvement, that might be OK. Two, I wonder what we'd do
about the GRANTED BY role_specification clause. We could leave it out,
but that would be asymmetric with other GRANT commands. We could also
support it and record that information and make this work more like
other cases, including, I suppose, the possibility of dependent
grants. We'd have to think about what that means exactly. If you
revoke CREATEROLE from someone who has granted CREATEROLE to others, I
suppose that's a clear dependent grant and needs to be recursively
revoked. But what about the implicit grants that were created because
the person had CREATEROLE? Are those also dependent grants? And what
about the roles themselves? Should revoking CREATEROLE drop the roles
that the user in question created? That gets complicated, because
those roles might own objects. That's scary, because you might not
expect revoking a role permission to result in tables getting dropped.
It's also problematic, because those tables might be in some other
database where they are inaccessible to the current session. All in
all I'm inclined to think that recursing to the roles themselves is a
bad plan, but it's debatable.

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