From 6854270da3af46901fd5056521d21cd382a0a345 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 25 Oct 2025 16:47:07 -0700 Subject: [PATCH] Fix for actor restrictions + config bug Described here: https://github.com/simonw/datasette/pull/2539#issuecomment-3447870261 --- datasette/default_permissions.py | 39 +++++++++ tests/test_permissions.py | 134 +++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 64df839c..3ad84a6d 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -101,6 +101,39 @@ async def _config_permission_rules(datasette, actor, action) -> list[PermissionS return None return actor_matches_allow(actor_dict, allow_block) + # Check if actor has restrictions - if so, we'll filter config rules + has_restrictions = actor_dict and "_r" in actor_dict if actor_dict else False + restrictions = actor_dict.get("_r", {}) if actor_dict else {} + + def is_in_restriction_allowlist(parent, child, action): + """Check if a resource is in the actor's restriction allowlist for this action""" + if not has_restrictions: + return True # No restrictions, all resources allowed + + # Check action with abbreviations + action_obj = datasette.actions.get(action) + action_checks = {action} + if action_obj and action_obj.abbr: + action_checks.add(action_obj.abbr) + + # Check global allowlist + if action_checks.intersection(restrictions.get("a", [])): + return True + + # Check database-level allowlist + if parent and action_checks.intersection( + restrictions.get("d", {}).get(parent, []) + ): + return True + + # Check table-level allowlist + if parent and child: + table_actions = restrictions.get("r", {}).get(parent, {}).get(child, []) + if action_checks.intersection(table_actions): + return True + + return False + rows = [] def add_row(parent, child, result, scope): @@ -119,6 +152,12 @@ async def _config_permission_rules(datasette, actor, action) -> list[PermissionS """For 'allow' blocks, always add a row if the block exists - deny if no match""" if allow_block is None: return + + # If actor has restrictions and this resource is NOT in allowlist, skip this config rule + # Restrictions act as a gating filter - config cannot grant access to restricted-out resources + if not is_in_restriction_allowlist(parent, child, action): + return + result = evaluate(allow_block) # If result is None (no match) or False, treat as deny rows.append( diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 9745e4cd..afc67119 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -1442,3 +1442,137 @@ async def test_actor_restrictions_empty_allowlist(perms_ds): result = await perms_ds.allowed(action="view-instance", actor=actor) assert result is False + + +@pytest.mark.asyncio +async def test_actor_restrictions_cannot_be_overridden_by_config(): + """Test that config permissions cannot override actor restrictions - issue #2534""" + from datasette.app import Datasette + from datasette.resources import TableResource + + # Create datasette with config that allows user to access both t1 AND t2 + config = { + "databases": { + "test_db": { + "tables": { + "t1": {"allow": {"id": "user"}}, + "t2": {"allow": {"id": "user"}}, + } + } + } + } + + ds = Datasette(config=config) + await ds.invoke_startup() + db = ds.add_memory_database("test_db") + await db.execute_write("create table t1 (id integer primary key)") + await db.execute_write("create table t2 (id integer primary key)") + + # Actor restricted to ONLY t1 (not t2) + # Even though config allows t2, restrictions should deny it + actor = {"id": "user", "_r": {"r": {"test_db": {"t1": ["vt"]}}}} + + # t1 should be allowed (in restrictions AND config allows) + result = await ds.allowed( + action="view-table", resource=TableResource("test_db", "t1"), actor=actor + ) + assert result is True, "t1 should be allowed - in restriction allowlist" + + # t2 should be DENIED (not in restrictions, even though config allows) + result = await ds.allowed( + action="view-table", resource=TableResource("test_db", "t2"), actor=actor + ) + assert ( + result is False + ), "t2 should be denied - NOT in restriction allowlist, config cannot override" + + +@pytest.mark.asyncio +async def test_actor_restrictions_with_database_level_config(perms_ds): + """Test database-level restrictions with table-level config - issue #2534""" + from datasette.resources import TableResource + + # Config allows specific tables only + perms_ds._config = { + "databases": { + "perms_ds_one": { + "tables": { + "t1": {"allow": {"id": "user"}}, + "t2": {"allow": {"id": "user"}}, + } + } + } + } + + # Actor has database-level restriction (all tables in perms_ds_one) + # Should only access tables that pass BOTH restrictions AND config + actor = {"id": "user", "_r": {"d": {"perms_ds_one": ["vt"]}}} + + # t1 - in restrictions (all tables) AND config allows + result = await perms_ds.allowed( + action="view-table", resource=TableResource("perms_ds_one", "t1"), actor=actor + ) + assert result is True + + # t2 - in restrictions (all tables) AND config allows + result = await perms_ds.allowed( + action="view-table", resource=TableResource("perms_ds_one", "t2"), actor=actor + ) + assert result is True + + # v1 (view) - in restrictions (all tables) AND config doesn't mention it + # Since actor has database-level restriction allowing all tables, v1 is allowed + # Config is additive, not restrictive - it doesn't create implicit denies + result = await perms_ds.allowed( + action="view-table", resource=TableResource("perms_ds_one", "v1"), actor=actor + ) + assert result is True, "v1 should be allowed - actor has db-level restriction" + + # Clean up + perms_ds._config = None + + +@pytest.mark.asyncio +async def test_actor_restrictions_parent_deny_blocks_config_child_allow(perms_ds): + """ + Test that table-level restrictions add parent-level deny to block + other tables in the same database, even if config allows them + """ + from datasette.resources import TableResource + + # Config allows both t1 and t2 + perms_ds._config = { + "databases": { + "perms_ds_one": { + "tables": { + "t1": {"allow": {"id": "user"}}, + "t2": {"allow": {"id": "user"}}, + } + } + } + } + + # Restriction allows ONLY t1 in perms_ds_one + # This should add: + # - parent-level DENY for perms_ds_one (to block other tables) + # - child-level ALLOW for t1 + actor = {"id": "user", "_r": {"r": {"perms_ds_one": {"t1": ["vt"]}}}} + + # t1 should work (child-level allow beats parent-level deny) + result = await perms_ds.allowed( + action="view-table", resource=TableResource("perms_ds_one", "t1"), actor=actor + ) + assert result is True + + # t2 should be DENIED by parent-level deny from restrictions + # even though config has child-level allow + # Because restrictions should run first + result = await perms_ds.allowed( + action="view-table", resource=TableResource("perms_ds_one", "t2"), actor=actor + ) + assert ( + result is False + ), "t2 should be denied - restriction parent deny should beat config child allow" + + # Clean up + perms_ds._config = None