From 73014abe8b63182842208d72d61e785bbf84d084 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Sun, 26 Oct 2025 16:53:49 -0700
Subject: [PATCH] Improved permissions UI WIP

---
 datasette/app.py                              |   6 +-
 .../templates/_permissions_debug_tabs.html    |  53 +++++++++
 .../{actions.html => debug_actions.html}      |   5 +-
 datasette/templates/debug_allowed.html        |   9 +-
 datasette/templates/debug_check.html          |  98 +++++-----------
 ...html => debug_permissions_playground.html} |  63 ++++++----
 datasette/templates/debug_rules.html          |   4 +-
 datasette/views/special.py                    | 111 +++++++++---------
 tests/test_api.py                             |  10 +-
 tests/test_html.py                            |  57 ++++++++-
 10 files changed, 249 insertions(+), 167 deletions(-)
 create mode 100644 datasette/templates/_permissions_debug_tabs.html
 rename datasette/templates/{actions.html => debug_actions.html} (91%)
 rename datasette/templates/{permissions_debug.html => debug_permissions_playground.html} (67%)

diff --git a/datasette/app.py b/datasette/app.py
index f141ed45..bfbf2360 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -1835,7 +1835,11 @@ class Datasette:
         )
         add_route(
             JsonDataView.as_view(
-                self, "actions.json", self._actions, template="actions.html"
+                self,
+                "actions.json",
+                self._actions,
+                template="debug_actions.html",
+                permission="permissions-debug",
             ),
             r"/-/actions(\.(?P<format>json))?$",
         )
diff --git a/datasette/templates/_permissions_debug_tabs.html b/datasette/templates/_permissions_debug_tabs.html
new file mode 100644
index 00000000..ab8be1fb
--- /dev/null
+++ b/datasette/templates/_permissions_debug_tabs.html
@@ -0,0 +1,53 @@
+{% if has_debug_permission %}
+{% set query_string = '?' + request.query_string if request.query_string else '' %}
+
+<style>
+.permissions-debug-tabs {
+    border-bottom: 2px solid #e0e0e0;
+    margin-bottom: 2em;
+    display: flex;
+    flex-wrap: wrap;
+    gap: 0.5em;
+}
+.permissions-debug-tabs a {
+    padding: 0.75em 1.25em;
+    text-decoration: none;
+    color: #333;
+    border-bottom: 3px solid transparent;
+    margin-bottom: -2px;
+    transition: all 0.2s;
+    font-weight: 500;
+}
+.permissions-debug-tabs a:hover {
+    background-color: #f5f5f5;
+    border-bottom-color: #999;
+}
+.permissions-debug-tabs a.active {
+    color: #0066cc;
+    border-bottom-color: #0066cc;
+    background-color: #f0f7ff;
+}
+@media only screen and (max-width: 576px) {
+    .permissions-debug-tabs {
+        flex-direction: column;
+        gap: 0;
+    }
+    .permissions-debug-tabs a {
+        border-bottom: 1px solid #e0e0e0;
+        margin-bottom: 0;
+    }
+    .permissions-debug-tabs a.active {
+        border-left: 3px solid #0066cc;
+        border-bottom: 1px solid #e0e0e0;
+    }
+}
+</style>
+
+<nav class="permissions-debug-tabs">
+    <a href="{{ urls.path('-/permissions') }}" {% if current_tab == "permissions" %}class="active"{% endif %}>Playground</a>
+    <a href="{{ urls.path('-/check') }}{{ query_string }}" {% if current_tab == "check" %}class="active"{% endif %}>Check</a>
+    <a href="{{ urls.path('-/allowed') }}{{ query_string }}" {% if current_tab == "allowed" %}class="active"{% endif %}>Allowed</a>
+    <a href="{{ urls.path('-/rules') }}{{ query_string }}" {% if current_tab == "rules" %}class="active"{% endif %}>Rules</a>
+    <a href="{{ urls.path('-/actions') }}" {% if current_tab == "actions" %}class="active"{% endif %}>Actions</a>
+</nav>
+{% endif %}
diff --git a/datasette/templates/actions.html b/datasette/templates/debug_actions.html
similarity index 91%
rename from datasette/templates/actions.html
rename to datasette/templates/debug_actions.html
index b4285d79..6dd5ac0e 100644
--- a/datasette/templates/actions.html
+++ b/datasette/templates/debug_actions.html
@@ -3,7 +3,10 @@
 {% block title %}Registered Actions{% endblock %}
 
 {% block content %}
-<h1>Registered Actions</h1>
+<h1>Registered actions</h1>
+
+{% set current_tab = "actions" %}
+{% include "_permissions_debug_tabs.html" %}
 
 <p style="margin-bottom: 2em;">
   This Datasette instance has registered {{ data|length }} action{{ data|length != 1 and "s" or "" }}.
diff --git a/datasette/templates/debug_allowed.html b/datasette/templates/debug_allowed.html
index c3688e26..e3dc5250 100644
--- a/datasette/templates/debug_allowed.html
+++ b/datasette/templates/debug_allowed.html
@@ -9,8 +9,10 @@
 {% endblock %}
 
 {% block content %}
+<h1>Allowed resources</h1>
 
-<h1>Allowed Resources</h1>
+{% set current_tab = "allowed" %}
+{% include "_permissions_debug_tabs.html" %}
 
 <p>Use this tool to check which resources the current actor is allowed to access for a given permission action. It queries the <code>/-/allowed.json</code> API endpoint.</p>
 
@@ -225,9 +227,6 @@ function displayResults(data) {
 
     // Update raw JSON
     document.getElementById('raw-json').innerHTML = jsonFormatHighlight(data);
-
-    // Scroll to results
-    resultsContainer.scrollIntoView({ behavior: 'smooth', block: 'nearest' });
 }
 
 function displayError(data) {
@@ -238,8 +237,6 @@ function displayError(data) {
     resultsContent.innerHTML = `<div class="error-message">Error: ${escapeHtml(data.error || 'Unknown error')}</div>`;
 
     document.getElementById('raw-json').innerHTML = jsonFormatHighlight(data);
-
-    resultsContainer.scrollIntoView({ behavior: 'smooth', block: 'nearest' });
 }
 
 // Disable child input if parent is empty
diff --git a/datasette/templates/debug_check.html b/datasette/templates/debug_check.html
index 47fce5cb..da990985 100644
--- a/datasette/templates/debug_check.html
+++ b/datasette/templates/debug_check.html
@@ -4,35 +4,9 @@
 
 {% block extra_head %}
 <script src="{{ base_url }}-/static/json-format-highlight-1.0.1.js"></script>
+{% include "_permission_ui_styles.html" %}
 {% include "_debug_common_functions.html" %}
 <style>
-.form-section {
-    margin-bottom: 1em;
-}
-.form-section label {
-    display: block;
-    margin-bottom: 0.3em;
-    font-weight: bold;
-}
-.form-section input[type="text"],
-.form-section select {
-    width: 100%;
-    max-width: 500px;
-    padding: 0.5em;
-    box-sizing: border-box;
-    border: 1px solid #ccc;
-    border-radius: 3px;
-}
-.form-section input[type="text"]:focus,
-.form-section select:focus {
-    outline: 2px solid #0066cc;
-    border-color: #0066cc;
-}
-.form-section small {
-    display: block;
-    margin-top: 0.3em;
-    color: #666;
-}
 #output {
     margin-top: 2em;
     padding: 1em;
@@ -74,28 +48,14 @@
 .details-section dd {
     margin-left: 1em;
 }
-#submit-btn {
-    padding: 0.6em 1.5em;
-    font-size: 1em;
-    background-color: #0066cc;
-    color: white;
-    border: none;
-    border-radius: 3px;
-    cursor: pointer;
-}
-#submit-btn:hover {
-    background-color: #0052a3;
-}
-#submit-btn:disabled {
-    background-color: #ccc;
-    cursor: not-allowed;
-}
 </style>
 {% endblock %}
 
 {% block content %}
+<h1>Permission check</h1>
 
-<h1>Permission Check</h1>
+{% set current_tab = "check" %}
+{% include "_permissions_debug_tabs.html" %}
 
 <p>Use this tool to test permission checks for the current actor. It queries the <code>/-/check.json</code> API endpoint.</p>
 
@@ -105,32 +65,36 @@
 <p>Current actor: <strong>anonymous (not logged in)</strong></p>
 {% endif %}
 
-<form id="check-form" class="core">
-    <div class="form-section">
-        <label for="action">Action (permission name):</label>
-        <select id="action" name="action" required>
-            <option value="">Select an action...</option>
-            {% for action_name in sorted_actions %}
-            <option value="{{ action_name }}">{{ action_name }}</option>
-            {% endfor %}
-        </select>
-        <small>The permission action to check</small>
-    </div>
+<div class="permission-form">
+    <form id="check-form">
+        <div class="form-section">
+            <label for="action">Action (permission name):</label>
+            <select id="action" name="action" required>
+                <option value="">Select an action...</option>
+                {% for action_name in sorted_actions %}
+                <option value="{{ action_name }}">{{ action_name }}</option>
+                {% endfor %}
+            </select>
+            <small>The permission action to check</small>
+        </div>
 
-    <div class="form-section">
-        <label for="parent">Parent resource (optional):</label>
-        <input type="text" id="parent" name="parent" placeholder="e.g., database name">
-        <small>For database-level permissions, specify the database name</small>
-    </div>
+        <div class="form-section">
+            <label for="parent">Parent resource (optional):</label>
+            <input type="text" id="parent" name="parent" placeholder="e.g., database name">
+            <small>For database-level permissions, specify the database name</small>
+        </div>
 
-    <div class="form-section">
-        <label for="child">Child resource (optional):</label>
-        <input type="text" id="child" name="child" placeholder="e.g., table name">
-        <small>For table-level permissions, specify the table name (requires parent)</small>
-    </div>
+        <div class="form-section">
+            <label for="child">Child resource (optional):</label>
+            <input type="text" id="child" name="child" placeholder="e.g., table name">
+            <small>For table-level permissions, specify the table name (requires parent)</small>
+        </div>
 
-    <button type="submit" id="submit-btn">Check Permission</button>
-</form>
+        <div class="form-actions">
+            <button type="submit" class="submit-btn" id="submit-btn">Check Permission</button>
+        </div>
+    </form>
+</div>
 
 <div id="output" style="display: none;">
     <h2>Result: <span class="result-badge" id="result-badge"></span></h2>
diff --git a/datasette/templates/permissions_debug.html b/datasette/templates/debug_permissions_playground.html
similarity index 67%
rename from datasette/templates/permissions_debug.html
rename to datasette/templates/debug_permissions_playground.html
index 1872da33..9b54df1f 100644
--- a/datasette/templates/permissions_debug.html
+++ b/datasette/templates/debug_permissions_playground.html
@@ -3,6 +3,7 @@
 {% block title %}Debug permissions{% endblock %}
 
 {% block extra_head %}
+{% include "_permission_ui_styles.html" %}
 <style type="text/css">
 .check-result-true {
     color: green;
@@ -42,32 +43,46 @@ textarea {
 {% endblock %}
 
 {% block content %}
+<h1>Permission playground</h1>
 
-<h1>Permission check testing tool</h1>
+{% set current_tab = "permissions" %}
+{% include "_permissions_debug_tabs.html" %}
 
 <p>This tool lets you simulate an actor and a permission check for that actor.</p>
 
-<form class="core" action="{{ urls.path('-/permissions') }}" id="debug-post" method="post" style="margin-bottom: 1em">
-    <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
-    <div class="two-col">
-        <p><label>Actor</label></p>
-        <textarea name="actor">{% if actor_input %}{{ actor_input }}{% else %}{"id": "root"}{% endif %}</textarea>
-    </div>
-    <div class="two-col" style="vertical-align: top">
-        <p><label for="permission" style="display:block">Permission</label>
-        <select name="permission" id="permission">
-            {% for permission in permissions %}
-                <option value="{{ permission.name }}">{{ permission.name }}</option>
-            {% endfor %}
-        </select>
-        <p><label for="resource_1">Database name</label><input type="text" id="resource_1" name="resource_1"></p>
-        <p><label for="resource_2">Table or query name</label><input type="text" id="resource_2" name="resource_2"></p>
-    </div>
-    <div style="margin-top: 1em;">
-        <input type="submit" value="Simulate permission check">
-    </div>
-    <pre style="margin-top: 1em" id="debugResult"></pre>
-</form>
+<div class="permission-form">
+    <form action="{{ urls.path('-/permissions') }}" id="debug-post" method="post">
+        <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
+        <div class="two-col">
+            <div class="form-section">
+                <label>Actor</label>
+                <textarea name="actor">{% if actor_input %}{{ actor_input }}{% else %}{"id": "root"}{% endif %}</textarea>
+            </div>
+        </div>
+        <div class="two-col" style="vertical-align: top">
+            <div class="form-section">
+                <label for="permission">Action</label>
+                <select name="permission" id="permission">
+                    {% for permission in permissions %}
+                        <option value="{{ permission.name }}">{{ permission.name }}</option>
+                    {% endfor %}
+                </select>
+            </div>
+            <div class="form-section">
+                <label for="resource_1">Parent</label>
+                <input type="text" id="resource_1" name="resource_1" placeholder="e.g., database name">
+            </div>
+            <div class="form-section">
+                <label for="resource_2">Child</label>
+                <input type="text" id="resource_2" name="resource_2" placeholder="e.g., table name">
+            </div>
+        </div>
+        <div class="form-actions">
+            <button type="submit" class="submit-btn">Simulate permission check</button>
+        </div>
+        <pre style="margin-top: 1em" id="debugResult"></pre>
+    </form>
+</div>
 
 <script>
 var rawPerms = {{ permissions|tojson }};
@@ -145,8 +160,4 @@ debugPost.addEventListener('submit', function(ev) {
     </div>
 {% endfor %}
 
-<h1>All registered permissions</h1>
-
-<pre>{{ permissions|tojson(2) }}</pre>
-
 {% endblock %}
diff --git a/datasette/templates/debug_rules.html b/datasette/templates/debug_rules.html
index a89303d2..82f35444 100644
--- a/datasette/templates/debug_rules.html
+++ b/datasette/templates/debug_rules.html
@@ -9,8 +9,10 @@
 {% endblock %}
 
 {% block content %}
+<h1>Permission rules</h1>
 
-<h1>Permission Rules</h1>
+{% set current_tab = "rules" %}
+{% include "_permissions_debug_tabs.html" %}
 
 <p>Use this tool to view the permission rules that allow the current actor to access resources for a given permission action. It queries the <code>/-/rules.json</code> API endpoint.</p>
 
diff --git a/datasette/views/special.py b/datasette/views/special.py
index 3cc83ba0..2f093f0c 100644
--- a/datasette/views/special.py
+++ b/datasette/views/special.py
@@ -62,14 +62,18 @@ class JsonDataView(BaseView):
                 add_cors_headers(headers)
             return Response.json(data, headers=headers)
         else:
+            context = {
+                "filename": self.filename,
+                "data": data,
+                "data_json": json.dumps(data, indent=4, default=repr),
+            }
+            # Add has_debug_permission if this view requires permissions-debug
+            if self.permission == "permissions-debug":
+                context["has_debug_permission"] = True
             return await self.render(
                 [self.template],
                 request=request,
-                context={
-                    "filename": self.filename,
-                    "data": data,
-                    "data_json": json.dumps(data, indent=4, default=repr),
-                },
+                context=context,
             )
 
 
@@ -150,12 +154,13 @@ class PermissionsDebugView(BaseView):
                 if (check.actor or {}).get("id") == request.actor["id"]
             ]
         return await self.render(
-            ["permissions_debug.html"],
+            ["debug_permissions_playground.html"],
             request,
             # list() avoids error if check is performed during template render:
             {
                 "permission_checks": permission_checks,
                 "filter": filter_,
+                "has_debug_permission": True,
                 "permissions": [
                     {
                         "name": p.name,
@@ -415,6 +420,7 @@ class PermissionRulesView(BaseView):
                 request,
                 {
                     "sorted_actions": sorted(self.ds.actions.keys()),
+                    "has_debug_permission": True,
                 },
             )
 
@@ -519,6 +525,44 @@ class PermissionRulesView(BaseView):
         return Response.json(response, headers=headers)
 
 
+async def _check_permission_for_actor(ds, action, parent, child, actor):
+    """Shared logic for checking permissions. Returns a dict with check results."""
+    if action not in ds.actions:
+        return {"error": f"Unknown action: {action}"}, 404
+
+    if child and not parent:
+        return {"error": "parent is required when child is provided"}, 400
+
+    # Use the action's properties to create the appropriate resource object
+    action_obj = ds.actions.get(action)
+    if not action_obj:
+        return {"error": f"Unknown action: {action}"}, 400
+
+    if action_obj.takes_parent and action_obj.takes_child:
+        resource_obj = action_obj.resource_class(database=parent, table=child)
+    elif action_obj.takes_parent:
+        resource_obj = action_obj.resource_class(database=parent)
+    else:
+        resource_obj = action_obj.resource_class()
+
+    allowed = await ds.allowed(action=action, resource=resource_obj, actor=actor)
+
+    response = {
+        "action": action,
+        "allowed": bool(allowed),
+        "resource": {
+            "parent": parent,
+            "child": child,
+            "path": _resource_path(parent, child),
+        },
+    }
+
+    if actor and "id" in actor:
+        response["actor_id"] = actor["id"]
+
+    return response, 200
+
+
 class PermissionCheckView(BaseView):
     name = "permission_check"
     has_json_alternate = False
@@ -533,6 +577,7 @@ class PermissionCheckView(BaseView):
                 request,
                 {
                     "sorted_actions": sorted(self.ds.actions.keys()),
+                    "has_debug_permission": True,
                 },
             )
 
@@ -540,62 +585,14 @@ class PermissionCheckView(BaseView):
         action = request.args.get("action")
         if not action:
             return Response.json({"error": "action parameter is required"}, status=400)
-        if action not in self.ds.actions:
-            return Response.json({"error": f"Unknown action: {action}"}, status=404)
 
         parent = request.args.get("parent")
         child = request.args.get("child")
-        if child and not parent:
-            return Response.json(
-                {"error": "parent is required when child is provided"}, status=400
-            )
 
-        # Use the action's properties to create the appropriate resource object
-        action_obj = self.ds.actions.get(action)
-        if not action_obj:
-            return Response.json({"error": f"Unknown action: {action}"}, status=400)
-
-        if action_obj.takes_parent and action_obj.takes_child:
-            resource_obj = action_obj.resource_class(database=parent, table=child)
-            resource = (parent, child)
-        elif action_obj.takes_parent:
-            resource_obj = action_obj.resource_class(database=parent)
-            resource = parent
-        else:
-            resource_obj = action_obj.resource_class()
-            resource = None
-
-        before_checks = len(self.ds._permission_checks)
-        allowed = await self.ds.allowed(
-            action=action, resource=resource_obj, actor=request.actor
+        response, status = await _check_permission_for_actor(
+            self.ds, action, parent, child, request.actor
         )
-
-        info = None
-        if len(self.ds._permission_checks) > before_checks:
-            for check in reversed(self.ds._permission_checks):
-                if (
-                    check.actor == request.actor
-                    and check.action == action
-                    and check.parent == parent
-                    and check.child == child
-                ):
-                    info = check
-                    break
-
-        response = {
-            "action": action,
-            "allowed": bool(allowed),
-            "resource": {
-                "parent": parent,
-                "child": child,
-                "path": _resource_path(parent, child),
-            },
-        }
-
-        if request.actor and "id" in request.actor:
-            response["actor_id"] = request.actor["id"]
-
-        return Response.json(response)
+        return Response.json(response, status=status)
 
 
 class AllowDebugView(BaseView):
diff --git a/tests/test_api.py b/tests/test_api.py
index 7c8a8300..2ac647c7 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -835,8 +835,14 @@ async def test_versions_json(ds_client):
 
 @pytest.mark.asyncio
 async def test_actions_json(ds_client):
-    response = await ds_client.get("/-/actions.json")
-    data = response.json()
+    original_root_enabled = ds_client.ds.root_enabled
+    try:
+        ds_client.ds.root_enabled = True
+        cookies = {"ds_actor": ds_client.actor_cookie({"id": "root"})}
+        response = await ds_client.get("/-/actions.json", cookies=cookies)
+        data = response.json()
+    finally:
+        ds_client.ds.root_enabled = original_root_enabled
     assert isinstance(data, list)
     assert len(data) > 0
     # Check structure of first action
diff --git a/tests/test_html.py b/tests/test_html.py
index 7fade152..dbe993c4 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -1180,9 +1180,54 @@ async def test_custom_csrf_error(ds_client):
 
 @pytest.mark.asyncio
 async def test_actions_page(ds_client):
-    response = await ds_client.get("/-/actions")
-    assert response.status_code == 200
-    assert "Registered Actions" in response.text
-    assert "<th>Name</th>" in response.text
-    assert "view-instance" in response.text
-    assert "view-database" in response.text
+    original_root_enabled = ds_client.ds.root_enabled
+    try:
+        ds_client.ds.root_enabled = True
+        cookies = {"ds_actor": ds_client.actor_cookie({"id": "root"})}
+        response = await ds_client.get("/-/actions", cookies=cookies)
+        assert response.status_code == 200
+        assert "Registered actions" in response.text
+        assert "<th>Name</th>" in response.text
+        assert "view-instance" in response.text
+        assert "view-database" in response.text
+    finally:
+        ds_client.ds.root_enabled = original_root_enabled
+
+
+@pytest.mark.asyncio
+async def test_permission_debug_tabs_with_query_string(ds_client):
+    """Test that navigation tabs persist query strings across Check, Allowed, and Rules pages"""
+    original_root_enabled = ds_client.ds.root_enabled
+    try:
+        ds_client.ds.root_enabled = True
+        cookies = {"ds_actor": ds_client.actor_cookie({"id": "root"})}
+
+        # Test /-/allowed with query string
+        response = await ds_client.get(
+            "/-/allowed?action=view-table&page_size=50", cookies=cookies
+        )
+        assert response.status_code == 200
+        # Check that Rules and Check tabs have the query string
+        assert 'href="/-/rules?action=view-table&amp;page_size=50"' in response.text
+        assert 'href="/-/check?action=view-table&amp;page_size=50"' in response.text
+        # Playground and Actions should not have query string
+        assert 'href="/-/permissions"' in response.text
+        assert 'href="/-/actions"' in response.text
+
+        # Test /-/rules with query string
+        response = await ds_client.get(
+            "/-/rules?action=view-database&parent=test", cookies=cookies
+        )
+        assert response.status_code == 200
+        # Check that Allowed and Check tabs have the query string
+        assert 'href="/-/allowed?action=view-database&amp;parent=test"' in response.text
+        assert 'href="/-/check?action=view-database&amp;parent=test"' in response.text
+
+        # Test /-/check with query string
+        response = await ds_client.get("/-/check?action=execute-sql", cookies=cookies)
+        assert response.status_code == 200
+        # Check that Allowed and Rules tabs have the query string
+        assert 'href="/-/allowed?action=execute-sql"' in response.text
+        assert 'href="/-/rules?action=execute-sql"' in response.text
+    finally:
+        ds_client.ds.root_enabled = original_root_enabled