Move takes_child/takes_parent information from Action to Resource (#2567)

Simplified Action by moving takes_child/takes_parent logic to Resource

- Removed InstanceResource - global actions are now simply those with resource_class=None
- Resource.parent_class - Replaced parent_name: str with parent_class: type[Resource] | None for direct class references
- Simplified Action dataclass - No more redundant fields, everything is derived from the Resource class structure
- Validation - The __init_subclass__ method now checks parent_class.parent_class to enforce the 2-level hierarchy

Closes #2563
This commit is contained in:
Simon Willison 2025-11-01 11:35:08 -07:00 committed by GitHub
commit 5705ce0d95
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 417 additions and 186 deletions

View file

@ -1308,7 +1308,7 @@ class Datasette:
Uses SQL to check permission for a single resource without fetching all resources.
This is efficient - it does NOT call allowed_resources() and check membership.
If resource is not provided, defaults to InstanceResource() for instance-level actions.
For global actions, resource should be None (or omitted).
Example:
from datasette.resources import TableResource
@ -1318,14 +1318,12 @@ class Datasette:
actor=actor
)
# For instance-level actions, resource can be omitted:
# For global actions, resource can be omitted:
can_debug = await datasette.allowed(action="permissions-debug", actor=actor)
"""
from datasette.utils.actions_sql import check_permission_for_resource
from datasette.resources import InstanceResource
if resource is None:
resource = InstanceResource()
# For global actions, resource remains None
# Check if this action has also_requires - if so, check that action first
action_obj = self.actions.get(action)
@ -1338,12 +1336,16 @@ class Datasette:
):
return False
# For global actions, resource is None
parent = resource.parent if resource else None
child = resource.child if resource else None
result = await check_permission_for_resource(
datasette=self,
actor=actor,
action=action,
parent=resource.parent,
child=resource.child,
parent=parent,
child=child,
)
# Log the permission check for debugging
@ -1352,8 +1354,8 @@ class Datasette:
when=datetime.datetime.now(datetime.timezone.utc).isoformat(),
actor=actor,
action=action,
parent=resource.parent,
child=resource.child,
parent=parent,
child=child,
result=result,
)
)
@ -1607,7 +1609,9 @@ class Datasette:
"description": action.description,
"takes_parent": action.takes_parent,
"takes_child": action.takes_child,
"resource_class": action.resource_class.__name__,
"resource_class": (
action.resource_class.__name__ if action.resource_class else None
),
"also_requires": action.also_requires,
}
for action in sorted(self.actions.values(), key=lambda a: a.name)

View file

@ -1,7 +1,6 @@
from datasette import hookimpl
from datasette.permissions import Action
from datasette.resources import (
InstanceResource,
DatabaseResource,
TableResource,
QueryResource,
@ -12,122 +11,91 @@ from datasette.resources import (
def register_actions():
"""Register the core Datasette actions."""
return (
# View actions
# Global actions (no resource_class)
Action(
name="view-instance",
abbr="vi",
description="View Datasette instance",
takes_parent=False,
takes_child=False,
resource_class=InstanceResource,
),
Action(
name="permissions-debug",
abbr="pd",
description="Access permission debug tool",
),
Action(
name="debug-menu",
abbr="dm",
description="View debug menu items",
),
# Database-level actions (parent-level)
Action(
name="view-database",
abbr="vd",
description="View database",
takes_parent=True,
takes_child=False,
resource_class=DatabaseResource,
),
Action(
name="view-database-download",
abbr="vdd",
description="Download database file",
takes_parent=True,
takes_child=False,
resource_class=DatabaseResource,
also_requires="view-database",
),
Action(
name="view-table",
abbr="vt",
description="View table",
takes_parent=True,
takes_child=True,
resource_class=TableResource,
),
Action(
name="view-query",
abbr="vq",
description="View named query results",
takes_parent=True,
takes_child=True,
resource_class=QueryResource,
),
Action(
name="execute-sql",
abbr="es",
description="Execute read-only SQL queries",
takes_parent=True,
takes_child=False,
resource_class=DatabaseResource,
also_requires="view-database",
),
# Debug actions
Action(
name="permissions-debug",
abbr="pd",
description="Access permission debug tool",
takes_parent=False,
takes_child=False,
resource_class=InstanceResource,
name="create-table",
abbr="ct",
description="Create tables",
resource_class=DatabaseResource,
),
# Table-level actions (child-level)
Action(
name="debug-menu",
abbr="dm",
description="View debug menu items",
takes_parent=False,
takes_child=False,
resource_class=InstanceResource,
name="view-table",
abbr="vt",
description="View table",
resource_class=TableResource,
),
# Write actions on tables
Action(
name="insert-row",
abbr="ir",
description="Insert rows",
takes_parent=True,
takes_child=True,
resource_class=TableResource,
),
Action(
name="delete-row",
abbr="dr",
description="Delete rows",
takes_parent=True,
takes_child=True,
resource_class=TableResource,
),
Action(
name="update-row",
abbr="ur",
description="Update rows",
takes_parent=True,
takes_child=True,
resource_class=TableResource,
),
Action(
name="alter-table",
abbr="at",
description="Alter tables",
takes_parent=True,
takes_child=True,
resource_class=TableResource,
),
Action(
name="drop-table",
abbr="dt",
description="Drop tables",
takes_parent=True,
takes_child=True,
resource_class=TableResource,
),
# Schema actions on databases
# Query-level actions (child-level)
Action(
name="create-table",
abbr="ct",
description="Create tables",
takes_parent=True,
takes_child=False,
resource_class=DatabaseResource,
name="view-query",
abbr="vq",
description="View named query results",
resource_class=QueryResource,
),
)

View file

@ -14,7 +14,7 @@ class Resource(ABC):
# Class-level metadata (subclasses must define these)
name: str = None # e.g., "table", "database", "model"
parent_name: str | None = None # e.g., "database" for tables
parent_class: type["Resource"] | None = None # e.g., DatabaseResource for tables
# Instance-level optional extra attributes
reasons: list[str] | None = None
@ -54,6 +54,29 @@ class Resource(ABC):
def private(self, value: bool):
self._private = value
@classmethod
def __init_subclass__(cls):
"""
Validate resource hierarchy doesn't exceed 2 levels.
Raises:
ValueError: If this resource would create a 3-level hierarchy
"""
super().__init_subclass__()
if cls.parent_class is None:
return # Top of hierarchy, nothing to validate
# Check if our parent has a parent - that would create 3 levels
if cls.parent_class.parent_class is not None:
# We have a parent, and that parent has a parent
# This creates a 3-level hierarchy, which is not allowed
raise ValueError(
f"Resource {cls.__name__} creates a 3-level hierarchy: "
f"{cls.parent_class.parent_class.__name__} -> {cls.parent_class.__name__} -> {cls.__name__}. "
f"Maximum 2 levels allowed (parent -> child)."
)
@classmethod
@abstractmethod
def resources_sql(cls) -> str:
@ -77,11 +100,32 @@ class Action:
name: str
abbr: str | None
description: str | None
takes_parent: bool
takes_child: bool
resource_class: type[Resource]
resource_class: type[Resource] | None = None
also_requires: str | None = None # Optional action name that must also be allowed
@property
def takes_parent(self) -> bool:
"""
Whether this action requires a parent identifier when instantiating its resource.
Returns False for global-only actions (no resource_class).
Returns True for all actions with a resource_class (all resources require a parent identifier).
"""
return self.resource_class is not None
@property
def takes_child(self) -> bool:
"""
Whether this action requires a child identifier when instantiating its resource.
Returns False for global actions (no resource_class).
Returns False for parent-level resources (DatabaseResource - parent_class is None).
Returns True for child-level resources (TableResource, QueryResource - have a parent_class).
"""
if self.resource_class is None:
return False
return self.resource_class.parent_class is not None
_reason_id = 1

View file

@ -3,25 +3,11 @@
from datasette.permissions import Resource
class InstanceResource(Resource):
"""The Datasette instance itself."""
name = "instance"
parent_name = None
def __init__(self):
super().__init__(parent=None, child=None)
@classmethod
async def resources_sql(cls, datasette) -> str:
return "SELECT NULL AS parent, NULL AS child"
class DatabaseResource(Resource):
"""A database in Datasette."""
name = "database"
parent_name = "instance"
parent_class = None # Top of the resource hierarchy
def __init__(self, database: str):
super().__init__(parent=database, child=None)
@ -38,7 +24,7 @@ class TableResource(Resource):
"""A table in a database."""
name = "table"
parent_name = "database"
parent_class = DatabaseResource
def __init__(self, database: str, table: str):
super().__init__(parent=database, child=table)
@ -58,7 +44,7 @@ class QueryResource(Resource):
"""A canned query in a database."""
name = "query"
parent_name = "database"
parent_class = DatabaseResource
def __init__(self, database: str, query: str):
super().__init__(parent=database, child=query)

View file

@ -1,7 +1,7 @@
import json
import logging
from datasette.events import LogoutEvent, LoginEvent, CreateTokenEvent
from datasette.resources import DatabaseResource, TableResource, InstanceResource
from datasette.resources import DatabaseResource, TableResource
from datasette.utils.asgi import Response, Forbidden
from datasette.utils import (
actor_matches_allow,
@ -491,12 +491,18 @@ async def _check_permission_for_actor(ds, action, parent, child, actor):
if not action_obj:
return {"error": f"Unknown action: {action}"}, 400
if action_obj.takes_parent and action_obj.takes_child:
# Global actions (no resource_class) don't have a resource
if action_obj.resource_class is None:
resource_obj = None
elif action_obj.takes_parent and action_obj.takes_child:
# Child-level resource (e.g., TableResource, QueryResource)
resource_obj = action_obj.resource_class(database=parent, table=child)
elif action_obj.takes_parent:
# Parent-level resource (e.g., DatabaseResource)
resource_obj = action_obj.resource_class(database=parent)
else:
resource_obj = action_obj.resource_class()
# This shouldn't happen given validation in Action.__post_init__
return {"error": f"Invalid action configuration: {action}"}, 500
allowed = await ds.allowed(action=action, resource=resource_obj, actor=actor)