Deepak Bhole of Red Hat asked me about the following situation:
regression=# create table test (f1 int);
CREATE TABLE
regression=# revoke insert,update,delete,references on test from postgres;
REVOKE
regression=# \z test Access privileges for database "regression"Schema | Name | Type | Access privileges
--------+------+-------+--------------------------------public | test | table | {postgres=*r***R**t*/postgres}
(1 row)
It seems unreasonably hard to interpret what those stars mean, don't you
think? Certainly you can't tell which star is which without hardwired
knowledge about the order in which the bits will be printed.
The problem here is that we allow the owner to revoke his own ordinary
privileges but not his grant options; so we end up with a permissions
configuration that was not considered in the design of the external
representation for ACL lists. (Per spec it is not possible to hold a
grant option for a privilege without holding the privilege itself too,
and I expect that this printout format was designed assuming that
restriction.)
I think the printout format is fine and the silent non-removal of grant
options was a bad idea, particularly since it doesn't seem to be saving
any code (GRANT/REVOKE check ownerness anyway). I propose that we take
out the special cases in merge_acl_with_grant that prohibit revoking an
owner's grant options, and instead adjust the grant statement code to
act as if those options are always present. Instead of the existing
if (stmt->is_grant && !pg_class_ownercheck(relOid, GetUserId()) && pg_class_aclcheck(relOid,
GetUserId(), ACL_GRANT_OPTION_FOR(privileges)) != ACLCHECK_OK)
aclcheck_error(ACLCHECK_NO_PRIV,ACL_KIND_CLASS, relvar->relname);
it'd be something like
if (pg_class_ownercheck(relOid, GetUserId()) { okay, assume we have all grant options }else if
(pg_class_aclcheck(relOid,GetUserId(), ...) != ACLCHECK_OK) { error }else { determine actual grant options for
non-owner}
Thus the effective behavior of grant/revoke would remain the same as
before, but we wouldn't have the contrary-to-spec cases in the visible
contents of the ACL list.
I am in the middle of fixing GRANT/REVOKE to conform to spec as
discussed in the bug #1150 thread, and will make this change too
if I don't hear any objections.
regards, tom lane
