Permission check now considers database, closes #31, closes #32

README.md

@@ -30,8 +30,16 @@ Use `/-/edit-schema/dbname` to create a new table in a specific database.
 
 By default only [the root actor](https://datasette.readthedocs.io/en/stable/authentication.html#using-the-root-actor) can access the page - so you'll need to run Datasette with the `--root` option and click on the link shown in the terminal to sign in and access the page.
 
+## Permissions
+
 The `edit-schema` permission governs access. You can use permission plugins such as [datasette-permissions-sql](https://github.com/simonw/datasette-permissions-sql) to grant additional access to the write interface.
 
+These permission checks will call the `permission_allowed()` plugin hook with three arguments:
+
+- `action` will be the string `"edit-schema"`
+- `actor` will be the currently authenticated actor - usually a dictionary
+- `resource` will be the string name of the database
+
 ## Screenshot
 
 ![datasette-edit-schema interface](https://raw.githubusercontent.com/simonw/datasette-edit-schema/main/datasette-edit-schema.png)

datasette_edit_schema/__init__.py

@@ -6,15 +6,22 @@
 
 
 @hookimpl
-def permission_allowed(actor, action):
-    if action == "edit-schema" and actor and actor.get("id") == "root":
+def permission_allowed(actor, action, resource):
+    if (
+        action == "edit-schema"
+        and actor
+        and actor.get("id") == "root"
+        and resource != "_internal"
+    ):
         return True
 
 
 @hookimpl
 def table_actions(datasette, actor, database, table):
     async def inner():
-        if not await datasette.permission_allowed(actor, "edit-schema", default=False):
+        if not await datasette.permission_allowed(
+            actor, "edit-schema", resource=database, default=False
+        ):
             return []
         return [
             {
@@ -47,34 +54,53 @@ def register_routes():
 
 
 def get_databases(datasette):
-    return [db for db in datasette.databases.values() if db.is_mutable]
+    return [
+        db
+        for db in datasette.databases.values()
+        if db.is_mutable and db.name != "_internal"
+    ]
 
 
-async def check_permissions(datasette, request):
+async def check_permissions(datasette, request, database):
     if not await datasette.permission_allowed(
-        request.actor, "edit-schema", default=False
+        request.actor, "edit-schema", resource=database, default=False
     ):
         raise Forbidden("Permission denied for edit-schema")
 
 
 async def edit_schema_index(datasette, request):
-    await check_permissions(datasette, request)
-    databases = get_databases(datasette)
-    if 1 == len(databases):
+    database_names = [db.name for db in get_databases(datasette)]
+    # Check permissions for each one
+    allowed_databases = [
+        name
+        for name in database_names
+        if await datasette.permission_allowed(
+            request.actor, "edit-schema", resource=name, default=False
+        )
+    ]
+    if not allowed_databases:
+        raise Forbidden("Permission denied for edit-schema")
+
+    if len(allowed_databases) == 1:
         return Response.redirect(
-            "/-/edit-schema/{}".format(quote_plus(databases[0].name))
+            "/-/edit-schema/{}".format(quote_plus(allowed_databases[0]))
         )
+
     return Response.html(
         await datasette.render_template(
-            "edit_schema_index.html", {"databases": databases}, request=request
+            "edit_schema_index.html",
+            {
+                "databases": allowed_databases,
+            },
+            request=request,
         )
     )
 
 
 async def edit_schema_database(request, datasette):
-    await check_permissions(datasette, request)
     databases = get_databases(datasette)
     database_name = request.url_vars["database"]
+    await check_permissions(datasette, request, database_name)
     just_these_tables = set(request.args.getlist("table"))
     try:
         database = [db for db in databases if db.name == database_name][0]
@@ -111,10 +137,10 @@ def get_columns(conn):
 
 
 async def edit_schema_table(request, datasette):
-    await check_permissions(datasette, request)
     table = unquote_plus(request.url_vars["table"])
     databases = get_databases(datasette)
     database_name = request.url_vars["database"]
+    await check_permissions(datasette, request, database_name)
     try:
         database = [db for db in databases if db.name == database_name][0]
     except IndexError:

datasette_edit_schema/templates/edit_schema_index.html

@@ -21,7 +21,7 @@ <h1>Edit schema</h1>
 
     <ul>
     {% for database in databases %}
-        <li><a href="/-/edit-schema/{{ database.name|quote_plus }}">{{ database.name }}</a></li>
+        <li><a href="/-/edit-schema/{{ database|quote_plus }}">{{ database }}</a></li>
     {% endfor %}
     </ul>
 {% else %}

tests/test_edit_schema.py

@@ -32,18 +32,25 @@ async def test_csrf_required(db_path):
     assert response.status_code == 403
 
 
-@pytest.mark.parametrize("authenticate", [True, False])
+@pytest.mark.parametrize(
+    "authenticated,path,should_allow",
+    (
+        (False, "/data/creatures", False),
+        (True, "/data/creatures", True),
+        (True, "/_internal/tables", False),
+    ),
+)
 @pytest.mark.asyncio
-async def test_table_actions(db_path, authenticate):
+async def test_table_actions(db_path, authenticated, path, should_allow):
     ds = Datasette([db_path])
     cookies = None
-    if authenticate:
+    if authenticated:
         cookies = {"ds_actor": ds.sign({"a": {"id": "root"}}, "actor")}
-    response = await ds.client.get("/data/creatures", cookies=cookies)
+    response = await ds.client.get(path, cookies=cookies)
     assert response.status_code == 200
-    fragment = '<li><a href="/-/edit-schema/data/creatures">Edit table schema</a></li>'
-    if authenticate:
-        # Should have column actions
+    fragment = '<li><a href="/-/edit-schema{}">Edit table schema</a></li>'.format(path)
+    if should_allow:
+        # Should have table action
         assert fragment in response.text
     else:
         assert fragment not in response.text