Date: Wed, 8 Jun 2022 10:16:26 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Joe Conway <mail@joeconway.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	"Bossart, Nathan" <bossartn@amazon.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: replacing role-level NOINHERIT with a grant-level option
Message-ID: <20220608141626.GG9030@tamriel.snowman.net>
References: 
 <CA+TgmoYA-zo8usqGoPGaXRFzJbzJxLuvk7RzNyWKhe5dHUuRsA@mail.gmail.com>
 <CA+Tgmoax3J2D1RoqSqBRuLVifMjinZ5SPvR+rBqBT89u57zOJQ@mail.gmail.com>
 <20220606232118.GA9030@tamriel.snowman.net>
 <CA+TgmoZmnu0hZdW-=v4NVZ+pvJRm=b+d5PMSgrQ9WcMrW-mCaw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="WHG05yakhlzm8Hk1"
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoZmnu0hZdW-=v4NVZ+pvJRm=b+d5PMSgrQ9WcMrW-mCaw@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20220608141626.GG9030%40tamriel.snowman.net>
Precedence: bulk


--WHG05yakhlzm8Hk1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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?
>=20
> 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:
>=20
> 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 =3D 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

--WHG05yakhlzm8Hk1
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJioK86AAoJEO1sijiDR2RVLOkQAKODggd4ts6bmc7f7kVbFUzO
z8fElG9H8dZRjsiBWnNe+ZoW5roMe3GnBNro+OJ35m4ZmCQbBx9KJIlOp9wZOz8n
eMlICYVF81oaDs6TuDYsKCfKP9OXLIgVT8N/npJ0bthMffmAnrbpiFJ8LVPbZDkt
gO3AJREYRqXSaIwnpaGWi7FQsdurXW1PpJYhGD5TesjWVxOU02cdCuhtwozWWToR
Rt9QCmhZjDiZ+nY5/bGtAe3eJgG0FbC3nKlaaPXszqvyNNKjPhUDc0OjC6rzzs6t
e9k6d8SmOBeR9wCNT8q6pHEqadRNiEG6rjgIiSPoZAk2T19Ns0m7ESQ7bWBTjLY+
m/B9w0R2XY39bFYc7kpoXRYFb7QogULb6M60ZNemeKkx5gzxr9ghlydzQtOKm8x3
LawpBKwu6m5IFLZilakME5a7FxNCikabFnspSnwhxrJ1ZmdxPlRDfiOipoYGukqz
8ia9ilxtEL3ChLuUkMsprsD1d/RlpQTIX0NWcIjxdyamgWEyfnoAcB3A5y2PJBWF
f0jz7UKaDOHHG7iSSRdJowJQqSl1/O/uMMtz85b0Q6JC086jfgVtF3Os8Xid0hvT
QrzQHKtA0XDj7Ve+8Xvl6oHu171D1WRpGS0iEXpUR5hsFHup2A3vpmsAn99YaYbg
M/VPgRsNJNl6wcd6vUWb
=b2K6
-----END PGP SIGNATURE-----

--WHG05yakhlzm8Hk1--