Skip to content

Commit f781a0f

Browse files
jconwayjconway
committed
Create a pg_shdepend entry for each role in TO clause of policies.
CreatePolicy() and AlterPolicy() omit to create a pg_shdepend entry for each role in the TO clause. Fix this by creating a new shared dependency type called SHARED_DEPENDENCY_POLICY and assigning it to each role. Reported by Noah Misch. Patch by me, reviewed by Alvaro Herrera. Back-patch to 9.5 where RLS was introduced.
1 parent 8c72a7f commit f781a0f

File tree

6 files changed

+177
-26
lines changed

6 files changed

+177
-26
lines changed

‎doc/src/sgml/catalogs.sgml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5792,6 +5792,16 @@
57925792
</listitem>
57935793
</varlistentry>
57945794

5795+
<varlistentry>
5796+
<term><symbol>SHARED_DEPENDENCY_POLICY</> (<literal>r</>)</term>
5797+
<listitem>
5798+
<para>
5799+
The referenced object (which must be a role) is mentioned as the
5800+
target of a dependent policy object.
5801+
</para>
5802+
</listitem>
5803+
</varlistentry>
5804+
57955805
<varlistentry>
57965806
<term><symbol>SHARED_DEPENDENCY_PIN</> (<literal>p</>)</term>
57975807
<listitem>

‎src/backend/catalog/pg_shdepend.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,8 @@ storeObjectDescription(StringInfo descs,
10831083
appendStringInfo(descs, _("owner of %s"), objdesc);
10841084
else if (deptype == SHARED_DEPENDENCY_ACL)
10851085
appendStringInfo(descs, _("privileges for %s"), objdesc);
1086+
else if (deptype == SHARED_DEPENDENCY_POLICY)
1087+
appendStringInfo(descs, _("target of %s"), objdesc);
10861088
else
10871089
elog(ERROR, "unrecognized dependency type: %d",
10881090
(int) deptype);

‎src/backend/commands/policy.c

Lines changed: 61 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "catalog/indexing.h"
2323
#include "catalog/namespace.h"
2424
#include "catalog/objectaccess.h"
25+
#include "catalog/pg_authid.h"
2526
#include "catalog/pg_policy.h"
2627
#include "catalog/pg_type.h"
2728
#include "commands/policy.h"
@@ -48,7 +49,7 @@
4849
static void RangeVarCallbackForPolicy(const RangeVar *rv,
4950
Oid relid, Oid oldrelid, void *arg);
5051
static char parse_policy_command(const char *cmd_name);
51-
static ArrayType *policy_role_list_to_array(List *roles);
52+
static Datum *policy_role_list_to_array(List *roles, int *num_roles);
5253

5354
/*
5455
* Callback to RangeVarGetRelidExtended().
@@ -130,30 +131,28 @@ parse_policy_command(const char *cmd_name)
130131

131132
/*
132133
* policy_role_list_to_array
133-
* helper function to convert a list of RoleSpecs to an array of role ids.
134+
* helper function to convert a list of RoleSpecs to an array of
135+
* role id Datums.
134136
*/
135-
static ArrayType *
136-
policy_role_list_to_array(List *roles)
137+
static Datum *
138+
policy_role_list_to_array(List *roles, int *num_roles)
137139
{
138-
ArrayType *role_ids;
139-
Datum *temp_array;
140+
Datum *role_oids;
140141
ListCell *cell;
141-
int num_roles;
142142
int i = 0;
143143

144144
/* Handle no roles being passed in as being for public */
145145
if (roles == NIL)
146146
{
147-
temp_array = (Datum *) palloc(sizeof(Datum));
148-
temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
147+
*num_roles = 1;
148+
role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
149+
role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
149150

150-
role_ids = construct_array(temp_array, 1, OIDOID, sizeof(Oid), true,
151-
'i');
152-
return role_ids;
151+
return role_oids;
153152
}
154153

155-
num_roles = list_length(roles);
156-
temp_array = (Datum *) palloc(num_roles * sizeof(Datum));
154+
*num_roles = list_length(roles);
155+
role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
157156

158157
foreach(cell, roles)
159158
{
@@ -164,24 +163,24 @@ policy_role_list_to_array(List *roles)
164163
*/
165164
if (spec->roletype == ROLESPEC_PUBLIC)
166165
{
167-
if (num_roles != 1)
166+
if (*num_roles != 1)
167+
{
168168
ereport(WARNING,
169169
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
170170
errmsg("ignoring roles specified other than public"),
171171
errhint("All roles are members of the public role.")));
172-
temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
173-
num_roles = 1;
174-
break;
172+
*num_roles = 1;
173+
}
174+
role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
175+
176+
return role_oids;
175177
}
176178
else
177-
temp_array[i++] =
179+
role_oids[i++] =
178180
ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
179181
}
180182

181-
role_ids = construct_array(temp_array, num_roles, OIDOID, sizeof(Oid), true,
182-
'i');
183-
184-
return role_ids;
183+
return role_oids;
185184
}
186185

187186
/*
@@ -463,6 +462,8 @@ CreatePolicy(CreatePolicyStmt *stmt)
463462
Relation target_table;
464463
Oid table_id;
465464
char polcmd;
465+
Datum *role_oids;
466+
int nitems = 0;
466467
ArrayType *role_ids;
467468
ParseState *qual_pstate;
468469
ParseState *with_check_pstate;
@@ -476,6 +477,7 @@ CreatePolicy(CreatePolicyStmt *stmt)
476477
bool isnull[Natts_pg_policy];
477478
ObjectAddress target;
478479
ObjectAddress myself;
480+
int i;
479481

480482
/* Parse command */
481483
polcmd = parse_policy_command(stmt->cmd);
@@ -498,9 +500,10 @@ CreatePolicy(CreatePolicyStmt *stmt)
498500
(errcode(ERRCODE_SYNTAX_ERROR),
499501
errmsg("only WITH CHECK expression allowed for INSERT")));
500502

501-
502503
/* Collect role ids */
503-
role_ids = policy_role_list_to_array(stmt->roles);
504+
role_oids = policy_role_list_to_array(stmt->roles, &nitems);
505+
role_ids = construct_array(role_oids, nitems, OIDOID,
506+
sizeof(Oid), true, 'i');
504507

505508
/* Parse the supplied clause */
506509
qual_pstate = make_parsestate(NULL);
@@ -614,6 +617,18 @@ CreatePolicy(CreatePolicyStmt *stmt)
614617
recordDependencyOnExpr(&myself, with_check_qual,
615618
with_check_pstate->p_rtable, DEPENDENCY_NORMAL);
616619

620+
/* Register role dependencies */
621+
target.classId = AuthIdRelationId;
622+
target.objectSubId = 0;
623+
for (i = 0; i < nitems; i++)
624+
{
625+
target.objectId = DatumGetObjectId(role_oids[i]);
626+
/* no dependency if public */
627+
if (target.objectId != ACL_ID_PUBLIC)
628+
recordSharedDependencyOn(&myself, &target,
629+
SHARED_DEPENDENCY_POLICY);
630+
}
631+
617632
/* Invalidate Relation Cache */
618633
CacheInvalidateRelcache(target_table);
619634

@@ -641,6 +656,8 @@ AlterPolicy(AlterPolicyStmt *stmt)
641656
Oid policy_id;
642657
Relation target_table;
643658
Oid table_id;
659+
Datum *role_oids;
660+
int nitems = 0;
644661
ArrayType *role_ids = NULL;
645662
List *qual_parse_rtable = NIL;
646663
List *with_check_parse_rtable = NIL;
@@ -658,10 +675,15 @@ AlterPolicy(AlterPolicyStmt *stmt)
658675
Datum cmd_datum;
659676
char polcmd;
660677
bool polcmd_isnull;
678+
int i;
661679

662680
/* Parse role_ids */
663681
if (stmt->roles != NULL)
664-
role_ids = policy_role_list_to_array(stmt->roles);
682+
{
683+
role_oids = policy_role_list_to_array(stmt->roles, &nitems);
684+
role_ids = construct_array(role_oids, nitems, OIDOID,
685+
sizeof(Oid), true, 'i');
686+
}
665687

666688
/* Get id of table. Also handles permissions checks. */
667689
table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock,
@@ -825,6 +847,19 @@ AlterPolicy(AlterPolicyStmt *stmt)
825847
recordDependencyOnExpr(&myself, with_check_qual, with_check_parse_rtable,
826848
DEPENDENCY_NORMAL);
827849

850+
/* Register role dependencies */
851+
deleteSharedDependencyRecordsFor(PolicyRelationId, policy_id, 0);
852+
target.classId = AuthIdRelationId;
853+
target.objectSubId = 0;
854+
for (i = 0; i < nitems; i++)
855+
{
856+
target.objectId = DatumGetObjectId(role_oids[i]);
857+
/* no dependency if public */
858+
if (target.objectId != ACL_ID_PUBLIC)
859+
recordSharedDependencyOn(&myself, &target,
860+
SHARED_DEPENDENCY_POLICY);
861+
}
862+
828863
heap_freetuple(new_tuple);
829864

830865
/* Invalidate Relation Cache */

‎src/include/catalog/dependency.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ typedef enum DependencyType
9696
* created for the owner of an object; hence two objects may be linked by
9797
* one or the other, but not both, of these dependency types.)
9898
*
99+
* (d) a SHARED_DEPENDENCY_POLICY entry means that the referenced object is
100+
* a role mentioned in a policy object. The referenced object must be a
101+
* pg_authid entry.
102+
*
99103
* SHARED_DEPENDENCY_INVALID is a value used as a parameter in internal
100104
* routines, and is not valid in the catalog itself.
101105
*/
@@ -104,6 +108,7 @@ typedef enum SharedDependencyType
104108
SHARED_DEPENDENCY_PIN = 'p',
105109
SHARED_DEPENDENCY_OWNER = 'o',
106110
SHARED_DEPENDENCY_ACL = 'a',
111+
SHARED_DEPENDENCY_POLICY = 'r',
107112
SHARED_DEPENDENCY_INVALID = 0
108113
} SharedDependencyType;
109114

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2942,6 +2942,61 @@ SELECT * FROM coll_t;
29422942

29432943
ROLLBACK;
29442944
--
2945+
-- Shared Object Dependencies
2946+
--
2947+
RESET SESSION AUTHORIZATION;
2948+
BEGIN;
2949+
CREATE ROLE alice;
2950+
CREATE ROLE bob;
2951+
CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
2952+
GRANT SELECT ON TABLE tbl1 TO alice;
2953+
CREATE POLICY P ON tbl1 TO alice, bob USING (true);
2954+
SELECT refclassid::regclass, deptype
2955+
FROM pg_depend
2956+
WHERE classid = 'pg_policy'::regclass
2957+
AND refobjid = 'tbl1'::regclass;
2958+
refclassid | deptype
2959+
------------+---------
2960+
pg_class | a
2961+
(1 row)
2962+
2963+
SELECT refclassid::regclass, deptype
2964+
FROM pg_shdepend
2965+
WHERE classid = 'pg_policy'::regclass
2966+
AND refobjid IN ('alice'::regrole, 'bob'::regrole);
2967+
refclassid | deptype
2968+
------------+---------
2969+
pg_authid | r
2970+
pg_authid | r
2971+
(2 rows)
2972+
2973+
SAVEPOINT q;
2974+
DROP ROLE alice; --fails due to dependency on POLICY p
2975+
ERROR: role "alice" cannot be dropped because some objects depend on it
2976+
DETAIL: target of policy p on table tbl1
2977+
privileges for table tbl1
2978+
ROLLBACK TO q;
2979+
ALTER POLICY p ON tbl1 TO bob USING (true);
2980+
SAVEPOINT q;
2981+
DROP ROLE alice; --fails due to dependency on GRANT SELECT
2982+
ERROR: role "alice" cannot be dropped because some objects depend on it
2983+
DETAIL: privileges for table tbl1
2984+
ROLLBACK TO q;
2985+
REVOKE ALL ON TABLE tbl1 FROM alice;
2986+
SAVEPOINT q;
2987+
DROP ROLE alice; --succeeds
2988+
ROLLBACK TO q;
2989+
SAVEPOINT q;
2990+
DROP ROLE bob; --fails due to dependency on POLICY p
2991+
ERROR: role "bob" cannot be dropped because some objects depend on it
2992+
DETAIL: target of policy p on table tbl1
2993+
ROLLBACK TO q;
2994+
DROP POLICY p ON tbl1;
2995+
SAVEPOINT q;
2996+
DROP ROLE bob; -- succeeds
2997+
ROLLBACK TO q;
2998+
ROLLBACK; -- cleanup
2999+
--
29453000
-- Clean up objects
29463001
--
29473002
RESET SESSION AUTHORIZATION;

‎src/test/regress/sql/rowsecurity.sql

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,50 @@ SELECT (string_to_array(polqual, ':'))[7] AS inputcollid FROM pg_policy WHERE po
12161216
SELECT * FROM coll_t;
12171217
ROLLBACK;
12181218

1219+
--
1220+
-- Shared Object Dependencies
1221+
--
1222+
RESET SESSION AUTHORIZATION;
1223+
BEGIN;
1224+
CREATE ROLE alice;
1225+
CREATE ROLE bob;
1226+
CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
1227+
GRANT SELECT ON TABLE tbl1 TO alice;
1228+
CREATE POLICY P ON tbl1 TO alice, bob USING (true);
1229+
SELECT refclassid::regclass, deptype
1230+
FROM pg_depend
1231+
WHERE classid = 'pg_policy'::regclass
1232+
AND refobjid = 'tbl1'::regclass;
1233+
SELECT refclassid::regclass, deptype
1234+
FROM pg_shdepend
1235+
WHERE classid = 'pg_policy'::regclass
1236+
AND refobjid IN ('alice'::regrole, 'bob'::regrole);
1237+
1238+
SAVEPOINT q;
1239+
DROP ROLE alice; --fails due to dependency on POLICY p
1240+
ROLLBACK TO q;
1241+
1242+
ALTER POLICY p ON tbl1 TO bob USING (true);
1243+
SAVEPOINT q;
1244+
DROP ROLE alice; --fails due to dependency on GRANT SELECT
1245+
ROLLBACK TO q;
1246+
1247+
REVOKE ALL ON TABLE tbl1 FROM alice;
1248+
SAVEPOINT q;
1249+
DROP ROLE alice; --succeeds
1250+
ROLLBACK TO q;
1251+
1252+
SAVEPOINT q;
1253+
DROP ROLE bob; --fails due to dependency on POLICY p
1254+
ROLLBACK TO q;
1255+
1256+
DROP POLICY p ON tbl1;
1257+
SAVEPOINT q;
1258+
DROP ROLE bob; -- succeeds
1259+
ROLLBACK TO q;
1260+
1261+
ROLLBACK; -- cleanup
1262+
12191263
--
12201264
-- Clean up objects
12211265
--

0 commit comments

Comments
 (0)