Cascading view permissions, closes #832

- If you have table permission but not database permission you can now view the table page
- New BaseView.check_permissions() method

datasette/views/base.py

@@ -69,6 +69,29 @@ class BaseView:
         if not ok:
             raise Forbidden(action)
 
+    async def check_permissions(self, request, permissions):
+        "permissions is a list of (action, resource) tuples or 'action' strings"
+        for permission in permissions:
+            if isinstance(permission, str):
+                action = permission
+                resource = None
+            elif isinstance(permission, (tuple, list)) and len(permission) == 2:
+                action, resource = permission
+            else:
+                assert (
+                    False
+                ), "permission should be string or tuple of two items: {}".format(
+                    repr(permission)
+                )
+            ok = await self.ds.permission_allowed(
+                request.actor, action, resource=resource, default=None,
+            )
+            if ok is not None:
+                if ok:
+                    return
+                else:
+                    raise Forbidden(action)
+
     def database_url(self, database):
         db = self.ds.databases[database]
         base_url = self.ds.config("base_url")

datasette/views/database.py

@@ -20,8 +20,9 @@ class DatabaseView(DataView):
     name = "database"
 
     async def data(self, request, database, hash, default_labels=False, _size=None):
-        await self.check_permission(request, "view-instance")
-        await self.check_permission(request, "view-database", database)
+        await self.check_permissions(
+            request, [("view-database", database), "view-instance",],
+        )
         metadata = (self.ds.metadata("databases") or {}).get(database, {})
         self.ds.update_with_inherited_metadata(metadata)
 
@@ -88,7 +89,7 @@ class DatabaseView(DataView):
                 "views": views,
                 "queries": canned_queries,
                 "private": not await self.ds.permission_allowed(
-                    None, "view-database", database
+                    None, "view-database", database, default=True
                 ),
                 "allow_execute_sql": await self.ds.permission_allowed(
                     request.actor, "execute-sql", database, default=True
@@ -150,17 +151,23 @@ class QueryView(DataView):
         if "_shape" in params:
             params.pop("_shape")
 
-        # Respect canned query permissions
-        await self.check_permission(request, "view-instance")
-        await self.check_permission(request, "view-database", database)
         private = False
         if canned_query:
-            await self.check_permission(request, "view-query", (database, canned_query))
+            # Respect canned query permissions
+            await self.check_permissions(
+                request,
+                [
+                    ("view-query", (database, canned_query)),
+                    ("view-database", database),
+                    "view-instance",
+                ],
+            )
             private = not await self.ds.permission_allowed(
                 None, "view-query", (database, canned_query), default=True
             )
         else:
             await self.check_permission(request, "execute-sql", database)
+
         # Extract any :named parameters
         named_parameters = named_parameters or self.re_named_parameter.findall(sql)
         named_parameter_values = {

datasette/views/table.py

@@ -269,9 +269,14 @@ class TableView(RowTableShared):
         if not is_view and not table_exists:
             raise NotFound("Table not found: {}".format(table))
 
-        await self.check_permission(request, "view-instance")
-        await self.check_permission(request, "view-database", database)
-        await self.check_permission(request, "view-table", (database, table))
+        await self.check_permissions(
+            request,
+            [
+                ("view-table", (database, table)),
+                ("view-database", database),
+                "view-instance",
+            ],
+        )
 
         private = not await self.ds.permission_allowed(
             None, "view-table", (database, table), default=True

tests/fixtures.py

@@ -462,7 +462,9 @@ METADATA = {
             "queries": {
                 "𝐜𝐢𝐭𝐢𝐞𝐬": "select id, name from facet_cities order by id limit 1;",
                 "pragma_cache_size": "PRAGMA cache_size;",
-                "magic_parameters": "select :_header_user_agent as user_agent, :_now_datetime_utc as datetime",
+                "magic_parameters": {
+                    "sql": "select :_header_user_agent as user_agent, :_now_datetime_utc as datetime",
+                },
                 "neighborhood_search": {
                     "sql": textwrap.dedent(
                         """

tests/test_permissions.py

@@ -1,5 +1,6 @@
 from .fixtures import app_client, assert_permissions_checked, make_app_client
 from bs4 import BeautifulSoup as Soup
+import copy
 import pytest
 
 
@@ -43,7 +44,7 @@ def test_view_database(allow, expected_anon, expected_auth):
             "/fixtures/compound_three_primary_keys/a,a,a",
         ):
             anon_response = client.get(path)
-            assert expected_anon == anon_response.status
+            assert expected_anon == anon_response.status, path
             if allow and path == "/fixtures" and anon_response.status == 200:
                 # Should be no padlock
                 assert ">fixtures 🔒</h1>" not in anon_response.text
@@ -348,3 +349,61 @@ def test_view_instance(path, view_instance_client):
     assert 403 == view_instance_client.get(path).status
     if path not in ("/-/permissions", "/-/messages", "/-/patterns"):
         assert 403 == view_instance_client.get(path + ".json").status
+
+
+@pytest.fixture(scope="session")
+def cascade_app_client():
+    with make_app_client() as client:
+        yield client
+
+
+@pytest.mark.parametrize(
+    "path,expected_status,permissions",
+    [
+        ("/", 403, []),
+        ("/", 200, ["instance"]),
+        # Can view table even if not allowed database or instance
+        ("/fixtures/facet_cities", 403, []),
+        ("/fixtures/facet_cities", 403, ["database"]),
+        ("/fixtures/facet_cities", 403, ["instance"]),
+        ("/fixtures/facet_cities", 200, ["table"]),
+        ("/fixtures/facet_cities", 200, ["table", "database"]),
+        ("/fixtures/facet_cities", 200, ["table", "database", "instance"]),
+        # Can view query even if not allowed database or instance
+        ("/fixtures/magic_parameters", 403, []),
+        ("/fixtures/magic_parameters", 403, ["database"]),
+        ("/fixtures/magic_parameters", 403, ["instance"]),
+        ("/fixtures/magic_parameters", 200, ["query"]),
+        ("/fixtures/magic_parameters", 200, ["query", "database"]),
+        ("/fixtures/magic_parameters", 200, ["query", "database", "instance"]),
+        # Can view database even if not allowed instance
+        ("/fixtures", 403, []),
+        ("/fixtures", 403, ["instance"]),
+        ("/fixtures", 200, ["database"]),
+    ],
+)
+def test_permissions_cascade(cascade_app_client, path, expected_status, permissions):
+    "Test that e.g. having view-table but NOT view-database lets you view table page, etc"
+    allow = {"id": "*"}
+    deny = {}
+    previous_metadata = cascade_app_client.ds._metadata
+    updated_metadata = copy.deepcopy(previous_metadata)
+    try:
+        # Set up the different allow blocks
+        updated_metadata["allow"] = allow if "instance" in permissions else deny
+        updated_metadata["databases"]["fixtures"]["allow"] = (
+            allow if "database" in permissions else deny
+        )
+        updated_metadata["databases"]["fixtures"]["tables"]["facet_cities"]["allow"] = (
+            allow if "table" in permissions else deny
+        )
+        updated_metadata["databases"]["fixtures"]["queries"]["magic_parameters"][
+            "allow"
+        ] = (allow if "query" in permissions else deny)
+        cascade_app_client.ds._metadata = updated_metadata
+        response = cascade_app_client.get(
+            path, cookies={"ds_actor": cascade_app_client.actor_cookie({"id": "test"})},
+        )
+        assert expected_status == response.status
+    finally:
+        cascade_app_client.ds._metadata = previous_metadata