Re: fixing CREATEROLE - Mailing list pgsql-hackers

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