Re: [PATCHES] Roles - SET ROLE Updated - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [PATCHES] Roles - SET ROLE Updated
Date
Msg-id 23444.1121984777@sss.pgh.pa.us
Whole thread Raw
In response to Re: [PATCHES] Roles - SET ROLE Updated  (Stephen Frost <sfrost@snowman.net>)
Responses Re: [PATCHES] Roles - SET ROLE Updated  (Stephen Frost <sfrost@snowman.net>)
List pgsql-hackers
Stephen Frost <sfrost@snowman.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
beNULL.  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
doso, the spec  says that CURRENT_USER is still SESSION_USER, CURRENT_ROLE is now  the selected role, and you have the
unionof 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
ofthe owner  of the module/procedure.  Per spec this identifier is either a  user or a role, not both, and either
CURRENT_USERor CURRENT_ROLE  reflects that with the other becoming NULL.  (The authorization  stack will pop back to
theprior 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,
itwould 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.
Againthis 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


pgsql-hackers by date:

Previous
From: Josh Berkus
Date:
Subject: Going to OSCON? We need your help!
Next
From: Stephen Frost
Date:
Subject: Re: [PATCHES] Roles - SET ROLE Updated