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.

Author: jconway

doc/src/sgml/catalogs.sgml

@@ -5792,6 +5792,16 @@
      </listitem>
     </varlistentry>
 
+    <varlistentry>
+     <term><symbol>SHARED_DEPENDENCY_POLICY</> (<literal>r</>)</term>
+     <listitem>
+      <para>
+       The referenced object (which must be a role) is mentioned as the
+       target of a dependent policy object.
+      </para>
+     </listitem>
+    </varlistentry>
+
     <varlistentry>
      <term><symbol>SHARED_DEPENDENCY_PIN</> (<literal>p</>)</term>
      <listitem>

src/backend/catalog/pg_shdepend.c

@@ -1083,6 +1083,8 @@ storeObjectDescription(StringInfo descs,
 				appendStringInfo(descs, _("owner of %s"), objdesc);
 			else if (deptype == SHARED_DEPENDENCY_ACL)
 				appendStringInfo(descs, _("privileges for %s"), objdesc);
+			else if (deptype == SHARED_DEPENDENCY_POLICY)
+				appendStringInfo(descs, _("target of %s"), objdesc);
 			else
 				elog(ERROR, "unrecognized dependency type: %d",
 					 (int) deptype);

src/backend/commands/policy.c

@@ -22,6 +22,7 @@
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
 #include "catalog/objectaccess.h"
+#include "catalog/pg_authid.h"
 #include "catalog/pg_policy.h"
 #include "catalog/pg_type.h"
 #include "commands/policy.h"
@@ -48,7 +49,7 @@
 static void RangeVarCallbackForPolicy(const RangeVar *rv,
 						  Oid relid, Oid oldrelid, void *arg);
 static char parse_policy_command(const char *cmd_name);
-static ArrayType *policy_role_list_to_array(List *roles);
+static Datum *policy_role_list_to_array(List *roles, int *num_roles);
 
 /*
  * Callback to RangeVarGetRelidExtended().
@@ -130,30 +131,28 @@ parse_policy_command(const char *cmd_name)
 
 /*
  * policy_role_list_to_array
- *	 helper function to convert a list of RoleSpecs to an array of role ids.
+ *	 helper function to convert a list of RoleSpecs to an array of
+ *	 role id Datums.
  */
-static ArrayType *
-policy_role_list_to_array(List *roles)
+static Datum *
+policy_role_list_to_array(List *roles, int *num_roles)
 {
-	ArrayType  *role_ids;
-	Datum	   *temp_array;
+	Datum	   *role_oids;
 	ListCell   *cell;
-	int			num_roles;
 	int			i = 0;
 
 	/* Handle no roles being passed in as being for public */
 	if (roles == NIL)
 	{
-		temp_array = (Datum *) palloc(sizeof(Datum));
-		temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
+	   *num_roles = 1;
+		role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
+		role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
-		role_ids = construct_array(temp_array, 1, OIDOID, sizeof(Oid), true,
-								   'i');
-		return role_ids;
+		return role_oids;
 	}
 
-	num_roles = list_length(roles);
-	temp_array = (Datum *) palloc(num_roles * sizeof(Datum));
+   *num_roles = list_length(roles);
+	role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
 
 	foreach(cell, roles)
 	{
@@ -164,24 +163,24 @@ policy_role_list_to_array(List *roles)
 		 */
 		if (spec->roletype == ROLESPEC_PUBLIC)
 		{
-			if (num_roles != 1)
+			if (*num_roles != 1)
+			{
 				ereport(WARNING,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						 errmsg("ignoring roles specified other than public"),
 					  errhint("All roles are members of the public role.")));
-			temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
-			num_roles = 1;
-			break;
+			   *num_roles = 1;
+			}
+			role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
+
+			return role_oids;
 		}
 		else
-			temp_array[i++] =
+			role_oids[i++] =
 				ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
 	}
 
-	role_ids = construct_array(temp_array, num_roles, OIDOID, sizeof(Oid), true,
-							   'i');
-
-	return role_ids;
+	return role_oids;
 }
 
 /*
@@ -463,6 +462,8 @@ CreatePolicy(CreatePolicyStmt *stmt)
 	Relation	target_table;
 	Oid			table_id;
 	char		polcmd;
+	Datum	   *role_oids;
+	int			nitems = 0;
 	ArrayType  *role_ids;
 	ParseState *qual_pstate;
 	ParseState *with_check_pstate;
@@ -476,6 +477,7 @@ CreatePolicy(CreatePolicyStmt *stmt)
 	bool		isnull[Natts_pg_policy];
 	ObjectAddress target;
 	ObjectAddress myself;
+	int			i;
 
 	/* Parse command */
 	polcmd = parse_policy_command(stmt->cmd);
@@ -498,9 +500,10 @@ CreatePolicy(CreatePolicyStmt *stmt)
 				(errcode(ERRCODE_SYNTAX_ERROR),
 				 errmsg("only WITH CHECK expression allowed for INSERT")));
 
-
 	/* Collect role ids */
-	role_ids = policy_role_list_to_array(stmt->roles);
+	role_oids = policy_role_list_to_array(stmt->roles, &nitems);
+	role_ids = construct_array(role_oids, nitems, OIDOID,
+							   sizeof(Oid), true, 'i');
 
 	/* Parse the supplied clause */
 	qual_pstate = make_parsestate(NULL);
@@ -614,6 +617,18 @@ CreatePolicy(CreatePolicyStmt *stmt)
 	recordDependencyOnExpr(&myself, with_check_qual,
 						   with_check_pstate->p_rtable, DEPENDENCY_NORMAL);
 
+	/* Register role dependencies */
+	target.classId = AuthIdRelationId;
+	target.objectSubId = 0;
+	for (i = 0; i < nitems; i++)
+	{
+		target.objectId = DatumGetObjectId(role_oids[i]);
+		/* no dependency if public */
+		if (target.objectId != ACL_ID_PUBLIC)
+			recordSharedDependencyOn(&myself, &target,
+									 SHARED_DEPENDENCY_POLICY);
+	}
+
 	/* Invalidate Relation Cache */
 	CacheInvalidateRelcache(target_table);
 
@@ -641,6 +656,8 @@ AlterPolicy(AlterPolicyStmt *stmt)
 	Oid			policy_id;
 	Relation	target_table;
 	Oid			table_id;
+	Datum	   *role_oids;
+	int			nitems = 0;
 	ArrayType  *role_ids = NULL;
 	List	   *qual_parse_rtable = NIL;
 	List	   *with_check_parse_rtable = NIL;
@@ -658,10 +675,15 @@ AlterPolicy(AlterPolicyStmt *stmt)
 	Datum		cmd_datum;
 	char		polcmd;
 	bool		polcmd_isnull;
+	int			i;
 
 	/* Parse role_ids */
 	if (stmt->roles != NULL)
-		role_ids = policy_role_list_to_array(stmt->roles);
+	{
+		role_oids = policy_role_list_to_array(stmt->roles, &nitems);
+		role_ids = construct_array(role_oids, nitems, OIDOID,
+								   sizeof(Oid), true, 'i');
+	}
 
 	/* Get id of table.  Also handles permissions checks. */
 	table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock,
@@ -825,6 +847,19 @@ AlterPolicy(AlterPolicyStmt *stmt)
 	recordDependencyOnExpr(&myself, with_check_qual, with_check_parse_rtable,
 						   DEPENDENCY_NORMAL);
 
+	/* Register role dependencies */
+	deleteSharedDependencyRecordsFor(PolicyRelationId, policy_id, 0);
+	target.classId = AuthIdRelationId;
+	target.objectSubId = 0;
+	for (i = 0; i < nitems; i++)
+	{
+		target.objectId = DatumGetObjectId(role_oids[i]);
+		/* no dependency if public */
+		if (target.objectId != ACL_ID_PUBLIC)
+			recordSharedDependencyOn(&myself, &target,
+									 SHARED_DEPENDENCY_POLICY);
+	}
+
 	heap_freetuple(new_tuple);
 
 	/* Invalidate Relation Cache */

src/include/catalog/dependency.h

@@ -96,6 +96,10 @@ typedef enum DependencyType
  * created for the owner of an object; hence two objects may be linked by
  * one or the other, but not both, of these dependency types.)
  *
+ * (d) a SHARED_DEPENDENCY_POLICY entry means that the referenced object is
+ * a role mentioned in a policy object.  The referenced object must be a
+ * pg_authid entry.
+ *
  * SHARED_DEPENDENCY_INVALID is a value used as a parameter in internal
  * routines, and is not valid in the catalog itself.
  */
@@ -104,6 +108,7 @@ typedef enum SharedDependencyType
 	SHARED_DEPENDENCY_PIN = 'p',
 	SHARED_DEPENDENCY_OWNER = 'o',
 	SHARED_DEPENDENCY_ACL = 'a',
+	SHARED_DEPENDENCY_POLICY = 'r',
 	SHARED_DEPENDENCY_INVALID = 0
 } SharedDependencyType;
 

src/test/regress/expected/rowsecurity.out

@@ -2942,6 +2942,61 @@ SELECT * FROM coll_t;
 
 ROLLBACK;
 --
+-- Shared Object Dependencies
+--
+RESET SESSION AUTHORIZATION;
+BEGIN;
+CREATE ROLE alice;
+CREATE ROLE bob;
+CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
+GRANT SELECT ON TABLE tbl1 TO alice;
+CREATE POLICY P ON tbl1 TO alice, bob USING (true);
+SELECT refclassid::regclass, deptype
+  FROM pg_depend
+  WHERE classid = 'pg_policy'::regclass
+  AND refobjid = 'tbl1'::regclass;
+ refclassid | deptype 
+------------+---------
+ pg_class   | a
+(1 row)
+
+SELECT refclassid::regclass, deptype
+  FROM pg_shdepend
+  WHERE classid = 'pg_policy'::regclass
+  AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+ refclassid | deptype 
+------------+---------
+ pg_authid  | r
+ pg_authid  | r
+(2 rows)
+
+SAVEPOINT q;
+DROP ROLE alice; --fails due to dependency on POLICY p
+ERROR:  role "alice" cannot be dropped because some objects depend on it
+DETAIL:  target of policy p on table tbl1
+privileges for table tbl1
+ROLLBACK TO q;
+ALTER POLICY p ON tbl1 TO bob USING (true);
+SAVEPOINT q;
+DROP ROLE alice; --fails due to dependency on GRANT SELECT
+ERROR:  role "alice" cannot be dropped because some objects depend on it
+DETAIL:  privileges for table tbl1
+ROLLBACK TO q;
+REVOKE ALL ON TABLE tbl1 FROM alice;
+SAVEPOINT q;
+DROP ROLE alice; --succeeds
+ROLLBACK TO q;
+SAVEPOINT q;
+DROP ROLE bob; --fails due to dependency on POLICY p
+ERROR:  role "bob" cannot be dropped because some objects depend on it
+DETAIL:  target of policy p on table tbl1
+ROLLBACK TO q;
+DROP POLICY p ON tbl1;
+SAVEPOINT q;
+DROP ROLE bob; -- succeeds
+ROLLBACK TO q;
+ROLLBACK; -- cleanup
+--
 -- Clean up objects
 --
 RESET SESSION AUTHORIZATION;

src/test/regress/sql/rowsecurity.sql

@@ -1216,6 +1216,50 @@ SELECT (string_to_array(polqual, ':'))[7] AS inputcollid FROM pg_policy WHERE po
 SELECT * FROM coll_t;
 ROLLBACK;
 
+--
+-- Shared Object Dependencies
+--
+RESET SESSION AUTHORIZATION;
+BEGIN;
+CREATE ROLE alice;
+CREATE ROLE bob;
+CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
+GRANT SELECT ON TABLE tbl1 TO alice;
+CREATE POLICY P ON tbl1 TO alice, bob USING (true);
+SELECT refclassid::regclass, deptype
+  FROM pg_depend
+  WHERE classid = 'pg_policy'::regclass
+  AND refobjid = 'tbl1'::regclass;
+SELECT refclassid::regclass, deptype
+  FROM pg_shdepend
+  WHERE classid = 'pg_policy'::regclass
+  AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+
+SAVEPOINT q;
+DROP ROLE alice; --fails due to dependency on POLICY p
+ROLLBACK TO q;
+
+ALTER POLICY p ON tbl1 TO bob USING (true);
+SAVEPOINT q;
+DROP ROLE alice; --fails due to dependency on GRANT SELECT
+ROLLBACK TO q;
+
+REVOKE ALL ON TABLE tbl1 FROM alice;
+SAVEPOINT q;
+DROP ROLE alice; --succeeds
+ROLLBACK TO q;
+
+SAVEPOINT q;
+DROP ROLE bob; --fails due to dependency on POLICY p
+ROLLBACK TO q;
+
+DROP POLICY p ON tbl1;
+SAVEPOINT q;
+DROP ROLE bob; -- succeeds
+ROLLBACK TO q;
+
+ROLLBACK; -- cleanup
+
 --
 -- Clean up objects
 --