From 83adf55b2da83fd9a227f7e4c8506d72def72294 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 23 Oct 2022 20:28:15 -0700 Subject: [PATCH 001/823] Deploy one-dot-zero branch preview --- .github/workflows/deploy-latest.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/deploy-latest.yml b/.github/workflows/deploy-latest.yml index 2b94a7f1..43a843ed 100644 --- a/.github/workflows/deploy-latest.yml +++ b/.github/workflows/deploy-latest.yml @@ -3,7 +3,8 @@ name: Deploy latest.datasette.io on: push: branches: - - main + - main + - 1.0-dev permissions: contents: read @@ -68,6 +69,8 @@ jobs: gcloud config set project datasette-222320 export SUFFIX="-${GITHUB_REF#refs/heads/}" export SUFFIX=${SUFFIX#-main} + # Replace 1.0 with one-dot-zero in SUFFIX + export SUFFIX=${SUFFIX//1.0/one-dot-zero} datasette publish cloudrun fixtures.db fixtures2.db extra_database.db \ -m fixtures.json \ --plugins-dir=plugins \ From 02ae1a002918eb91f794e912c32742559da34cf5 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 11:59:03 -0700 Subject: [PATCH 002/823] Upgrade Docker images to Python 3.11, closes #1853 --- Dockerfile | 2 +- datasette/utils/__init__.py | 2 +- demos/apache-proxy/Dockerfile | 2 +- docs/publish.rst | 2 +- tests/test_package.py | 2 +- tests/test_publish_cloudrun.py | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index ee7ed957..9a8f06cf 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.10.6-slim-bullseye as build +FROM python:3.11.0-slim-bullseye as build # Version of Datasette to install, e.g. 0.55 # docker build . -t datasette --build-arg VERSION=0.55 diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 2bdea673..803ba96d 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -390,7 +390,7 @@ def make_dockerfile( "SQLITE_EXTENSIONS" ] = "/usr/lib/x86_64-linux-gnu/mod_spatialite.so" return """ -FROM python:3.10.6-slim-bullseye +FROM python:3.11.0-slim-bullseye COPY . /app WORKDIR /app {apt_get_extras} diff --git a/demos/apache-proxy/Dockerfile b/demos/apache-proxy/Dockerfile index 70b33bec..9a8448da 100644 --- a/demos/apache-proxy/Dockerfile +++ b/demos/apache-proxy/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.10.6-slim-bullseye +FROM python:3.11.0-slim-bullseye RUN apt-get update && \ apt-get install -y apache2 supervisor && \ diff --git a/docs/publish.rst b/docs/publish.rst index d817ed31..4ba94792 100644 --- a/docs/publish.rst +++ b/docs/publish.rst @@ -146,7 +146,7 @@ Here's example output for the package command:: $ datasette package parlgov.db --extra-options="--setting sql_time_limit_ms 2500" Sending build context to Docker daemon 4.459MB - Step 1/7 : FROM python:3.10.6-slim-bullseye + Step 1/7 : FROM python:3.11.0-slim-bullseye ---> 79e1dc9af1c1 Step 2/7 : COPY . /app ---> Using cache diff --git a/tests/test_package.py b/tests/test_package.py index ac15e61e..f05f3ece 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -12,7 +12,7 @@ class CaptureDockerfile: EXPECTED_DOCKERFILE = """ -FROM python:3.10.6-slim-bullseye +FROM python:3.11.0-slim-bullseye COPY . /app WORKDIR /app diff --git a/tests/test_publish_cloudrun.py b/tests/test_publish_cloudrun.py index e64534d2..158a090e 100644 --- a/tests/test_publish_cloudrun.py +++ b/tests/test_publish_cloudrun.py @@ -242,7 +242,7 @@ def test_publish_cloudrun_plugin_secrets( ) expected = textwrap.dedent( r""" - FROM python:3.10.6-slim-bullseye + FROM python:3.11.0-slim-bullseye COPY . /app WORKDIR /app @@ -309,7 +309,7 @@ def test_publish_cloudrun_apt_get_install( ) expected = textwrap.dedent( r""" - FROM python:3.10.6-slim-bullseye + FROM python:3.11.0-slim-bullseye COPY . /app WORKDIR /app From 6d085af28c63c28ecda388fc0552c91f756be0c6 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 07:13:43 -0700 Subject: [PATCH 003/823] Python 3.11 in CI --- .github/workflows/publish.yml | 16 ++++++++-------- .github/workflows/test.yml | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 9ef09d2e..fa608055 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -12,14 +12,14 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.7", "3.8", "3.9", "3.10"] + python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - - uses: actions/cache@v2 + - uses: actions/cache@v3 name: Configure pip caching with: path: ~/.cache/pip @@ -37,12 +37,12 @@ jobs: runs-on: ubuntu-latest needs: [test] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up Python - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: - python-version: '3.10' - - uses: actions/cache@v2 + python-version: '3.11' + - uses: actions/cache@v3 name: Configure pip caching with: path: ~/.cache/pip diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e38d5ee9..886f649a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,14 +10,14 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.7", "3.8", "3.9", "3.10", "3.11-dev"] + python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - - uses: actions/cache@v2 + - uses: actions/cache@v3 name: Configure pip caching with: path: ~/.cache/pip From 05b479224fa57af3ab2d03769edd5081dad62a19 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 12:16:48 -0700 Subject: [PATCH 004/823] Don't need pysqlite3-binary any more, refs #1853 --- .github/workflows/deploy-latest.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/deploy-latest.yml b/.github/workflows/deploy-latest.yml index 43a843ed..5598dc12 100644 --- a/.github/workflows/deploy-latest.yml +++ b/.github/workflows/deploy-latest.yml @@ -14,12 +14,12 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out datasette - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up Python - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: - python-version: "3.10" - - uses: actions/cache@v2 + python-version: "3.11" + - uses: actions/cache@v3 name: Configure pip caching with: path: ~/.cache/pip @@ -77,7 +77,6 @@ jobs: --branch=$GITHUB_SHA \ --version-note=$GITHUB_SHA \ --extra-options="--setting template_debug 1 --setting trace_debug 1 --crossdb" \ - --install=pysqlite3-binary \ --service "datasette-latest$SUFFIX" - name: Deploy to docs as well (only for main) if: ${{ github.ref == 'refs/heads/main' }} From f9ae92b37796f7f559d57b1ee9718aa4d43547e8 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 12:42:21 -0700 Subject: [PATCH 005/823] Poll until servers start, refs #1854 --- tests/conftest.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 215853b3..f4638a14 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +import httpx import os import pathlib import pytest @@ -110,8 +111,13 @@ def ds_localhost_http_server(): # Avoid FileNotFoundError: [Errno 2] No such file or directory: cwd=tempfile.gettempdir(), ) - # Give the server time to start - time.sleep(1.5) + # Loop until port 8041 serves traffic + while True: + try: + httpx.get("http://localhost:8041/") + break + except httpx.ConnectError: + time.sleep(0.1) # Check it started successfully assert not ds_proc.poll(), ds_proc.stdout.read().decode("utf-8") yield ds_proc @@ -146,8 +152,12 @@ def ds_localhost_https_server(tmp_path_factory): stderr=subprocess.STDOUT, cwd=tempfile.gettempdir(), ) - # Give the server time to start - time.sleep(1.5) + while True: + try: + httpx.get("https://localhost:8042/", verify=client_cert) + break + except httpx.ConnectError: + time.sleep(0.1) # Check it started successfully assert not ds_proc.poll(), ds_proc.stdout.read().decode("utf-8") yield ds_proc, client_cert @@ -168,8 +178,15 @@ def ds_unix_domain_socket_server(tmp_path_factory): stderr=subprocess.STDOUT, cwd=tempfile.gettempdir(), ) - # Give the server time to start - time.sleep(1.5) + # Poll until available + transport = httpx.HTTPTransport(uds=uds) + client = httpx.Client(transport=transport) + while True: + try: + client.get("http://localhost/_memory.json") + break + except httpx.ConnectError: + time.sleep(0.1) # Check it started successfully assert not ds_proc.poll(), ds_proc.stdout.read().decode("utf-8") yield ds_proc, uds From 42f8b402e6aa56af4bbe921e346af8df42acd50f Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 17:07:58 -0700 Subject: [PATCH 006/823] Initial prototype of create API token page, refs #1852 --- datasette/app.py | 5 ++ datasette/templates/create_token.html | 83 +++++++++++++++++++++++++++ datasette/views/special.py | 54 +++++++++++++++++ 3 files changed, 142 insertions(+) create mode 100644 datasette/templates/create_token.html diff --git a/datasette/app.py b/datasette/app.py index 9df16558..cab9d142 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -33,6 +33,7 @@ from .views.special import ( JsonDataView, PatternPortfolioView, AuthTokenView, + CreateTokenView, LogoutView, AllowDebugView, PermissionsDebugView, @@ -1212,6 +1213,10 @@ class Datasette: AuthTokenView.as_view(self), r"/-/auth-token$", ) + add_route( + CreateTokenView.as_view(self), + r"/-/create-token$", + ) add_route( LogoutView.as_view(self), r"/-/logout$", diff --git a/datasette/templates/create_token.html b/datasette/templates/create_token.html new file mode 100644 index 00000000..a94881ed --- /dev/null +++ b/datasette/templates/create_token.html @@ -0,0 +1,83 @@ +{% extends "base.html" %} + +{% block title %}Create an API token{% endblock %} + +{% block content %} + +

Create an API token

+ +

This token will allow API access with the same abilities as your current user.

+ +{% if errors %} + {% for error in errors %} +

{{ error }}

+ {% endfor %} +{% endif %} + +
+
+
+ +
+ + + +
+
+ +{% if token %} +
+

Your API token

+
+ + +
+ +
+ Token details +
{{ token_bits|tojson }}
+
+
+ {% endif %} + + + +{% endblock %} diff --git a/datasette/views/special.py b/datasette/views/special.py index dd834528..f2e69412 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -3,6 +3,7 @@ from datasette.utils.asgi import Response, Forbidden from datasette.utils import actor_matches_allow, add_cors_headers from .base import BaseView import secrets +import time class JsonDataView(BaseView): @@ -163,3 +164,56 @@ class MessagesDebugView(BaseView): else: datasette.add_message(request, message, getattr(datasette, message_type)) return Response.redirect(self.ds.urls.instance()) + + +class CreateTokenView(BaseView): + name = "create_token" + has_json_alternate = False + + async def get(self, request): + if not request.actor: + raise Forbidden("You must be logged in to create a token") + return await self.render( + ["create_token.html"], + request, + {"actor": request.actor}, + ) + + async def post(self, request): + if not request.actor: + raise Forbidden("You must be logged in to create a token") + post = await request.post_vars() + expires = None + errors = [] + if post.get("expire_type"): + duration = post.get("expire_duration") + if not duration or not duration.isdigit() or not int(duration) > 0: + errors.append("Invalid expire duration") + else: + unit = post["expire_type"] + if unit == "minutes": + expires = int(duration) * 60 + elif unit == "hours": + expires = int(duration) * 60 * 60 + elif unit == "days": + expires = int(duration) * 60 * 60 * 24 + else: + errors.append("Invalid expire duration unit") + token_bits = None + token = None + if not errors: + token_bits = { + "a": request.actor, + "e": (int(time.time()) + expires) if expires else None, + } + token = self.ds.sign(token_bits, "token") + return await self.render( + ["create_token.html"], + request, + { + "actor": request.actor, + "errors": errors, + "token": token, + "token_bits": token_bits, + }, + ) From 68ccb7578b5d3bf68b86fb2f5cf8753098dfe075 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 18:40:07 -0700 Subject: [PATCH 007/823] dstoke_ prefix for tokens Refs https://github.com/simonw/datasette/issues/1852#issuecomment-1291290451 --- datasette/views/special.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datasette/views/special.py b/datasette/views/special.py index f2e69412..d3f202f4 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -206,7 +206,7 @@ class CreateTokenView(BaseView): "a": request.actor, "e": (int(time.time()) + expires) if expires else None, } - token = self.ds.sign(token_bits, "token") + token = "dstok_{}".format(self.ds.sign(token_bits, "token")) return await self.render( ["create_token.html"], request, From 7ab091e8ef8d3af1e23b5a81ffad2bd8c96cc47c Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 19:04:05 -0700 Subject: [PATCH 008/823] Tests and docs for /-/create-token, refs #1852 --- datasette/views/special.py | 14 +++++--- docs/authentication.rst | 15 +++++++++ tests/test_auth.py | 68 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 4 deletions(-) diff --git a/datasette/views/special.py b/datasette/views/special.py index d3f202f4..7f70eb1f 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -170,9 +170,16 @@ class CreateTokenView(BaseView): name = "create_token" has_json_alternate = False - async def get(self, request): + def check_permission(self, request): if not request.actor: raise Forbidden("You must be logged in to create a token") + if not request.actor.get("id"): + raise Forbidden( + "You must be logged in as an actor with an ID to create a token" + ) + + async def get(self, request): + self.check_permission(request) return await self.render( ["create_token.html"], request, @@ -180,8 +187,7 @@ class CreateTokenView(BaseView): ) async def post(self, request): - if not request.actor: - raise Forbidden("You must be logged in to create a token") + self.check_permission(request) post = await request.post_vars() expires = None errors = [] @@ -203,7 +209,7 @@ class CreateTokenView(BaseView): token = None if not errors: token_bits = { - "a": request.actor, + "a": request.actor["id"], "e": (int(time.time()) + expires) if expires else None, } token = "dstok_{}".format(self.ds.sign(token_bits, "token")) diff --git a/docs/authentication.rst b/docs/authentication.rst index 685dab15..fc903fbb 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -333,6 +333,21 @@ To limit this ability for just one specific database, use this: } } +.. _CreateTokenView: + +API Tokens +========== + +Datasette includes a default mechanism for generating API tokens that can be used to authenticate requests. + +Authenticated users can create new API tokens using a form on the ``/-/create-token`` page. + +Created tokens can then be passed in the ``Authorization: Bearer token_here`` header of HTTP requests to Datasette. + +A token created by a user will include that user's ``"id"`` in the token payload, so any permissions granted to that user based on their ID will be made available to the token as well. + +Coming soon: a mechanism for creating tokens that can only perform a subset of the actions available to the user who created them. + .. _permissions_plugins: Checking permissions in plugins diff --git a/tests/test_auth.py b/tests/test_auth.py index 4ef35a76..3aaab50d 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -110,3 +110,71 @@ def test_no_logout_button_in_navigation_if_no_ds_actor_cookie(app_client, path): response = app_client.get(path + "?_bot=1") assert "bot" in response.text assert '
' not in response.text + + +@pytest.mark.parametrize( + "post_data,errors,expected_duration", + ( + ({"expire_type": ""}, [], None), + ({"expire_type": "x"}, ["Invalid expire duration"], None), + ({"expire_type": "minutes"}, ["Invalid expire duration"], None), + ( + {"expire_type": "minutes", "expire_duration": "x"}, + ["Invalid expire duration"], + None, + ), + ( + {"expire_type": "minutes", "expire_duration": "-1"}, + ["Invalid expire duration"], + None, + ), + ( + {"expire_type": "minutes", "expire_duration": "0"}, + ["Invalid expire duration"], + None, + ), + ( + {"expire_type": "minutes", "expire_duration": "10"}, + [], + 600, + ), + ( + {"expire_type": "hours", "expire_duration": "10"}, + [], + 10 * 60 * 60, + ), + ( + {"expire_type": "days", "expire_duration": "3"}, + [], + 60 * 60 * 24 * 3, + ), + ), +) +def test_auth_create_token(app_client, post_data, errors, expected_duration): + assert app_client.get("/-/create-token").status == 403 + ds_actor = app_client.actor_cookie({"id": "test"}) + response = app_client.get("/-/create-token", cookies={"ds_actor": ds_actor}) + assert response.status == 200 + assert ">Create an API token<" in response.text + # Now try actually creating one + response2 = app_client.post( + "/-/create-token", + post_data, + csrftoken_from=True, + cookies={"ds_actor": ds_actor}, + ) + assert response2.status == 200 + if errors: + for error in errors: + assert '

{}

'.format(error) in response2.text + else: + # Extract token from page + token = response2.text.split('value="dstok_')[1].split('"')[0] + details = app_client.ds.unsign(token, "token") + assert details.keys() == {"a", "e"} + assert details["a"] == "test" + if expected_duration is None: + assert details["e"] is None + else: + about_right = int(time.time()) + expected_duration + assert about_right - 2 < details["e"] < about_right + 2 From b29e487bc3fde6418bf45bda7cfed2e081ff03fb Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 19:18:41 -0700 Subject: [PATCH 009/823] actor_from_request for dstok_ tokens, refs #1852 --- datasette/default_permissions.py | 25 +++++++++++++++++++++++++ datasette/utils/testing.py | 2 ++ tests/test_auth.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index b58d8d1b..4d836ddc 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -1,5 +1,7 @@ from datasette import hookimpl from datasette.utils import actor_matches_allow +import itsdangerous +import time @hookimpl(tryfirst=True) @@ -45,3 +47,26 @@ def permission_allowed(datasette, actor, action, resource): return actor_matches_allow(actor, database_allow_sql) return inner + + +@hookimpl +def actor_from_request(datasette, request): + prefix = "dstok_" + authorization = request.headers.get("authorization") + if not authorization: + return None + if not authorization.startswith("Bearer "): + return None + token = authorization[len("Bearer ") :] + if not token.startswith(prefix): + return None + token = token[len(prefix) :] + try: + decoded = datasette.unsign(token, namespace="token") + except itsdangerous.BadSignature: + return None + expires_at = decoded.get("e") + if expires_at is not None: + if expires_at < time.time(): + return None + return {"id": decoded["a"], "dstok": True} diff --git a/datasette/utils/testing.py b/datasette/utils/testing.py index b28fc575..4f76a799 100644 --- a/datasette/utils/testing.py +++ b/datasette/utils/testing.py @@ -62,6 +62,7 @@ class TestClient: method="GET", cookies=None, if_none_match=None, + headers=None, ): return await self._request( path=path, @@ -70,6 +71,7 @@ class TestClient: method=method, cookies=cookies, if_none_match=if_none_match, + headers=headers, ) @async_to_sync diff --git a/tests/test_auth.py b/tests/test_auth.py index 3aaab50d..be21d6a5 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -178,3 +178,35 @@ def test_auth_create_token(app_client, post_data, errors, expected_duration): else: about_right = int(time.time()) + expected_duration assert about_right - 2 < details["e"] < about_right + 2 + + +@pytest.mark.parametrize( + "scenario,should_work", + ( + ("no_token", False), + ("invalid_token", False), + ("expired_token", False), + ("valid_unlimited_token", True), + ("valid_expiring_token", True), + ), +) +def test_auth_with_dstok_token(app_client, scenario, should_work): + token = None + if scenario == "valid_unlimited_token": + token = app_client.ds.sign({"a": "test"}, "token") + elif scenario == "valid_expiring_token": + token = app_client.ds.sign({"a": "test", "e": int(time.time()) + 1000}, "token") + elif scenario == "expired_token": + token = app_client.ds.sign({"a": "test", "e": int(time.time()) - 1000}, "token") + elif scenario == "invalid_token": + token = "invalid" + if token: + token = "dstok_{}".format(token) + headers = {} + if token: + headers["Authorization"] = "Bearer {}".format(token) + response = app_client.get("/-/actor.json", headers=headers) + if should_work: + assert response.json == {"actor": {"id": "test", "dstok": True}} + else: + assert response.json == {"actor": None} From 0f013ff497df62e1dd2075777b9817555646010e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 19:43:55 -0700 Subject: [PATCH 010/823] Mechanism to prevent tokens creating tokens, closes #1857 --- datasette/default_permissions.py | 2 +- datasette/views/special.py | 4 ++++ docs/authentication.rst | 2 ++ tests/test_auth.py | 11 ++++++++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 4d836ddc..d908af7a 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -69,4 +69,4 @@ def actor_from_request(datasette, request): if expires_at is not None: if expires_at < time.time(): return None - return {"id": decoded["a"], "dstok": True} + return {"id": decoded["a"], "token": "dstok"} diff --git a/datasette/views/special.py b/datasette/views/special.py index 7f70eb1f..91130353 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -177,6 +177,10 @@ class CreateTokenView(BaseView): raise Forbidden( "You must be logged in as an actor with an ID to create a token" ) + if request.actor.get("token"): + raise Forbidden( + "Token authentication cannot be used to create additional tokens" + ) async def get(self, request): self.check_permission(request) diff --git a/docs/authentication.rst b/docs/authentication.rst index fc903fbb..cbecd296 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -348,6 +348,8 @@ A token created by a user will include that user's ``"id"`` in the token payload Coming soon: a mechanism for creating tokens that can only perform a subset of the actions available to the user who created them. +This page cannot be accessed by actors with a ``"token": "some-value"`` property. This is to prevent API tokens from being used to automatically create more tokens. Datasette plugins that implement their own form of API token authentication should follow this convention. + .. _permissions_plugins: Checking permissions in plugins diff --git a/tests/test_auth.py b/tests/test_auth.py index be21d6a5..397d51d7 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -180,6 +180,15 @@ def test_auth_create_token(app_client, post_data, errors, expected_duration): assert about_right - 2 < details["e"] < about_right + 2 +def test_auth_create_token_not_allowed_for_tokens(app_client): + ds_tok = app_client.ds.sign({"a": "test", "token": "dstok"}, "token") + response = app_client.get( + "/-/create-token", + headers={"Authorization": "Bearer dstok_{}".format(ds_tok)}, + ) + assert response.status == 403 + + @pytest.mark.parametrize( "scenario,should_work", ( @@ -207,6 +216,6 @@ def test_auth_with_dstok_token(app_client, scenario, should_work): headers["Authorization"] = "Bearer {}".format(token) response = app_client.get("/-/actor.json", headers=headers) if should_work: - assert response.json == {"actor": {"id": "test", "dstok": True}} + assert response.json == {"actor": {"id": "test", "token": "dstok"}} else: assert response.json == {"actor": None} From c23fa850e7f21977e367e3467656055216978e8a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 19:55:47 -0700 Subject: [PATCH 011/823] allow_signed_tokens setting, closes #1856 --- datasette/app.py | 5 +++++ datasette/default_permissions.py | 2 ++ datasette/views/special.py | 2 ++ docs/authentication.rst | 2 ++ docs/cli-reference.rst | 2 ++ docs/plugins.rst | 1 + docs/settings.rst | 13 +++++++++++++ tests/test_auth.py | 26 +++++++++++++++++++++----- 8 files changed, 48 insertions(+), 5 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index cab9d142..c868f8d3 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -124,6 +124,11 @@ SETTINGS = ( True, "Allow users to download the original SQLite database files", ), + Setting( + "allow_signed_tokens", + True, + "Allow users to create and use signed API tokens", + ), Setting("suggest_facets", True, "Calculate and display suggested facets"), Setting( "default_cache_ttl", diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index d908af7a..49ca8851 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -52,6 +52,8 @@ def permission_allowed(datasette, actor, action, resource): @hookimpl def actor_from_request(datasette, request): prefix = "dstok_" + if not datasette.setting("allow_signed_tokens"): + return None authorization = request.headers.get("authorization") if not authorization: return None diff --git a/datasette/views/special.py b/datasette/views/special.py index 91130353..89015958 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -171,6 +171,8 @@ class CreateTokenView(BaseView): has_json_alternate = False def check_permission(self, request): + if not self.ds.setting("allow_signed_tokens"): + raise Forbidden("Signed tokens are not enabled for this Datasette instance") if not request.actor: raise Forbidden("You must be logged in to create a token") if not request.actor.get("id"): diff --git a/docs/authentication.rst b/docs/authentication.rst index cbecd296..50304ec5 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -350,6 +350,8 @@ Coming soon: a mechanism for creating tokens that can only perform a subset of t This page cannot be accessed by actors with a ``"token": "some-value"`` property. This is to prevent API tokens from being used to automatically create more tokens. Datasette plugins that implement their own form of API token authentication should follow this convention. +You can disable this feature using the :ref:`allow_signed_tokens ` setting. + .. _permissions_plugins: Checking permissions in plugins diff --git a/docs/cli-reference.rst b/docs/cli-reference.rst index 4a8465cb..fd5e2404 100644 --- a/docs/cli-reference.rst +++ b/docs/cli-reference.rst @@ -226,6 +226,8 @@ These can be passed to ``datasette serve`` using ``datasette serve --setting nam ?_facet= parameter (default=True) allow_download Allow users to download the original SQLite database files (default=True) + allow_signed_tokens Allow users to create and use signed API tokens + (default=True) suggest_facets Calculate and display suggested facets (default=True) default_cache_ttl Default HTTP cache TTL (used in Cache-Control: diff --git a/docs/plugins.rst b/docs/plugins.rst index 29078054..9efef32f 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -151,6 +151,7 @@ If you run ``datasette plugins --all`` it will include default plugins that ship "templates": false, "version": null, "hooks": [ + "actor_from_request", "permission_allowed" ] }, diff --git a/docs/settings.rst b/docs/settings.rst index a6d50543..be640b21 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -169,6 +169,19 @@ Should users be able to download the original SQLite database using a link on th datasette mydatabase.db --setting allow_download off +.. _setting_allow_signed_tokens: + +allow_signed_tokens +~~~~~~~~~~~~~~~~~~~ + +Should users be able to create signed API tokens to access Datasette? + +This is turned on by default. Use the following to turn it off:: + + datasette mydatabase.db --setting allow_signed_tokens off + +Turning this setting off will disable the ``/-/create-token`` page, :ref:`described here `. It will also cause any incoming ``Authorization: Bearer dstok_...`` API tokens to be ignored. + .. _setting_default_cache_ttl: default_cache_ttl diff --git a/tests/test_auth.py b/tests/test_auth.py index 397d51d7..a79dafd8 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -189,9 +189,20 @@ def test_auth_create_token_not_allowed_for_tokens(app_client): assert response.status == 403 +def test_auth_create_token_not_allowed_if_allow_signed_tokens_off(app_client): + app_client.ds._settings["allow_signed_tokens"] = False + try: + ds_actor = app_client.actor_cookie({"id": "test"}) + response = app_client.get("/-/create-token", cookies={"ds_actor": ds_actor}) + assert response.status == 403 + finally: + app_client.ds._settings["allow_signed_tokens"] = True + + @pytest.mark.parametrize( "scenario,should_work", ( + ("allow_signed_tokens_off", False), ("no_token", False), ("invalid_token", False), ("expired_token", False), @@ -201,7 +212,7 @@ def test_auth_create_token_not_allowed_for_tokens(app_client): ) def test_auth_with_dstok_token(app_client, scenario, should_work): token = None - if scenario == "valid_unlimited_token": + if scenario in ("valid_unlimited_token", "allow_signed_tokens_off"): token = app_client.ds.sign({"a": "test"}, "token") elif scenario == "valid_expiring_token": token = app_client.ds.sign({"a": "test", "e": int(time.time()) + 1000}, "token") @@ -211,11 +222,16 @@ def test_auth_with_dstok_token(app_client, scenario, should_work): token = "invalid" if token: token = "dstok_{}".format(token) + if scenario == "allow_signed_tokens_off": + app_client.ds._settings["allow_signed_tokens"] = False headers = {} if token: headers["Authorization"] = "Bearer {}".format(token) response = app_client.get("/-/actor.json", headers=headers) - if should_work: - assert response.json == {"actor": {"id": "test", "token": "dstok"}} - else: - assert response.json == {"actor": None} + try: + if should_work: + assert response.json == {"actor": {"id": "test", "token": "dstok"}} + else: + assert response.json == {"actor": None} + finally: + app_client.ds._settings["allow_signed_tokens"] = True From c36a74ece1e475291af326d493d8db9ff3afdd30 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 21:04:39 -0700 Subject: [PATCH 012/823] Try shutting down executor in tests to free up thread local SQLite connections, refs #1843 --- tests/fixtures.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/fixtures.py b/tests/fixtures.py index 13a3dffa..d1afd2f3 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -166,6 +166,7 @@ def make_app_client( # Close the connection to avoid "too many open files" errors conn.close() os.remove(filepath) + ds.executor.shutdown() @pytest.fixture(scope="session") From c556fad65d8a45ce85027678796a12ac9107d9ed Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 21:25:47 -0700 Subject: [PATCH 013/823] Try to address too many files error again, refs #1843 --- tests/fixtures.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index d1afd2f3..92a10da6 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -131,10 +131,14 @@ def make_app_client( for sql, params in TABLE_PARAMETERIZED_SQL: with conn: conn.execute(sql, params) + # Close the connection to avoid "too many open files" errors + conn.close() if extra_databases is not None: for extra_filename, extra_sql in extra_databases.items(): extra_filepath = os.path.join(tmpdir, extra_filename) - sqlite3.connect(extra_filepath).executescript(extra_sql) + c2 = sqlite3.connect(extra_filepath) + c2.executescript(extra_sql) + c2.close() # Insert at start to help test /-/databases ordering: files.insert(0, extra_filepath) os.chdir(os.path.dirname(filepath)) @@ -163,10 +167,7 @@ def make_app_client( crossdb=crossdb, ) yield TestClient(ds) - # Close the connection to avoid "too many open files" errors - conn.close() os.remove(filepath) - ds.executor.shutdown() @pytest.fixture(scope="session") From c7956eed7777c62653b4d508570c5d77cfead7d9 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 25 Oct 2022 21:26:12 -0700 Subject: [PATCH 014/823] datasette create-token command, refs #1859 --- datasette/default_permissions.py | 38 ++++++++++++++++++++++++++++ docs/authentication.rst | 23 +++++++++++++++++ docs/cli-reference.rst | 43 ++++++++++++++++++++++++++------ docs/plugins.rst | 3 ++- tests/test_api.py | 1 + tests/test_auth.py | 28 +++++++++++++++++++++ tests/test_plugins.py | 2 ++ 7 files changed, 130 insertions(+), 8 deletions(-) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 49ca8851..12499c16 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -1,6 +1,8 @@ from datasette import hookimpl from datasette.utils import actor_matches_allow +import click import itsdangerous +import json import time @@ -72,3 +74,39 @@ def actor_from_request(datasette, request): if expires_at < time.time(): return None return {"id": decoded["a"], "token": "dstok"} + + +@hookimpl +def register_commands(cli): + from datasette.app import Datasette + + @cli.command() + @click.argument("id") + @click.option( + "--secret", + help="Secret used for signing the API tokens", + envvar="DATASETTE_SECRET", + required=True, + ) + @click.option( + "-e", + "--expires-after", + help="Token should expire after this many seconds", + type=int, + ) + @click.option( + "--debug", + help="Show decoded token", + is_flag=True, + ) + def create_token(id, secret, expires_after, debug): + "Create a signed API token for the specified actor ID" + ds = Datasette(secret=secret) + bits = {"a": id, "token": "dstok"} + if expires_after: + bits["e"] = int(time.time()) + expires_after + token = ds.sign(bits, namespace="token") + click.echo("dstok_{}".format(token)) + if debug: + click.echo("\nDecoded:\n") + click.echo(json.dumps(ds.unsign(token, namespace="token"), indent=2)) diff --git a/docs/authentication.rst b/docs/authentication.rst index 50304ec5..0835e17c 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -352,6 +352,29 @@ This page cannot be accessed by actors with a ``"token": "some-value"`` property You can disable this feature using the :ref:`allow_signed_tokens ` setting. +.. _authentication_cli_create_token: + +datasette create-token +---------------------- + +You can also create tokens on the command line using the ``datasette create-token`` command. + +This command takes one required argument - the ID of the actor to be associated with the created token. + +You can specify an ``--expires-after`` option in seconds. If omitted, the token will never expire. + +The command will sign the token using the ``DATASETTE_SECRET`` environment variable, if available. You can also pass the secret using the ``--secret`` option. + +This means you can run the command locally to create tokens for use with a deployed Datasette instance, provided you know that instance's secret. + +To create a token for the ``root`` actor that will expire in one hour:: + + datasette create-token root --expires-after 3600 + +To create a secret that never expires using a specific secret:: + + datasette create-token root --secret my-secret-goes-here + .. _permissions_plugins: Checking permissions in plugins diff --git a/docs/cli-reference.rst b/docs/cli-reference.rst index fd5e2404..b40c6b2c 100644 --- a/docs/cli-reference.rst +++ b/docs/cli-reference.rst @@ -47,13 +47,14 @@ Running ``datasette --help`` shows a list of all of the available commands. --help Show this message and exit. Commands: - serve* Serve up specified SQLite database files with a web UI - inspect Generate JSON summary of provided database files - install Install plugins and packages from PyPI into the same... - package Package SQLite files into a Datasette Docker container - plugins List currently installed plugins - publish Publish specified SQLite database files to the internet along... - uninstall Uninstall plugins and Python packages from the Datasette... + serve* Serve up specified SQLite database files with a web UI + create-token Create a signed API token for the specified actor ID + inspect Generate JSON summary of provided database files + install Install plugins and packages from PyPI into the same... + package Package SQLite files into a Datasette Docker container + plugins List currently installed plugins + publish Publish specified SQLite database files to the internet... + uninstall Uninstall plugins and Python packages from the Datasette... .. [[[end]]] @@ -591,3 +592,31 @@ This performance optimization is used automatically by some of the ``datasette p .. [[[end]]] + + +.. _cli_help_create_token___help: + +datasette create-token +====================== + +Create a signed API token, see :ref:`authentication_cli_create_token`. + +.. [[[cog + help(["create-token", "--help"]) +.. ]]] + +:: + + Usage: datasette create-token [OPTIONS] ID + + Create a signed API token for the specified actor ID + + Options: + --secret TEXT Secret used for signing the API tokens + [required] + -e, --expires-after INTEGER Token should expire after this many seconds + --debug Show decoded token + --help Show this message and exit. + + +.. [[[end]]] diff --git a/docs/plugins.rst b/docs/plugins.rst index 9efef32f..3ae42293 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -152,7 +152,8 @@ If you run ``datasette plugins --all`` it will include default plugins that ship "version": null, "hooks": [ "actor_from_request", - "permission_allowed" + "permission_allowed", + "register_commands" ] }, { diff --git a/tests/test_api.py b/tests/test_api.py index ad74d16e..f7cbe950 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -806,6 +806,7 @@ def test_settings_json(app_client): "max_returned_rows": 100, "sql_time_limit_ms": 200, "allow_download": True, + "allow_signed_tokens": True, "allow_facet": True, "suggest_facets": True, "default_cache_ttl": 5, diff --git a/tests/test_auth.py b/tests/test_auth.py index a79dafd8..f2d82107 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,5 +1,7 @@ from .fixtures import app_client +from click.testing import CliRunner from datasette.utils import baseconv +from datasette.cli import cli import pytest import time @@ -235,3 +237,29 @@ def test_auth_with_dstok_token(app_client, scenario, should_work): assert response.json == {"actor": None} finally: app_client.ds._settings["allow_signed_tokens"] = True + + +@pytest.mark.parametrize("expires", (None, 1000, -1000)) +def test_cli_create_token(app_client, expires): + secret = app_client.ds._secret + runner = CliRunner(mix_stderr=False) + args = ["create-token", "--secret", secret, "test"] + if expires: + args += ["--expires-after", str(expires)] + result = runner.invoke(cli, args) + assert result.exit_code == 0 + token = result.output.strip() + assert token.startswith("dstok_") + details = app_client.ds.unsign(token[len("dstok_") :], "token") + expected_keys = {"a", "token"} + if expires: + expected_keys.add("e") + assert details.keys() == expected_keys + assert details["a"] == "test" + response = app_client.get( + "/-/actor.json", headers={"Authorization": "Bearer {}".format(token)} + ) + if expires is None or expires > 0: + assert response.json == {"actor": {"id": "test", "token": "dstok"}} + else: + assert response.json == {"actor": None} diff --git a/tests/test_plugins.py b/tests/test_plugins.py index e0a7bc76..de3fde8e 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -971,6 +971,7 @@ def test_hook_register_commands(): "plugins", "publish", "uninstall", + "create-token", } # Now install a plugin @@ -1001,6 +1002,7 @@ def test_hook_register_commands(): "uninstall", "verify", "unverify", + "create-token", } pm.unregister(name="verify") importlib.reload(cli) From 55f860c304aea813cb7ed740cc5625560a0722a0 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 26 Oct 2022 14:13:31 -0700 Subject: [PATCH 015/823] Fix bug with breadcrumbs and request=None, closes #1849 --- datasette/app.py | 9 ++++++--- tests/test_internals_datasette.py | 9 +++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index c868f8d3..596ff44d 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -639,15 +639,18 @@ class Datasette: async def _crumb_items(self, request, table=None, database=None): crumbs = [] + actor = None + if request: + actor = request.actor # Top-level link if await self.permission_allowed( - actor=request.actor, action="view-instance", default=True + actor=actor, action="view-instance", default=True ): crumbs.append({"href": self.urls.instance(), "label": "home"}) # Database link if database: if await self.permission_allowed( - actor=request.actor, + actor=actor, action="view-database", resource=database, default=True, @@ -662,7 +665,7 @@ class Datasette: if table: assert database, "table= requires database=" if await self.permission_allowed( - actor=request.actor, + actor=actor, action="view-table", resource=(database, table), default=True, diff --git a/tests/test_internals_datasette.py b/tests/test_internals_datasette.py index c82cafb3..1b4732af 100644 --- a/tests/test_internals_datasette.py +++ b/tests/test_internals_datasette.py @@ -125,3 +125,12 @@ async def test_datasette_ensure_permissions_check_visibility( visible, private = await ds.check_visibility(actor, permissions=permissions) assert visible == should_allow assert private == expected_private + + +@pytest.mark.asyncio +async def test_datasette_render_template_no_request(): + # https://github.com/simonw/datasette/issues/1849 + ds = Datasette([], memory=True) + await ds.invoke_startup() + rendered = await ds.render_template("error.html") + assert "Error " in rendered From af5d5d0243631562ad83f2c318bff31a077feb5d Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 26 Oct 2022 14:34:33 -0700 Subject: [PATCH 016/823] Allow leading comments on SQL queries, refs #1860 --- datasette/utils/__init__.py | 27 +++++++++++++++++++++------ tests/test_utils.py | 7 +++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 803ba96d..977a66d6 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -205,13 +205,28 @@ class InvalidSql(Exception): pass +# Allow SQL to start with a /* */ or -- comment +comment_re = ( + # Start of string, then any amount of whitespace + r"^(\s*" + + + # Comment that starts with -- and ends at a newline + r"(?:\-\-.*?\n\s*)" + + + # Comment that starts with /* and ends with */ + r"|(?:/\*[\s\S]*?\*/)" + + + # Whitespace + r")*\s*" +) + allowed_sql_res = [ - re.compile(r"^select\b"), - re.compile(r"^explain\s+select\b"), - re.compile(r"^explain\s+query\s+plan\s+select\b"), - re.compile(r"^with\b"), - re.compile(r"^explain\s+with\b"), - re.compile(r"^explain\s+query\s+plan\s+with\b"), + re.compile(comment_re + r"select\b"), + re.compile(comment_re + r"explain\s+select\b"), + re.compile(comment_re + r"explain\s+query\s+plan\s+select\b"), + re.compile(comment_re + r"with\b"), + re.compile(comment_re + r"explain\s+with\b"), + re.compile(comment_re + r"explain\s+query\s+plan\s+with\b"), ] allowed_pragmas = ( "database_list", diff --git a/tests/test_utils.py b/tests/test_utils.py index d71a612d..e89f1e6b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -141,6 +141,7 @@ def test_custom_json_encoder(obj, expected): "update blah set some_column='# Hello there\n\n* This is a list\n* of items\n--\n[And a link](https://github.com/simonw/datasette-render-markdown).'\nas demo_markdown", "PRAGMA case_sensitive_like = true", "SELECT * FROM pragma_not_on_allow_list('idx52')", + "/* This comment is not valid. select 1", ], ) def test_validate_sql_select_bad(bad_sql): @@ -166,6 +167,12 @@ def test_validate_sql_select_bad(bad_sql): "explain query plan WITH RECURSIVE cnt(x) AS (SELECT 1 UNION ALL SELECT x+1 FROM cnt LIMIT 10) SELECT x FROM cnt;", "SELECT * FROM pragma_index_info('idx52')", "select * from pragma_table_xinfo('table')", + # Various types of comment + "-- comment\nselect 1", + "-- one line\n -- two line\nselect 1", + " /* comment */\nselect 1", + " /* comment */select 1", + "/* comment */\n -- another\n /* one more */ select 1", ], ) def test_validate_sql_select_good(good_sql): From 382a87158337540f991c6dc887080f7b37c7c26e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 26 Oct 2022 14:13:31 -0700 Subject: [PATCH 017/823] max_signed_tokens_ttl setting, closes #1858 Also redesigned token format to include creation time and optional duration. --- datasette/app.py | 5 ++++ datasette/default_permissions.py | 33 +++++++++++++++++---- datasette/views/special.py | 20 ++++++++----- docs/settings.rst | 15 ++++++++++ tests/test_api.py | 1 + tests/test_auth.py | 50 ++++++++++++++++++++++++-------- 6 files changed, 99 insertions(+), 25 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 596ff44d..894d7f0f 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -129,6 +129,11 @@ SETTINGS = ( True, "Allow users to create and use signed API tokens", ), + Setting( + "max_signed_tokens_ttl", + 0, + "Maximum allowed expiry time for signed API tokens", + ), Setting("suggest_facets", True, "Calculate and display suggested facets"), Setting( "default_cache_ttl", diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 12499c16..c502dd70 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -56,6 +56,7 @@ def actor_from_request(datasette, request): prefix = "dstok_" if not datasette.setting("allow_signed_tokens"): return None + max_signed_tokens_ttl = datasette.setting("max_signed_tokens_ttl") authorization = request.headers.get("authorization") if not authorization: return None @@ -69,11 +70,31 @@ def actor_from_request(datasette, request): decoded = datasette.unsign(token, namespace="token") except itsdangerous.BadSignature: return None - expires_at = decoded.get("e") - if expires_at is not None: - if expires_at < time.time(): + if "t" not in decoded: + # Missing timestamp + return None + created = decoded["t"] + if not isinstance(created, int): + # Invalid timestamp + return None + duration = decoded.get("d") + if duration is not None and not isinstance(duration, int): + # Invalid duration + return None + if (duration is None and max_signed_tokens_ttl) or ( + duration is not None + and max_signed_tokens_ttl + and duration > max_signed_tokens_ttl + ): + duration = max_signed_tokens_ttl + if duration: + if time.time() - created > duration: + # Expired return None - return {"id": decoded["a"], "token": "dstok"} + actor = {"id": decoded["a"], "token": "dstok"} + if duration: + actor["token_expires"] = created + duration + return actor @hookimpl @@ -102,9 +123,9 @@ def register_commands(cli): def create_token(id, secret, expires_after, debug): "Create a signed API token for the specified actor ID" ds = Datasette(secret=secret) - bits = {"a": id, "token": "dstok"} + bits = {"a": id, "token": "dstok", "t": int(time.time())} if expires_after: - bits["e"] = int(time.time()) + expires_after + bits["d"] = expires_after token = ds.sign(bits, namespace="token") click.echo("dstok_{}".format(token)) if debug: diff --git a/datasette/views/special.py b/datasette/views/special.py index 89015958..b754a2f0 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -195,20 +195,24 @@ class CreateTokenView(BaseView): async def post(self, request): self.check_permission(request) post = await request.post_vars() - expires = None errors = [] + duration = None if post.get("expire_type"): - duration = post.get("expire_duration") - if not duration or not duration.isdigit() or not int(duration) > 0: + duration_string = post.get("expire_duration") + if ( + not duration_string + or not duration_string.isdigit() + or not int(duration_string) > 0 + ): errors.append("Invalid expire duration") else: unit = post["expire_type"] if unit == "minutes": - expires = int(duration) * 60 + duration = int(duration_string) * 60 elif unit == "hours": - expires = int(duration) * 60 * 60 + duration = int(duration_string) * 60 * 60 elif unit == "days": - expires = int(duration) * 60 * 60 * 24 + duration = int(duration_string) * 60 * 60 * 24 else: errors.append("Invalid expire duration unit") token_bits = None @@ -216,8 +220,10 @@ class CreateTokenView(BaseView): if not errors: token_bits = { "a": request.actor["id"], - "e": (int(time.time()) + expires) if expires else None, + "t": int(time.time()), } + if duration: + token_bits["d"] = duration token = "dstok_{}".format(self.ds.sign(token_bits, "token")) return await self.render( ["create_token.html"], diff --git a/docs/settings.rst b/docs/settings.rst index be640b21..a990c78c 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -182,6 +182,21 @@ This is turned on by default. Use the following to turn it off:: Turning this setting off will disable the ``/-/create-token`` page, :ref:`described here `. It will also cause any incoming ``Authorization: Bearer dstok_...`` API tokens to be ignored. +.. _setting_max_signed_tokens_ttl: + +max_signed_tokens_ttl +~~~~~~~~~~~~~~~~~~~~~ + +Maximum allowed expiry time for signed API tokens created by users. + +Defaults to ``0`` which means no limit - tokens can be created that will never expire. + +Set this to a value in seconds to limit the maximum expiry time. For example, to set that limit to 24 hours you would use:: + + datasette mydatabase.db --setting max_signed_tokens_ttl 86400 + +This setting is enforced when incoming tokens are processed. + .. _setting_default_cache_ttl: default_cache_ttl diff --git a/tests/test_api.py b/tests/test_api.py index f7cbe950..fc171421 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -807,6 +807,7 @@ def test_settings_json(app_client): "sql_time_limit_ms": 200, "allow_download": True, "allow_signed_tokens": True, + "max_signed_tokens_ttl": 0, "allow_facet": True, "suggest_facets": True, "default_cache_ttl": 5, diff --git a/tests/test_auth.py b/tests/test_auth.py index f2d82107..fa1b2e46 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -173,13 +173,19 @@ def test_auth_create_token(app_client, post_data, errors, expected_duration): # Extract token from page token = response2.text.split('value="dstok_')[1].split('"')[0] details = app_client.ds.unsign(token, "token") - assert details.keys() == {"a", "e"} + assert details.keys() == {"a", "t", "d"} or details.keys() == {"a", "t"} assert details["a"] == "test" if expected_duration is None: - assert details["e"] is None + assert "d" not in details else: - about_right = int(time.time()) + expected_duration - assert about_right - 2 < details["e"] < about_right + 2 + assert details["d"] == expected_duration + # And test that token + response3 = app_client.get( + "/-/actor.json", + headers={"Authorization": "Bearer {}".format("dstok_{}".format(token))}, + ) + assert response3.status == 200 + assert response3.json["actor"]["id"] == "test" def test_auth_create_token_not_allowed_for_tokens(app_client): @@ -206,6 +212,7 @@ def test_auth_create_token_not_allowed_if_allow_signed_tokens_off(app_client): ( ("allow_signed_tokens_off", False), ("no_token", False), + ("no_timestamp", False), ("invalid_token", False), ("expired_token", False), ("valid_unlimited_token", True), @@ -214,12 +221,15 @@ def test_auth_create_token_not_allowed_if_allow_signed_tokens_off(app_client): ) def test_auth_with_dstok_token(app_client, scenario, should_work): token = None + _time = int(time.time()) if scenario in ("valid_unlimited_token", "allow_signed_tokens_off"): - token = app_client.ds.sign({"a": "test"}, "token") + token = app_client.ds.sign({"a": "test", "t": _time}, "token") elif scenario == "valid_expiring_token": - token = app_client.ds.sign({"a": "test", "e": int(time.time()) + 1000}, "token") + token = app_client.ds.sign({"a": "test", "t": _time - 50, "d": 1000}, "token") elif scenario == "expired_token": - token = app_client.ds.sign({"a": "test", "e": int(time.time()) - 1000}, "token") + token = app_client.ds.sign({"a": "test", "t": _time - 2000, "d": 1000}, "token") + elif scenario == "no_timestamp": + token = app_client.ds.sign({"a": "test"}, "token") elif scenario == "invalid_token": token = "invalid" if token: @@ -232,7 +242,16 @@ def test_auth_with_dstok_token(app_client, scenario, should_work): response = app_client.get("/-/actor.json", headers=headers) try: if should_work: - assert response.json == {"actor": {"id": "test", "token": "dstok"}} + assert response.json.keys() == {"actor"} + actor = response.json["actor"] + expected_keys = {"id", "token"} + if scenario != "valid_unlimited_token": + expected_keys.add("token_expires") + assert actor.keys() == expected_keys + assert actor["id"] == "test" + assert actor["token"] == "dstok" + if scenario != "valid_unlimited_token": + assert isinstance(actor["token_expires"], int) else: assert response.json == {"actor": None} finally: @@ -251,15 +270,22 @@ def test_cli_create_token(app_client, expires): token = result.output.strip() assert token.startswith("dstok_") details = app_client.ds.unsign(token[len("dstok_") :], "token") - expected_keys = {"a", "token"} + expected_keys = {"a", "token", "t"} if expires: - expected_keys.add("e") + expected_keys.add("d") assert details.keys() == expected_keys assert details["a"] == "test" response = app_client.get( "/-/actor.json", headers={"Authorization": "Bearer {}".format(token)} ) if expires is None or expires > 0: - assert response.json == {"actor": {"id": "test", "token": "dstok"}} + expected_actor = { + "id": "test", + "token": "dstok", + } + if expires and expires > 0: + expected_actor["token_expires"] = details["t"] + expires + assert response.json == {"actor": expected_actor} else: - assert response.json == {"actor": None} + expected_actor = None + assert response.json == {"actor": expected_actor} From 51c436fed29205721dcf17fa31d7e7090d34ebb8 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 26 Oct 2022 20:57:02 -0700 Subject: [PATCH 018/823] First draft of insert row write API, refs #1851 --- datasette/default_permissions.py | 2 +- datasette/views/table.py | 76 +++++++++++++++++++++++++++----- docs/authentication.rst | 12 +++++ docs/cli-reference.rst | 2 + docs/json_api.rst | 38 ++++++++++++++++ 5 files changed, 119 insertions(+), 11 deletions(-) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index c502dd70..87684e2a 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -9,7 +9,7 @@ import time @hookimpl(tryfirst=True) def permission_allowed(datasette, actor, action, resource): async def inner(): - if action in ("permissions-debug", "debug-menu"): + if action in ("permissions-debug", "debug-menu", "insert-row"): if actor and actor.get("id") == "root": return True elif action == "view-instance": diff --git a/datasette/views/table.py b/datasette/views/table.py index f73b0957..74d1c532 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -28,7 +28,7 @@ from datasette.utils import ( urlsafe_components, value_as_boolean, ) -from datasette.utils.asgi import BadRequest, Forbidden, NotFound +from datasette.utils.asgi import BadRequest, Forbidden, NotFound, Response from datasette.filters import Filters from .base import DataView, DatasetteError, ureg from .database import QueryView @@ -103,15 +103,71 @@ class TableView(DataView): canned_query = await self.ds.get_canned_query( database_name, table_name, request.actor ) - assert canned_query, "You may only POST to a canned query" - return await QueryView(self.ds).data( - request, - canned_query["sql"], - metadata=canned_query, - editable=False, - canned_query=table_name, - named_parameters=canned_query.get("params"), - write=bool(canned_query.get("write")), + if canned_query: + return await QueryView(self.ds).data( + request, + canned_query["sql"], + metadata=canned_query, + editable=False, + canned_query=table_name, + named_parameters=canned_query.get("params"), + write=bool(canned_query.get("write")), + ) + else: + # Handle POST to a table + return await self.table_post(request, database_name, table_name) + + async def table_post(self, request, database_name, table_name): + # Table must exist (may handle table creation in the future) + db = self.ds.get_database(database_name) + if not await db.table_exists(table_name): + raise NotFound("Table not found: {}".format(table_name)) + # Must have insert-row permission + if not await self.ds.permission_allowed( + request.actor, "insert-row", resource=(database_name, table_name) + ): + raise Forbidden("Permission denied") + if request.headers.get("content-type") != "application/json": + # TODO: handle form-encoded data + raise BadRequest("Must send JSON data") + data = json.loads(await request.post_body()) + if "row" not in data: + raise BadRequest('Must send "row" data') + row = data["row"] + if not isinstance(row, dict): + raise BadRequest("row must be a dictionary") + # Verify all columns exist + columns = await db.table_columns(table_name) + pks = await db.primary_keys(table_name) + for key in row: + if key not in columns: + raise BadRequest("Column not found: {}".format(key)) + if key in pks: + raise BadRequest( + "Cannot insert into primary key column: {}".format(key) + ) + # Perform the insert + sql = "INSERT INTO [{table}] ({columns}) VALUES ({values})".format( + table=escape_sqlite(table_name), + columns=", ".join(escape_sqlite(c) for c in row), + values=", ".join("?" for c in row), + ) + cursor = await db.execute_write(sql, list(row.values())) + # Return the new row + rowid = cursor.lastrowid + new_row = ( + await db.execute( + "SELECT * FROM [{table}] WHERE rowid = ?".format( + table=escape_sqlite(table_name) + ), + [rowid], + ) + ).first() + return Response.json( + { + "row": dict(new_row), + }, + status=201, ) async def columns_to_select(self, table_columns, pks, request): diff --git a/docs/authentication.rst b/docs/authentication.rst index 0835e17c..233a50d2 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -547,6 +547,18 @@ Actor is allowed to view (and execute) a :ref:`canned query ` pa Default *allow*. +.. _permissions_insert_row: + +insert-row +---------- + +Actor is allowed to insert rows into a table. + +``resource`` - tuple: (string, string) + The name of the database, then the name of the table + +Default *deny*. + .. _permissions_execute_sql: execute-sql diff --git a/docs/cli-reference.rst b/docs/cli-reference.rst index b40c6b2c..56156568 100644 --- a/docs/cli-reference.rst +++ b/docs/cli-reference.rst @@ -229,6 +229,8 @@ These can be passed to ``datasette serve`` using ``datasette serve --setting nam database files (default=True) allow_signed_tokens Allow users to create and use signed API tokens (default=True) + max_signed_tokens_ttl Maximum allowed expiry time for signed API tokens + (default=0) suggest_facets Calculate and display suggested facets (default=True) default_cache_ttl Default HTTP cache TTL (used in Cache-Control: diff --git a/docs/json_api.rst b/docs/json_api.rst index d3fdb1e4..b339a738 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -455,3 +455,41 @@ You can find this near the top of the source code of those pages, looking like t The JSON URL is also made available in a ``Link`` HTTP header for the page:: Link: https://latest.datasette.io/fixtures/sortable.json; rel="alternate"; type="application/json+datasette" + +.. _json_api_write: + +The JSON write API +------------------ + +Datasette provides a write API for JSON data. This is a POST-only API that requires an authenticated API token, see :ref:`CreateTokenView`. + +.. _json_api_write_insert_row: + +Inserting a single row +~~~~~~~~~~~~~~~~~~~~~~ + +This requires the :ref:`permissions_insert_row` permission. + +:: + + POST // + Content-Type: application/json + Authorization: Bearer dstok_ + { + "row": { + "column1": "value1", + "column2": "value2" + } + } + +If successful, this will return a ``201`` status code and the newly inserted row, for example: + +.. code-block:: json + + { + "row": { + "id": 1, + "column1": "value1", + "column2": "value2" + } + } From 918f3561208ee58c44773d30e21bace7d7c7cf3b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 06:56:11 -0700 Subject: [PATCH 019/823] Delete mirror-master-and-main.yml Closes #1865 --- .github/workflows/mirror-master-and-main.yml | 21 -------------------- 1 file changed, 21 deletions(-) delete mode 100644 .github/workflows/mirror-master-and-main.yml diff --git a/.github/workflows/mirror-master-and-main.yml b/.github/workflows/mirror-master-and-main.yml deleted file mode 100644 index 8418df40..00000000 --- a/.github/workflows/mirror-master-and-main.yml +++ /dev/null @@ -1,21 +0,0 @@ -name: Mirror "master" and "main" branches -on: - push: - branches: - - master - - main - -jobs: - mirror: - runs-on: ubuntu-latest - steps: - - name: Mirror to "master" - uses: zofrex/mirror-branch@ea152f124954fa4eb26eea3fe0dbe313a3a08d94 - with: - target-branch: master - force: false - - name: Mirror to "main" - uses: zofrex/mirror-branch@ea152f124954fa4eb26eea3fe0dbe313a3a08d94 - with: - target-branch: main - force: false From b597bb6b3e7c4b449654bbfa5b01ceff3eb3cb33 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 11:47:41 -0700 Subject: [PATCH 020/823] Better comment handling in SQL regex, refs #1860 --- datasette/utils/__init__.py | 9 +++++---- tests/test_utils.py | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 977a66d6..5acfb8b4 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -208,16 +208,16 @@ class InvalidSql(Exception): # Allow SQL to start with a /* */ or -- comment comment_re = ( # Start of string, then any amount of whitespace - r"^(\s*" + r"^\s*(" + # Comment that starts with -- and ends at a newline r"(?:\-\-.*?\n\s*)" + - # Comment that starts with /* and ends with */ - r"|(?:/\*[\s\S]*?\*/)" + # Comment that starts with /* and ends with */ - but does not have */ in it + r"|(?:\/\*((?!\*\/)[\s\S])*\*\/)" + # Whitespace - r")*\s*" + r"\s*)*\s*" ) allowed_sql_res = [ @@ -228,6 +228,7 @@ allowed_sql_res = [ re.compile(comment_re + r"explain\s+with\b"), re.compile(comment_re + r"explain\s+query\s+plan\s+with\b"), ] + allowed_pragmas = ( "database_list", "foreign_key_list", diff --git a/tests/test_utils.py b/tests/test_utils.py index e89f1e6b..c1589107 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -142,6 +142,7 @@ def test_custom_json_encoder(obj, expected): "PRAGMA case_sensitive_like = true", "SELECT * FROM pragma_not_on_allow_list('idx52')", "/* This comment is not valid. select 1", + "/**/\nupdate foo set bar = 1\n/* test */ select 1", ], ) def test_validate_sql_select_bad(bad_sql): From 6958e21b5c2012adf5655d2512cb4106490d10f2 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 11:50:54 -0700 Subject: [PATCH 021/823] Add test for /* multi line */ comment, refs #1860 --- tests/test_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index c1589107..8b64f865 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -174,6 +174,7 @@ def test_validate_sql_select_bad(bad_sql): " /* comment */\nselect 1", " /* comment */select 1", "/* comment */\n -- another\n /* one more */ select 1", + "/* This comment \n has multiple lines */\nselect 1", ], ) def test_validate_sql_select_good(good_sql): From a51608090b5ee37593078f71d18b33767ef3af79 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 12:06:18 -0700 Subject: [PATCH 022/823] Slight tweak to insert row API design, refs #1851 https://github.com/simonw/datasette/issues/1851#issuecomment-1292997608 --- datasette/views/table.py | 10 +++++----- docs/json_api.rst | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/datasette/views/table.py b/datasette/views/table.py index 74d1c532..056b7b04 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -131,11 +131,11 @@ class TableView(DataView): # TODO: handle form-encoded data raise BadRequest("Must send JSON data") data = json.loads(await request.post_body()) - if "row" not in data: - raise BadRequest('Must send "row" data') - row = data["row"] + if "insert" not in data: + raise BadRequest('Must send a "insert" key containing a dictionary') + row = data["insert"] if not isinstance(row, dict): - raise BadRequest("row must be a dictionary") + raise BadRequest("insert must be a dictionary") # Verify all columns exist columns = await db.table_columns(table_name) pks = await db.primary_keys(table_name) @@ -165,7 +165,7 @@ class TableView(DataView): ).first() return Response.json( { - "row": dict(new_row), + "inserted_row": dict(new_row), }, status=201, ) diff --git a/docs/json_api.rst b/docs/json_api.rst index b339a738..2ed8a354 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -476,7 +476,7 @@ This requires the :ref:`permissions_insert_row` permission. Content-Type: application/json Authorization: Bearer dstok_ { - "row": { + "insert": { "column1": "value1", "column2": "value2" } @@ -487,7 +487,7 @@ If successful, this will return a ``201`` status code and the newly inserted row .. code-block:: json { - "row": { + "inserted_row": { "id": 1, "column1": "value1", "column2": "value2" From a2a5dff709c6f1676ac30b5e734c2763002562cf Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 12:08:26 -0700 Subject: [PATCH 023/823] Missing tests for insert row API, refs #1851 --- tests/test_api_write.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 tests/test_api_write.py diff --git a/tests/test_api_write.py b/tests/test_api_write.py new file mode 100644 index 00000000..86c221d0 --- /dev/null +++ b/tests/test_api_write.py @@ -0,0 +1,38 @@ +from datasette.app import Datasette +from datasette.utils import sqlite3 +import pytest +import time + + +@pytest.fixture +def ds_write(tmp_path_factory): + db_directory = tmp_path_factory.mktemp("dbs") + db_path = str(db_directory / "data.db") + db = sqlite3.connect(str(db_path)) + db.execute("vacuum") + db.execute("create table docs (id integer primary key, title text, score float)") + ds = Datasette([db_path]) + yield ds + db.close() + + +@pytest.mark.asyncio +async def test_write_row(ds_write): + token = "dstok_{}".format( + ds_write.sign( + {"a": "root", "token": "dstok", "t": int(time.time())}, namespace="token" + ) + ) + response = await ds_write.client.post( + "/data/docs", + json={"insert": {"title": "Test", "score": 1.0}}, + headers={ + "Authorization": "Bearer {}".format(token), + "Content-Type": "application/json", + }, + ) + expected_row = {"id": 1, "title": "Test", "score": 1.0} + assert response.status_code == 201 + assert response.json()["inserted_row"] == expected_row + rows = (await ds_write.get_database("data").execute("select * from docs")).rows + assert dict(rows[0]) == expected_row From 6e788b49edf4f842c0817f006eb9d865778eea5e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 13:17:18 -0700 Subject: [PATCH 024/823] New URL design /db/table/-/insert, refs #1851 --- datasette/app.py | 6 +++- datasette/views/table.py | 69 +++++++++++++++++++++++++++++++++++++++- docs/json_api.rst | 18 ++++++----- tests/test_api_write.py | 6 ++-- 4 files changed, 86 insertions(+), 13 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 894d7f0f..8bc5fe36 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -39,7 +39,7 @@ from .views.special import ( PermissionsDebugView, MessagesDebugView, ) -from .views.table import TableView +from .views.table import TableView, TableInsertView from .views.row import RowView from .renderer import json_renderer from .url_builder import Urls @@ -1262,6 +1262,10 @@ class Datasette: RowView.as_view(self), r"/(?P[^\/\.]+)/(?P
[^/]+?)/(?P[^/]+?)(\.(?P\w+))?$", ) + add_route( + TableInsertView.as_view(self), + r"/(?P[^\/\.]+)/(?P
[^\/\.]+)/-/insert$", + ) return [ # Compile any strings to regular expressions ((re.compile(pattern) if isinstance(pattern, str) else pattern), view) diff --git a/datasette/views/table.py b/datasette/views/table.py index 056b7b04..be3d4f93 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -30,7 +30,7 @@ from datasette.utils import ( ) from datasette.utils.asgi import BadRequest, Forbidden, NotFound, Response from datasette.filters import Filters -from .base import DataView, DatasetteError, ureg +from .base import BaseView, DataView, DatasetteError, ureg from .database import QueryView LINK_WITH_LABEL = ( @@ -1077,3 +1077,70 @@ async def display_columns_and_rows( } columns = [first_column] + columns return columns, cell_rows + + +class TableInsertView(BaseView): + name = "table-insert" + + def __init__(self, datasette): + self.ds = datasette + + async def post(self, request): + database_route = tilde_decode(request.url_vars["database"]) + try: + db = self.ds.get_database(route=database_route) + except KeyError: + raise NotFound("Database not found: {}".format(database_route)) + database_name = db.name + table_name = tilde_decode(request.url_vars["table"]) + # Table must exist (may handle table creation in the future) + db = self.ds.get_database(database_name) + if not await db.table_exists(table_name): + raise NotFound("Table not found: {}".format(table_name)) + # Must have insert-row permission + if not await self.ds.permission_allowed( + request.actor, "insert-row", resource=(database_name, table_name) + ): + raise Forbidden("Permission denied") + if request.headers.get("content-type") != "application/json": + # TODO: handle form-encoded data + raise BadRequest("Must send JSON data") + data = json.loads(await request.post_body()) + if "row" not in data: + raise BadRequest('Must send a "row" key containing a dictionary') + row = data["row"] + if not isinstance(row, dict): + raise BadRequest("row must be a dictionary") + # Verify all columns exist + columns = await db.table_columns(table_name) + pks = await db.primary_keys(table_name) + for key in row: + if key not in columns: + raise BadRequest("Column not found: {}".format(key)) + if key in pks: + raise BadRequest( + "Cannot insert into primary key column: {}".format(key) + ) + # Perform the insert + sql = "INSERT INTO [{table}] ({columns}) VALUES ({values})".format( + table=escape_sqlite(table_name), + columns=", ".join(escape_sqlite(c) for c in row), + values=", ".join("?" for c in row), + ) + cursor = await db.execute_write(sql, list(row.values())) + # Return the new row + rowid = cursor.lastrowid + new_row = ( + await db.execute( + "SELECT * FROM [{table}] WHERE rowid = ?".format( + table=escape_sqlite(table_name) + ), + [rowid], + ) + ).first() + return Response.json( + { + "inserted": [dict(new_row)], + }, + status=201, + ) diff --git a/docs/json_api.rst b/docs/json_api.rst index 2ed8a354..4a7961f2 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -463,7 +463,7 @@ The JSON write API Datasette provides a write API for JSON data. This is a POST-only API that requires an authenticated API token, see :ref:`CreateTokenView`. -.. _json_api_write_insert_row: +.. _TableInsertView: Inserting a single row ~~~~~~~~~~~~~~~~~~~~~~ @@ -472,11 +472,11 @@ This requires the :ref:`permissions_insert_row` permission. :: - POST //
+ POST //
/-/insert Content-Type: application/json Authorization: Bearer dstok_ { - "insert": { + "row": { "column1": "value1", "column2": "value2" } @@ -487,9 +487,11 @@ If successful, this will return a ``201`` status code and the newly inserted row .. code-block:: json { - "inserted_row": { - "id": 1, - "column1": "value1", - "column2": "value2" - } + "inserted": [ + { + "id": 1, + "column1": "value1", + "column2": "value2" + } + ] } diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 86c221d0..e8222e43 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -24,8 +24,8 @@ async def test_write_row(ds_write): ) ) response = await ds_write.client.post( - "/data/docs", - json={"insert": {"title": "Test", "score": 1.0}}, + "/data/docs/-/insert", + json={"row": {"title": "Test", "score": 1.0}}, headers={ "Authorization": "Bearer {}".format(token), "Content-Type": "application/json", @@ -33,6 +33,6 @@ async def test_write_row(ds_write): ) expected_row = {"id": 1, "title": "Test", "score": 1.0} assert response.status_code == 201 - assert response.json()["inserted_row"] == expected_row + assert response.json()["inserted"] == [expected_row] rows = (await ds_write.get_database("data").execute("select * from docs")).rows assert dict(rows[0]) == expected_row From 2ea60e12d90b7cec03ebab728854d3ec4d553f54 Mon Sep 17 00:00:00 2001 From: Forest Gregg Date: Thu, 27 Oct 2022 16:51:20 -0400 Subject: [PATCH 025/823] Make hash and size a lazy property (#1837) * use inspect data for hash and file size * make hash and cached_size lazy properties * move hash property near size --- datasette/database.py | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/datasette/database.py b/datasette/database.py index d75bd70c..af1df0a8 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -39,7 +39,7 @@ class Database: self.memory_name = memory_name if memory_name is not None: self.is_memory = True - self.hash = None + self.cached_hash = None self.cached_size = None self._cached_table_counts = None self._write_thread = None @@ -47,14 +47,6 @@ class Database: # These are used when in non-threaded mode: self._read_connection = None self._write_connection = None - if not self.is_mutable and not self.is_memory: - if self.ds.inspect_data and self.ds.inspect_data.get(self.name): - self.hash = self.ds.inspect_data[self.name]["hash"] - self.cached_size = self.ds.inspect_data[self.name]["size"] - else: - p = Path(path) - self.hash = inspect_hash(p) - self.cached_size = p.stat().st_size @property def cached_table_counts(self): @@ -266,14 +258,34 @@ class Database: results = await self.execute_fn(sql_operation_in_thread) return results + @property + def hash(self): + if self.cached_hash is not None: + return self.cached_hash + elif self.is_mutable or self.is_memory: + return None + elif self.ds.inspect_data and self.ds.inspect_data.get(self.name): + self.cached_hash = self.ds.inspect_data[self.name]["hash"] + return self.cached_hash + else: + p = Path(self.path) + self.cached_hash = inspect_hash(p) + return self.cached_hash + @property def size(self): - if self.is_memory: - return 0 if self.cached_size is not None: return self.cached_size - else: + elif self.is_memory: + return 0 + elif self.is_mutable: return Path(self.path).stat().st_size + elif self.ds.inspect_data and self.ds.inspect_data.get(self.name): + self.cached_size = self.ds.inspect_data[self.name]["size"] + return self.cached_size + else: + self.cached_size = Path(self.path).stat().st_size + return self.cached_size async def table_counts(self, limit=10): if not self.is_mutable and self.cached_table_counts is not None: From 641bc4453b5ef1dff0b2fc7dfad0b692be7aa61c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 27 Oct 2022 13:51:45 -0700 Subject: [PATCH 026/823] Bump black from 22.8.0 to 22.10.0 (#1839) Bumps [black](https://github.com/psf/black) from 22.8.0 to 22.10.0. - [Release notes](https://github.com/psf/black/releases) - [Changelog](https://github.com/psf/black/blob/main/CHANGES.md) - [Commits](https://github.com/psf/black/compare/22.8.0...22.10.0) --- updated-dependencies: - dependency-name: black dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index fe258adb..625557ae 100644 --- a/setup.py +++ b/setup.py @@ -76,7 +76,7 @@ setup( "pytest-xdist>=2.2.1", "pytest-asyncio>=0.17", "beautifulsoup4>=4.8.1", - "black==22.8.0", + "black==22.10.0", "blacken-docs==1.12.1", "pytest-timeout>=1.4.2", "trustme>=0.7", From 26af9b9c4a6c62ee15870caa1c7bc455165d3b11 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 13:58:00 -0700 Subject: [PATCH 027/823] Release notes for 0.63, refs #1869 --- docs/changelog.rst | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2255dcce..01957e4f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,36 +4,42 @@ Changelog ========= -.. _v0_63a1: +.. _v0_63: -0.63a1 (2022-10-23) -------------------- +0.63 (2022-10-27) +----------------- +Features +~~~~~~~~ + +- Now tested against Python 3.11. Docker containers used by ``datasette publish`` and ``datasette package`` both now use that version of Python. (:issue:`1853`) +- ``--load-extension`` option now supports entrypoints. Thanks, Alex Garcia. (`#1789 `__) +- Facet size can now be set per-table with the new ``facet_size`` table metadata option. (:issue:`1804`) +- The :ref:`setting_truncate_cells_html` setting now also affects long URLs in columns. (:issue:`1805`) +- The non-JavaScript SQL editor textarea now increases height to fit the SQL query. (:issue:`1786`) +- Facets are now displayed with better line-breaks in long values. Thanks, Daniel Rech. (`#1794 `__) +- The ``settings.json`` file used in :ref:`config_dir` is now validated on startup. (:issue:`1816`) +- SQL queries can now include leading SQL comments, using ``/* ... */`` or ``-- ...`` syntax. Thanks, Charles Nepote. (:issue:`1860`) - SQL query is now re-displayed when terminated with a time limit error. (:issue:`1819`) -- New documentation on :ref:`deploying_openrc` - thanks, Adam Simpson. (`#1825 `__) - The :ref:`inspect data ` mechanism is now used to speed up server startup - thanks, Forest Gregg. (:issue:`1834`) - In :ref:`config_dir` databases with filenames ending in ``.sqlite`` or ``.sqlite3`` are now automatically added to the Datasette instance. (:issue:`1646`) - Breadcrumb navigation display now respects the current user's permissions. (:issue:`1831`) -- Screenshots in the documentation are now maintained using `shot-scraper `__, as described in `Automating screenshots for the Datasette documentation using shot-scraper `__. (:issue:`1844`) -- The :ref:`datasette.check_visibility() ` method now accepts an optional ``permissions=`` list, allowing it to take multiple permissions into account at once when deciding if something should be shown as public or private. This has been used to correctly display padlock icons in more places in the Datasette interface. (:issue:`1829`) - -.. _v0_63a0: - -0.63a0 (2022-09-26) -------------------- +Plugin hooks and internals +~~~~~~~~~~~~~~~~~~~~~~~~~~ - The :ref:`plugin_hook_prepare_jinja2_environment` plugin hook now accepts an optional ``datasette`` argument. Hook implementations can also now return an ``async`` function which will be awaited automatically. (:issue:`1809`) -- ``--load-extension`` option now supports entrypoints. Thanks, Alex Garcia. (`#1789 `__) -- New tutorial: `Cleaning data with sqlite-utils and Datasette `__. -- Facet size can now be set per-table with the new ``facet_size`` table metadata option. (:issue:`1804`) -- ``truncate_cells_html`` setting now also affects long URLs in columns. (:issue:`1805`) - ``Database(is_mutable=)`` now defaults to ``True``. (:issue:`1808`) -- Non-JavaScript textarea now increases height to fit the SQL query. (:issue:`1786`) -- More detailed command descriptions on the :ref:`CLI reference ` page. (:issue:`1787`) +- The :ref:`datasette.check_visibility() ` method now accepts an optional ``permissions=`` list, allowing it to take multiple permissions into account at once when deciding if something should be shown as public or private. This has been used to correctly display padlock icons in more places in the Datasette interface. (:issue:`1829`) - Datasette no longer enforces upper bounds on its dependencies. (:issue:`1800`) -- Facets are now displayed with better line-breaks in long values. Thanks, Daniel Rech. (`#1794 `__) -- The ``settings.json`` file used in :ref:`config_dir` is now validated on startup. (:issue:`1816`) + +Documentation +~~~~~~~~~~~~~ + +- New tutorial: `Cleaning data with sqlite-utils and Datasette `__. +- Screenshots in the documentation are now maintained using `shot-scraper `__, as described in `Automating screenshots for the Datasette documentation using shot-scraper `__. (:issue:`1844`) +- More detailed command descriptions on the :ref:`CLI reference ` page. (:issue:`1787`) +- New documentation on :ref:`deploying_openrc` - thanks, Adam Simpson. (`#1825 `__) .. _v0_62: From 61171f01549549e5fb25c72b13280d941d96dbf1 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 15:11:26 -0700 Subject: [PATCH 028/823] Release 0.63 Refs #1646, #1786, #1787, #1789, #1794, #1800, #1804, #1805, #1808, #1809, #1816, #1819, #1825, #1829, #1831, #1834, #1844, #1853, #1860 Closes #1869 --- datasette/version.py | 2 +- docs/changelog.rst | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/datasette/version.py b/datasette/version.py index eb36da45..ac012640 100644 --- a/datasette/version.py +++ b/datasette/version.py @@ -1,2 +1,2 @@ -__version__ = "0.63a1" +__version__ = "0.63" __version_info__ = tuple(__version__.split(".")) diff --git a/docs/changelog.rst b/docs/changelog.rst index 01957e4f..f573afb3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,8 @@ Changelog 0.63 (2022-10-27) ----------------- +See `Datasette 0.63: The annotated release notes `__ for more background on the changes in this release. + Features ~~~~~~~~ From c9b5f5d598e7f85cd3e1ce020351a27da334408b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 27 Oct 2022 17:58:36 -0700 Subject: [PATCH 029/823] Depend on sqlite-utils>=3.30 Decided to use the most recent version in case I decide later to use the flatten() utility function. Refs #1850 --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 625557ae..99e2a4ad 100644 --- a/setup.py +++ b/setup.py @@ -57,6 +57,7 @@ setup( "PyYAML>=5.3", "mergedeep>=1.1.1", "itsdangerous>=1.1", + "sqlite-utils>=3.30", ], entry_points=""" [console_scripts] From c35859ae3df163406f1a1895ccf9803e933b2d8e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 29 Oct 2022 23:03:45 -0700 Subject: [PATCH 030/823] API for bulk inserts, closes #1866 --- datasette/app.py | 5 ++ datasette/views/table.py | 136 +++++++++++++++++++++---------- docs/cli-reference.rst | 2 + docs/json_api.rst | 48 ++++++++++- docs/settings.rst | 11 +++ tests/test_api.py | 1 + tests/test_api_write.py | 168 +++++++++++++++++++++++++++++++++++++-- 7 files changed, 320 insertions(+), 51 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 8bc5fe36..f80d3792 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -99,6 +99,11 @@ SETTINGS = ( 1000, "Maximum rows that can be returned from a table or custom query", ), + Setting( + "max_insert_rows", + 100, + "Maximum rows that can be inserted at a time using the bulk insert API", + ), Setting( "num_sql_threads", 3, diff --git a/datasette/views/table.py b/datasette/views/table.py index be3d4f93..fd203036 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -30,6 +30,7 @@ from datasette.utils import ( ) from datasette.utils.asgi import BadRequest, Forbidden, NotFound, Response from datasette.filters import Filters +import sqlite_utils from .base import BaseView, DataView, DatasetteError, ureg from .database import QueryView @@ -1085,62 +1086,109 @@ class TableInsertView(BaseView): def __init__(self, datasette): self.ds = datasette + async def _validate_data(self, request, db, table_name): + errors = [] + + def _errors(errors): + return None, errors, {} + + if request.headers.get("content-type") != "application/json": + # TODO: handle form-encoded data + return _errors(["Invalid content-type, must be application/json"]) + body = await request.post_body() + try: + data = json.loads(body) + except json.JSONDecodeError as e: + return _errors(["Invalid JSON: {}".format(e)]) + if not isinstance(data, dict): + return _errors(["JSON must be a dictionary"]) + keys = data.keys() + # keys must contain "row" or "rows" + if "row" not in keys and "rows" not in keys: + return _errors(['JSON must have one or other of "row" or "rows"']) + rows = [] + if "row" in keys: + if "rows" in keys: + return _errors(['Cannot use "row" and "rows" at the same time']) + row = data["row"] + if not isinstance(row, dict): + return _errors(['"row" must be a dictionary']) + rows = [row] + data["return_rows"] = True + else: + rows = data["rows"] + if not isinstance(rows, list): + return _errors(['"rows" must be a list']) + for row in rows: + if not isinstance(row, dict): + return _errors(['"rows" must be a list of dictionaries']) + # Does this exceed max_insert_rows? + max_insert_rows = self.ds.setting("max_insert_rows") + if len(rows) > max_insert_rows: + return _errors( + ["Too many rows, maximum allowed is {}".format(max_insert_rows)] + ) + # Validate columns of each row + columns = await db.table_columns(table_name) + # TODO: There are cases where pks are OK, if not using auto-incrementing pk + pks = await db.primary_keys(table_name) + allowed_columns = set(columns) - set(pks) + for i, row in enumerate(rows): + invalid_columns = set(row.keys()) - allowed_columns + if invalid_columns: + errors.append( + "Row {} has invalid columns: {}".format( + i, ", ".join(sorted(invalid_columns)) + ) + ) + if errors: + return _errors(errors) + extra = {key: data[key] for key in data if key not in ("rows", "row")} + return rows, errors, extra + async def post(self, request): + def _error(messages, status=400): + return Response.json({"ok": False, "errors": messages}, status=status) + database_route = tilde_decode(request.url_vars["database"]) try: db = self.ds.get_database(route=database_route) except KeyError: - raise NotFound("Database not found: {}".format(database_route)) + return _error(["Database not found: {}".format(database_route)], 404) database_name = db.name table_name = tilde_decode(request.url_vars["table"]) + # Table must exist (may handle table creation in the future) db = self.ds.get_database(database_name) if not await db.table_exists(table_name): - raise NotFound("Table not found: {}".format(table_name)) + return _error(["Table not found: {}".format(table_name)], 404) # Must have insert-row permission if not await self.ds.permission_allowed( request.actor, "insert-row", resource=(database_name, table_name) ): - raise Forbidden("Permission denied") - if request.headers.get("content-type") != "application/json": - # TODO: handle form-encoded data - raise BadRequest("Must send JSON data") - data = json.loads(await request.post_body()) - if "row" not in data: - raise BadRequest('Must send a "row" key containing a dictionary') - row = data["row"] - if not isinstance(row, dict): - raise BadRequest("row must be a dictionary") - # Verify all columns exist - columns = await db.table_columns(table_name) - pks = await db.primary_keys(table_name) - for key in row: - if key not in columns: - raise BadRequest("Column not found: {}".format(key)) - if key in pks: - raise BadRequest( - "Cannot insert into primary key column: {}".format(key) + return _error(["Permission denied"], 403) + rows, errors, extra = await self._validate_data(request, db, table_name) + if errors: + return _error(errors, 400) + + should_return = bool(extra.get("return_rows", False)) + # Insert rows + def insert_rows(conn): + table = sqlite_utils.Database(conn)[table_name] + if should_return: + rowids = [] + for row in rows: + rowids.append(table.insert(row).last_rowid) + return list( + table.rows_where( + "rowid in ({})".format(",".join("?" for _ in rowids)), rowids + ) ) - # Perform the insert - sql = "INSERT INTO [{table}] ({columns}) VALUES ({values})".format( - table=escape_sqlite(table_name), - columns=", ".join(escape_sqlite(c) for c in row), - values=", ".join("?" for c in row), - ) - cursor = await db.execute_write(sql, list(row.values())) - # Return the new row - rowid = cursor.lastrowid - new_row = ( - await db.execute( - "SELECT * FROM [{table}] WHERE rowid = ?".format( - table=escape_sqlite(table_name) - ), - [rowid], - ) - ).first() - return Response.json( - { - "inserted": [dict(new_row)], - }, - status=201, - ) + else: + table.insert_all(rows) + + rows = await db.execute_write_fn(insert_rows) + result = {"ok": True} + if should_return: + result["inserted"] = rows + return Response.json(result, status=201) diff --git a/docs/cli-reference.rst b/docs/cli-reference.rst index 56156568..649a3dcd 100644 --- a/docs/cli-reference.rst +++ b/docs/cli-reference.rst @@ -213,6 +213,8 @@ These can be passed to ``datasette serve`` using ``datasette serve --setting nam (default=100) max_returned_rows Maximum rows that can be returned from a table or custom query (default=1000) + max_insert_rows Maximum rows that can be inserted at a time using + the bulk insert API (default=1000) num_sql_threads Number of threads in the thread pool for executing SQLite queries (default=3) sql_time_limit_ms Time limit for a SQL query in milliseconds diff --git a/docs/json_api.rst b/docs/json_api.rst index 4a7961f2..01558c23 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -465,11 +465,13 @@ Datasette provides a write API for JSON data. This is a POST-only API that requi .. _TableInsertView: -Inserting a single row -~~~~~~~~~~~~~~~~~~~~~~ +Inserting rows +~~~~~~~~~~~~~~ This requires the :ref:`permissions_insert_row` permission. +A single row can be inserted using the ``"row"`` key: + :: POST //
/-/insert @@ -495,3 +497,45 @@ If successful, this will return a ``201`` status code and the newly inserted row } ] } + +To insert multiple rows at a time, use the same API method but send a list of dictionaries as the ``"rows"`` key: + +:: + + POST //
/-/insert + Content-Type: application/json + Authorization: Bearer dstok_ + { + "rows": [ + { + "column1": "value1", + "column2": "value2" + }, + { + "column1": "value3", + "column2": "value4" + } + ] + } + +If successful, this will return a ``201`` status code and an empty ``{}`` response body. + +To return the newly inserted rows, add the ``"return_rows": true`` key to the request body: + +.. code-block:: json + + { + "rows": [ + { + "column1": "value1", + "column2": "value2" + }, + { + "column1": "value3", + "column2": "value4" + } + ], + "return_rows": true + } + +This will return the same ``"inserted"`` key as the single row example above. There is a small performance penalty for using this option. diff --git a/docs/settings.rst b/docs/settings.rst index a990c78c..b86b18bd 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -96,6 +96,17 @@ You can increase or decrease this limit like so:: datasette mydatabase.db --setting max_returned_rows 2000 +.. _setting_max_insert_rows: + +max_insert_rows +~~~~~~~~~~~~~~~ + +Maximum rows that can be inserted at a time using the bulk insert API, see :ref:`TableInsertView`. Defaults to 100. + +You can increase or decrease this limit like so:: + + datasette mydatabase.db --setting max_insert_rows 1000 + .. _setting_num_sql_threads: num_sql_threads diff --git a/tests/test_api.py b/tests/test_api.py index fc171421..ebd675b9 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -804,6 +804,7 @@ def test_settings_json(app_client): "facet_suggest_time_limit_ms": 50, "facet_time_limit_ms": 200, "max_returned_rows": 100, + "max_insert_rows": 100, "sql_time_limit_ms": 200, "allow_download": True, "allow_signed_tokens": True, diff --git a/tests/test_api_write.py b/tests/test_api_write.py index e8222e43..4a5a58aa 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -18,11 +18,7 @@ def ds_write(tmp_path_factory): @pytest.mark.asyncio async def test_write_row(ds_write): - token = "dstok_{}".format( - ds_write.sign( - {"a": "root", "token": "dstok", "t": int(time.time())}, namespace="token" - ) - ) + token = write_token(ds_write) response = await ds_write.client.post( "/data/docs/-/insert", json={"row": {"title": "Test", "score": 1.0}}, @@ -36,3 +32,165 @@ async def test_write_row(ds_write): assert response.json()["inserted"] == [expected_row] rows = (await ds_write.get_database("data").execute("select * from docs")).rows assert dict(rows[0]) == expected_row + + +@pytest.mark.asyncio +@pytest.mark.parametrize("return_rows", (True, False)) +async def test_write_rows(ds_write, return_rows): + token = write_token(ds_write) + data = {"rows": [{"title": "Test {}".format(i), "score": 1.0} for i in range(20)]} + if return_rows: + data["return_rows"] = True + response = await ds_write.client.post( + "/data/docs/-/insert", + json=data, + headers={ + "Authorization": "Bearer {}".format(token), + "Content-Type": "application/json", + }, + ) + assert response.status_code == 201 + actual_rows = [ + dict(r) + for r in ( + await ds_write.get_database("data").execute("select * from docs") + ).rows + ] + assert len(actual_rows) == 20 + assert actual_rows == [ + {"id": i + 1, "title": "Test {}".format(i), "score": 1.0} for i in range(20) + ] + assert response.json()["ok"] is True + if return_rows: + assert response.json()["inserted"] == actual_rows + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path,input,special_case,expected_status,expected_errors", + ( + ( + "/data2/docs/-/insert", + {}, + None, + 404, + ["Database not found: data2"], + ), + ( + "/data/docs2/-/insert", + {}, + None, + 404, + ["Table not found: docs2"], + ), + ( + "/data/docs/-/insert", + {"rows": [{"title": "Test"} for i in range(10)]}, + "bad_token", + 403, + ["Permission denied"], + ), + ( + "/data/docs/-/insert", + {}, + "invalid_json", + 400, + [ + "Invalid JSON: Expecting property name enclosed in double quotes: line 1 column 2 (char 1)" + ], + ), + ( + "/data/docs/-/insert", + {}, + "invalid_content_type", + 400, + ["Invalid content-type, must be application/json"], + ), + ( + "/data/docs/-/insert", + [], + None, + 400, + ["JSON must be a dictionary"], + ), + ( + "/data/docs/-/insert", + {"row": "blah"}, + None, + 400, + ['"row" must be a dictionary'], + ), + ( + "/data/docs/-/insert", + {"blah": "blah"}, + None, + 400, + ['JSON must have one or other of "row" or "rows"'], + ), + ( + "/data/docs/-/insert", + {"rows": "blah"}, + None, + 400, + ['"rows" must be a list'], + ), + ( + "/data/docs/-/insert", + {"rows": ["blah"]}, + None, + 400, + ['"rows" must be a list of dictionaries'], + ), + ( + "/data/docs/-/insert", + {"rows": [{"title": "Test"} for i in range(101)]}, + None, + 400, + ["Too many rows, maximum allowed is 100"], + ), + # Validate columns of each row + ( + "/data/docs/-/insert", + {"rows": [{"title": "Test", "bad": 1, "worse": 2} for i in range(2)]}, + None, + 400, + [ + "Row 0 has invalid columns: bad, worse", + "Row 1 has invalid columns: bad, worse", + ], + ), + ), +) +async def test_write_row_errors( + ds_write, path, input, special_case, expected_status, expected_errors +): + token = write_token(ds_write) + if special_case == "bad_token": + token += "bad" + kwargs = dict( + json=input, + headers={ + "Authorization": "Bearer {}".format(token), + "Content-Type": "text/plain" + if special_case == "invalid_content_type" + else "application/json", + }, + ) + if special_case == "invalid_json": + del kwargs["json"] + kwargs["content"] = "{bad json" + response = await ds_write.client.post( + path, + **kwargs, + ) + assert response.status_code == expected_status + assert response.json()["ok"] is False + assert response.json()["errors"] == expected_errors + + +def write_token(ds): + return "dstok_{}".format( + ds.sign( + {"a": "root", "token": "dstok", "t": int(time.time())}, namespace="token" + ) + ) From f6bf2d8045cc239fe34357342bff1440561c8909 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 29 Oct 2022 23:20:11 -0700 Subject: [PATCH 031/823] Initial prototype of API explorer at /-/api, refs #1871 --- datasette/app.py | 5 ++ datasette/templates/api_explorer.html | 73 +++++++++++++++++++++++++++ datasette/views/special.py | 8 +++ tests/test_docs.py | 2 +- 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 datasette/templates/api_explorer.html diff --git a/datasette/app.py b/datasette/app.py index f80d3792..c3d802a4 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -33,6 +33,7 @@ from .views.special import ( JsonDataView, PatternPortfolioView, AuthTokenView, + ApiExplorerView, CreateTokenView, LogoutView, AllowDebugView, @@ -1235,6 +1236,10 @@ class Datasette: CreateTokenView.as_view(self), r"/-/create-token$", ) + add_route( + ApiExplorerView.as_view(self), + r"/-/api$", + ) add_route( LogoutView.as_view(self), r"/-/logout$", diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html new file mode 100644 index 00000000..034bee60 --- /dev/null +++ b/datasette/templates/api_explorer.html @@ -0,0 +1,73 @@ +{% extends "base.html" %} + +{% block title %}API Explorer{% endblock %} + +{% block content %} + +

API Explorer

+ +

Use this tool to try out the Datasette write API.

+ +{% if errors %} + {% for error in errors %} +

{{ error }}

+ {% endfor %} +{% endif %} + + +
+ + +
+
+ + +
+
+ +
+

+ + + + +{% endblock %} diff --git a/datasette/views/special.py b/datasette/views/special.py index b754a2f0..9922a621 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -235,3 +235,11 @@ class CreateTokenView(BaseView): "token_bits": token_bits, }, ) + + +class ApiExplorerView(BaseView): + name = "api_explorer" + has_json_alternate = False + + async def get(self, request): + return await self.render(["api_explorer.html"], request) diff --git a/tests/test_docs.py b/tests/test_docs.py index cd5a6c13..e9b813fe 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -62,7 +62,7 @@ def documented_views(): if first_word.endswith("View"): view_labels.add(first_word) # We deliberately don't document these: - view_labels.update(("PatternPortfolioView", "AuthTokenView")) + view_labels.update(("PatternPortfolioView", "AuthTokenView", "ApiExplorerView")) return view_labels From 9eb9ffae3ddd4e8ff0b713bf6fd6a0afed3368d7 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 30 Oct 2022 13:09:55 -0700 Subject: [PATCH 032/823] Drop API token requirement from API explorer, refs #1871 --- datasette/default_permissions.py | 9 +++++++++ datasette/templates/api_explorer.html | 13 ++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 87684e2a..151ba2b5 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -131,3 +131,12 @@ def register_commands(cli): if debug: click.echo("\nDecoded:\n") click.echo(json.dumps(ds.unsign(token, namespace="token"), indent=2)) + + +@hookimpl +def skip_csrf(scope): + # Skip CSRF check for requests with content-type: application/json + if scope["type"] == "http": + headers = scope.get("headers") or {} + if dict(headers).get(b"content-type") == b"application/json": + return True diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html index 034bee60..01b182d8 100644 --- a/datasette/templates/api_explorer.html +++ b/datasette/templates/api_explorer.html @@ -15,16 +15,13 @@ {% endif %}
-
- - -
- +
-
- +
+ +

@@ -46,7 +43,6 @@ form.addEventListener("submit", (ev) => { var formData = new FormData(form); var json = formData.get('json'); var path = formData.get('path'); - var token = formData.get('token'); // Validate JSON try { var data = JSON.parse(json); @@ -60,7 +56,6 @@ form.addEventListener("submit", (ev) => { body: json, headers: { 'Content-Type': 'application/json', - 'Authorization': `Bearer ${token}` } }).then(r => r.json()).then(r => { alert(JSON.stringify(r, null, 2)); From fedbfcc36873366143195d8fe124e1859bf88346 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 30 Oct 2022 14:49:07 -0700 Subject: [PATCH 033/823] Neater display of output and errors in API explorer, refs #1871 --- datasette/templates/api_explorer.html | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html index 01b182d8..38fdb7bc 100644 --- a/datasette/templates/api_explorer.html +++ b/datasette/templates/api_explorer.html @@ -26,6 +26,12 @@

+ + """.format( escape(ex.sql) ) diff --git a/tests/test_api.py b/tests/test_api.py index ebd675b9..de0223e2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -662,7 +662,11 @@ def test_sql_time_limit(app_client_shorter_time_limit): "

SQL query took too long. The time limit is controlled by the\n" 'sql_time_limit_ms\n' "configuration option.

\n" - "
select sleep(0.5)
" + '\n' + "" ), "status": 400, "title": "SQL Interrupted", diff --git a/tests/test_html.py b/tests/test_html.py index 4b394199..7cfe9d90 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -172,7 +172,7 @@ def test_sql_time_limit(app_client_shorter_time_limit): """ sql_time_limit_ms """.strip(), - "
select sleep(0.5)
", + '', ] for expected_html_fragment in expected_html_fragments: assert expected_html_fragment in response.text From 9bec7c38eb93cde5afb16df9bdd96aea2a5b0459 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 1 Nov 2022 11:07:59 -0700 Subject: [PATCH 038/823] ignore and replace options for bulk inserts, refs #1873 Also removed the rule that you cannot include primary keys in the rows you insert. And added validation that catches invalid parameters in the incoming JSON. And renamed "inserted" to "rows" in the returned JSON for return_rows: true --- datasette/views/table.py | 41 ++++++++++++++------ docs/json_api.rst | 4 +- tests/test_api_write.py | 83 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 111 insertions(+), 17 deletions(-) diff --git a/datasette/views/table.py b/datasette/views/table.py index 1e3d566e..7692a4e3 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -1107,6 +1107,7 @@ class TableInsertView(BaseView): if not isinstance(data, dict): return _errors(["JSON must be a dictionary"]) keys = data.keys() + # keys must contain "row" or "rows" if "row" not in keys and "rows" not in keys: return _errors(['JSON must have one or other of "row" or "rows"']) @@ -1126,19 +1127,31 @@ class TableInsertView(BaseView): for row in rows: if not isinstance(row, dict): return _errors(['"rows" must be a list of dictionaries']) + # Does this exceed max_insert_rows? max_insert_rows = self.ds.setting("max_insert_rows") if len(rows) > max_insert_rows: return _errors( ["Too many rows, maximum allowed is {}".format(max_insert_rows)] ) + + # Validate other parameters + extras = { + key: value for key, value in data.items() if key not in ("row", "rows") + } + valid_extras = {"return_rows", "ignore", "replace"} + invalid_extras = extras.keys() - valid_extras + if invalid_extras: + return _errors( + ['Invalid parameter: "{}"'.format('", "'.join(sorted(invalid_extras)))] + ) + if extras.get("ignore") and extras.get("replace"): + return _errors(['Cannot use "ignore" and "replace" at the same time']) + # Validate columns of each row - columns = await db.table_columns(table_name) - # TODO: There are cases where pks are OK, if not using auto-incrementing pk - pks = await db.primary_keys(table_name) - allowed_columns = set(columns) - set(pks) + columns = set(await db.table_columns(table_name)) for i, row in enumerate(rows): - invalid_columns = set(row.keys()) - allowed_columns + invalid_columns = set(row.keys()) - columns if invalid_columns: errors.append( "Row {} has invalid columns: {}".format( @@ -1147,8 +1160,7 @@ class TableInsertView(BaseView): ) if errors: return _errors(errors) - extra = {key: data[key] for key in data if key not in ("rows", "row")} - return rows, errors, extra + return rows, errors, extras async def post(self, request): database_route = tilde_decode(request.url_vars["database"]) @@ -1168,18 +1180,23 @@ class TableInsertView(BaseView): request.actor, "insert-row", resource=(database_name, table_name) ): return _error(["Permission denied"], 403) - rows, errors, extra = await self._validate_data(request, db, table_name) + rows, errors, extras = await self._validate_data(request, db, table_name) if errors: return _error(errors, 400) - should_return = bool(extra.get("return_rows", False)) + ignore = extras.get("ignore") + replace = extras.get("replace") + + should_return = bool(extras.get("return_rows", False)) # Insert rows def insert_rows(conn): table = sqlite_utils.Database(conn)[table_name] if should_return: rowids = [] for row in rows: - rowids.append(table.insert(row).last_rowid) + rowids.append( + table.insert(row, ignore=ignore, replace=replace).last_rowid + ) return list( table.rows_where( "rowid in ({})".format(",".join("?" for _ in rowids)), @@ -1187,12 +1204,12 @@ class TableInsertView(BaseView): ) ) else: - table.insert_all(rows) + table.insert_all(rows, ignore=ignore, replace=replace) rows = await db.execute_write_fn(insert_rows) result = {"ok": True} if should_return: - result["inserted"] = rows + result["rows"] = rows return Response.json(result, status=201) diff --git a/docs/json_api.rst b/docs/json_api.rst index da4500ab..34c13211 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -489,7 +489,7 @@ If successful, this will return a ``201`` status code and the newly inserted row .. code-block:: json { - "inserted": [ + "rows": [ { "id": 1, "column1": "value1", @@ -538,7 +538,7 @@ To return the newly inserted rows, add the ``"return_rows": true`` key to the re "return_rows": true } -This will return the same ``"inserted"`` key as the single row example above. There is a small performance penalty for using this option. +This will return the same ``"rows"`` key as the single row example above. There is a small performance penalty for using this option. .. _RowDeleteView: diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 1cfba104..d0b0f324 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -37,7 +37,7 @@ async def test_write_row(ds_write): ) expected_row = {"id": 1, "title": "Test", "score": 1.0} assert response.status_code == 201 - assert response.json()["inserted"] == [expected_row] + assert response.json()["rows"] == [expected_row] rows = (await ds_write.get_database("data").execute("select * from docs")).rows assert dict(rows[0]) == expected_row @@ -70,7 +70,7 @@ async def test_write_rows(ds_write, return_rows): ] assert response.json()["ok"] is True if return_rows: - assert response.json()["inserted"] == actual_rows + assert response.json()["rows"] == actual_rows @pytest.mark.asyncio @@ -156,6 +156,27 @@ async def test_write_rows(ds_write, return_rows): 400, ["Too many rows, maximum allowed is 100"], ), + ( + "/data/docs/-/insert", + {"rows": [{"title": "Test"}], "ignore": True, "replace": True}, + None, + 400, + ['Cannot use "ignore" and "replace" at the same time'], + ), + ( + "/data/docs/-/insert", + {"rows": [{"title": "Test"}], "invalid_param": True}, + None, + 400, + ['Invalid parameter: "invalid_param"'], + ), + ( + "/data/docs/-/insert", + {"rows": [{"title": "Test"}], "one": True, "two": True}, + None, + 400, + ['Invalid parameter: "one", "two"'], + ), # Validate columns of each row ( "/data/docs/-/insert", @@ -196,6 +217,62 @@ async def test_write_row_errors( assert response.json()["errors"] == expected_errors +@pytest.mark.asyncio +@pytest.mark.parametrize( + "ignore,replace,expected_rows", + ( + ( + True, + False, + [ + {"id": 1, "title": "Exists", "score": None}, + ], + ), + ( + False, + True, + [ + {"id": 1, "title": "One", "score": None}, + ], + ), + ), +) +@pytest.mark.parametrize("should_return", (True, False)) +async def test_insert_ignore_replace( + ds_write, ignore, replace, expected_rows, should_return +): + await ds_write.get_database("data").execute_write( + "insert into docs (id, title) values (1, 'Exists')" + ) + token = write_token(ds_write) + data = {"rows": [{"id": 1, "title": "One"}]} + if ignore: + data["ignore"] = True + if replace: + data["replace"] = True + if should_return: + data["return_rows"] = True + response = await ds_write.client.post( + "/data/docs/-/insert", + json=data, + headers={ + "Authorization": "Bearer {}".format(token), + "Content-Type": "application/json", + }, + ) + assert response.status_code == 201 + actual_rows = [ + dict(r) + for r in ( + await ds_write.get_database("data").execute("select * from docs") + ).rows + ] + assert actual_rows == expected_rows + assert response.json()["ok"] is True + if should_return: + assert response.json()["rows"] == expected_rows + + @pytest.mark.asyncio @pytest.mark.parametrize("scenario", ("no_token", "no_perm", "bad_table", "has_perm")) async def test_delete_row(ds_write, scenario): @@ -217,7 +294,7 @@ async def test_delete_row(ds_write, scenario): }, ) assert insert_response.status_code == 201 - pk = insert_response.json()["inserted"][0]["id"] + pk = insert_response.json()["rows"][0]["id"] path = "/data/{}/{}/-/delete".format( "docs" if scenario != "bad_table" else "bad_table", pk From 497290beaf32e6b779f9683ef15f1c5bc142a41a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 1 Nov 2022 12:59:17 -0700 Subject: [PATCH 039/823] Handle database errors in /-/insert, refs #1866, #1873 Also improved API explorer to show HTTP status of response, refs #1871 --- datasette/templates/api_explorer.html | 14 +++++++++----- datasette/views/table.py | 5 ++++- tests/test_api_write.py | 11 +++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html index 38fdb7bc..93bacde3 100644 --- a/datasette/templates/api_explorer.html +++ b/datasette/templates/api_explorer.html @@ -27,7 +27,8 @@ @@ -64,12 +65,15 @@ form.addEventListener("submit", (ev) => { headers: { 'Content-Type': 'application/json', } - }).then(r => r.json()).then(r => { + }).then(r => { + document.getElementById('response-status').textContent = r.status; + return r.json(); + }).then(data => { var errorList = output.querySelector('.errors'); - if (r.errors) { + if (data.errors) { errorList.style.display = 'block'; errorList.innerHTML = ''; - r.errors.forEach(error => { + data.errors.forEach(error => { var li = document.createElement('li'); li.textContent = error; errorList.appendChild(li); @@ -77,7 +81,7 @@ form.addEventListener("submit", (ev) => { } else { errorList.style.display = 'none'; } - output.querySelector('pre').innerText = JSON.stringify(r, null, 2); + output.querySelector('pre').innerText = JSON.stringify(data, null, 2); output.style.display = 'block'; }).catch(err => { alert("Error: " + err); diff --git a/datasette/views/table.py b/datasette/views/table.py index 7692a4e3..61227206 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -1206,7 +1206,10 @@ class TableInsertView(BaseView): else: table.insert_all(rows, ignore=ignore, replace=replace) - rows = await db.execute_write_fn(insert_rows) + try: + rows = await db.execute_write_fn(insert_rows) + except Exception as e: + return _error([str(e)]) result = {"ok": True} if should_return: result["rows"] = rows diff --git a/tests/test_api_write.py b/tests/test_api_write.py index d0b0f324..0b567f48 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -156,6 +156,13 @@ async def test_write_rows(ds_write, return_rows): 400, ["Too many rows, maximum allowed is 100"], ), + ( + "/data/docs/-/insert", + {"rows": [{"id": 1, "title": "Test"}]}, + "duplicate_id", + 400, + ["UNIQUE constraint failed: docs.id"], + ), ( "/data/docs/-/insert", {"rows": [{"title": "Test"}], "ignore": True, "replace": True}, @@ -194,6 +201,10 @@ async def test_write_row_errors( ds_write, path, input, special_case, expected_status, expected_errors ): token = write_token(ds_write) + if special_case == "duplicate_id": + await ds_write.get_database("data").execute_write( + "insert into docs (id) values (1)" + ) if special_case == "bad_token": token += "bad" kwargs = dict( From 0b166befc0096fca30d71e19608a928d59c331a4 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 1 Nov 2022 17:31:22 -0700 Subject: [PATCH 040/823] API explorer can now do GET, has JSON syntax highlighting Refs #1871 --- .../static/json-format-highlight-1.0.1.js | 43 +++++++++++ datasette/templates/api_explorer.html | 77 +++++++++++++++---- 2 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 datasette/static/json-format-highlight-1.0.1.js diff --git a/datasette/static/json-format-highlight-1.0.1.js b/datasette/static/json-format-highlight-1.0.1.js new file mode 100644 index 00000000..e87c76e1 --- /dev/null +++ b/datasette/static/json-format-highlight-1.0.1.js @@ -0,0 +1,43 @@ +/* +https://github.com/luyilin/json-format-highlight +From https://unpkg.com/json-format-highlight@1.0.1/dist/json-format-highlight.js +MIT Licensed +*/ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + (global.jsonFormatHighlight = factory()); +}(this, (function () { 'use strict'; + +var defaultColors = { + keyColor: 'dimgray', + numberColor: 'lightskyblue', + stringColor: 'lightcoral', + trueColor: 'lightseagreen', + falseColor: '#f66578', + nullColor: 'cornflowerblue' +}; + +function index (json, colorOptions) { + if ( colorOptions === void 0 ) colorOptions = {}; + + if (!json) { return; } + if (typeof json !== 'string') { + json = JSON.stringify(json, null, 2); + } + var colors = Object.assign({}, defaultColors, colorOptions); + json = json.replace(/&/g, '&').replace(//g, '>'); + return json.replace(/("(\\u[a-zA-Z0-9]{4}|\\[^u]|[^\\"])*"(\s*:)?|\b(true|false|null)\b|-?\d+(?:\.\d*)?(?:[eE][+]?\d+)?)/g, function (match) { + var color = colors.numberColor; + if (/^"/.test(match)) { + color = /:$/.test(match) ? colors.keyColor : colors.stringColor; + } else { + color = /true/.test(match) ? colors.trueColor : /false/.test(match) ? colors.falseColor : /null/.test(match) ? colors.nullColor : color; + } + return ("" + match + ""); + }); +} + +return index; + +}))); diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html index 93bacde3..de5337e3 100644 --- a/datasette/templates/api_explorer.html +++ b/datasette/templates/api_explorer.html @@ -2,6 +2,10 @@ {% block title %}API Explorer{% endblock %} +{% block extra_head %} + +{% endblock %} + {% block content %}

API Explorer

@@ -14,17 +18,30 @@ {% endfor %} {% endif %} -
-
- - -
-
- - -
-

- +
+ GET +
+
+ + + +
+ +
+
+ POST +
+
+ + +
+
+ + +
+

+ +
{% else %} - {% if not canned_write and not error %} + {% if not canned_query_write and not error %}

0 results

{% endif %} {% endif %} diff --git a/datasette/views/database.py b/datasette/views/database.py index 0770a380..658c35e6 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -1,4 +1,3 @@ -from asyncinject import Registry from dataclasses import dataclass, field from typing import Callable from urllib.parse import parse_qsl, urlencode @@ -33,7 +32,7 @@ from datasette.utils import ( from datasette.utils.asgi import AsgiFileDownload, NotFound, Response, Forbidden from datasette.plugins import pm -from .base import BaseView, DatasetteError, DataView, View, _error, stream_csv +from .base import BaseView, DatasetteError, View, _error, stream_csv class DatabaseView(View): @@ -57,7 +56,7 @@ class DatabaseView(View): sql = (request.args.get("sql") or "").strip() if sql: - return await query_view(request, datasette) + return await QueryView()(request, datasette) if format_ not in ("html", "json"): raise NotFound("Invalid format: {}".format(format_)) @@ -65,10 +64,6 @@ class DatabaseView(View): metadata = (datasette.metadata("databases") or {}).get(database, {}) datasette.update_with_inherited_metadata(metadata) - table_counts = await db.table_counts(5) - hidden_table_names = set(await db.hidden_table_names()) - all_foreign_keys = await db.get_all_foreign_keys() - sql_views = [] for view_name in await db.view_names(): view_visible, view_private = await datasette.check_visibility( @@ -196,8 +191,13 @@ class QueryContext: # urls: dict = field( # metadata={"help": "Object containing URL helpers like `database()`"} # ) - canned_write: bool = field( - metadata={"help": "Boolean indicating if this canned query allows writes"} + canned_query_write: bool = field( + metadata={ + "help": "Boolean indicating if this is a canned query that allows writes" + } + ) + metadata: dict = field( + metadata={"help": "Metadata about the database or the canned query"} ) db_is_immutable: bool = field( metadata={"help": "Boolean indicating if this database is immutable"} @@ -232,7 +232,6 @@ class QueryContext: show_hide_hidden: str = field( metadata={"help": "Hidden input field for the _show_sql parameter"} ) - metadata: dict = field(metadata={"help": "Metadata about the query/database"}) database_color: Callable = field( metadata={"help": "Function that returns a color for a given database name"} ) @@ -242,6 +241,12 @@ class QueryContext: alternate_url_json: str = field( metadata={"help": "URL for alternate JSON version of this page"} ) + # TODO: refactor this to somewhere else, probably ds.render_template() + select_templates: list = field( + metadata={ + "help": "List of templates that were considered for rendering this page" + } + ) async def get_tables(datasette, request, db): @@ -320,287 +325,105 @@ async def database_download(request, datasette): ) -async def query_view( - request, - datasette, - # canned_query=None, - # _size=None, - # named_parameters=None, - # write=False, -): - db = await datasette.resolve_database(request) - database = db.name - # Flattened because of ?sql=&name1=value1&name2=value2 feature - params = {key: request.args.get(key) for key in request.args} - sql = None - if "sql" in params: - sql = params.pop("sql") - if "_shape" in params: - params.pop("_shape") +class QueryView(View): + async def post(self, request, datasette): + from datasette.app import TableNotFound - # extras come from original request.args to avoid being flattened - extras = request.args.getlist("_extra") + db = await datasette.resolve_database(request) - # TODO: Behave differently for canned query here: - await datasette.ensure_permissions(request.actor, [("execute-sql", database)]) - - _, private = await datasette.check_visibility( - request.actor, - permissions=[ - ("view-database", database), - "view-instance", - ], - ) - - extra_args = {} - if params.get("_timelimit"): - extra_args["custom_time_limit"] = int(params["_timelimit"]) - - format_ = request.url_vars.get("format") or "html" - query_error = None - try: - validate_sql_select(sql) - results = await datasette.execute( - database, sql, params, truncate=True, **extra_args - ) - columns = results.columns - rows = results.rows - except QueryInterrupted as ex: - raise DatasetteError( - textwrap.dedent( - """ -

SQL query took too long. The time limit is controlled by the - sql_time_limit_ms - configuration option.

- - - """.format( - markupsafe.escape(ex.sql) - ) - ).strip(), - title="SQL Interrupted", - status=400, - message_is_html=True, - ) - except sqlite3.DatabaseError as ex: - query_error = str(ex) - results = None - rows = [] - columns = [] - except (sqlite3.OperationalError, InvalidSql) as ex: - raise DatasetteError(str(ex), title="Invalid SQL", status=400) - except sqlite3.OperationalError as ex: - raise DatasetteError(str(ex)) - except DatasetteError: - raise - - # Handle formats from plugins - if format_ == "csv": - - async def fetch_data_for_csv(request, _next=None): - results = await db.execute(sql, params, truncate=True) - data = {"rows": results.rows, "columns": results.columns} - return data, None, None - - return await stream_csv(datasette, fetch_data_for_csv, request, db.name) - elif format_ in datasette.renderers.keys(): - # Dispatch request to the correct output format renderer - # (CSV is not handled here due to streaming) - result = call_with_supported_arguments( - datasette.renderers[format_][0], - datasette=datasette, - columns=columns, - rows=rows, - sql=sql, - query_name=None, - database=database, - table=None, - request=request, - view_name="table", - truncated=results.truncated if results else False, - error=query_error, - # These will be deprecated in Datasette 1.0: - args=request.args, - data={"rows": rows, "columns": columns}, - ) - if asyncio.iscoroutine(result): - result = await result - if result is None: - raise NotFound("No data") - if isinstance(result, dict): - r = Response( - body=result.get("body"), - status=result.get("status_code") or 200, - content_type=result.get("content_type", "text/plain"), - headers=result.get("headers"), + # We must be a canned query + table_found = False + try: + await datasette.resolve_table(request) + table_found = True + except TableNotFound as table_not_found: + canned_query = await datasette.get_canned_query( + table_not_found.database_name, table_not_found.table, request.actor ) - elif isinstance(result, Response): - r = result - # if status_code is not None: - # # Over-ride the status code - # r.status = status_code - else: - assert False, f"{result} should be dict or Response" - elif format_ == "html": - headers = {} - templates = [f"query-{to_css_class(database)}.html", "query.html"] - template = datasette.jinja_env.select_template(templates) - alternate_url_json = datasette.absolute_url( - request, - datasette.urls.path(path_with_format(request=request, format="json")), - ) - data = {} - headers.update( - { - "Link": '{}; rel="alternate"; type="application/json+datasette"'.format( - alternate_url_json - ) - } - ) - metadata = (datasette.metadata("databases") or {}).get(database, {}) - datasette.update_with_inherited_metadata(metadata) + if canned_query is None: + raise + if table_found: + # That should not have happened + raise DatasetteError("Unexpected table found on POST", status=404) - renderers = {} - for key, (_, can_render) in datasette.renderers.items(): - it_can_render = call_with_supported_arguments( - can_render, - datasette=datasette, - columns=data.get("columns") or [], - rows=data.get("rows") or [], - sql=data.get("query", {}).get("sql", None), - query_name=data.get("query_name"), - database=database, - table=data.get("table"), - request=request, - view_name="database", + # If database is immutable, return an error + if not db.is_mutable: + raise Forbidden("Database is immutable") + + # Process the POST + body = await request.post_body() + body = body.decode("utf-8").strip() + if body.startswith("{") and body.endswith("}"): + params = json.loads(body) + # But we want key=value strings + for key, value in params.items(): + params[key] = str(value) + else: + params = dict(parse_qsl(body, keep_blank_values=True)) + # Should we return JSON? + should_return_json = ( + request.headers.get("accept") == "application/json" + or request.args.get("_json") + or params.get("_json") + ) + params_for_query = MagicParameters(params, request, datasette) + ok = None + redirect_url = None + try: + cursor = await db.execute_write(canned_query["sql"], params_for_query) + message = canned_query.get( + "on_success_message" + ) or "Query executed, {} row{} affected".format( + cursor.rowcount, "" if cursor.rowcount == 1 else "s" + ) + message_type = datasette.INFO + redirect_url = canned_query.get("on_success_redirect") + ok = True + except Exception as ex: + message = canned_query.get("on_error_message") or str(ex) + message_type = datasette.ERROR + redirect_url = canned_query.get("on_error_redirect") + ok = False + if should_return_json: + return Response.json( + { + "ok": ok, + "message": message, + "redirect": redirect_url, + } ) - it_can_render = await await_me_maybe(it_can_render) - if it_can_render: - renderers[key] = datasette.urls.path( - path_with_format(request=request, format=key) - ) - - allow_execute_sql = await datasette.permission_allowed( - request.actor, "execute-sql", database - ) - - show_hide_hidden = "" - if metadata.get("hide_sql"): - if bool(params.get("_show_sql")): - show_hide_link = path_with_removed_args(request, {"_show_sql"}) - show_hide_text = "hide" - show_hide_hidden = '' - else: - show_hide_link = path_with_added_args(request, {"_show_sql": 1}) - show_hide_text = "show" else: - if bool(params.get("_hide_sql")): - show_hide_link = path_with_removed_args(request, {"_hide_sql"}) - show_hide_text = "show" - show_hide_hidden = '' - else: - show_hide_link = path_with_added_args(request, {"_hide_sql": 1}) - show_hide_text = "hide" - hide_sql = show_hide_text == "show" + datasette.add_message(request, message, message_type) + return Response.redirect(redirect_url or request.path) - # Extract any :named parameters - named_parameters = await derive_named_parameters( - datasette.get_database(database), sql - ) - named_parameter_values = { - named_parameter: params.get(named_parameter) or "" - for named_parameter in named_parameters - if not named_parameter.startswith("_") - } + async def get(self, request, datasette): + from datasette.app import TableNotFound - # Set to blank string if missing from params - for named_parameter in named_parameters: - if named_parameter not in params and not named_parameter.startswith("_"): - params[named_parameter] = "" - - r = Response.html( - await datasette.render_template( - template, - QueryContext( - database=database, - query={ - "sql": sql, - "params": params, - }, - canned_query=None, - private=private, - canned_write=False, - db_is_immutable=not db.is_mutable, - error=query_error, - hide_sql=hide_sql, - show_hide_link=datasette.urls.path(show_hide_link), - show_hide_text=show_hide_text, - editable=True, # TODO - allow_execute_sql=allow_execute_sql, - tables=await get_tables(datasette, request, db), - named_parameter_values=named_parameter_values, - edit_sql_url="todo", - display_rows=await display_rows( - datasette, database, request, rows, columns - ), - table_columns=await _table_columns(datasette, database) - if allow_execute_sql - else {}, - columns=columns, - renderers=renderers, - url_csv=datasette.urls.path( - path_with_format( - request=request, format="csv", extra_qs={"_size": "max"} - ) - ), - show_hide_hidden=markupsafe.Markup(show_hide_hidden), - metadata=metadata, - database_color=lambda _: "#ff0000", - alternate_url_json=alternate_url_json, - ), - request=request, - view_name="database", - ), - headers=headers, - ) - else: - assert False, "Invalid format: {}".format(format_) - if datasette.cors: - add_cors_headers(r.headers) - return r - - -class QueryView(DataView): - async def data( - self, - request, - sql, - editable=True, - canned_query=None, - metadata=None, - _size=None, - named_parameters=None, - write=False, - default_labels=None, - ): - db = await self.ds.resolve_database(request) + db = await datasette.resolve_database(request) database = db.name - params = {key: request.args.get(key) for key in request.args} - if "sql" in params: - params.pop("sql") - if "_shape" in params: - params.pop("_shape") + + # Are we a canned query? + canned_query = None + canned_query_write = False + if "table" in request.url_vars: + try: + await datasette.resolve_table(request) + except TableNotFound as table_not_found: + # Was this actually a canned query? + canned_query = await datasette.get_canned_query( + table_not_found.database_name, table_not_found.table, request.actor + ) + if canned_query is None: + raise + canned_query_write = bool(canned_query.get("write")) private = False if canned_query: # Respect canned query permissions - visible, private = await self.ds.check_visibility( + visible, private = await datasette.check_visibility( request.actor, permissions=[ - ("view-query", (database, canned_query)), + ("view-query", (database, canned_query["name"])), ("view-database", database), "view-instance", ], @@ -609,18 +432,32 @@ class QueryView(DataView): raise Forbidden("You do not have permission to view this query") else: - await self.ds.ensure_permissions(request.actor, [("execute-sql", database)]) + await datasette.ensure_permissions( + request.actor, [("execute-sql", database)] + ) + + # Flattened because of ?sql=&name1=value1&name2=value2 feature + params = {key: request.args.get(key) for key in request.args} + sql = None + + if canned_query: + sql = canned_query["sql"] + elif "sql" in params: + sql = params.pop("sql") # Extract any :named parameters - named_parameters = named_parameters or await derive_named_parameters( - self.ds.get_database(database), sql - ) + named_parameters = [] + if canned_query and canned_query.get("params"): + named_parameters = canned_query["params"] + if not named_parameters: + named_parameters = await derive_named_parameters( + datasette.get_database(database), sql + ) named_parameter_values = { named_parameter: params.get(named_parameter) or "" for named_parameter in named_parameters if not named_parameter.startswith("_") } - # Set to blank string if missing from params for named_parameter in named_parameters: if named_parameter not in params and not named_parameter.startswith("_"): @@ -629,212 +466,159 @@ class QueryView(DataView): extra_args = {} if params.get("_timelimit"): extra_args["custom_time_limit"] = int(params["_timelimit"]) - if _size: - extra_args["page_size"] = _size - templates = [f"query-{to_css_class(database)}.html", "query.html"] - if canned_query: - templates.insert( - 0, - f"query-{to_css_class(database)}-{to_css_class(canned_query)}.html", - ) + format_ = request.url_vars.get("format") or "html" query_error = None + results = None + rows = [] + columns = [] - # Execute query - as write or as read - if write: - if request.method == "POST": - # If database is immutable, return an error - if not db.is_mutable: - raise Forbidden("Database is immutable") - body = await request.post_body() - body = body.decode("utf-8").strip() - if body.startswith("{") and body.endswith("}"): - params = json.loads(body) - # But we want key=value strings - for key, value in params.items(): - params[key] = str(value) - else: - params = dict(parse_qsl(body, keep_blank_values=True)) - # Should we return JSON? - should_return_json = ( - request.headers.get("accept") == "application/json" - or request.args.get("_json") - or params.get("_json") - ) - if canned_query: - params_for_query = MagicParameters(params, request, self.ds) - else: - params_for_query = params - ok = None - try: - cursor = await self.ds.databases[database].execute_write( - sql, params_for_query - ) - message = metadata.get( - "on_success_message" - ) or "Query executed, {} row{} affected".format( - cursor.rowcount, "" if cursor.rowcount == 1 else "s" - ) - message_type = self.ds.INFO - redirect_url = metadata.get("on_success_redirect") - ok = True - except Exception as e: - message = metadata.get("on_error_message") or str(e) - message_type = self.ds.ERROR - redirect_url = metadata.get("on_error_redirect") - ok = False - if should_return_json: - return Response.json( - { - "ok": ok, - "message": message, - "redirect": redirect_url, - } - ) - else: - self.ds.add_message(request, message, message_type) - return self.redirect(request, redirect_url or request.path) - else: + params_for_query = params - async def extra_template(): - return { - "request": request, - "db_is_immutable": not db.is_mutable, - "path_with_added_args": path_with_added_args, - "path_with_removed_args": path_with_removed_args, - "named_parameter_values": named_parameter_values, - "canned_query": canned_query, - "success_message": request.args.get("_success") or "", - "canned_write": True, - } - - return ( - { - "database": database, - "rows": [], - "truncated": False, - "columns": [], - "query": {"sql": sql, "params": params}, - "private": private, - }, - extra_template, - templates, - ) - else: # Not a write - if canned_query: - params_for_query = MagicParameters(params, request, self.ds) - else: - params_for_query = params + if not canned_query_write: try: - results = await self.ds.execute( + if not canned_query: + # For regular queries we only allow SELECT, plus other rules + validate_sql_select(sql) + else: + # Canned queries can run magic parameters + params_for_query = MagicParameters(params, request, datasette) + results = await datasette.execute( database, sql, params_for_query, truncate=True, **extra_args ) - columns = [r[0] for r in results.description] - except sqlite3.DatabaseError as e: - query_error = e + columns = results.columns + rows = results.rows + except QueryInterrupted as ex: + raise DatasetteError( + textwrap.dedent( + """ +

SQL query took too long. The time limit is controlled by the + sql_time_limit_ms + configuration option.

+ + + """.format( + markupsafe.escape(ex.sql) + ) + ).strip(), + title="SQL Interrupted", + status=400, + message_is_html=True, + ) + except sqlite3.DatabaseError as ex: + query_error = str(ex) results = None + rows = [] columns = [] + except (sqlite3.OperationalError, InvalidSql) as ex: + raise DatasetteError(str(ex), title="Invalid SQL", status=400) + except sqlite3.OperationalError as ex: + raise DatasetteError(str(ex)) + except DatasetteError: + raise - allow_execute_sql = await self.ds.permission_allowed( - request.actor, "execute-sql", database - ) + # Handle formats from plugins + if format_ == "csv": - async def extra_template(): - display_rows = [] - truncate_cells = self.ds.setting("truncate_cells_html") - for row in results.rows if results else []: - display_row = [] - for column, value in zip(results.columns, row): - display_value = value - # Let the plugins have a go - # pylint: disable=no-member - plugin_display_value = None - for candidate in pm.hook.render_cell( - row=row, - value=value, - column=column, - table=None, - database=database, - datasette=self.ds, - request=request, - ): - candidate = await await_me_maybe(candidate) - if candidate is not None: - plugin_display_value = candidate - break - if plugin_display_value is not None: - display_value = plugin_display_value - else: - if value in ("", None): - display_value = markupsafe.Markup(" ") - elif is_url(str(display_value).strip()): - display_value = markupsafe.Markup( - '{truncated_url}'.format( - url=markupsafe.escape(value.strip()), - truncated_url=markupsafe.escape( - truncate_url(value.strip(), truncate_cells) - ), - ) - ) - elif isinstance(display_value, bytes): - blob_url = path_with_format( - request=request, - format="blob", - extra_qs={ - "_blob_column": column, - "_blob_hash": hashlib.sha256( - display_value - ).hexdigest(), - }, - ) - formatted = format_bytes(len(value)) - display_value = markupsafe.Markup( - '<Binary: {:,} byte{}>'.format( - blob_url, - ' title="{}"'.format(formatted) - if "bytes" not in formatted - else "", - len(value), - "" if len(value) == 1 else "s", - ) - ) - else: - display_value = str(value) - if truncate_cells and len(display_value) > truncate_cells: - display_value = ( - display_value[:truncate_cells] + "\u2026" - ) - display_row.append(display_value) - display_rows.append(display_row) + async def fetch_data_for_csv(request, _next=None): + results = await db.execute(sql, params, truncate=True) + data = {"rows": results.rows, "columns": results.columns} + return data, None, None - # Show 'Edit SQL' button only if: - # - User is allowed to execute SQL - # - SQL is an approved SELECT statement - # - No magic parameters, so no :_ in the SQL string - edit_sql_url = None - is_validated_sql = False - try: - validate_sql_select(sql) - is_validated_sql = True - except InvalidSql: - pass - if allow_execute_sql and is_validated_sql and ":_" not in sql: - edit_sql_url = ( - self.ds.urls.database(database) - + "?" - + urlencode( - { - **{ - "sql": sql, - }, - **named_parameter_values, - } - ) + return await stream_csv(datasette, fetch_data_for_csv, request, db.name) + elif format_ in datasette.renderers.keys(): + # Dispatch request to the correct output format renderer + # (CSV is not handled here due to streaming) + result = call_with_supported_arguments( + datasette.renderers[format_][0], + datasette=datasette, + columns=columns, + rows=rows, + sql=sql, + query_name=canned_query["name"] if canned_query else None, + database=database, + table=None, + request=request, + view_name="table", + truncated=results.truncated if results else False, + error=query_error, + # These will be deprecated in Datasette 1.0: + args=request.args, + data={"rows": rows, "columns": columns}, + ) + if asyncio.iscoroutine(result): + result = await result + if result is None: + raise NotFound("No data") + if isinstance(result, dict): + r = Response( + body=result.get("body"), + status=result.get("status_code") or 200, + content_type=result.get("content_type", "text/plain"), + headers=result.get("headers"), + ) + elif isinstance(result, Response): + r = result + # if status_code is not None: + # # Over-ride the status code + # r.status = status_code + else: + assert False, f"{result} should be dict or Response" + elif format_ == "html": + headers = {} + templates = [f"query-{to_css_class(database)}.html", "query.html"] + if canned_query: + templates.insert( + 0, + f"query-{to_css_class(database)}-{to_css_class(canned_query['name'])}.html", ) + template = datasette.jinja_env.select_template(templates) + alternate_url_json = datasette.absolute_url( + request, + datasette.urls.path(path_with_format(request=request, format="json")), + ) + data = {} + headers.update( + { + "Link": '{}; rel="alternate"; type="application/json+datasette"'.format( + alternate_url_json + ) + } + ) + metadata = (datasette.metadata("databases") or {}).get(database, {}) + datasette.update_with_inherited_metadata(metadata) + + renderers = {} + for key, (_, can_render) in datasette.renderers.items(): + it_can_render = call_with_supported_arguments( + can_render, + datasette=datasette, + columns=data.get("columns") or [], + rows=data.get("rows") or [], + sql=data.get("query", {}).get("sql", None), + query_name=data.get("query_name"), + database=database, + table=data.get("table"), + request=request, + view_name="database", + ) + it_can_render = await await_me_maybe(it_can_render) + if it_can_render: + renderers[key] = datasette.urls.path( + path_with_format(request=request, format=key) + ) + + allow_execute_sql = await datasette.permission_allowed( + request.actor, "execute-sql", database + ) + show_hide_hidden = "" - if metadata.get("hide_sql"): + if canned_query and canned_query.get("hide_sql"): if bool(params.get("_show_sql")): show_hide_link = path_with_removed_args(request, {"_show_sql"}) show_hide_text = "hide" @@ -855,42 +639,86 @@ class QueryView(DataView): show_hide_link = path_with_added_args(request, {"_hide_sql": 1}) show_hide_text = "hide" hide_sql = show_hide_text == "show" - return { - "display_rows": display_rows, - "custom_sql": True, - "named_parameter_values": named_parameter_values, - "editable": editable, - "canned_query": canned_query, - "edit_sql_url": edit_sql_url, - "metadata": metadata, - "settings": self.ds.settings_dict(), - "request": request, - "show_hide_link": self.ds.urls.path(show_hide_link), - "show_hide_text": show_hide_text, - "show_hide_hidden": markupsafe.Markup(show_hide_hidden), - "hide_sql": hide_sql, - "table_columns": await _table_columns(self.ds, database) - if allow_execute_sql - else {}, - } - return ( - { - "ok": not query_error, - "database": database, - "query_name": canned_query, - "rows": results.rows if results else [], - "truncated": results.truncated if results else False, - "columns": columns, - "query": {"sql": sql, "params": params}, - "error": str(query_error) if query_error else None, - "private": private, - "allow_execute_sql": allow_execute_sql, - }, - extra_template, - templates, - 400 if query_error else 200, - ) + # Show 'Edit SQL' button only if: + # - User is allowed to execute SQL + # - SQL is an approved SELECT statement + # - No magic parameters, so no :_ in the SQL string + edit_sql_url = None + is_validated_sql = False + try: + validate_sql_select(sql) + is_validated_sql = True + except InvalidSql: + pass + if allow_execute_sql and is_validated_sql and ":_" not in sql: + edit_sql_url = ( + datasette.urls.database(database) + + "?" + + urlencode( + { + **{ + "sql": sql, + }, + **named_parameter_values, + } + ) + ) + + r = Response.html( + await datasette.render_template( + template, + QueryContext( + database=database, + query={ + "sql": sql, + "params": params, + }, + canned_query=canned_query["name"] if canned_query else None, + private=private, + canned_query_write=canned_query_write, + db_is_immutable=not db.is_mutable, + error=query_error, + hide_sql=hide_sql, + show_hide_link=datasette.urls.path(show_hide_link), + show_hide_text=show_hide_text, + editable=not canned_query, + allow_execute_sql=allow_execute_sql, + tables=await get_tables(datasette, request, db), + named_parameter_values=named_parameter_values, + edit_sql_url=edit_sql_url, + display_rows=await display_rows( + datasette, database, request, rows, columns + ), + table_columns=await _table_columns(datasette, database) + if allow_execute_sql + else {}, + columns=columns, + renderers=renderers, + url_csv=datasette.urls.path( + path_with_format( + request=request, format="csv", extra_qs={"_size": "max"} + ) + ), + show_hide_hidden=markupsafe.Markup(show_hide_hidden), + metadata=canned_query or metadata, + database_color=lambda _: "#ff0000", + alternate_url_json=alternate_url_json, + select_templates=[ + f"{'*' if template_name == template.name else ''}{template_name}" + for template_name in templates + ], + ), + request=request, + view_name="database", + ), + headers=headers, + ) + else: + assert False, "Invalid format: {}".format(format_) + if datasette.cors: + add_cors_headers(r.headers) + return r class MagicParameters(dict): diff --git a/datasette/views/table.py b/datasette/views/table.py index 77acfd95..28264e92 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -9,7 +9,6 @@ import markupsafe from datasette.plugins import pm from datasette.database import QueryInterrupted from datasette import tracer -from datasette.renderer import json_renderer from datasette.utils import ( add_cors_headers, await_me_maybe, @@ -21,7 +20,6 @@ from datasette.utils import ( tilde_encode, escape_sqlite, filters_should_redirect, - format_bytes, is_url, path_from_row_pks, path_with_added_args, @@ -38,7 +36,7 @@ from datasette.utils import ( from datasette.utils.asgi import BadRequest, Forbidden, NotFound, Response from datasette.filters import Filters import sqlite_utils -from .base import BaseView, DataView, DatasetteError, ureg, _error, stream_csv +from .base import BaseView, DatasetteError, ureg, _error, stream_csv from .database import QueryView LINK_WITH_LABEL = ( @@ -698,57 +696,6 @@ async def table_view(datasette, request): return response -class CannedQueryView(DataView): - def __init__(self, datasette): - self.ds = datasette - - async def post(self, request): - from datasette.app import TableNotFound - - try: - await self.ds.resolve_table(request) - except TableNotFound as e: - # Was this actually a canned query? - canned_query = await self.ds.get_canned_query( - e.database_name, e.table, request.actor - ) - if canned_query: - # Handle POST to a canned query - return await QueryView(self.ds).data( - request, - canned_query["sql"], - metadata=canned_query, - editable=False, - canned_query=e.table, - named_parameters=canned_query.get("params"), - write=bool(canned_query.get("write")), - ) - - return Response.text("Method not allowed", status=405) - - async def data(self, request, **kwargs): - from datasette.app import TableNotFound - - try: - await self.ds.resolve_table(request) - except TableNotFound as not_found: - canned_query = await self.ds.get_canned_query( - not_found.database_name, not_found.table, request.actor - ) - if canned_query: - return await QueryView(self.ds).data( - request, - canned_query["sql"], - metadata=canned_query, - editable=False, - canned_query=not_found.table, - named_parameters=canned_query.get("params"), - write=bool(canned_query.get("write")), - ) - else: - raise - - async def table_view_traced(datasette, request): from datasette.app import TableNotFound @@ -761,10 +708,7 @@ async def table_view_traced(datasette, request): ) # If this is a canned query, not a table, then dispatch to QueryView instead if canned_query: - if request.method == "POST": - return await CannedQueryView(datasette).post(request) - else: - return await CannedQueryView(datasette).get(request) + return await QueryView()(request, datasette) else: raise diff --git a/tests/test_canned_queries.py b/tests/test_canned_queries.py index d6a88733..e9ad3239 100644 --- a/tests/test_canned_queries.py +++ b/tests/test_canned_queries.py @@ -95,12 +95,12 @@ def test_insert(canned_write_client): csrftoken_from=True, cookies={"foo": "bar"}, ) - assert 302 == response.status - assert "/data/add_name?success" == response.headers["Location"] messages = canned_write_client.ds.unsign( response.cookies["ds_messages"], "messages" ) - assert [["Query executed, 1 row affected", 1]] == messages + assert messages == [["Query executed, 1 row affected", 1]] + assert response.status == 302 + assert response.headers["Location"] == "/data/add_name?success" @pytest.mark.parametrize( @@ -382,11 +382,11 @@ def test_magic_parameters_cannot_be_used_in_arbitrary_queries(magic_parameters_c def test_canned_write_custom_template(canned_write_client): response = canned_write_client.get("/data/update_name") assert response.status == 200 + assert "!!!CUSTOM_UPDATE_NAME_TEMPLATE!!!" in response.text assert ( "" in response.text ) - assert "!!!CUSTOM_UPDATE_NAME_TEMPLATE!!!" in response.text # And test for link rel=alternate while we're here: assert ( '' From 8920d425f4d417cfd998b61016c5ff3530cd34e1 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 10:20:58 -0700 Subject: [PATCH 309/823] 1.0a3 release notes, smaller changes section - refs #2135 --- docs/changelog.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ee48d075..b4416f94 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,25 @@ Changelog ========= +.. _v1_0_a3: + +1.0a3 (2023-08-09) +------------------ + +This alpha release previews the updated design for Datasette's default JSON API. + +Smaller changes +~~~~~~~~~~~~~~~ + +- Datasette documentation now shows YAML examples for :ref:`metadata` by default, with a tab interface for switching to JSON. (:issue:`1153`) +- :ref:`plugin_register_output_renderer` plugins now have access to ``error`` and ``truncated`` arguments, allowing them to display error messages and take into account truncated results. (:issue:`2130`) +- ``render_cell()`` plugin hook now also supports an optional ``request`` argument. (:issue:`2007`) +- New ``Justfile`` to support development workflows for Datasette using `Just `__. +- ``datasette.render_template()`` can now accepts a ``datasette.views.Context`` subclass as an alternative to a dictionary. (:issue:`2127`) +- ``datasette install -e path`` option for editable installations, useful while developing plugins. (:issue:`2106`) +- When started with the ``--cors`` option Datasette now serves an ``Access-Control-Max-Age: 3600`` header, ensuring CORS OPTIONS requests are repeated no more than once an hour. (:issue:`2079`) +- Fixed a bug where the ``_internal`` database could display ``None`` instead of ``null`` for in-memory databases. (:issue:`1970`) + .. _v0_64_2: 0.64.2 (2023-03-08) From e34d09c6ec16ff5e7717e112afdad67f7c05a62a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 12:01:59 -0700 Subject: [PATCH 310/823] Don't include columns in query JSON, refs #2136 --- datasette/renderer.py | 8 +++++++- datasette/views/database.py | 2 +- tests/test_api.py | 1 - tests/test_cli_serve_get.py | 11 ++++++----- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/datasette/renderer.py b/datasette/renderer.py index 0bd74e81..224031a7 100644 --- a/datasette/renderer.py +++ b/datasette/renderer.py @@ -27,7 +27,7 @@ def convert_specific_columns_to_json(rows, columns, json_cols): return new_rows -def json_renderer(args, data, error, truncated=None): +def json_renderer(request, args, data, error, truncated=None): """Render a response as JSON""" status_code = 200 @@ -106,6 +106,12 @@ def json_renderer(args, data, error, truncated=None): "status": 400, "title": None, } + + # Don't include "columns" in output + # https://github.com/simonw/datasette/issues/2136 + if isinstance(data, dict) and "columns" not in request.args.getlist("_extra"): + data.pop("columns", None) + # Handle _nl option for _shape=array nl = args.get("_nl", "") if nl and shape == "array": diff --git a/datasette/views/database.py b/datasette/views/database.py index 658c35e6..cf76f3c2 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -548,7 +548,7 @@ class QueryView(View): error=query_error, # These will be deprecated in Datasette 1.0: args=request.args, - data={"rows": rows, "columns": columns}, + data={"ok": True, "rows": rows, "columns": columns}, ) if asyncio.iscoroutine(result): result = await result diff --git a/tests/test_api.py b/tests/test_api.py index 28415a0b..f96f571e 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -649,7 +649,6 @@ async def test_custom_sql(ds_client): {"content": "RENDER_CELL_DEMO"}, {"content": "RENDER_CELL_ASYNC"}, ], - "columns": ["content"], "ok": True, "truncated": False, } diff --git a/tests/test_cli_serve_get.py b/tests/test_cli_serve_get.py index 2e0390bb..dc7fc1e2 100644 --- a/tests/test_cli_serve_get.py +++ b/tests/test_cli_serve_get.py @@ -34,11 +34,12 @@ def test_serve_with_get(tmp_path_factory): "/_memory.json?sql=select+sqlite_version()", ], ) - assert 0 == result.exit_code, result.output - assert { - "truncated": False, - "columns": ["sqlite_version()"], - }.items() <= json.loads(result.output).items() + assert result.exit_code == 0, result.output + data = json.loads(result.output) + # Should have a single row with a single column + assert len(data["rows"]) == 1 + assert list(data["rows"][0].keys()) == ["sqlite_version()"] + assert set(data.keys()) == {"rows", "ok", "truncated"} # The plugin should have created hello.txt assert (plugins_dir / "hello.txt").read_text() == "hello" From 856ca68d94708c6e94673cb6bc28bf3e3ca17845 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 12:04:40 -0700 Subject: [PATCH 311/823] Update default JSON representation docs, refs #2135 --- docs/json_api.rst | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/docs/json_api.rst b/docs/json_api.rst index c273c2a8..16b997eb 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -9,10 +9,10 @@ through the Datasette user interface can also be accessed as JSON via the API. To access the API for a page, either click on the ``.json`` link on that page or edit the URL and add a ``.json`` extension to it. -.. _json_api_shapes: +.. _json_api_default: -Different shapes ----------------- +Default representation +---------------------- The default JSON representation of data from a SQLite table or custom query looks like this: @@ -21,7 +21,6 @@ looks like this: { "ok": true, - "next": null, "rows": [ { "id": 3, @@ -39,13 +38,22 @@ looks like this: "id": 1, "name": "San Francisco" } - ] + ], + "truncated": false } -The ``rows`` key is a list of objects, each one representing a row. ``next`` indicates if -there is another page, and ``ok`` is always ``true`` if an error did not occur. +``"ok"`` is always ``true`` if an error did not occur. -If ``next`` is present then the next page in the pagination set can be retrieved using ``?_next=VALUE``. +The ``"rows"`` key is a list of objects, each one representing a row. + +The ``"truncated"`` key lets you know if the query was truncated. This can happen if a SQL query returns more than 1,000 results (or the :ref:`setting_max_returned_rows` setting). + +For table pages, an additional key ``"next"`` may be present. This indicates that the next page in the pagination set can be retrieved using ``?_next=VALUE``. + +.. _json_api_shapes: + +Different shapes +---------------- The ``_shape`` parameter can be used to access alternative formats for the ``rows`` key which may be more convenient for your application. There are three From 90cb9ca58d910f49e8f117bbdd94df6f0855cf99 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 12:11:16 -0700 Subject: [PATCH 312/823] JSON changes in release notes, refs #2135 --- docs/changelog.rst | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b4416f94..4c70855b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,7 +9,40 @@ Changelog 1.0a3 (2023-08-09) ------------------ -This alpha release previews the updated design for Datasette's default JSON API. +This alpha release previews the updated design for Datasette's default JSON API. (:issue:`782`) + +The new :ref:`default JSON representation ` for both table pages (``/dbname/table.json``) and arbitrary SQL queries (``/dbname.json?sql=...``) is now shaped like this: + +.. code-block:: json + + { + "ok": true, + "rows": [ + { + "id": 3, + "name": "Detroit" + }, + { + "id": 2, + "name": "Los Angeles" + }, + { + "id": 4, + "name": "Memnonia" + }, + { + "id": 1, + "name": "San Francisco" + } + ], + "truncated": false + } + +Tables will include an additional ``"next"`` key for pagination, which can be passed to ``?_next=`` to fetch the next page of results. + +The various ``?_shape=`` options continue to work as before - see :ref:`json_api_shapes` for details. + +A new ``?_extra=`` mechanism is available for tables, but has not yet been stabilized or documented. Details on that are available in :issue:`262`. Smaller changes ~~~~~~~~~~~~~~~ From 19ab4552e212c9845a59461cc73e82d5ae8c278a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 12:13:11 -0700 Subject: [PATCH 313/823] Release 1.0a3 Closes #2135 Refs #262, #782, #1153, #1970, #2007, #2079, #2106, #2127, #2130 --- datasette/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datasette/version.py b/datasette/version.py index 3b81ab21..61dee464 100644 --- a/datasette/version.py +++ b/datasette/version.py @@ -1,2 +1,2 @@ -__version__ = "1.0a2" +__version__ = "1.0a3" __version_info__ = tuple(__version__.split(".")) From 4a42476bb7ce4c5ed941f944115dedd9bce34656 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 15:04:16 -0700 Subject: [PATCH 314/823] datasette plugins --requirements, closes #2133 --- datasette/cli.py | 12 ++++++++++-- docs/cli-reference.rst | 1 + docs/plugins.rst | 32 ++++++++++++++++++++++++++++---- tests/test_cli.py | 3 +++ 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/datasette/cli.py b/datasette/cli.py index 32266888..21fd25d6 100644 --- a/datasette/cli.py +++ b/datasette/cli.py @@ -223,15 +223,23 @@ pm.hook.publish_subcommand(publish=publish) @cli.command() @click.option("--all", help="Include built-in default plugins", is_flag=True) +@click.option( + "--requirements", help="Output requirements.txt of installed plugins", is_flag=True +) @click.option( "--plugins-dir", type=click.Path(exists=True, file_okay=False, dir_okay=True), help="Path to directory containing custom plugins", ) -def plugins(all, plugins_dir): +def plugins(all, requirements, plugins_dir): """List currently installed plugins""" app = Datasette([], plugins_dir=plugins_dir) - click.echo(json.dumps(app._plugins(all=all), indent=4)) + if requirements: + for plugin in app._plugins(): + if plugin["version"]: + click.echo("{}=={}".format(plugin["name"], plugin["version"])) + else: + click.echo(json.dumps(app._plugins(all=all), indent=4)) @cli.command() diff --git a/docs/cli-reference.rst b/docs/cli-reference.rst index 2177fc9e..7a96d311 100644 --- a/docs/cli-reference.rst +++ b/docs/cli-reference.rst @@ -282,6 +282,7 @@ Output JSON showing all currently installed plugins, their versions, whether the Options: --all Include built-in default plugins + --requirements Output requirements.txt of installed plugins --plugins-dir DIRECTORY Path to directory containing custom plugins --help Show this message and exit. diff --git a/docs/plugins.rst b/docs/plugins.rst index 979f94dd..19bfdd0c 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -90,7 +90,12 @@ You can see a list of installed plugins by navigating to the ``/-/plugins`` page You can also use the ``datasette plugins`` command:: - $ datasette plugins + datasette plugins + +Which outputs: + +.. code-block:: json + [ { "name": "datasette_json_html", @@ -107,7 +112,8 @@ You can also use the ``datasette plugins`` command:: cog.out("\n") result = CliRunner().invoke(cli.cli, ["plugins", "--all"]) # cog.out() with text containing newlines was unindenting for some reason - cog.outl("If you run ``datasette plugins --all`` it will include default plugins that ship as part of Datasette::\n") + cog.outl("If you run ``datasette plugins --all`` it will include default plugins that ship as part of Datasette:\n") + cog.outl(".. code-block:: json\n") plugins = [p for p in json.loads(result.output) if p["name"].startswith("datasette.")] indented = textwrap.indent(json.dumps(plugins, indent=4), " ") for line in indented.split("\n"): @@ -115,7 +121,9 @@ You can also use the ``datasette plugins`` command:: cog.out("\n\n") .. ]]] -If you run ``datasette plugins --all`` it will include default plugins that ship as part of Datasette:: +If you run ``datasette plugins --all`` it will include default plugins that ship as part of Datasette: + +.. code-block:: json [ { @@ -236,6 +244,22 @@ If you run ``datasette plugins --all`` it will include default plugins that ship You can add the ``--plugins-dir=`` option to include any plugins found in that directory. +Add ``--requirements`` to output a list of installed plugins that can then be installed in another Datasette instance using ``datasette install -r requirements.txt``:: + + datasette plugins --requirements + +The output will look something like this:: + + datasette-codespaces==0.1.1 + datasette-graphql==2.2 + datasette-json-html==1.0.1 + datasette-pretty-json==0.2.2 + datasette-x-forwarded-host==0.1 + +To write that to a ``requirements.txt`` file, run this:: + + datasette plugins --requirements > requirements.txt + .. _plugins_configuration: Plugin configuration @@ -390,7 +414,7 @@ Any values embedded in ``metadata.yaml`` will be visible to anyone who views the If you are publishing your data using the :ref:`datasette publish ` family of commands, you can use the ``--plugin-secret`` option to set these secrets at publish time. For example, using Heroku you might run the following command:: - $ datasette publish heroku my_database.db \ + datasette publish heroku my_database.db \ --name my-heroku-app-demo \ --install=datasette-auth-github \ --plugin-secret datasette-auth-github client_id your_client_id \ diff --git a/tests/test_cli.py b/tests/test_cli.py index 75724f61..056e2821 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -108,6 +108,9 @@ def test_plugins_cli(app_client): assert set(names).issuperset({p["name"] for p in EXPECTED_PLUGINS}) # And the following too: assert set(names).issuperset(DEFAULT_PLUGINS) + # --requirements should be empty because there are no installed non-plugins-dir plugins + result3 = runner.invoke(cli, ["plugins", "--requirements"]) + assert result3.output == "" def test_metadata_yaml(): From a3593c901580ea50854c3e0774b0ba0126e8a76f Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 17:32:07 -0700 Subject: [PATCH 315/823] on_success_message_sql, closes #2138 --- datasette/views/database.py | 29 ++++++++++++++++---- docs/sql_queries.rst | 21 ++++++++++---- tests/test_canned_queries.py | 53 +++++++++++++++++++++++++++++++----- 3 files changed, 85 insertions(+), 18 deletions(-) diff --git a/datasette/views/database.py b/datasette/views/database.py index cf76f3c2..79b3f88d 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -360,6 +360,10 @@ class QueryView(View): params[key] = str(value) else: params = dict(parse_qsl(body, keep_blank_values=True)) + + # Don't ever send csrftoken as a SQL parameter + params.pop("csrftoken", None) + # Should we return JSON? should_return_json = ( request.headers.get("accept") == "application/json" @@ -371,12 +375,27 @@ class QueryView(View): redirect_url = None try: cursor = await db.execute_write(canned_query["sql"], params_for_query) - message = canned_query.get( - "on_success_message" - ) or "Query executed, {} row{} affected".format( - cursor.rowcount, "" if cursor.rowcount == 1 else "s" - ) + # success message can come from on_success_message or on_success_message_sql + message = None message_type = datasette.INFO + on_success_message_sql = canned_query.get("on_success_message_sql") + if on_success_message_sql: + try: + message_result = ( + await db.execute(on_success_message_sql, params_for_query) + ).first() + if message_result: + message = message_result[0] + except Exception as ex: + message = "Error running on_success_message_sql: {}".format(ex) + message_type = datasette.ERROR + if not message: + message = canned_query.get( + "on_success_message" + ) or "Query executed, {} row{} affected".format( + cursor.rowcount, "" if cursor.rowcount == 1 else "s" + ) + redirect_url = canned_query.get("on_success_redirect") ok = True except Exception as ex: diff --git a/docs/sql_queries.rst b/docs/sql_queries.rst index 3c2cb228..1ae07e1f 100644 --- a/docs/sql_queries.rst +++ b/docs/sql_queries.rst @@ -392,6 +392,7 @@ This configuration will create a page at ``/mydatabase/add_name`` displaying a f You can customize how Datasette represents success and errors using the following optional properties: - ``on_success_message`` - the message shown when a query is successful +- ``on_success_message_sql`` - alternative to ``on_success_message``: a SQL query that should be executed to generate the message - ``on_success_redirect`` - the path or URL the user is redirected to on success - ``on_error_message`` - the message shown when a query throws an error - ``on_error_redirect`` - the path or URL the user is redirected to on error @@ -405,11 +406,12 @@ For example: "queries": { "add_name": { "sql": "INSERT INTO names (name) VALUES (:name)", + "params": ["name"], "write": True, - "on_success_message": "Name inserted", + "on_success_message_sql": "select 'Name inserted: ' || :name", "on_success_redirect": "/mydatabase/names", "on_error_message": "Name insert failed", - "on_error_redirect": "/mydatabase" + "on_error_redirect": "/mydatabase", } } } @@ -426,8 +428,10 @@ For example: queries: add_name: sql: INSERT INTO names (name) VALUES (:name) + params: + - name write: true - on_success_message: Name inserted + on_success_message_sql: 'select ''Name inserted: '' || :name' on_success_redirect: /mydatabase/names on_error_message: Name insert failed on_error_redirect: /mydatabase @@ -443,8 +447,11 @@ For example: "queries": { "add_name": { "sql": "INSERT INTO names (name) VALUES (:name)", + "params": [ + "name" + ], "write": true, - "on_success_message": "Name inserted", + "on_success_message_sql": "select 'Name inserted: ' || :name", "on_success_redirect": "/mydatabase/names", "on_error_message": "Name insert failed", "on_error_redirect": "/mydatabase" @@ -455,10 +462,12 @@ For example: } .. [[[end]]] -You can use ``"params"`` to explicitly list the named parameters that should be displayed as form fields - otherwise they will be automatically detected. +You can use ``"params"`` to explicitly list the named parameters that should be displayed as form fields - otherwise they will be automatically detected. ``"params"`` is not necessary in the above example, since without it ``"name"`` would be automatically detected from the query. You can pre-populate form fields when the page first loads using a query string, e.g. ``/mydatabase/add_name?name=Prepopulated``. The user will have to submit the form to execute the query. +If you specify a query in ``"on_success_message_sql"``, that query will be executed after the main query. The first column of the first row return by that query will be displayed as a success message. Named parameters from the main query will be made available to the success message query as well. + .. _canned_queries_magic_parameters: Magic parameters @@ -589,7 +598,7 @@ The JSON response will look like this: "redirect": "/data/add_name" } -The ``"message"`` and ``"redirect"`` values here will take into account ``on_success_message``, ``on_success_redirect``, ``on_error_message`` and ``on_error_redirect``, if they have been set. +The ``"message"`` and ``"redirect"`` values here will take into account ``on_success_message``, ``on_success_message_sql``, ``on_success_redirect``, ``on_error_message`` and ``on_error_redirect``, if they have been set. .. _pagination: diff --git a/tests/test_canned_queries.py b/tests/test_canned_queries.py index e9ad3239..5256c24c 100644 --- a/tests/test_canned_queries.py +++ b/tests/test_canned_queries.py @@ -31,9 +31,15 @@ def canned_write_client(tmpdir): }, "add_name_specify_id": { "sql": "insert into names (rowid, name) values (:rowid, :name)", + "on_success_message_sql": "select 'Name added: ' || :name || ' with rowid ' || :rowid", "write": True, "on_error_redirect": "/data/add_name_specify_id?error", }, + "add_name_specify_id_with_error_in_on_success_message_sql": { + "sql": "insert into names (rowid, name) values (:rowid, :name)", + "on_success_message_sql": "select this is bad SQL", + "write": True, + }, "delete_name": { "sql": "delete from names where rowid = :rowid", "write": True, @@ -179,6 +185,34 @@ def test_insert_error(canned_write_client): ) +def test_on_success_message_sql(canned_write_client): + response = canned_write_client.post( + "/data/add_name_specify_id", + {"rowid": 5, "name": "Should be OK"}, + csrftoken_from=True, + ) + assert response.status == 302 + assert response.headers["Location"] == "/data/add_name_specify_id" + messages = canned_write_client.ds.unsign( + response.cookies["ds_messages"], "messages" + ) + assert messages == [["Name added: Should be OK with rowid 5", 1]] + + +def test_error_in_on_success_message_sql(canned_write_client): + response = canned_write_client.post( + "/data/add_name_specify_id_with_error_in_on_success_message_sql", + {"rowid": 1, "name": "Should fail"}, + csrftoken_from=True, + ) + messages = canned_write_client.ds.unsign( + response.cookies["ds_messages"], "messages" + ) + assert messages == [ + ["Error running on_success_message_sql: no such column: bad", 3] + ] + + def test_custom_params(canned_write_client): response = canned_write_client.get("/data/update_name?extra=foo") assert '' in response.text @@ -232,21 +266,22 @@ def test_canned_query_permissions_on_database_page(canned_write_client): query_names = { q["name"] for q in canned_write_client.get("/data.json").json["queries"] } - assert { + assert query_names == { + "add_name_specify_id_with_error_in_on_success_message_sql", + "from_hook", + "update_name", + "add_name_specify_id", + "from_async_hook", "canned_read", "add_name", - "add_name_specify_id", - "update_name", - "from_async_hook", - "from_hook", - } == query_names + } # With auth shows four response = canned_write_client.get( "/data.json", cookies={"ds_actor": canned_write_client.actor_cookie({"id": "root"})}, ) - assert 200 == response.status + assert response.status == 200 query_names_and_private = sorted( [ {"name": q["name"], "private": q["private"]} @@ -257,6 +292,10 @@ def test_canned_query_permissions_on_database_page(canned_write_client): assert query_names_and_private == [ {"name": "add_name", "private": False}, {"name": "add_name_specify_id", "private": False}, + { + "name": "add_name_specify_id_with_error_in_on_success_message_sql", + "private": False, + }, {"name": "canned_read", "private": False}, {"name": "delete_name", "private": True}, {"name": "from_async_hook", "private": False}, From 33251d04e78d575cca62bb59069bb43a7d924746 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Aug 2023 17:56:27 -0700 Subject: [PATCH 316/823] Canned query write counters demo, refs #2134 --- .github/workflows/deploy-latest.yml | 30 +++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/.github/workflows/deploy-latest.yml b/.github/workflows/deploy-latest.yml index ed60376c..4746aa07 100644 --- a/.github/workflows/deploy-latest.yml +++ b/.github/workflows/deploy-latest.yml @@ -57,6 +57,36 @@ jobs: db.route = "alternative-route" ' > plugins/alternative_route.py cp fixtures.db fixtures2.db + - name: And the counters writable canned query demo + run: | + cat > plugins/counters.py < Date: Thu, 10 Aug 2023 22:16:19 -0700 Subject: [PATCH 317/823] Fixed display of database color Closes #2139, closes #2119 --- datasette/database.py | 7 +++++++ datasette/templates/database.html | 2 +- datasette/templates/query.html | 2 +- datasette/templates/row.html | 2 +- datasette/templates/table.html | 2 +- datasette/views/base.py | 4 ---- datasette/views/database.py | 8 +++----- datasette/views/index.py | 4 +--- datasette/views/row.py | 4 +++- datasette/views/table.py | 2 +- tests/test_html.py | 20 ++++++++++++++++++++ 11 files changed, 39 insertions(+), 18 deletions(-) diff --git a/datasette/database.py b/datasette/database.py index d8043c24..af39ac9e 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -1,6 +1,7 @@ import asyncio from collections import namedtuple from pathlib import Path +import hashlib import janus import queue import sys @@ -62,6 +63,12 @@ class Database: } return self._cached_table_counts + @property + def color(self): + if self.hash: + return self.hash[:6] + return hashlib.md5(self.name.encode("utf8")).hexdigest()[:6] + def suggest_name(self): if self.path: return Path(self.path).stem diff --git a/datasette/templates/database.html b/datasette/templates/database.html index 7acf0369..3d4dae07 100644 --- a/datasette/templates/database.html +++ b/datasette/templates/database.html @@ -10,7 +10,7 @@ {% block body_class %}db db-{{ database|to_css_class }}{% endblock %} {% block content %} - + + + + + + + +{% endblock %} diff --git a/datasette/templates/debug_check.html b/datasette/templates/debug_check.html new file mode 100644 index 00000000..b8bbd0a6 --- /dev/null +++ b/datasette/templates/debug_check.html @@ -0,0 +1,350 @@ +{% extends "base.html" %} + +{% block title %}Permission Check{% endblock %} + +{% block extra_head %} + + +{% endblock %} + +{% block content %} + +

Permission Check

+ +

Use this tool to test permission checks for the current actor. It queries the /-/check.json API endpoint.

+ +{% if request.actor %} +

Current actor: {{ request.actor.get("id", "anonymous") }}

+{% else %} +

Current actor: anonymous (not logged in)

+{% endif %} + +
+
+ + + The permission action to check +
+ +
+ + + For database-level permissions, specify the database name +
+ +
+ + + For table-level permissions, specify the table name (requires parent) +
+ + +
+ + + + + +{% endblock %} diff --git a/datasette/templates/debug_rules.html b/datasette/templates/debug_rules.html new file mode 100644 index 00000000..f45daf2f --- /dev/null +++ b/datasette/templates/debug_rules.html @@ -0,0 +1,268 @@ +{% extends "base.html" %} + +{% block title %}Permission Rules{% endblock %} + +{% block extra_head %} + +{% include "_permission_ui_styles.html" %} +{% endblock %} + +{% block content %} + +

Permission Rules

+ +

Use this tool to view the permission rules that allow the current actor to access resources for a given permission action. It queries the /-/rules.json API endpoint.

+ +{% if request.actor %} +

Current actor: {{ request.actor.get("id", "anonymous") }}

+{% else %} +

Current actor: anonymous (not logged in)

+{% endif %} + +
+
+
+ + + The permission action to check +
+ +
+ + + Number of results per page (max 200) +
+ +
+ +
+
+
+ + + + + +{% endblock %} diff --git a/datasette/utils/permissions.py b/datasette/utils/permissions.py new file mode 100644 index 00000000..7dc2eb4d --- /dev/null +++ b/datasette/utils/permissions.py @@ -0,0 +1,244 @@ +# perm_utils.py +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union +import sqlite3 + + +# ----------------------------- +# Plugin interface & utilities +# ----------------------------- + + +@dataclass +class PluginSQL: + """ + A plugin contributes SQL that yields: + parent TEXT NULL, + child TEXT NULL, + allow INTEGER, -- 1 allow, 0 deny + reason TEXT + """ + + source: str # identifier used for auditing (e.g., plugin name) + sql: str # SQL that SELECTs the 4 columns above + params: Dict[str, Any] # bound params for the SQL (values only; no ':' prefix) + + +def _namespace_params(i: int, params: Dict[str, Any]) -> Tuple[str, Dict[str, Any]]: + """ + Rewrite parameter placeholders to distinct names per plugin block. + Returns (rewritten_sql, namespaced_params). + """ + + replacements = {key: f"{key}_{i}" for key in params.keys()} + + def rewrite(s: str) -> str: + for key in sorted(replacements.keys(), key=len, reverse=True): + s = s.replace(f":{key}", f":{replacements[key]}") + return s + + namespaced: Dict[str, Any] = {} + for key, value in params.items(): + namespaced[replacements[key]] = value + return rewrite, namespaced + + +PluginProvider = Callable[[str], PluginSQL] +PluginOrFactory = Union[PluginSQL, PluginProvider] + + +def build_rules_union( + actor: str, plugins: Sequence[PluginSQL] +) -> Tuple[str, Dict[str, Any]]: + """ + Compose plugin SQL into a UNION ALL with namespaced parameters. + + Returns: + union_sql: a SELECT with columns (parent, child, allow, reason, source_plugin) + params: dict of bound parameters including :actor and namespaced plugin params + """ + parts: List[str] = [] + params: Dict[str, Any] = {"actor": actor} + + for i, p in enumerate(plugins): + rewrite, ns_params = _namespace_params(i, p.params) + sql_block = rewrite(p.sql) + params.update(ns_params) + + parts.append( + f""" + SELECT parent, child, allow, reason, '{p.source}' AS source_plugin FROM ( + {sql_block} + ) + """.strip() + ) + + if not parts: + # Empty UNION that returns no rows + union_sql = "SELECT NULL parent, NULL child, NULL allow, NULL reason, 'none' source_plugin WHERE 0" + else: + union_sql = "\nUNION ALL\n".join(parts) + + return union_sql, params + + +# ----------------------------------------------- +# Core resolvers (no temp tables, no custom UDFs) +# ----------------------------------------------- + + +async def resolve_permissions_from_catalog( + db, + actor: str, + plugins: Sequence[PluginOrFactory], + action: str, + candidate_sql: str, + candidate_params: Optional[Dict[str, Any]] = None, + *, + implicit_deny: bool = True, +) -> List[Dict[str, Any]]: + """ + Resolve permissions by embedding the provided *candidate_sql* in a CTE. + + Expectations: + - candidate_sql SELECTs: parent TEXT, child TEXT + (Use child=NULL for parent-scoped actions like "execute-sql".) + - *db* exposes: rows = await db.execute(sql, params) + where rows is an iterable of sqlite3.Row + - plugins are either PluginSQL objects or callables accepting (action: str) + and returning PluginSQL instances selecting (parent, child, allow, reason) + + Decision policy: + 1) Specificity first: child (depth=2) > parent (depth=1) > root (depth=0) + 2) Within the same depth: deny (0) beats allow (1) + 3) If no matching rule: + - implicit_deny=True -> treat as allow=0, reason='implicit deny' + - implicit_deny=False -> allow=None, reason=None + + Returns: list of dict rows + - parent, child, allow, reason, source_plugin, depth + - resource (rendered "/parent/child" or "/parent" or "/") + """ + resolved_plugins: List[PluginSQL] = [] + for plugin in plugins: + if callable(plugin) and not isinstance(plugin, PluginSQL): + resolved = plugin(action) # type: ignore[arg-type] + else: + resolved = plugin # type: ignore[assignment] + if not isinstance(resolved, PluginSQL): + raise TypeError("Plugin providers must return PluginSQL instances") + resolved_plugins.append(resolved) + + union_sql, rule_params = build_rules_union(actor, resolved_plugins) + all_params = { + **(candidate_params or {}), + **rule_params, + "actor": actor, + "action": action, + } + + sql = f""" + WITH + cands AS ( + {candidate_sql} + ), + rules AS ( + {union_sql} + ), + matched AS ( + SELECT + c.parent, c.child, + r.allow, r.reason, r.source_plugin, + CASE + WHEN r.child IS NOT NULL THEN 2 -- child-level (most specific) + WHEN r.parent IS NOT NULL THEN 1 -- parent-level + ELSE 0 -- root/global + END AS depth + FROM cands c + JOIN rules r + ON (r.parent IS NULL OR r.parent = c.parent) + AND (r.child IS NULL OR r.child = c.child) + ), + ranked AS ( + SELECT *, + ROW_NUMBER() OVER ( + PARTITION BY parent, child + ORDER BY + depth DESC, -- specificity first + CASE WHEN allow=0 THEN 0 ELSE 1 END, -- deny over allow at same depth + source_plugin -- stable tie-break + ) AS rn + FROM matched + ), + winner AS ( + SELECT parent, child, + allow, reason, source_plugin, depth + FROM ranked WHERE rn = 1 + ) + SELECT + c.parent, c.child, + COALESCE(w.allow, CASE WHEN :implicit_deny THEN 0 ELSE NULL END) AS allow, + COALESCE(w.reason, CASE WHEN :implicit_deny THEN 'implicit deny' ELSE NULL END) AS reason, + w.source_plugin, + COALESCE(w.depth, -1) AS depth, + :action AS action, + CASE + WHEN c.parent IS NULL THEN '/' + WHEN c.child IS NULL THEN '/' || c.parent + ELSE '/' || c.parent || '/' || c.child + END AS resource + FROM cands c + LEFT JOIN winner w + ON ((w.parent = c.parent) OR (w.parent IS NULL AND c.parent IS NULL)) + AND ((w.child = c.child ) OR (w.child IS NULL AND c.child IS NULL)) + ORDER BY c.parent, c.child + """ + + rows_iter: Iterable[sqlite3.Row] = await db.execute( + sql, + {**all_params, "implicit_deny": 1 if implicit_deny else 0}, + ) + return [dict(r) for r in rows_iter] + + +async def resolve_permissions_with_candidates( + db, + actor: str, + plugins: Sequence[PluginOrFactory], + candidates: List[Tuple[str, Optional[str]]], + action: str, + *, + implicit_deny: bool = True, +) -> List[Dict[str, Any]]: + """ + Resolve permissions without any external candidate table by embedding + the candidates as a UNION of parameterized SELECTs in a CTE. + + candidates: list of (parent, child) where child can be None for parent-scoped actions. + """ + # Build a small CTE for candidates. + cand_rows_sql: List[str] = [] + cand_params: Dict[str, Any] = {} + for i, (parent, child) in enumerate(candidates): + pkey = f"cand_p_{i}" + ckey = f"cand_c_{i}" + cand_params[pkey] = parent + cand_params[ckey] = child + cand_rows_sql.append(f"SELECT :{pkey} AS parent, :{ckey} AS child") + candidate_sql = ( + "\nUNION ALL\n".join(cand_rows_sql) + if cand_rows_sql + else "SELECT NULL AS parent, NULL AS child WHERE 0" + ) + + return await resolve_permissions_from_catalog( + db, + actor, + plugins, + action, + candidate_sql=candidate_sql, + candidate_params=cand_params, + implicit_deny=implicit_deny, + ) diff --git a/datasette/views/base.py b/datasette/views/base.py index bdc9f742..ea48a398 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -1,6 +1,7 @@ import asyncio import csv import hashlib +import json import sys import textwrap import time @@ -173,6 +174,24 @@ class BaseView: headers=headers, ) + async def respond_json_or_html(self, request, data, filename): + """Return JSON or HTML with pretty JSON depending on format parameter.""" + as_format = request.url_vars.get("format") + if as_format: + headers = {} + if self.ds.cors: + add_cors_headers(headers) + return Response.json(data, headers=headers) + else: + return await self.render( + ["show_json.html"], + request=request, + context={ + "filename": filename, + "data_json": json.dumps(data, indent=4, default=repr), + }, + ) + @classmethod def as_view(cls, *class_args, **class_kwargs): async def view(request, send): diff --git a/datasette/views/special.py b/datasette/views/special.py index e6fbc9f3..7e5ce517 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -1,17 +1,32 @@ import json +import logging from datasette.events import LogoutEvent, LoginEvent, CreateTokenEvent from datasette.utils.asgi import Response, Forbidden from datasette.utils import ( actor_matches_allow, add_cors_headers, + await_me_maybe, tilde_encode, tilde_decode, ) +from datasette.utils.permissions import PluginSQL, resolve_permissions_from_catalog +from datasette.plugins import pm from .base import BaseView, View import secrets import urllib +logger = logging.getLogger(__name__) + + +def _resource_path(parent, child): + if parent is None: + return "/" + if child is None: + return f"/{parent}" + return f"/{parent}/{child}" + + class JsonDataView(BaseView): name = "json_data" @@ -30,32 +45,13 @@ class JsonDataView(BaseView): self.permission = permission async def get(self, request): - as_format = request.url_vars["format"] if self.permission: await self.ds.ensure_permissions(request.actor, [self.permission]) if self.needs_request: data = self.data_callback(request) else: data = self.data_callback() - if as_format: - headers = {} - if self.ds.cors: - add_cors_headers(headers) - return Response( - json.dumps(data, default=repr), - content_type="application/json; charset=utf-8", - headers=headers, - ) - - else: - return await self.render( - ["show_json.html"], - request=request, - context={ - "filename": self.filename, - "data_json": json.dumps(data, indent=4, default=repr), - }, - ) + return await self.respond_json_or_html(request, data, self.filename) class PatternPortfolioView(View): @@ -187,6 +183,402 @@ class PermissionsDebugView(BaseView): ) +class AllowedResourcesView(BaseView): + name = "allowed" + has_json_alternate = False + + CANDIDATE_SQL = { + "view-table": ( + "SELECT database_name AS parent, table_name AS child FROM catalog_tables", + {}, + ), + "view-database": ( + "SELECT database_name AS parent, NULL AS child FROM catalog_databases", + {}, + ), + "view-instance": ("SELECT NULL AS parent, NULL AS child", {}), + "execute-sql": ( + "SELECT database_name AS parent, NULL AS child FROM catalog_databases", + {}, + ), + } + + async def get(self, request): + await self.ds.refresh_schemas() + + # Check if user has permissions-debug (to show sensitive fields) + has_debug_permission = await self.ds.permission_allowed( + request.actor, "permissions-debug" + ) + + # Check if this is a request for JSON (has .json extension) + as_format = request.url_vars.get("format") + + if not as_format: + # Render the HTML form (even if query parameters are present) + return await self.render( + ["debug_allowed.html"], + request, + { + "supported_actions": sorted(self.CANDIDATE_SQL.keys()), + }, + ) + + # JSON API - action parameter is required + action = request.args.get("action") + if not action: + return Response.json({"error": "action parameter is required"}, status=400) + if action not in self.ds.permissions: + return Response.json({"error": f"Unknown action: {action}"}, status=404) + if action not in self.CANDIDATE_SQL: + return Response.json( + {"error": f"Action '{action}' is not supported by this endpoint"}, + status=400, + ) + + actor = request.actor if isinstance(request.actor, dict) else None + parent_filter = request.args.get("parent") + child_filter = request.args.get("child") + if child_filter and not parent_filter: + return Response.json( + {"error": "parent must be provided when child is specified"}, + status=400, + ) + + try: + page = int(request.args.get("page", "1")) + page_size = int(request.args.get("page_size", "50")) + except ValueError: + return Response.json( + {"error": "page and page_size must be integers"}, status=400 + ) + if page < 1: + return Response.json({"error": "page must be >= 1"}, status=400) + if page_size < 1: + return Response.json({"error": "page_size must be >= 1"}, status=400) + max_page_size = 200 + if page_size > max_page_size: + page_size = max_page_size + offset = (page - 1) * page_size + + candidate_sql, candidate_params = self.CANDIDATE_SQL[action] + + db = self.ds.get_internal_database() + required_tables = set() + if "catalog_tables" in candidate_sql: + required_tables.add("catalog_tables") + if "catalog_databases" in candidate_sql: + required_tables.add("catalog_databases") + + for table in required_tables: + if not await db.table_exists(table): + headers = {} + if self.ds.cors: + add_cors_headers(headers) + return Response.json( + { + "action": action, + "actor_id": (actor or {}).get("id") if actor else None, + "page": page, + "page_size": page_size, + "total": 0, + "items": [], + }, + headers=headers, + ) + + plugins = [] + for block in pm.hook.permission_resources_sql( + datasette=self.ds, + actor=actor, + action=action, + ): + block = await await_me_maybe(block) + if block is None: + continue + if isinstance(block, (list, tuple)): + candidates = block + else: + candidates = [block] + for candidate in candidates: + if candidate is None: + continue + if not isinstance(candidate, PluginSQL): + logger.warning( + "Skipping permission_resources_sql result %r from plugin; expected PluginSQL", + candidate, + ) + continue + plugins.append(candidate) + + actor_id = actor.get("id") if actor else None + rows = await resolve_permissions_from_catalog( + db, + actor=str(actor_id) if actor_id is not None else "", + plugins=plugins, + action=action, + candidate_sql=candidate_sql, + candidate_params=candidate_params, + implicit_deny=True, + ) + + allowed_rows = [row for row in rows if row["allow"] == 1] + if parent_filter is not None: + allowed_rows = [ + row for row in allowed_rows if row["parent"] == parent_filter + ] + if child_filter is not None: + allowed_rows = [row for row in allowed_rows if row["child"] == child_filter] + total = len(allowed_rows) + paged_rows = allowed_rows[offset : offset + page_size] + + items = [] + for row in paged_rows: + item = { + "parent": row["parent"], + "child": row["child"], + "resource": row["resource"], + } + # Only include sensitive fields if user has permissions-debug + if has_debug_permission: + item["reason"] = row["reason"] + item["source_plugin"] = row["source_plugin"] + items.append(item) + + def build_page_url(page_number): + pairs = [] + for key in request.args: + if key in {"page", "page_size"}: + continue + for value in request.args.getlist(key): + pairs.append((key, value)) + pairs.append(("page", str(page_number))) + pairs.append(("page_size", str(page_size))) + query = urllib.parse.urlencode(pairs) + return f"{request.path}?{query}" + + response = { + "action": action, + "actor_id": actor_id, + "page": page, + "page_size": page_size, + "total": total, + "items": items, + } + + if total > offset + page_size: + response["next_url"] = build_page_url(page + 1) + if page > 1: + response["previous_url"] = build_page_url(page - 1) + + headers = {} + if self.ds.cors: + add_cors_headers(headers) + return Response.json(response, headers=headers) + + +class PermissionRulesView(BaseView): + name = "permission_rules" + has_json_alternate = False + + async def get(self, request): + await self.ds.ensure_permissions(request.actor, ["view-instance"]) + if not await self.ds.permission_allowed(request.actor, "permissions-debug"): + raise Forbidden("Permission denied") + + # Check if this is a request for JSON (has .json extension) + as_format = request.url_vars.get("format") + + if not as_format: + # Render the HTML form (even if query parameters are present) + return await self.render( + ["debug_rules.html"], + request, + { + "sorted_permissions": sorted(self.ds.permissions.keys()), + }, + ) + + # JSON API - action parameter is required + action = request.args.get("action") + if not action: + return Response.json({"error": "action parameter is required"}, status=400) + if action not in self.ds.permissions: + return Response.json({"error": f"Unknown action: {action}"}, status=404) + + actor = request.actor if isinstance(request.actor, dict) else None + + try: + page = int(request.args.get("page", "1")) + page_size = int(request.args.get("page_size", "50")) + except ValueError: + return Response.json( + {"error": "page and page_size must be integers"}, status=400 + ) + if page < 1: + return Response.json({"error": "page must be >= 1"}, status=400) + if page_size < 1: + return Response.json({"error": "page_size must be >= 1"}, status=400) + max_page_size = 200 + if page_size > max_page_size: + page_size = max_page_size + offset = (page - 1) * page_size + + union_sql, union_params = await self.ds.allowed_resources_sql(actor, action) + await self.ds.refresh_schemas() + db = self.ds.get_internal_database() + + count_query = f""" + WITH rules AS ( + {union_sql} + ) + SELECT COUNT(*) AS count + FROM rules + """ + count_row = (await db.execute(count_query, union_params)).first() + total = count_row["count"] if count_row else 0 + + data_query = f""" + WITH rules AS ( + {union_sql} + ) + SELECT parent, child, allow, reason, source_plugin + FROM rules + ORDER BY allow DESC, (parent IS NOT NULL), parent, child + LIMIT :limit OFFSET :offset + """ + params = {**union_params, "limit": page_size, "offset": offset} + rows = await db.execute(data_query, params) + + items = [] + for row in rows: + parent = row["parent"] + child = row["child"] + items.append( + { + "parent": parent, + "child": child, + "resource": _resource_path(parent, child), + "allow": row["allow"], + "reason": row["reason"], + "source_plugin": row["source_plugin"], + } + ) + + def build_page_url(page_number): + pairs = [] + for key in request.args: + if key in {"page", "page_size"}: + continue + for value in request.args.getlist(key): + pairs.append((key, value)) + pairs.append(("page", str(page_number))) + pairs.append(("page_size", str(page_size))) + query = urllib.parse.urlencode(pairs) + return f"{request.path}?{query}" + + response = { + "action": action, + "actor_id": (actor or {}).get("id") if actor else None, + "page": page, + "page_size": page_size, + "total": total, + "items": items, + } + + if total > offset + page_size: + response["next_url"] = build_page_url(page + 1) + if page > 1: + response["previous_url"] = build_page_url(page - 1) + + headers = {} + if self.ds.cors: + add_cors_headers(headers) + return Response.json(response, headers=headers) + + +class PermissionCheckView(BaseView): + name = "permission_check" + has_json_alternate = False + + async def get(self, request): + # Check if user has permissions-debug (to show sensitive fields) + has_debug_permission = await self.ds.permission_allowed( + request.actor, "permissions-debug" + ) + + # Check if this is a request for JSON (has .json extension) + as_format = request.url_vars.get("format") + + if not as_format: + # Render the HTML form (even if query parameters are present) + return await self.render( + ["debug_check.html"], + request, + { + "sorted_permissions": sorted(self.ds.permissions.keys()), + }, + ) + + # JSON API - action parameter is required + action = request.args.get("action") + if not action: + return Response.json({"error": "action parameter is required"}, status=400) + if action not in self.ds.permissions: + 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 + ) + + if parent and child: + resource = (parent, child) + elif parent: + resource = parent + else: + resource = None + + before_checks = len(self.ds._permission_checks) + allowed = await self.ds.permission_allowed_2(request.actor, action, resource) + + info = None + if len(self.ds._permission_checks) > before_checks: + for check in reversed(self.ds._permission_checks): + if ( + check.get("actor") == request.actor + and check.get("action") == action + and check.get("resource") == resource + ): + 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"] + + if info is not None: + response["used_default"] = info.get("used_default") + response["depth"] = info.get("depth") + # Only include sensitive fields if user has permissions-debug + if has_debug_permission: + response["reason"] = info.get("reason") + response["source_plugin"] = info.get("source_plugin") + + return Response.json(response) + + class AllowDebugView(BaseView): name = "allow_debug" has_json_alternate = False diff --git a/docs/authentication.rst b/docs/authentication.rst index 0343dc94..d16a7230 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -1050,6 +1050,62 @@ It also provides an interface for running hypothetical permission checks against This is designed to help administrators and plugin authors understand exactly how permission checks are being carried out, in order to effectively configure Datasette's permission system. +.. _AllowedResourcesView: + +Allowed resources view +====================== + +The ``/-/allowed`` endpoint displays resources that the current actor can access for a supplied ``action`` query string argument. + +This endpoint provides an interactive HTML form interface. Add ``.json`` to the URL path (e.g. ``/-/allowed.json``) to get the raw JSON response instead. + +Pass ``?action=view-table`` (or another action) to select the action. Optional ``parent=`` and ``child=`` query parameters can narrow the results to a specific database/table pair. + +This endpoint is publicly accessible to help users understand their own permissions. However, potentially sensitive fields (``reason`` and ``source_plugin``) are only included in responses for users with the ``permissions-debug`` permission. + +Datasette includes helper endpoints for exploring the action-based permission resolver: + +``/-/allowed`` + Returns a paginated list of resources that the current actor is allowed to access for a given action. Pass ``?action=view-table`` (or another action) to select the action, and optional ``parent=``/``child=`` query parameters to narrow the results to a specific database/table pair. + +``/-/rules`` + Lists the raw permission rules (both allow and deny) contributing to each resource for the supplied action. This includes configuration-derived and plugin-provided rules. **Requires the permissions-debug permission** (only available to the root user by default). + +``/-/check`` + Evaluates whether the current actor can perform ``action`` against an optional ``parent``/``child`` resource tuple, returning the winning rule and reason. + +These endpoints work in conjunction with :ref:`plugin_hook_permission_resources_sql` and make it easier to verify that configuration allow blocks and plugins are behaving as intended. + +All three endpoints support both HTML and JSON responses. Visit the endpoint directly for an interactive HTML form interface, or add ``.json`` to the URL for a raw JSON response. + +**Security note:** The ``/-/check`` and ``/-/allowed`` endpoints are publicly accessible to help users understand their own permissions. However, potentially sensitive fields (``reason`` and ``source_plugin``) are only included in responses for users with the ``permissions-debug`` permission. The ``/-/rules`` endpoint requires the ``permissions-debug`` permission for all access. + +.. _PermissionRulesView: + +Permission rules view +====================== + +The ``/-/rules`` endpoint displays all permission rules (both allow and deny) for each candidate resource for the requested action. + +This endpoint provides an interactive HTML form interface. Add ``.json`` to the URL path (e.g. ``/-/rules.json?action=view-table``) to get the raw JSON response instead. + +Pass ``?action=`` as a query parameter to specify which action to check. + +**Requires the permissions-debug permission** - this endpoint returns a 403 Forbidden error for users without this permission. The :ref:`root user ` has this permission by default. + +.. _PermissionCheckView: + +Permission check view +====================== + +The ``/-/check`` endpoint evaluates a single action/resource pair and returns information indicating whether the access was allowed along with diagnostic information. + +This endpoint provides an interactive HTML form interface. Add ``.json`` to the URL path (e.g. ``/-/check.json?action=view-instance``) to get the raw JSON response instead. + +Pass ``?action=`` to specify the action to check, and optional ``?parent=`` and ``?child=`` parameters to specify the resource. + +This endpoint is publicly accessible to help users understand their own permissions. However, potentially sensitive fields (``reason`` and ``source_plugin``) are only included in responses for users with the ``permissions-debug`` permission. + .. _authentication_ds_actor: The ds_actor cookie diff --git a/docs/contributing.rst b/docs/contributing.rst index c1268321..b4aab6ed 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -13,6 +13,7 @@ General guidelines * **main should always be releasable**. Incomplete features should live in branches. This ensures that any small bug fixes can be quickly released. * **The ideal commit** should bundle together the implementation, unit tests and associated documentation updates. The commit message should link to an associated issue. * **New plugin hooks** should only be shipped if accompanied by a separate release of a non-demo plugin that uses them. +* **New user-facing views and documentation** should be added or updated alongside their implementation. The `/docs` folder includes pages for plugin hooks and built-in views—please ensure any new hooks or views are reflected there so the documentation tests continue to pass. .. _devenvironment: diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 5b3baf3f..244f448d 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -1290,12 +1290,13 @@ Here's an example that allows users to view the ``admin_log`` table only if thei if not actor: return False user_id = actor["id"] - return await datasette.get_database( + result = await datasette.get_database( "staff" ).execute( "select count(*) from admin_users where user_id = :user_id", {"user_id": user_id}, ) + return result.first()[0] > 0 return inner @@ -1303,6 +1304,184 @@ See :ref:`built-in permissions ` for a full list of permissions tha Example: `datasette-permissions-sql `_ +.. _plugin_hook_permission_resources_sql: + +permission_resources_sql(datasette, actor, action) +------------------------------------------------- + +``datasette`` - :ref:`internals_datasette` + Access to the Datasette instance. + +``actor`` - dictionary or None + The current actor dictionary. ``None`` for anonymous requests. + +``action`` - string + The permission action being evaluated. Examples include ``"view-table"`` or ``"insert-row"``. + +Return value + A :class:`datasette.utils.permissions.PluginSQL` object, ``None`` or an iterable of ``PluginSQL`` objects. + +Datasette's action-based permission resolver calls this hook to gather SQL rows describing which +resources an actor may access (``allow = 1``) or should be denied (``allow = 0``) for a specific action. +Each SQL snippet should return ``parent``, ``child``, ``allow`` and ``reason`` columns. Any bound parameters +supplied via ``PluginSQL.params`` are automatically namespaced per plugin. + + +Permission plugin examples +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +These snippets show how to use the new ``permission_resources_sql`` hook to +contribute rows to the action-based permission resolver. Each hook receives the +current actor dictionary (or ``None``) and must return ``None`` or an instance or list of +``datasette.utils.permissions.PluginSQL`` (or a coroutine that resolves to that). + +Allow Alice to view a specific table +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This plugin grants the actor with ``id == "alice"`` permission to perform the +``view-table`` action against the ``sales`` table inside the ``accounting`` database. + +.. code-block:: python + + from datasette import hookimpl + from datasette.utils.permissions import PluginSQL + + + @hookimpl + def permission_resources_sql(datasette, actor, action): + if action != "view-table": + return None + if not actor or actor.get("id") != "alice": + return None + + return PluginSQL( + source="alice_sales_allow", + sql=""" + SELECT + 'accounting' AS parent, + 'sales' AS child, + 1 AS allow, + 'alice can view accounting/sales' AS reason + """, + params={}, + ) + +Restrict execute-sql to a database prefix +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Only allow ``execute-sql`` against databases whose name begins with +``analytics_``. This shows how to use parameters that the permission resolver +will pass through to the SQL snippet. + +.. code-block:: python + + from datasette import hookimpl + from datasette.utils.permissions import PluginSQL + + + @hookimpl + def permission_resources_sql(datasette, actor, action): + if action != "execute-sql": + return None + + return PluginSQL( + source="analytics_execute_sql", + sql=""" + SELECT + parent, + NULL AS child, + 1 AS allow, + 'execute-sql allowed for analytics_*' AS reason + FROM catalog_databases + WHERE database_name LIKE :prefix + """, + params={ + "prefix": "analytics_%", + }, + ) + +Read permissions from a custom table +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This example stores grants in an internal table called ``permission_grants`` +with columns ``(actor_id, action, parent, child, allow, reason)``. + +.. code-block:: python + + from datasette import hookimpl + from datasette.utils.permissions import PluginSQL + + + @hookimpl + def permission_resources_sql(datasette, actor, action): + if not actor: + return None + + return PluginSQL( + source="permission_grants_table", + sql=""" + SELECT + parent, + child, + allow, + COALESCE(reason, 'permission_grants table') AS reason + FROM permission_grants + WHERE actor_id = :actor_id + AND action = :action + """, + params={ + "actor_id": actor.get("id"), + "action": action, + }, + ) + +Default deny with an exception +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Combine a root-level deny with a specific table allow for trusted users. +The resolver will automatically apply the most specific rule. + +.. code-block:: python + + from datasette import hookimpl + from datasette.utils.permissions import PluginSQL + + + TRUSTED = {"alice", "bob"} + + + @hookimpl + def permission_resources_sql(datasette, actor, action): + if action != "view-table": + return None + + actor_id = (actor or {}).get("id") + + if actor_id not in TRUSTED: + return PluginSQL( + source="view_table_root_deny", + sql=""" + SELECT NULL AS parent, NULL AS child, 0 AS allow, + 'default deny view-table' AS reason + """, + params={}, + ) + + return PluginSQL( + source="trusted_allow", + sql=""" + SELECT NULL AS parent, NULL AS child, 0 AS allow, + 'default deny view-table' AS reason + UNION ALL + SELECT 'reports' AS parent, 'daily_metrics' AS child, 1 AS allow, + 'trusted user access' AS reason + """, + params={"actor_id": actor_id}, + ) + +The ``UNION ALL`` ensures the deny rule is always present, while the second row +adds the exception for trusted users. + .. _plugin_hook_register_magic_parameters: register_magic_parameters(datasette) diff --git a/docs/plugins.rst b/docs/plugins.rst index 03ddf8f0..4bc1b2a9 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -224,6 +224,7 @@ If you run ``datasette plugins --all`` it will include default plugins that ship "hooks": [ "actor_from_request", "permission_allowed", + "permission_resources_sql", "register_permissions", "skip_csrf" ] diff --git a/pytest.ini b/pytest.ini index 9f2caac0..29b84ea5 100644 --- a/pytest.ini +++ b/pytest.ini @@ -6,5 +6,4 @@ filterwarnings= ignore:Using or importing the ABCs::bs4.element markers = serial: tests to avoid using with pytest-xdist -asyncio_mode = strict -asyncio_default_fixture_loop_scope = function \ No newline at end of file +asyncio_mode = strict \ No newline at end of file diff --git a/setup.py b/setup.py index f68e621e..214ce36e 100644 --- a/setup.py +++ b/setup.py @@ -80,7 +80,7 @@ setup( "test": [ "pytest>=5.2.2", "pytest-xdist>=2.2.1", - "pytest-asyncio>=0.17", + "pytest-asyncio>=1.2.0", "beautifulsoup4>=4.8.1", "black==25.1.0", "blacken-docs==1.19.1", diff --git a/tests/test_config_permission_rules.py b/tests/test_config_permission_rules.py new file mode 100644 index 00000000..aeebcc29 --- /dev/null +++ b/tests/test_config_permission_rules.py @@ -0,0 +1,118 @@ +import pytest + +from datasette.app import Datasette +from datasette.database import Database + + +async def setup_datasette(config=None, databases=None): + ds = Datasette(memory=True, config=config) + for name in databases or []: + ds.add_database(Database(ds, memory_name=f"{name}_memory"), name=name) + await ds.invoke_startup() + await ds.refresh_schemas() + return ds + + +@pytest.mark.asyncio +async def test_root_permissions_allow(): + config = {"permissions": {"execute-sql": {"id": "alice"}}} + ds = await setup_datasette(config=config, databases=["content"]) + + assert await ds.permission_allowed_2({"id": "alice"}, "execute-sql", "content") + assert not await ds.permission_allowed_2({"id": "bob"}, "execute-sql", "content") + + +@pytest.mark.asyncio +async def test_database_permission(): + config = { + "databases": { + "content": { + "permissions": { + "insert-row": {"id": "alice"}, + } + } + } + } + ds = await setup_datasette(config=config, databases=["content"]) + + assert await ds.permission_allowed_2( + {"id": "alice"}, "insert-row", ("content", "repos") + ) + assert not await ds.permission_allowed_2( + {"id": "bob"}, "insert-row", ("content", "repos") + ) + + +@pytest.mark.asyncio +async def test_table_permission(): + config = { + "databases": { + "content": { + "tables": {"repos": {"permissions": {"delete-row": {"id": "alice"}}}} + } + } + } + ds = await setup_datasette(config=config, databases=["content"]) + + assert await ds.permission_allowed_2( + {"id": "alice"}, "delete-row", ("content", "repos") + ) + assert not await ds.permission_allowed_2( + {"id": "bob"}, "delete-row", ("content", "repos") + ) + + +@pytest.mark.asyncio +async def test_view_table_allow_block(): + config = { + "databases": {"content": {"tables": {"repos": {"allow": {"id": "alice"}}}}} + } + ds = await setup_datasette(config=config, databases=["content"]) + + assert await ds.permission_allowed_2( + {"id": "alice"}, "view-table", ("content", "repos") + ) + assert not await ds.permission_allowed_2( + {"id": "bob"}, "view-table", ("content", "repos") + ) + assert await ds.permission_allowed_2( + {"id": "bob"}, "view-table", ("content", "other") + ) + + +@pytest.mark.asyncio +async def test_view_table_allow_false_blocks(): + config = {"databases": {"content": {"tables": {"repos": {"allow": False}}}}} + ds = await setup_datasette(config=config, databases=["content"]) + + assert not await ds.permission_allowed_2( + {"id": "alice"}, "view-table", ("content", "repos") + ) + + +@pytest.mark.asyncio +async def test_allow_sql_blocks(): + config = {"allow_sql": {"id": "alice"}} + ds = await setup_datasette(config=config, databases=["content"]) + + assert await ds.permission_allowed_2({"id": "alice"}, "execute-sql", "content") + assert not await ds.permission_allowed_2({"id": "bob"}, "execute-sql", "content") + + config = {"databases": {"content": {"allow_sql": {"id": "bob"}}}} + ds = await setup_datasette(config=config, databases=["content"]) + + assert await ds.permission_allowed_2({"id": "bob"}, "execute-sql", "content") + assert not await ds.permission_allowed_2({"id": "alice"}, "execute-sql", "content") + + config = {"allow_sql": False} + ds = await setup_datasette(config=config, databases=["content"]) + assert not await ds.permission_allowed_2({"id": "alice"}, "execute-sql", "content") + + +@pytest.mark.asyncio +async def test_view_instance_allow_block(): + config = {"allow": {"id": "alice"}} + ds = await setup_datasette(config=config) + + assert await ds.permission_allowed_2({"id": "alice"}, "view-instance") + assert not await ds.permission_allowed_2({"id": "bob"}, "view-instance") diff --git a/tests/test_permission_endpoints.py b/tests/test_permission_endpoints.py new file mode 100644 index 00000000..3952259e --- /dev/null +++ b/tests/test_permission_endpoints.py @@ -0,0 +1,495 @@ +""" +Tests for permission inspection endpoints: +- /-/check.json +- /-/allowed.json +- /-/rules.json +""" + +import pytest +import pytest_asyncio +from datasette.app import Datasette + + +@pytest_asyncio.fixture +async def ds_with_permissions(): + """Create a Datasette instance with some permission rules configured.""" + ds = Datasette( + config={ + "databases": { + "content": { + "allow": {"id": "*"}, # Allow all authenticated users + "tables": { + "articles": { + "allow": {"id": "editor"}, # Only editor can view + } + }, + }, + "private": { + "allow": False, # Deny everyone + }, + } + } + ) + await ds.invoke_startup() + # Add some test databases + ds.add_memory_database("content") + ds.add_memory_database("private") + return ds + + +# /-/check.json tests +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path,expected_status,expected_keys", + [ + # Valid request + ( + "/-/check.json?action=view-instance", + 200, + {"action", "allowed", "resource"}, + ), + # Missing action parameter + ("/-/check.json", 400, {"error"}), + # Invalid action + ("/-/check.json?action=nonexistent", 404, {"error"}), + # With parent parameter + ( + "/-/check.json?action=view-database&parent=content", + 200, + {"action", "allowed", "resource"}, + ), + # With parent and child parameters + ( + "/-/check.json?action=view-table&parent=content&child=articles", + 200, + {"action", "allowed", "resource"}, + ), + ], +) +async def test_check_json_basic( + ds_with_permissions, path, expected_status, expected_keys +): + response = await ds_with_permissions.client.get(path) + assert response.status_code == expected_status + data = response.json() + assert expected_keys.issubset(data.keys()) + + +@pytest.mark.asyncio +async def test_check_json_response_structure(ds_with_permissions): + """Test that /-/check.json returns the expected structure.""" + response = await ds_with_permissions.client.get( + "/-/check.json?action=view-instance" + ) + assert response.status_code == 200 + data = response.json() + + # Check required fields + assert "action" in data + assert "allowed" in data + assert "resource" in data + + # Check resource structure + assert "parent" in data["resource"] + assert "child" in data["resource"] + assert "path" in data["resource"] + + # Check allowed is boolean + assert isinstance(data["allowed"], bool) + + +@pytest.mark.asyncio +async def test_check_json_redacts_sensitive_fields_without_debug_permission( + ds_with_permissions, +): + """Test that /-/check.json redacts reason and source_plugin without permissions-debug.""" + # Anonymous user should not see sensitive fields + response = await ds_with_permissions.client.get( + "/-/check.json?action=view-instance" + ) + assert response.status_code == 200 + data = response.json() + # Sensitive fields should not be present + assert "reason" not in data + assert "source_plugin" not in data + # But these non-sensitive fields should be present + assert "used_default" in data + assert "depth" in data + + +@pytest.mark.asyncio +async def test_check_json_shows_sensitive_fields_with_debug_permission( + ds_with_permissions, +): + """Test that /-/check.json shows reason and source_plugin with permissions-debug.""" + # User with permissions-debug should see sensitive fields + response = await ds_with_permissions.client.get( + "/-/check.json?action=view-instance", + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == 200 + data = response.json() + # Sensitive fields should be present + assert "reason" in data + assert "source_plugin" in data + assert "used_default" in data + assert "depth" in data + + +@pytest.mark.asyncio +async def test_check_json_child_requires_parent(ds_with_permissions): + """Test that child parameter requires parent parameter.""" + response = await ds_with_permissions.client.get( + "/-/check.json?action=view-table&child=articles" + ) + assert response.status_code == 400 + data = response.json() + assert "error" in data + assert "parent" in data["error"].lower() + + +# /-/allowed.json tests +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path,expected_status,expected_keys", + [ + # Valid supported actions + ( + "/-/allowed.json?action=view-instance", + 200, + {"action", "items", "total", "page"}, + ), + ( + "/-/allowed.json?action=view-database", + 200, + {"action", "items", "total", "page"}, + ), + ( + "/-/allowed.json?action=view-table", + 200, + {"action", "items", "total", "page"}, + ), + ( + "/-/allowed.json?action=execute-sql", + 200, + {"action", "items", "total", "page"}, + ), + # Missing action parameter + ("/-/allowed.json", 400, {"error"}), + # Invalid action + ("/-/allowed.json?action=nonexistent", 404, {"error"}), + # Unsupported action (valid but not in CANDIDATE_SQL) + ("/-/allowed.json?action=insert-row", 400, {"error"}), + ], +) +async def test_allowed_json_basic( + ds_with_permissions, path, expected_status, expected_keys +): + response = await ds_with_permissions.client.get(path) + assert response.status_code == expected_status + data = response.json() + assert expected_keys.issubset(data.keys()) + + +@pytest.mark.asyncio +async def test_allowed_json_response_structure(ds_with_permissions): + """Test that /-/allowed.json returns the expected structure.""" + response = await ds_with_permissions.client.get( + "/-/allowed.json?action=view-instance" + ) + assert response.status_code == 200 + data = response.json() + + # Check required fields + assert "action" in data + assert "actor_id" in data + assert "page" in data + assert "page_size" in data + assert "total" in data + assert "items" in data + + # Check items structure + assert isinstance(data["items"], list) + if data["items"]: + item = data["items"][0] + assert "parent" in item + assert "child" in item + assert "resource" in item + + +@pytest.mark.asyncio +async def test_allowed_json_redacts_sensitive_fields_without_debug_permission( + ds_with_permissions, +): + """Test that /-/allowed.json redacts reason and source_plugin without permissions-debug.""" + # Anonymous user should not see sensitive fields + response = await ds_with_permissions.client.get( + "/-/allowed.json?action=view-instance" + ) + assert response.status_code == 200 + data = response.json() + if data["items"]: + item = data["items"][0] + assert "reason" not in item + assert "source_plugin" not in item + + +@pytest.mark.asyncio +async def test_allowed_json_shows_sensitive_fields_with_debug_permission( + ds_with_permissions, +): + """Test that /-/allowed.json shows reason and source_plugin with permissions-debug.""" + # User with permissions-debug should see sensitive fields + response = await ds_with_permissions.client.get( + "/-/allowed.json?action=view-instance", + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == 200 + data = response.json() + if data["items"]: + item = data["items"][0] + assert "reason" in item + assert "source_plugin" in item + + +@pytest.mark.asyncio +async def test_allowed_json_only_shows_allowed_resources(ds_with_permissions): + """Test that /-/allowed.json only shows resources with allow=1.""" + response = await ds_with_permissions.client.get( + "/-/allowed.json?action=view-instance" + ) + assert response.status_code == 200 + data = response.json() + + # All items should have allow implicitly set to 1 (not in response but verified by the endpoint logic) + # The endpoint filters to only show allowed resources + assert isinstance(data["items"], list) + assert data["total"] >= 0 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "page,page_size", + [ + (1, 10), + (2, 50), + (1, 200), # max page size + ], +) +async def test_allowed_json_pagination(ds_with_permissions, page, page_size): + """Test pagination parameters.""" + response = await ds_with_permissions.client.get( + f"/-/allowed.json?action=view-instance&page={page}&page_size={page_size}" + ) + assert response.status_code == 200 + data = response.json() + assert data["page"] == page + assert data["page_size"] == min(page_size, 200) # Capped at 200 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "params,expected_status", + [ + ("page=0", 400), # page must be >= 1 + ("page=-1", 400), + ("page_size=0", 400), # page_size must be >= 1 + ("page_size=-1", 400), + ("page=abc", 400), # page must be integer + ("page_size=xyz", 400), # page_size must be integer + ], +) +async def test_allowed_json_pagination_errors( + ds_with_permissions, params, expected_status +): + """Test pagination error handling.""" + response = await ds_with_permissions.client.get( + f"/-/allowed.json?action=view-instance&{params}" + ) + assert response.status_code == expected_status + + +# /-/rules.json tests +@pytest.mark.asyncio +async def test_rules_json_requires_permissions_debug(ds_with_permissions): + """Test that /-/rules.json requires permissions-debug permission.""" + # Anonymous user should be denied + response = await ds_with_permissions.client.get( + "/-/rules.json?action=view-instance" + ) + assert response.status_code == 403 + + # Regular authenticated user should also be denied + response = await ds_with_permissions.client.get( + "/-/rules.json?action=view-instance", + cookies={ + "ds_actor": ds_with_permissions.client.actor_cookie({"id": "regular-user"}) + }, + ) + assert response.status_code == 403 + + # User with permissions-debug should be allowed + response = await ds_with_permissions.client.get( + "/-/rules.json?action=view-instance", + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == 200 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path,expected_status,expected_keys", + [ + # Valid request + ( + "/-/rules.json?action=view-instance", + 200, + {"action", "items", "total", "page"}, + ), + ( + "/-/rules.json?action=view-database", + 200, + {"action", "items", "total", "page"}, + ), + # Missing action parameter + ("/-/rules.json", 400, {"error"}), + # Invalid action + ("/-/rules.json?action=nonexistent", 404, {"error"}), + ], +) +async def test_rules_json_basic( + ds_with_permissions, path, expected_status, expected_keys +): + # Use debugger user who has permissions-debug + response = await ds_with_permissions.client.get( + path, + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == expected_status + data = response.json() + assert expected_keys.issubset(data.keys()) + + +@pytest.mark.asyncio +async def test_rules_json_response_structure(ds_with_permissions): + """Test that /-/rules.json returns the expected structure.""" + response = await ds_with_permissions.client.get( + "/-/rules.json?action=view-instance", + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == 200 + data = response.json() + + # Check required fields + assert "action" in data + assert "actor_id" in data + assert "page" in data + assert "page_size" in data + assert "total" in data + assert "items" in data + + # Check items structure + assert isinstance(data["items"], list) + if data["items"]: + item = data["items"][0] + assert "parent" in item + assert "child" in item + assert "resource" in item + assert "allow" in item # Important: should include allow field + assert "reason" in item + assert "source_plugin" in item + + +@pytest.mark.asyncio +async def test_rules_json_includes_both_allow_and_deny(ds_with_permissions): + """Test that /-/rules.json includes both allow and deny rules.""" + response = await ds_with_permissions.client.get( + "/-/rules.json?action=view-database", + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == 200 + data = response.json() + + # Check that items have the allow field + assert isinstance(data["items"], list) + if data["items"]: + # Verify allow field exists and is 0 or 1 + for item in data["items"]: + assert "allow" in item + assert item["allow"] in (0, 1) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "page,page_size", + [ + (1, 10), + (2, 50), + (1, 200), # max page size + ], +) +async def test_rules_json_pagination(ds_with_permissions, page, page_size): + """Test pagination parameters.""" + response = await ds_with_permissions.client.get( + f"/-/rules.json?action=view-instance&page={page}&page_size={page_size}", + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == 200 + data = response.json() + assert data["page"] == page + assert data["page_size"] == min(page_size, 200) # Capped at 200 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "params,expected_status", + [ + ("page=0", 400), # page must be >= 1 + ("page=-1", 400), + ("page_size=0", 400), # page_size must be >= 1 + ("page_size=-1", 400), + ("page=abc", 400), # page must be integer + ("page_size=xyz", 400), # page_size must be integer + ], +) +async def test_rules_json_pagination_errors( + ds_with_permissions, params, expected_status +): + """Test pagination error handling.""" + response = await ds_with_permissions.client.get( + f"/-/rules.json?action=view-instance&{params}", + cookies={"ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"})}, + ) + assert response.status_code == expected_status + + +# Test that HTML endpoints return HTML (not JSON) when accessed without .json +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path,needs_debug", + [ + ("/-/check", False), + ("/-/check?action=view-instance", False), + ("/-/allowed", False), + ("/-/allowed?action=view-instance", False), + ("/-/rules", True), + ("/-/rules?action=view-instance", True), + ], +) +async def test_html_endpoints_return_html(ds_with_permissions, path, needs_debug): + """Test that endpoints without .json extension return HTML.""" + if needs_debug: + # Rules endpoint requires permissions-debug + response = await ds_with_permissions.client.get( + path, + cookies={ + "ds_actor": ds_with_permissions.client.actor_cookie({"id": "root"}) + }, + ) + else: + response = await ds_with_permissions.client.get(path) + assert response.status_code == 200 + assert "text/html" in response.headers["content-type"] + # Check for HTML structure + text = response.text + assert "" in text or " PluginProvider: + def provider(action: str) -> PluginSQL: + return PluginSQL( + "allow_all", + """ + SELECT NULL AS parent, NULL AS child, 1 AS allow, + 'global allow for ' || :user || ' on ' || :action AS reason + WHERE :actor = :user + """, + {"user": user, "action": action}, + ) + + return provider + + +def plugin_deny_specific_table(user: str, parent: str, child: str) -> PluginProvider: + def provider(action: str) -> PluginSQL: + return PluginSQL( + "deny_specific_table", + """ + SELECT :parent AS parent, :child AS child, 0 AS allow, + 'deny ' || :parent || '/' || :child || ' for ' || :user || ' on ' || :action AS reason + WHERE :actor = :user + """, + {"parent": parent, "child": child, "user": user, "action": action}, + ) + + return provider + + +def plugin_org_policy_deny_parent(parent: str) -> PluginProvider: + def provider(action: str) -> PluginSQL: + return PluginSQL( + "org_policy_parent_deny", + """ + SELECT :parent AS parent, NULL AS child, 0 AS allow, + 'org policy: parent ' || :parent || ' denied on ' || :action AS reason + """, + {"parent": parent, "action": action}, + ) + + return provider + + +def plugin_allow_parent_for_user(user: str, parent: str) -> PluginProvider: + def provider(action: str) -> PluginSQL: + return PluginSQL( + "allow_parent", + """ + SELECT :parent AS parent, NULL AS child, 1 AS allow, + 'allow full parent for ' || :user || ' on ' || :action AS reason + WHERE :actor = :user + """, + {"parent": parent, "user": user, "action": action}, + ) + + return provider + + +def plugin_child_allow_for_user(user: str, parent: str, child: str) -> PluginProvider: + def provider(action: str) -> PluginSQL: + return PluginSQL( + "allow_child", + """ + SELECT :parent AS parent, :child AS child, 1 AS allow, + 'allow child for ' || :user || ' on ' || :action AS reason + WHERE :actor = :user + """, + {"parent": parent, "child": child, "user": user, "action": action}, + ) + + return provider + + +def plugin_root_deny_for_all() -> PluginProvider: + def provider(action: str) -> PluginSQL: + return PluginSQL( + "root_deny", + """ + SELECT NULL AS parent, NULL AS child, 0 AS allow, 'root deny for all on ' || :action AS reason + """, + {"action": action}, + ) + + return provider + + +def plugin_conflicting_same_child_rules( + user: str, parent: str, child: str +) -> List[PluginProvider]: + def allow_provider(action: str) -> PluginSQL: + return PluginSQL( + "conflict_child_allow", + """ + SELECT :parent AS parent, :child AS child, 1 AS allow, + 'team grant at child for ' || :user || ' on ' || :action AS reason + WHERE :actor = :user + """, + {"parent": parent, "child": child, "user": user, "action": action}, + ) + + def deny_provider(action: str) -> PluginSQL: + return PluginSQL( + "conflict_child_deny", + """ + SELECT :parent AS parent, :child AS child, 0 AS allow, + 'exception deny at child for ' || :user || ' on ' || :action AS reason + WHERE :actor = :user + """, + {"parent": parent, "child": child, "user": user, "action": action}, + ) + + return [allow_provider, deny_provider] + + +def plugin_allow_all_for_action(user: str, allowed_action: str) -> PluginProvider: + def provider(action: str) -> PluginSQL: + if action != allowed_action: + return PluginSQL( + f"allow_all_{allowed_action}_noop", + NO_RULES_SQL, + {}, + ) + return PluginSQL( + f"allow_all_{allowed_action}", + """ + SELECT NULL AS parent, NULL AS child, 1 AS allow, + 'global allow for ' || :user || ' on ' || :action AS reason + WHERE :actor = :user + """, + {"user": user, "action": action}, + ) + + return provider + + +VIEW_TABLE = "view-table" + + +# ---------- Catalog DDL (from your schema) ---------- +CATALOG_DDL = """ +CREATE TABLE IF NOT EXISTS catalog_databases ( + database_name TEXT PRIMARY KEY, + path TEXT, + is_memory INTEGER, + schema_version INTEGER +); +CREATE TABLE IF NOT EXISTS catalog_tables ( + database_name TEXT, + table_name TEXT, + rootpage INTEGER, + sql TEXT, + PRIMARY KEY (database_name, table_name), + FOREIGN KEY (database_name) REFERENCES catalog_databases(database_name) +); +""" + +PARENTS = ["accounting", "hr", "analytics"] +SPECIALS = {"accounting": ["sales"], "analytics": ["secret"], "hr": []} + +TABLE_CANDIDATES_SQL = ( + "SELECT database_name AS parent, table_name AS child FROM catalog_tables" +) +PARENT_CANDIDATES_SQL = ( + "SELECT database_name AS parent, NULL AS child FROM catalog_databases" +) + + +# ---------- Helpers ---------- +async def seed_catalog(db, per_parent: int = 10) -> None: + await db.execute_write_script(CATALOG_DDL) + # databases + db_rows = [(p, f"/{p}.db", 0, 1) for p in PARENTS] + await db.execute_write_many( + "INSERT OR REPLACE INTO catalog_databases(database_name, path, is_memory, schema_version) VALUES (?,?,?,?)", + db_rows, + ) + + # tables + def tables_for(parent: str, n: int): + base = [f"table{i:02d}" for i in range(1, n + 1)] + for s in SPECIALS.get(parent, []): + if s not in base: + base[0] = s + return base + + table_rows = [] + for p in PARENTS: + for t in tables_for(p, per_parent): + table_rows.append((p, t, 0, f"CREATE TABLE {t} (id INTEGER PRIMARY KEY)")) + await db.execute_write_many( + "INSERT OR REPLACE INTO catalog_tables(database_name, table_name, rootpage, sql) VALUES (?,?,?,?)", + table_rows, + ) + + +def res_allowed(rows, parent=None): + return sorted( + r["resource"] + for r in rows + if r["allow"] == 1 and (parent is None or r["parent"] == parent) + ) + + +def res_denied(rows, parent=None): + return sorted( + r["resource"] + for r in rows + if r["allow"] == 0 and (parent is None or r["parent"] == parent) + ) + + +# ---------- Tests ---------- +@pytest.mark.asyncio +async def test_alice_global_allow_with_specific_denies_catalog(db): + await seed_catalog(db) + plugins = [ + plugin_allow_all_for_user("alice"), + plugin_deny_specific_table("alice", "accounting", "sales"), + plugin_org_policy_deny_parent("hr"), + ] + rows = await resolve_permissions_from_catalog( + db, "alice", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True + ) + # Alice can see everything except accounting/sales and hr/* + assert "/accounting/sales" in res_denied(rows) + for r in rows: + if r["parent"] == "hr": + assert r["allow"] == 0 + elif r["resource"] == "/accounting/sales": + assert r["allow"] == 0 + else: + assert r["allow"] == 1 + + +@pytest.mark.asyncio +async def test_carol_parent_allow_but_child_conflict_deny_wins_catalog(db): + await seed_catalog(db) + plugins = [ + plugin_org_policy_deny_parent("hr"), + plugin_allow_parent_for_user("carol", "analytics"), + *plugin_conflicting_same_child_rules("carol", "analytics", "secret"), + ] + rows = await resolve_permissions_from_catalog( + db, "carol", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True + ) + allowed_analytics = res_allowed(rows, parent="analytics") + denied_analytics = res_denied(rows, parent="analytics") + + assert "/analytics/secret" in denied_analytics + # 10 analytics children total, 1 denied + assert len(allowed_analytics) == 9 + + +@pytest.mark.asyncio +async def test_specificity_child_allow_overrides_parent_deny_catalog(db): + await seed_catalog(db) + plugins = [ + plugin_allow_all_for_user("alice"), + plugin_org_policy_deny_parent("analytics"), # parent-level deny + plugin_child_allow_for_user( + "alice", "analytics", "table02" + ), # child allow beats parent deny + ] + rows = await resolve_permissions_from_catalog( + db, "alice", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True + ) + + # table02 allowed, other analytics tables denied + assert any(r["resource"] == "/analytics/table02" and r["allow"] == 1 for r in rows) + assert all( + (r["parent"] != "analytics" or r["child"] == "table02" or r["allow"] == 0) + for r in rows + ) + + +@pytest.mark.asyncio +async def test_root_deny_all_but_parent_allow_rescues_specific_parent_catalog(db): + await seed_catalog(db) + plugins = [ + plugin_root_deny_for_all(), # root deny + plugin_allow_parent_for_user( + "bob", "accounting" + ), # parent allow (more specific) + ] + rows = await resolve_permissions_from_catalog( + db, "bob", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True + ) + for r in rows: + if r["parent"] == "accounting": + assert r["allow"] == 1 + else: + assert r["allow"] == 0 + + +@pytest.mark.asyncio +async def test_parent_scoped_candidates(db): + await seed_catalog(db) + plugins = [ + plugin_org_policy_deny_parent("hr"), + plugin_allow_parent_for_user("carol", "analytics"), + ] + rows = await resolve_permissions_from_catalog( + db, "carol", plugins, VIEW_TABLE, PARENT_CANDIDATES_SQL, implicit_deny=True + ) + d = {r["resource"]: r["allow"] for r in rows} + assert d["/analytics"] == 1 + assert d["/hr"] == 0 + + +@pytest.mark.asyncio +async def test_implicit_deny_behavior(db): + await seed_catalog(db) + plugins = [] # no rules at all + + # implicit_deny=True -> everything denied with reason 'implicit deny' + rows = await resolve_permissions_from_catalog( + db, "erin", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=True + ) + assert all(r["allow"] == 0 and r["reason"] == "implicit deny" for r in rows) + + # implicit_deny=False -> no winner => allow is None, reason is None + rows2 = await resolve_permissions_from_catalog( + db, "erin", plugins, VIEW_TABLE, TABLE_CANDIDATES_SQL, implicit_deny=False + ) + assert all(r["allow"] is None and r["reason"] is None for r in rows2) + + +@pytest.mark.asyncio +async def test_candidate_filters_via_params(db): + await seed_catalog(db) + # Add some metadata to test filtering + # Mark 'hr' as is_memory=1 and increment analytics schema_version + await db.execute_write( + "UPDATE catalog_databases SET is_memory=1 WHERE database_name='hr'" + ) + await db.execute_write( + "UPDATE catalog_databases SET schema_version=2 WHERE database_name='analytics'" + ) + + # Candidate SQL that filters by db metadata via params + candidate_sql = """ + SELECT t.database_name AS parent, t.table_name AS child + FROM catalog_tables t + JOIN catalog_databases d ON d.database_name = t.database_name + WHERE (:exclude_memory = 1 AND d.is_memory = 1) IS NOT 1 + AND (:min_schema_version IS NULL OR d.schema_version >= :min_schema_version) + """ + + plugins = [ + plugin_root_deny_for_all(), + plugin_allow_parent_for_user( + "dev", "analytics" + ), # analytics rescued if included by candidates + ] + + # Case 1: exclude memory dbs, require schema_version >= 2 -> only analytics appear, and thus are allowed + rows = await resolve_permissions_from_catalog( + db, + "dev", + plugins, + VIEW_TABLE, + candidate_sql, + candidate_params={"exclude_memory": 1, "min_schema_version": 2}, + implicit_deny=True, + ) + assert rows and all(r["parent"] == "analytics" for r in rows) + assert all(r["allow"] == 1 for r in rows) + + # Case 2: include memory dbs, min_schema_version = None -> accounting/hr/analytics appear, + # but root deny wins except where specifically allowed (none except analytics parent allow doesn’t apply to table depth if candidate includes children; still fine—policy is explicit). + rows2 = await resolve_permissions_from_catalog( + db, + "dev", + plugins, + VIEW_TABLE, + candidate_sql, + candidate_params={"exclude_memory": 0, "min_schema_version": None}, + implicit_deny=True, + ) + assert any(r["parent"] == "accounting" for r in rows2) + assert any(r["parent"] == "hr" for r in rows2) + # For table-scoped candidates, the parent-level allow does not override root deny unless you have child-level rules + assert all(r["allow"] in (0, 1) for r in rows2) + + +@pytest.mark.asyncio +async def test_action_specific_rules(db): + await seed_catalog(db) + plugins = [plugin_allow_all_for_action("dana", VIEW_TABLE)] + + view_rows = await resolve_permissions_from_catalog( + db, + "dana", + plugins, + VIEW_TABLE, + TABLE_CANDIDATES_SQL, + implicit_deny=True, + ) + assert view_rows and all(r["allow"] == 1 for r in view_rows) + assert all(r["action"] == VIEW_TABLE for r in view_rows) + + insert_rows = await resolve_permissions_from_catalog( + db, + "dana", + plugins, + "insert-row", + TABLE_CANDIDATES_SQL, + implicit_deny=True, + ) + assert insert_rows and all(r["allow"] == 0 for r in insert_rows) + assert all(r["reason"] == "implicit deny" for r in insert_rows) + assert all(r["action"] == "insert-row" for r in insert_rows) From e2a739c4965b520e994aeabebcc9a83d3079d94b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 8 Oct 2025 20:32:16 -0700 Subject: [PATCH 640/823] Fix for asyncio.iscoroutinefunction deprecation warnings Closes #2512 Refs https://github.com/simonw/asyncinject/issues/18 --- datasette/utils/check_callable.py | 6 +++--- setup.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/datasette/utils/check_callable.py b/datasette/utils/check_callable.py index 5b8a30ac..a0997d20 100644 --- a/datasette/utils/check_callable.py +++ b/datasette/utils/check_callable.py @@ -1,4 +1,4 @@ -import asyncio +import inspect import types from typing import NamedTuple, Any @@ -17,9 +17,9 @@ def check_callable(obj: Any) -> CallableStatus: return CallableStatus(True, False) if isinstance(obj, types.FunctionType): - return CallableStatus(True, asyncio.iscoroutinefunction(obj)) + return CallableStatus(True, inspect.iscoroutinefunction(obj)) if hasattr(obj, "__call__"): - return CallableStatus(True, asyncio.iscoroutinefunction(obj.__call__)) + return CallableStatus(True, inspect.iscoroutinefunction(obj.__call__)) assert False, "obj {} is somehow callable with no __call__ method".format(repr(obj)) diff --git a/setup.py b/setup.py index 214ce36e..fa5be8e5 100644 --- a/setup.py +++ b/setup.py @@ -58,7 +58,7 @@ setup( "mergedeep>=1.1.1", "itsdangerous>=1.1", "sqlite-utils>=3.30", - "asyncinject>=0.5", + "asyncinject>=0.6.1", "setuptools", "pip", ], From 659673614a917f298748020ed8efafc162d84985 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 8 Oct 2025 21:53:34 -0700 Subject: [PATCH 641/823] Refactor debug templates to use shared JavaScript functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracted common JavaScript utilities from debug_allowed.html, debug_check.html, and debug_rules.html into a new _debug_common_functions.html include template. This eliminates code duplication and improves maintainability. The shared functions include: - populateFormFromURL(): Populates form fields from URL query parameters - updateURL(formId, page): Updates browser URL with form values - escapeHtml(text): HTML escaping utility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../templates/_debug_common_functions.html | 70 ++++++++++++++++ datasette/templates/debug_allowed.html | 81 +++++-------------- datasette/templates/debug_check.html | 57 ++++--------- datasette/templates/debug_rules.html | 70 +++++----------- 4 files changed, 120 insertions(+), 158 deletions(-) create mode 100644 datasette/templates/_debug_common_functions.html diff --git a/datasette/templates/_debug_common_functions.html b/datasette/templates/_debug_common_functions.html new file mode 100644 index 00000000..6dd5a9d9 --- /dev/null +++ b/datasette/templates/_debug_common_functions.html @@ -0,0 +1,70 @@ + diff --git a/datasette/templates/debug_allowed.html b/datasette/templates/debug_allowed.html index 5f22b6a4..031ff07d 100644 --- a/datasette/templates/debug_allowed.html +++ b/datasette/templates/debug_allowed.html @@ -5,6 +5,7 @@ {% block extra_head %} {% include "_permission_ui_styles.html" %} +{% include "_debug_common_functions.html" %} {% endblock %} {% block content %} @@ -81,70 +82,31 @@ const pagination = document.getElementById('pagination'); const submitBtn = document.getElementById('submit-btn'); let currentData = null; -// Populate form from URL parameters on page load -function populateFormFromURL() { - const params = new URLSearchParams(window.location.search); - - const action = params.get('action'); - if (action) { - document.getElementById('action').value = action; - } - - const parent = params.get('parent'); - if (parent) { - document.getElementById('parent').value = parent; - } - - const child = params.get('child'); - if (child) { - document.getElementById('child').value = child; - } - - const pageSize = params.get('page_size'); - if (pageSize) { - document.getElementById('page_size').value = pageSize; - } - - const page = params.get('page'); - - // If parameters are present, automatically fetch results - if (action) { - fetchResults(page ? parseInt(page) : 1, false); - } -} - -// Update URL with current form values and page -function updateURL(page = 1) { - const formData = new FormData(form); - const params = new URLSearchParams(); - - for (const [key, value] of formData.entries()) { - if (value) { - params.append(key, value); - } - } - - if (page > 1) { - params.set('page', page.toString()); - } - - const newURL = window.location.pathname + (params.toString() ? '?' + params.toString() : ''); - window.history.pushState({}, '', newURL); -} - form.addEventListener('submit', async (ev) => { ev.preventDefault(); - updateURL(1); + updateURL('allowed-form', 1); await fetchResults(1, false); }); // Handle browser back/forward window.addEventListener('popstate', () => { - populateFormFromURL(); + const params = populateFormFromURL(); + const action = params.get('action'); + const page = params.get('page'); + if (action) { + fetchResults(page ? parseInt(page) : 1, false); + } }); // Populate form on initial load -populateFormFromURL(); +(function() { + const params = populateFormFromURL(); + const action = params.get('action'); + const page = params.get('page'); + if (action) { + fetchResults(page ? parseInt(page) : 1, false); + } +})(); async function fetchResults(page = 1, updateHistory = true) { submitBtn.disabled = true; @@ -230,7 +192,7 @@ function displayResults(data) { prevLink.textContent = '← Previous'; prevLink.addEventListener('click', (e) => { e.preventDefault(); - updateURL(data.page - 1); + updateURL('allowed-form', data.page - 1); fetchResults(data.page - 1, false); }); pagination.appendChild(prevLink); @@ -246,7 +208,7 @@ function displayResults(data) { nextLink.textContent = 'Next →'; nextLink.addEventListener('click', (e) => { e.preventDefault(); - updateURL(data.page + 1); + updateURL('allowed-form', data.page + 1); fetchResults(data.page + 1, false); }); pagination.appendChild(nextLink); @@ -272,13 +234,6 @@ function displayError(data) { resultsContainer.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); } -function escapeHtml(text) { - if (text === null || text === undefined) return ''; - const div = document.createElement('div'); - div.textContent = text; - return div.innerHTML; -} - // Disable child input if parent is empty const parentInput = document.getElementById('parent'); const childInput = document.getElementById('child'); diff --git a/datasette/templates/debug_check.html b/datasette/templates/debug_check.html index b8bbd0a6..2e077327 100644 --- a/datasette/templates/debug_check.html +++ b/datasette/templates/debug_check.html @@ -4,6 +4,7 @@ {% block extra_head %} +{% include "_debug_common_functions.html" %} + + +
+
+ +
+
+
+ Navigate + Enter Select + Esc Close +
+
+
+ `; + } + + setupEventListeners() { + const dialog = this.shadowRoot.querySelector('dialog'); + const input = this.shadowRoot.querySelector('.search-input'); + const resultsContainer = this.shadowRoot.querySelector('.results-container'); + + // Global keyboard listener for "/" + document.addEventListener('keydown', (e) => { + if (e.key === '/' && !this.isInputFocused() && !dialog.open) { + e.preventDefault(); + this.openMenu(); + } + }); + + // Input event + input.addEventListener('input', (e) => { + this.handleSearch(e.target.value); + }); + + // Keyboard navigation + input.addEventListener('keydown', (e) => { + if (e.key === 'ArrowDown') { + e.preventDefault(); + this.moveSelection(1); + } else if (e.key === 'ArrowUp') { + e.preventDefault(); + this.moveSelection(-1); + } else if (e.key === 'Enter') { + e.preventDefault(); + this.selectCurrentItem(); + } else if (e.key === 'Escape') { + this.closeMenu(); + } + }); + + // Click on result item + resultsContainer.addEventListener('click', (e) => { + const item = e.target.closest('.result-item'); + if (item) { + const index = parseInt(item.dataset.index); + this.selectItem(index); + } + }); + + // Close on backdrop click + dialog.addEventListener('click', (e) => { + if (e.target === dialog) { + this.closeMenu(); + } + }); + + // Initial load + this.loadInitialData(); + } + + isInputFocused() { + const activeElement = document.activeElement; + return activeElement && ( + activeElement.tagName === 'INPUT' || + activeElement.tagName === 'TEXTAREA' || + activeElement.isContentEditable + ); + } + + loadInitialData() { + const itemsAttr = this.getAttribute('items'); + if (itemsAttr) { + try { + this.allItems = JSON.parse(itemsAttr); + this.matches = this.allItems; + } catch (e) { + console.error('Failed to parse items attribute:', e); + this.allItems = []; + this.matches = []; + } + } + } + + handleSearch(query) { + clearTimeout(this.debounceTimer); + + this.debounceTimer = setTimeout(() => { + const url = this.getAttribute('url'); + + if (url) { + // Fetch from API + this.fetchResults(url, query); + } else { + // Filter local items + this.filterLocalItems(query); + } + }, 200); + } + + async fetchResults(url, query) { + try { + const searchUrl = `${url}?q=${encodeURIComponent(query)}`; + const response = await fetch(searchUrl); + const data = await response.json(); + this.matches = data.matches || []; + this.selectedIndex = this.matches.length > 0 ? 0 : -1; + this.renderResults(); + } catch (e) { + console.error('Failed to fetch search results:', e); + this.matches = []; + this.renderResults(); + } + } + + filterLocalItems(query) { + if (!query.trim()) { + this.matches = []; + } else { + const lowerQuery = query.toLowerCase(); + this.matches = (this.allItems || []).filter(item => + item.name.toLowerCase().includes(lowerQuery) || + item.url.toLowerCase().includes(lowerQuery) + ); + } + this.selectedIndex = this.matches.length > 0 ? 0 : -1; + this.renderResults(); + } + + renderResults() { + const container = this.shadowRoot.querySelector('.results-container'); + const input = this.shadowRoot.querySelector('.search-input'); + + if (this.matches.length === 0) { + const message = input.value.trim() ? 'No results found' : 'Start typing to search...'; + container.innerHTML = `
${message}
`; + return; + } + + container.innerHTML = this.matches.map((match, index) => ` +
+
+
${this.escapeHtml(match.name)}
+
${this.escapeHtml(match.url)}
+
+
+ `).join(''); + + // Scroll selected item into view + if (this.selectedIndex >= 0) { + const selectedItem = container.children[this.selectedIndex]; + if (selectedItem) { + selectedItem.scrollIntoView({ block: 'nearest' }); + } + } + } + + moveSelection(direction) { + const newIndex = this.selectedIndex + direction; + if (newIndex >= 0 && newIndex < this.matches.length) { + this.selectedIndex = newIndex; + this.renderResults(); + } + } + + selectCurrentItem() { + if (this.selectedIndex >= 0 && this.selectedIndex < this.matches.length) { + this.selectItem(this.selectedIndex); + } + } + + selectItem(index) { + const match = this.matches[index]; + if (match) { + // Dispatch custom event + this.dispatchEvent(new CustomEvent('select', { + detail: match, + bubbles: true, + composed: true + })); + + // Navigate to URL + window.location.href = match.url; + + this.closeMenu(); + } + } + + openMenu() { + const dialog = this.shadowRoot.querySelector('dialog'); + const input = this.shadowRoot.querySelector('.search-input'); + + dialog.showModal(); + input.value = ''; + input.focus(); + + // Reset state - start with no items shown + this.matches = []; + this.selectedIndex = -1; + this.renderResults(); + } + + closeMenu() { + const dialog = this.shadowRoot.querySelector('dialog'); + dialog.close(); + } + + escapeHtml(text) { + const div = document.createElement('div'); + div.textContent = text; + return div.innerHTML; + } +} + +// Register the custom element +customElements.define('navigation-search', NavigationSearch); \ No newline at end of file diff --git a/datasette/templates/base.html b/datasette/templates/base.html index 0b2def5a..0d89e11c 100644 --- a/datasette/templates/base.html +++ b/datasette/templates/base.html @@ -72,5 +72,7 @@ {% endfor %} {% if select_templates %}{% endif %} + + diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py new file mode 100644 index 00000000..4dda404b --- /dev/null +++ b/datasette/utils/actions_sql.py @@ -0,0 +1,275 @@ +""" +SQL query builder for hierarchical permission checking. + +This module implements a cascading permission system based on the pattern +from the sqlite-permissions-poc. It builds SQL queries that: + +1. Start with all resources of a given type (from resource_type.resources_sql()) +2. Gather permission rules from plugins (via permission_resources_sql hook) +3. Apply cascading logic: child → parent → global +4. Apply DENY-beats-ALLOW at each level + +The core pattern is: +- Resources are identified by (parent, child) tuples +- Rules are evaluated at three levels: + - child: exact match on (parent, child) + - parent: match on (parent, NULL) + - global: match on (NULL, NULL) +- At the same level, DENY (allow=0) beats ALLOW (allow=1) +- Across levels, child beats parent beats global +""" + +from typing import Optional +from datasette.plugins import pm +from datasette.utils import await_me_maybe +from datasette.utils.permissions import PluginSQL + + +async def build_allowed_resources_sql( + datasette, + actor: dict | None, + action: str, +) -> tuple[str, dict]: + """ + Build a SQL query that returns all resources the actor can access for this action. + + Args: + datasette: The Datasette instance + actor: The actor dict (or None for unauthenticated) + action: The action name (e.g., "view-table", "view-database") + + Returns: + A tuple of (sql_query, params_dict) + + The returned SQL query will have three columns: + - parent: The parent resource identifier (or NULL) + - child: The child resource identifier (or NULL) + - reason: The reason from the rule that granted access + + Example: + For action="view-table", this might return: + SELECT parent, child, reason FROM ... WHERE is_allowed = 1 + + Results would be like: + ('analytics', 'users', 'role-based: analysts can access analytics DB') + ('analytics', 'events', 'role-based: analysts can access analytics DB') + ('production', 'orders', 'business-exception: allow production.orders for carol') + """ + # Get the Action object + action_obj = datasette.actions.get(action) + if not action_obj: + raise ValueError(f"Unknown action: {action}") + + # Get base resources SQL from the resource class + base_resources_sql = action_obj.resource_class.resources_sql() + + # Get all permission rule fragments from plugins via the hook + rule_results = pm.hook.permission_resources_sql( + datasette=datasette, + actor=actor, + action=action, + ) + + # Combine rule fragments and collect parameters + all_params = {} + rule_sqls = [] + + for result in rule_results: + result = await await_me_maybe(result) + if result is None: + continue + if isinstance(result, list): + for plugin_sql in result: + if isinstance(plugin_sql, PluginSQL): + rule_sqls.append(plugin_sql.sql) + all_params.update(plugin_sql.params) + elif isinstance(result, PluginSQL): + rule_sqls.append(result.sql) + all_params.update(result.params) + + # If no rules, return empty result (deny all) + if not rule_sqls: + return "SELECT NULL AS parent, NULL AS child WHERE 0", {} + + # Build the cascading permission query + rules_union = " UNION ALL ".join(rule_sqls) + + query = f""" +WITH +base AS ( + {base_resources_sql} +), +all_rules AS ( + {rules_union} +), +child_lvl AS ( + SELECT b.parent, b.child, + MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, + MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow, + MAX(CASE WHEN ar.allow = 0 THEN ar.reason ELSE NULL END) AS deny_reason, + MAX(CASE WHEN ar.allow = 1 THEN ar.reason ELSE NULL END) AS allow_reason + FROM base b + LEFT JOIN all_rules ar ON ar.parent = b.parent AND ar.child = b.child + GROUP BY b.parent, b.child +), +parent_lvl AS ( + SELECT b.parent, b.child, + MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, + MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow, + MAX(CASE WHEN ar.allow = 0 THEN ar.reason ELSE NULL END) AS deny_reason, + MAX(CASE WHEN ar.allow = 1 THEN ar.reason ELSE NULL END) AS allow_reason + FROM base b + LEFT JOIN all_rules ar ON ar.parent = b.parent AND ar.child IS NULL + GROUP BY b.parent, b.child +), +global_lvl AS ( + SELECT b.parent, b.child, + MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, + MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow, + MAX(CASE WHEN ar.allow = 0 THEN ar.reason ELSE NULL END) AS deny_reason, + MAX(CASE WHEN ar.allow = 1 THEN ar.reason ELSE NULL END) AS allow_reason + FROM base b + LEFT JOIN all_rules ar ON ar.parent IS NULL AND ar.child IS NULL + GROUP BY b.parent, b.child +), +decisions AS ( + SELECT + b.parent, b.child, + CASE + WHEN cl.any_deny = 1 THEN 0 + WHEN cl.any_allow = 1 THEN 1 + WHEN pl.any_deny = 1 THEN 0 + WHEN pl.any_allow = 1 THEN 1 + WHEN gl.any_deny = 1 THEN 0 + WHEN gl.any_allow = 1 THEN 1 + ELSE 0 + END AS is_allowed, + CASE + WHEN cl.any_deny = 1 THEN cl.deny_reason + WHEN cl.any_allow = 1 THEN cl.allow_reason + WHEN pl.any_deny = 1 THEN pl.deny_reason + WHEN pl.any_allow = 1 THEN pl.allow_reason + WHEN gl.any_deny = 1 THEN gl.deny_reason + WHEN gl.any_allow = 1 THEN gl.allow_reason + ELSE 'default deny' + END AS reason + FROM base b + JOIN child_lvl cl USING (parent, child) + JOIN parent_lvl pl USING (parent, child) + JOIN global_lvl gl USING (parent, child) +) +SELECT parent, child, reason +FROM decisions +WHERE is_allowed = 1 +ORDER BY parent, child +""" + return query.strip(), all_params + + +async def check_permission_for_resource( + datasette, + actor: dict | None, + action: str, + parent: Optional[str], + child: Optional[str], +) -> bool: + """ + Check if an actor has permission for a specific action on a specific resource. + + Args: + datasette: The Datasette instance + actor: The actor dict (or None) + action: The action name + parent: The parent resource identifier (e.g., database name, or None) + child: The child resource identifier (e.g., table name, or None) + + Returns: + True if the actor is allowed, False otherwise + + This builds the cascading permission query and checks if the specific + resource is in the allowed set. + """ + # Get the Action object + action_obj = datasette.actions.get(action) + if not action_obj: + raise ValueError(f"Unknown action: {action}") + + # Get all permission rule fragments from plugins via the hook + rule_results = pm.hook.permission_resources_sql( + datasette=datasette, + actor=actor, + action=action, + ) + + # Combine rule fragments and collect parameters + all_params = {} + rule_sqls = [] + + for result in rule_results: + result = await await_me_maybe(result) + if result is None: + continue + if isinstance(result, list): + for plugin_sql in result: + if isinstance(plugin_sql, PluginSQL): + rule_sqls.append(plugin_sql.sql) + all_params.update(plugin_sql.params) + elif isinstance(result, PluginSQL): + rule_sqls.append(result.sql) + all_params.update(result.params) + + # If no rules, default deny + if not rule_sqls: + return False + + # Build a simplified query that just checks for this one resource + rules_union = " UNION ALL ".join(rule_sqls) + + # Add parameters for the resource we're checking + all_params["_check_parent"] = parent + all_params["_check_child"] = child + + query = f""" +WITH +all_rules AS ( + {rules_union} +), +child_lvl AS ( + SELECT + MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, + MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow + FROM all_rules ar + WHERE ar.parent = :_check_parent AND ar.child = :_check_child +), +parent_lvl AS ( + SELECT + MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, + MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow + FROM all_rules ar + WHERE ar.parent = :_check_parent AND ar.child IS NULL +), +global_lvl AS ( + SELECT + MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny, + MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow + FROM all_rules ar + WHERE ar.parent IS NULL AND ar.child IS NULL +) +SELECT + CASE + WHEN cl.any_deny = 1 THEN 0 + WHEN cl.any_allow = 1 THEN 1 + WHEN pl.any_deny = 1 THEN 0 + WHEN pl.any_allow = 1 THEN 1 + WHEN gl.any_deny = 1 THEN 0 + WHEN gl.any_allow = 1 THEN 1 + ELSE 0 + END AS is_allowed +FROM child_lvl cl, parent_lvl pl, global_lvl gl +""" + + # Execute the query against the internal database + result = await datasette.get_internal_database().execute(query, all_params) + if result.rows: + return bool(result.rows[0][0]) + return False diff --git a/datasette/views/special.py b/datasette/views/special.py index 7e5ce517..2c5004d0 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -923,3 +923,48 @@ class ApiExplorerView(BaseView): "private": private, }, ) + + +class TablesView(BaseView): + """ + Simple endpoint that uses the new allowed_resources() API. + Returns JSON list of all tables the actor can view. + + Supports ?q=foo+bar to filter tables matching .*foo.*bar.* pattern, + ordered by shortest name first. + """ + + name = "tables" + has_json_alternate = False + + async def get(self, request): + # Use the new allowed_resources() method + tables = await self.ds.allowed_resources("view-table", request.actor) + + # Convert to list of matches with name and url + matches = [ + { + "name": f"{table.parent}/{table.child}", + "url": self.ds.urls.table(table.parent, table.child), + } + for table in tables + ] + + # Apply search filter if q parameter is present + q = request.args.get("q", "").strip() + if q: + import re + + # Split search terms by whitespace + terms = q.split() + # Build regex pattern: .*term1.*term2.*term3.* + pattern = ".*" + ".*".join(re.escape(term) for term in terms) + ".*" + regex = re.compile(pattern, re.IGNORECASE) + + # Filter tables matching the pattern (extract table name from "db/table") + matches = [m for m in matches if regex.match(m["name"].split("/", 1)[1])] + + # Sort by shortest table name first + matches.sort(key=lambda m: len(m["name"].split("/", 1)[1])) + + return Response.json({"matches": matches}) diff --git a/docs/introspection.rst b/docs/introspection.rst index ff78ec78..19c6bffb 100644 --- a/docs/introspection.rst +++ b/docs/introspection.rst @@ -144,6 +144,47 @@ Shows currently attached databases. `Databases example `_: + +.. code-block:: json + + { + "matches": [ + { + "name": "fixtures/facetable", + "url": "/fixtures/facetable" + }, + { + "name": "fixtures/searchable", + "url": "/fixtures/searchable" + } + ] + } + +Search example with ``?q=facet`` returns only tables matching ``.*facet.*``: + +.. code-block:: json + + { + "matches": [ + { + "name": "fixtures/facetable", + "url": "/fixtures/facetable" + } + ] + } + +When multiple search terms are provided (e.g., ``?q=user+profile``), tables must match the pattern ``.*user.*profile.*``. Results are ordered by shortest table name first. + .. _JsonDataView_threads: /-/threads diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 244f448d..66c78f7e 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -782,6 +782,9 @@ The plugin hook can then be used to register the new facet class like this: register_permissions(datasette) -------------------------------- +.. note:: + This hook is deprecated. Use :ref:`plugin_register_actions` instead, which provides a more flexible resource-based permission system. + If your plugin needs to register additional permissions unique to that plugin - ``upload-csvs`` for example - you can return a list of those permissions from this hook. .. code-block:: python @@ -824,6 +827,141 @@ The fields of the ``Permission`` class are as follows: This should only be ``True`` if you want anonymous users to be able to take this action. +.. _plugin_register_actions: + +register_actions(datasette) +---------------------------- + +If your plugin needs to register actions that can be checked with Datasette's new resource-based permission system, return a list of those actions from this hook. + +Actions define what operations can be performed on resources (like viewing a table, executing SQL, or custom plugin actions). + +.. code-block:: python + + from datasette import hookimpl + from datasette.permissions import Action, Resource + + + class DocumentCollectionResource(Resource): + """A collection of documents.""" + + name = "document-collection" + parent_name = None + + def __init__(self, collection: str): + super().__init__(parent=collection, child=None) + + @classmethod + def resources_sql(cls) -> str: + return """ + SELECT collection_name AS parent, NULL AS child + FROM document_collections + """ + + + class DocumentResource(Resource): + """A document in a collection.""" + + name = "document" + parent_name = "document-collection" + + def __init__(self, collection: str, document: str): + super().__init__(parent=collection, child=document) + + @classmethod + def resources_sql(cls) -> str: + return """ + SELECT collection_name AS parent, document_id AS child + FROM documents + """ + + + @hookimpl + def register_actions(datasette): + return [ + Action( + name="list-documents", + abbr="ld", + description="List documents in a collection", + takes_parent=True, + takes_child=False, + resource_class=DocumentCollectionResource, + ), + Action( + name="view-document", + abbr="vdoc", + description="View document", + takes_parent=True, + takes_child=True, + resource_class=DocumentResource, + ), + Action( + name="edit-document", + abbr="edoc", + description="Edit document", + takes_parent=True, + takes_child=True, + resource_class=DocumentResource, + ), + ] + +The fields of the ``Action`` dataclass are as follows: + +``name`` - string + The name of the action, e.g. ``view-document``. This should be unique across all plugins. + +``abbr`` - string or None + An abbreviation of the action, e.g. ``vdoc``. This is optional. Since this needs to be unique across all installed plugins it's best to choose carefully or use ``None``. + +``description`` - string or None + A human-readable description of what the action allows you to do. + +``takes_parent`` - boolean + ``True`` if this action requires a parent identifier (like a database name). + +``takes_child`` - boolean + ``True`` if this action requires a child identifier (like a table or document name). + +``resource_class`` - type[Resource] + The Resource subclass that defines what kind of resource this action applies to. Your Resource subclass must: + + - Define a ``name`` class attribute (e.g., ``"document"``) + - Optionally define a ``parent_name`` class attribute (e.g., ``"collection"``) + - Implement a ``resources_sql()`` classmethod that returns SQL returning all resources as ``(parent, child)`` columns + - Have an ``__init__`` method that accepts appropriate parameters and calls ``super().__init__(parent=..., child=...)`` + +The ``resources_sql()`` method +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``resources_sql()`` classmethod is crucial to Datasette's permission system. It returns a SQL query that lists all resources of that type that exist in the system. + +This SQL query is used by Datasette to efficiently check permissions across multiple resources at once. When a user requests a list of resources (like tables, documents, or other entities), Datasette uses this SQL to: + +1. Get all resources of this type from your data catalog +2. Combine it with permission rules from the ``permission_resources_sql`` hook +3. Use SQL joins and filtering to determine which resources the actor can access +4. Return only the permitted resources + +The SQL query **must** return exactly two columns: + +- ``parent`` - The parent identifier (e.g., database name, collection name), or ``NULL`` for top-level resources +- ``child`` - The child identifier (e.g., table name, document ID), or ``NULL`` for parent-only resources + +For example, if you're building a document management plugin with collections and documents stored in a ``documents`` table, your ``resources_sql()`` might look like: + +.. code-block:: python + + @classmethod + def resources_sql(cls) -> str: + return """ + SELECT collection_name AS parent, document_id AS child + FROM documents + """ + +This tells Datasette "here's how to find all documents in the system - look in the documents table and get the collection name and document ID for each one." + +The permission system then uses this query along with rules from plugins to determine which documents each user can access, all efficiently in SQL rather than loading everything into Python. + .. _plugin_asgi_wrapper: asgi_wrapper(datasette) diff --git a/tests/test_actions_sql.py b/tests/test_actions_sql.py new file mode 100644 index 00000000..8fc8803d --- /dev/null +++ b/tests/test_actions_sql.py @@ -0,0 +1,317 @@ +""" +Tests for the new Resource-based permission system. + +These tests verify: +1. The new Datasette.allowed_resources() method +2. The new Datasette.allowed() method +3. The new Datasette.allowed_resources_with_reasons() method +4. That SQL does the heavy lifting (no Python filtering) +""" + +import pytest +import pytest_asyncio +from datasette.app import Datasette +from datasette.plugins import pm +from datasette.utils.permissions import PluginSQL +from datasette.default_actions import TableResource +from datasette import hookimpl + + +# Test plugin that provides permission rules +class PermissionRulesPlugin: + def __init__(self, rules_callback): + self.rules_callback = rules_callback + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + """Return permission rules based on the callback""" + return self.rules_callback(datasette, actor, action) + + +@pytest_asyncio.fixture +async def test_ds(): + """Create a test Datasette instance with sample data""" + ds = Datasette() + await ds.invoke_startup() + + # Add test databases with some tables + db = ds.add_memory_database("analytics") + await db.execute_write("CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY)") + await db.execute_write("CREATE TABLE IF NOT EXISTS events (id INTEGER PRIMARY KEY)") + await db.execute_write( + "CREATE TABLE IF NOT EXISTS sensitive (id INTEGER PRIMARY KEY)" + ) + + db2 = ds.add_memory_database("production") + await db2.execute_write( + "CREATE TABLE IF NOT EXISTS customers (id INTEGER PRIMARY KEY)" + ) + await db2.execute_write( + "CREATE TABLE IF NOT EXISTS orders (id INTEGER PRIMARY KEY)" + ) + + # Refresh schemas to populate catalog_tables in internal database + await ds._refresh_schemas() + + return ds + + +@pytest.mark.asyncio +async def test_allowed_resources_global_allow(test_ds): + """Test allowed_resources() with a global allow rule""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("id") == "alice": + sql = "SELECT NULL AS parent, NULL AS child, 1 AS allow, 'global: alice has access' AS reason" + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + # Use the new allowed_resources() method + tables = await test_ds.allowed_resources("view-table", {"id": "alice"}) + + # Alice should see all tables + assert len(tables) == 5 + assert all(isinstance(t, TableResource) for t in tables) + + # Check specific tables are present + table_set = set((t.parent, t.child) for t in tables) + assert ("analytics", "events") in table_set + assert ("analytics", "users") in table_set + assert ("analytics", "sensitive") in table_set + assert ("production", "customers") in table_set + assert ("production", "orders") in table_set + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_allowed_specific_resource(test_ds): + """Test allowed() method checks specific resource efficiently""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("role") == "analyst": + # Allow analytics database, deny everything else (global deny) + sql = """ + SELECT NULL AS parent, NULL AS child, 0 AS allow, 'global deny' AS reason + UNION ALL + SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, 'analyst access' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + actor = {"id": "bob", "role": "analyst"} + + # Check specific resources using allowed() + # This should use SQL WHERE clause, not fetch all resources + assert await test_ds.allowed( + "view-table", TableResource("analytics", "users"), actor + ) + assert await test_ds.allowed( + "view-table", TableResource("analytics", "events"), actor + ) + assert not await test_ds.allowed( + "view-table", TableResource("production", "orders"), actor + ) + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_allowed_resources_with_reasons(test_ds): + """Test allowed_resources_with_reasons() exposes debugging info""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("role") == "analyst": + sql = """ + SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, + 'parent: analyst access to analytics' AS reason + UNION ALL + SELECT 'analytics' AS parent, 'sensitive' AS child, 0 AS allow, + 'child: sensitive data denied' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + # Use allowed_resources_with_reasons to get debugging info + allowed = await test_ds.allowed_resources_with_reasons( + "view-table", {"id": "bob", "role": "analyst"} + ) + + # Should get analytics tables except sensitive + assert len(allowed) >= 2 # At least users and events + + # Check we can access both resource and reason + for item in allowed: + assert isinstance(item.resource, TableResource) + assert isinstance(item.reason, str) + if item.resource.parent == "analytics": + # Should mention parent-level reason + assert "analyst access" in item.reason.lower() + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_child_deny_overrides_parent_allow(test_ds): + """Test that child-level DENY beats parent-level ALLOW""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("role") == "analyst": + sql = """ + SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, + 'parent: allow analytics' AS reason + UNION ALL + SELECT 'analytics' AS parent, 'sensitive' AS child, 0 AS allow, + 'child: deny sensitive' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + actor = {"id": "bob", "role": "analyst"} + tables = await test_ds.allowed_resources("view-table", actor) + + # Should see analytics tables except sensitive + analytics_tables = [t for t in tables if t.parent == "analytics"] + assert len(analytics_tables) >= 2 + + table_names = {t.child for t in analytics_tables} + assert "users" in table_names + assert "events" in table_names + assert "sensitive" not in table_names + + # Verify with allowed() method + assert await test_ds.allowed( + "view-table", TableResource("analytics", "users"), actor + ) + assert not await test_ds.allowed( + "view-table", TableResource("analytics", "sensitive"), actor + ) + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_child_allow_overrides_parent_deny(test_ds): + """Test that child-level ALLOW beats parent-level DENY""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("id") == "carol": + sql = """ + SELECT 'production' AS parent, NULL AS child, 0 AS allow, + 'parent: deny production' AS reason + UNION ALL + SELECT 'production' AS parent, 'orders' AS child, 1 AS allow, + 'child: carol can see orders' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + actor = {"id": "carol"} + tables = await test_ds.allowed_resources("view-table", actor) + + # Should only see production.orders + production_tables = [t for t in tables if t.parent == "production"] + assert len(production_tables) == 1 + assert production_tables[0].child == "orders" + + # Verify with allowed() method + assert await test_ds.allowed( + "view-table", TableResource("production", "orders"), actor + ) + assert not await test_ds.allowed( + "view-table", TableResource("production", "customers"), actor + ) + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_resource_equality_and_hashing(test_ds): + """Test that Resource instances support equality and hashing""" + + # Create some resources + r1 = TableResource("analytics", "users") + r2 = TableResource("analytics", "users") + r3 = TableResource("analytics", "events") + + # Test equality + assert r1 == r2 + assert r1 != r3 + + # Test they can be used in sets + resource_set = {r1, r2, r3} + assert len(resource_set) == 2 # r1 and r2 are the same + + # Test they can be used as dict keys + resource_dict = {r1: "data1", r3: "data2"} + assert resource_dict[r2] == "data1" # r2 same as r1 + + +@pytest.mark.asyncio +async def test_sql_does_filtering_not_python(test_ds): + """ + Verify that allowed() uses SQL WHERE clause, not Python filtering. + + This test doesn't actually verify the SQL itself (that would require + query introspection), but it demonstrates the API contract. + """ + + def rules_callback(datasette, actor, action): + # Deny everything by default, allow only analytics.users specifically + sql = """ + SELECT NULL AS parent, NULL AS child, 0 AS allow, + 'global deny' AS reason + UNION ALL + SELECT 'analytics' AS parent, 'users' AS child, 1 AS allow, + 'specific allow' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + actor = {"id": "dave"} + + # allowed() should execute a targeted SQL query + # NOT fetch all resources and filter in Python + assert await test_ds.allowed( + "view-table", TableResource("analytics", "users"), actor + ) + assert not await test_ds.allowed( + "view-table", TableResource("analytics", "events"), actor + ) + + # allowed_resources() should also use SQL filtering + tables = await test_ds.allowed_resources("view-table", actor) + assert len(tables) == 1 + assert tables[0].parent == "analytics" + assert tables[0].child == "users" + + finally: + pm.unregister(plugin, name="test_plugin") diff --git a/tests/test_tables_endpoint.py b/tests/test_tables_endpoint.py new file mode 100644 index 00000000..a3305406 --- /dev/null +++ b/tests/test_tables_endpoint.py @@ -0,0 +1,544 @@ +""" +Tests for the /-/tables endpoint. + +These tests verify that the new TablesView correctly uses the allowed_resources() API. +""" + +import pytest +import pytest_asyncio +from datasette.app import Datasette +from datasette.plugins import pm +from datasette.utils.permissions import PluginSQL +from datasette import hookimpl + + +# Test plugin that provides permission rules +class PermissionRulesPlugin: + def __init__(self, rules_callback): + self.rules_callback = rules_callback + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + return self.rules_callback(datasette, actor, action) + + +@pytest_asyncio.fixture(scope="function") +async def test_ds(): + """Create a test Datasette instance with sample data (fresh for each test)""" + ds = Datasette() + await ds.invoke_startup() + + # Add test databases with some tables + db = ds.add_memory_database("analytics") + await db.execute_write("CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY)") + await db.execute_write("CREATE TABLE IF NOT EXISTS events (id INTEGER PRIMARY KEY)") + await db.execute_write( + "CREATE TABLE IF NOT EXISTS sensitive (id INTEGER PRIMARY KEY)" + ) + + db2 = ds.add_memory_database("production") + await db2.execute_write( + "CREATE TABLE IF NOT EXISTS customers (id INTEGER PRIMARY KEY)" + ) + await db2.execute_write( + "CREATE TABLE IF NOT EXISTS orders (id INTEGER PRIMARY KEY)" + ) + + # Refresh schemas to populate catalog_tables in internal database + await ds._refresh_schemas() + + return ds + + +@pytest.mark.asyncio +async def test_tables_endpoint_global_access(test_ds): + """Test /-/tables with global access permissions""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("id") == "alice": + sql = "SELECT NULL AS parent, NULL AS child, 1 AS allow, 'global: alice has access' AS reason" + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + # Use the allowed_resources API directly + tables = await test_ds.allowed_resources("view-table", {"id": "alice"}) + + # Convert to the format the endpoint returns + result = [ + { + "name": f"{t.parent}/{t.child}", + "url": test_ds.urls.table(t.parent, t.child), + } + for t in tables + ] + + # Alice should see all tables + assert len(result) == 5 + table_names = {m["name"] for m in result} + assert "analytics/events" in table_names + assert "analytics/users" in table_names + assert "analytics/sensitive" in table_names + assert "production/customers" in table_names + assert "production/orders" in table_names + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_tables_endpoint_database_restriction(test_ds): + """Test /-/tables with database-level restriction""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("role") == "analyst": + # Allow only analytics database + sql = "SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, 'analyst access' AS reason" + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + tables = await test_ds.allowed_resources( + "view-table", {"id": "bob", "role": "analyst"} + ) + result = [ + { + "name": f"{t.parent}/{t.child}", + "url": test_ds.urls.table(t.parent, t.child), + } + for t in tables + ] + + # Bob should only see analytics tables + analytics_tables = [m for m in result if m["name"].startswith("analytics/")] + production_tables = [m for m in result if m["name"].startswith("production/")] + + assert len(analytics_tables) == 3 + table_names = {m["name"] for m in analytics_tables} + assert "analytics/events" in table_names + assert "analytics/users" in table_names + assert "analytics/sensitive" in table_names + + # Should not see production tables (unless default_permissions allows them) + # Note: default_permissions.py provides default allows, so we just check analytics are present + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_tables_endpoint_table_exception(test_ds): + """Test /-/tables with table-level exception (deny database, allow specific table)""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("id") == "carol": + # Deny analytics database, but allow analytics.users specifically + sql = """ + SELECT 'analytics' AS parent, NULL AS child, 0 AS allow, 'deny analytics' AS reason + UNION ALL + SELECT 'analytics' AS parent, 'users' AS child, 1 AS allow, 'carol exception' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + tables = await test_ds.allowed_resources("view-table", {"id": "carol"}) + result = [ + { + "name": f"{t.parent}/{t.child}", + "url": test_ds.urls.table(t.parent, t.child), + } + for t in tables + ] + + # Carol should see analytics.users but not other analytics tables + analytics_tables = [m for m in result if m["name"].startswith("analytics/")] + assert len(analytics_tables) == 1 + table_names = {m["name"] for m in analytics_tables} + assert "analytics/users" in table_names + + # Should NOT see analytics.events or analytics.sensitive + assert "analytics/events" not in table_names + assert "analytics/sensitive" not in table_names + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_tables_endpoint_deny_overrides_allow(test_ds): + """Test that child-level DENY beats parent-level ALLOW""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("role") == "analyst": + # Allow analytics, but deny sensitive table + sql = """ + SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, 'allow analytics' AS reason + UNION ALL + SELECT 'analytics' AS parent, 'sensitive' AS child, 0 AS allow, 'deny sensitive' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + tables = await test_ds.allowed_resources( + "view-table", {"id": "bob", "role": "analyst"} + ) + result = [ + { + "name": f"{t.parent}/{t.child}", + "url": test_ds.urls.table(t.parent, t.child), + } + for t in tables + ] + + analytics_tables = [m for m in result if m["name"].startswith("analytics/")] + + # Should see users and events but NOT sensitive + table_names = {m["name"] for m in analytics_tables} + assert "analytics/users" in table_names + assert "analytics/events" in table_names + assert "analytics/sensitive" not in table_names + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_tables_endpoint_no_permissions(): + """Test /-/tables when user has no custom permissions (only defaults)""" + + ds = Datasette() + await ds.invoke_startup() + + # Add a single database + db = ds.add_memory_database("testdb") + await db.execute_write("CREATE TABLE items (id INTEGER PRIMARY KEY)") + await ds._refresh_schemas() + + # Unknown actor with no custom permissions + tables = await ds.allowed_resources("view-table", {"id": "unknown"}) + result = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in tables + ] + + # Should see tables (due to default_permissions.py providing default allow) + assert len(result) >= 1 + assert any(m["name"].endswith("/items") for m in result) + + +@pytest.mark.asyncio +async def test_tables_endpoint_specific_table_only(test_ds): + """Test /-/tables when only specific tables are allowed (no parent/global rules)""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("id") == "dave": + # Allow only specific tables, no parent-level or global rules + sql = """ + SELECT 'analytics' AS parent, 'users' AS child, 1 AS allow, 'specific table 1' AS reason + UNION ALL + SELECT 'production' AS parent, 'orders' AS child, 1 AS allow, 'specific table 2' AS reason + """ + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + tables = await test_ds.allowed_resources("view-table", {"id": "dave"}) + result = [ + { + "name": f"{t.parent}/{t.child}", + "url": test_ds.urls.table(t.parent, t.child), + } + for t in tables + ] + + # Should see only the two specifically allowed tables + specific_tables = [ + m for m in result if m["name"] in ("analytics/users", "production/orders") + ] + + assert len(specific_tables) == 2 + table_names = {m["name"] for m in specific_tables} + assert "analytics/users" in table_names + assert "production/orders" in table_names + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_tables_endpoint_empty_result(test_ds): + """Test /-/tables when all tables are explicitly denied""" + + def rules_callback(datasette, actor, action): + if actor and actor.get("id") == "blocked": + # Global deny + sql = "SELECT NULL AS parent, NULL AS child, 0 AS allow, 'global deny' AS reason" + return PluginSQL(source="test", sql=sql, params={}) + return None + + plugin = PermissionRulesPlugin(rules_callback) + pm.register(plugin, name="test_plugin") + + try: + tables = await test_ds.allowed_resources("view-table", {"id": "blocked"}) + result = [ + { + "name": f"{t.parent}/{t.child}", + "url": test_ds.urls.table(t.parent, t.child), + } + for t in tables + ] + + # Global deny should block access to all tables + assert len(result) == 0 + + finally: + pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_tables_endpoint_search_single_term(): + """Test /-/tables?q=user to filter tables matching 'user'""" + + ds = Datasette() + await ds.invoke_startup() + + # Add database with various table names + db = ds.add_memory_database("search_test") + await db.execute_write("CREATE TABLE users (id INTEGER)") + await db.execute_write("CREATE TABLE user_profiles (id INTEGER)") + await db.execute_write("CREATE TABLE events (id INTEGER)") + await db.execute_write("CREATE TABLE posts (id INTEGER)") + await ds._refresh_schemas() + + # Get all tables in the new format + all_tables = await ds.allowed_resources("view-table", None) + matches = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in all_tables + ] + + # Filter for "user" (extract table name from "db/table") + import re + + pattern = ".*user.*" + regex = re.compile(pattern, re.IGNORECASE) + filtered = [m for m in matches if regex.match(m["name"].split("/", 1)[1])] + + # Should match users and user_profiles but not events or posts + table_names = {m["name"].split("/", 1)[1] for m in filtered} + assert "users" in table_names + assert "user_profiles" in table_names + assert "events" not in table_names + assert "posts" not in table_names + + +@pytest.mark.asyncio +async def test_tables_endpoint_search_multiple_terms(): + """Test /-/tables?q=user+profile to filter tables matching .*user.*profile.*""" + + ds = Datasette() + await ds.invoke_startup() + + # Add database with various table names + db = ds.add_memory_database("search_test2") + await db.execute_write("CREATE TABLE user_profiles (id INTEGER)") + await db.execute_write("CREATE TABLE users (id INTEGER)") + await db.execute_write("CREATE TABLE profile_settings (id INTEGER)") + await db.execute_write("CREATE TABLE events (id INTEGER)") + await ds._refresh_schemas() + + # Get all tables in the new format + all_tables = await ds.allowed_resources("view-table", None) + matches = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in all_tables + ] + + # Filter for "user profile" (two terms, extract table name from "db/table") + import re + + terms = ["user", "profile"] + pattern = ".*" + ".*".join(re.escape(term) for term in terms) + ".*" + regex = re.compile(pattern, re.IGNORECASE) + filtered = [m for m in matches if regex.match(m["name"].split("/", 1)[1])] + + # Should match only user_profiles (has both user and profile in that order) + table_names = {m["name"].split("/", 1)[1] for m in filtered} + assert "user_profiles" in table_names + assert "users" not in table_names # doesn't have "profile" + assert "profile_settings" not in table_names # doesn't have "user" + + +@pytest.mark.asyncio +async def test_tables_endpoint_search_ordering(): + """Test that search results are ordered by shortest name first""" + + ds = Datasette() + await ds.invoke_startup() + + # Add database with tables of various lengths containing "user" + db = ds.add_memory_database("order_test") + await db.execute_write("CREATE TABLE users (id INTEGER)") + await db.execute_write("CREATE TABLE user_profiles (id INTEGER)") + await db.execute_write( + "CREATE TABLE u (id INTEGER)" + ) # Shortest, but doesn't match "user" + await db.execute_write( + "CREATE TABLE user_authentication_tokens (id INTEGER)" + ) # Longest + await db.execute_write("CREATE TABLE user_data (id INTEGER)") + await ds._refresh_schemas() + + # Get all tables in the new format + all_tables = await ds.allowed_resources("view-table", None) + matches = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in all_tables + ] + + # Filter for "user" and sort by table name length + import re + + pattern = ".*user.*" + regex = re.compile(pattern, re.IGNORECASE) + filtered = [m for m in matches if regex.match(m["name"].split("/", 1)[1])] + filtered.sort(key=lambda m: len(m["name"].split("/", 1)[1])) + + # Should be ordered: users, user_data, user_profiles, user_authentication_tokens + matching_names = [m["name"].split("/", 1)[1] for m in filtered] + assert matching_names[0] == "users" # shortest + assert len(matching_names[0]) < len(matching_names[1]) + assert len(matching_names[-1]) > len(matching_names[-2]) + assert matching_names[-1] == "user_authentication_tokens" # longest + + +@pytest.mark.asyncio +async def test_tables_endpoint_search_case_insensitive(): + """Test that search is case-insensitive""" + + ds = Datasette() + await ds.invoke_startup() + + # Add database with mixed case table names + db = ds.add_memory_database("case_test") + await db.execute_write("CREATE TABLE Users (id INTEGER)") + await db.execute_write("CREATE TABLE USER_PROFILES (id INTEGER)") + await db.execute_write("CREATE TABLE user_data (id INTEGER)") + await ds._refresh_schemas() + + # Get all tables in the new format + all_tables = await ds.allowed_resources("view-table", None) + matches = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in all_tables + ] + + # Filter for "user" (lowercase) should match all case variants + import re + + pattern = ".*user.*" + regex = re.compile(pattern, re.IGNORECASE) + filtered = [m for m in matches if regex.match(m["name"].split("/", 1)[1])] + + # Should match all three tables regardless of case + table_names = {m["name"].split("/", 1)[1] for m in filtered} + assert "Users" in table_names + assert "USER_PROFILES" in table_names + assert "user_data" in table_names + assert len(filtered) >= 3 + + +@pytest.mark.asyncio +async def test_tables_endpoint_search_no_matches(): + """Test search with no matching tables returns empty list""" + + ds = Datasette() + await ds.invoke_startup() + + # Add database with tables that won't match search + db = ds.add_memory_database("nomatch_test") + await db.execute_write("CREATE TABLE events (id INTEGER)") + await db.execute_write("CREATE TABLE posts (id INTEGER)") + await ds._refresh_schemas() + + # Get all tables in the new format + all_tables = await ds.allowed_resources("view-table", None) + matches = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in all_tables + ] + + # Filter for "zzz" which doesn't exist + import re + + pattern = ".*zzz.*" + regex = re.compile(pattern, re.IGNORECASE) + filtered = [m for m in matches if regex.match(m["name"].split("/", 1)[1])] + + # Should return empty list + assert len(filtered) == 0 + + +@pytest.mark.asyncio +async def test_tables_endpoint_config_database_allow(): + """Test that database-level allow blocks work for view-table action""" + + # Simulate: -s databases.fixtures.allow.id root + config = {"databases": {"fixtures": {"allow": {"id": "root"}}}} + + ds = Datasette(config=config) + await ds.invoke_startup() + + # Create databases + fixtures_db = ds.add_memory_database("fixtures") + await fixtures_db.execute_write("CREATE TABLE users (id INTEGER)") + await fixtures_db.execute_write("CREATE TABLE posts (id INTEGER)") + + content_db = ds.add_memory_database("content") + await content_db.execute_write("CREATE TABLE articles (id INTEGER)") + + await ds._refresh_schemas() + + # Root user should see fixtures tables + root_tables = await ds.allowed_resources("view-table", {"id": "root"}) + root_list = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in root_tables + ] + fixtures_tables_root = [m for m in root_list if m["name"].startswith("fixtures/")] + assert len(fixtures_tables_root) == 2 + table_names = {m["name"] for m in fixtures_tables_root} + assert "fixtures/users" in table_names + assert "fixtures/posts" in table_names + + # Alice should NOT see fixtures tables + alice_tables = await ds.allowed_resources("view-table", {"id": "alice"}) + alice_list = [ + {"name": f"{t.parent}/{t.child}", "url": ds.urls.table(t.parent, t.child)} + for t in alice_tables + ] + fixtures_tables_alice = [m for m in alice_list if m["name"].startswith("fixtures/")] + assert len(fixtures_tables_alice) == 0 + + # But Alice should see content tables (no restrictions) + content_tables_alice = [m for m in alice_list if m["name"].startswith("content/")] + assert len(content_tables_alice) == 1 + assert "content/articles" in {m["name"] for m in content_tables_alice} From 5b0baf7cd5ea99c6366052649f31e0a3a608d014 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 20 Oct 2025 16:03:22 -0700 Subject: [PATCH 647/823] Ran prettier --- datasette/static/navigation-search.js | 415 +++++++++++++------------- 1 file changed, 213 insertions(+), 202 deletions(-) diff --git a/datasette/static/navigation-search.js b/datasette/static/navigation-search.js index 202839d5..7204ab93 100644 --- a/datasette/static/navigation-search.js +++ b/datasette/static/navigation-search.js @@ -1,17 +1,17 @@ class NavigationSearch extends HTMLElement { - constructor() { - super(); - this.attachShadow({ mode: 'open' }); - this.selectedIndex = -1; - this.matches = []; - this.debounceTimer = null; - - this.render(); - this.setupEventListeners(); - } + constructor() { + super(); + this.attachShadow({ mode: "open" }); + this.selectedIndex = -1; + this.matches = []; + this.debounceTimer = null; - render() { - this.shadowRoot.innerHTML = ` + this.render(); + this.setupEventListeners(); + } + + render() { + this.shadowRoot.innerHTML = ` + + +{% 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 %} -

Registered Actions

+

Registered actions

+ +{% set current_tab = "actions" %} +{% include "_permissions_debug_tabs.html" %}

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 %} +

Allowed resources

-

Allowed Resources

+{% set current_tab = "allowed" %} +{% include "_permissions_debug_tabs.html" %}

Use this tool to check which resources the current actor is allowed to access for a given permission action. It queries the /-/allowed.json API endpoint.

@@ -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 = `
Error: ${escapeHtml(data.error || 'Unknown error')}
`; 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 %} +{% include "_permission_ui_styles.html" %} {% include "_debug_common_functions.html" %} {% endblock %} {% block content %} +

Permission check

-

Permission Check

+{% set current_tab = "check" %} +{% include "_permissions_debug_tabs.html" %}

Use this tool to test permission checks for the current actor. It queries the /-/check.json API endpoint.

@@ -105,32 +65,36 @@

Current actor: anonymous (not logged in)

{% endif %} -
-
- - - The permission action to check -
+
+ +
+ + + The permission action to check +
-
- - - For database-level permissions, specify the database name -
+
+ + + For database-level permissions, specify the database name +
-
- - - For table-level permissions, specify the table name (requires parent) -
+
+ + + For table-level permissions, specify the table name (requires parent) +
- - +
+ +
+ +