Ensure :actor, :actor_id and :action are all available to permissions SQL, closes #2520

- Updated build_rules_union() to accept actor as dict and provide :actor (JSON) and :actor_id
- Updated resolve_permissions_from_catalog() and resolve_permissions_with_candidates() to accept actor dict
- :actor is now the full actor dict as JSON (use json_extract() to access fields)
- :actor_id is the actor's id field for simple comparisons
- :action continues to be available as before
- Updated all call sites and tests to use new parameter format
- Added test demonstrating all three parameters working together
This commit is contained in:
Simon Willison 2025-10-23 09:48:55 -07:00
commit 65c427e4ee
3 changed files with 105 additions and 26 deletions

View file

@ -1,6 +1,7 @@
# perm_utils.py # perm_utils.py
from __future__ import annotations from __future__ import annotations
import json
from typing import Any, Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union from typing import Any, Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union
import sqlite3 import sqlite3
@ -36,17 +37,19 @@ PluginOrFactory = Union[PermissionSQL, PluginProvider]
def build_rules_union( def build_rules_union(
actor: str, plugins: Sequence[PermissionSQL] actor: Optional[dict], plugins: Sequence[PermissionSQL]
) -> Tuple[str, Dict[str, Any]]: ) -> Tuple[str, Dict[str, Any]]:
""" """
Compose plugin SQL into a UNION ALL with namespaced parameters. Compose plugin SQL into a UNION ALL with namespaced parameters.
Returns: Returns:
union_sql: a SELECT with columns (parent, child, allow, reason, source_plugin) union_sql: a SELECT with columns (parent, child, allow, reason, source_plugin)
params: dict of bound parameters including :actor and namespaced plugin params params: dict of bound parameters including :actor (JSON), :actor_id, and namespaced plugin params
""" """
parts: List[str] = [] parts: List[str] = []
params: Dict[str, Any] = {"actor": actor} actor_json = json.dumps(actor) if actor else None
actor_id = actor.get("id") if actor else None
params: Dict[str, Any] = {"actor": actor_json, "actor_id": actor_id}
for i, p in enumerate(plugins): for i, p in enumerate(plugins):
rewrite, ns_params = _namespace_params(i, p.params) rewrite, ns_params = _namespace_params(i, p.params)
@ -77,7 +80,7 @@ def build_rules_union(
async def resolve_permissions_from_catalog( async def resolve_permissions_from_catalog(
db, db,
actor: str, actor: Optional[dict],
plugins: Sequence[PluginOrFactory], plugins: Sequence[PluginOrFactory],
action: str, action: str,
candidate_sql: str, candidate_sql: str,
@ -95,6 +98,7 @@ async def resolve_permissions_from_catalog(
where rows is an iterable of sqlite3.Row where rows is an iterable of sqlite3.Row
- plugins are either PermissionSQL objects or callables accepting (action: str) - plugins are either PermissionSQL objects or callables accepting (action: str)
and returning PermissionSQL instances selecting (parent, child, allow, reason) and returning PermissionSQL instances selecting (parent, child, allow, reason)
- actor is the actor dict (or None), made available as :actor (JSON), :actor_id, and :action
Decision policy: Decision policy:
1) Specificity first: child (depth=2) > parent (depth=1) > root (depth=0) 1) Specificity first: child (depth=2) > parent (depth=1) > root (depth=0)
@ -121,7 +125,6 @@ async def resolve_permissions_from_catalog(
all_params = { all_params = {
**(candidate_params or {}), **(candidate_params or {}),
**rule_params, **rule_params,
"actor": actor,
"action": action, "action": action,
} }
@ -191,7 +194,7 @@ async def resolve_permissions_from_catalog(
async def resolve_permissions_with_candidates( async def resolve_permissions_with_candidates(
db, db,
actor: str, actor: Optional[dict],
plugins: Sequence[PluginOrFactory], plugins: Sequence[PluginOrFactory],
candidates: List[Tuple[str, Optional[str]]], candidates: List[Tuple[str, Optional[str]]],
action: str, action: str,
@ -203,6 +206,7 @@ async def resolve_permissions_with_candidates(
the candidates as a UNION of parameterized SELECTs in a CTE. the candidates as a UNION of parameterized SELECTs in a CTE.
candidates: list of (parent, child) where child can be None for parent-scoped actions. candidates: list of (parent, child) where child can be None for parent-scoped actions.
actor: actor dict (or None), made available as :actor (JSON), :actor_id, and :action
""" """
# Build a small CTE for candidates. # Build a small CTE for candidates.
cand_rows_sql: List[str] = [] cand_rows_sql: List[str] = []

View file

@ -312,10 +312,9 @@ class AllowedResourcesView(BaseView):
continue continue
plugins.append(candidate) plugins.append(candidate)
actor_id = actor.get("id") if actor else None
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, db,
actor=str(actor_id) if actor_id is not None else "", actor=actor,
plugins=plugins, plugins=plugins,
action=action, action=action,
candidate_sql=candidate_sql, candidate_sql=candidate_sql,

View file

@ -32,7 +32,7 @@ def plugin_allow_all_for_user(user: str) -> PluginProvider:
""" """
SELECT NULL AS parent, NULL AS child, 1 AS allow, SELECT NULL AS parent, NULL AS child, 1 AS allow,
'global allow for ' || :user || ' on ' || :action AS reason 'global allow for ' || :user || ' on ' || :action AS reason
WHERE :actor = :user WHERE :actor_id = :user
""", """,
{"user": user, "action": action}, {"user": user, "action": action},
) )
@ -47,7 +47,7 @@ def plugin_deny_specific_table(user: str, parent: str, child: str) -> PluginProv
""" """
SELECT :parent AS parent, :child AS child, 0 AS allow, SELECT :parent AS parent, :child AS child, 0 AS allow,
'deny ' || :parent || '/' || :child || ' for ' || :user || ' on ' || :action AS reason 'deny ' || :parent || '/' || :child || ' for ' || :user || ' on ' || :action AS reason
WHERE :actor = :user WHERE :actor_id = :user
""", """,
{"parent": parent, "child": child, "user": user, "action": action}, {"parent": parent, "child": child, "user": user, "action": action},
) )
@ -76,7 +76,7 @@ def plugin_allow_parent_for_user(user: str, parent: str) -> PluginProvider:
""" """
SELECT :parent AS parent, NULL AS child, 1 AS allow, SELECT :parent AS parent, NULL AS child, 1 AS allow,
'allow full parent for ' || :user || ' on ' || :action AS reason 'allow full parent for ' || :user || ' on ' || :action AS reason
WHERE :actor = :user WHERE :actor_id = :user
""", """,
{"parent": parent, "user": user, "action": action}, {"parent": parent, "user": user, "action": action},
) )
@ -91,7 +91,7 @@ def plugin_child_allow_for_user(user: str, parent: str, child: str) -> PluginPro
""" """
SELECT :parent AS parent, :child AS child, 1 AS allow, SELECT :parent AS parent, :child AS child, 1 AS allow,
'allow child for ' || :user || ' on ' || :action AS reason 'allow child for ' || :user || ' on ' || :action AS reason
WHERE :actor = :user WHERE :actor_id = :user
""", """,
{"parent": parent, "child": child, "user": user, "action": action}, {"parent": parent, "child": child, "user": user, "action": action},
) )
@ -121,7 +121,7 @@ def plugin_conflicting_same_child_rules(
""" """
SELECT :parent AS parent, :child AS child, 1 AS allow, SELECT :parent AS parent, :child AS child, 1 AS allow,
'team grant at child for ' || :user || ' on ' || :action AS reason 'team grant at child for ' || :user || ' on ' || :action AS reason
WHERE :actor = :user WHERE :actor_id = :user
""", """,
{"parent": parent, "child": child, "user": user, "action": action}, {"parent": parent, "child": child, "user": user, "action": action},
) )
@ -132,7 +132,7 @@ def plugin_conflicting_same_child_rules(
""" """
SELECT :parent AS parent, :child AS child, 0 AS allow, SELECT :parent AS parent, :child AS child, 0 AS allow,
'exception deny at child for ' || :user || ' on ' || :action AS reason 'exception deny at child for ' || :user || ' on ' || :action AS reason
WHERE :actor = :user WHERE :actor_id = :user
""", """,
{"parent": parent, "child": child, "user": user, "action": action}, {"parent": parent, "child": child, "user": user, "action": action},
) )
@ -153,7 +153,7 @@ def plugin_allow_all_for_action(user: str, allowed_action: str) -> PluginProvide
""" """
SELECT NULL AS parent, NULL AS child, 1 AS allow, SELECT NULL AS parent, NULL AS child, 1 AS allow,
'global allow for ' || :user || ' on ' || :action AS reason 'global allow for ' || :user || ' on ' || :action AS reason
WHERE :actor = :user WHERE :actor_id = :user
""", """,
{"user": user, "action": action}, {"user": user, "action": action},
) )
@ -247,7 +247,12 @@ async def test_alice_global_allow_with_specific_denies_catalog(db):
plugin_org_policy_deny_parent("hr"), plugin_org_policy_deny_parent("hr"),
] ]
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, "alice", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True db,
{"id": "alice"},
plugins,
VIEW_TABLE,
TABLE_CANDIDATES_SQL,
implicit_deny=True,
) )
# Alice can see everything except accounting/sales and hr/* # Alice can see everything except accounting/sales and hr/*
assert "/accounting/sales" in res_denied(rows) assert "/accounting/sales" in res_denied(rows)
@ -269,7 +274,12 @@ async def test_carol_parent_allow_but_child_conflict_deny_wins_catalog(db):
*plugin_conflicting_same_child_rules("carol", "analytics", "secret"), *plugin_conflicting_same_child_rules("carol", "analytics", "secret"),
] ]
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, "carol", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True db,
{"id": "carol"},
plugins,
VIEW_TABLE,
TABLE_CANDIDATES_SQL,
implicit_deny=True,
) )
allowed_analytics = res_allowed(rows, parent="analytics") allowed_analytics = res_allowed(rows, parent="analytics")
denied_analytics = res_denied(rows, parent="analytics") denied_analytics = res_denied(rows, parent="analytics")
@ -290,7 +300,12 @@ async def test_specificity_child_allow_overrides_parent_deny_catalog(db):
), # child allow beats parent deny ), # child allow beats parent deny
] ]
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, "alice", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True db,
{"id": "alice"},
plugins,
VIEW_TABLE,
TABLE_CANDIDATES_SQL,
implicit_deny=True,
) )
# table02 allowed, other analytics tables denied # table02 allowed, other analytics tables denied
@ -311,7 +326,7 @@ async def test_root_deny_all_but_parent_allow_rescues_specific_parent_catalog(db
), # parent allow (more specific) ), # parent allow (more specific)
] ]
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, "bob", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True db, {"id": "bob"}, plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True
) )
for r in rows: for r in rows:
if r["parent"] == "accounting": if r["parent"] == "accounting":
@ -328,7 +343,12 @@ async def test_parent_scoped_candidates(db):
plugin_allow_parent_for_user("carol", "analytics"), plugin_allow_parent_for_user("carol", "analytics"),
] ]
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, "carol", plugins, VIEW_TABLE, PARENT_CANDIDATES_SQL, implicit_deny=True db,
{"id": "carol"},
plugins,
VIEW_TABLE,
PARENT_CANDIDATES_SQL,
implicit_deny=True,
) )
d = {r["resource"]: r["allow"] for r in rows} d = {r["resource"]: r["allow"] for r in rows}
assert d["/analytics"] == 1 assert d["/analytics"] == 1
@ -342,13 +362,23 @@ async def test_implicit_deny_behavior(db):
# implicit_deny=True -> everything denied with reason 'implicit deny' # implicit_deny=True -> everything denied with reason 'implicit deny'
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, "erin", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True db,
{"id": "erin"},
plugins,
VIEW_TABLE,
TABLE_CANDIDATES_SQL,
implicit_deny=True,
) )
assert all(r["allow"] == 0 and r["reason"] == "implicit deny" for r in rows) assert all(r["allow"] == 0 and r["reason"] == "implicit deny" for r in rows)
# implicit_deny=False -> no winner => allow is None, reason is None # implicit_deny=False -> no winner => allow is None, reason is None
rows2 = await resolve_permissions_from_catalog( rows2 = await resolve_permissions_from_catalog(
db, "erin", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=False db,
{"id": "erin"},
plugins,
VIEW_TABLE,
TABLE_CANDIDATES_SQL,
implicit_deny=False,
) )
assert all(r["allow"] is None and r["reason"] is None for r in rows2) assert all(r["allow"] is None and r["reason"] is None for r in rows2)
@ -384,7 +414,7 @@ async def test_candidate_filters_via_params(db):
# Case 1: exclude memory dbs, require schema_version >= 2 -> only analytics appear, and thus are allowed # Case 1: exclude memory dbs, require schema_version >= 2 -> only analytics appear, and thus are allowed
rows = await resolve_permissions_from_catalog( rows = await resolve_permissions_from_catalog(
db, db,
"dev", {"id": "dev"},
plugins, plugins,
VIEW_TABLE, VIEW_TABLE,
candidate_sql, candidate_sql,
@ -398,7 +428,7 @@ async def test_candidate_filters_via_params(db):
# but root deny wins except where specifically allowed (none except analytics parent allow doesnt apply to table depth if candidate includes children; still fine—policy is explicit). # but root deny wins except where specifically allowed (none except analytics parent allow doesnt apply to table depth if candidate includes children; still fine—policy is explicit).
rows2 = await resolve_permissions_from_catalog( rows2 = await resolve_permissions_from_catalog(
db, db,
"dev", {"id": "dev"},
plugins, plugins,
VIEW_TABLE, VIEW_TABLE,
candidate_sql, candidate_sql,
@ -418,7 +448,7 @@ async def test_action_specific_rules(db):
view_rows = await resolve_permissions_from_catalog( view_rows = await resolve_permissions_from_catalog(
db, db,
"dana", {"id": "dana"},
plugins, plugins,
VIEW_TABLE, VIEW_TABLE,
TABLE_CANDIDATES_SQL, TABLE_CANDIDATES_SQL,
@ -429,7 +459,7 @@ async def test_action_specific_rules(db):
insert_rows = await resolve_permissions_from_catalog( insert_rows = await resolve_permissions_from_catalog(
db, db,
"dana", {"id": "dana"},
plugins, plugins,
"insert-row", "insert-row",
TABLE_CANDIDATES_SQL, TABLE_CANDIDATES_SQL,
@ -438,3 +468,49 @@ async def test_action_specific_rules(db):
assert insert_rows and all(r["allow"] == 0 for r in insert_rows) assert insert_rows and all(r["allow"] == 0 for r in insert_rows)
assert all(r["reason"] == "implicit deny" for r in insert_rows) assert all(r["reason"] == "implicit deny" for r in insert_rows)
assert all(r["action"] == "insert-row" for r in insert_rows) assert all(r["action"] == "insert-row" for r in insert_rows)
@pytest.mark.asyncio
async def test_actor_actor_id_action_parameters_available(db):
"""Test that :actor (JSON), :actor_id, and :action are all available in SQL"""
await seed_catalog(db)
def plugin_using_all_parameters() -> PluginProvider:
def provider(action: str) -> PermissionSQL:
return PermissionSQL(
"test_all_params",
"""
SELECT NULL AS parent, NULL AS child, 1 AS allow,
'Actor ID: ' || COALESCE(:actor_id, 'null') ||
', Actor JSON: ' || COALESCE(:actor, 'null') ||
', Action: ' || :action AS reason
WHERE :actor_id = 'test_user' AND :action = 'view-table'
AND json_extract(:actor, '$.role') = 'admin'
""",
{},
)
return provider
plugins = [plugin_using_all_parameters()]
# Test with full actor dict
rows = await resolve_permissions_from_catalog(
db,
{"id": "test_user", "role": "admin"},
plugins,
"view-table",
TABLE_CANDIDATES_SQL,
implicit_deny=True,
)
# Should have allowed rows with reason containing all the info
allowed = [r for r in rows if r["allow"] == 1]
assert len(allowed) > 0
# Check that the reason string contains evidence of all parameters
reason = allowed[0]["reason"]
assert "test_user" in reason
assert "view-table" in reason
# The :actor parameter should be the JSON string
assert "Actor JSON:" in reason