Re: fixing CREATEROLE

Robert Haas:
> 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.

The list of role attributes can currently be roughly divided into the
following categories:
- Settings with role-specific values: CONNECTION LIMIT, PASSWORD, VALID
UNTIL. It's hard to imagine storing them anywhere else, because they
need to have a different value for each role. Those are not just "flags"
like all the other attributes.
- Two special attributes in INHERIT and BYPASSRLS regarding
security/privileges. Those were invented because there was no other
syntax to do the same thing. Those could be interpreted as privileges to
do something, too - but lacking the ability to do that explicit. There
is no SET BYPASSRLS ON/OFF or SET INHERIT ON/OFF. Of course the INHERIT
case is now a bit different, because there is the inherit grant option
you introduced.
- Cluster-wide privileges: SUPERUSER, CREATEDB, CREATEROLE, LOGIN,
REPLICATION. Those can't be granted on some kind of object, because
there is no such global object. You could imagine inventing some kind of
global CLUSTER object and do something like GRANT SUPERUSER ON CLUSTER
TO alice; instead. Turning those into role attributes was the choice
made instead. Most likely it would have been only a syntactic difference
anyway: Even if there was something like GRANT .. ON CLUSTER, you'd most
likely implement that as... storing those grants as role attributes.

Your patch is introducing a new category of role attributes - those that
are affecting default behavior. But there is already a way to express
this right now, and that's ALTER DEFAULT PRIVILEGES in this case. Imho,
the question asked should not be "why change from syntax A to B?" but
rather: Why introduce a new category of role attributes, when there is a
way to express the same concept already? I can't see any compelling
reason for that, yet.

Or not "yet", but rather "anymore". When I understood and remember
correctly, you implemented it in a way that a user could not change
those new attributes on their own role. This is in fact different to how
ALTER DEFAULT PRIVILEGES works, so you could have made an argument that
this was better expressed as role attributes. But I think this was asked
and agreed on to act differently, so that the user can change this
default behavior of what happens when they create a role for themselves.
And now this reason is gone - there is no reason NOT to implement it as
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.

With how you implemented it right now, is it possible to do the following?

CREATE ROLE alice;
REVOKE ADMIN OPTION FOR alice FROM CURRENT_USER;

If the answer is yes, then there is no reason to allow a user to set a
shortcut for SET and INHERIT, but not for ADMIN.

If the answer is no, then you could just not allow specifying the ADMIN
option in the ALTER DEFAULT PRIVILEGES statement and always force it to
be TRUE.

> 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.

Before I proposed that I was confused for a moment about this, too - but
it turns out to be wrong. ALTER DEFAULT PRIVILEGES in general works as:

When object A is created, issue a GRANT ON A automatically.

In my proposal, the "object" is not the GRANT of that role. It's the
role itself. So the default privileges express what should happen when
the role is created. The default privileges would NOT affect a regular
GRANT role TO role_spec command. They only run that command when a role
is created.

> 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.

I agree, this would not have been any better.

> 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.

I'm not sure how that relates to the role attributes vs. default
privileges discussion. Those seem to be orthogonal to the question of
how to treat the CREATEROLE privilege itself. Right now, it's a role
attribute. I proposed "database roles" and making CREATEROLE a privilege
on the database level. David Johnston proposed to use a pg_createrole
built-in role instead. Your proposal here is to invent a CREATEROLE
privilege that can be granted, which is very similar to what I wrote
above about "GRANT CREATEROLE ON CLUSTER". Side note: Without the ON
CLUSTER, there'd be no target object in your GRANT statement and as such
CREATEROLE should be treated as a role name - so I'm not sure your
proposal actually works. In any case: All those proposals change the
semantics of how this whole CREATEROLE "privilege" works in terms of
inheritance etc. However, those proposals all don't really change the
way you'll want to treat the ADMIN option on the role, I think, and can
all be made to create that implicit GRANT WITH ADMIN, when you create
the role. And once you do that, the question of how that GRANT looks by
default comes up - so in all those scenarios, we could talk about role
attributes vs. default privileges. Or we could just decide not to,
because is it really that hard to just issue a GRANT statement
immediately after CREATE ROLE, when you want to have SET or INHERIT
options on that role?

The answer to that question was "yes it is too hard" a while back and as
such DEFAULT PRIVILEGES were introduced.

Best,

Wolfgang