diff --git a/datasette/permissions.py b/datasette/permissions.py index b5e72b8e..b868d025 100644 --- a/datasette/permissions.py +++ b/datasette/permissions.py @@ -105,7 +105,7 @@ class Resource(ABC): @classmethod @abstractmethod - def resources_sql(cls) -> str: + def resources_sql(cls, datasette, actor=None) -> str: """ Return SQL query that returns all resources of this type. diff --git a/datasette/resources.py b/datasette/resources.py index 641afb2f..236b3598 100644 --- a/datasette/resources.py +++ b/datasette/resources.py @@ -13,7 +13,7 @@ class DatabaseResource(Resource): super().__init__(parent=database, child=None) @classmethod - async def resources_sql(cls, datasette) -> str: + async def resources_sql(cls, datasette, actor=None) -> str: return """ SELECT database_name AS parent, NULL AS child FROM catalog_databases @@ -30,7 +30,7 @@ class TableResource(Resource): super().__init__(parent=database, child=table) @classmethod - async def resources_sql(cls, datasette) -> str: + async def resources_sql(cls, datasette, actor=None) -> str: return """ SELECT database_name AS parent, table_name AS child FROM catalog_tables @@ -50,7 +50,7 @@ class QueryResource(Resource): super().__init__(parent=database, child=query) @classmethod - async def resources_sql(cls, datasette) -> str: + async def resources_sql(cls, datasette, actor=None) -> str: from datasette.plugins import pm from datasette.utils import await_me_maybe @@ -59,14 +59,16 @@ class QueryResource(Resource): result = await db.execute("SELECT database_name FROM catalog_databases") databases = [row[0] for row in result.rows] - # Gather all canned queries from all databases + # Gather canned queries for this actor from all databases. + # This keeps allowed_resources("view-query", actor=...) consistent with + # actor-specific canned_queries() implementations. query_pairs = [] for database_name in databases: # Call the hook to get queries (including from config via default plugin) for queries_result in pm.hook.canned_queries( datasette=datasette, database=database_name, - actor=None, # Get ALL queries for resource enumeration + actor=actor, ): queries = await await_me_maybe(queries_result) if queries: diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index 14383253..e679ae76 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -147,7 +147,9 @@ async def _build_single_action_sql( raise ValueError(f"Unknown action: {action}") # Get base resources SQL from the resource class - base_resources_sql = await action_obj.resource_class.resources_sql(datasette) + base_resources_sql = await action_obj.resource_class.resources_sql( + datasette, actor=actor + ) permission_sqls = await gather_permission_sql_from_hooks( datasette=datasette, diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 42a19ca4..be8969d7 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -943,6 +943,68 @@ async def test_permissions_in_config( perms_ds.config = previous_config +@pytest.mark.asyncio +async def test_allowed_resources_view_query_includes_actor_specific_canned_queries(): + """ + Actor-specific canned queries should be listed by allowed_resources("view-query"). + + This test is intentionally explicit about the previous bug: + - the canned query only exists for actor "alice" + - the permission rule only allows actor "alice" to view it + - allowed() succeeds for that specific query resource + - allowed_resources("view-query", actor) must include the same query + + Before the fix, QueryResource.resources_sql() called canned_queries(..., actor=None), + so the query was omitted from resource enumeration and allowed_resources() returned + an empty list even though allowed() returned True. + """ + from datasette import hookimpl + from datasette.permissions import PermissionSQL + from datasette.resources import QueryResource + + class ActorSpecificQueryPlugin: + __name__ = "ActorSpecificQueryPlugin" + + @hookimpl + def canned_queries(self, datasette, database, actor): + if database == "testdb" and actor and actor.get("id") == "alice": + return {"user_only": {"sql": "select 1 as n"}} + return {} + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + if action == "view-query" and actor and actor.get("id") == "alice": + return PermissionSQL(sql=""" + SELECT 'testdb' AS parent, 'user_only' AS child, 1 AS allow, + 'alice can view her actor-specific canned query' AS reason + """) + return None + + ds = Datasette(default_deny=True) + await ds.invoke_startup() + ds.add_memory_database("testdb") + await ds._refresh_schemas() + + plugin = ActorSpecificQueryPlugin() + ds.pm.register(plugin, name="actor_specific_query_plugin") + + try: + actor = {"id": "alice"} + + assert await ds.allowed( + action="view-query", + resource=QueryResource("testdb", "user_only"), + actor=actor, + ) + + page = await ds.allowed_resources("view-query", actor) + assert [(resource.parent, resource.child) for resource in page.resources] == [ + ("testdb", "user_only") + ] + finally: + ds.pm.unregister(name="actor_specific_query_plugin") + + @pytest.mark.asyncio async def test_actor_endpoint_allows_any_token(): ds = Datasette() diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 47d727f2..4ce2c7c0 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -1672,7 +1672,7 @@ async def test_hook_register_actions_with_custom_resources(): super().__init__(parent=collection, child=None) @classmethod - async def resources_sql(cls, datasette) -> str: + async def resources_sql(cls, datasette, actor=None) -> str: return """ SELECT 'collection1' AS parent, NULL AS child UNION ALL @@ -1689,7 +1689,7 @@ async def test_hook_register_actions_with_custom_resources(): super().__init__(parent=collection, child=document) @classmethod - async def resources_sql(cls, datasette) -> str: + async def resources_sql(cls, datasette, actor=None) -> str: return """ SELECT 'collection1' AS parent, 'doc1' AS child UNION ALL