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.

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9096ee5d517de88aff7d8cd0c233cc8862c13b3c..7781c56f0eb28b5924f314977d245b6b49a7f938 100644 (file)
--- a/doc/src/sgml/catalogs.sgml
+++ b/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>

diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 34fe4e2474585a725241cf57560ec807459f4bd3..43076c9c287d25ecd41c91db01d32901c6b139d0 100644 (file)
--- a/src/backend/catalog/pg_shdepend.c
+++ b/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);

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 17b48d49596b1c5f3872753a2255b484fa6cd3a9..9544f75032b4b1bf614686b0aaf8ad3064cbde73 100644 (file)
--- a/src/backend/commands/policy.c
+++ b/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 */

diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index aa3f3d90a18e68b45251a71d88eb1f5d6907a36e..fbcf9044325d44965ae65999f47bb7c0a8ae7d0b 100644 (file)
--- a/src/include/catalog/dependency.h
+++ b/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;
 

diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index fd8e180f8a8e88b18fdb4898d708ae62da4c25b2..4749efc5679d9cf31cd50ea2badb16c9c077c456 100644 (file)
--- a/src/test/regress/expected/rowsecurity.out
+++ b/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;

diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 32f10d8649f1e12502e40c7a5486cba4134ece3e..529edd01c7f0b95072d61518547dd01714d061aa 100644 (file)
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/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
 --