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 65d7794b4e4944eb2ef590877e3ddc1f08479e96..fadc4e3cc5da379e389299cb286adba17c1b32e8 100644 (file)
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -948,11 +948,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 bcb2ea54aed0714218dc2c5eeb5233dcd1b365ad..a5941e1343a78b9addc83072a992c20dcb37a90f 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -10,14 +10,16 @@ DROP ROLE IF EXISTS regressuser1;
 DROP ROLE IF EXISTS regressuser2;
 DROP ROLE IF EXISTS regressuser3;
 DROP ROLE IF EXISTS regressuser4;
+DROP ROLE IF EXISTS regressuser5;
 RESET client_min_messages;
 -- test proper begins here
 CREATE USER regressuser1;
 CREATE USER regressuser2;
 CREATE USER regressuser3;
 CREATE USER regressuser4;
-CREATE USER regressuser4;      -- duplicate
-ERROR:  role "regressuser4" already exists
+CREATE USER regressuser5;
+CREATE USER regressuser5;      -- duplicate
+ERROR:  role "regressuser5" already exists
 CREATE GROUP regressgroup1;
 CREATE GROUP regressgroup2 WITH USER regressuser1, regressuser2;
 ALTER GROUP regressgroup1 ADD USER regressuser4;
@@ -581,6 +583,45 @@ 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 for database "regression"
+ Schema |     Name      | Type  |                                                                                 Access privileges                                                                                  
+--------+---------------+-------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ public | dep_priv_test | table | {regressuser1=arwdxt/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 for database "regression"
+ Schema |     Name      | Type  |                                                                   Access privileges                                                                   
+--------+---------------+-------+-------------------------------------------------------------------------------------------------------------------------------------------------------
+ public | dep_priv_test | table | {regressuser1=arwdxt/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 for database "regression"
+ Schema |     Name      | Type  |                                      Access privileges                                       
+--------+---------------+-------+----------------------------------------------------------------------------------------------
+ public | dep_priv_test | table | {regressuser1=arwdxt/regressuser1,regressuser2=r*/regressuser1,regressuser3=r*/regressuser1}
+(1 row)
+
+set session role regressuser1;
+drop table dep_priv_test;
 -- clean up
 \c regression
 DROP FUNCTION testfunc2(int);
@@ -605,3 +646,4 @@ DROP USER regressuser1;
 DROP USER regressuser2;
 DROP USER regressuser3;
 DROP USER regressuser4;
+DROP USER regressuser5;

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 4c0d9c43f2783a31e7f6f3a6c5a41275a3450578..1b1e1d4f0c537ae3a6f277518b438680422ac17c 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -14,6 +14,7 @@ DROP ROLE IF EXISTS regressuser1;
 DROP ROLE IF EXISTS regressuser2;
 DROP ROLE IF EXISTS regressuser3;
 DROP ROLE IF EXISTS regressuser4;
+DROP ROLE IF EXISTS regressuser5;
 
 RESET client_min_messages;
 
@@ -23,7 +24,8 @@ CREATE USER regressuser1;
 CREATE USER regressuser2;
 CREATE USER regressuser3;
 CREATE USER regressuser4;
-CREATE USER regressuser4;      -- duplicate
+CREATE USER regressuser5;
+CREATE USER regressuser5;      -- duplicate
 
 CREATE GROUP regressgroup1;
 CREATE GROUP regressgroup2 WITH USER regressuser1, regressuser2;
@@ -332,6 +334,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 regression
@@ -359,3 +385,4 @@ DROP USER regressuser1;
 DROP USER regressuser2;
 DROP USER regressuser3;
 DROP USER regressuser4;
+DROP USER regressuser5;