Handle dependencies properly in ALTER POLICY
ALTER POLICY hadn't fully considered partial policy alternation (eg: change just the roles on the policy, or just change one of the expressions) when rebuilding the dependencies. Instead, it would happily remove all dependencies which existed for the policy and then only recreate the dependencies for the objects referred to in the specific ALTER POLICY command. Correct that by extracting and building the dependencies for all objects referenced by the policy, regardless of if they were provided as part of the ALTER POLICY command or were already in place as part of the pre-existing policy.
This commit is contained in:
Stephen Frost
parent
acfcd45cac
commit
ed8bec915e
|
|
@ -766,6 +766,35 @@ AlterPolicy(AlterPolicyStmt *stmt)
|
|||
replaces[Anum_pg_policy_polroles - 1] = true;
|
||||
values[Anum_pg_policy_polroles - 1] = PointerGetDatum(role_ids);
|
||||
}
|
||||
else
|
||||
{
|
||||
Oid *roles;
|
||||
Datum roles_datum;
|
||||
bool attr_isnull;
|
||||
ArrayType *policy_roles;
|
||||
|
||||
/*
|
||||
* We need to pull the set of roles this policy applies to from
|
||||
* what's in the catalog, so that we can recreate the dependencies
|
||||
* correctly for the policy.
|
||||
*/
|
||||
|
||||
roles_datum = heap_getattr(policy_tuple, Anum_pg_policy_polroles,
|
||||
RelationGetDescr(pg_policy_rel),
|
||||
&attr_isnull);
|
||||
Assert(!attr_isnull);
|
||||
|
||||
policy_roles = DatumGetArrayTypePCopy(roles_datum);
|
||||
|
||||
roles = (Oid *) ARR_DATA_PTR(policy_roles);
|
||||
|
||||
nitems = ARR_DIMS(policy_roles)[0];
|
||||
|
||||
role_oids = (Datum *) palloc(nitems * sizeof(Datum));
|
||||
|
||||
for (i = 0; i < nitems; i++)
|
||||
role_oids[i] = ObjectIdGetDatum(roles[i]);
|
||||
}
|
||||
|
||||
if (qual != NULL)
|
||||
{
|
||||
|
|
@ -773,6 +802,40 @@ AlterPolicy(AlterPolicyStmt *stmt)
|
|||
values[Anum_pg_policy_polqual - 1]
|
||||
= CStringGetTextDatum(nodeToString(qual));
|
||||
}
|
||||
else
|
||||
{
|
||||
Datum value_datum;
|
||||
bool attr_isnull;
|
||||
|
||||
/*
|
||||
* We need to pull the USING expression and build the range table for
|
||||
* the policy from what's in the catalog, so that we can recreate
|
||||
* the dependencies correctly for the policy.
|
||||
*/
|
||||
|
||||
/* Check if the policy has a USING expr */
|
||||
value_datum = heap_getattr(policy_tuple, Anum_pg_policy_polqual,
|
||||
RelationGetDescr(pg_policy_rel),
|
||||
&attr_isnull);
|
||||
if (!attr_isnull)
|
||||
{
|
||||
char *qual_value;
|
||||
ParseState *qual_pstate = make_parsestate(NULL);
|
||||
|
||||
/* parsestate is built just to build the range table */
|
||||
qual_pstate = make_parsestate(NULL);
|
||||
|
||||
qual_value = TextDatumGetCString(value_datum);
|
||||
qual = stringToNode(qual_value);
|
||||
|
||||
/* Add this rel to the parsestate's rangetable, for dependencies */
|
||||
addRangeTableEntryForRelation(qual_pstate, target_table, NULL,
|
||||
false, false);
|
||||
|
||||
qual_parse_rtable = qual_pstate->p_rtable;
|
||||
free_parsestate(qual_pstate);
|
||||
}
|
||||
}
|
||||
|
||||
if (with_check_qual != NULL)
|
||||
{
|
||||
|
|
@ -780,6 +843,40 @@ AlterPolicy(AlterPolicyStmt *stmt)
|
|||
values[Anum_pg_policy_polwithcheck - 1]
|
||||
= CStringGetTextDatum(nodeToString(with_check_qual));
|
||||
}
|
||||
else
|
||||
{
|
||||
Datum value_datum;
|
||||
bool attr_isnull;
|
||||
|
||||
/*
|
||||
* We need to pull the WITH CHECK expression and build the range table
|
||||
* for the policy from what's in the catalog, so that we can recreate
|
||||
* the dependencies correctly for the policy.
|
||||
*/
|
||||
|
||||
/* Check if the policy has a WITH CHECK expr */
|
||||
value_datum = heap_getattr(policy_tuple, Anum_pg_policy_polwithcheck,
|
||||
RelationGetDescr(pg_policy_rel),
|
||||
&attr_isnull);
|
||||
if (!attr_isnull)
|
||||
{
|
||||
char *with_check_value;
|
||||
ParseState *with_check_pstate = make_parsestate(NULL);
|
||||
|
||||
/* 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, target_table, NULL,
|
||||
false, false);
|
||||
|
||||
with_check_parse_rtable = with_check_pstate->p_rtable;
|
||||
free_parsestate(with_check_pstate);
|
||||
}
|
||||
}
|
||||
|
||||
new_tuple = heap_modify_tuple(policy_tuple,
|
||||
RelationGetDescr(pg_policy_rel),
|
||||
|
|
|
|||
|
|
@ -3246,6 +3246,49 @@ SET row_security = on;
|
|||
UPDATE r1 SET a = 30 RETURNING *;
|
||||
ERROR: new row violates row-level security policy for table "r1"
|
||||
DROP TABLE r1;
|
||||
-- Check dependency handling
|
||||
RESET SESSION AUTHORIZATION;
|
||||
CREATE TABLE dep1 (c1 int);
|
||||
CREATE TABLE dep2 (c1 int);
|
||||
CREATE POLICY dep_p1 ON dep1 TO rls_regress_user1 USING (c1 > (select max(dep2.c1) from dep2));
|
||||
ALTER POLICY dep_p1 ON dep1 TO rls_regress_user1,rls_regress_user2;
|
||||
-- Should return one
|
||||
SELECT count(*) = 1 FROM pg_depend
|
||||
WHERE objid = (SELECT oid FROM pg_policy WHERE polname = 'dep_p1')
|
||||
AND refobjid = (SELECT oid FROM pg_class WHERE relname = 'dep2');
|
||||
?column?
|
||||
----------
|
||||
t
|
||||
(1 row)
|
||||
|
||||
ALTER POLICY dep_p1 ON dep1 USING (true);
|
||||
-- Should return one
|
||||
SELECT count(*) = 1 FROM pg_shdepend
|
||||
WHERE objid = (SELECT oid FROM pg_policy WHERE polname = 'dep_p1')
|
||||
AND refobjid = (SELECT oid FROM pg_authid WHERE rolname = 'rls_regress_user1');
|
||||
?column?
|
||||
----------
|
||||
t
|
||||
(1 row)
|
||||
|
||||
-- Should return one
|
||||
SELECT count(*) = 1 FROM pg_shdepend
|
||||
WHERE objid = (SELECT oid FROM pg_policy WHERE polname = 'dep_p1')
|
||||
AND refobjid = (SELECT oid FROM pg_authid WHERE rolname = 'rls_regress_user2');
|
||||
?column?
|
||||
----------
|
||||
t
|
||||
(1 row)
|
||||
|
||||
-- Should return zero
|
||||
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');
|
||||
?column?
|
||||
----------
|
||||
t
|
||||
(1 row)
|
||||
|
||||
--
|
||||
-- Clean up objects
|
||||
--
|
||||
|
|
|
|||
|
|
@ -1490,6 +1490,37 @@ UPDATE r1 SET a = 30 RETURNING *;
|
|||
|
||||
DROP TABLE r1;
|
||||
|
||||
-- Check dependency handling
|
||||
RESET SESSION AUTHORIZATION;
|
||||
CREATE TABLE dep1 (c1 int);
|
||||
CREATE TABLE dep2 (c1 int);
|
||||
|
||||
CREATE POLICY dep_p1 ON dep1 TO rls_regress_user1 USING (c1 > (select max(dep2.c1) from dep2));
|
||||
ALTER POLICY dep_p1 ON dep1 TO rls_regress_user1,rls_regress_user2;
|
||||
|
||||
-- Should return one
|
||||
SELECT count(*) = 1 FROM pg_depend
|
||||
WHERE objid = (SELECT oid FROM pg_policy WHERE polname = 'dep_p1')
|
||||
AND refobjid = (SELECT oid FROM pg_class WHERE relname = 'dep2');
|
||||
|
||||
ALTER POLICY dep_p1 ON dep1 USING (true);
|
||||
|
||||
-- Should return one
|
||||
SELECT count(*) = 1 FROM pg_shdepend
|
||||
WHERE objid = (SELECT oid FROM pg_policy WHERE polname = 'dep_p1')
|
||||
AND refobjid = (SELECT oid FROM pg_authid WHERE rolname = 'rls_regress_user1');
|
||||
|
||||
-- Should return one
|
||||
SELECT count(*) = 1 FROM pg_shdepend
|
||||
WHERE objid = (SELECT oid FROM pg_policy WHERE polname = 'dep_p1')
|
||||
AND refobjid = (SELECT oid FROM pg_authid WHERE rolname = 'rls_regress_user2');
|
||||
|
||||
-- Should return zero
|
||||
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');
|
||||
|
||||
|
||||
--
|
||||
-- Clean up objects
|
||||
--
|
||||
|
|
|
|||
Loading…
Reference in New Issue