From 95286fbb60bc147e589f84e5974903b1f94b779d Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 26 Oct 2025 09:39:51 -0700 Subject: [PATCH] Refactor check_visibility() to use Resource objects, refs #2537 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated check_visibility() method signature to accept Resource objects (DatabaseResource, TableResource, QueryResource) instead of plain strings and tuples. Changes: - Updated check_visibility() signature to only accept Resource objects - Added validation with helpful error message for incorrect types - Updated all check_visibility() calls throughout the codebase: - datasette/views/database.py: Use DatabaseResource and QueryResource - datasette/views/special.py: Use DatabaseResource and TableResource - datasette/views/row.py: Use TableResource - datasette/views/table.py: Use TableResource - datasette/app.py: Use TableResource in expand_foreign_keys - Updated tests to use Resource objects - Updated documentation in docs/internals.rst: - Removed outdated permissions parameter - Updated examples to use Resource objects 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- datasette/app.py | 30 ++++++++++++++---------------- datasette/views/database.py | 6 +++--- datasette/views/row.py | 2 +- datasette/views/special.py | 6 ++++-- datasette/views/table.py | 2 +- docs/internals.rst | 30 ++++++++---------------------- tests/test_internals_datasette.py | 5 +++-- 7 files changed, 34 insertions(+), 47 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index a3037e3a..4c1c2bd2 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1072,7 +1072,7 @@ class Datasette: self, actor: dict, action: str, - resource: Optional[Union[str, Tuple[str, str]]] = None, + resource: Optional["Resource"] = None, ): """ Check if actor can see a resource and if it's private. @@ -1081,26 +1081,22 @@ class Datasette: - visible: bool - can the actor see it? - private: bool - if visible, can anonymous users NOT see it? """ - # Convert old-style resource to Resource object - if resource is None: - resource_obj = None - elif isinstance(resource, str): - # Database resource - resource_obj = self.resource_for_action(action, parent=resource, child=None) - elif isinstance(resource, tuple) and len(resource) == 2: - # Database + child resource (table or query) - resource_obj = self.resource_for_action( - action, parent=resource[0], child=resource[1] + from datasette.permissions import Resource + + # Validate that resource is a Resource object or None + if resource is not None and not isinstance(resource, Resource): + raise TypeError( + f"resource must be a Resource object or None, not {type(resource).__name__}. " + f"Use DatabaseResource(database=...), TableResource(database=..., table=...), " + f"or QueryResource(database=..., query=...) instead." ) - else: - resource_obj = None # Check if actor can see it - if not await self.allowed(action=action, resource=resource_obj, actor=actor): + if not await self.allowed(action=action, resource=resource, actor=actor): return False, False # Check if anonymous user can see it (for "private" flag) - if not await self.allowed(action=action, resource=resource_obj, actor=None): + if not await self.allowed(action=action, resource=resource, actor=None): # Actor can see it but anonymous cannot - it's private return True, True @@ -1386,12 +1382,14 @@ class Datasette: except IndexError: return {} # Ensure user has permission to view the referenced table + from datasette.resources import TableResource + other_table = fk["other_table"] other_column = fk["other_column"] visible, _ = await self.check_visibility( actor, action="view-table", - resource=(database, other_table), + resource=TableResource(database=database, table=other_table), ) if not visible: return {} diff --git a/datasette/views/database.py b/datasette/views/database.py index 9a5cbade..15eb271d 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -13,7 +13,7 @@ from typing import List from datasette.events import AlterTableEvent, CreateTableEvent, InsertRowsEvent from datasette.database import QueryInterrupted -from datasette.resources import DatabaseResource +from datasette.resources import DatabaseResource, QueryResource from datasette.utils import ( add_cors_headers, await_me_maybe, @@ -51,7 +51,7 @@ class DatabaseView(View): visible, private = await datasette.check_visibility( request.actor, action="view-database", - resource=database, + resource=DatabaseResource(database=database), ) if not visible: raise Forbidden("You do not have permission to view this database") @@ -541,7 +541,7 @@ class QueryView(View): visible, private = await datasette.check_visibility( request.actor, action="view-query", - resource=(database, canned_query["name"]), + resource=QueryResource(database=database, query=canned_query["name"]), ) if not visible: raise Forbidden("You do not have permission to view this query") diff --git a/datasette/views/row.py b/datasette/views/row.py index 87eed5a4..31d31bee 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -29,7 +29,7 @@ class RowView(DataView): visible, private = await self.ds.check_visibility( request.actor, action="view-table", - resource=(database, table), + resource=TableResource(database=database, table=table), ) if not visible: raise Forbidden("You do not have permission to view this table") diff --git a/datasette/views/special.py b/datasette/views/special.py index cf7bb1cb..27454a37 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -789,7 +789,9 @@ class ApiExplorerView(BaseView): if name == "_internal": continue database_visible, _ = await self.ds.check_visibility( - request.actor, action="view-database", resource=name + request.actor, + action="view-database", + resource=DatabaseResource(database=name), ) if not database_visible: continue @@ -799,7 +801,7 @@ class ApiExplorerView(BaseView): visible, _ = await self.ds.check_visibility( request.actor, action="view-table", - resource=(name, table), + resource=TableResource(database=name, table=table), ) if not visible: continue diff --git a/datasette/views/table.py b/datasette/views/table.py index 5efb9d36..3c7c976f 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -978,7 +978,7 @@ async def table_view_data( visible, private = await datasette.check_visibility( request.actor, action="view-table", - resource=(database_name, table_name), + resource=TableResource(database=database_name, table=table_name), ) if not visible: raise Forbidden("You do not have permission to view this table") diff --git a/docs/internals.rst b/docs/internals.rst index 03a33010..d71c3906 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -454,20 +454,17 @@ Example: .. _datasette_check_visibility: -await .check_visibility(actor, action=None, resource=None, permissions=None) ----------------------------------------------------------------------------- +await .check_visibility(actor, action, resource=None) +----------------------------------------------------- ``actor`` - dictionary The authenticated actor. This is usually ``request.actor``. -``action`` - string, optional +``action`` - string The name of the action that is being permission checked. -``resource`` - string or tuple, optional - The resource, e.g. the name of the database, or a tuple of two strings containing the name of the database and the name of the table. Only some permissions apply to a resource. - -``permissions`` - list of ``action`` strings or ``(action, resource)`` tuples, optional - Provide this instead of ``action`` and ``resource`` to check multiple permissions at once. +``resource`` - Resource object, optional + The resource being checked, as a Resource object such as ``DatabaseResource(database=...)``, ``TableResource(database=..., table=...)``, or ``QueryResource(database=..., query=...)``. Only some permissions apply to a resource. This convenience method can be used to answer the question "should this item be considered private, in that it is visible to me but it is not visible to anonymous users?" @@ -477,23 +474,12 @@ This example checks if the user can access a specific table, and sets ``private` .. code-block:: python + from datasette.resources import TableResource + visible, private = await datasette.check_visibility( request.actor, action="view-table", - resource=(database, table), - ) - -The following example runs three checks in a row. If any of the checks are denied before one of them is explicitly granted then ``visible`` will be ``False``. ``private`` will be ``True`` if an anonymous user would not be able to view the resource. - -.. code-block:: python - - visible, private = await datasette.check_visibility( - request.actor, - permissions=[ - ("view-table", (database, table)), - ("view-database", database), - "view-instance", - ], + resource=TableResource(database=database, table=table), ) .. _datasette_create_token: diff --git a/tests/test_internals_datasette.py b/tests/test_internals_datasette.py index 136385d7..629427b0 100644 --- a/tests/test_internals_datasette.py +++ b/tests/test_internals_datasette.py @@ -5,6 +5,7 @@ Tests for the datasette.app.Datasette class import dataclasses from datasette import Forbidden, Context from datasette.app import Datasette, Database +from datasette.resources import DatabaseResource from itsdangerous import BadSignature import pytest @@ -93,7 +94,7 @@ ALLOW_ROOT = {"allow": {"id": "root"}} None, {"databases": {"_memory": ALLOW_ROOT}}, "view-database", - "_memory", + DatabaseResource(database="_memory"), False, False, ), @@ -101,7 +102,7 @@ ALLOW_ROOT = {"allow": {"id": "root"}} ROOT, {"databases": {"_memory": ALLOW_ROOT}}, "view-database", - "_memory", + DatabaseResource(database="_memory"), True, True, ),