Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 0101f77

Browse files
committed
Fix a bug in roles_is_member_of.
Commit e3ce2de rearranged this function to be able to identify which inherited role had admin option on the target role, but it got the order of operations wrong, causing the function to return wrong answers in the presence of non-inherited grants. Fix that, and add a test case that verifies the correct behavior. Patch by me, reviewed by Nathan Bossart Discussion: http://postgr.es/m/CA+TgmoYamnu-xt-u7CqjYWnRiJ6BQaSpYOHXP=r4QGTfd1N_EA@mail.gmail.com
1 parent c7892c2 commit 0101f77

File tree

3 files changed

+61
-4
lines changed

3 files changed

+61
-4
lines changed

src/backend/utils/adt/acl.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -4852,10 +4852,6 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type,
48524852
Form_pg_auth_members form = (Form_pg_auth_members) GETSTRUCT(tup);
48534853
Oid otherid = form->roleid;
48544854

4855-
/* If we're supposed to ignore non-heritable grants, do so. */
4856-
if (type == ROLERECURSE_PRIVS && !form->inherit_option)
4857-
continue;
4858-
48594855
/*
48604856
* While otherid==InvalidOid shouldn't appear in the catalog, the
48614857
* OidIsValid() avoids crashing if that arises.
@@ -4864,6 +4860,10 @@ roles_is_member_of(Oid roleid, enum RoleRecurseType type,
48644860
OidIsValid(admin_of) && !OidIsValid(*admin_role))
48654861
*admin_role = memberid;
48664862

4863+
/* If we're supposed to ignore non-heritable grants, do so. */
4864+
if (type == ROLERECURSE_PRIVS && !form->inherit_option)
4865+
continue;
4866+
48674867
/*
48684868
* Even though there shouldn't be any loops in the membership
48694869
* graph, we must test for having already seen this role. It is

src/test/regress/expected/privileges.out

+32
Original file line numberDiff line numberDiff line change
@@ -2777,3 +2777,35 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_shmem_allocations;
27772777
RESET ROLE;
27782778
-- clean up
27792779
DROP ROLE regress_readallstats;
2780+
-- test role grantor machinery
2781+
CREATE ROLE regress_group;
2782+
CREATE ROLE regress_group_direct_manager;
2783+
CREATE ROLE regress_group_indirect_manager;
2784+
CREATE ROLE regress_group_member;
2785+
GRANT regress_group TO regress_group_direct_manager WITH INHERIT FALSE, ADMIN TRUE;
2786+
GRANT regress_group_direct_manager TO regress_group_indirect_manager;
2787+
SET SESSION AUTHORIZATION regress_group_direct_manager;
2788+
GRANT regress_group TO regress_group_member;
2789+
SELECT member::regrole::text, CASE WHEN grantor = 10 THEN 'BOOTSTRAP SUPERUSER' ELSE grantor::regrole::text END FROM pg_auth_members WHERE roleid = 'regress_group'::regrole ORDER BY 1, 2;
2790+
member | grantor
2791+
------------------------------+------------------------------
2792+
regress_group_direct_manager | BOOTSTRAP SUPERUSER
2793+
regress_group_member | regress_group_direct_manager
2794+
(2 rows)
2795+
2796+
REVOKE regress_group FROM regress_group_member;
2797+
SET SESSION AUTHORIZATION regress_group_indirect_manager;
2798+
GRANT regress_group TO regress_group_member;
2799+
SELECT member::regrole::text, CASE WHEN grantor = 10 THEN 'BOOTSTRAP SUPERUSER' ELSE grantor::regrole::text END FROM pg_auth_members WHERE roleid = 'regress_group'::regrole ORDER BY 1, 2;
2800+
member | grantor
2801+
------------------------------+------------------------------
2802+
regress_group_direct_manager | BOOTSTRAP SUPERUSER
2803+
regress_group_member | regress_group_direct_manager
2804+
(2 rows)
2805+
2806+
REVOKE regress_group FROM regress_group_member;
2807+
RESET SESSION AUTHORIZATION;
2808+
DROP ROLE regress_group;
2809+
DROP ROLE regress_group_direct_manager;
2810+
DROP ROLE regress_group_indirect_manager;
2811+
DROP ROLE regress_group_member;

src/test/regress/sql/privileges.sql

+25
Original file line numberDiff line numberDiff line change
@@ -1788,3 +1788,28 @@ RESET ROLE;
17881788

17891789
-- clean up
17901790
DROP ROLE regress_readallstats;
1791+
1792+
-- test role grantor machinery
1793+
CREATE ROLE regress_group;
1794+
CREATE ROLE regress_group_direct_manager;
1795+
CREATE ROLE regress_group_indirect_manager;
1796+
CREATE ROLE regress_group_member;
1797+
1798+
GRANT regress_group TO regress_group_direct_manager WITH INHERIT FALSE, ADMIN TRUE;
1799+
GRANT regress_group_direct_manager TO regress_group_indirect_manager;
1800+
1801+
SET SESSION AUTHORIZATION regress_group_direct_manager;
1802+
GRANT regress_group TO regress_group_member;
1803+
SELECT member::regrole::text, CASE WHEN grantor = 10 THEN 'BOOTSTRAP SUPERUSER' ELSE grantor::regrole::text END FROM pg_auth_members WHERE roleid = 'regress_group'::regrole ORDER BY 1, 2;
1804+
REVOKE regress_group FROM regress_group_member;
1805+
1806+
SET SESSION AUTHORIZATION regress_group_indirect_manager;
1807+
GRANT regress_group TO regress_group_member;
1808+
SELECT member::regrole::text, CASE WHEN grantor = 10 THEN 'BOOTSTRAP SUPERUSER' ELSE grantor::regrole::text END FROM pg_auth_members WHERE roleid = 'regress_group'::regrole ORDER BY 1, 2;
1809+
REVOKE regress_group FROM regress_group_member;
1810+
1811+
RESET SESSION AUTHORIZATION;
1812+
DROP ROLE regress_group;
1813+
DROP ROLE regress_group_direct_manager;
1814+
DROP ROLE regress_group_indirect_manager;
1815+
DROP ROLE regress_group_member;

0 commit comments

Comments
 (0)