Skip to content

Commit

Permalink
Respect query permissions on database page, refs #800
Browse files Browse the repository at this point in the history
  • Loading branch information
simonw committed Jun 6, 2020
1 parent 14f6b4d commit 3f83d46
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 3 deletions.
2 changes: 1 addition & 1 deletion datasette/templates/database.html
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ <h2 id="views">Views</h2>
<h2 id="queries">Queries</h2>
<ul>
{% for query in queries %}
<li><a href="{{ database_url(database) }}/{{ query.name|urlencode }}{% if query.fragment %}#{{ query.fragment }}{% endif %}" title="{{ query.description or query.sql }}">{{ query.title or query.name }}</a></li>
<li><a href="{{ database_url(database) }}/{{ query.name|urlencode }}{% if query.fragment %}#{{ query.fragment }}{% endif %}" title="{{ query.description or query.sql }}">{{ query.title or query.name }}</a> {% if query.requires_auth %} - requires authentication{% endif %}</li>
{% endfor %}
</ul>
{% endif %}
Expand Down
1 change: 1 addition & 0 deletions datasette/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ def call_with_supported_arguments(fn, **kwargs):


def actor_matches_allow(actor, allow):
actor = actor or {}
if allow is None:
return True
for key, values in allow.items():
Expand Down
13 changes: 12 additions & 1 deletion datasette/views/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import jinja2

from datasette.utils import (
actor_matches_allow,
to_css_class,
validate_sql_select,
is_url,
Expand Down Expand Up @@ -53,14 +54,24 @@ async def data(self, request, database, hash, default_labels=False, _size=None):
)

tables.sort(key=lambda t: (t["hidden"], t["name"]))
canned_queries = [
dict(
query,
requires_auth=not actor_matches_allow(None, query.get("allow", None)),
)
for query in self.ds.get_canned_queries(database)
if actor_matches_allow(
request.scope.get("actor", None), query.get("allow", None)
)
]
return (
{
"database": database,
"size": db.size,
"tables": tables,
"hidden_count": len([t for t in tables if t["hidden"]]),
"views": views,
"queries": self.ds.get_canned_queries(database),
"queries": canned_queries,
},
{
"show_hidden": request.args.get("_show_hidden"),
Expand Down
31 changes: 30 additions & 1 deletion tests/test_canned_write.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def canned_write_client():
"sql": "delete from names where rowid = :rowid",
"write": True,
"on_success_message": "Name deleted",
"allow": {"id": "root"},
},
"update_name": {
"sql": "update names set name = :name where rowid = :rowid",
Expand Down Expand Up @@ -52,7 +53,11 @@ def test_insert(canned_write_client):

def test_custom_success_message(canned_write_client):
response = canned_write_client.post(
"/data/delete_name", {"rowid": 1}, allow_redirects=False, csrftoken_from=True
"/data/delete_name",
{"rowid": 1},
cookies={"ds_actor": canned_write_client.ds.sign({"id": "root"}, "actor")},
allow_redirects=False,
csrftoken_from=True,
)
assert 302 == response.status
messages = canned_write_client.ds.unsign(
Expand Down Expand Up @@ -93,3 +98,27 @@ def test_insert_error(canned_write_client):
def test_custom_params(canned_write_client):
response = canned_write_client.get("/data/update_name?extra=foo")
assert '<input type="text" id="qp3" name="extra" value="foo">' in response.text


def test_canned_query_permissions_on_database_page(canned_write_client):
# Without auth only shows three queries
query_names = [
q["name"] for q in canned_write_client.get("/data.json").json["queries"]
]
assert ["add_name", "add_name_specify_id", "update_name"] == query_names

# With auth shows four
response = canned_write_client.get(
"/data.json",
cookies={"ds_actor": canned_write_client.ds.sign({"id": "root"}, "actor")},
)
assert 200 == response.status
assert [
{"name": "add_name", "requires_auth": False},
{"name": "add_name_specify_id", "requires_auth": False},
{"name": "delete_name", "requires_auth": True},
{"name": "update_name", "requires_auth": False},
] == [
{"name": q["name"], "requires_auth": q["requires_auth"]}
for q in response.json["queries"]
]
3 changes: 3 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,9 @@ def test_multi_params(data, should_raise):
[
({"id": "root"}, None, True),
({"id": "root"}, {}, False),
(None, None, True),
(None, {}, False),
(None, {"id": "root"}, False),
# Special "*" value for any key:
({"id": "root"}, {"id": "*"}, True),
({}, {"id": "*"}, False),
Expand Down

0 comments on commit 3f83d46

Please sign in to comment.