Re: pg_auth_members.grantor is bunk
Stephen Frost <sfrost@snowman.net>
From: Stephen Frost <sfrost@snowman.net>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Mark Dilger <mark.dilger@enterprisedb.com>, Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2022-06-06T23:41:02Z
Lists: pgsql-hackers
Greetings, * Robert Haas (robertmhaas@gmail.com) wrote: > On Thu, Jun 2, 2022 at 3:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > I sort of thought http://postgr.es/m/3981966.1646429663@sss.pgh.pa.us > > > constituted a completed investigation of this sort. No? > > > > I didn't think so. It's clear that the spec expects us to track the > > grantor, but I didn't chase down what it expects us to *do* with that > > information, nor what it thinks the rules are for merging multiple > > authorizations. > > Hmm, OK. Well, one problem is that I've never had any luck > interpreting what the spec says about anything, and I've sort of given > up. But even if that were not so, I'm a little unclear what other > conclusion is possible here. The spec either wants the same behavior > that we already have for other object types, which is what I am here > proposing that we do, or it wants something different. If it wants > something different, it probably wants that for all object types, not > just roles. Since I doubt we would want the behavior for roles to be > inconsistent with what we do for all other object types, in that case > we would probably either change the behavior for all other object > types to something new, and then clean up the role stuff afterwards, > or else first do what I proposed here and then later change it all at > once. In which case the proposal that I've made is as good a way to > start as any. > > Now, if it happens to be the case that the spec proposes a different > behavior for roles than for non-role objects, and if the behavior for > roles is something other than the only we currently have for non-role > objects, then I'd agree that the plan I propose here needs revision. I > suspect that's unlikely but I can't make anything of the spec so .... > maybe? Thankfully, at least from my reading, the spec isn't all that complicated on this particular point. The spec talks about "role authorization descriptor"s and those are "created with role name, grantee, and grantor" and then further says "redundant duplicate role authorization descriptors are destroyed", presumably meaning that the entire thing has to be identical. In other words, yeah, the PK should include the grantor. There's a further comment that the 'set of involved grantees' is the union of all the 'grantees', clearly indicating that you can have multiple GRANT 'foo' to 'bar's with distinct grantees. In terms of how that's then used, yeah, it's during REVOKE because a REVOKE is only able to 'find' role authorization descriptors which match the triple of role revoked, grantee, grantor (though there's a caveat in that the 'grantor' role could be the current role, or the current user). Interestingly, at least in my looking it over today, it doesn't seem that the 'grantor' could be 'any applicable role' (which is what's usually used to indicate that it could be any role that the current role inherits), meaning you have to include the GRANTED BY in the REVOKE statement or do a SET ROLE first when doing a REVOKE if it's for a role that you aren't currently running as (but which you are a member of). Anyhow, in other words, I do think Robert's got it right here. Happy to discuss further though if there are doubts. Thanks, Stephen
Commits
-
Make role grant system more consistent with other privileges.
- ce6b672e4455 16.0 landed
-
Ensure that pg_auth_members.grantor is always valid.
- 6566133c5f52 16.0 landed
-
Remove the ability of a role to administer itself.
- 79de9842ab03 15.0 landed
-
Add tests of the CREATEROLE attribute
- e9d4001ec592 15.0 landed
-
Replace explicit PIN entries in pg_depend with an OID range test.
- a49d08123599 15.0 cited
-
Shore up ADMIN OPTION restrictions.
- fea164a72a7b 9.4.0 cited
-
Add pg_has_role() family of privilege inquiry functions modeled after the
- f9fd1764615e 8.1.0 cited
-
Align GRANT/REVOKE behavior more closely with the SQL spec, per discussion
- 4b2dafcc0b1a 8.0.0 cited