Tests and fixes for #2102

datasette/default_permissions.py

@@ -190,9 +190,22 @@ def permission_allowed_actor_restrictions(datasette, actor, action, resource):
     # Special case for view-instance: it's allowed if there are any view-database
     # or view-table permissions defined
     if action == "view-instance":
+        all_rules = _r.get("a") or []
+        if (
+            "view-database" in all_rules
+            or "vd" in all_rules
+            or "view-table" in all_rules
+            or "vt" in all_rules
+        ):
+            return None
         database_rules = _r.get("d") or {}
         for rules in database_rules.values():
-            if "vd" in rules or "view-database" in rules:
+            if (
+                "vd" in rules
+                or "view-database" in rules
+                or "vt" in rules
+                or "view-table" in rules
+            ):
                 return None
         # Now check resources
         resource_rules = _r.get("r") or {}
@@ -205,12 +218,33 @@ def permission_allowed_actor_restrictions(datasette, actor, action, resource):
     # defined within that database
     if action == "view-database":
         database_name = resource
+        all_rules = _r.get("a") or []
+        if (
+            "view-database" in all_rules
+            or "vd" in all_rules
+            or "view-table" in all_rules
+            or "vt" in all_rules
+        ):
+            return None
+        database_rules = (_r.get("d") or {}).get(database_name) or {}
+        if "vt" in database_rules or "view-table" in database_rules:
+            return None
         resource_rules = _r.get("r") or {}
         resources_in_database = resource_rules.get(database_name) or {}
         for rules in resources_in_database.values():
             if "vt" in rules or "view-table" in rules:
                 return None
 
+    if action == "view-table":
+        # Can view table if they have view-table in database or instance too
+        database_name = resource[0]
+        all_rules = _r.get("a") or []
+        if "view-table" in all_rules or "vt" in all_rules:
+            return None
+        database_rules = (_r.get("d") or {}).get(database_name) or {}
+        if "vt" in database_rules or "view-table" in database_rules:
+            return None
+
     # Does this action have an abbreviation?
     to_check = {action}
     permission = datasette.permissions.get(action)

tests/test_permissions.py

@@ -35,6 +35,8 @@ async def perms_ds():
     one = ds.add_memory_database("perms_ds_one")
     two = ds.add_memory_database("perms_ds_two")
     await one.execute_write("create table if not exists t1 (id integer primary key)")
+    await one.execute_write("insert or ignore into t1 (id) values (1)")
+    await one.execute_write("create view if not exists v1 as select * from t1")
     await one.execute_write("create table if not exists t2 (id integer primary key)")
     await two.execute_write("create table if not exists t1 (id integer primary key)")
     return ds
@@ -1031,3 +1033,136 @@ async def test_view_table_token_can_access_table(perms_ds):
     cookies = {"ds_actor": perms_ds.client.actor_cookie(actor)}
     response = await perms_ds.client.get("/perms_ds_two/t1.json", cookies=cookies)
     assert response.status_code == 200
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "restrictions,verb,path,body,expected_status",
+    (
+        # No restrictions
+        (None, "get", "/.json", None, 200),
+        (None, "get", "/perms_ds_one.json", None, 200),
+        (None, "get", "/perms_ds_one/t1.json", None, 200),
+        (None, "get", "/perms_ds_one/t1/1.json", None, 200),
+        (None, "get", "/perms_ds_one/v1.json", None, 200),
+        # Restricted to just view-instance
+        ({"a": ["vi"]}, "get", "/.json", None, 200),
+        ({"a": ["vi"]}, "get", "/perms_ds_one.json", None, 403),
+        ({"a": ["vi"]}, "get", "/perms_ds_one/t1.json", None, 403),
+        ({"a": ["vi"]}, "get", "/perms_ds_one/t1/1.json", None, 403),
+        ({"a": ["vi"]}, "get", "/perms_ds_one/v1.json", None, 403),
+        # Restricted to just view-database
+        ({"a": ["vd"]}, "get", "/.json", None, 200),  # Can see instance too
+        ({"a": ["vd"]}, "get", "/perms_ds_one.json", None, 200),
+        ({"a": ["vd"]}, "get", "/perms_ds_one/t1.json", None, 403),
+        ({"a": ["vd"]}, "get", "/perms_ds_one/t1/1.json", None, 403),
+        ({"a": ["vd"]}, "get", "/perms_ds_one/v1.json", None, 403),
+        # Restricted to just view-table for specific database
+        (
+            {"d": {"perms_ds_one": ["vt"]}},
+            "get",
+            "/.json",
+            None,
+            200,
+        ),  # Can see instance
+        (
+            {"d": {"perms_ds_one": ["vt"]}},
+            "get",
+            "/perms_ds_one.json",
+            None,
+            200,
+        ),  # and this database
+        (
+            {"d": {"perms_ds_one": ["vt"]}},
+            "get",
+            "/perms_ds_two.json",
+            None,
+            403,
+        ),  # But not this one
+        (
+            # Can see the table
+            {"d": {"perms_ds_one": ["vt"]}},
+            "get",
+            "/perms_ds_one/t1.json",
+            None,
+            200,
+        ),
+        (
+            # And the view
+            {"d": {"perms_ds_one": ["vt"]}},
+            "get",
+            "/perms_ds_one/v1.json",
+            None,
+            200,
+        ),
+        # view-table access to a specific table
+        (
+            {"r": {"perms_ds_one": {"t1": ["vt"]}}},
+            "get",
+            "/.json",
+            None,
+            200,
+        ),
+        (
+            {"r": {"perms_ds_one": {"t1": ["vt"]}}},
+            "get",
+            "/perms_ds_one.json",
+            None,
+            200,
+        ),
+        (
+            {"r": {"perms_ds_one": {"t1": ["vt"]}}},
+            "get",
+            "/perms_ds_one/t1.json",
+            None,
+            200,
+        ),
+        # But cannot see the other table
+        (
+            {"r": {"perms_ds_one": {"t1": ["vt"]}}},
+            "get",
+            "/perms_ds_one/t2.json",
+            None,
+            403,
+        ),
+        # Or the view
+        (
+            {"r": {"perms_ds_one": {"t1": ["vt"]}}},
+            "get",
+            "/perms_ds_one/v1.json",
+            None,
+            403,
+        ),
+    ),
+)
+async def test_actor_restrictions(
+    perms_ds, restrictions, verb, path, body, expected_status
+):
+    actor = {"id": "user"}
+    if restrictions:
+        actor["_r"] = restrictions
+    method = getattr(perms_ds.client, verb)
+    kwargs = {"cookies": {"ds_actor": perms_ds.client.actor_cookie(actor)}}
+    if body:
+        kwargs["json"] = body
+    perms_ds._permission_checks.clear()
+    response = await method(path, **kwargs)
+    assert response.status_code == expected_status, json.dumps(
+        {
+            "verb": verb,
+            "path": path,
+            "body": body,
+            "restrictions": restrictions,
+            "expected_status": expected_status,
+            "response_status": response.status_code,
+            "checks": [
+                {
+                    "action": check["action"],
+                    "resource": check["resource"],
+                    "result": check["result"],
+                }
+                for check in perms_ds._permission_checks
+            ],
+        },
+        indent=2,
+    )