mirror of
https://github.com/simonw/datasette.git
synced 2026-05-31 06:07:05 +02:00
Nicer UI around Execute Write SQL denied
Refs https://github.com/simonw/datasette/issues/2753#issuecomment-4569117665
This commit is contained in:
parent
6a998610ee
commit
e5b6166fa3
4 changed files with 160 additions and 34 deletions
|
|
@ -40,6 +40,26 @@
|
|||
border-radius: 0.25rem;
|
||||
min-width: 13rem;
|
||||
}
|
||||
.execute-write-submit-row {
|
||||
align-items: center;
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
gap: 0.45rem 0.75rem;
|
||||
}
|
||||
.execute-write-submit-row [hidden] {
|
||||
display: none;
|
||||
}
|
||||
form.sql.core input[data-execute-write-submit]:disabled {
|
||||
background: #d0d7de;
|
||||
border-color: #b6c0cc;
|
||||
color: #5f6975;
|
||||
cursor: not-allowed;
|
||||
opacity: 1;
|
||||
}
|
||||
.execute-write-disabled-reason {
|
||||
color: #4f5b6d;
|
||||
font-size: 0.85rem;
|
||||
}
|
||||
</style>
|
||||
{% include "_execute_write_analysis_styles.html" %}
|
||||
{% include "_sql_parameter_styles.html" %}
|
||||
|
|
@ -119,9 +139,10 @@
|
|||
{% endif %}
|
||||
</div>
|
||||
|
||||
<p>
|
||||
<input type="submit" value="Execute" data-execute-write-submit{% if execute_disabled %} disabled{% endif %}>
|
||||
{% if save_query_base_url %}<a href="{{ save_query_url or save_query_base_url }}" class="save-query" data-save-query-link data-save-query-base-url="{{ save_query_base_url }}"{% if not save_query_url %} hidden{% endif %}>Save this query</a>{% endif %}
|
||||
<p class="execute-write-submit-row"{% if save_query_base_url %} data-save-query-base-url="{{ save_query_base_url }}"{% endif %}>
|
||||
<input type="submit" value="Execute" data-execute-write-submit aria-describedby="execute-write-disabled-reason"{% if execute_disabled %} disabled{% endif %}>
|
||||
<span id="execute-write-disabled-reason" class="execute-write-disabled-reason" data-execute-write-disabled-reason aria-live="polite"{% if not execute_disabled_reason %} hidden{% endif %}>{{ execute_disabled_reason or "" }}</span>
|
||||
{% if save_query_url %}<a href="{{ save_query_url }}" class="save-query" data-save-query-link>Save this query</a>{% endif %}
|
||||
</p>
|
||||
</form>
|
||||
|
||||
|
|
@ -143,25 +164,55 @@ window.addEventListener("DOMContentLoaded", () => {
|
|||
const submitButton = form
|
||||
? form.querySelector("[data-execute-write-submit]")
|
||||
: null;
|
||||
const saveQueryLink = form
|
||||
const submitDisabledReason = form
|
||||
? form.querySelector("[data-execute-write-disabled-reason]")
|
||||
: null;
|
||||
const submitRow = form
|
||||
? form.querySelector(".execute-write-submit-row")
|
||||
: null;
|
||||
let saveQueryLink = form
|
||||
? form.querySelector("[data-save-query-link]")
|
||||
: null;
|
||||
|
||||
function updateSubmitState(data) {
|
||||
if (submitButton) {
|
||||
submitButton.disabled = data.execute_disabled;
|
||||
}
|
||||
if (!submitDisabledReason) {
|
||||
return;
|
||||
}
|
||||
const reason = data.execute_disabled_reason || "";
|
||||
submitDisabledReason.textContent = reason;
|
||||
submitDisabledReason.hidden = !reason;
|
||||
}
|
||||
|
||||
function updateSaveQueryLink(data) {
|
||||
if (!saveQueryLink) {
|
||||
if (!submitRow || !submitRow.dataset.saveQueryBaseUrl) {
|
||||
return;
|
||||
}
|
||||
const sql = window.editor
|
||||
? window.editor.state.doc.toString()
|
||||
: executeWriteSqlInput.value;
|
||||
if (!sql.trim() || !data.ok || data.execute_disabled) {
|
||||
saveQueryLink.hidden = true;
|
||||
if (saveQueryLink) {
|
||||
saveQueryLink.remove();
|
||||
saveQueryLink = null;
|
||||
}
|
||||
return;
|
||||
}
|
||||
const url = new URL(saveQueryLink.dataset.saveQueryBaseUrl, window.location.href);
|
||||
if (!saveQueryLink) {
|
||||
saveQueryLink = document.createElement("a");
|
||||
saveQueryLink.className = "save-query";
|
||||
saveQueryLink.setAttribute("data-save-query-link", "");
|
||||
saveQueryLink.textContent = "Save this query";
|
||||
submitRow.appendChild(saveQueryLink);
|
||||
}
|
||||
const url = new URL(
|
||||
submitRow.dataset.saveQueryBaseUrl,
|
||||
window.location.href
|
||||
);
|
||||
url.searchParams.set("sql", sql);
|
||||
saveQueryLink.href = url.pathname + url.search + url.hash;
|
||||
saveQueryLink.hidden = false;
|
||||
}
|
||||
|
||||
window.datasetteSqlParameters.setupSqlParameterRefresh({
|
||||
|
|
@ -170,9 +221,7 @@ window.addEventListener("DOMContentLoaded", () => {
|
|||
allowExpand: true,
|
||||
onData(data) {
|
||||
window.datasetteSqlAnalysis.renderAnalysis(analysisSection, data);
|
||||
if (submitButton) {
|
||||
submitButton.disabled = data.execute_disabled;
|
||||
}
|
||||
updateSubmitState(data);
|
||||
updateSaveQueryLink(data);
|
||||
},
|
||||
onError(error) {
|
||||
|
|
@ -180,12 +229,11 @@ window.addEventListener("DOMContentLoaded", () => {
|
|||
analysis_error: error.message,
|
||||
analysis_rows: [],
|
||||
});
|
||||
if (submitButton) {
|
||||
submitButton.disabled = true;
|
||||
}
|
||||
if (saveQueryLink) {
|
||||
saveQueryLink.hidden = true;
|
||||
}
|
||||
updateSubmitState({
|
||||
execute_disabled: true,
|
||||
execute_disabled_reason: error.message,
|
||||
});
|
||||
updateSaveQueryLink({ ok: false, execute_disabled: true });
|
||||
},
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from .query_helpers import (
|
|||
_coerce_execute_write_payload,
|
||||
_derived_query_parameters,
|
||||
_execute_write_analysis_data,
|
||||
_execute_write_disabled_reason,
|
||||
_inserted_row_url,
|
||||
_json_or_form_payload,
|
||||
_prepare_execute_write,
|
||||
|
|
@ -80,13 +81,12 @@ class ExecuteWriteView(BaseView):
|
|||
)
|
||||
save_query_base_url = None
|
||||
save_query_url = None
|
||||
execute_disabled_reason = _execute_write_disabled_reason(
|
||||
sql, analysis_error, analysis_rows
|
||||
)
|
||||
if allow_save_query:
|
||||
save_query_base_url = self.ds.urls.database(db.name) + "/-/queries/store"
|
||||
if (
|
||||
sql
|
||||
and analysis_error is None
|
||||
and not any(row["allowed"] is False for row in analysis_rows)
|
||||
):
|
||||
if not execute_disabled_reason:
|
||||
save_query_url = save_query_base_url + "?" + urlencode({"sql": sql})
|
||||
|
||||
response = await self.render(
|
||||
|
|
@ -103,11 +103,8 @@ class ExecuteWriteView(BaseView):
|
|||
"execution_message": execution_message,
|
||||
"execution_links": execution_links,
|
||||
"execution_ok": execution_ok,
|
||||
"execute_disabled": bool(
|
||||
(not sql)
|
||||
or analysis_error
|
||||
or any(row["allowed"] is False for row in analysis_rows)
|
||||
),
|
||||
"execute_disabled": bool(execute_disabled_reason),
|
||||
"execute_disabled_reason": execute_disabled_reason,
|
||||
"table_columns": table_columns,
|
||||
"write_template_tables": write_template_tables,
|
||||
"save_query_url": save_query_url,
|
||||
|
|
|
|||
|
|
@ -268,6 +268,16 @@ async def _analysis_rows_with_permissions(
|
|||
return rows
|
||||
|
||||
|
||||
def _execute_write_disabled_reason(sql, analysis_error, analysis_rows):
|
||||
if not (sql and sql.strip()):
|
||||
return "Enter writable SQL before executing."
|
||||
if analysis_error:
|
||||
return analysis_error
|
||||
if any(row.get("allowed") is False for row in analysis_rows):
|
||||
return "You do not have permission for every operation listed above."
|
||||
return None
|
||||
|
||||
|
||||
def _coerce_execute_write_payload(data, is_json):
|
||||
if not isinstance(data, dict):
|
||||
raise QueryValidationError("JSON must be a dictionary")
|
||||
|
|
@ -358,16 +368,16 @@ async def _execute_write_analysis_data(datasette, db, sql, actor):
|
|||
)
|
||||
except (QueryValidationError, sqlite3.DatabaseError) as ex:
|
||||
analysis_error = getattr(ex, "message", str(ex))
|
||||
execute_disabled_reason = _execute_write_disabled_reason(
|
||||
sql, analysis_error, analysis_rows
|
||||
)
|
||||
return {
|
||||
"ok": analysis_error is None,
|
||||
"parameters": parameter_names,
|
||||
"analysis_error": analysis_error,
|
||||
"analysis_rows": analysis_rows,
|
||||
"execute_disabled": bool(
|
||||
(not sql)
|
||||
or analysis_error
|
||||
or any(row["allowed"] is False for row in analysis_rows)
|
||||
),
|
||||
"execute_disabled": bool(execute_disabled_reason),
|
||||
"execute_disabled_reason": execute_disabled_reason,
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1374,6 +1374,10 @@ async def test_execute_write_get_prepopulates_without_executing():
|
|||
assert 'addEventListener("paste"' in response.text
|
||||
assert "setupSqlParameterRefresh" in response.text
|
||||
assert "datasetteSqlAnalysis.renderAnalysis" in response.text
|
||||
assert "input[data-execute-write-submit]:disabled" in response.text
|
||||
assert (
|
||||
'data-execute-write-disabled-reason aria-live="polite" hidden' in response.text
|
||||
)
|
||||
assert '<table class="execute-write-analysis">' in response.text
|
||||
assert '<th scope="col">Required permission</th>' in response.text
|
||||
assert "<td><code>insert</code></td>" in response.text
|
||||
|
|
@ -1390,7 +1394,9 @@ async def test_execute_write_get_prepopulates_without_executing():
|
|||
)
|
||||
assert '<textarea id="sql-editor" name="sql"></textarea>' in empty_response.text
|
||||
assert 'executeWriteSqlInput.value = "\\n\\n\\n";' in empty_response.text
|
||||
assert "hidden>Save this query</a>" in empty_response.text
|
||||
assert "Enter writable SQL before executing." in empty_response.text
|
||||
assert 'data-save-query-base-url="/data/-/queries/store"' in empty_response.text
|
||||
assert '<a href="/data/-/queries/store' not in empty_response.text
|
||||
|
||||
read_only_response = await ds.client.get(
|
||||
"/data/-/execute-write?sql=select+*+from+dogs",
|
||||
|
|
@ -1400,7 +1406,67 @@ async def test_execute_write_get_prepopulates_without_executing():
|
|||
"Use /-/query for read-only SQL; this endpoint only executes writes"
|
||||
in read_only_response.text
|
||||
)
|
||||
assert "hidden>Save this query</a>" in read_only_response.text
|
||||
assert (
|
||||
'<input type="submit" value="Execute" data-execute-write-submit '
|
||||
'aria-describedby="execute-write-disabled-reason" disabled>'
|
||||
) in read_only_response.text
|
||||
assert 'data-save-query-base-url="/data/-/queries/store"' in read_only_response.text
|
||||
assert '<a href="/data/-/queries/store' not in read_only_response.text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execute_write_disabled_submit_explains_denied_operations():
|
||||
ds = Datasette(
|
||||
memory=True,
|
||||
default_deny=True,
|
||||
config={
|
||||
"databases": {
|
||||
"data": {
|
||||
"permissions": {
|
||||
"view-database": {"id": "writer"},
|
||||
"execute-sql": {"id": "writer"},
|
||||
"execute-write-sql": {"id": "writer"},
|
||||
"store-query": {"id": "writer"},
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
)
|
||||
db = ds.add_memory_database("execute_write_denied_submit", name="data")
|
||||
await db.execute_write("create table dogs (id integer primary key, name text)")
|
||||
await ds.invoke_startup()
|
||||
|
||||
response = await ds.client.get(
|
||||
"/data/-/execute-write?sql=insert+into+dogs+(name)+values+('Cleo')",
|
||||
actor={"id": "writer"},
|
||||
)
|
||||
analysis_response = await ds.client.get(
|
||||
"/data/-/execute-write/analyze",
|
||||
actor={"id": "writer"},
|
||||
params={"sql": "insert into dogs (name) values ('Cleo')"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert (
|
||||
'<input type="submit" value="Execute" data-execute-write-submit '
|
||||
'aria-describedby="execute-write-disabled-reason" disabled>'
|
||||
) in response.text
|
||||
assert (
|
||||
'<span id="execute-write-disabled-reason" '
|
||||
'class="execute-write-disabled-reason" '
|
||||
'data-execute-write-disabled-reason aria-live="polite">'
|
||||
"You do not have permission for every operation listed above.</span>"
|
||||
) in response.text
|
||||
assert '<span class="execute-write-analysis-denied">no</span>' in response.text
|
||||
assert 'data-save-query-base-url="/data/-/queries/store"' in response.text
|
||||
assert '<a href="/data/-/queries/store' not in response.text
|
||||
|
||||
assert analysis_response.status_code == 200
|
||||
data = analysis_response.json()
|
||||
assert data["execute_disabled"] is True
|
||||
assert data["execute_disabled_reason"] == (
|
||||
"You do not have permission for every operation listed above."
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -1433,6 +1499,7 @@ async def test_execute_write_analyze_endpoint_uses_sql_only():
|
|||
assert data["parameters"] == ["name"]
|
||||
assert data["analysis_error"] is None
|
||||
assert data["execute_disabled"] is False
|
||||
assert data["execute_disabled_reason"] is None
|
||||
assert data["analysis_rows"] == [
|
||||
{
|
||||
"operation": "insert",
|
||||
|
|
@ -1450,6 +1517,7 @@ async def test_execute_write_analyze_endpoint_uses_sql_only():
|
|||
assert function_data["ok"] is True
|
||||
assert function_data["parameters"] == ["name"]
|
||||
assert function_data["execute_disabled"] is False
|
||||
assert function_data["execute_disabled_reason"] is None
|
||||
assert function_data["analysis_rows"] == [
|
||||
{
|
||||
"operation": "insert",
|
||||
|
|
@ -1469,6 +1537,9 @@ async def test_execute_write_analyze_endpoint_uses_sql_only():
|
|||
"Use /-/query for read-only SQL; this endpoint only executes writes"
|
||||
)
|
||||
assert read_only_data["execute_disabled"] is True
|
||||
assert read_only_data["execute_disabled_reason"] == (
|
||||
"Use /-/query for read-only SQL; this endpoint only executes writes"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue