diff --git a/datasette/app.py b/datasette/app.py index 8b8b601f..81d23acb 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -55,6 +55,7 @@ from .views.stored_queries import ( QueryCreateAnalyzeView, QueryDeleteView, QueryDefinitionView, + QueryEditView, GlobalQueryListView, QueryListView, QueryParametersView, @@ -2493,6 +2494,10 @@ class Datasette: QueryDefinitionView.as_view(self), r"/(?P[^\/\.]+)/(?P[^\/\.]+)/-/definition$", ) + add_route( + QueryEditView.as_view(self), + r"/(?P[^\/\.]+)/(?P[^\/\.]+)/-/edit$", + ) add_route( QueryUpdateView.as_view(self), r"/(?P[^\/\.]+)/(?P[^\/\.]+)/-/update$", diff --git a/datasette/default_query_actions.py b/datasette/default_query_actions.py new file mode 100644 index 00000000..2183e70b --- /dev/null +++ b/datasette/default_query_actions.py @@ -0,0 +1,48 @@ +from datasette import hookimpl +from datasette.resources import QueryResource + + +@hookimpl +def query_actions(datasette, actor, database, query_name, request): + # Only stored queries (with a name) can be edited or deleted + if not query_name: + return None + + async def inner(): + query = await datasette.get_query(database, query_name) + if query is None: + return [] + # Config-defined and trusted queries are managed outside the UI + if query.source == "config" or query.is_trusted: + return [] + + links = [] + if await datasette.allowed( + action="update-query", + resource=QueryResource(database, query_name), + actor=actor, + ): + links.append( + { + "href": datasette.urls.table(database, query_name) + "/-/edit", + "label": "Edit this query", + "description": ( + "Change the title, description, SQL or visibility." + ), + } + ) + if await datasette.allowed( + action="delete-query", + resource=QueryResource(database, query_name), + actor=actor, + ): + links.append( + { + "href": datasette.urls.table(database, query_name) + "/-/delete", + "label": "Delete this query", + "description": "Permanently remove this saved query.", + } + ) + return links + + return inner diff --git a/datasette/plugins.py b/datasette/plugins.py index 5a31cdad..f0fbc7f8 100644 --- a/datasette/plugins.py +++ b/datasette/plugins.py @@ -31,6 +31,7 @@ DEFAULT_PLUGINS = ( "datasette.default_debug_menu", "datasette.default_jump_items", "datasette.default_database_actions", + "datasette.default_query_actions", "datasette.handle_exception", "datasette.forbidden", "datasette.events", diff --git a/datasette/templates/_query_form_styles.html b/datasette/templates/_query_form_styles.html new file mode 100644 index 00000000..cf2dd42c --- /dev/null +++ b/datasette/templates/_query_form_styles.html @@ -0,0 +1,138 @@ + diff --git a/datasette/templates/query_create.html b/datasette/templates/query_create.html index ec910456..f2016f27 100644 --- a/datasette/templates/query_create.html +++ b/datasette/templates/query_create.html @@ -6,139 +6,7 @@ {{- super() -}} {% include "_codemirror.html" %} {% include "_execute_write_analysis_styles.html" %} - +{% include "_query_form_styles.html" %} {% endblock %} {% block body_class %}query-create db-{{ database|to_css_class }}{% endblock %} diff --git a/datasette/templates/query_delete.html b/datasette/templates/query_delete.html new file mode 100644 index 00000000..4d0699a7 --- /dev/null +++ b/datasette/templates/query_delete.html @@ -0,0 +1,82 @@ +{% extends "base.html" %} + +{% block title %}Delete query: {{ query.name }}{% endblock %} + +{% block extra_head %} +{{- super() -}} + +{% endblock %} + +{% block body_class %}query-delete db-{{ database|to_css_class }}{% endblock %} + +{% block crumbs %} +{{ crumbs.nav(request=request, database=database) }} +{% endblock %} + +{% block content %} + +
+ +

Delete query: {{ query.title or query.name }}

+ +

Are you sure you want to delete this saved query? This cannot be undone.

+ +
+
URL
+
{{ query_url }}
+ {% if query.description %} +
Description
+
{{ query.description }}
+ {% endif %} +
SQL
+ 
{{ query.sql }}
+
+ +
+

+ + Cancel +

+
+ +
+ +{% endblock %} diff --git a/datasette/templates/query_edit.html b/datasette/templates/query_edit.html new file mode 100644 index 00000000..3eadf42a --- /dev/null +++ b/datasette/templates/query_edit.html @@ -0,0 +1,133 @@ +{% extends "base.html" %} + +{% block title %}Edit query: {{ name }}{% endblock %} + +{% block extra_head %} +{{- super() -}} +{% include "_codemirror.html" %} +{% include "_execute_write_analysis_styles.html" %} +{% include "_query_form_styles.html" %} +{% endblock %} + +{% block body_class %}query-edit db-{{ database|to_css_class }}{% endblock %} + +{% block crumbs %} +{{ crumbs.nav(request=request, database=database) }} +{% endblock %} + +{% block content %} + +
+ +

Edit query: {{ title or name }}

+ +
+
+

+

{{ query_url }}

+

+
+ +

+ +

+ {% if analysis_error %}This query cannot be saved until the SQL is valid.{% elif not has_sql %}Enter SQL to analyze this query.{% elif analysis_is_write %}This query updates data in the database.{% else %}This is a read-only query.{% endif %} + + + Queries marked private can only be seen and edited by you, their owner. +

+

Cancel

+ +
+ {% if has_sql %} +

Query operations

+ {% if analysis_error %} +

{{ analysis_error }}

+ {% elif analysis_rows %} +
+ + + + + + + + + + + {% for row in analysis_rows %} + + + + + + + + {% endfor %} + +
OperationDatabaseTableRequired permissionAllowed
{{ row.operation }}{{ row.database }}{{ row.table }}{% if row.required_permission %}{{ row.required_permission }}{% else %}n/a{% endif %}{% if row.allowed is none %}n/a{% elif row.allowed %}yes{% else %}no{% endif %}
+ {% else %} +

Analysis will show each affected table and required permission.

+ {% endif %} + {% endif %} +
+
+ +
+ +{% include "_codemirror_foot.html" %} +{% include "_sql_parameter_scripts.html" %} +{% include "_execute_write_analysis_scripts.html" %} + + + +{% endblock %} diff --git a/datasette/templates/query_list.html b/datasette/templates/query_list.html index fa4859b1..a8c9a391 100644 --- a/datasette/templates/query_list.html +++ b/datasette/templates/query_list.html @@ -205,32 +205,32 @@

Queries

-
-

- - - {% if filters.is_write %}{% endif %} - {% if filters.is_private %}{% endif %} - {% if filters.source %}{% endif %} - {% if filters.owner_id %}{% endif %} - -

-
- - - {% if queries %} +
+

+ + + {% if filters.is_write %}{% endif %} + {% if filters.is_private %}{% endif %} + {% if filters.source %}{% endif %} + {% if filters.owner_id %}{% endif %} + +

+
+ + +
diff --git a/datasette/views/query_helpers.py b/datasette/views/query_helpers.py index f30a30bc..9efe3f81 100644 --- a/datasette/views/query_helpers.py +++ b/datasette/views/query_helpers.py @@ -436,6 +436,35 @@ async def _query_create_form_context( } +async def _query_edit_form_context( + datasette, + request, + db, + existing: StoredQuery, + *, + sql=None, + title=None, + description=None, + is_private=None, +): + sql = existing.sql if sql is None else sql + title = existing.title if title is None else title + description = existing.description if description is None else description + is_private = existing.is_private if is_private is None else is_private + analysis_data = await _query_create_analysis_data(datasette, db, sql, request.actor) + return { + "database": db.name, + "database_color": db.color, + "name": existing.name, + "sql": sql, + "title": title or "", + "description": description or "", + "is_private": is_private, + "query_url": datasette.urls.table(db.name, existing.name), + **analysis_data, + } + + async def _inserted_row_url(datasette, db, analysis, cursor): if cursor.rowcount != 1: return None diff --git a/datasette/views/stored_queries.py b/datasette/views/stored_queries.py index 8c4e849e..2753f876 100644 --- a/datasette/views/stored_queries.py +++ b/datasette/views/stored_queries.py @@ -18,6 +18,7 @@ from .query_helpers import ( _query_create_analysis_data, _query_create_form_context, _query_create_form_error_message, + _query_edit_form_context, _query_list_limit, ) @@ -464,13 +465,164 @@ class QueryUpdateView(BaseView): return Response.json({"ok": True}) -class QueryDeleteView(BaseView): - name = "query-delete" +class QueryEditView(BaseView): + name = "query-edit" + has_json_alternate = False - async def post(self, request): + async def _load(self, request): db = await self.ds.resolve_database(request) query_name = tilde_decode(request.url_vars["query"]) existing = await self.ds.get_query(db.name, query_name) + return db, query_name, existing + + async def _render_form( + self, + request, + db, + existing, + *, + sql=None, + title=None, + description=None, + is_private=None, + status=200, + ): + response = await self.render( + ["query_edit.html"], + request, + await _query_edit_form_context( + self.ds, + request, + db, + existing, + sql=sql, + title=title, + description=description, + is_private=is_private, + ), + ) + response.status = status + return response + + async def get(self, request): + db, query_name, existing = await self._load(request) + if existing is None: + return _error(["Query not found: {}".format(query_name)], 404) + await self.ds.ensure_permission( + action="update-query", + resource=QueryResource(db.name, query_name), + actor=request.actor, + ) + if existing.is_trusted: + return _error(["Trusted queries cannot be edited"], 403) + return await self._render_form(request, db, existing) + + async def post(self, request): + db, query_name, existing = await self._load(request) + if existing is None: + return _error(["Query not found: {}".format(query_name)], 404) + if not await self.ds.allowed( + action="update-query", + resource=QueryResource(db.name, query_name), + actor=request.actor, + ): + return _error(["Permission denied: need update-query"], 403) + if existing.is_trusted: + return _error(["Trusted queries cannot be edited"], 403) + + data, _ = await _json_or_form_payload(request) + if not isinstance(data, dict): + return _error(["Invalid form submission"], 400) + sql = data.get("sql") + sql = existing.sql if sql is None else sql.strip() + title = data.get("title") or "" + description = data.get("description") or "" + is_private = _as_bool(data.get("is_private")) + + update = { + "title": title, + "description": description, + "is_private": is_private, + } + if sql != existing.sql: + if not await self.ds.allowed( + action="execute-sql", + resource=DatabaseResource(db.name), + actor=request.actor, + ): + self.ds.add_message( + request, + "Permission denied: need execute-sql to change the SQL", + self.ds.ERROR, + ) + return await self._render_form( + request, + db, + existing, + sql=sql, + title=title, + description=description, + is_private=is_private, + status=403, + ) + update["sql"] = sql + + try: + update_kwargs = await _prepare_query_update( + self.ds, request, db, existing, update + ) + except QueryValidationError as ex: + self.ds.add_message(request, ex.message, self.ds.ERROR) + return await self._render_form( + request, + db, + existing, + sql=sql, + title=title, + description=description, + is_private=is_private, + status=ex.status, + ) + + await self.ds.update_query(db.name, query_name, **update_kwargs) + self.ds.add_message(request, "Query updated", self.ds.INFO) + return Response.redirect( + self.ds.urls.path(self.ds.urls.table(db.name, query_name)) + ) + + +class QueryDeleteView(BaseView): + name = "query-delete" + has_json_alternate = False + + async def _load(self, request): + db = await self.ds.resolve_database(request) + query_name = tilde_decode(request.url_vars["query"]) + existing = await self.ds.get_query(db.name, query_name) + return db, query_name, existing + + async def get(self, request): + db, query_name, existing = await self._load(request) + if existing is None: + return _error(["Query not found: {}".format(query_name)], 404) + await self.ds.ensure_permission( + action="delete-query", + resource=QueryResource(db.name, query_name), + actor=request.actor, + ) + return await self.render( + ["query_delete.html"], + request, + { + "database": db.name, + "database_color": db.color, + "query": stored_query_to_dict(existing), + "query_url": self.ds.urls.table(db.name, query_name), + }, + ) + + async def post(self, request): + db, query_name, existing = await self._load(request) if existing is None: return _error(["Query not found: {}".format(query_name)], 404) if not await self.ds.allowed( @@ -479,5 +631,14 @@ class QueryDeleteView(BaseView): actor=request.actor, ): return _error(["Permission denied: need delete-query"], 403) + + data, is_json = await _json_or_form_payload(request) await self.ds.remove_query(db.name, query_name) - return Response.json({"ok": True}) + if is_json: + return Response.json({"ok": True}) + self.ds.add_message( + request, + "Query “{}” deleted".format(existing.title or query_name), + self.ds.INFO, + ) + return Response.redirect(self.ds.urls.path(self.ds.urls.database(db.name))) diff --git a/docs/changelog.rst b/docs/changelog.rst index d5f8fa14..75e4f3e8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,13 @@ Changelog ========= +.. _v1_0_a33: + +1.0a33 (unreleased) +------------------- + +- Stored queries can now be edited and deleted from the web interface. The stored query page gained a "Query actions" menu with **Edit this query** and **Delete this query** links for actors with the necessary permissions. The owner of a query can always edit or delete it; for queries that are not private, any actor with the :ref:`update-query ` or :ref:`delete-query ` permission can do so too. Private queries remain editable and deletable only by their owner. See :ref:`stored_queries` for details. (:issue:`2735`) + .. _v1_0_a32: 1.0a32 (2026-05-31) diff --git a/docs/plugins.rst b/docs/plugins.rst index d578e9e2..c2eb282a 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -271,6 +271,15 @@ If you run ``datasette plugins --all`` it will include default plugins that ship "register_token_handler" ] }, + { + "name": "datasette.default_query_actions", + "static": false, + "templates": false, + "version": null, + "hooks": [ + "query_actions" + ] + }, { "name": "datasette.events", "static": false, diff --git a/docs/sql_queries.rst b/docs/sql_queries.rst index c0ba67f0..371348fb 100644 --- a/docs/sql_queries.rst +++ b/docs/sql_queries.rst @@ -142,6 +142,15 @@ Datasette stores both configured queries and user-created queries in the ``queri Stored queries created by users default to private. Private stored queries can only be viewed, updated or deleted by the actor that created them. Broad ``view-query``, ``update-query`` or ``delete-query`` permission grants still do not allow other actors to access another actor's private stored queries. +Editing and deleting stored queries ++++++++++++++++++++++++++++++++++++ + +The page for a stored query includes a "Query actions" menu with **Edit this query** and **Delete this query** links for actors who have permission to use them. + +The owner of a stored query can always edit and delete it. For queries that are not private, any actor granted the ``update-query`` or ``delete-query`` permission can edit or delete the query, even if they did not create it. Private queries can only be edited or deleted by their owner, regardless of any broad permission grants. + +Editing a query lets you change its title, description, SQL and whether it is private. Changing the SQL also requires the ``execute-sql`` permission (and the relevant write permissions for writable queries). The same operations are available through the JSON API by sending a ``POST`` to ``///-/update`` or ``///-/delete``. Trusted stored queries cannot be edited or deleted through the web interface or the JSON API. + Stored queries created by users are untrusted. This means they execute using the permissions of the actor who runs them, as if that actor had pasted the SQL into the regular custom SQL interface or write SQL interface. Read-only stored queries require ``execute-sql``. Writable stored queries require ``execute-write-sql`` plus the relevant table-level write permissions. SQL functions are allowed and are not separately restricted by Datasette permissions. .. _trusted_stored_queries: diff --git a/tests/test_docs.py b/tests/test_docs.py index 9cf39f41..51caf595 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -77,6 +77,7 @@ def documented_views(): "QueryCreateAnalyzeView", "QueryDeleteView", "QueryDefinitionView", + "QueryEditView", "QueryListView", "QueryParametersView", "QueryStoreView", diff --git a/tests/test_queries.py b/tests/test_queries.py index 25e423d4..6e9bcbdb 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -3,6 +3,7 @@ import re from html import unescape import pytest +from bs4 import BeautifulSoup as Soup from datasette.app import Datasette from datasette.resources import DatabaseResource, QueryResource @@ -712,6 +713,10 @@ async def test_query_list_search_filter_and_html(): "/data/-/queries?is_private=1", actor={"id": "root"}, ) + no_results_response = await ds.client.get( + "/data/-/queries?q=nope", + actor={"id": "root"}, + ) assert html_response.status_code == 200 assert "Demo query 02" in html_response.text @@ -799,6 +804,13 @@ async def test_query_list_search_filter_and_html(): 'Not private0' not in filtered_private_response.text ) + assert no_results_response.status_code == 200 + assert "No queries found." in no_results_response.text + assert 'class="query-list-filters core"' not in no_results_response.text + assert 'id="query-search"' not in no_results_response.text + assert 'class="query-list-facets"' not in no_results_response.text + assert "

Mode

" not in no_results_response.text + assert "

Visibility

" not in no_results_response.text @pytest.mark.asyncio @@ -1114,6 +1126,227 @@ async def test_query_update_api_rejects_trusted_queries_but_internal_update_allo assert query.title == "Internal" +async def _make_ds_with_user_query(name, *, is_private=False, owner_id="owner"): + ds = Datasette(memory=True, settings={"default_allow_sql": True}) + db = ds.add_memory_database(name, name="data") + await db.execute_write("create table dogs (id integer primary key, name text)") + await ds.invoke_startup() + await ds.add_query( + "data", + "saved", + "select * from dogs", + title="Saved query", + description="A saved query", + source="user", + owner_id=owner_id, + is_private=is_private, + ) + return ds + + +@pytest.mark.asyncio +async def test_query_edit_form_renders_and_updates_for_owner(): + ds = await _make_ds_with_user_query("query_edit_owner") + actor = {"id": "owner"} + + # GET renders the form pre-filled with existing values + get_response = await ds.client.get("/data/saved/-/edit", actor=actor) + assert get_response.status_code == 200 + assert 'value="Saved query"' in get_response.text + assert ">A saved query" in get_response.text + assert "select * from dogs" in get_response.text + # URL slug is shown but not editable + assert 'name="name"' not in get_response.text + + # POST updates the query and redirects back to the query page + post_response = await ds.client.post( + "/data/saved/-/edit", + actor=actor, + data={ + "title": "Updated title", + "description": "Updated description", + "sql": "select id from dogs", + "is_private": "1", + }, + ) + assert post_response.status_code == 302 + assert post_response.headers["location"] == "/data/saved" + + query = await ds.get_query("data", "saved") + assert query.title == "Updated title" + assert query.description == "Updated description" + assert query.sql == "select id from dogs" + assert query.is_private is True + + +@pytest.mark.asyncio +async def test_query_edit_metadata_only_does_not_require_execute_sql(): + # An owner who can no longer execute SQL can still edit title/description + ds = await _make_ds_with_user_query("query_edit_metadata_only") + actor = {"id": "owner"} + + post_response = await ds.client.post( + "/data/saved/-/edit", + actor=actor, + data={ + "title": "Renamed", + "description": "A saved query", + "sql": "select * from dogs", + }, + ) + assert post_response.status_code == 302 + query = await ds.get_query("data", "saved") + assert query.title == "Renamed" + + +@pytest.mark.asyncio +async def test_private_query_edit_delete_restricted_to_owner(): + ds = await _make_ds_with_user_query( + "query_edit_private", is_private=True, owner_id="owner" + ) + + # A different actor cannot view, edit or delete the private query + other = {"id": "intruder"} + assert (await ds.client.get("/data/saved/-/edit", actor=other)).status_code == 403 + assert (await ds.client.get("/data/saved/-/delete", actor=other)).status_code == 403 + delete_attempt = await ds.client.post( + "/data/saved/-/delete", + actor=other, + data={}, + ) + assert delete_attempt.status_code == 403 + assert await ds.get_query("data", "saved") is not None + + # The owner can edit and delete + owner = {"id": "owner"} + assert (await ds.client.get("/data/saved/-/edit", actor=owner)).status_code == 200 + + +@pytest.mark.asyncio +async def test_non_private_query_editable_by_permitted_non_owner(): + ds = Datasette( + memory=True, + default_deny=True, + config={ + "databases": { + "data": { + "permissions": { + "execute-sql": {"id": "editor"}, + "update-query": {"id": "editor"}, + "delete-query": {"id": "editor"}, + } + } + } + }, + ) + db = ds.add_memory_database("query_non_private_editor", name="data") + await db.execute_write("create table dogs (id integer primary key, name text)") + await ds.invoke_startup() + await ds.add_query( + "data", + "saved", + "select * from dogs", + title="Shared", + source="user", + owner_id="owner", + is_private=False, + ) + + editor = {"id": "editor"} + # Editor (not the owner) can edit because the query is not private + post_response = await ds.client.post( + "/data/saved/-/edit", + actor=editor, + data={ + "title": "Edited by editor", + "description": "", + "sql": "select * from dogs", + }, + ) + assert post_response.status_code == 302 + query = await ds.get_query("data", "saved") + assert query.title == "Edited by editor" + + # Editor can also delete it + delete_response = await ds.client.post( + "/data/saved/-/delete", + actor=editor, + data={}, + ) + assert delete_response.status_code == 302 + assert await ds.get_query("data", "saved") is None + + +@pytest.mark.asyncio +async def test_query_delete_confirmation_and_form_delete(): + ds = await _make_ds_with_user_query("query_delete_form") + actor = {"id": "owner"} + + get_response = await ds.client.get("/data/saved/-/delete", actor=actor) + assert get_response.status_code == 200 + assert "Are you sure" in get_response.text + assert "select * from dogs" in get_response.text + soup = Soup(get_response.text, "html.parser") + form = soup.select_one("form.query-delete-form") + assert form is not None + assert "core" in form["class"] + assert form.select_one('input[type="submit"][value="Delete query"]') is not None + + post_response = await ds.client.post( + "/data/saved/-/delete", + actor=actor, + data={}, + ) + assert post_response.status_code == 302 + assert post_response.headers["location"] == "/data" + assert await ds.get_query("data", "saved") is None + + +@pytest.mark.asyncio +async def test_query_action_menu_shows_edit_and_delete_for_owner(): + ds = await _make_ds_with_user_query("query_action_menu") + + owner_response = await ds.client.get("/data/saved", actor={"id": "owner"}) + assert owner_response.status_code == 200 + assert "/data/saved/-/edit" in owner_response.text + assert "/data/saved/-/delete" in owner_response.text + + # A different actor (the query is public) cannot edit/delete by default + other_response = await ds.client.get("/data/saved", actor={"id": "stranger"}) + assert other_response.status_code == 200 + assert "/data/saved/-/edit" not in other_response.text + assert "/data/saved/-/delete" not in other_response.text + + +@pytest.mark.asyncio +async def test_query_edit_rejected_for_trusted_query(): + ds = Datasette( + memory=True, + default_deny=True, + config={ + "databases": { + "data": { + "permissions": { + "execute-sql": {"id": "editor"}, + "update-query": {"id": "editor"}, + }, + "queries": {"trusted_report": {"sql": "select 1 as one"}}, + } + } + }, + ) + ds.add_memory_database("query_edit_trusted", name="data") + await ds.invoke_startup() + + response = await ds.client.get( + "/data/trusted_report/-/edit", actor={"id": "editor"} + ) + assert response.status_code == 403 + # Edit/delete links should not appear on a trusted/config query page + page = await ds.client.get("/data/trusted_report", actor={"id": "editor"}) + assert "/data/trusted_report/-/edit" not in page.text + + @pytest.mark.asyncio async def test_query_store_api_rejects_magic_parameters(): ds = Datasette(memory=True, default_deny=True)