From e238df3959a0e32837ac8d5c3e49ecbcfe394de4 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Wed, 14 Dec 2022 12:04:23 -0800
Subject: [PATCH] Handle non-initials in permission_allowed_actor_restrictions,
 closes #1956

---
 datasette/default_permissions.py | 16 +++++++++++-----
 tests/test_permissions.py        | 16 ++++++++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py
index e94014e7..f9f52e1a 100644
--- a/datasette/default_permissions.py
+++ b/datasette/default_permissions.py
@@ -173,14 +173,20 @@ async def _resolve_metadata_view_permissions(datasette, actor, action, resource)
 
 
 @hookimpl(specname="permission_allowed")
-def permission_allowed_actor_restrictions(actor, action, resource):
+def permission_allowed_actor_restrictions(datasette, 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("-")])
+
+    # Does this action have an abbreviation?
+    to_check = {action}
+    permission = datasette.permissions.get(action)
+    if permission and permission.abbr:
+        to_check.add(permission.abbr)
+
     # 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
@@ -188,14 +194,14 @@ def permission_allowed_actor_restrictions(actor, action, resource):
     all_allowed = _r.get("a")
     if all_allowed is not None:
         assert isinstance(all_allowed, list)
-        if action_initials in all_allowed:
+        if to_check.intersection(all_allowed):
             return None
     # How about for the current database?
     if isinstance(resource, str):
         database_allowed = _r.get("d", {}).get(resource)
         if database_allowed is not None:
             assert isinstance(database_allowed, list)
-            if action_initials in database_allowed:
+            if to_check.intersection(database_allowed):
                 return None
     # Or the current table? That's any time the resource is (database, table)
     if resource is not None and not isinstance(resource, str) and len(resource) == 2:
@@ -204,7 +210,7 @@ def permission_allowed_actor_restrictions(actor, action, resource):
         # 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:
+            if to_check.intersection(table_allowed):
                 return None
     # This action is not specifically allowed, so reject it
     return False
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 1ed82e30..d39b0634 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -609,6 +609,22 @@ DEF = "USE_DEFAULT"
             "t2",
             False,
         ),
+        # non-abbreviations should work too
+        ({"id": "t", "_r": {"a": ["view-instance"]}}, "view-instance", None, None, DEF),
+        (
+            {"id": "t", "_r": {"d": {"one": ["view-database"]}}},
+            "view-database",
+            "one",
+            None,
+            DEF,
+        ),
+        (
+            {"id": "t", "_r": {"r": {"one": {"t1": ["view-table"]}}}},
+            "view-table",
+            "one",
+            "t1",
+            DEF,
+        ),
     ),
 )
 async def test_actor_restricted_permissions(