Re: replacing role-level NOINHERIT with a grant-level option

Greetings,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Mon, Jun 6, 2022 at 7:21 PM Stephen Frost <sfrost@snowman.net> wrote:
> > > To revoke a grant entirely, you just say REVOKE foo FROM bar, as now.
> > > To change an option for an existing grant, you can re-execute the
> > > grant statement with a different WITH clause. Any options that are
> > > explicitly mentioned will be changed to have the associated values;
> > > unmentioned options will retain their existing values. If you want to
> > > change the value of a Boolean option to false, you have a second
> > > option, which is to write "REVOKE option_name OPTION FOR foo FROM
> > > bar," which means exactly the same thing as "GRANT foo TO bar WITH
> > > option_name FALSE".
> >
> > I'm a bit concerned about this because, iiuc, it would mean:
> >
> > GRANT foo TO bar WITH FRUIT KIWI, SARDINES;
> > GRANT foo TO bar WITH FRUIT STRAWBERRY;
> >
> > would mean that the GRANT of FRUIT would then *only* have STRAWBERRY,
> > right?
>
> I think that you are misunderstanding what kind of option I intended
> FRUIT to be. Here, I was imagining FRUIT as a property that every
> grant has. Any given grant is either strawberry, or it's kiwi, or it's
> banana. It cannot be more than one of those things, nor can it be none
> of those things. It follows that if you execute GRANT without
> specifying a FRUIT, there's some default - hopefully banana, but
> that's a matter of taste. Later, you can change the fruit associated
> with a grant, but you cannot remove it, because there's no such thing
> as a fruitless grant. Imagine that the catalog representation is a
> "char" that is either 's', 'k', or 'b'.

Ah, yeah, if it's always single-value then that seems reasonable to me
too.  If we ever get to wanting to support multiple choices for a given
option then we could possibly require they be provided as an ARRAY or
using ()'s or something else, but we probably don't need to try and sort
that today.

> > In your proposal, does:
> >
> > GRANT foo TO bar WITH FRUIT STRAWBERRY;
> >
> > mean that 'foo' is grant'd to 'bar' too?  Seems to be closest to current
> > usage and the spec's ideas on these things.  I'm thinking that could be
> > dealt with by having a MEMBERSHIP option (which would be a separate
> > column in pg_auth_members and default would be 'true') but otherwise
> > using exactly what you have here, eg:
>
> Currently, GRANT always creates an entry in pg_auth_members, or
> modifies an existing one, or does nothing because the one that's there
> is the same as the one it would have created. I think we should stick
> with that idea.

Alright.

> That's why I proposed the name SET, not MEMBERSHIP. You would still
> get a catalog entry in pg_auth_members, so you are still a member in
> some loose sense even if your grant has INHERIT FALSE and SET FALSE,
> but in such a case the fact that you are a member isn't really doing
> anything for you in terms of getting you access to privileges because
> you're neither allowed to exercise them implicitly nor SET ROLE to the
> role.

Naming things is hard. :)  I'm still not a fan of calling that option
'SET' and 'membership' feels like how it's typically described today
when someone has the rights of a group (implicitly or explicitly).  As
for what to call "has a pg_auth_members row but no actual access", maybe
'associated'?

That does lead me down a bit of a rabbit hole because every role in the
entire system could be considered 'associated' with every other one and
if the case of "no pg_auth_members row" is identical to the case of
"pg_auth_members row with everything off/default" then it feels a bit
odd to have an entry for it- and is there any way to get rid of that
entry?

All that said ... we have a similar thing with GRANT today when it comes
to privileges on objects in that we go from NULL to owner-all+whatever,
and while it's a bit odd, it works well enough.

> I find that idea - that GRANT always grants membership but membership
> by itself doesn't really do anything for you unless you've got some
> other options enabled somewhere - more appealing than the design you
> seem to have in mind, which seems to me that membership is the same
> thing as the ability to SET ROLE and thus, if the ability to SET ROLE
> has not been granted, you have a grant that didn't confirm membership
> in any sense. I'm not saying we couldn't make that work, but I think
> it's awkward to make that work. Among other problems, what happens
> with the actual catalog representation? You could for example still
> create a role in pg_auth_members and then have a Boolean column
> membership = false, but that's a bit odd. Or you could add a new
> catalog or you could rename the existing catalog, but that's more
> complicated for not much benefit. I think there's some fuzziness at
> the semantic level with this kind of thing too: if I do a GRANT with
> MEMBERSHIP FALSE, what exactly is it that I am granting? I like the
> conceptual simplicity of being able to say that a GRANT always confers
> membership, but membership does not intrinsically include the ability
> to SET ROLE -- that's a Boolean property of membership, not membership
> itself.

I agree with having the ability to have the SET ROLE privilege be
distinct and able to be given, or not.  I don't think we need a new
catalog either, my thought was more along the lines of just renaming
what you proposed as being 'SET' to be 'MEMBERSHIP' while mostly keeping
the rest the same, but I did want to ask the question that didn't get
answered above:

> > In your proposal, does:
> >
> > GRANT foo TO bar WITH FRUIT STRAWBERRY;
> >
> > mean that 'foo' is grant'd to 'bar' too?

That is, regardless of how we track these things in the catalog or such,
we have to respect that:

GRANT foo TO bar;

is a SQL-defined thing that says that a 'role authorization descriptor'
is created.  SET ROLE then checks if a role authorization descriptor
exists or not matching the current role to the new role and if it does
then the current role is changed to the new role.  What I was really
trying to get at above is that:

GRANT foo TO bar WITH $anything-other-than-SET-false;

should probably also create a 'role authorization descriptor' that SET
ROLE will pick up on.  In other words, the 'SET' thing, or if we call
that something else, should exist as a distinct column in
pg_auth_members, but the default value of it should be 'true', with the
ability for it to be turned to false either at GRANT time or with a
REVOKE.

Thanks,

Stephen