From 974c803b987a6386be6a201b5f07961f147810f8 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Fri, 22 Dec 2023 17:04:01 -0800
Subject: [PATCH] edit-schema-drop-table, refs #22

---
 README.md                                     |  1 +
 datasette_edit_schema/__init__.py             | 21 +++++-
 .../templates/edit_schema_table.html          | 16 ++--
 tests/test_edit_schema.py                     | 74 +++++++++++++++++--
 4 files changed, 96 insertions(+), 16 deletions(-)

diff --git a/README.md b/README.md
index 56592f7..d4219e4 100644
--- a/README.md
+++ b/README.md
@@ -50,6 +50,7 @@ You can instead use more finely-grained permissions.
 
 - `edit-schema-create-table` allows users to create a new table. The `resource` will be the name of the database.
 - `edit-schema-alter-table` allows users to alter the schema of a table. The `resource` will be a tuple of `(database_name, table_name)`.
+- `edit-schema-drop-table` allows users to drop a table. The `resource` will be a tuple of `(database_name, table_name)`. This permission will not work on its own, you need to grant the user `edit-schema-alter-table` as well.
 
 ## Screenshot
 
diff --git a/datasette_edit_schema/__init__.py b/datasette_edit_schema/__init__.py
index c2c0820..4d35416 100644
--- a/datasette_edit_schema/__init__.py
+++ b/datasette_edit_schema/__init__.py
@@ -61,7 +61,7 @@ async def can_alter_table(datasette, actor, database, table):
         actor, "edit-schema", resource=database, default=False
     ):
         return True
-    # Or maybe they have edit-schema-create-table
+    # Or maybe they have edit-schema-alter-table
     if await datasette.permission_allowed(
         actor, "edit-schema-alter-table", resource=(database, table), default=False
     ):
@@ -69,6 +69,19 @@ async def can_alter_table(datasette, actor, database, table):
     return False
 
 
+async def can_drop_table(datasette, actor, database, table):
+    if await datasette.permission_allowed(
+        actor, "edit-schema", resource=database, default=False
+    ):
+        return True
+    # Or maybe they have edit-schema-drop-table
+    if await datasette.permission_allowed(
+        actor, "edit-schema-drop-table", resource=(database, table), default=False
+    ):
+        return True
+    return False
+
+
 @hookimpl
 def database_actions(datasette, actor, database):
     async def inner():
@@ -547,6 +560,9 @@ def get_columns_and_schema_and_fks_and_pks_and_indexes(conn):
                 "current_pk": pks[0] if len(pks) == 1 else None,
                 "existing_indexes": existing_indexes,
                 "non_primary_key_columns": non_primary_key_columns,
+                "can_drop_table": await can_drop_table(
+                    datasette, request.actor, database_name, table
+                ),
             },
             request=request,
         )
@@ -554,6 +570,9 @@ def get_columns_and_schema_and_fks_and_pks_and_indexes(conn):
 
 
 async def drop_table(request, datasette, database, table):
+    if not await can_drop_table(datasette, request.actor, database.name, table):
+        raise Forbidden("Permission denied for edit-schema-drop-table")
+
     def do_drop_table(conn):
         db = sqlite_utils.Database(conn)
         db[table].disable_fts()
diff --git a/datasette_edit_schema/templates/edit_schema_table.html b/datasette_edit_schema/templates/edit_schema_table.html
index f136e1e..d3727ef 100644
--- a/datasette_edit_schema/templates/edit_schema_table.html
+++ b/datasette_edit_schema/templates/edit_schema_table.html
@@ -224,13 +224,15 @@ <h3>Existing indexes</h3>
     </form>
 {% endif %}
 
-<h2>Delete table</h2>
+{% if can_drop_table %}
+    <h2>Drop table</h2>
 
-<form id="delete-table-form" action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
-    <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
-    <input type="hidden" name="drop_table" value="1">
-    <input type="submit" class="button-red" value="Delete this table">
-</form>
+    <form id="drop-table-form" action="{{ base_url }}-/edit-schema/{{ database.name|quote_plus }}/{{ table|quote_plus }}" method="post">
+        <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
+        <input type="hidden" name="drop_table" value="1">
+        <input type="submit" class="button-red" value="Drop this table">
+    </form>
+{% endif %}
 
 <h2>Current table schema</h2>
 <pre>{{ schema }}</pre>
@@ -247,7 +249,7 @@ <h2>Current table schema</h2>
     }), 200);
 });
 
-document.getElementById('delete-table-form').addEventListener('submit', function(event) {
+document.getElementById('drop-table-form').addEventListener('submit', function(event) {
     const userConfirmation = confirm("Are you sure you want to delete this table? This cannot be reversed.");
     if (!userConfirmation) {
         event.preventDefault();
diff --git a/tests/test_edit_schema.py b/tests/test_edit_schema.py
index 92a893d..81565ac 100644
--- a/tests/test_edit_schema.py
+++ b/tests/test_edit_schema.py
@@ -88,22 +88,80 @@ async def test_post_without_operation_raises_error(db_path):
 
 
 @pytest.mark.asyncio
-async def test_drop_table(db_path):
-    ds = Datasette([db_path])
+@pytest.mark.parametrize(
+    "actor_id,should_allow",
+    (
+        (None, False),
+        ("user_with_edit_schema", True),
+        ("user_with_just_create_table", False),
+        ("user_with_just_alter_table", False),
+        ("user_with_alter_table_and_drop_table", True),
+    ),
+)
+async def test_drop_table(permission_plugin, db_path, actor_id, should_allow):
+    ds = Datasette([db_path], pdb=True)
+    ds._rules_allow = [
+        Rule(
+            actor_id="user_with_edit_schema",
+            action="edit-schema",
+            database="data",
+            resource=None,
+        ),
+        Rule(
+            actor_id="user_with_alter_table_and_drop_table",
+            action="edit-schema-drop-table",
+            database="data",
+            resource="creatures",
+        ),
+        Rule(
+            actor_id="user_with_alter_table_and_drop_table",
+            action="edit-schema-alter-table",
+            database="data",
+            resource="creatures",
+        ),
+        Rule(
+            actor_id="user_with_just_create_table",
+            action="edit-schema-create-table",
+            database="data",
+            resource=None,
+        ),
+        Rule(
+            actor_id="user_with_just_alter_table",
+            action="edit-schema-alter-table",
+            database="data",
+            resource="creatures",
+        ),
+    ]
     db = sqlite_utils.Database(db_path)
     assert "creatures" in db.table_names()
-    cookies = {"ds_actor": ds.sign({"a": {"id": "root"}}, "actor")}
+    cookies = {}
+    if actor_id:
+        cookies = {"ds_actor": ds.sign({"a": {"id": actor_id}}, "actor")}
     # Get a csrftoken
-    csrftoken = (
-        await ds.client.get("/-/edit-schema/data/creatures", cookies=cookies)
-    ).cookies["ds_csrftoken"]
+    form_response = await ds.client.get(
+        "/-/edit-schema/data/creatures", cookies=cookies
+    )
+    if actor_id in (None, "user_with_just_create_table"):
+        assert form_response.status_code == 403
+        return
+    assert form_response.status_code == 200
+    csrftoken = form_response.cookies["ds_csrftoken"]
+    if should_allow:
+        assert 'name="drop_table"' in form_response.text
+    else:
+        assert 'name="drop_table"' not in form_response.text
+    # Try submitting form anyway
     response = await ds.client.post(
         "/-/edit-schema/data/creatures",
         data={"drop_table": "1", "csrftoken": csrftoken},
         cookies=dict(cookies, ds_csrftoken=csrftoken),
     )
-    assert response.status_code == 302
-    assert "creatures" not in db.table_names()
+    if should_allow:
+        assert response.status_code == 302
+        assert "creatures" not in db.table_names()
+    else:
+        assert response.status_code == 403
+        assert "creatures" in db.table_names()
 
 
 @pytest.mark.asyncio