From 10ea23a59c9221c439b8a3bd6c979c885954c14e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 25 Oct 2025 09:59:21 -0700 Subject: [PATCH] Add PermissionCheck dataclass with parent/child fields, refs #2528 Instead of logging permission checks as dicts with a 'resource' key, use a typed dataclass with separate parent and child fields. Changes: - Created PermissionCheck dataclass in app.py - Updated permission check logging to use dataclass - Updated PermissionsDebugView to use dataclass attributes - Updated PermissionCheckView to check parent/child instead of resource - Updated permissions_debug.html template to display parent/child - Updated test expectations to use dataclass attributes This provides better type safety and cleaner separation between parent and child resource identifiers. --- datasette/app.py | 37 +++++++++++----------- datasette/templates/permissions_debug.html | 10 ++++-- datasette/views/special.py | 17 ++++------ tests/test_permissions.py | 7 ++-- 4 files changed, 37 insertions(+), 34 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index cc388cb8..b355715a 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -120,6 +120,17 @@ from .resources import InstanceResource, DatabaseResource, TableResource app_root = Path(__file__).parent.parent + +@dataclasses.dataclass +class PermissionCheck: + """Represents a logged permission check for debugging purposes.""" + when: str + actor: Optional[Dict[str, Any]] + action: str + parent: Optional[str] + child: Optional[str] + result: bool + # https://github.com/simonw/datasette/issues/283#issuecomment-781591015 SQLITE_LIMIT_ATTACHED = 10 @@ -1287,25 +1298,15 @@ class Datasette: result = False # Log the permission check for debugging - # Convert Resource to old-style format for backward compatibility with debug tools - if resource.parent and resource.child: - old_style_resource = (resource.parent, resource.child) - elif resource.parent: - old_style_resource = resource.parent - else: - old_style_resource = None - self._permission_checks.append( - { - "when": datetime.datetime.now(datetime.timezone.utc).isoformat(), - "actor": actor, - "action": action, - "resource": old_style_resource, - "result": result, - "reason": None, # Not tracked in new system - "source_plugin": None, # Not tracked in new system - "depth": None, # Not tracked in new system - } + PermissionCheck( + when=datetime.datetime.now(datetime.timezone.utc).isoformat(), + actor=actor, + action=action, + parent=resource.parent, + child=resource.child, + result=result, + ) ) return result diff --git a/datasette/templates/permissions_debug.html b/datasette/templates/permissions_debug.html index cb3b4d71..5a84e8b7 100644 --- a/datasette/templates/permissions_debug.html +++ b/datasette/templates/permissions_debug.html @@ -133,8 +133,14 @@ debugPost.addEventListener('submit', function(ev) { {% endif %}

Actor: {{ check.actor|tojson }}

- {% if check.resource %} -

Resource: {{ check.resource }}

+ {% if check.parent %} +

Resource: + {% if check.child %} + {{ check.parent }} / {{ check.child }} + {% else %} + {{ check.parent }} + {% endif %} +

{% endif %} {% endfor %} diff --git a/datasette/views/special.py b/datasette/views/special.py index 6668de96..6f7a222b 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -120,13 +120,13 @@ class PermissionsDebugView(BaseView): permission_checks = [ check for check in permission_checks - if (check["actor"] or {}).get("id") != request.actor["id"] + if (check.actor or {}).get("id") != request.actor["id"] ] elif filter_ == "only-yours": permission_checks = [ check for check in permission_checks - if (check["actor"] or {}).get("id") == request.actor["id"] + if (check.actor or {}).get("id") == request.actor["id"] ] return await self.render( ["permissions_debug.html"], @@ -540,9 +540,10 @@ class PermissionCheckView(BaseView): if len(self.ds._permission_checks) > before_checks: for check in reversed(self.ds._permission_checks): if ( - check.get("actor") == request.actor - and check.get("action") == action - and check.get("resource") == resource + check.actor == request.actor + and check.action == action + and check.parent == parent + and check.child == child ): info = check break @@ -560,12 +561,6 @@ class PermissionCheckView(BaseView): if request.actor and "id" in request.actor: response["actor_id"] = request.actor["id"] - if info is not None: - response["depth"] = info.get("depth") - # Only include sensitive fields if user has permissions-debug - if has_debug_permission: - response["reason"] = info.get("reason") - return Response.json(response) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 7e21a1df..0bf28e2e 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -1259,9 +1259,10 @@ async def test_actor_restrictions( "response_status": response.status_code, "checks": [ { - "action": check["action"], - "resource": check["resource"], - "result": check["result"], + "action": check.action, + "parent": check.parent, + "child": check.child, + "result": check.result, } for check in perms_ds._permission_checks ],