Re: [PATCHES] Roles - SET ROLE Updated

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> To support having certain roles turned on and certain roles turned off
> would be some additional effort. I think we'd need a list of
> 'ENABLED_ROLES' and then correct recursion based off of that list
> instead of just starting from a single point like we do now.

That seems fairly ugly and messy though, because it wouldn't readily
translate to the case of SECURITY DEFINER procedures.

Let's review the bidding. As I presently understand it:

1. At session start, you are the SESSION_USER, which is also
CURRENT_USER, and (per spec anyway) CURRENT_ROLE should be NULL.
You have only the privileges granted directly to SESSION_USER
or PUBLIC.

2. You are allowed to SET ROLE to any role that the SESSION_USER is
(directly or indirectly) a member of. After you do so, the spec
says that CURRENT_USER is still SESSION_USER, CURRENT_ROLE is now
the selected role, and you have the union of the rights of those
two authorization identifiers.

3. If you enter a module (call a SECURITY DEFINER procedure), then
the current authorization identifier becomes that of the owner
of the module/procedure. Per spec this identifier is either a
user or a role, not both, and either CURRENT_USER or CURRENT_ROLE
reflects that with the other becoming NULL. (The authorization
stack will pop back to the prior state when you exit the call.)
Note: this is the case that allows CURRENT_USER to become null.

4. I'm not totally clear about whether the spec allows a module to be
entered without starting a transaction. If so, it would be legal
for a module owned by a user (not a role) to SET ROLE to one of
the roles that user is a member of. Again this would have effect
only till module exit.

Now #4 doesn't currently apply to us, since we have neither modules
nor procedures that can be entered outside a transaction. And frankly
I don't see the point in it anyway --- you may as well let the module
or procedure be owned by the desired role in the first place.

ISTM most of the messiness here comes from the spec's hard-and-fast
distinction between users and roles, which they then have to blur again
in order to talk about "authorization identifiers". Our current
implementation treats these as a single kind of entity with some
optional attributes (eg rolcanlogin), and I think that that's a much
cleaner way of doing it.

What I would like to do is say that there is exactly one active
authorization identifier at any instant. That means that if you SET
ROLE then you no longer have the privileges attached to SESSION_USER
(except the right to SET ROLE to a different one of his roles).
This is contrary to spec but it seems to be a pretty widely accepted
way of doing things; and it makes SET ROLE effectively equivalent to
the change in privileges associated with entering a SECURITY DEFINER
procedure owned by that role, so there's a much cleaner conceptual
model.

Furthermore, I still feel that both CURRENT_USER and CURRENT_ROLE should
reflect this single active authorization id. The spec's distinction is
artificial and doesn't gain anything, except the ability to break code
that assumes CURRENT_USER is always meaningful.

If we don't do it that way then we have a bunch of API that breaks down:
all of the has_foo_privilege functions stop working, because they don't
have a signature that allows both a user and a role to be passed to
them.

I think we do need to invent the concept of roles (pg_authid entries)
that either do or do not inherit privileges from roles they are members
of. A non-inheriting role can do SET ROLE to gain the privileges of a
role it is a member of, but it does not have those privileges without
SET ROLE. We could drive this off rolcanlogin but it's probably better
to invent an additional flag column in pg_authid (call it rolinherit,
maybe). Users by default would not inherit, which puts us a great deal
closer to the spec's semantics.

Thoughts?

regards, tom lane