Skip to content

Commit bf3b85c

Browse files
tglsfdctglsfdc
committed
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 4b2dafc, 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.
1 parent d17cf76 commit bf3b85c

File tree

3 files changed

+76
-2
lines changed

3 files changed

+76
-2
lines changed

‎src/backend/utils/adt/acl.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,11 +1163,11 @@ recursive_revoke(Acl *acl,
11631163
if (grantee == ownerId)
11641164
return acl;
11651165

1166-
/* The grantee might still have the privileges via another grantor */
1166+
/* The grantee might still have some grant options via another grantor */
11671167
still_has = aclmask(acl, grantee, ownerId,
11681168
ACL_GRANT_OPTION_FOR(revoke_privs),
11691169
ACLMASK_ALL);
1170-
revoke_privs &= ~still_has;
1170+
revoke_privs &= ~ACL_OPTION_TO_PRIVS(still_has);
11711171
if (revoke_privs == ACL_NO_RIGHTS)
11721172
return acl;
11731173

‎src/test/regress/expected/privileges.out

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,56 @@ SELECT has_function_privilege('regressuser1', 'testns.testfunc(int)', 'EXECUTE')
12161216
SET client_min_messages TO 'warning';
12171217
DROP SCHEMA testns CASCADE;
12181218
RESET client_min_messages;
1219+
-- test that dependent privileges are revoked (or not) properly
1220+
\c -
1221+
set session role regressuser1;
1222+
create table dep_priv_test (a int);
1223+
grant select on dep_priv_test to regressuser2 with grant option;
1224+
grant select on dep_priv_test to regressuser3 with grant option;
1225+
set session role regressuser2;
1226+
grant select on dep_priv_test to regressuser4 with grant option;
1227+
set session role regressuser3;
1228+
grant select on dep_priv_test to regressuser4 with grant option;
1229+
set session role regressuser4;
1230+
grant select on dep_priv_test to regressuser5;
1231+
\dp dep_priv_test
1232+
Access privileges
1233+
Schema | Name | Type | Access privileges | Column access privileges
1234+
--------+---------------+-------+-----------------------------------+--------------------------
1235+
public | dep_priv_test | table | regressuser1=arwdDxt/regressuser1+|
1236+
| | | regressuser2=r*/regressuser1 +|
1237+
| | | regressuser3=r*/regressuser1 +|
1238+
| | | regressuser4=r*/regressuser2 +|
1239+
| | | regressuser4=r*/regressuser3 +|
1240+
| | | regressuser5=r/regressuser4 |
1241+
(1 row)
1242+
1243+
set session role regressuser2;
1244+
revoke select on dep_priv_test from regressuser4 cascade;
1245+
\dp dep_priv_test
1246+
Access privileges
1247+
Schema | Name | Type | Access privileges | Column access privileges
1248+
--------+---------------+-------+-----------------------------------+--------------------------
1249+
public | dep_priv_test | table | regressuser1=arwdDxt/regressuser1+|
1250+
| | | regressuser2=r*/regressuser1 +|
1251+
| | | regressuser3=r*/regressuser1 +|
1252+
| | | regressuser4=r*/regressuser3 +|
1253+
| | | regressuser5=r/regressuser4 |
1254+
(1 row)
1255+
1256+
set session role regressuser3;
1257+
revoke select on dep_priv_test from regressuser4 cascade;
1258+
\dp dep_priv_test
1259+
Access privileges
1260+
Schema | Name | Type | Access privileges | Column access privileges
1261+
--------+---------------+-------+-----------------------------------+--------------------------
1262+
public | dep_priv_test | table | regressuser1=arwdDxt/regressuser1+|
1263+
| | | regressuser2=r*/regressuser1 +|
1264+
| | | regressuser3=r*/regressuser1 |
1265+
(1 row)
1266+
1267+
set session role regressuser1;
1268+
drop table dep_priv_test;
12191269
-- clean up
12201270
\c
12211271
drop sequence x_seq;

‎src/test/regress/sql/privileges.sql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,30 @@ DROP SCHEMA testns CASCADE;
671671
RESET client_min_messages;
672672

673673

674+
-- test that dependent privileges are revoked (or not) properly
675+
\c -
676+
677+
set session role regressuser1;
678+
create table dep_priv_test (a int);
679+
grant select on dep_priv_test to regressuser2 with grant option;
680+
grant select on dep_priv_test to regressuser3 with grant option;
681+
set session role regressuser2;
682+
grant select on dep_priv_test to regressuser4 with grant option;
683+
set session role regressuser3;
684+
grant select on dep_priv_test to regressuser4 with grant option;
685+
set session role regressuser4;
686+
grant select on dep_priv_test to regressuser5;
687+
\dp dep_priv_test
688+
set session role regressuser2;
689+
revoke select on dep_priv_test from regressuser4 cascade;
690+
\dp dep_priv_test
691+
set session role regressuser3;
692+
revoke select on dep_priv_test from regressuser4 cascade;
693+
\dp dep_priv_test
694+
set session role regressuser1;
695+
drop table dep_priv_test;
696+
697+
674698
-- clean up
675699

676700
\c

0 commit comments

Comments
 (0)