diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 04993a68..cd92ba73 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -303,50 +303,33 @@ def restrictions_allow_action( action: str, resource: str | tuple[str, str], ): - "Do these restrictions allow the requested action against the requested resource?" - if action == "view-instance": - # Special case for view-instance: it's allowed if the restrictions include any - # actions that have the implies_can_view=True flag set - all_rules = restrictions.get("a") or [] - for database_rules in (restrictions.get("d") or {}).values(): - all_rules += database_rules - for database_resource_rules in (restrictions.get("r") or {}).values(): - for resource_rules in database_resource_rules.values(): - all_rules += resource_rules - actions = [datasette.get_action(action) for action in all_rules] - if any(a for a in actions if a and a.implies_can_view): - return True - - if action == "view-database": - # Special case for view-database: it's allowed if the restrictions include any - # actions that have the implies_can_view=True flag set AND takes_parent - all_rules = restrictions.get("a") or [] - database_rules = list((restrictions.get("d") or {}).get(resource) or []) - all_rules += database_rules - resource_rules = ((restrictions.get("r") or {}).get(resource) or {}).values() - for resource_rules in (restrictions.get("r") or {}).values(): - for table_rules in resource_rules.values(): - all_rules += table_rules - actions = [datasette.get_action(action) for action in all_rules] - if any(a for a in actions if a and a.implies_can_view and a.takes_parent): - return True + """ + Check if actor restrictions allow the requested action against the requested resource. + Restrictions work on an exact-match basis: if an actor has view-table permission, + they can view tables, but NOT automatically view-instance or view-database. + Each permission is checked independently without implication logic. + """ # Does this action have an abbreviation? to_check = {action} action_obj = datasette.actions.get(action) if action_obj and action_obj.abbr: to_check.add(action_obj.abbr) - # 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 - # use it to return YES (True) because that might over-ride other - # restrictions placed on this actor + # Check if restrictions explicitly allow this action + # Restrictions can be at three levels: + # - "a": global (any resource) + # - "d": per-database + # - "r": per-table/resource + + # Check global level (any resource) all_allowed = restrictions.get("a") if all_allowed is not None: assert isinstance(all_allowed, list) if to_check.intersection(all_allowed): return True - # How about for the current database? + + # Check database level if resource: if isinstance(resource, str): database_name = resource @@ -357,17 +340,17 @@ def restrictions_allow_action( assert isinstance(database_allowed, list) if to_check.intersection(database_allowed): return True - # Or the current table? That's any time the resource is (database, table) + + # Check table/resource level if resource is not None and not isinstance(resource, str) and len(resource) == 2: database, table = resource table_allowed = restrictions.get("r", {}).get(database, {}).get(table) - # TODO: What should this do for canned queries? if table_allowed is not None: assert isinstance(table_allowed, list) if to_check.intersection(table_allowed): return True - # This action is not specifically allowed, so reject it + # This action is not explicitly allowed, so reject it return False diff --git a/tests/test_permissions.py b/tests/test_permissions.py index dbdad368..af7b4a46 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -1240,25 +1240,30 @@ async def test_actor_restrictions( @pytest.mark.parametrize( "restrictions,action,resource,expected", ( + # Exact match: view-instance restriction allows view-instance action ({"a": ["view-instance"]}, "view-instance", None, True), - # view-table and view-database implies view-instance - ({"a": ["view-table"]}, "view-instance", None, True), - ({"a": ["view-database"]}, "view-instance", None, True), + # No implication: view-table does NOT imply view-instance + ({"a": ["view-table"]}, "view-instance", None, False), + ({"a": ["view-database"]}, "view-instance", None, False), # update-row does not imply view-instance ({"a": ["update-row"]}, "view-instance", None, False), - # view-table on a resource implies view-instance - ({"r": {"db1": {"t1": ["view-table"]}}}, "view-instance", None, True), - # execute-sql on a database implies view-instance, view-database - ({"d": {"db1": ["es"]}}, "view-instance", None, True), - ({"d": {"db1": ["es"]}}, "view-database", "db1", True), + # view-table on a resource does NOT imply view-instance + ({"r": {"db1": {"t1": ["view-table"]}}}, "view-instance", None, False), + # execute-sql on a database does NOT imply view-instance or view-database + ({"d": {"db1": ["es"]}}, "view-instance", None, False), + ({"d": {"db1": ["es"]}}, "view-database", "db1", False), ({"d": {"db1": ["es"]}}, "view-database", "db2", False), + # But execute-sql abbreviation DOES allow execute-sql action on that database + ({"d": {"db1": ["es"]}}, "execute-sql", "db1", True), # update-row on a resource does not imply view-instance ({"r": {"db1": {"t1": ["update-row"]}}}, "view-instance", None, False), - # view-database on a resource implies view-instance - ({"d": {"db1": ["view-database"]}}, "view-instance", None, True), + # view-database on a database does NOT imply view-instance + ({"d": {"db1": ["view-database"]}}, "view-instance", None, False), + # But it DOES allow view-database on that specific database + ({"d": {"db1": ["view-database"]}}, "view-database", "db1", True), # Having view-table on "a" allows access to any specific table ({"a": ["view-table"]}, "view-table", ("dbname", "tablename"), True), - # Ditto for on the database + # Having view-table on a database allows access to tables in that database ( {"d": {"dbname": ["view-table"]}}, "view-table",