aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2012-08-23 17:25:33 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2012-08-23 17:25:33 -0400
commitd6a3863506599a4d22dd62e0601da9ab2ed933da (patch)
treee5b3f6db4c75d8e4b584167c24278210ec53914f
parentdda44070cc9c9bb3d0fedcca4e9610ea7ec8ee77 (diff)
downloadpostgresql-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.c4
-rw-r--r--src/test/regress/expected/privileges.out50
-rw-r--r--src/test/regress/sql/privileges.sql24
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