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.

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 77322a115f1f7b36e08541ae6ab5e3fd873aea5b..1d6ae8b4f3e08a85c0b9d4b809c802c2b61b31b9 100644 (file)
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -1230,11 +1230,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 f99ebde33cc61c863220ddca1d0071809a4ab00b..e8930cb2eb58543e45a0924a1c1a1fe846fe0ced 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1331,6 +1331,56 @@ SELECT has_function_privilege('regressuser1', 'testns.testfunc(int)', 'EXECUTE')
 SET client_min_messages TO 'warning';
 DROP SCHEMA testns CASCADE;
 RESET client_min_messages;
+-- 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 sequence x_seq;

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 3dd77a791793390c23911a55ae01573ce8080cdf..d4d328e649f7244dd8cb520058849221aab40edf 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -795,6 +795,30 @@ DROP SCHEMA testns CASCADE;
 RESET client_min_messages;
 
 
+-- 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