mirror of
https://github.com/simonw/datasette.git
synced 2026-05-31 14:16:59 +02:00
allowed_resources(view-query, actor) fix
Previously we could not filter for canned queries that a specific actor could view.
This commit is contained in:
parent
fd016f7986
commit
e800312b54
5 changed files with 75 additions and 9 deletions
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue