From bcc781f4c50a8870e3389c4e60acb625c34b0317 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Thu, 3 Nov 2022 17:12:23 -0700
Subject: [PATCH] Implementation and tests for _r field on actor, refs #1855

New mechanism for restricting permissions further for a given actor.

This still needs documentation. It will eventually be used by the mechanism to issue
signed API tokens that are only able to perform a subset of actions.

This also adds tests that exercise the POST /-/permissions tool, refs #1881
---
 datasette/default_permissions.py |  42 ++++++++++++-
 datasette/views/special.py       |   2 +
 tests/test_permissions.py        | 100 +++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py
index 32b0c758..0964d536 100644
--- a/datasette/default_permissions.py
+++ b/datasette/default_permissions.py
@@ -6,8 +6,8 @@ import json
 import time
 
 
-@hookimpl(tryfirst=True)
-def permission_allowed(datasette, actor, action, resource):
+@hookimpl(tryfirst=True, specname="permission_allowed")
+def permission_allowed_default(datasette, actor, action, resource):
     async def inner():
         if action in (
             "permissions-debug",
@@ -57,6 +57,44 @@ def permission_allowed(datasette, actor, action, resource):
     return inner
 
 
+@hookimpl(specname="permission_allowed")
+def permission_allowed_actor_restrictions(actor, action, resource):
+    if actor is None:
+        return None
+    if "_r" not in actor:
+        # No restrictions, so we have no opinion
+        return None
+    _r = actor.get("_r")
+    action_initials = "".join([word[0] for word in action.split("-")])
+    # If _r 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
+    all_allowed = _r.get("a")
+    if all_allowed is not None:
+        assert isinstance(all_allowed, list)
+        if action_initials in all_allowed:
+            return None
+    # How about for the current database?
+    if action in ("view-database", "view-database-download", "execute-sql"):
+        database_allowed = _r.get("d", {}).get(resource)
+        if database_allowed is not None:
+            assert isinstance(database_allowed, list)
+            if action_initials in database_allowed:
+                return None
+    # Or the current table? That's any time the resource is (database, table)
+    if not isinstance(resource, str) and len(resource) == 2:
+        database, table = resource
+        table_allowed = _r.get("t", {}).get(database, {}).get(table)
+        # TODO: What should this do for canned queries?
+        if table_allowed is not None:
+            assert isinstance(table_allowed, list)
+            if action_initials in table_allowed:
+                return None
+    # This action is not specifically allowed, so reject it
+    return False
+
+
 @hookimpl
 def actor_from_request(datasette, request):
     prefix = "dstok_"
diff --git a/datasette/views/special.py b/datasette/views/special.py
index ff014246..79e9da72 100644
--- a/datasette/views/special.py
+++ b/datasette/views/special.py
@@ -126,6 +126,8 @@ class PermissionsDebugView(BaseView):
         if resource_2:
             resource.append(resource_2)
         resource = tuple(resource)
+        if len(resource) == 1:
+            resource = resource[0]
         result = await self.ds.permission_allowed(
             actor, permission, resource, default="USE_DEFAULT"
         )
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 8812d0f7..a45a0fb1 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -1,6 +1,9 @@
+from datasette.app import Datasette
 from .fixtures import app_client, assert_permissions_checked, make_app_client
 from bs4 import BeautifulSoup as Soup
 import copy
+import json
+import pytest_asyncio
 import pytest
 import urllib
 
@@ -19,6 +22,18 @@ def padlock_client():
         yield client
 
 
+@pytest_asyncio.fixture
+async def perms_ds():
+    ds = Datasette()
+    await ds.invoke_startup()
+    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("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
+
+
 @pytest.mark.parametrize(
     "allow,expected_anon,expected_auth",
     [
@@ -527,3 +542,88 @@ def test_padlocks_on_database_page(cascade_app_client):
         assert ">simple_view</a></li>" in response.text
     finally:
         cascade_app_client.ds._metadata_local = previous_metadata
+
+
+DEF = "USE_DEFAULT"
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "actor,permission,resource_1,resource_2,expected_result",
+    (
+        # Without restrictions the defaults apply
+        ({"id": "t"}, "view-instance", None, None, DEF),
+        ({"id": "t"}, "view-database", "one", None, DEF),
+        ({"id": "t"}, "view-table", "one", "t1", DEF),
+        # If there is an _r block, everything gets denied unless explicitly allowed
+        ({"id": "t", "_r": {}}, "view-instance", None, None, False),
+        ({"id": "t", "_r": {}}, "view-database", "one", None, False),
+        ({"id": "t", "_r": {}}, "view-table", "one", "t1", False),
+        # Explicit allowing works at the "a" for all level:
+        ({"id": "t", "_r": {"a": ["vi"]}}, "view-instance", None, None, DEF),
+        ({"id": "t", "_r": {"a": ["vd"]}}, "view-database", "one", None, DEF),
+        ({"id": "t", "_r": {"a": ["vt"]}}, "view-table", "one", "t1", DEF),
+        # But not if it's the wrong permission
+        ({"id": "t", "_r": {"a": ["vd"]}}, "view-instance", None, None, False),
+        ({"id": "t", "_r": {"a": ["vi"]}}, "view-database", "one", None, False),
+        ({"id": "t", "_r": {"a": ["vd"]}}, "view-table", "one", "t1", False),
+        # Works at the "d" for database level:
+        ({"id": "t", "_r": {"d": {"one": ["vd"]}}}, "view-database", "one", None, DEF),
+        (
+            {"id": "t", "_r": {"d": {"one": ["vdd"]}}},
+            "view-database-download",
+            "one",
+            None,
+            DEF,
+        ),
+        ({"id": "t", "_r": {"d": {"one": ["es"]}}}, "execute-sql", "one", None, DEF),
+        # Works at the "t" for table level:
+        (
+            {"id": "t", "_r": {"t": {"one": {"t1": ["vt"]}}}},
+            "view-table",
+            "one",
+            "t1",
+            DEF,
+        ),
+        (
+            {"id": "t", "_r": {"t": {"one": {"t1": ["vt"]}}}},
+            "view-table",
+            "one",
+            "t2",
+            False,
+        ),
+    ),
+)
+async def test_actor_restricted_permissions(
+    perms_ds, actor, permission, resource_1, resource_2, expected_result
+):
+    cookies = {"ds_actor": perms_ds.sign({"a": {"id": "root"}}, "actor")}
+    csrftoken = (await perms_ds.client.get("/-/permissions", cookies=cookies)).cookies[
+        "ds_csrftoken"
+    ]
+    cookies["ds_csrftoken"] = csrftoken
+    response = await perms_ds.client.post(
+        "/-/permissions",
+        data={
+            "actor": json.dumps(actor),
+            "permission": permission,
+            "resource_1": resource_1,
+            "resource_2": resource_2,
+            "csrftoken": csrftoken,
+        },
+        cookies=cookies,
+    )
+    expected_resource = []
+    if resource_1:
+        expected_resource.append(resource_1)
+    if resource_2:
+        expected_resource.append(resource_2)
+    if len(expected_resource) == 1:
+        expected_resource = expected_resource[0]
+    expected = {
+        "actor": actor,
+        "permission": permission,
+        "resource": expected_resource,
+        "result": expected_result,
+    }
+    assert response.json() == expected