Re: CREATEROLE and role ownership hierarchies

Greetings,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> 1. Don't allow a CREATEROLE user to give out membership in groups
> which that user does not possess. Leaving aside the details of any
> previously-proposed patches and just speaking theoretically, how can
> this be implemented? I can think of a few ideas. We could (1A) just
> change CREATEROLE to work that way, but IIUC that would break the use
> case you outline here, so I guess that's off the table unless I am
> misunderstanding the situation. We could also (1B) add a second role
> attribute with a different name, like, err, CREATEROLEWEAKLY, that
> behaves in that way, leaving the existing one untouched. But we could
> also take it a lot further, because someone might want to let an
> account hand out a set of privileges which corresponds neither to the
> privileges of that account nor to the full set of available
> privileges. That leads to another idea: (1C) implement an in-database
> system that lets you specify which privileges an account has, and,
> separately, which ones it can assign to others. I am skeptical of that
> idea because it seems really, really complicated, not only from an
> implementation standpoint but even just from a user-experience
> standpoint. Suppose user 'userbot' has rights to grant a suitable set
> of groups to the new users that it creates -- but then someone creates
> a new group. Should that also be added to the things 'userbot' can
> grant or not? What if we have 'userbot1' through 'userbot6' and each
> of them can grant a different set of roles? I wouldn't mind (1D)
> providing a hook that allows the system administrator to install a
> loadable module that can enforce any rules it likes, but it seems way
> too complicated to me to expose all of this configuration as SQL,
> especially because for what I want to do, either (1A) or (1B) is
> adequate, and (1B) is a LOT simpler than (1C). It also caters to what
> I believe to be a common thing to want, without prejudice to the
> possibility that other people want other things.

I'm generally in support of changing CREATEROLE to only allow roles that
the role with CREATEROLE is an admin of to be allowed as part of the
command (throwing an error in other cases). That doesn't solve other
use-cases which would certainly be nice to solve but it would at least
reduce the shock people have when they discover how CREATEROLE actually
works (that is, the way we document it to work, but that's ultimately
not what people expect).

If that's all this was about then that would be one thing, but folks are
interested in doing more here and that's good because there's a lot here
that could be (and I'd say should be..) done.

I'm not a fan of 1B. In general, I'm in support of 1C but I don't feel
that absolutely everything must be done for 1C right from the start-
rather, I would argue that we'd be better off building a way for 1C to
be improved upon in the future, akin to our existing privilege system
where we've added things like the ability to GRANT TRUNCATE rights which
didn't originally exist. I don't think 1D is a reasonable way to
accomplish that though, particularly as this involves storing
information about roles which needs to be cleaned up if those roles are
removed or modified. I also don't really agree with the statement that
this ends up being too complicated for SQL.

> 2. Only allow a CREATEROLE user to drop users which that account
> created, and not just any role that isn't a superuser. Again leaving
> aside previous proposals, this cannot be implemented without providing
> some means by which we know which CREATEROLE user created which other
> user. I believe there are a variety of words we could use to describe
> that linkage, and I don't deeply care which ones we pick, although I
> have my own preferences. We could speak of the CREATEROLE user being
> the owner, manager, or administrator of the created role. We could
> speak of a new kind of object, a TENANT, of which the CREATEROLE user
> is the administrator and to which the created user is linked. I
> proposed this previously and it's still my favorite idea. There are no
> doubt other options as well. But it's axiomatic that we cannot
> restrict the rights of a CREATEROLE user to drop other roles to a
> subset of roles without having some way to define which subset is at
> issue.

I don't think it's a great plan to limit who is allowed to DROP roles to
be just those that a given role created. I also don't like the idea of
introducing a single field for owner/manager/tenant/whatever to the role
system- instead we should add other ways that roles can be associated to
each other by extending the existing system that we have for that, which
is role membership. Role membership today is pretty limited but I don't
see any reason why we couldn't improve on that in a way that's flexible
and allows us to define new associations in the future. The biggest
difference between a 'tenant' or such as proposed vs. a role association
is in where the information is tracked and what exactly it means.
Saying "I want a owner" or such is easy because it's basically punting
on the complciated bit of asking the question: what does that *mean*
when it comes to what rights that includes vs. doesn't? What if I only
want some of those rights to be given away but not all of them? We have
that system for tables/schemas/etc, and it hasn't been great as we've
seen through the various requests to add things like GRANT TRUNCATE.

> But if you DO want the userbot to be able to access that
> functionality, then things are more complicated, because now the
> linkage has to be special-purpose. In that scenario, we can't say that
> the right of a CREATEROLE user to drop a certain other role implies
> having the privileges of that other role, because in your use case,
> you don't want that, whereas in mine, I do. What makes this
> particularly ugly is that we can't, as things currently stand, use a
> role as the grouping mechanism, because of the fact that a role can
> revoke membership in itself from some other role. It will not do for
> roles to remove themselves from the set of roles that the CREATEROLE
> user can drop. If we changed that behavior, then perhaps we could just
> define a way to say that role X can drop roles if they are members of
> group G. In my tenant scenario, G would be granted to X, and in your
> userbot scenario, it wouldn't. Everybody wins, except for any people
> who like the ability of roles to revoke themselves from any group
> whatsoever.

The ability of a role to revoke itself from some other role is just
something we need to accept as being a change that needs to be made, and
I do believe that such a change is supported by the standard, in that a
REVOKE will only work if you have the right to make it as the user who
performed the GRANT in the first place.

> So that leads to these questions: (2A) Do you care about restricting
> which roles the userbot can drop? (2B) If yes, do you endorse
> restricting the ability of roles to revoke themselves from other
> roles?

As with Joshua, and as hopefully came across from the above discussion,
I'm also a 'yes and yes' on these two.

> I think that we don't have any great problems here, at least as far as
> this very specific issue is concerned, if either the answer to (2A) is
> no or the answer to (2B) is yes. However, if the answer to (2A) is yes
> and the answer to (2B) is no, there are difficulties. Evidently in
> that case we need some new kind of thing that behaves mostly likes a
> group of roles but isn't actually a group of roles -- and that thing
> needs to prohibit self-revocation. Given what I've written above, you
> may be able to guess my preferred solution: let's call it a TENANT.
> Then, my pseudo-super-user can have permission to (i) create roles in
> that tenant, (ii) drop roles in that tenant, and (iii) assume the
> privileges of roles in that tenant -- and your userbot can have
> privileges to do (i) and (ii) but not (iii). All we need do is add a
> roltenant column to pg_authid and find three bits someplace
> corresponding to (i)-(iii), and we are home.

Where are those bits going to go though..? I don't think they should go
into pg_authid, nor do I feel that this 'tenant' or such should go there
either because pg_authid is about describing individual roles, not about
role associations. Instead, I'd suggest that those bits go into
pg_auth_members in the form of additional columns to describe the role
associations. That is, instead of the existance of a row in
pg_auth_members meaning that one role has membership in another role, we
give users the choice of if that's the case or not with a separate
column. That would then neatly give us a way for a role to have admin
rights over another role but not membership in that role. We could then
further extend this by adding other columns to pg_auth_members for other
rights as users decide they need them- such as the ability for a role to
DROP some set of roles.

> 2A, yes
> 2B, yes, and IIUC this already exists:
> postgres=> select current_user;
> current_user
> --------------
> joshua
> (1 row)
>
> postgres=> REVOKE employees FROM joshua;
> ERROR: must have admin option on role "employees"

That's not the right direction though, or, at least, might not be in the
case being discussed (though, I suppose, we could discuss that..). In
what you're showing, employees doesn't have the rights of joshua, but
joshua has the rights of employees. If, instead, joshua was GRANT'd to
admin and joshua decided that they didn't care for that, they can:

=> select current_user;
current_user
--------------
joshua
(1 row)

=> \du
List of roles
Role name | Attributes | Member of
-----------+------------------------------------------------------------+-------------
admin | Cannot login | {joshua}
employees | Cannot login | {}
joshua | | {employees}
sfrost | Superuser, Create role, Create DB, Replication, Bypass RLS | {}

=> revoke joshua from admin;
REVOKE ROLE

=*> \du
List of roles
Role name | Attributes | Member of
-----------+------------------------------------------------------------+-------------
admin | Cannot login | {}
employees | Cannot login | {}
joshua | | {employees}
sfrost | Superuser, Create role, Create DB, Replication, Bypass RLS | {}

Even though, in this case, it was 'sfrost' (a superuser) who GRANT'd
joshua to admin.

Thanks,

Stephen