diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 9afb088e..5642cdfe 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -19,7 +19,7 @@ async def actor_restrictions_sql(datasette, actor, action): return None restrictions = actor.get("_r") if isinstance(actor, dict) else None - if not restrictions: + if restrictions is None: return [] # Check if this action appears in restrictions (with abbreviations) @@ -28,91 +28,63 @@ async def actor_restrictions_sql(datasette, actor, action): if action_obj and action_obj.abbr: action_checks.add(action_obj.abbr) - # Check if this action is in the allowlist anywhere in restrictions - is_in_allowlist = False + # Check if globally allowed in restrictions global_actions = restrictions.get("a", []) - if action_checks.intersection(global_actions): - is_in_allowlist = True + is_globally_allowed = action_checks.intersection(global_actions) - if not is_in_allowlist: - for db_actions in restrictions.get("d", {}).values(): - if action_checks.intersection(db_actions): - is_in_allowlist = True - break + if is_globally_allowed: + # Globally allowed - no restriction filtering needed + return [] - if not is_in_allowlist: - for tables in restrictions.get("r", {}).values(): - for table_actions in tables.values(): - if action_checks.intersection(table_actions): - is_in_allowlist = True - break - if is_in_allowlist: - break - - # If action not in allowlist at all, add global deny and return - if not is_in_allowlist: - sql = "SELECT NULL AS parent, NULL AS child, 0 AS allow, :actor_deny_reason AS reason" - return [ - PermissionSQL( - sql=sql, - params={ - "actor_deny_reason": f"actor restrictions: {action} not in allowlist" - }, - ) - ] - - # Action IS in allowlist - build deny + specific allows - selects = [] - params = {} + # Not globally allowed - build restriction_sql that lists allowlisted resources + restriction_selects = [] + restriction_params = {} param_counter = 0 - def add_row(parent, child, allow, reason): - """Helper to add a parameterized SELECT statement.""" - nonlocal param_counter - prefix = f"restr_{param_counter}" - param_counter += 1 - - selects.append( - f"SELECT :{prefix}_parent AS parent, :{prefix}_child AS child, " - f":{prefix}_allow AS allow, :{prefix}_reason AS reason" - ) - params[f"{prefix}_parent"] = parent - params[f"{prefix}_child"] = child - params[f"{prefix}_allow"] = 1 if allow else 0 - params[f"{prefix}_reason"] = reason - - # If NOT globally allowed, add global deny as gatekeeper - is_globally_allowed = action_checks.intersection(global_actions) - if not is_globally_allowed: - add_row(None, None, 0, f"actor restrictions: {action} denied by default") - else: - # Globally allowed - add global allow - add_row(None, None, 1, f"actor restrictions: global {action}") - - # Add database-level allows + # Add database-level allowlisted resources db_restrictions = restrictions.get("d", {}) for db_name, db_actions in db_restrictions.items(): if action_checks.intersection(db_actions): - add_row(db_name, None, 1, f"actor restrictions: database {db_name}") + prefix = f"restr_{param_counter}" + param_counter += 1 + restriction_selects.append( + f"SELECT :{prefix}_parent AS parent, NULL AS child" + ) + restriction_params[f"{prefix}_parent"] = db_name - # Add resource/table-level allows + # Add table-level allowlisted resources resource_restrictions = restrictions.get("r", {}) for db_name, tables in resource_restrictions.items(): for table_name, table_actions in tables.items(): if action_checks.intersection(table_actions): - add_row( - db_name, - table_name, - 1, - f"actor restrictions: {db_name}/{table_name}", + prefix = f"restr_{param_counter}" + param_counter += 1 + restriction_selects.append( + f"SELECT :{prefix}_parent AS parent, :{prefix}_child AS child" ) + restriction_params[f"{prefix}_parent"] = db_name + restriction_params[f"{prefix}_child"] = table_name - if not selects: - return [] + if not restriction_selects: + # Action not in allowlist - return empty restriction (INTERSECT will return no results) + return [ + PermissionSQL( + params={"deny": f"actor restrictions: {action} not in allowlist"}, + restriction_sql="SELECT NULL AS parent, NULL AS child WHERE 0", # Empty set + ) + ] - sql = "\nUNION ALL\n".join(selects) + # Build restriction SQL that returns allowed (parent, child) pairs + restriction_sql = "\nUNION ALL\n".join(restriction_selects) - return [PermissionSQL(sql=sql, params=params)] + # Return restriction-only PermissionSQL (sql=None means no permission rules) + # The restriction_sql does the actual filtering via INTERSECT + return [ + PermissionSQL( + params=restriction_params, + restriction_sql=restriction_sql, + ) + ] @hookimpl(specname="permission_resources_sql") @@ -375,13 +347,11 @@ async def default_allow_sql_check(datasette, actor, action): @hookimpl(specname="permission_resources_sql") async def default_action_permissions_sql(datasette, actor, action): - """Apply default allow rules for standard view/execute actions.""" - # Only apply defaults if actor has no restrictions - # If actor has restrictions, they've already added their own deny/allow rules - has_restrictions = actor and "_r" in actor - if has_restrictions: - return None + """Apply default allow rules for standard view/execute actions. + With the INTERSECT-based restriction approach, these defaults are always generated + and then filtered by restriction_sql if the actor has restrictions. + """ default_allow_actions = { "view-instance", "view-database", diff --git a/datasette/permissions.py b/datasette/permissions.py index 7b1fc90c..c91385a0 100644 --- a/datasette/permissions.py +++ b/datasette/permissions.py @@ -138,13 +138,20 @@ class PermissionSQL: child TEXT NULL, allow INTEGER, -- 1 allow, 0 deny reason TEXT + + For restriction-only plugins, sql can be None and only restriction_sql is provided. """ - sql: str # SQL that SELECTs the 4 columns above + sql: str | None = ( + None # SQL that SELECTs the 4 columns above (can be None for restriction-only) + ) params: dict[str, Any] | None = ( None # bound params for the SQL (values only; no ':' prefix) ) source: str | None = None # System will set this to the plugin name + restriction_sql: str | None = ( + None # Optional SQL that returns (parent, child) for restriction filtering + ) @classmethod def allow(cls, reason: str, _allow: bool = True) -> "PermissionSQL": diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index 13594a2d..7121e2d0 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -157,8 +157,19 @@ async def _build_single_action_sql( all_params = {} rule_sqls = [] + restriction_sqls = [] for permission_sql in permission_sqls: + # Always collect params (even from restriction-only plugins) + all_params.update(permission_sql.params or {}) + + # Collect restriction SQL filters + if permission_sql.restriction_sql: + restriction_sqls.append(permission_sql.restriction_sql) + + # Skip plugins that only provide restriction_sql (no permission rules) + if permission_sql.sql is None: + continue rule_sqls.append( f""" SELECT parent, child, allow, reason, '{permission_sql.source}' AS source_plugin FROM ( @@ -166,7 +177,6 @@ async def _build_single_action_sql( ) """.strip() ) - all_params.update(permission_sql.params or {}) # If no rules, return empty result (deny all) if not rule_sqls: @@ -200,6 +210,9 @@ async def _build_single_action_sql( anon_params = {} for permission_sql in anon_permission_sqls: + # Skip plugins that only provide restriction_sql (no permission rules) + if permission_sql.sql is None: + continue rewritten_sql = permission_sql.sql for key, value in (permission_sql.params or {}).items(): anon_key = f"anon_{key}" @@ -360,6 +373,17 @@ async def _build_single_action_sql( query_parts.append(")") + # Add restriction list CTE if there are restrictions + if restriction_sqls: + # Wrap each restriction_sql in a subquery to avoid operator precedence issues + # with UNION ALL inside the restriction SQL statements + restriction_intersect = "\nINTERSECT\n".join( + f"SELECT * FROM ({sql})" for sql in restriction_sqls + ) + query_parts.extend( + [",", "restriction_list AS (", f" {restriction_intersect}", ")"] + ) + # Final SELECT select_cols = "parent, child, reason" if include_is_private: @@ -369,6 +393,17 @@ async def _build_single_action_sql( query_parts.append("FROM decisions") query_parts.append("WHERE is_allowed = 1") + # Add restriction filter if there are restrictions + if restriction_sqls: + query_parts.append( + """ + AND EXISTS ( + SELECT 1 FROM restriction_list r + WHERE (r.parent = decisions.parent OR r.parent IS NULL) + AND (r.child = decisions.child OR r.child IS NULL) + )""" + ) + # Add parent filter if specified if parent is not None: query_parts.append(" AND parent = :filter_parent") @@ -405,11 +440,24 @@ async def build_permission_rules_sql( return ( "SELECT NULL AS parent, NULL AS child, 0 AS allow, NULL AS reason, NULL AS source_plugin WHERE 0", {}, + [], ) union_parts = [] all_params = {} + restriction_sqls = [] + for permission_sql in permission_sqls: + all_params.update(permission_sql.params or {}) + + # Collect restriction SQL filters + if permission_sql.restriction_sql: + restriction_sqls.append(permission_sql.restriction_sql) + + # Skip plugins that only provide restriction_sql (no permission rules) + if permission_sql.sql is None: + continue + union_parts.append( f""" SELECT parent, child, allow, reason, '{permission_sql.source}' AS source_plugin FROM ( @@ -417,10 +465,9 @@ async def build_permission_rules_sql( ) """.strip() ) - all_params.update(permission_sql.params or {}) rules_union = " UNION ALL ".join(union_parts) - return rules_union, all_params + return rules_union, all_params, restriction_sqls async def check_permission_for_resource( @@ -447,7 +494,9 @@ async def check_permission_for_resource( This builds the cascading permission query and checks if the specific resource is in the allowed set. """ - rules_union, all_params = await build_permission_rules_sql(datasette, actor, action) + rules_union, all_params, restriction_sqls = await build_permission_rules_sql( + datasette, actor, action + ) # If no rules (empty SQL), default deny if not rules_union: @@ -457,43 +506,57 @@ async def check_permission_for_resource( all_params["_check_parent"] = parent all_params["_check_child"] = child + # If there are restriction filters, check if the resource passes them first + if restriction_sqls: + # Check if resource is in restriction allowlist + # Database-level restrictions (parent, NULL) should match all children (parent, *) + # Wrap each restriction_sql in a subquery to avoid operator precedence issues + restriction_check = "\nINTERSECT\n".join( + f"SELECT * FROM ({sql})" for sql in restriction_sqls + ) + restriction_query = f""" +WITH restriction_list AS ( + {restriction_check} +) +SELECT EXISTS ( + SELECT 1 FROM restriction_list + WHERE (parent = :_check_parent OR parent IS NULL) + AND (child = :_check_child OR child IS NULL) +) AS in_allowlist +""" + result = await datasette.get_internal_database().execute( + restriction_query, all_params + ) + if result.rows and not result.rows[0][0]: + # Resource not in restriction allowlist - deny + return False + query = f""" WITH all_rules AS ( {rules_union} ), -child_lvl AS ( - SELECT - MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, - MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow +matched_rules AS ( + SELECT ar.*, + CASE + WHEN ar.child IS NOT NULL THEN 2 -- child-level (most specific) + WHEN ar.parent IS NOT NULL THEN 1 -- parent-level + ELSE 0 -- root/global + END AS depth FROM all_rules ar - WHERE ar.parent = :_check_parent AND ar.child = :_check_child + WHERE (ar.parent IS NULL OR ar.parent = :_check_parent) + AND (ar.child IS NULL OR ar.child = :_check_child) ), -parent_lvl AS ( - SELECT - MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, - MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow - FROM all_rules ar - WHERE ar.parent = :_check_parent AND ar.child IS NULL -), -global_lvl AS ( - SELECT - MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, - MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow - FROM all_rules ar - WHERE ar.parent IS NULL AND ar.child IS NULL +winner AS ( + SELECT * + FROM matched_rules + ORDER BY + depth DESC, -- specificity first (higher depth wins) + CASE WHEN allow=0 THEN 0 ELSE 1 END, -- then deny over allow + source_plugin -- stable tie-break + LIMIT 1 ) -SELECT - CASE - WHEN cl.any_deny = 1 THEN 0 - WHEN cl.any_allow = 1 THEN 1 - WHEN pl.any_deny = 1 THEN 0 - WHEN pl.any_allow = 1 THEN 1 - WHEN gl.any_deny = 1 THEN 0 - WHEN gl.any_allow = 1 THEN 1 - ELSE 0 - END AS is_allowed -FROM child_lvl cl, parent_lvl pl, global_lvl gl +SELECT COALESCE((SELECT allow FROM winner), 0) AS is_allowed """ # Execute the query against the internal database diff --git a/datasette/utils/permissions.py b/datasette/utils/permissions.py index 75307abf..58be53a3 100644 --- a/datasette/utils/permissions.py +++ b/datasette/utils/permissions.py @@ -99,6 +99,10 @@ def build_rules_union( # No namespacing - just use plugin params as-is params.update(p.params or {}) + # Skip plugins that only provide restriction_sql (no permission rules) + if p.sql is None: + continue + parts.append( f""" SELECT parent, child, allow, reason, '{p.source}' AS source_plugin FROM ( @@ -155,6 +159,8 @@ async def resolve_permissions_from_catalog( - resource (rendered "/parent/child" or "/parent" or "/") """ resolved_plugins: List[PermissionSQL] = [] + restriction_sqls: List[str] = [] + for plugin in plugins: if callable(plugin) and not isinstance(plugin, PermissionSQL): resolved = plugin(action) # type: ignore[arg-type] @@ -164,6 +170,10 @@ async def resolve_permissions_from_catalog( raise TypeError("Plugin providers must return PermissionSQL instances") resolved_plugins.append(resolved) + # Collect restriction SQL filters + if resolved.restriction_sql: + restriction_sqls.append(resolved.restriction_sql) + union_sql, rule_params = build_rules_union(actor, resolved_plugins) all_params = { **(candidate_params or {}), @@ -199,8 +209,8 @@ async def resolve_permissions_from_catalog( PARTITION BY parent, child ORDER BY depth DESC, -- specificity first - CASE WHEN allow=0 THEN 0 ELSE 1 END, -- deny over allow at same depth - source_plugin -- stable tie-break + CASE WHEN allow=0 THEN 0 ELSE 1 END, -- then deny over allow at same depth + source_plugin -- stable tie-break ) AS rn FROM matched ), @@ -228,6 +238,145 @@ async def resolve_permissions_from_catalog( ORDER BY c.parent, c.child """ + # If there are restriction filters, wrap the query with INTERSECT + # This ensures only resources in the restriction allowlist are returned + if restriction_sqls: + # Start with the main query, but select only parent/child for the INTERSECT + main_query_for_intersect = f""" + WITH + cands AS ( + {candidate_sql} + ), + rules AS ( + {union_sql} + ), + matched AS ( + SELECT + c.parent, c.child, + r.allow, r.reason, r.source_plugin, + CASE + WHEN r.child IS NOT NULL THEN 2 -- child-level (most specific) + WHEN r.parent IS NOT NULL THEN 1 -- parent-level + ELSE 0 -- root/global + END AS depth + FROM cands c + JOIN rules r + ON (r.parent IS NULL OR r.parent = c.parent) + AND (r.child IS NULL OR r.child = c.child) + ), + ranked AS ( + SELECT *, + ROW_NUMBER() OVER ( + PARTITION BY parent, child + ORDER BY + depth DESC, -- specificity first + CASE WHEN allow=0 THEN 0 ELSE 1 END, -- then deny over allow at same depth + source_plugin -- stable tie-break + ) AS rn + FROM matched + ), + winner AS ( + SELECT parent, child, + allow, reason, source_plugin, depth + FROM ranked WHERE rn = 1 + ), + permitted_resources AS ( + SELECT c.parent, c.child + FROM cands c + LEFT JOIN winner w + ON ((w.parent = c.parent) OR (w.parent IS NULL AND c.parent IS NULL)) + AND ((w.child = c.child ) OR (w.child IS NULL AND c.child IS NULL)) + WHERE COALESCE(w.allow, CASE WHEN :implicit_deny THEN 0 ELSE NULL END) = 1 + ) + SELECT parent, child FROM permitted_resources + """ + + # Build restriction list with INTERSECT (all must match) + # Then filter to resources that match hierarchically + # Wrap each restriction_sql in a subquery to avoid operator precedence issues + # with UNION ALL inside the restriction SQL statements + restriction_intersect = "\nINTERSECT\n".join( + f"SELECT * FROM ({sql})" for sql in restriction_sqls + ) + + # Combine: resources allowed by permissions AND in restriction allowlist + # Database-level restrictions (parent, NULL) should match all children (parent, *) + filtered_resources = f""" + WITH restriction_list AS ( + {restriction_intersect} + ), + permitted AS ( + {main_query_for_intersect} + ), + filtered AS ( + SELECT p.parent, p.child + FROM permitted p + WHERE EXISTS ( + SELECT 1 FROM restriction_list r + WHERE (r.parent = p.parent OR r.parent IS NULL) + AND (r.child = p.child OR r.child IS NULL) + ) + ) + """ + + # Now join back to get full results for only the filtered resources + sql = f""" + {filtered_resources} + , cands AS ( + {candidate_sql} + ), + rules AS ( + {union_sql} + ), + matched AS ( + SELECT + c.parent, c.child, + r.allow, r.reason, r.source_plugin, + CASE + WHEN r.child IS NOT NULL THEN 2 -- child-level (most specific) + WHEN r.parent IS NOT NULL THEN 1 -- parent-level + ELSE 0 -- root/global + END AS depth + FROM cands c + JOIN rules r + ON (r.parent IS NULL OR r.parent = c.parent) + AND (r.child IS NULL OR r.child = c.child) + ), + ranked AS ( + SELECT *, + ROW_NUMBER() OVER ( + PARTITION BY parent, child + ORDER BY + depth DESC, -- specificity first + CASE WHEN allow=0 THEN 0 ELSE 1 END, -- then deny over allow at same depth + source_plugin -- stable tie-break + ) AS rn + FROM matched + ), + winner AS ( + SELECT parent, child, + allow, reason, source_plugin, depth + FROM ranked WHERE rn = 1 + ) + SELECT + c.parent, c.child, + COALESCE(w.allow, CASE WHEN :implicit_deny THEN 0 ELSE NULL END) AS allow, + COALESCE(w.reason, CASE WHEN :implicit_deny THEN 'implicit deny' ELSE NULL END) AS reason, + w.source_plugin, + COALESCE(w.depth, -1) AS depth, + :action AS action, + CASE + WHEN c.parent IS NULL THEN '/' + WHEN c.child IS NULL THEN '/' || c.parent + ELSE '/' || c.parent || '/' || c.child + END AS resource + FROM filtered c + LEFT JOIN winner w + ON ((w.parent = c.parent) OR (w.parent IS NULL AND c.parent IS NULL)) + AND ((w.child = c.child ) OR (w.child IS NULL AND c.child IS NULL)) + ORDER BY c.parent, c.child + """ + rows_iter: Iterable[sqlite3.Row] = await db.execute( sql, {**all_params, "implicit_deny": 1 if implicit_deny else 0}, diff --git a/datasette/views/special.py b/datasette/views/special.py index 5a341911..a1d736c5 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -403,7 +403,7 @@ class PermissionRulesView(BaseView): from datasette.utils.actions_sql import build_permission_rules_sql - union_sql, union_params = await build_permission_rules_sql( + union_sql, union_params, restriction_sqls = await build_permission_rules_sql( self.ds, actor, action ) await self.ds.refresh_schemas() diff --git a/tests/test_actor_restriction_bug.py b/tests/test_actor_restriction_bug.py new file mode 100644 index 00000000..0bfc9e1e --- /dev/null +++ b/tests/test_actor_restriction_bug.py @@ -0,0 +1,133 @@ +""" +Test for actor restrictions bug with database-level config. + +This test currently FAILS, demonstrating the bug where database-level +config allow blocks can bypass table-level restrictions. +""" + +import pytest +from datasette.app import Datasette +from datasette.resources import TableResource + + +@pytest.mark.asyncio +async def test_table_restrictions_not_bypassed_by_database_level_config(): + """ + Actor restrictions should act as hard limits that config cannot override. + + BUG: When an actor has table-level restrictions (e.g., only table2 and table3) + but config has a database-level allow block, the database-level config rule + currently allows ALL tables, not just those in the restriction allowlist. + + This test documents the expected behavior and will FAIL until the bug is fixed. + """ + # Config grants access at DATABASE level (not table level) + config = { + "databases": { + "test_db_rnbbdlc": { + "allow": { + "id": "user" + } # Database-level allow - grants access to all tables + } + } + } + + ds = Datasette(config=config) + await ds.invoke_startup() + db = ds.add_memory_database("test_db_rnbbdlc") + await db.execute_write("create table table1 (id integer primary key)") + await db.execute_write("create table table2 (id integer primary key)") + await db.execute_write("create table table3 (id integer primary key)") + await db.execute_write("create table table4 (id integer primary key)") + + # Actor restricted to ONLY table2 and table3 + # Even though config allows the whole database, restrictions should limit access + actor = { + "id": "user", + "_r": { + "r": { # Resource-level (table-level) restrictions + "test_db_rnbbdlc": { + "table2": ["vt"], # vt = view-table abbreviation + "table3": ["vt"], + } + } + }, + } + + # table2 should be allowed (in restriction allowlist AND config allows) + result = await ds.allowed( + action="view-table", + resource=TableResource("test_db_rnbbdlc", "table2"), + actor=actor, + ) + assert result is True, "table2 should be allowed - in restriction allowlist" + + # table3 should be allowed (in restriction allowlist AND config allows) + result = await ds.allowed( + action="view-table", + resource=TableResource("test_db_rnbbdlc", "table3"), + actor=actor, + ) + assert result is True, "table3 should be allowed - in restriction allowlist" + + # table1 should be DENIED (NOT in restriction allowlist) + # Even though database-level config allows it, restrictions should deny it + result = await ds.allowed( + action="view-table", + resource=TableResource("test_db_rnbbdlc", "table1"), + actor=actor, + ) + assert ( + result is False + ), "table1 should be DENIED - not in restriction allowlist, config cannot override" + + # table4 should be DENIED (NOT in restriction allowlist) + # Even though database-level config allows it, restrictions should deny it + result = await ds.allowed( + action="view-table", + resource=TableResource("test_db_rnbbdlc", "table4"), + actor=actor, + ) + assert ( + result is False + ), "table4 should be DENIED - not in restriction allowlist, config cannot override" + + +@pytest.mark.asyncio +async def test_database_restrictions_with_database_level_config(): + """ + Verify that database-level restrictions work correctly with database-level config. + + This should pass - it's testing the case where restriction granularity + matches config granularity. + """ + config = { + "databases": {"test_db_rwdl": {"allow": {"id": "user"}}} # Database-level allow + } + + ds = Datasette(config=config) + await ds.invoke_startup() + db = ds.add_memory_database("test_db_rwdl") + await db.execute_write("create table table1 (id integer primary key)") + await db.execute_write("create table table2 (id integer primary key)") + + # Actor has database-level restriction (all tables in test_db_rwdl) + actor = { + "id": "user", + "_r": {"d": {"test_db_rwdl": ["vt"]}}, # Database-level restrictions + } + + # Both tables should be allowed (database-level restriction matches database-level config) + result = await ds.allowed( + action="view-table", + resource=TableResource("test_db_rwdl", "table1"), + actor=actor, + ) + assert result is True, "table1 should be allowed" + + result = await ds.allowed( + action="view-table", + resource=TableResource("test_db_rwdl", "table2"), + actor=actor, + ) + assert result is True, "table2 should be allowed" diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 5a530b25..971b7e82 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -1630,6 +1630,16 @@ async def test_hook_register_actions_with_custom_resources(): reason="user2 granted view-document-collection" ) + # Default allow for view-document-collection (like other view-* actions) + if action == "view-document-collection": + return PermissionSQL.allow( + reason="default allow for view-document-collection" + ) + + # Default allow for view-document (like other view-* actions) + if action == "view-document": + return PermissionSQL.allow(reason="default allow for view-document") + # Register the plugin temporarily plugin = TestPlugin() pm.register(plugin, name="test_custom_resources_plugin") diff --git a/tests/test_restriction_sql.py b/tests/test_restriction_sql.py new file mode 100644 index 00000000..7d6d8a5a --- /dev/null +++ b/tests/test_restriction_sql.py @@ -0,0 +1,315 @@ +import pytest +from datasette.app import Datasette +from datasette.permissions import PermissionSQL +from datasette.resources import TableResource + + +@pytest.mark.asyncio +async def test_multiple_restriction_sources_intersect(): + """ + Test that when multiple plugins return restriction_sql, they are INTERSECTed. + + This tests the case where both actor _r restrictions AND a plugin + provide restriction_sql - both must pass for access to be granted. + """ + from datasette import hookimpl + from datasette.plugins import pm + + class RestrictivePlugin: + __name__ = "RestrictivePlugin" + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + # Plugin adds additional restriction: only db1_multi_intersect allowed + if action == "view-table": + return PermissionSQL( + restriction_sql="SELECT 'db1_multi_intersect' AS parent, NULL AS child", + params={}, + ) + return None + + plugin = RestrictivePlugin() + pm.register(plugin, name="restrictive_plugin") + + try: + ds = Datasette() + await ds.invoke_startup() + db1 = ds.add_memory_database("db1_multi_intersect") + db2 = ds.add_memory_database("db2_multi_intersect") + await db1.execute_write("CREATE TABLE t1 (id INTEGER)") + await db2.execute_write("CREATE TABLE t1 (id INTEGER)") + await ds._refresh_schemas() # Populate catalog tables + + # Actor has restrictions allowing both databases + # But plugin only allows db1_multi_intersect + # INTERSECT means only db1_multi_intersect/t1 should pass + actor = { + "id": "user", + "_r": {"d": {"db1_multi_intersect": ["vt"], "db2_multi_intersect": ["vt"]}}, + } + + page = await ds.allowed_resources("view-table", actor) + resources = {(r.parent, r.child) for r in page.resources} + + # Should only see db1_multi_intersect/t1 (intersection of actor restrictions and plugin restrictions) + assert ("db1_multi_intersect", "t1") in resources + assert ("db2_multi_intersect", "t1") not in resources + finally: + pm.unregister(name="restrictive_plugin") + + +@pytest.mark.asyncio +async def test_restriction_sql_with_overlapping_databases_and_tables(): + """ + Test actor with both database-level and table-level restrictions for same database. + + When actor has: + - Database-level: db1_overlapping allowed (all tables) + - Table-level: db1_overlapping/t1 allowed + + Both entries are UNION'd (OR'ed) within the actor's restrictions. + Database-level restriction allows ALL tables, so table-level is redundant. + """ + ds = Datasette() + await ds.invoke_startup() + db = ds.add_memory_database("db1_overlapping") + await db.execute_write("CREATE TABLE t1 (id INTEGER)") + await db.execute_write("CREATE TABLE t2 (id INTEGER)") + await ds._refresh_schemas() + + # Actor has BOTH database-level (db1_overlapping all tables) AND table-level (db1_overlapping/t1 only) + actor = { + "id": "user", + "_r": { + "d": { + "db1_overlapping": ["vt"] + }, # Database-level: all tables in db1_overlapping + "r": { + "db1_overlapping": {"t1": ["vt"]} + }, # Table-level: only t1 in db1_overlapping + }, + } + + # Within actor restrictions, entries are UNION'd (OR'ed): + # - Database level allows: (db1_overlapping, NULL) → matches all tables via hierarchical matching + # - Table level allows: (db1_overlapping, t1) → redundant, already covered by database level + # Result: Both tables are allowed + page = await ds.allowed_resources("view-table", actor) + resources = {(r.parent, r.child) for r in page.resources} + + assert ("db1_overlapping", "t1") in resources + # Database-level restriction allows all tables, so t2 is also allowed + assert ("db1_overlapping", "t2") in resources + + +@pytest.mark.asyncio +async def test_restriction_sql_empty_allowlist_query(): + """ + Test the specific SQL query generated when action is not in allowlist. + + actor_restrictions_sql() returns "SELECT NULL AS parent, NULL AS child WHERE 0" + Verify this produces an empty result set. + """ + ds = Datasette() + await ds.invoke_startup() + db = ds.add_memory_database("db1_empty_allowlist") + await db.execute_write("CREATE TABLE t1 (id INTEGER)") + await ds._refresh_schemas() + + # Actor has restrictions but action not in allowlist + actor = {"id": "user", "_r": {"r": {"db1_empty_allowlist": {"t1": ["vt"]}}}} + + # Try to view-database (only view-table is in allowlist) + page = await ds.allowed_resources("view-database", actor) + + # Should be empty + assert len(page.resources) == 0 + + +@pytest.mark.asyncio +async def test_restriction_sql_with_pagination(): + """ + Test that restrictions work correctly with keyset pagination. + """ + ds = Datasette() + await ds.invoke_startup() + db = ds.add_memory_database("db1_pagination") + + # Create many tables + for i in range(10): + await db.execute_write(f"CREATE TABLE t{i:02d} (id INTEGER)") + await ds._refresh_schemas() + + # Actor restricted to only odd-numbered tables + restrictions = {"r": {"db1_pagination": {}}} + for i in range(10): + if i % 2 == 1: # Only odd tables + restrictions["r"]["db1_pagination"][f"t{i:02d}"] = ["vt"] + + actor = {"id": "user", "_r": restrictions} + + # Get first page with small limit + page1 = await ds.allowed_resources( + "view-table", actor, parent="db1_pagination", limit=2 + ) + assert len(page1.resources) == 2 + assert page1.next is not None + + # Get second page using next token + page2 = await ds.allowed_resources( + "view-table", actor, parent="db1_pagination", limit=2, next=page1.next + ) + assert len(page2.resources) == 2 + + # Should have no overlap + page1_ids = {r.child for r in page1.resources} + page2_ids = {r.child for r in page2.resources} + assert page1_ids.isdisjoint(page2_ids) + + # All should be odd-numbered tables + all_ids = page1_ids | page2_ids + for table_id in all_ids: + table_num = int(table_id[1:]) # Extract number from "t01", "t03", etc. + assert table_num % 2 == 1, f"Table {table_id} should be odd-numbered" + + +@pytest.mark.asyncio +async def test_also_requires_with_restrictions(): + """ + Test that also_requires actions properly respect restrictions. + + execute-sql requires view-database. With restrictions, both must pass. + """ + ds = Datasette() + await ds.invoke_startup() + db1 = ds.add_memory_database("db1_also_requires") + db2 = ds.add_memory_database("db2_also_requires") + await ds._refresh_schemas() + + # Actor restricted to only db1_also_requires for view-database + # execute-sql requires view-database, so should only work on db1_also_requires + actor = { + "id": "user", + "_r": { + "d": { + "db1_also_requires": ["vd", "es"], + "db2_also_requires": [ + "es" + ], # They have execute-sql but not view-database + } + }, + } + + # db1_also_requires should allow execute-sql + result = await ds.allowed( + action="execute-sql", + resource=TableResource("db1_also_requires", None), + actor=actor, + ) + assert result is True + + # db2_also_requires should not (they have execute-sql but not view-database) + result = await ds.allowed( + action="execute-sql", + resource=TableResource("db2_also_requires", None), + actor=actor, + ) + assert result is False + + +@pytest.mark.asyncio +async def test_restriction_abbreviations_and_full_names(): + """ + Test that both abbreviations and full action names work in restrictions. + """ + ds = Datasette() + await ds.invoke_startup() + db = ds.add_memory_database("db1_abbrev") + await db.execute_write("CREATE TABLE t1 (id INTEGER)") + await ds._refresh_schemas() + + # Test with abbreviation + actor_abbr = {"id": "user", "_r": {"r": {"db1_abbrev": {"t1": ["vt"]}}}} + result = await ds.allowed( + action="view-table", + resource=TableResource("db1_abbrev", "t1"), + actor=actor_abbr, + ) + assert result is True + + # Test with full name + actor_full = {"id": "user", "_r": {"r": {"db1_abbrev": {"t1": ["view-table"]}}}} + result = await ds.allowed( + action="view-table", + resource=TableResource("db1_abbrev", "t1"), + actor=actor_full, + ) + assert result is True + + # Test with mixed + actor_mixed = {"id": "user", "_r": {"d": {"db1_abbrev": ["view-database", "vt"]}}} + result = await ds.allowed( + action="view-table", + resource=TableResource("db1_abbrev", "t1"), + actor=actor_mixed, + ) + assert result is True + + +@pytest.mark.asyncio +async def test_permission_resources_sql_multiple_restriction_sources_intersect(): + """ + Test that when multiple plugins return restriction_sql, they are INTERSECTed. + + This tests the case where both actor _r restrictions AND a plugin + provide restriction_sql - both must pass for access to be granted. + """ + from datasette import hookimpl + from datasette.plugins import pm + + class RestrictivePlugin: + __name__ = "RestrictivePlugin" + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + # Plugin adds additional restriction: only db1_multi_restrictions allowed + if action == "view-table": + return PermissionSQL( + restriction_sql="SELECT 'db1_multi_restrictions' AS parent, NULL AS child", + params={}, + ) + return None + + plugin = RestrictivePlugin() + pm.register(plugin, name="restrictive_plugin") + + try: + ds = Datasette() + await ds.invoke_startup() + db1 = ds.add_memory_database("db1_multi_restrictions") + db2 = ds.add_memory_database("db2_multi_restrictions") + await db1.execute_write("CREATE TABLE t1 (id INTEGER)") + await db2.execute_write("CREATE TABLE t1 (id INTEGER)") + await ds._refresh_schemas() # Populate catalog tables + + # Actor has restrictions allowing both databases + # But plugin only allows db1 + # INTERSECT means only db1/t1 should pass + actor = { + "id": "user", + "_r": { + "d": { + "db1_multi_restrictions": ["vt"], + "db2_multi_restrictions": ["vt"], + } + }, + } + + page = await ds.allowed_resources("view-table", actor) + resources = {(r.parent, r.child) for r in page.resources} + + # Should only see db1/t1 (intersection of actor restrictions and plugin restrictions) + assert ("db1_multi_restrictions", "t1") in resources + assert ("db2_multi_restrictions", "t1") not in resources + finally: + pm.unregister(name="restrictive_plugin")