Fix permission_allowed_sql_bridge to not apply defaults, closes #2526

The bridge was incorrectly using the new allowed() method which applies
default allow rules. This caused actors without restrictions to get True
instead of USE_DEFAULT, breaking backward compatibility.

Fixed by:
- Removing the code that converted to resource objects and called allowed()
- Bridge now ONLY checks config-based rules via _config_permission_rules()
- Returns None when no config rules exist, allowing Permission.default to apply
- This maintains backward compatibility with the permission_allowed() API

All 177 permission tests now pass, including test_actor_restricted_permissions
and test_permissions_checked which were previously failing.
This commit is contained in:
Simon Willison 2025-10-23 14:34:48 -07:00
commit 2039e238d9
2 changed files with 7 additions and 28 deletions

View file

@ -130,33 +130,15 @@ def register_permissions():
@hookimpl(tryfirst=True, specname="permission_allowed")
async def permission_allowed_sql_bridge(datasette, actor, action, resource):
"""
Bridge the old permission_allowed API to the new SQL-based system.
Bridge config-based permission rules to the old permission_allowed API.
This allows views using the old string/tuple resource API to benefit from
the SQL-based permission rules including config blocks.
config blocks defined in datasette.yaml without using the new resource-based system.
Note: This does NOT apply default allow rules - those should come from the
Permission object's default value to maintain backward compatibility.
"""
# First try to use the new resource-based system if possible
if action in datasette.actions:
action_obj = datasette.actions[action]
if action_obj and action_obj.resource_class:
# Try to convert string/tuple resource to Resource object
resource_obj = None
try:
if resource is None:
# Can't create a resource object without a resource identifier
pass # Fall through to config checking below
elif isinstance(resource, str):
resource_obj = action_obj.resource_class(database=resource)
elif isinstance(resource, tuple) and len(resource) == 2:
resource_obj = action_obj.resource_class(database=resource[0], table=resource[1])
except Exception:
pass # Fall through to config checking below
if resource_obj is not None:
# Successfully created resource object, use new system
return await datasette.allowed(action=action, resource=resource_obj, actor=actor)
# Fall back to checking config permission blocks manually via SQL
# Only check config-based rules - don't apply defaults
config_rules = await _config_permission_rules(datasette, actor, action)
if not config_rules:
return None

View file

@ -344,16 +344,13 @@ def test_query_list_respects_view_query():
("execute-sql", "fixtures"),
],
),
pytest.param(
(
"/fixtures.db",
[
"view-instance",
("view-database", "fixtures"),
("view-database-download", "fixtures"),
],
marks=pytest.mark.xfail(
reason="ensure_permissions() short-circuits, not checking all permissions - see #2526"
),
),
(
"/fixtures/neighborhood_search",