From b9c6e7a0f6a9288261f13f9923e42603fe59d739 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 23 Oct 2025 09:34:19 -0700 Subject: [PATCH] PluginSQL renamed to PermissionSQL, closes #2524 --- datasette/app.py | 9 +++---- datasette/default_permissions.py | 10 ++++---- datasette/hookspecs.py | 4 ++-- datasette/permissions.py | 17 +++++++++++++- datasette/utils/actions_sql.py | 10 ++++---- datasette/utils/permissions.py | 36 +++++++++------------------- datasette/views/special.py | 7 +++--- tests/test_actions_sql.py | 14 +++++------ tests/test_plugins.py | 4 ++-- tests/test_tables_endpoint.py | 14 +++++------ tests/test_utils_permissions.py | 40 ++++++++++++++++---------------- 11 files changed, 84 insertions(+), 81 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 90030e2c..241bec2f 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -115,7 +115,8 @@ from .tracer import AsgiTracer from .plugins import pm, DEFAULT_PLUGINS, get_plugins from .version import __version__ -from .utils.permissions import build_rules_union, PluginSQL +from .permissions import PermissionSQL +from .utils.permissions import build_rules_union app_root = Path(__file__).parent.parent @@ -1067,11 +1068,11 @@ class Datasette: async def allowed_resources_sql( self, actor: dict | None, action: str ) -> tuple[str, dict]: - """Combine permission_resources_sql PluginSQL blocks into a UNION query. + """Combine permission_resources_sql PermissionSQL blocks into a UNION query. Returns a (sql, params) tuple suitable for execution against SQLite. """ - plugin_blocks: List[PluginSQL] = [] + plugin_blocks: List[PermissionSQL] = [] for block in pm.hook.permission_resources_sql( datasette=self, actor=actor, @@ -1087,7 +1088,7 @@ class Datasette: for candidate in candidates: if candidate is None: continue - if not isinstance(candidate, PluginSQL): + if not isinstance(candidate, PermissionSQL): continue plugin_blocks.append(candidate) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 25bc9590..d8efffa4 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -1,5 +1,5 @@ from datasette import hookimpl, Permission -from datasette.utils.permissions import PluginSQL +from datasette.permissions import PermissionSQL from datasette.utils import actor_matches_allow import itsdangerous import time @@ -174,7 +174,7 @@ def permission_allowed_default(datasette, actor, action, resource): @hookimpl async def permission_resources_sql(datasette, actor, action): - rules: list[PluginSQL] = [] + rules: list[PermissionSQL] = [] config_rules = await _config_permission_rules(datasette, actor, action) rules.extend(config_rules) @@ -191,7 +191,7 @@ async def permission_resources_sql(datasette, actor, action): "SELECT NULL AS parent, NULL AS child, 1 AS allow, " f"'{reason}' AS reason" ) rules.append( - PluginSQL( + PermissionSQL( source="default_permissions", sql=sql, params={}, @@ -205,7 +205,7 @@ async def permission_resources_sql(datasette, actor, action): return rules -async def _config_permission_rules(datasette, actor, action) -> list[PluginSQL]: +async def _config_permission_rules(datasette, actor, action) -> list[PermissionSQL]: config = datasette.config or {} if actor is None: @@ -332,7 +332,7 @@ async def _config_permission_rules(datasette, actor, action) -> list[PluginSQL]: params[f"{key}_reason"] = reason sql = "\nUNION ALL\n".join(parts) - return [PluginSQL(source="config_permissions", sql=sql, params=params)] + return [PermissionSQL(source="config_permissions", sql=sql, params=params)] async def _resolve_config_permissions_blocks(datasette, actor, action, resource): diff --git a/datasette/hookspecs.py b/datasette/hookspecs.py index 35c4062d..5477a407 100644 --- a/datasette/hookspecs.py +++ b/datasette/hookspecs.py @@ -124,8 +124,8 @@ def permission_allowed(datasette, actor, action, resource): def permission_resources_sql(datasette, actor, action): """Return SQL query fragments for permission checks on resources. - Returns None, a PluginSQL object, or a list of PluginSQL objects. - Each PluginSQL contains SQL that should return rows with columns: + Returns None, a PermissionSQL object, or a list of PermissionSQL objects. + Each PermissionSQL contains SQL that should return rows with columns: parent (str|None), child (str|None), allow (int), reason (str). Used to efficiently check permissions across multiple resources at once. diff --git a/datasette/permissions.py b/datasette/permissions.py index f83780e6..df30c8ce 100644 --- a/datasette/permissions.py +++ b/datasette/permissions.py @@ -1,6 +1,6 @@ from abc import ABC, abstractmethod from dataclasses import dataclass -from typing import Optional, NamedTuple +from typing import Any, Dict, Optional, NamedTuple class Resource(ABC): @@ -86,6 +86,21 @@ class Action: resource_class: type[Resource] +@dataclass +class PermissionSQL: + """ + A plugin contributes SQL that yields: + parent TEXT NULL, + child TEXT NULL, + allow INTEGER, -- 1 allow, 0 deny + reason TEXT + """ + + source: str # identifier used for auditing (e.g., plugin name) + sql: str # SQL that SELECTs the 4 columns above + params: Dict[str, Any] # bound params for the SQL (values only; no ':' prefix) + + # This is obsolete, replaced by Action and ResourceType @dataclass class Permission: diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index 4dda404b..c0d55d08 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -22,7 +22,7 @@ The core pattern is: from typing import Optional from datasette.plugins import pm from datasette.utils import await_me_maybe -from datasette.utils.permissions import PluginSQL +from datasette.permissions import PermissionSQL async def build_allowed_resources_sql( @@ -80,10 +80,10 @@ async def build_allowed_resources_sql( continue if isinstance(result, list): for plugin_sql in result: - if isinstance(plugin_sql, PluginSQL): + if isinstance(plugin_sql, PermissionSQL): rule_sqls.append(plugin_sql.sql) all_params.update(plugin_sql.params) - elif isinstance(result, PluginSQL): + elif isinstance(result, PermissionSQL): rule_sqls.append(result.sql) all_params.update(result.params) @@ -211,10 +211,10 @@ async def check_permission_for_resource( continue if isinstance(result, list): for plugin_sql in result: - if isinstance(plugin_sql, PluginSQL): + if isinstance(plugin_sql, PermissionSQL): rule_sqls.append(plugin_sql.sql) all_params.update(plugin_sql.params) - elif isinstance(result, PluginSQL): + elif isinstance(result, PermissionSQL): rule_sqls.append(result.sql) all_params.update(result.params) diff --git a/datasette/utils/permissions.py b/datasette/utils/permissions.py index 7dc2eb4d..43752b68 100644 --- a/datasette/utils/permissions.py +++ b/datasette/utils/permissions.py @@ -1,31 +1,17 @@ # perm_utils.py from __future__ import annotations -from dataclasses import dataclass from typing import Any, Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union import sqlite3 +from datasette.permissions import PermissionSQL + # ----------------------------- # Plugin interface & utilities # ----------------------------- -@dataclass -class PluginSQL: - """ - A plugin contributes SQL that yields: - parent TEXT NULL, - child TEXT NULL, - allow INTEGER, -- 1 allow, 0 deny - reason TEXT - """ - - source: str # identifier used for auditing (e.g., plugin name) - sql: str # SQL that SELECTs the 4 columns above - params: Dict[str, Any] # bound params for the SQL (values only; no ':' prefix) - - def _namespace_params(i: int, params: Dict[str, Any]) -> Tuple[str, Dict[str, Any]]: """ Rewrite parameter placeholders to distinct names per plugin block. @@ -45,12 +31,12 @@ def _namespace_params(i: int, params: Dict[str, Any]) -> Tuple[str, Dict[str, An return rewrite, namespaced -PluginProvider = Callable[[str], PluginSQL] -PluginOrFactory = Union[PluginSQL, PluginProvider] +PluginProvider = Callable[[str], PermissionSQL] +PluginOrFactory = Union[PermissionSQL, PluginProvider] def build_rules_union( - actor: str, plugins: Sequence[PluginSQL] + actor: str, plugins: Sequence[PermissionSQL] ) -> Tuple[str, Dict[str, Any]]: """ Compose plugin SQL into a UNION ALL with namespaced parameters. @@ -107,8 +93,8 @@ async def resolve_permissions_from_catalog( (Use child=NULL for parent-scoped actions like "execute-sql".) - *db* exposes: rows = await db.execute(sql, params) where rows is an iterable of sqlite3.Row - - plugins are either PluginSQL objects or callables accepting (action: str) - and returning PluginSQL instances selecting (parent, child, allow, reason) + - plugins are either PermissionSQL objects or callables accepting (action: str) + and returning PermissionSQL instances selecting (parent, child, allow, reason) Decision policy: 1) Specificity first: child (depth=2) > parent (depth=1) > root (depth=0) @@ -121,14 +107,14 @@ async def resolve_permissions_from_catalog( - parent, child, allow, reason, source_plugin, depth - resource (rendered "/parent/child" or "/parent" or "/") """ - resolved_plugins: List[PluginSQL] = [] + resolved_plugins: List[PermissionSQL] = [] for plugin in plugins: - if callable(plugin) and not isinstance(plugin, PluginSQL): + if callable(plugin) and not isinstance(plugin, PermissionSQL): resolved = plugin(action) # type: ignore[arg-type] else: resolved = plugin # type: ignore[assignment] - if not isinstance(resolved, PluginSQL): - raise TypeError("Plugin providers must return PluginSQL instances") + if not isinstance(resolved, PermissionSQL): + raise TypeError("Plugin providers must return PermissionSQL instances") resolved_plugins.append(resolved) union_sql, rule_params = build_rules_union(actor, resolved_plugins) diff --git a/datasette/views/special.py b/datasette/views/special.py index 2c5004d0..a4388163 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -9,7 +9,8 @@ from datasette.utils import ( tilde_encode, tilde_decode, ) -from datasette.utils.permissions import PluginSQL, resolve_permissions_from_catalog +from datasette.permissions import PermissionSQL +from datasette.utils.permissions import resolve_permissions_from_catalog from datasette.plugins import pm from .base import BaseView, View import secrets @@ -303,9 +304,9 @@ class AllowedResourcesView(BaseView): for candidate in candidates: if candidate is None: continue - if not isinstance(candidate, PluginSQL): + if not isinstance(candidate, PermissionSQL): logger.warning( - "Skipping permission_resources_sql result %r from plugin; expected PluginSQL", + "Skipping permission_resources_sql result %r from plugin; expected PermissionSQL", candidate, ) continue diff --git a/tests/test_actions_sql.py b/tests/test_actions_sql.py index 756e3801..d9cfd1ef 100644 --- a/tests/test_actions_sql.py +++ b/tests/test_actions_sql.py @@ -12,7 +12,7 @@ import pytest import pytest_asyncio from datasette.app import Datasette from datasette.plugins import pm -from datasette.utils.permissions import PluginSQL +from datasette.permissions import PermissionSQL from datasette.resources import TableResource from datasette import hookimpl @@ -63,7 +63,7 @@ async def test_allowed_resources_global_allow(test_ds): def rules_callback(datasette, actor, action): if actor and actor.get("id") == "alice": sql = "SELECT NULL AS parent, NULL AS child, 1 AS allow, 'global: alice has access' AS reason" - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -101,7 +101,7 @@ async def test_allowed_specific_resource(test_ds): UNION ALL SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, 'analyst access' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -145,7 +145,7 @@ async def test_allowed_resources_with_reasons(test_ds): SELECT 'analytics' AS parent, 'sensitive' AS child, 0 AS allow, 'child: sensitive data denied' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -185,7 +185,7 @@ async def test_child_deny_overrides_parent_allow(test_ds): SELECT 'analytics' AS parent, 'sensitive' AS child, 0 AS allow, 'child: deny sensitive' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -233,7 +233,7 @@ async def test_child_allow_overrides_parent_deny(test_ds): SELECT 'production' AS parent, 'orders' AS child, 1 AS allow, 'child: carol can see orders' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -304,7 +304,7 @@ async def test_sql_does_filtering_not_python(test_ds): SELECT 'analytics' AS parent, 'users' AS child, 1 AS allow, 'specific allow' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) plugin = PermissionRulesPlugin(rules_callback) pm.register(plugin, name="test_plugin") diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 055808d5..445891e4 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -12,7 +12,7 @@ from datasette.app import Datasette from datasette import cli, hookimpl, Permission from datasette.filters import FilterArguments from datasette.plugins import get_plugins, DEFAULT_PLUGINS, pm -from datasette.utils.permissions import PluginSQL +from datasette.permissions import PermissionSQL from datasette.utils.sqlite import sqlite3 from datasette.utils import StartupError, await_me_maybe from jinja2 import ChoiceLoader, FileSystemLoader @@ -722,7 +722,7 @@ async def test_hook_permission_resources_sql(): collected.append(block) assert collected - assert all(isinstance(item, PluginSQL) for item in collected) + assert all(isinstance(item, PermissionSQL) for item in collected) @pytest.mark.asyncio diff --git a/tests/test_tables_endpoint.py b/tests/test_tables_endpoint.py index a3305406..45b6dcfb 100644 --- a/tests/test_tables_endpoint.py +++ b/tests/test_tables_endpoint.py @@ -8,7 +8,7 @@ import pytest import pytest_asyncio from datasette.app import Datasette from datasette.plugins import pm -from datasette.utils.permissions import PluginSQL +from datasette.permissions import PermissionSQL from datasette import hookimpl @@ -57,7 +57,7 @@ async def test_tables_endpoint_global_access(test_ds): def rules_callback(datasette, actor, action): if actor and actor.get("id") == "alice": sql = "SELECT NULL AS parent, NULL AS child, 1 AS allow, 'global: alice has access' AS reason" - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -97,7 +97,7 @@ async def test_tables_endpoint_database_restriction(test_ds): if actor and actor.get("role") == "analyst": # Allow only analytics database sql = "SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, 'analyst access' AS reason" - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -144,7 +144,7 @@ async def test_tables_endpoint_table_exception(test_ds): UNION ALL SELECT 'analytics' AS parent, 'users' AS child, 1 AS allow, 'carol exception' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -186,7 +186,7 @@ async def test_tables_endpoint_deny_overrides_allow(test_ds): UNION ALL SELECT 'analytics' AS parent, 'sensitive' AS child, 0 AS allow, 'deny sensitive' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -252,7 +252,7 @@ async def test_tables_endpoint_specific_table_only(test_ds): UNION ALL SELECT 'production' AS parent, 'orders' AS child, 1 AS allow, 'specific table 2' AS reason """ - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) @@ -290,7 +290,7 @@ async def test_tables_endpoint_empty_result(test_ds): if actor and actor.get("id") == "blocked": # Global deny sql = "SELECT NULL AS parent, NULL AS child, 0 AS allow, 'global deny' AS reason" - return PluginSQL(source="test", sql=sql, params={}) + return PermissionSQL(source="test", sql=sql, params={}) return None plugin = PermissionRulesPlugin(rules_callback) diff --git a/tests/test_utils_permissions.py b/tests/test_utils_permissions.py index 81fbfacd..0693cbc1 100644 --- a/tests/test_utils_permissions.py +++ b/tests/test_utils_permissions.py @@ -1,7 +1,7 @@ import pytest from datasette.app import Datasette +from datasette.permissions import PermissionSQL from datasette.utils.permissions import ( - PluginSQL, PluginProvider, resolve_permissions_from_catalog, ) @@ -26,8 +26,8 @@ NO_RULES_SQL = ( def plugin_allow_all_for_user(user: str) -> PluginProvider: - def provider(action: str) -> PluginSQL: - return PluginSQL( + def provider(action: str) -> PermissionSQL: + return PermissionSQL( "allow_all", """ SELECT NULL AS parent, NULL AS child, 1 AS allow, @@ -41,8 +41,8 @@ def plugin_allow_all_for_user(user: str) -> PluginProvider: def plugin_deny_specific_table(user: str, parent: str, child: str) -> PluginProvider: - def provider(action: str) -> PluginSQL: - return PluginSQL( + def provider(action: str) -> PermissionSQL: + return PermissionSQL( "deny_specific_table", """ SELECT :parent AS parent, :child AS child, 0 AS allow, @@ -56,8 +56,8 @@ def plugin_deny_specific_table(user: str, parent: str, child: str) -> PluginProv def plugin_org_policy_deny_parent(parent: str) -> PluginProvider: - def provider(action: str) -> PluginSQL: - return PluginSQL( + def provider(action: str) -> PermissionSQL: + return PermissionSQL( "org_policy_parent_deny", """ SELECT :parent AS parent, NULL AS child, 0 AS allow, @@ -70,8 +70,8 @@ def plugin_org_policy_deny_parent(parent: str) -> PluginProvider: def plugin_allow_parent_for_user(user: str, parent: str) -> PluginProvider: - def provider(action: str) -> PluginSQL: - return PluginSQL( + def provider(action: str) -> PermissionSQL: + return PermissionSQL( "allow_parent", """ SELECT :parent AS parent, NULL AS child, 1 AS allow, @@ -85,8 +85,8 @@ def plugin_allow_parent_for_user(user: str, parent: str) -> PluginProvider: def plugin_child_allow_for_user(user: str, parent: str, child: str) -> PluginProvider: - def provider(action: str) -> PluginSQL: - return PluginSQL( + def provider(action: str) -> PermissionSQL: + return PermissionSQL( "allow_child", """ SELECT :parent AS parent, :child AS child, 1 AS allow, @@ -100,8 +100,8 @@ def plugin_child_allow_for_user(user: str, parent: str, child: str) -> PluginPro def plugin_root_deny_for_all() -> PluginProvider: - def provider(action: str) -> PluginSQL: - return PluginSQL( + def provider(action: str) -> PermissionSQL: + return PermissionSQL( "root_deny", """ SELECT NULL AS parent, NULL AS child, 0 AS allow, 'root deny for all on ' || :action AS reason @@ -115,8 +115,8 @@ def plugin_root_deny_for_all() -> PluginProvider: def plugin_conflicting_same_child_rules( user: str, parent: str, child: str ) -> List[PluginProvider]: - def allow_provider(action: str) -> PluginSQL: - return PluginSQL( + def allow_provider(action: str) -> PermissionSQL: + return PermissionSQL( "conflict_child_allow", """ SELECT :parent AS parent, :child AS child, 1 AS allow, @@ -126,8 +126,8 @@ def plugin_conflicting_same_child_rules( {"parent": parent, "child": child, "user": user, "action": action}, ) - def deny_provider(action: str) -> PluginSQL: - return PluginSQL( + def deny_provider(action: str) -> PermissionSQL: + return PermissionSQL( "conflict_child_deny", """ SELECT :parent AS parent, :child AS child, 0 AS allow, @@ -141,14 +141,14 @@ def plugin_conflicting_same_child_rules( def plugin_allow_all_for_action(user: str, allowed_action: str) -> PluginProvider: - def provider(action: str) -> PluginSQL: + def provider(action: str) -> PermissionSQL: if action != allowed_action: - return PluginSQL( + return PermissionSQL( f"allow_all_{allowed_action}_noop", NO_RULES_SQL, {}, ) - return PluginSQL( + return PermissionSQL( f"allow_all_{allowed_action}", """ SELECT NULL AS parent, NULL AS child, 1 AS allow,