mirror of
https://github.com/simonw/datasette.git
synced 2025-12-10 16:51:24 +01:00
Fix for actor restrictions + config bug
Described here: https://github.com/simonw/datasette/pull/2539#issuecomment-3447870261
This commit is contained in:
parent
fb9cd5c72c
commit
6854270da3
2 changed files with 173 additions and 0 deletions
|
|
@ -101,6 +101,39 @@ async def _config_permission_rules(datasette, actor, action) -> list[PermissionS
|
||||||
return None
|
return None
|
||||||
return actor_matches_allow(actor_dict, allow_block)
|
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 = []
|
rows = []
|
||||||
|
|
||||||
def add_row(parent, child, result, scope):
|
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"""
|
"""For 'allow' blocks, always add a row if the block exists - deny if no match"""
|
||||||
if allow_block is None:
|
if allow_block is None:
|
||||||
return
|
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)
|
result = evaluate(allow_block)
|
||||||
# If result is None (no match) or False, treat as deny
|
# If result is None (no match) or False, treat as deny
|
||||||
rows.append(
|
rows.append(
|
||||||
|
|
|
||||||
|
|
@ -1442,3 +1442,137 @@ async def test_actor_restrictions_empty_allowlist(perms_ds):
|
||||||
|
|
||||||
result = await perms_ds.allowed(action="view-instance", actor=actor)
|
result = await perms_ds.allowed(action="view-instance", actor=actor)
|
||||||
assert result is False
|
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
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue