From: Tom Lane Date: Thu, 23 Aug 2012 21:25:38 +0000 (-0400) Subject: Fix cascading privilege revoke to notice when privileges are still held. X-Git-Tag: REL8_3_21~12 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9969e159ea9bf9083f12f5017071ca00ea1d3fc7;p=postgresql 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 65d7794b4e..fadc4e3cc5 100644 --- 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 bcb2ea54ae..a5941e1343 100644 --- 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 4c0d9c43f2..1b1e1d4f0c 100644 --- 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;