Fix permission endpoint tests by resolving method signature conflicts

- Renamed internal allowed_resources_sql() to _build_permission_rules_sql()
  to avoid conflict with public method
- Made public allowed_resources_sql() keyword-only to prevent argument order bugs
- Fixed PermissionRulesView to use _build_permission_rules_sql() which returns
  full permission rules (with allow/deny) instead of filtered resources
- Fixed _build_permission_rules_sql() to pass actor dict to build_rules_union()
- Added actor_id extraction in AllowedResourcesView
- Added root_enabled=True to test fixture to grant permissions-debug to root user

All 51 tests in test_permission_endpoints.py now pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Simon Willison 2025-10-23 14:53:07 -07:00
commit 4d93149c2b
3 changed files with 11 additions and 8 deletions

View file

@ -1066,12 +1066,13 @@ class Datasette:
)
return result
async def allowed_resources_sql(
async def _build_permission_rules_sql(
self, actor: dict | None, action: str
) -> tuple[str, dict]:
"""Combine permission_resources_sql PermissionSQL blocks into a UNION query.
Returns a (sql, params) tuple suitable for execution against SQLite.
Internal helper for permission_allowed_2.
"""
plugin_blocks: List[PermissionSQL] = []
for block in pm.hook.permission_resources_sql(
@ -1093,9 +1094,8 @@ class Datasette:
continue
plugin_blocks.append(candidate)
actor_id = actor.get("id") if actor else None
sql, params = build_rules_union(
actor=str(actor_id) if actor_id is not None else "",
actor=actor,
plugins=plugin_blocks,
)
return sql, params
@ -1123,7 +1123,7 @@ class Datasette:
elif resource is not None:
raise TypeError("resource must be None, str, or (parent, child) tuple")
union_sql, union_params = await self.allowed_resources_sql(actor_dict, action)
union_sql, union_params = await self._build_permission_rules_sql(actor_dict, action)
query = f"""
WITH rules AS (
@ -1275,6 +1275,7 @@ class Datasette:
async def allowed_resources_sql(
self,
*,
action: str,
actor: dict | None = None,
) -> tuple[str, dict]:
@ -1285,7 +1286,7 @@ class Datasette:
The query returns rows with (parent, child, reason) columns.
Example:
query, params = await datasette.allowed_resources_sql("view-table", actor)
query, params = await datasette.allowed_resources_sql(action="view-table", actor=actor)
result = await datasette.get_internal_database().execute(query, params)
"""
from datasette.utils.actions_sql import build_allowed_resources_sql
@ -1318,7 +1319,7 @@ class Datasette:
if not action_obj:
raise ValueError(f"Unknown action: {action}")
query, params = await self.allowed_resources_sql(action, actor)
query, params = await self.allowed_resources_sql(action=action, actor=actor)
result = await self.get_internal_database().execute(query, params)
# Instantiate the appropriate Resource subclass for each row
@ -1355,7 +1356,7 @@ class Datasette:
if not action_obj:
raise ValueError(f"Unknown action: {action}")
query, params = await self.allowed_resources_sql(action, actor)
query, params = await self.allowed_resources_sql(action=action, actor=actor)
result = await self.get_internal_database().execute(query, params)
resource_class = action_obj.resource_class

View file

@ -238,6 +238,7 @@ class AllowedResourcesView(BaseView):
)
actor = request.actor if isinstance(request.actor, dict) else None
actor_id = actor.get("id") if actor else None
parent_filter = request.args.get("parent")
child_filter = request.args.get("child")
if child_filter and not parent_filter:
@ -424,7 +425,7 @@ class PermissionRulesView(BaseView):
page_size = max_page_size
offset = (page - 1) * page_size
union_sql, union_params = await self.ds.allowed_resources_sql(actor, action)
union_sql, union_params = await self.ds._build_permission_rules_sql(actor, action)
await self.ds.refresh_schemas()
db = self.ds.get_internal_database()

View file

@ -30,6 +30,7 @@ async def ds_with_permissions():
}
}
)
ds.root_enabled = True
await ds.invoke_startup()
# Add some test databases
ds.add_memory_database("content")