mirror of
https://github.com/simonw/datasette.git
synced 2025-12-10 16:51:24 +01:00
Fix actor restrictions to work with new actions system
- Updated restrictions_allow_action() to use datasette.actions instead of datasette.permissions - Changed references from Permission to Action objects - Updated takes_database checks to takes_parent - Added get_action() method to Datasette class for looking up actions by name or abbreviation - Integrated actor restriction checking into allowed() method - Actor restrictions (_r in actor dict) are now properly enforced after SQL permission checks This fixes tests in test_api_write.py where actors with restricted permissions were incorrectly being granted access to actions outside their restrictions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
dc241e8691
commit
e1582c1424
2 changed files with 38 additions and 9 deletions
|
|
@ -547,6 +547,18 @@ class Datasette:
|
||||||
"No permission found with name or abbreviation {}".format(name_or_abbr)
|
"No permission found with name or abbreviation {}".format(name_or_abbr)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def get_action(self, name_or_abbr: str):
|
||||||
|
"""
|
||||||
|
Returns an Action object for the given name or abbreviation. Returns None if not found.
|
||||||
|
"""
|
||||||
|
if name_or_abbr in self.actions:
|
||||||
|
return self.actions[name_or_abbr]
|
||||||
|
# Try abbreviation
|
||||||
|
for action in self.actions.values():
|
||||||
|
if action.abbr == name_or_abbr:
|
||||||
|
return action
|
||||||
|
return None
|
||||||
|
|
||||||
async def refresh_schemas(self):
|
async def refresh_schemas(self):
|
||||||
if self._refresh_schemas_lock.locked():
|
if self._refresh_schemas_lock.locked():
|
||||||
return
|
return
|
||||||
|
|
@ -1282,6 +1294,23 @@ class Datasette:
|
||||||
child=resource.child,
|
child=resource.child,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Check actor restrictions after SQL permissions
|
||||||
|
# If the SQL check says "yes" but actor has restrictions, verify action is allowed
|
||||||
|
if result and actor and "_r" in actor:
|
||||||
|
from datasette.default_permissions import restrictions_allow_action
|
||||||
|
|
||||||
|
# Convert Resource to old-style format for restrictions check
|
||||||
|
if resource.parent and resource.child:
|
||||||
|
old_style_resource = (resource.parent, resource.child)
|
||||||
|
elif resource.parent:
|
||||||
|
old_style_resource = resource.parent
|
||||||
|
else:
|
||||||
|
old_style_resource = None
|
||||||
|
|
||||||
|
# If restrictions don't allow this action, deny it
|
||||||
|
if not restrictions_allow_action(self, actor["_r"], action, old_style_resource):
|
||||||
|
result = False
|
||||||
|
|
||||||
# Log the permission check for debugging
|
# Log the permission check for debugging
|
||||||
# Convert Resource to old-style format for backward compatibility with debug tools
|
# Convert Resource to old-style format for backward compatibility with debug tools
|
||||||
if resource.parent and resource.child:
|
if resource.parent and resource.child:
|
||||||
|
|
|
||||||
|
|
@ -306,20 +306,20 @@ def restrictions_allow_action(
|
||||||
"Do these restrictions allow the requested action against the requested resource?"
|
"Do these restrictions allow the requested action against the requested resource?"
|
||||||
if action == "view-instance":
|
if action == "view-instance":
|
||||||
# Special case for view-instance: it's allowed if the restrictions include any
|
# Special case for view-instance: it's allowed if the restrictions include any
|
||||||
# permissions that have the implies_can_view=True flag set
|
# actions that have the implies_can_view=True flag set
|
||||||
all_rules = restrictions.get("a") or []
|
all_rules = restrictions.get("a") or []
|
||||||
for database_rules in (restrictions.get("d") or {}).values():
|
for database_rules in (restrictions.get("d") or {}).values():
|
||||||
all_rules += database_rules
|
all_rules += database_rules
|
||||||
for database_resource_rules in (restrictions.get("r") or {}).values():
|
for database_resource_rules in (restrictions.get("r") or {}).values():
|
||||||
for resource_rules in database_resource_rules.values():
|
for resource_rules in database_resource_rules.values():
|
||||||
all_rules += resource_rules
|
all_rules += resource_rules
|
||||||
permissions = [datasette.get_permission(action) for action in all_rules]
|
actions = [datasette.get_action(action) for action in all_rules]
|
||||||
if any(p for p in permissions if p.implies_can_view):
|
if any(a for a in actions if a and a.implies_can_view):
|
||||||
return True
|
return True
|
||||||
|
|
||||||
if action == "view-database":
|
if action == "view-database":
|
||||||
# Special case for view-database: it's allowed if the restrictions include any
|
# Special case for view-database: it's allowed if the restrictions include any
|
||||||
# permissions that have the implies_can_view=True flag set AND takes_database
|
# actions that have the implies_can_view=True flag set AND takes_parent
|
||||||
all_rules = restrictions.get("a") or []
|
all_rules = restrictions.get("a") or []
|
||||||
database_rules = list((restrictions.get("d") or {}).get(resource) or [])
|
database_rules = list((restrictions.get("d") or {}).get(resource) or [])
|
||||||
all_rules += database_rules
|
all_rules += database_rules
|
||||||
|
|
@ -327,15 +327,15 @@ def restrictions_allow_action(
|
||||||
for resource_rules in (restrictions.get("r") or {}).values():
|
for resource_rules in (restrictions.get("r") or {}).values():
|
||||||
for table_rules in resource_rules.values():
|
for table_rules in resource_rules.values():
|
||||||
all_rules += table_rules
|
all_rules += table_rules
|
||||||
permissions = [datasette.get_permission(action) for action in all_rules]
|
actions = [datasette.get_action(action) for action in all_rules]
|
||||||
if any(p for p in permissions if p.implies_can_view and p.takes_database):
|
if any(a for a in actions if a and a.implies_can_view and a.takes_parent):
|
||||||
return True
|
return True
|
||||||
|
|
||||||
# Does this action have an abbreviation?
|
# Does this action have an abbreviation?
|
||||||
to_check = {action}
|
to_check = {action}
|
||||||
permission = datasette.permissions.get(action)
|
action_obj = datasette.actions.get(action)
|
||||||
if permission and permission.abbr:
|
if action_obj and action_obj.abbr:
|
||||||
to_check.add(permission.abbr)
|
to_check.add(action_obj.abbr)
|
||||||
|
|
||||||
# If restrictions is defined then we use those to further restrict the actor
|
# If restrictions is defined then we use those to further restrict the actor
|
||||||
# Crucially, we only use this to say NO (return False) - we never
|
# Crucially, we only use this to say NO (return False) - we never
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue