Handle policies during DROP OWNED BY

DROP OWNED BY handled GRANT-based ACLs but was not removing roles from
policies.  Fix that by having DROP OWNED BY remove the role specified
from the list of roles the policy (or policies) apply to, or the entire
policy (or policies) if it only applied to the role specified.

As with ACLs, the DROP OWNED BY caller must have permission to modify
the policy or a WARNING is thrown and no change is made to the policy.

diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 43076c9c287d25ecd41c91db01d32901c6b139d0..eeb231be7e62e6dbb611028dbc0d49938766e0a6 100644 (file)
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -50,6 +50,7 @@
 #include "commands/defrem.h"
 #include "commands/event_trigger.h"
 #include "commands/extension.h"
+#include "commands/policy.h"
 #include "commands/proclang.h"
 #include "commands/schemacmds.h"
 #include "commands/tablecmds.h"
@@ -1245,6 +1246,18 @@ shdepDropOwned(List *roleids, DropBehavior behavior)
                                                                                        sdepForm->classid,
                                                                                        sdepForm->objid);
                                        break;
+                               case SHARED_DEPENDENCY_POLICY:
+                                       /* If unable to remove role from policy, remove policy. */
+                                       if (!RemoveRoleFromObjectPolicy(roleid,
+                                                                                                       sdepForm->classid,
+                                                                                                       sdepForm->objid))
+                                       {
+                                               obj.classId = sdepForm->classid;
+                                               obj.objectId = sdepForm->objid;
+                                               obj.objectSubId = sdepForm->objsubid;
+                                               add_exact_object_address(&obj, deleteobjs);
+                                       }
+                                       break;
                                case SHARED_DEPENDENCY_OWNER:
                                        /* If a local object, save it for deletion below */
                                        if (sdepForm->dbid == MyDatabaseId)

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 09164c6ed3664ce785933e5b95a87b276649f815..6b9c3065b4f9ab1d3661a34f7ae47316e23b30f4 100644 (file)
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -407,6 +407,262 @@ RemovePolicyById(Oid policy_id)
        heap_close(pg_policy_rel, RowExclusiveLock);
 }
 
+/*
+ * RemoveRoleFromObjectPolicy -
+ *      remove a role from a policy by its OID.  If the role is not a member of
+ *      the policy then an error is raised.  False is returned to indicate that
+ *      the role could not be removed due to being the only role on the policy
+ *      and therefore the entire policy should be removed.
+ *
+ * Note that a warning will be thrown and true will be returned on a
+ * permission error, as the policy should not be removed in that case.
+ *
+ * roleid - the oid of the role to remove
+ * classid - should always be PolicyRelationId
+ * policy_id - the oid of the policy.
+ */
+bool
+RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
+{
+       Relation        pg_policy_rel;
+       SysScanDesc sscan;
+       ScanKeyData skey[1];
+       HeapTuple       tuple;
+       Oid                     relid;
+       Relation        rel;
+       ArrayType  *policy_roles;
+       int                     num_roles;
+       Datum           roles_datum;
+       bool            attr_isnull;
+       bool            noperm = true;
+
+       Assert(classid == PolicyRelationId);
+
+       pg_policy_rel = heap_open(PolicyRelationId, RowExclusiveLock);
+
+       /*
+        * Find the policy to update.
+        */
+       ScanKeyInit(&skey[0],
+                               ObjectIdAttributeNumber,
+                               BTEqualStrategyNumber, F_OIDEQ,
+                               ObjectIdGetDatum(policy_id));
+
+       sscan = systable_beginscan(pg_policy_rel, PolicyOidIndexId, true,
+                                                          NULL, 1, skey);
+
+       tuple = systable_getnext(sscan);
+
+       /* Raise an error if we don't find the policy. */
+       if (!HeapTupleIsValid(tuple))
+               elog(ERROR, "could not find tuple for policy %u", policy_id);
+
+       /*
+        * Open and exclusive-lock the relation the policy belongs to.
+        */
+       relid = ((Form_pg_policy) GETSTRUCT(tuple))->polrelid;
+
+       rel = relation_open(relid, AccessExclusiveLock);
+
+       if (rel->rd_rel->relkind != RELKIND_RELATION)
+               ereport(ERROR,
+                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                errmsg("\"%s\" is not a table",
+                                               RelationGetRelationName(rel))));
+
+       if (!allowSystemTableMods && IsSystemRelation(rel))
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                errmsg("permission denied: \"%s\" is a system catalog",
+                                               RelationGetRelationName(rel))));
+
+       /* Get the current set of roles */
+       roles_datum = heap_getattr(tuple,
+                                                          Anum_pg_policy_polroles,
+                                                          RelationGetDescr(pg_policy_rel),
+                                                          &attr_isnull);
+
+       Assert(!attr_isnull);
+
+       policy_roles = DatumGetArrayTypePCopy(roles_datum);
+
+       /* We should be removing exactly one entry from the roles array */
+       num_roles = ARR_DIMS(policy_roles)[0] - 1;
+
+       Assert(num_roles >= 0);
+
+       /* Must own relation. */
+       if (pg_class_ownercheck(relid, GetUserId()))
+               noperm = false; /* user is allowed to modify this policy */
+       else
+               ereport(WARNING,
+                               (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
+                                errmsg("role \"%s\" could not be removed from policy \"%s\" on \"%s\"",
+                                               GetUserNameFromId(roleid, false),
+                                               NameStr(((Form_pg_policy) GETSTRUCT(tuple))->polname),
+                                               RelationGetRelationName(rel))));
+
+       /*
+        * If multiple roles exist on this policy, then remove the one we were
+        * asked to and leave the rest.
+        */
+       if (!noperm && num_roles > 0)
+       {
+               int                     i, j;
+               Oid                *roles = (Oid *) ARR_DATA_PTR(policy_roles);
+               Datum      *role_oids;
+               char       *qual_value;
+               Node       *qual_expr;
+               List       *qual_parse_rtable = NIL;
+               char       *with_check_value;
+               Node       *with_check_qual;
+               List       *with_check_parse_rtable = NIL;
+               Datum           values[Natts_pg_policy];
+               bool            isnull[Natts_pg_policy];
+               bool            replaces[Natts_pg_policy];
+               Datum           value_datum;
+               ArrayType  *role_ids;
+               HeapTuple       new_tuple;
+               ObjectAddress target;
+               ObjectAddress myself;
+
+               /* zero-clear */
+               memset(values, 0, sizeof(values));
+               memset(replaces, 0, sizeof(replaces));
+               memset(isnull, 0, sizeof(isnull));
+
+               /*
+                * All of the dependencies will be removed from the policy and then
+                * re-added.  In order to get them correct, we need to extract out
+                * the expressions in the policy and construct a parsestate just
+                * enough to build the range table(s) to then pass to
+                * recordDependencyOnExpr().
+                */
+
+               /* Get policy qual, to update dependencies */
+               value_datum = heap_getattr(tuple, Anum_pg_policy_polqual,
+                                                                  RelationGetDescr(pg_policy_rel), &attr_isnull);
+               if (!attr_isnull)
+               {
+                       ParseState *qual_pstate;
+
+                       /* parsestate is built just to build the range table */
+                       qual_pstate = make_parsestate(NULL);
+
+                       qual_value = TextDatumGetCString(value_datum);
+                       qual_expr = stringToNode(qual_value);
+
+                       /* Add this rel to the parsestate's rangetable, for dependencies */
+                       addRangeTableEntryForRelation(qual_pstate, rel, NULL, false, false);
+
+                       qual_parse_rtable = qual_pstate->p_rtable;
+                       free_parsestate(qual_pstate);
+               }
+               else
+                       qual_expr = NULL;
+
+               /* Get WITH CHECK qual, to update dependencies */
+               value_datum = heap_getattr(tuple, Anum_pg_policy_polwithcheck,
+                                                                  RelationGetDescr(pg_policy_rel), &attr_isnull);
+               if (!attr_isnull)
+               {
+                       ParseState *with_check_pstate;
+
+                       /* parsestate is built just to build the range table */
+                       with_check_pstate = make_parsestate(NULL);
+
+                       with_check_value = TextDatumGetCString(value_datum);
+                       with_check_qual = stringToNode(with_check_value);
+
+                       /* Add this rel to the parsestate's rangetable, for dependencies */
+                       addRangeTableEntryForRelation(with_check_pstate, rel, NULL, false,
+                                                                                 false);
+
+                       with_check_parse_rtable = with_check_pstate->p_rtable;
+                       free_parsestate(with_check_pstate);
+               }
+               else
+                       with_check_qual = NULL;
+
+               /* Rebuild the roles array to then update the pg_policy tuple with */
+               role_oids = (Datum *) palloc(num_roles * sizeof(Datum));
+               for (i = 0, j = 0; i < ARR_DIMS(policy_roles)[0]; i++)
+                       /* Copy over all of the roles which are not the one being removed */
+                       if (roles[i] != roleid)
+                               role_oids[j++] = ObjectIdGetDatum(roles[i]);
+
+               /* We should have only removed the one role */
+               Assert(j == num_roles);
+
+               /* This is the array for the new tuple */
+               role_ids = construct_array(role_oids, num_roles, OIDOID,
+                                                                  sizeof(Oid), true, 'i');
+
+               replaces[Anum_pg_policy_polroles - 1] = true;
+               values[Anum_pg_policy_polroles - 1] = PointerGetDatum(role_ids);
+
+               new_tuple = heap_modify_tuple(tuple,
+                                                                         RelationGetDescr(pg_policy_rel),
+                                                                         values, isnull, replaces);
+               simple_heap_update(pg_policy_rel, &new_tuple->t_self, new_tuple);
+
+               /* Update Catalog Indexes */
+               CatalogUpdateIndexes(pg_policy_rel, new_tuple);
+
+               /* Remove all old dependencies. */
+               deleteDependencyRecordsFor(PolicyRelationId, policy_id, false);
+
+               /* Record the new set of dependencies */
+               target.classId = RelationRelationId;
+               target.objectId = relid;
+               target.objectSubId = 0;
+
+               myself.classId = PolicyRelationId;
+               myself.objectId = policy_id;
+               myself.objectSubId = 0;
+
+               recordDependencyOn(&myself, &target, DEPENDENCY_AUTO);
+
+               if (qual_expr)
+                       recordDependencyOnExpr(&myself, qual_expr, qual_parse_rtable,
+                                                                  DEPENDENCY_NORMAL);
+
+               if (with_check_qual)
+                       recordDependencyOnExpr(&myself, with_check_qual,
+                                                                  with_check_parse_rtable,
+                                                                  DEPENDENCY_NORMAL);
+
+               /* Remove all the old shared dependencies (roles) */
+               deleteSharedDependencyRecordsFor(PolicyRelationId, policy_id, 0);
+
+               /* Record the new shared dependencies (roles) */
+               target.classId = AuthIdRelationId;
+               target.objectSubId = 0;
+               for (i = 0; i < num_roles; i++)
+               {
+                       target.objectId = DatumGetObjectId(role_oids[i]);
+                       /* no need for dependency on the public role */
+                       if (target.objectId != ACL_ID_PUBLIC)
+                               recordSharedDependencyOn(&myself, &target,
+                                                                                SHARED_DEPENDENCY_POLICY);
+               }
+
+               InvokeObjectPostAlterHook(PolicyRelationId, policy_id, 0);
+
+               heap_freetuple(new_tuple);
+
+               /* Invalidate Relation Cache */
+               CacheInvalidateRelcache(rel);
+       }
+
+       /* Clean up. */
+       systable_endscan(sscan);
+       relation_close(rel, AccessExclusiveLock);
+       heap_close(pg_policy_rel, RowExclusiveLock);
+
+       return(noperm || num_roles > 0);
+}
+
 /*
  * CreatePolicy -
  *      handles the execution of the CREATE POLICY command.

diff --git a/src/include/commands/policy.h b/src/include/commands/policy.h
index be000432312ff2e2b6e23a2800b1d5ccdb62dff6..4b1588756d28ed5ca5e2b64df4d95d4d77208e20 100644 (file)
--- a/src/include/commands/policy.h
+++ b/src/include/commands/policy.h
@@ -23,6 +23,8 @@ extern void RelationBuildRowSecurity(Relation relation);
 
 extern void RemovePolicyById(Oid policy_id);
 
+extern bool RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid objid);
+
 extern ObjectAddress CreatePolicy(CreatePolicyStmt *stmt);
 extern ObjectAddress AlterPolicy(AlterPolicyStmt *stmt);
 

diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index e7b6ff40afcce4c784fad6b46b6494db516aa429..d4b9c706465491fb33ff9bb628af7a09abb57c2d 100644 (file)
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -3289,6 +3289,20 @@ SELECT count(*) = 0 FROM pg_depend
  t
 (1 row)
 
+-- DROP OWNED BY testing
+RESET SESSION AUTHORIZATION;
+CREATE ROLE dob_role1;
+CREATE ROLE dob_role2;
+CREATE TABLE dob_t1 (c1 int);
+CREATE POLICY p1 ON dob_t1 TO dob_role1 USING (true);
+DROP OWNED BY dob_role1;
+DROP POLICY p1 ON dob_t1; -- should fail, already gone
+ERROR:  policy "p1" for table "dob_t1" does not exist
+CREATE POLICY p1 ON dob_t1 TO dob_role1,dob_role2 USING (true);
+DROP OWNED BY dob_role1;
+DROP POLICY p1 ON dob_t1; -- should succeed
+DROP USER dob_role1;
+DROP USER dob_role2;
 --
 -- Clean up objects
 --

diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index b06a2066144be1a5bb0dbd0c73bf27ebb5f5508f..3966a55f2ca0d67b8f11d048d59c69a28c2a35a8 100644 (file)
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -1520,6 +1520,24 @@ SELECT count(*) = 0 FROM pg_depend
                                   WHERE objid = (SELECT oid FROM pg_policy WHERE polname = 'dep_p1')
                                         AND refobjid = (SELECT oid FROM pg_class WHERE relname = 'dep2');
 
+-- DROP OWNED BY testing
+RESET SESSION AUTHORIZATION;
+
+CREATE ROLE dob_role1;
+CREATE ROLE dob_role2;
+
+CREATE TABLE dob_t1 (c1 int);
+
+CREATE POLICY p1 ON dob_t1 TO dob_role1 USING (true);
+DROP OWNED BY dob_role1;
+DROP POLICY p1 ON dob_t1; -- should fail, already gone
+
+CREATE POLICY p1 ON dob_t1 TO dob_role1,dob_role2 USING (true);
+DROP OWNED BY dob_role1;
+DROP POLICY p1 ON dob_t1; -- should succeed
+
+DROP USER dob_role1;
+DROP USER dob_role2;
 
 --
 -- Clean up objects