Skip to content

Commit

Permalink
Fix DELETE/UPDATE on parent types with children that have no policies (
Browse files Browse the repository at this point in the history
…#6136)

Previously we would spuriously ignore the policies on those children,
as the result of an incorrect mechanism being used to detect when a
type has policies when compiling DML.

Just check in the obvious way.

Fixes #6133
  • Loading branch information
msullivan committed Sep 21, 2023
1 parent 567e14e commit 652d06e
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 6 deletions.
14 changes: 8 additions & 6 deletions edb/edgeql/compiler/policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ def get_access_policies(
stype: s_objtypes.ObjectType, *, ctx: context.ContextLevel,
) -> Tuple[s_policies.AccessPolicy, ...]:
schema = ctx.env.schema
if not ctx.env.options.apply_query_rewrites:
return ()

# The apply_access_policies config flag disables user-specified
# access polices, but not stdlib ones
if (
Expand Down Expand Up @@ -380,7 +383,8 @@ def compile_dml_write_policies(
ctx: context.ContextLevel,
) -> Optional[irast.WritePolicies]:
"""Compile policy filters and wrap them into irast.WritePolicies"""
if not ctx.env.type_rewrites.get((stype, False)):
pols = get_access_policies(stype, ctx=ctx)
if not pols:
return None

with ctx.detached() as _, _.newscope(fenced=True) as subctx:
Expand All @@ -391,10 +395,6 @@ def compile_dml_write_policies(
schema = subctx.env.schema
subctx.anchors = subctx.anchors.copy()

pols = get_access_policies(stype, ctx=ctx)
if not pols:
return None

policies = []
for pol in pols:
if mode not in pol.get_access_kinds(schema):
Expand Down Expand Up @@ -425,7 +425,7 @@ def compile_dml_read_policies(
ctx: context.ContextLevel,
) -> Optional[irast.ReadPolicyExpr]:
"""Compile a policy filter for a DML statement at a particular type"""
if not ctx.env.type_rewrites.get((stype, False)):
if not get_access_policies(stype, ctx=ctx):
return None

with ctx.detached() as _, _.newscope(fenced=True) as subctx:
Expand All @@ -450,6 +450,8 @@ def _prepare_dml_policy_context(
*,
ctx: context.ContextLevel,
) -> None:
# It doesn't matter whether we skip subtypes here, so don't skip
# subtypes if it has already been compiled that way, otherwise do.
skip_subtypes = (stype, False) not in ctx.env.type_rewrites
result = setgen.class_set(
stype, path_id=result.path_id, skip_subtypes=skip_subtypes, ctx=ctx
Expand Down
40 changes: 40 additions & 0 deletions tests/test_edgeql_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -1247,3 +1247,43 @@ async def test_edgeql_policies_optional_leakage_01(self):
r'''select Src''',
[],
)

async def test_edgeql_policies_parent_update_01(self):
await self.con.execute('''
CREATE ABSTRACT TYPE Base {
CREATE PROPERTY name: std::str;
CREATE ACCESS POLICY sel_ins
ALLOW SELECT, INSERT USING (true);
};
CREATE TYPE Child EXTENDING Base;
INSERT Child;
''')

await self.assert_query_result(
'''
update Base set { name := '!!!' }
''',
[],
)

await self.assert_query_result(
'''
delete Base
''',
[],
)

await self.con.execute('''
ALTER TYPE Base {
CREATE ACCESS POLICY upd_read
ALLOW UPDATE READ USING (true);
};
''')

async with self.assertRaisesRegexTx(
edgedb.InvalidValueError,
r"access policy violation on update"):
await self.con.query('''
update Base set { name := '!!!' }
''')

0 comments on commit 652d06e

Please sign in to comment.