diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2012-08-23 17:25:33 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2012-08-23 17:25:33 -0400 |
commit | d6a3863506599a4d22dd62e0601da9ab2ed933da (patch) | |
tree | e5b3f6db4c75d8e4b584167c24278210ec53914f | |
parent | dda44070cc9c9bb3d0fedcca4e9610ea7ec8ee77 (diff) | |
download | postgresql-d6a3863506599a4d22dd62e0601da9ab2ed933da.tar.gz postgresql-d6a3863506599a4d22dd62e0601da9ab2ed933da.zip |
Fix cascading privilege revoke to notice when privileges are still held.
If we revoke a grant option from some role X, but X still holds the option
via another grant, we should not recursively revoke the privilege from
role(s) Y that X had granted it to. This was supposedly fixed as one
aspect of commit 4b2dafcc0b1a579ef5daaa2728223006d1ff98e9, but I must not
have tested it, because in fact that code never worked: it forgot to shift
the grant-option bits back over when masking the bits being revoked.
Per bug #6728 from Daniel German. Back-patch to all active branches,
since this has been wrong since 8.0.
-rw-r--r-- | src/backend/utils/adt/acl.c | 4 | ||||
-rw-r--r-- | src/test/regress/expected/privileges.out | 50 | ||||
-rw-r--r-- | src/test/regress/sql/privileges.sql | 24 |
3 files changed, 76 insertions, 2 deletions
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 67c4ea36e36..58167a8e03b 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -1030,11 +1030,11 @@ recursive_revoke(Acl *acl, if (grantee == ownerId) return acl; - /* The grantee might still have the privileges via another grantor */ + /* The grantee might still have some grant options via another grantor */ still_has = aclmask(acl, grantee, ownerId, ACL_GRANT_OPTION_FOR(revoke_privs), ACLMASK_ALL); - revoke_privs &= ~still_has; + revoke_privs &= ~ACL_OPTION_TO_PRIVS(still_has); if (revoke_privs == ACL_NO_RIGHTS) return acl; diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index a17ff59d0c8..ded20ecb28d 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -815,6 +815,56 @@ SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION') t (1 row) +-- test that dependent privileges are revoked (or not) properly +\c - +set session role regressuser1; +create table dep_priv_test (a int); +grant select on dep_priv_test to regressuser2 with grant option; +grant select on dep_priv_test to regressuser3 with grant option; +set session role regressuser2; +grant select on dep_priv_test to regressuser4 with grant option; +set session role regressuser3; +grant select on dep_priv_test to regressuser4 with grant option; +set session role regressuser4; +grant select on dep_priv_test to regressuser5; +\dp dep_priv_test + Access privileges + Schema | Name | Type | Access privileges | Column access privileges +--------+---------------+-------+-----------------------------------+-------------------------- + public | dep_priv_test | table | regressuser1=arwdDxt/regressuser1 | + : regressuser2=r*/regressuser1 + : regressuser3=r*/regressuser1 + : regressuser4=r*/regressuser2 + : regressuser4=r*/regressuser3 + : regressuser5=r/regressuser4 +(1 row) + +set session role regressuser2; +revoke select on dep_priv_test from regressuser4 cascade; +\dp dep_priv_test + Access privileges + Schema | Name | Type | Access privileges | Column access privileges +--------+---------------+-------+-----------------------------------+-------------------------- + public | dep_priv_test | table | regressuser1=arwdDxt/regressuser1 | + : regressuser2=r*/regressuser1 + : regressuser3=r*/regressuser1 + : regressuser4=r*/regressuser3 + : regressuser5=r/regressuser4 +(1 row) + +set session role regressuser3; +revoke select on dep_priv_test from regressuser4 cascade; +\dp dep_priv_test + Access privileges + Schema | Name | Type | Access privileges | Column access privileges +--------+---------------+-------+-----------------------------------+-------------------------- + public | dep_priv_test | table | regressuser1=arwdDxt/regressuser1 | + : regressuser2=r*/regressuser1 + : regressuser3=r*/regressuser1 +(1 row) + +set session role regressuser1; +drop table dep_priv_test; -- clean up \c DROP FUNCTION testfunc2(int); diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 5aa1012f3f6..50334ee8441 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -469,6 +469,30 @@ SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- false SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true +-- test that dependent privileges are revoked (or not) properly +\c - + +set session role regressuser1; +create table dep_priv_test (a int); +grant select on dep_priv_test to regressuser2 with grant option; +grant select on dep_priv_test to regressuser3 with grant option; +set session role regressuser2; +grant select on dep_priv_test to regressuser4 with grant option; +set session role regressuser3; +grant select on dep_priv_test to regressuser4 with grant option; +set session role regressuser4; +grant select on dep_priv_test to regressuser5; +\dp dep_priv_test +set session role regressuser2; +revoke select on dep_priv_test from regressuser4 cascade; +\dp dep_priv_test +set session role regressuser3; +revoke select on dep_priv_test from regressuser4 cascade; +\dp dep_priv_test +set session role regressuser1; +drop table dep_priv_test; + + -- clean up \c |