From 18c5bd1805cb3bda588aa4beb33ca55b90f99a7e Mon Sep 17 00:00:00 2001
From: Simon Willison {{ metadata.title or database }}
{% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
-
0 results
+ {% if not canned_write %} +0 results
+ {% endif %} {% endif %} {% include "_codemirror_foot.html" %} diff --git a/datasette/views/database.py b/datasette/views/database.py index 15545fb8..558dd0f0 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -106,6 +106,8 @@ class QueryView(DataView): canned_query=None, metadata=None, _size=None, + named_parameters=None, + write=False, ): params = {key: request.args.get(key) for key in request.args} if "sql" in params: @@ -113,7 +115,7 @@ class QueryView(DataView): if "_shape" in params: params.pop("_shape") # Extract any :named parameters - named_parameters = self.re_named_parameter.findall(sql) + named_parameters = named_parameters or self.re_named_parameter.findall(sql) named_parameter_values = { named_parameter: params.get(named_parameter) or "" for named_parameter in named_parameters @@ -129,12 +131,60 @@ class QueryView(DataView): extra_args["custom_time_limit"] = int(params["_timelimit"]) if _size: extra_args["page_size"] = _size - results = await self.ds.execute( - database, sql, params, truncate=True, **extra_args - ) - columns = [r[0] for r in results.description] templates = ["query-{}.html".format(to_css_class(database)), "query.html"] + + # Execute query - as write or as read + if write: + if request.method == "POST": + params = await request.post_vars() + try: + cursor = await self.ds.databases[database].execute_write( + sql, params, block=True + ) + 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") + 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") + self.ds.add_message(request, message, message_type) + return self.redirect(request, redirect_url or request.path) + else: + + async def extra_template(): + return { + "request": request, + "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}, + }, + extra_template, + templates, + ) + else: # Not a write + results = await self.ds.execute( + database, sql, params, truncate=True, **extra_args + ) + columns = [r[0] for r in results.description] + if canned_query: templates.insert( 0, diff --git a/datasette/views/table.py b/datasette/views/table.py index 2e9515c3..79bf8b08 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -221,6 +221,22 @@ class RowTableShared(DataView): class TableView(RowTableShared): name = "table" + async def post(self, request, db_name, table_and_format): + # Handle POST to a canned query + canned_query = self.ds.get_canned_query(db_name, table_and_format) + assert canned_query, "You may only POST to a canned query" + return await QueryView(self.ds).data( + request, + db_name, + None, + canned_query["sql"], + metadata=canned_query, + editable=False, + canned_query=table_and_format, + named_parameters=canned_query.get("params"), + write=bool(canned_query.get("write")), + ) + async def data( self, request, @@ -241,6 +257,8 @@ class TableView(RowTableShared): metadata=canned_query, editable=False, canned_query=table, + named_parameters=canned_query.get("params"), + write=bool(canned_query.get("write")), ) db = self.ds.databases[database] diff --git a/docs/sql_queries.rst b/docs/sql_queries.rst index c3efd930..dc239a84 100644 --- a/docs/sql_queries.rst +++ b/docs/sql_queries.rst @@ -161,11 +161,12 @@ You can set a default fragment hash that will be included in the link to the can { "databases": { - "fixtures": { - "queries": { - "neighborhood_search": { - "sql": "select neighborhood, facet_cities.name, state\nfrom facetable join facet_cities on facetable.city_id = facet_cities.id\nwhere neighborhood like '%' || :text || '%' order by neighborhood;", - "fragment": "fragment-goes-here" + "fixtures": { + "queries": { + "neighborhood_search": { + "sql": "select neighborhood, facet_cities.name, state\nfrom facetable join facet_cities on facetable.city_id = facet_cities.id\nwhere neighborhood like '%' || :text || '%' order by neighborhood;", + "fragment": "fragment-goes-here" + } } } } @@ -173,6 +174,60 @@ You can set a default fragment hash that will be included in the link to the can `See hereSet a message:
-+ {% if canned_query %}{% endif %}
Actor: {{ check.actor|tojson }}
{% if check.resource_type %} -Resource: {{ check.resource_type }}: {{ check.resource_identifier }}
+Resource: {{ check.resource_type }} = {{ check.resource_identifier }}
{% endif %}{")
@@ -1211,7 +1211,7 @@ def test_config_template_debug_off(app_client):
def test_debug_context_includes_extra_template_vars():
# https://github.com/simonw/datasette/issues/693
- for client in make_app_client(config={"template_debug": True}):
+ with make_app_client(config={"template_debug": True}) as client:
response = client.get("/fixtures/facetable?_context=1")
# scope_path is added by PLUGIN1
assert "scope_path" in response.text
@@ -1292,7 +1292,7 @@ def test_metadata_sort_desc(app_client):
],
)
def test_base_url_config(base_url, path):
- for client in make_app_client(config={"base_url": base_url}):
+ with make_app_client(config={"base_url": base_url}) as client:
response = client.get(base_url + path.lstrip("/"))
soup = Soup(response.body, "html.parser")
for el in soup.findAll(["a", "link", "script"]):
diff --git a/tests/test_plugins.py b/tests/test_plugins.py
index f69e7fa7..c782b87b 100644
--- a/tests/test_plugins.py
+++ b/tests/test_plugins.py
@@ -229,9 +229,9 @@ def test_plugins_asgi_wrapper(app_client):
def test_plugins_extra_template_vars(restore_working_directory):
- for client in make_app_client(
+ with make_app_client(
template_dir=str(pathlib.Path(__file__).parent / "test_templates")
- ):
+ ) as client:
response = client.get("/-/metadata")
assert response.status == 200
extra_template_vars = json.loads(
@@ -254,9 +254,9 @@ def test_plugins_extra_template_vars(restore_working_directory):
def test_plugins_async_template_function(restore_working_directory):
- for client in make_app_client(
+ with make_app_client(
template_dir=str(pathlib.Path(__file__).parent / "test_templates")
- ):
+ ) as client:
response = client.get("/-/metadata")
assert response.status == 200
extra_from_awaitable_function = (
From ece0ba6f4bc152af6f605fc5f536ffa46af95274 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Sun, 7 Jun 2020 14:23:16 -0700
Subject: [PATCH 0047/1855] Test + default impl for view-query permission, refs
#811
---
datasette/default_permissions.py | 21 ++++++++++++++++++---
tests/test_permissions.py | 22 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 3 deletions(-)
create mode 100644 tests/test_permissions.py
diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py
index 0b0d17f9..40ae54ab 100644
--- a/datasette/default_permissions.py
+++ b/datasette/default_permissions.py
@@ -1,7 +1,22 @@
from datasette import hookimpl
+from datasette.utils import actor_matches_allow
@hookimpl
-def permission_allowed(actor, action, resource_type, resource_identifier):
- if actor and actor.get("id") == "root" and action == "permissions-debug":
- return True
+def permission_allowed(datasette, actor, action, resource_type, resource_identifier):
+ if action == "permissions-debug":
+ if actor and actor.get("id") == "root":
+ return True
+ elif action == "view-query":
+ # Check if this query has a "allow" block in metadata
+ assert resource_type == "query"
+ database, query_name = resource_identifier
+ queries_metadata = datasette.metadata("queries", database=database)
+ assert query_name in queries_metadata
+ if isinstance(queries_metadata[query_name], str):
+ return True
+ allow = queries_metadata[query_name].get("allow")
+ print("checking allow - actor = {}, allow = {}".format(actor, allow))
+ if allow is None:
+ return True
+ return actor_matches_allow(actor, allow)
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
new file mode 100644
index 00000000..c90fdf7a
--- /dev/null
+++ b/tests/test_permissions.py
@@ -0,0 +1,22 @@
+from .fixtures import make_app_client
+import pytest
+
+
+@pytest.mark.parametrize(
+ "allow,expected_anon,expected_auth",
+ [(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
+)
+def test_execute_sql(allow, expected_anon, expected_auth):
+ with make_app_client(
+ metadata={
+ "databases": {
+ "fixtures": {"queries": {"q": {"sql": "select 1 + 1", "allow": allow}}}
+ }
+ }
+ ) as client:
+ anon_response = client.get("/fixtures/q")
+ assert expected_anon == anon_response.status
+ auth_response = client.get(
+ "/fixtures/q", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
+ )
+ assert expected_auth == auth_response.status
From 8571ce388a23dd98adbdc1b7eff6c6eef5a9d1af Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Sun, 7 Jun 2020 14:30:39 -0700
Subject: [PATCH 0048/1855] Implemented view-instance permission, refs #811
---
datasette/default_permissions.py | 4 ++++
tests/test_permissions.py | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py
index 40ae54ab..ee182c85 100644
--- a/datasette/default_permissions.py
+++ b/datasette/default_permissions.py
@@ -7,6 +7,10 @@ def permission_allowed(datasette, actor, action, resource_type, resource_identif
if action == "permissions-debug":
if actor and actor.get("id") == "root":
return True
+ elif action == "view-instance":
+ allow = datasette.metadata("allow")
+ if allow is not None:
+ return actor_matches_allow(actor, allow)
elif action == "view-query":
# Check if this query has a "allow" block in metadata
assert resource_type == "query"
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index c90fdf7a..b5c2e00c 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -20,3 +20,23 @@ def test_execute_sql(allow, expected_anon, expected_auth):
"/fixtures/q", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
)
assert expected_auth == auth_response.status
+
+
+@pytest.mark.parametrize(
+ "allow,expected_anon,expected_auth",
+ [(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
+)
+def test_view_instance(allow, expected_anon, expected_auth):
+ with make_app_client(metadata={"allow": allow}) as client:
+ for path in (
+ "/",
+ "/fixtures",
+ "/fixtures/compound_three_primary_keys",
+ "/fixtures/compound_three_primary_keys/a,a,a",
+ ):
+ anon_response = client.get(path)
+ assert expected_anon == anon_response.status
+ auth_response = client.get(
+ path, cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
+ )
+ assert expected_auth == auth_response.status
From cd92e4fe2a47039a8c780e4e7183a0d2e7446884 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Sun, 7 Jun 2020 14:33:52 -0700
Subject: [PATCH 0049/1855] Fixed test name, this executes view-query, not
execute-sql - refs #811
---
tests/test_permissions.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index b5c2e00c..bf66bc9c 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -6,7 +6,7 @@ import pytest
"allow,expected_anon,expected_auth",
[(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
)
-def test_execute_sql(allow, expected_anon, expected_auth):
+def test_view_query(allow, expected_anon, expected_auth):
with make_app_client(
metadata={
"databases": {
From 613fa551a1be31645deb0ece4b46638c181827e0 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Sun, 7 Jun 2020 20:14:27 -0700
Subject: [PATCH 0050/1855] Removed view-row permission, for the moment - refs
#811
https://github.com/simonw/datasette/issues/811#issuecomment-640338347
---
datasette/views/table.py | 3 ---
docs/authentication.rst | 13 -------------
tests/test_html.py | 1 -
3 files changed, 17 deletions(-)
diff --git a/datasette/views/table.py b/datasette/views/table.py
index 10d6725a..935fed3d 100644
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@@ -851,9 +851,6 @@ class RowView(RowTableShared):
await self.check_permission(request, "view-instance")
await self.check_permission(request, "view-database", "database", database)
await self.check_permission(request, "view-table", "table", (database, table))
- await self.check_permission(
- request, "view-row", "row", tuple([database, table] + list(pk_values))
- )
db = self.ds.databases[database]
pks = await db.primary_keys(table)
use_rowid = not pks
diff --git a/docs/authentication.rst b/docs/authentication.rst
index 1bf2a1a5..2caca66f 100644
--- a/docs/authentication.rst
+++ b/docs/authentication.rst
@@ -206,19 +206,6 @@ Actor is allowed to view a table (or view) page, e.g. https://latest.datasette.i
``resource_identifier`` - tuple: (string, string)
The name of the database, then the name of the table
-.. _permissions_view_row:
-
-view-row
---------
-
-Actor is allowed to view a row page, e.g. https://latest.datasette.io/fixtures/compound_primary_key/a,b
-
-``resource_type`` - string
- "row"
-
-``resource_identifier`` - tuple: (string, string, strings...)
- The name of the database, then the name of the table, then the primary key of the row. The primary key may be a single value or multiple values, so the ``resource_identifier`` tuple may be three or more items long.
-
.. _permissions_view_query:
view-query
diff --git a/tests/test_html.py b/tests/test_html.py
index 4e913bcf..e05640d7 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -210,7 +210,6 @@ def test_row_page_does_not_truncate():
[
"view-instance",
("view-table", "table", ("fixtures", "facetable")),
- ("view-row", "row", ("fixtures", "facetable", "1")),
],
)
table = Soup(response.body, "html.parser").find("table")
From 9b42e1a4f5902fb7d6ad0111189900e2656ffda3 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Sun, 7 Jun 2020 20:50:37 -0700
Subject: [PATCH 0051/1855] view-database permission
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Also now using š to indicate private resources - resources that
would not be available to the anonymous user. Refs #811
---
datasette/default_permissions.py | 7 +++++-
datasette/templates/database.html | 2 +-
datasette/templates/index.html | 2 +-
datasette/views/database.py | 3 +--
datasette/views/index.py | 19 +++++++++++++++-
tests/test_canned_write.py | 11 +++++-----
tests/test_html.py | 5 +----
tests/test_permissions.py | 36 +++++++++++++++++++++++++++++++
8 files changed, 69 insertions(+), 16 deletions(-)
diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py
index ee182c85..40be8d34 100644
--- a/datasette/default_permissions.py
+++ b/datasette/default_permissions.py
@@ -11,6 +11,12 @@ def permission_allowed(datasette, actor, action, resource_type, resource_identif
allow = datasette.metadata("allow")
if allow is not None:
return actor_matches_allow(actor, allow)
+ elif action == "view-database":
+ assert resource_type == "database"
+ database_allow = datasette.metadata("allow", database=resource_identifier)
+ if database_allow is None:
+ return True
+ return actor_matches_allow(actor, database_allow)
elif action == "view-query":
# Check if this query has a "allow" block in metadata
assert resource_type == "query"
@@ -20,7 +26,6 @@ def permission_allowed(datasette, actor, action, resource_type, resource_identif
if isinstance(queries_metadata[query_name], str):
return True
allow = queries_metadata[query_name].get("allow")
- print("checking allow - actor = {}, allow = {}".format(actor, allow))
if allow is None:
return True
return actor_matches_allow(actor, allow)
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index fc88003c..eaebfdf7 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -60,7 +60,7 @@
Queries
{% for query in queries %}
- - {{ query.title or query.name }} {% if query.requires_auth %} - requires authentication{% endif %}
+ - {{ query.title or query.name }} {% if query.private %} š{% endif %}
{% endfor %}
{% endif %}
diff --git a/datasette/templates/index.html b/datasette/templates/index.html
index b394564a..3b8568b3 100644
--- a/datasette/templates/index.html
+++ b/datasette/templates/index.html
@@ -10,7 +10,7 @@
{% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
{% for database in databases %}
- {{ database.name }}
+ {{ database.name }}{% if database.private %} š{% endif %}
{% if database.show_table_row_counts %}{{ "{:,}".format(database.table_rows_sum) }} rows in {% endif %}{{ database.tables_count }} table{% if database.tables_count != 1 %}s{% endif %}{% if database.tables_count and database.hidden_tables_count %}, {% endif -%}
{% if database.hidden_tables_count -%}
diff --git a/datasette/views/database.py b/datasette/views/database.py
index 961ab61e..4804b2a9 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -58,8 +58,7 @@ class DatabaseView(DataView):
tables.sort(key=lambda t: (t["hidden"], t["name"]))
canned_queries = [
dict(
- query,
- requires_auth=not actor_matches_allow(None, query.get("allow", None)),
+ query, private=not actor_matches_allow(None, query.get("allow", None)),
)
for query in self.ds.get_canned_queries(database)
if actor_matches_allow(
diff --git a/datasette/views/index.py b/datasette/views/index.py
index 5f903474..7b88028b 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -2,7 +2,7 @@ import hashlib
import json
from datasette.utils import CustomJSONEncoder
-from datasette.utils.asgi import Response
+from datasette.utils.asgi import Response, Forbidden
from datasette.version import __version__
from .base import BaseView
@@ -25,6 +25,22 @@ class IndexView(BaseView):
await self.check_permission(request, "view-instance")
databases = []
for name, db in self.ds.databases.items():
+ # Check permission
+ allowed = await self.ds.permission_allowed(
+ request.scope.get("actor"),
+ "view-database",
+ resource_type="database",
+ resource_identifier=name,
+ default=True,
+ )
+ if not allowed:
+ continue
+ private = not await self.ds.permission_allowed(
+ None,
+ "view-database",
+ resource_type="database",
+ resource_identifier=name,
+ )
table_names = await db.table_names()
hidden_table_names = set(await db.hidden_table_names())
views = await db.view_names()
@@ -95,6 +111,7 @@ class IndexView(BaseView):
),
"hidden_tables_count": len(hidden_tables),
"views_count": len(views),
+ "private": private,
}
)
diff --git a/tests/test_canned_write.py b/tests/test_canned_write.py
index c217be8f..dc3fba3f 100644
--- a/tests/test_canned_write.py
+++ b/tests/test_canned_write.py
@@ -120,13 +120,12 @@ def test_canned_query_permissions_on_database_page(canned_write_client):
)
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": "add_name", "private": False},
+ {"name": "add_name_specify_id", "private": False},
+ {"name": "delete_name", "private": True},
+ {"name": "update_name", "private": False},
] == [
- {"name": q["name"], "requires_auth": q["requires_auth"]}
- for q in response.json["queries"]
+ {"name": q["name"], "private": q["private"]} for q in response.json["queries"]
]
diff --git a/tests/test_html.py b/tests/test_html.py
index e05640d7..3f6dc4df 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -207,10 +207,7 @@ def test_row_page_does_not_truncate():
assert response.status == 200
assert_permissions_checked(
client.ds,
- [
- "view-instance",
- ("view-table", "table", ("fixtures", "facetable")),
- ],
+ ["view-instance", ("view-table", "table", ("fixtures", "facetable")),],
)
table = Soup(response.body, "html.parser").find("table")
assert table["class"] == ["rows-and-columns"]
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index bf66bc9c..21014a25 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -40,3 +40,39 @@ def test_view_instance(allow, expected_anon, expected_auth):
path, cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
)
assert expected_auth == auth_response.status
+
+
+@pytest.mark.parametrize(
+ "allow,expected_anon,expected_auth",
+ [(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
+)
+def test_view_database(allow, expected_anon, expected_auth):
+ with make_app_client(
+ metadata={"databases": {"fixtures": {"allow": allow}}}
+ ) as client:
+ for path in (
+ "/fixtures",
+ "/fixtures/compound_three_primary_keys",
+ "/fixtures/compound_three_primary_keys/a,a,a",
+ ):
+ anon_response = client.get(path)
+ assert expected_anon == anon_response.status
+ auth_response = client.get(
+ path, cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
+ )
+ assert expected_auth == auth_response.status
+
+
+def test_database_list_respects_view_database():
+ with make_app_client(
+ metadata={"databases": {"fixtures": {"allow": {"id": "root"}}}},
+ extra_databases={"data.db": "create table names (name text)"},
+ ) as client:
+ anon_response = client.get("/")
+ assert 'data' in anon_response.text
+ assert 'fixtures' not in anon_response.text
+ auth_response = client.get(
+ "/", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
+ )
+ assert 'data' in auth_response.text
+ assert 'fixtures š' in auth_response.text
From b26292a4582ea7fe16c59d0ac99f3bd8c3d4b1d0 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Sun, 7 Jun 2020 20:56:49 -0700
Subject: [PATCH 0052/1855] Test that view-query is respected by query list,
refs #811
---
datasette/templates/database.html | 2 +-
tests/test_permissions.py | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index eaebfdf7..dfafc049 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -60,7 +60,7 @@
Queries
{% for query in queries %}
- - {{ query.title or query.name }} {% if query.private %} š{% endif %}
+ - {{ query.title or query.name }}{% if query.private %} š{% endif %}
{% endfor %}
{% endif %}
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 21014a25..e66b9291 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -22,6 +22,26 @@ def test_view_query(allow, expected_anon, expected_auth):
assert expected_auth == auth_response.status
+def test_query_list_respects_view_query():
+ with make_app_client(
+ metadata={
+ "databases": {
+ "fixtures": {
+ "queries": {"q": {"sql": "select 1 + 1", "allow": {"id": "root"}}}
+ }
+ }
+ }
+ ) as client:
+ html_fragment = 'q š '
+ anon_response = client.get("/fixtures")
+ assert html_fragment not in anon_response.text
+ assert '"/fixtures/q"' not in anon_response.text
+ auth_response = client.get(
+ "/fixtures", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
+ )
+ assert html_fragment in auth_response.text
+
+
@pytest.mark.parametrize(
"allow,expected_anon,expected_auth",
[(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
From 9397d718345c4b35d2a5c55bfcbd1468876b5ab9 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Sun, 7 Jun 2020 21:47:22 -0700
Subject: [PATCH 0053/1855] Implemented view-table, refs #811
---
datasette/default_permissions.py | 8 ++
datasette/templates/database.html | 2 +-
datasette/views/database.py | 16 ++++
tests/test_permissions.py | 123 ++++++++++++++++++++----------
4 files changed, 108 insertions(+), 41 deletions(-)
diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py
index 40be8d34..dd1770a3 100644
--- a/datasette/default_permissions.py
+++ b/datasette/default_permissions.py
@@ -17,6 +17,14 @@ def permission_allowed(datasette, actor, action, resource_type, resource_identif
if database_allow is None:
return True
return actor_matches_allow(actor, database_allow)
+ elif action == "view-table":
+ assert resource_type == "table"
+ database, table = resource_identifier
+ tables = datasette.metadata("tables", database=database) or {}
+ table_allow = (tables.get(table) or {}).get("allow")
+ if table_allow is None:
+ return True
+ return actor_matches_allow(actor, table_allow)
elif action == "view-query":
# Check if this query has a "allow" block in metadata
assert resource_type == "query"
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index dfafc049..1187267d 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -36,7 +36,7 @@
{% for table in tables %}
{% if show_hidden or not table.hidden %}
- {{ table.name }}{% if table.hidden %} (hidden){% endif %}
+ {{ table.name }}{% if table.private %} š{% endif %}{% if table.hidden %} (hidden){% endif %}
{% for column in table.columns[:9] %}{{ column }}{% if not loop.last %}, {% endif %}{% endfor %}{% if table.columns|length > 9 %}...{% endif %}
{% if table.count is none %}Many rows{% else %}{{ "{:,}".format(table.count) }} row{% if table.count == 1 %}{% else %}s{% endif %}{% endif %}
diff --git a/datasette/views/database.py b/datasette/views/database.py
index 4804b2a9..ba3d22d9 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -42,6 +42,21 @@ class DatabaseView(DataView):
tables = []
for table in table_counts:
+ allowed = await self.ds.permission_allowed(
+ request.scope.get("actor"),
+ "view-table",
+ resource_type="table",
+ resource_identifier=(database, table),
+ default=True,
+ )
+ if not allowed:
+ continue
+ private = not await self.ds.permission_allowed(
+ None,
+ "view-table",
+ resource_type="table",
+ resource_identifier=(database, table),
+ )
table_columns = await db.table_columns(table)
tables.append(
{
@@ -52,6 +67,7 @@ class DatabaseView(DataView):
"hidden": table in hidden_table_names,
"fts_table": await db.fts_table(table),
"foreign_keys": all_foreign_keys[table],
+ "private": private,
}
)
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index e66b9291..7c5b02c0 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -2,46 +2,6 @@ from .fixtures import make_app_client
import pytest
-@pytest.mark.parametrize(
- "allow,expected_anon,expected_auth",
- [(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
-)
-def test_view_query(allow, expected_anon, expected_auth):
- with make_app_client(
- metadata={
- "databases": {
- "fixtures": {"queries": {"q": {"sql": "select 1 + 1", "allow": allow}}}
- }
- }
- ) as client:
- anon_response = client.get("/fixtures/q")
- assert expected_anon == anon_response.status
- auth_response = client.get(
- "/fixtures/q", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
- )
- assert expected_auth == auth_response.status
-
-
-def test_query_list_respects_view_query():
- with make_app_client(
- metadata={
- "databases": {
- "fixtures": {
- "queries": {"q": {"sql": "select 1 + 1", "allow": {"id": "root"}}}
- }
- }
- }
- ) as client:
- html_fragment = 'q š '
- anon_response = client.get("/fixtures")
- assert html_fragment not in anon_response.text
- assert '"/fixtures/q"' not in anon_response.text
- auth_response = client.get(
- "/fixtures", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
- )
- assert html_fragment in auth_response.text
-
-
@pytest.mark.parametrize(
"allow,expected_anon,expected_auth",
[(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
@@ -96,3 +56,86 @@ def test_database_list_respects_view_database():
)
assert 'data' in auth_response.text
assert 'fixtures š' in auth_response.text
+
+
+@pytest.mark.parametrize(
+ "allow,expected_anon,expected_auth",
+ [(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
+)
+def test_view_table(allow, expected_anon, expected_auth):
+ with make_app_client(
+ metadata={
+ "databases": {
+ "fixtures": {
+ "tables": {"compound_three_primary_keys": {"allow": allow}}
+ }
+ }
+ }
+ ) as client:
+ anon_response = client.get("/fixtures/compound_three_primary_keys")
+ assert expected_anon == anon_response.status
+ auth_response = client.get(
+ "/fixtures/compound_three_primary_keys",
+ cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
+ )
+ assert expected_auth == auth_response.status
+
+
+def test_table_list_respects_view_table():
+ with make_app_client(
+ metadata={
+ "databases": {
+ "fixtures": {
+ "tables": {"compound_three_primary_keys": {"allow": {"id": "root"}}}
+ }
+ }
+ }
+ ) as client:
+ html_fragment = 'compound_three_primary_keys š'
+ anon_response = client.get("/fixtures")
+ assert html_fragment not in anon_response.text
+ assert '"/fixtures/compound_three_primary_keys"' not in anon_response.text
+ auth_response = client.get(
+ "/fixtures", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
+ )
+ assert html_fragment in auth_response.text
+
+
+@pytest.mark.parametrize(
+ "allow,expected_anon,expected_auth",
+ [(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
+)
+def test_view_query(allow, expected_anon, expected_auth):
+ with make_app_client(
+ metadata={
+ "databases": {
+ "fixtures": {"queries": {"q": {"sql": "select 1 + 1", "allow": allow}}}
+ }
+ }
+ ) as client:
+ anon_response = client.get("/fixtures/q")
+ assert expected_anon == anon_response.status
+ auth_response = client.get(
+ "/fixtures/q", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
+ )
+ assert expected_auth == auth_response.status
+
+
+def test_query_list_respects_view_query():
+ with make_app_client(
+ metadata={
+ "databases": {
+ "fixtures": {
+ "queries": {"q": {"sql": "select 1 + 1", "allow": {"id": "root"}}}
+ }
+ }
+ }
+ ) as client:
+ html_fragment = 'q š '
+ anon_response = client.get("/fixtures")
+ assert html_fragment not in anon_response.text
+ assert '"/fixtures/q"' not in anon_response.text
+ auth_response = client.get(
+ "/fixtures", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
+ )
+ assert html_fragment in auth_response.text
From e18f8c3f871fe1e9e00554b5c6c75409cc1a5e6d Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 06:49:55 -0700
Subject: [PATCH 0054/1855] New check_visibility() utility function, refs #811
---
datasette/utils/__init__.py | 23 +++++++++++++++++++++++
datasette/views/database.py | 35 ++++++++++++++++-------------------
datasette/views/index.py | 19 ++++---------------
3 files changed, 43 insertions(+), 34 deletions(-)
diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py
index 077728f4..3d964049 100644
--- a/datasette/utils/__init__.py
+++ b/datasette/utils/__init__.py
@@ -874,3 +874,26 @@ def actor_matches_allow(actor, allow):
if actor_values.intersection(values):
return True
return False
+
+
+async def check_visibility(
+ datasette, actor, action, resource_type, resource_identifier, default=True
+):
+ "Returns (visible, private) - visible = can you see it, private = can others see it too"
+ visible = await datasette.permission_allowed(
+ actor,
+ action,
+ resource_type=resource_type,
+ resource_identifier=resource_identifier,
+ default=default,
+ )
+ if not visible:
+ return (False, False)
+ private = not await datasette.permission_allowed(
+ None,
+ action,
+ resource_type=resource_type,
+ resource_identifier=resource_identifier,
+ default=default,
+ )
+ return visible, private
diff --git a/datasette/views/database.py b/datasette/views/database.py
index ba3d22d9..afbb6b05 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -3,6 +3,7 @@ import jinja2
from datasette.utils import (
actor_matches_allow,
+ check_visibility,
to_css_class,
validate_sql_select,
is_url,
@@ -42,21 +43,15 @@ class DatabaseView(DataView):
tables = []
for table in table_counts:
- allowed = await self.ds.permission_allowed(
+ visible, private = await check_visibility(
+ self.ds,
request.scope.get("actor"),
"view-table",
- resource_type="table",
- resource_identifier=(database, table),
- default=True,
+ "table",
+ (database, table),
)
- if not allowed:
+ if not visible:
continue
- private = not await self.ds.permission_allowed(
- None,
- "view-table",
- resource_type="table",
- resource_identifier=(database, table),
- )
table_columns = await db.table_columns(table)
tables.append(
{
@@ -72,15 +67,17 @@ class DatabaseView(DataView):
)
tables.sort(key=lambda t: (t["hidden"], t["name"]))
- canned_queries = [
- dict(
- query, private=not actor_matches_allow(None, query.get("allow", None)),
+ canned_queries = []
+ for query in self.ds.get_canned_queries(database):
+ visible, private = await check_visibility(
+ self.ds,
+ request.scope.get("actor"),
+ "view-query",
+ "query",
+ (database, query["name"]),
)
- for query in self.ds.get_canned_queries(database)
- if actor_matches_allow(
- request.scope.get("actor", None), query.get("allow", None)
- )
- ]
+ if visible:
+ canned_queries.append(dict(query, private=private))
return (
{
"database": database,
diff --git a/datasette/views/index.py b/datasette/views/index.py
index 7b88028b..0f7fb613 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -1,7 +1,7 @@
import hashlib
import json
-from datasette.utils import CustomJSONEncoder
+from datasette.utils import check_visibility, CustomJSONEncoder
from datasette.utils.asgi import Response, Forbidden
from datasette.version import __version__
@@ -25,22 +25,11 @@ class IndexView(BaseView):
await self.check_permission(request, "view-instance")
databases = []
for name, db in self.ds.databases.items():
- # Check permission
- allowed = await self.ds.permission_allowed(
- request.scope.get("actor"),
- "view-database",
- resource_type="database",
- resource_identifier=name,
- default=True,
+ visible, private = await check_visibility(
+ self.ds, request.scope.get("actor"), "view-database", "database", name,
)
- if not allowed:
+ if not visible:
continue
- private = not await self.ds.permission_allowed(
- None,
- "view-database",
- resource_type="database",
- resource_identifier=name,
- )
table_names = await db.table_names()
hidden_table_names = set(await db.hidden_table_names())
views = await db.view_names()
From cc218fa9be55842656d030545c308392e3736053 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 07:02:31 -0700
Subject: [PATCH 0055/1855] Move assert_permissions_checked() calls from
test_html.py to test_permissions.py, refs #811
---
datasette/app.py | 2 +-
tests/test_html.py | 49 ------------------------------------
tests/test_permissions.py | 52 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 52 insertions(+), 51 deletions(-)
diff --git a/datasette/app.py b/datasette/app.py
index f433a10a..23c293c9 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -298,7 +298,7 @@ class Datasette:
pm.hook.prepare_jinja2_environment(env=self.jinja_env)
self._register_renderers()
- self._permission_checks = collections.deque(maxlen=30)
+ self._permission_checks = collections.deque(maxlen=200)
self._root_token = os.urandom(32).hex()
def sign(self, value, namespace="default"):
diff --git a/tests/test_html.py b/tests/test_html.py
index 3f6dc4df..cb0e0c90 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -4,7 +4,6 @@ from .fixtures import ( # noqa
app_client_shorter_time_limit,
app_client_two_attached_databases,
app_client_with_hash,
- assert_permissions_checked,
make_app_client,
METADATA,
)
@@ -18,7 +17,6 @@ import urllib.parse
def test_homepage(app_client_two_attached_databases):
response = app_client_two_attached_databases.get("/")
- assert_permissions_checked(app_client_two_attached_databases.ds, ["view-instance"])
assert response.status == 200
assert "text/html; charset=utf-8" == response.headers["content-type"]
soup = Soup(response.body, "html.parser")
@@ -77,9 +75,6 @@ def test_static_mounts():
def test_memory_database_page():
with make_app_client(memory=True) as client:
response = client.get("/:memory:")
- assert_permissions_checked(
- client.ds, ["view-instance", ("view-database", "database", ":memory:")]
- )
assert response.status == 200
@@ -92,9 +87,6 @@ def test_database_page_redirects_with_url_hash(app_client_with_hash):
def test_database_page(app_client):
response = app_client.get("/fixtures")
- assert_permissions_checked(
- app_client.ds, ["view-instance", ("view-database", "database", "fixtures")]
- )
soup = Soup(response.body, "html.parser")
queries_ul = soup.find("h2", text="Queries").find_next_sibling("ul")
assert queries_ul is not None
@@ -205,10 +197,6 @@ def test_row_page_does_not_truncate():
with make_app_client(config={"truncate_cells_html": 5}) as client:
response = client.get("/fixtures/facetable/1")
assert response.status == 200
- assert_permissions_checked(
- client.ds,
- ["view-instance", ("view-table", "table", ("fixtures", "facetable")),],
- )
table = Soup(response.body, "html.parser").find("table")
assert table["class"] == ["rows-and-columns"]
assert ["Mission"] == [
@@ -518,14 +506,6 @@ def test_templates_considered(app_client, path, expected_considered):
def test_table_html_simple_primary_key(app_client):
response = app_client.get("/fixtures/simple_primary_key?_size=3")
- assert_permissions_checked(
- app_client.ds,
- [
- "view-instance",
- ("view-database", "database", "fixtures"),
- ("view-table", "table", ("fixtures", "simple_primary_key")),
- ],
- )
assert response.status == 200
table = Soup(response.body, "html.parser").find("table")
assert table["class"] == ["rows-and-columns"]
@@ -881,19 +861,6 @@ def test_database_metadata(app_client):
assert_footer_links(soup)
-def test_database_query_permission_checks(app_client):
- response = app_client.get("/fixtures?sql=select+1")
- assert response.status == 200
- assert_permissions_checked(
- app_client.ds,
- [
- "view-instance",
- ("view-database", "database", "fixtures"),
- ("execute-sql", "database", "fixtures"),
- ],
- )
-
-
def test_database_metadata_with_custom_sql(app_client):
response = app_client.get("/fixtures?sql=select+*+from+simple_primary_key")
assert response.status == 200
@@ -929,14 +896,6 @@ def test_database_download_allowed_for_immutable():
assert len(soup.findAll("a", {"href": re.compile(r"\.db$")}))
# Check we can actually download it
assert 200 == client.get("/fixtures.db").status
- assert_permissions_checked(
- client.ds,
- [
- "view-instance",
- ("view-database", "database", "fixtures"),
- ("view-database-download", "database", "fixtures"),
- ],
- )
def test_database_download_disallowed_for_mutable(app_client):
@@ -1032,14 +991,6 @@ def test_404_content_type(app_client):
def test_canned_query_with_custom_metadata(app_client):
response = app_client.get("/fixtures/neighborhood_search?text=town")
- assert_permissions_checked(
- app_client.ds,
- [
- "view-instance",
- ("view-database", "database", "fixtures"),
- ("view-query", "query", ("fixtures", "neighborhood_search")),
- ],
- )
assert response.status == 200
soup = Soup(response.body, "html.parser")
assert "Search neighborhoods" == soup.find("h1").text
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 7c5b02c0..df905aa1 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -1,4 +1,4 @@
-from .fixtures import make_app_client
+from .fixtures import app_client, assert_permissions_checked, make_app_client
import pytest
@@ -139,3 +139,53 @@ def test_query_list_respects_view_query():
"/fixtures", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
)
assert html_fragment in auth_response.text
+
+
+@pytest.mark.parametrize(
+ "path,permissions",
+ [
+ ("/", ["view-instance"]),
+ ("/fixtures", ["view-instance", ("view-database", "database", "fixtures")]),
+ (
+ "/fixtures/facetable/1",
+ ["view-instance", ("view-table", "table", ("fixtures", "facetable"))],
+ ),
+ (
+ "/fixtures/simple_primary_key",
+ [
+ "view-instance",
+ ("view-database", "database", "fixtures"),
+ ("view-table", "table", ("fixtures", "simple_primary_key")),
+ ],
+ ),
+ (
+ "/fixtures?sql=select+1",
+ [
+ "view-instance",
+ ("view-database", "database", "fixtures"),
+ ("execute-sql", "database", "fixtures"),
+ ],
+ ),
+ (
+ "/fixtures.db",
+ [
+ "view-instance",
+ ("view-database", "database", "fixtures"),
+ ("view-database-download", "database", "fixtures"),
+ ],
+ ),
+ (
+ "/fixtures/neighborhood_search",
+ [
+ "view-instance",
+ ("view-database", "database", "fixtures"),
+ ("view-query", "query", ("fixtures", "neighborhood_search")),
+ ],
+ ),
+ ],
+)
+def test_permissions_checked(app_client, path, permissions):
+ app_client.ds._permission_checks.clear()
+ response = app_client.get(path)
+ assert response.status in (200, 403)
+ assert_permissions_checked(app_client.ds, permissions)
From 1cf86e5eccf3f92b483bacbad860879cf39b0ad6 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 07:18:37 -0700
Subject: [PATCH 0056/1855] Show padlock on private index page, refs #811
---
datasette/templates/index.html | 2 +-
datasette/views/index.py | 3 +++
tests/test_permissions.py | 6 ++++++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/datasette/templates/index.html b/datasette/templates/index.html
index 3b8568b3..5a8dccae 100644
--- a/datasette/templates/index.html
+++ b/datasette/templates/index.html
@@ -5,7 +5,7 @@
{% block body_class %}index{% endblock %}
{% block content %}
-{{ metadata.title or "Datasette" }}
+{{ metadata.title or "Datasette" }}{% if private %} š{% endif %}
{% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
diff --git a/datasette/views/index.py b/datasette/views/index.py
index 0f7fb613..8cbe28f0 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -121,5 +121,8 @@ class IndexView(BaseView):
"databases": databases,
"metadata": self.ds.metadata(),
"datasette_version": __version__,
+ "private": not await self.ds.permission_allowed(
+ None, "view-instance"
+ ),
},
)
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index df905aa1..5dcf46ad 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -16,10 +16,16 @@ def test_view_instance(allow, expected_anon, expected_auth):
):
anon_response = client.get(path)
assert expected_anon == anon_response.status
+ if allow and path == "/" and anon_response.status == 200:
+ # Should be no padlock
+ assert "Datasette š
" not in anon_response.text
auth_response = client.get(
path, cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
)
assert expected_auth == auth_response.status
+ # Check for the padlock
+ if allow and path == "/" and expected_anon == 403 and expected_auth == 200:
+ assert "Datasette š
" in auth_response.text
@pytest.mark.parametrize(
From 3ce7f2e7dae010de97b67618c111ea5853164a69 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 07:23:10 -0700
Subject: [PATCH 0057/1855] Show padlock on private database page, refs #811
---
datasette/templates/database.html | 2 +-
datasette/views/database.py | 3 +++
tests/test_permissions.py | 10 ++++++++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index 1187267d..089142e2 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -18,7 +18,7 @@
{% block content %}
-{{ metadata.title or database }}
+{{ metadata.title or database }}{% if private %} š{% endif %}
{% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
diff --git a/datasette/views/database.py b/datasette/views/database.py
index afbb6b05..2d7e6b31 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -86,6 +86,9 @@ class DatabaseView(DataView):
"hidden_count": len([t for t in tables if t["hidden"]]),
"views": views,
"queries": canned_queries,
+ "private": not await self.ds.permission_allowed(
+ None, "view-database", "database", database
+ ),
},
{
"show_hidden": request.args.get("_show_hidden"),
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 5dcf46ad..d76d1e15 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -43,10 +43,20 @@ def test_view_database(allow, expected_anon, expected_auth):
):
anon_response = client.get(path)
assert expected_anon == anon_response.status
+ if allow and path == "/fixtures" and anon_response.status == 200:
+ # Should be no padlock
+ assert ">fixtures š" not in anon_response.text
auth_response = client.get(
path, cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
)
assert expected_auth == auth_response.status
+ if (
+ allow
+ and path == "/fixtures"
+ and expected_anon == 403
+ and expected_auth == 200
+ ):
+ assert ">fixtures š" in auth_response.text
def test_database_list_respects_view_database():
From 2a8b39800f194925658bd9e1b5e4cc12619d5e9c Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 07:50:06 -0700
Subject: [PATCH 0058/1855] Updated tests, refs #811
---
tests/test_api.py | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/tests/test_api.py b/tests/test_api.py
index 22378946..13a98b6a 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -70,6 +70,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "Table With Space In Name",
@@ -79,6 +80,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "attraction_characteristic",
@@ -97,6 +99,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "binary_data",
@@ -106,6 +109,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "complex_foreign_keys",
@@ -134,6 +138,7 @@ def test_database_page(app_client):
},
],
},
+ "private": False,
},
{
"name": "compound_primary_key",
@@ -143,6 +148,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "compound_three_primary_keys",
@@ -152,6 +158,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "custom_foreign_key_label",
@@ -170,6 +177,7 @@ def test_database_page(app_client):
}
],
},
+ "private": False,
},
{
"name": "facet_cities",
@@ -188,6 +196,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "facetable",
@@ -217,6 +226,7 @@ def test_database_page(app_client):
}
],
},
+ "private": False,
},
{
"name": "foreign_key_references",
@@ -240,6 +250,7 @@ def test_database_page(app_client):
},
],
},
+ "private": False,
},
{
"name": "infinity",
@@ -249,6 +260,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "primary_key_multiple_columns",
@@ -267,6 +279,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "primary_key_multiple_columns_explicit_label",
@@ -285,6 +298,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "roadside_attraction_characteristics",
@@ -308,6 +322,7 @@ def test_database_page(app_client):
},
],
},
+ "private": False,
},
{
"name": "roadside_attractions",
@@ -326,6 +341,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "searchable",
@@ -344,6 +360,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "searchable_tags",
@@ -363,6 +380,7 @@ def test_database_page(app_client):
},
],
},
+ "private": False,
},
{
"name": "select",
@@ -372,6 +390,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "simple_primary_key",
@@ -405,6 +424,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "sortable",
@@ -422,6 +442,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "table/with/slashes.csv",
@@ -431,6 +452,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "tags",
@@ -449,6 +471,7 @@ def test_database_page(app_client):
],
"outgoing": [],
},
+ "private": False,
},
{
"name": "units",
@@ -458,6 +481,7 @@ def test_database_page(app_client):
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "no_primary_key",
@@ -467,6 +491,7 @@ def test_database_page(app_client):
"hidden": True,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "searchable_fts",
@@ -476,6 +501,7 @@ def test_database_page(app_client):
"hidden": True,
"fts_table": "searchable_fts",
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "searchable_fts_content",
@@ -491,6 +517,7 @@ def test_database_page(app_client):
"hidden": True,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "searchable_fts_segdir",
@@ -507,6 +534,7 @@ def test_database_page(app_client):
"hidden": True,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
{
"name": "searchable_fts_segments",
@@ -516,6 +544,7 @@ def test_database_page(app_client):
"hidden": True,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
+ "private": False,
},
] == data["tables"]
@@ -537,6 +566,7 @@ def test_no_files_uses_memory_database(app_client_no_files):
"tables_and_views_more": False,
"tables_and_views_truncated": [],
"views_count": 0,
+ "private": False,
}
} == response.json
# Try that SQL query
From 177059284dc953e6c76f86213aa470db2ff3eaca Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 10:05:32 -0700
Subject: [PATCH 0059/1855] New request.actor property, refs #811
---
datasette/app.py | 2 +-
datasette/utils/asgi.py | 4 ++++
datasette/views/base.py | 2 +-
datasette/views/database.py | 4 ++--
datasette/views/index.py | 2 +-
datasette/views/special.py | 2 +-
docs/authentication.rst | 2 ++
docs/internals.rst | 5 ++++-
8 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/datasette/app.py b/datasette/app.py
index 23c293c9..87e542c1 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -667,7 +667,7 @@ class Datasette:
return d
def _actor(self, request):
- return {"actor": request.scope.get("actor", None)}
+ return {"actor": request.actor}
def table_metadata(self, database, table):
"Fetch table-specific metadata."
diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py
index fa78c8df..bca9c9ab 100644
--- a/datasette/utils/asgi.py
+++ b/datasette/utils/asgi.py
@@ -74,6 +74,10 @@ class Request:
def args(self):
return MultiParams(parse_qs(qs=self.query_string))
+ @property
+ def actor(self):
+ return self.scope.get("actor", None)
+
async def post_vars(self):
body = []
body = b""
diff --git a/datasette/views/base.py b/datasette/views/base.py
index 9c2cbbcc..000d354b 100644
--- a/datasette/views/base.py
+++ b/datasette/views/base.py
@@ -68,7 +68,7 @@ class BaseView(AsgiView):
self, request, action, resource_type=None, resource_identifier=None
):
ok = await self.ds.permission_allowed(
- request.scope.get("actor"),
+ request.actor,
action,
resource_type=resource_type,
resource_identifier=resource_identifier,
diff --git a/datasette/views/database.py b/datasette/views/database.py
index 2d7e6b31..dee6c9c8 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -45,7 +45,7 @@ class DatabaseView(DataView):
for table in table_counts:
visible, private = await check_visibility(
self.ds,
- request.scope.get("actor"),
+ request.actor,
"view-table",
"table",
(database, table),
@@ -71,7 +71,7 @@ class DatabaseView(DataView):
for query in self.ds.get_canned_queries(database):
visible, private = await check_visibility(
self.ds,
- request.scope.get("actor"),
+ request.actor,
"view-query",
"query",
(database, query["name"]),
diff --git a/datasette/views/index.py b/datasette/views/index.py
index 8cbe28f0..609bfa6a 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -26,7 +26,7 @@ class IndexView(BaseView):
databases = []
for name, db in self.ds.databases.items():
visible, private = await check_visibility(
- self.ds, request.scope.get("actor"), "view-database", "database", name,
+ self.ds, request.actor, "view-database", "database", name,
)
if not visible:
continue
diff --git a/datasette/views/special.py b/datasette/views/special.py
index 37c04697..b8bd57c6 100644
--- a/datasette/views/special.py
+++ b/datasette/views/special.py
@@ -86,7 +86,7 @@ class PermissionsDebugView(BaseView):
async def get(self, request):
if not await self.ds.permission_allowed(
- request.scope.get("actor"), "permissions-debug"
+ request.actor, "permissions-debug"
):
return Response("Permission denied", status=403)
return await self.render(
diff --git a/docs/authentication.rst b/docs/authentication.rst
index 2caca66f..bda6a0b7 100644
--- a/docs/authentication.rst
+++ b/docs/authentication.rst
@@ -140,6 +140,8 @@ Plugins that wish to implement the same permissions scheme as canned queries can
actor_matches_allow({"id": "root"}, {"id": "*"})
# returns True
+The currently authenticated actor is made available to plugins as ``request.actor``.
+
.. _PermissionsDebugView:
Permissions Debug
diff --git a/docs/internals.rst b/docs/internals.rst
index 25b2d875..7498f017 100644
--- a/docs/internals.rst
+++ b/docs/internals.rst
@@ -42,6 +42,9 @@ The request object is passed to various plugin hooks. It represents an incoming
``.args`` - MultiParams
An object representing the parsed querystring parameters, see below.
+``.actor`` - dictionary (str -> Any) or None
+ The currently authenticated actor (see :ref:`actors `), or ``None`` if the request is unauthenticated.
+
The object also has one awaitable method:
``await request.post_vars()`` - dictionary
@@ -122,7 +125,7 @@ await .permission_allowed(actor, action, resource_type=None, resource_identifier
-----------------------------------------------------------------------------------------------------
``actor`` - dictionary
- The authenticated actor. This is usually ``request.scope.get("actor")``.
+ The authenticated actor. This is usually ``request.actor``.
``action`` - string
The name of the action that is being permission checked.
From ab14b20b248dafbe7f9f9487985614939c83b517 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 10:16:24 -0700
Subject: [PATCH 0060/1855] Get tests working again
---
datasette/views/database.py | 6 +-----
datasette/views/index.py | 2 +-
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/datasette/views/database.py b/datasette/views/database.py
index dee6c9c8..6f6404a7 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -44,11 +44,7 @@ class DatabaseView(DataView):
tables = []
for table in table_counts:
visible, private = await check_visibility(
- self.ds,
- request.actor,
- "view-table",
- "table",
- (database, table),
+ self.ds, request.actor, "view-table", "table", (database, table),
)
if not visible:
continue
diff --git a/datasette/views/index.py b/datasette/views/index.py
index 609bfa6a..59d3e042 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -122,7 +122,7 @@ class IndexView(BaseView):
"metadata": self.ds.metadata(),
"datasette_version": __version__,
"private": not await self.ds.permission_allowed(
- None, "view-instance"
+ None, "view-instance", default=True
),
},
)
From dfff34e1987976e72f58ee7b274952840b1f4b71 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 11:03:33 -0700
Subject: [PATCH 0061/1855] Applied black, refs #811
---
datasette/views/special.py | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/datasette/views/special.py b/datasette/views/special.py
index b8bd57c6..7a5fbe21 100644
--- a/datasette/views/special.py
+++ b/datasette/views/special.py
@@ -85,9 +85,7 @@ class PermissionsDebugView(BaseView):
self.ds = datasette
async def get(self, request):
- if not await self.ds.permission_allowed(
- request.actor, "permissions-debug"
- ):
+ if not await self.ds.permission_allowed(request.actor, "permissions-debug"):
return Response("Permission denied", status=403)
return await self.render(
["permissions_debug.html"],
From aa420009c08921d0c9a68cf60a57959be0e8a2e5 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 11:07:11 -0700
Subject: [PATCH 0062/1855] Show padlock on private table page, refs #811
---
datasette/templates/table.html | 2 +-
datasette/views/table.py | 5 +++++
tests/test_permissions.py | 5 +++++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/datasette/templates/table.html b/datasette/templates/table.html
index fa6766a8..1289e125 100644
--- a/datasette/templates/table.html
+++ b/datasette/templates/table.html
@@ -26,7 +26,7 @@
{% block content %}
-{{ metadata.title or table }}{% if is_view %} (view){% endif %}
+{{ metadata.title or table }}{% if is_view %} (view){% endif %}{% if private %} š{% endif %}
{% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
diff --git a/datasette/views/table.py b/datasette/views/table.py
index 935fed3d..cd952568 100644
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@@ -271,6 +271,10 @@ class TableView(RowTableShared):
await self.check_permission(request, "view-database", "database", database)
await self.check_permission(request, "view-table", "table", (database, table))
+ private = not await self.ds.permission_allowed(
+ None, "view-table", "table", (database, table), default=True
+ )
+
pks = await db.primary_keys(table)
table_columns = await db.table_columns(table)
@@ -834,6 +838,7 @@ class TableView(RowTableShared):
"suggested_facets": suggested_facets,
"next": next_value and str(next_value) or None,
"next_url": next_url,
+ "private": private,
},
extra_template,
(
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index d76d1e15..733afd5f 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -90,11 +90,16 @@ def test_view_table(allow, expected_anon, expected_auth):
) as client:
anon_response = client.get("/fixtures/compound_three_primary_keys")
assert expected_anon == anon_response.status
+ if allow and anon_response.status == 200:
+ # Should be no padlock
+ assert ">compound_three_primary_keys š" not in anon_response.text
auth_response = client.get(
"/fixtures/compound_three_primary_keys",
cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
)
assert expected_auth == auth_response.status
+ if allow and expected_anon == 403 and expected_auth == 200:
+ assert ">compound_three_primary_keys š" in auth_response.text
def test_table_list_respects_view_table():
From 9ac27f67fe346e753b562b711a2086e4c616d51d Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 11:13:32 -0700
Subject: [PATCH 0063/1855] Show padlock on private query page, refs #811
---
datasette/templates/query.html | 2 +-
datasette/views/database.py | 6 ++++++
tests/test_permissions.py | 5 +++++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/datasette/templates/query.html b/datasette/templates/query.html
index a7cb6647..7771b101 100644
--- a/datasette/templates/query.html
+++ b/datasette/templates/query.html
@@ -28,7 +28,7 @@
{% block content %}
-{{ metadata.title or database }}
+{{ metadata.title or database }}{% if private %} š{% endif %}
{% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
diff --git a/datasette/views/database.py b/datasette/views/database.py
index 6f6404a7..30817106 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -147,10 +147,14 @@ class QueryView(DataView):
# Respect canned query permissions
await self.check_permission(request, "view-instance")
await self.check_permission(request, "view-database", "database", database)
+ private = False
if canned_query:
await self.check_permission(
request, "view-query", "query", (database, canned_query)
)
+ private = not await self.ds.permission_allowed(
+ None, "view-query", "query", (database, canned_query), default=True
+ )
else:
await self.check_permission(request, "execute-sql", "database", database)
# Extract any :named parameters
@@ -214,6 +218,7 @@ class QueryView(DataView):
"truncated": False,
"columns": [],
"query": {"sql": sql, "params": params},
+ "private": private,
},
extra_template,
templates,
@@ -282,6 +287,7 @@ class QueryView(DataView):
"truncated": results.truncated,
"columns": columns,
"query": {"sql": sql, "params": params},
+ "private": private,
},
extra_template,
templates,
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 733afd5f..55b2d673 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -136,10 +136,15 @@ def test_view_query(allow, expected_anon, expected_auth):
) as client:
anon_response = client.get("/fixtures/q")
assert expected_anon == anon_response.status
+ if allow and anon_response.status == 200:
+ # Should be no padlock
+ assert ">fixtures š" not in anon_response.text
auth_response = client.get(
"/fixtures/q", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
)
assert expected_auth == auth_response.status
+ if allow and expected_anon == 403 and expected_auth == 200:
+ assert ">fixtures š" in auth_response.text
def test_query_list_respects_view_query():
From dcec89270a2e3b9fabed93f1d7b9be3ef86e9ed2 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 11:20:21 -0700
Subject: [PATCH 0064/1855] View list respects view-table permission, refs #811
Also makes a small change to the /fixtures.json JSON:
"views": ["view_name"]
Is now:
"views": [{"name": "view_name", "private": true}]
---
datasette/templates/database.html | 2 +-
datasette/views/database.py | 11 ++++++++++-
tests/test_permissions.py | 18 +++++++++++++-----
3 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index 089142e2..100faee4 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -51,7 +51,7 @@
Views
{% for view in views %}
- - {{ view }}
+ - {{ view.name }}{% if view.private %} š{% endif %}
{% endfor %}
{% endif %}
diff --git a/datasette/views/database.py b/datasette/views/database.py
index 30817106..824cb632 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -37,10 +37,19 @@ class DatabaseView(DataView):
db = self.ds.databases[database]
table_counts = await db.table_counts(5)
- views = await db.view_names()
hidden_table_names = set(await db.hidden_table_names())
all_foreign_keys = await db.get_all_foreign_keys()
+ views = []
+ for view_name in await db.view_names():
+ visible, private = await check_visibility(
+ self.ds, request.actor, "view-table", "table", (database, view_name),
+ )
+ if visible:
+ views.append(
+ {"name": view_name, "private": private,}
+ )
+
tables = []
for table in table_counts:
visible, private = await check_visibility(
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 55b2d673..5c338e04 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -107,19 +107,27 @@ def test_table_list_respects_view_table():
metadata={
"databases": {
"fixtures": {
- "tables": {"compound_three_primary_keys": {"allow": {"id": "root"}}}
+ "tables": {
+ "compound_three_primary_keys": {"allow": {"id": "root"}},
+ # And a SQL view too:
+ "paginated_view": {"allow": {"id": "root"}},
+ }
}
}
}
) as client:
- html_fragment = 'compound_three_primary_keys š'
+ html_fragments = [
+ ">compound_three_primary_keys š",
+ ">paginated_view š",
+ ]
anon_response = client.get("/fixtures")
- assert html_fragment not in anon_response.text
- assert '"/fixtures/compound_three_primary_keys"' not in anon_response.text
+ for html_fragment in html_fragments:
+ assert html_fragment not in anon_response.text
auth_response = client.get(
"/fixtures", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")}
)
- assert html_fragment in auth_response.text
+ for html_fragment in html_fragments:
+ assert html_fragment in auth_response.text
@pytest.mark.parametrize(
From 5598c5de011db95396b65b5c8c251cbe6884d6ae Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 11:34:14 -0700
Subject: [PATCH 0065/1855] Database list on index page respects table/view
permissions, refs #811
---
datasette/templates/index.html | 2 +-
datasette/views/index.py | 25 ++++++++++++++++++++-----
tests/test_permissions.py | 31 +++++++++++++++++++++++++++++++
3 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/datasette/templates/index.html b/datasette/templates/index.html
index 5a8dccae..c1adfc59 100644
--- a/datasette/templates/index.html
+++ b/datasette/templates/index.html
@@ -22,7 +22,7 @@
{% endif %}
{% for table in database.tables_and_views_truncated %}{{ table.name }}{% if not loop.last %}, {% endif %}{% endfor %}{% if database.tables_and_views_more %}, ...{% endif %}
+ }}"{% if table.count %} title="{{ table.count }} rows"{% endif %}>{{ table.name }}{% if table.private %} š{% endif %}{% if not loop.last %}, {% endif %}{% endfor %}{% if database.tables_and_views_more %}, ...{% endif %}
{% endfor %}
{% endblock %}
diff --git a/datasette/views/index.py b/datasette/views/index.py
index 59d3e042..a3e8388c 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -25,14 +25,22 @@ class IndexView(BaseView):
await self.check_permission(request, "view-instance")
databases = []
for name, db in self.ds.databases.items():
- visible, private = await check_visibility(
+ visible, database_private = await check_visibility(
self.ds, request.actor, "view-database", "database", name,
)
if not visible:
continue
table_names = await db.table_names()
hidden_table_names = set(await db.hidden_table_names())
- views = await db.view_names()
+
+ views = []
+ for view_name in await db.view_names():
+ visible, private = await check_visibility(
+ self.ds, request.actor, "view-table", "table", (name, view_name),
+ )
+ if visible:
+ views.append({"name": view_name, "private": private})
+
# Perform counts only for immutable or DBS with <= COUNT_TABLE_LIMIT tables
table_counts = {}
if not db.is_mutable or db.size < COUNT_DB_SIZE_LIMIT:
@@ -40,8 +48,14 @@ class IndexView(BaseView):
# If any of these are None it means at least one timed out - ignore them all
if any(v is None for v in table_counts.values()):
table_counts = {}
+
tables = {}
for table in table_names:
+ visible, private = await check_visibility(
+ self.ds, request.actor, "view-table", "table", (name, table),
+ )
+ if not visible:
+ continue
table_columns = await db.table_columns(table)
tables[table] = {
"name": table,
@@ -51,6 +65,7 @@ class IndexView(BaseView):
"hidden": table in hidden_table_names,
"fts_table": await db.fts_table(table),
"num_relationships_for_sorting": 0,
+ "private": private,
}
if request.args.get("_sort") == "relationships" or not table_counts:
@@ -78,8 +93,8 @@ class IndexView(BaseView):
# Only add views if this is less than TRUNCATE_AT
if len(tables_and_views_truncated) < TRUNCATE_AT:
num_views_to_add = TRUNCATE_AT - len(tables_and_views_truncated)
- for view_name in views[:num_views_to_add]:
- tables_and_views_truncated.append({"name": view_name})
+ for view in views[:num_views_to_add]:
+ tables_and_views_truncated.append(view)
databases.append(
{
@@ -100,7 +115,7 @@ class IndexView(BaseView):
),
"hidden_tables_count": len(hidden_tables),
"views_count": len(views),
- "private": private,
+ "private": database_private,
}
)
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 5c338e04..475f93dd 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -74,6 +74,37 @@ def test_database_list_respects_view_database():
assert 'fixtures š' in auth_response.text
+def test_database_list_respects_view_table():
+ with make_app_client(
+ metadata={
+ "databases": {
+ "data": {
+ "tables": {
+ "names": {"allow": {"id": "root"}},
+ "v": {"allow": {"id": "root"}},
+ }
+ }
+ }
+ },
+ extra_databases={
+ "data.db": "create table names (name text); create view v as select * from names"
+ },
+ ) as client:
+ html_fragments = [
+ ">names š",
+ ">v š",
+ ]
+ anon_response_text = client.get("/").text
+ assert "0 rows in 0 tables" in anon_response_text
+ for html_fragment in html_fragments:
+ assert html_fragment not in anon_response_text
+ auth_response_text = client.get(
+ "/", cookies={"ds_actor": client.ds.sign({"id": "root"}, "actor")},
+ ).text
+ for html_fragment in html_fragments:
+ assert html_fragment in auth_response_text
+
+
@pytest.mark.parametrize(
"allow,expected_anon,expected_auth",
[(None, 200, 200), ({}, 403, 403), ({"id": "root"}, 403, 200),],
From c9f1ec616e5a8c83f554baaedd38663569fb9b91 Mon Sep 17 00:00:00 2001
From: Simon Willison
Date: Mon, 8 Jun 2020 11:51:03 -0700
Subject: [PATCH 0066/1855] Removed resource_type from permissions system,
closes #817
Refs #811, #699
---
datasette/app.py | 4 +---
datasette/default_permissions.py | 5 +---
datasette/hookspecs.py | 2 +-
datasette/templates/permissions_debug.html | 4 ++--
datasette/utils/__init__.py | 16 +++----------
datasette/views/base.py | 5 +---
datasette/views/database.py | 28 ++++++++--------------
datasette/views/index.py | 6 ++---
datasette/views/table.py | 10 ++++----
docs/authentication.rst | 19 ++-------------
docs/internals.rst | 7 ++----
docs/plugins.rst | 9 +++----
tests/conftest.py | 4 ++--
tests/fixtures.py | 9 +++----
14 files changed, 39 insertions(+), 89 deletions(-)
diff --git a/datasette/app.py b/datasette/app.py
index 87e542c1..c12e0af0 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -465,7 +465,7 @@ class Datasette:
return []
async def permission_allowed(
- self, actor, action, resource_type=None, resource_identifier=None, default=False
+ self, actor, action, resource_identifier=None, default=False
):
"Check permissions using the permissions_allowed plugin hook"
result = None
@@ -473,7 +473,6 @@ class Datasette:
datasette=self,
actor=actor,
action=action,
- resource_type=resource_type,
resource_identifier=resource_identifier,
):
if callable(check):
@@ -491,7 +490,6 @@ class Datasette:
"when": datetime.datetime.utcnow().isoformat(),
"actor": actor,
"action": action,
- "resource_type": resource_type,
"resource_identifier": resource_identifier,
"used_default": used_default,
"result": result,
diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py
index dd1770a3..d27704aa 100644
--- a/datasette/default_permissions.py
+++ b/datasette/default_permissions.py
@@ -3,7 +3,7 @@ from datasette.utils import actor_matches_allow
@hookimpl
-def permission_allowed(datasette, actor, action, resource_type, resource_identifier):
+def permission_allowed(datasette, actor, action, resource_identifier):
if action == "permissions-debug":
if actor and actor.get("id") == "root":
return True
@@ -12,13 +12,11 @@ def permission_allowed(datasette, actor, action, resource_type, resource_identif
if allow is not None:
return actor_matches_allow(actor, allow)
elif action == "view-database":
- assert resource_type == "database"
database_allow = datasette.metadata("allow", database=resource_identifier)
if database_allow is None:
return True
return actor_matches_allow(actor, database_allow)
elif action == "view-table":
- assert resource_type == "table"
database, table = resource_identifier
tables = datasette.metadata("tables", database=database) or {}
table_allow = (tables.get(table) or {}).get("allow")
@@ -27,7 +25,6 @@ def permission_allowed(datasette, actor, action, resource_type, resource_identif
return actor_matches_allow(actor, table_allow)
elif action == "view-query":
# Check if this query has a "allow" block in metadata
- assert resource_type == "query"
database, query_name = resource_identifier
queries_metadata = datasette.metadata("queries", database=database)
assert query_name in queries_metadata
diff --git a/datasette/hookspecs.py b/datasette/hookspecs.py
index 71d06661..3c202553 100644
--- a/datasette/hookspecs.py
+++ b/datasette/hookspecs.py
@@ -66,5 +66,5 @@ def actor_from_request(datasette, request):
@hookspec
-def permission_allowed(datasette, actor, action, resource_type, resource_identifier):
+def permission_allowed(datasette, actor, action, resource_identifier):
"Check if actor is allowed to perfom this action - return True, False or None"
diff --git a/datasette/templates/permissions_debug.html b/datasette/templates/permissions_debug.html
index dda57dfa..7d3ee712 100644
--- a/datasette/templates/permissions_debug.html
+++ b/datasette/templates/permissions_debug.html
@@ -46,8 +46,8 @@
{% endif %}
Actor: {{ check.actor|tojson }}
- {% if check.resource_type %}
- Resource: {{ check.resource_type }} = {{ check.resource_identifier }}
+ {% if check.resource_identifier %}
+ Resource: {{ check.resource_identifier }}
{% endif %}
Actor: {{ check.actor|tojson }}
- {% if check.resource_identifier %} -Resource: {{ check.resource_identifier }}
+ {% if check.resource %} +Resource: {{ check.resource }}
{% endif %} {% endfor %} diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 257d1285..7c1f34e0 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -876,14 +876,14 @@ def actor_matches_allow(actor, allow): return False -async def check_visibility(datasette, actor, action, resource_identifier, default=True): +async def check_visibility(datasette, actor, action, resource, default=True): "Returns (visible, private) - visible = can you see it, private = can others see it too" visible = await datasette.permission_allowed( - actor, action, resource_identifier=resource_identifier, default=default, + actor, action, resource=resource, default=default, ) if not visible: return (False, False) private = not await datasette.permission_allowed( - None, action, resource_identifier=resource_identifier, default=default, + None, action, resource=resource, default=default, ) return visible, private diff --git a/datasette/views/base.py b/datasette/views/base.py index 2ca5e86a..f327c6cd 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -64,12 +64,9 @@ class BaseView(AsgiView): response.body = b"" return response - async def check_permission(self, request, action, resource_identifier=None): + async def check_permission(self, request, action, resource=None): ok = await self.ds.permission_allowed( - request.actor, - action, - resource_identifier=resource_identifier, - default=True, + request.actor, action, resource=resource, default=True, ) if not ok: raise Forbidden(action) diff --git a/datasette/views/database.py b/datasette/views/database.py index d562ecb1..e1b29c27 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -88,7 +88,7 @@ class DatabaseView(DataView): "views": views, "queries": canned_queries, "private": not await self.ds.permission_allowed( - None, "view-database", "database", database + None, "view-database", database ), }, { diff --git a/docs/authentication.rst b/docs/authentication.rst index 67112969..f5209dfc 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -159,7 +159,7 @@ This is designed to help administrators and plugin authors understand exactly ho Permissions =========== -This section lists all of the permission checks that are carried out by Datasette core, along with the ``resource_identifier`` if it was passed. +This section lists all of the permission checks that are carried out by Datasette core, along with the ``resource`` if it was passed. .. _permissions_view_instance: @@ -176,7 +176,7 @@ view-database Actor is allowed to view a database page, e.g. https://latest.datasette.io/fixtures -``resource_identifier`` - string +``resource`` - string The name of the database .. _permissions_view_database_download: @@ -186,7 +186,7 @@ view-database-download Actor is allowed to download a database, e.g. https://latest.datasette.io/fixtures.db -``resource_identifier`` - string +``resource`` - string The name of the database .. _permissions_view_table: @@ -196,7 +196,7 @@ view-table Actor is allowed to view a table (or view) page, e.g. https://latest.datasette.io/fixtures/complex_foreign_keys -``resource_identifier`` - tuple: (string, string) +``resource`` - tuple: (string, string) The name of the database, then the name of the table .. _permissions_view_query: @@ -206,7 +206,7 @@ view-query Actor is allowed to view a :ref:`canned query{% if query %}{{ query.sql }}{% endif %}
diff --git a/datasette/templates/table.html b/datasette/templates/table.html
index 1289e125..373fd576 100644
--- a/datasette/templates/table.html
+++ b/datasette/templates/table.html
@@ -109,7 +109,7 @@
{% endif %}
-{% if query.sql and config.allow_sql %}
+{% if query.sql and allow_execute_sql %}
{% endif %}
diff --git a/datasette/views/database.py b/datasette/views/database.py
index e1b29c27..ee99bc2d 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -26,8 +26,6 @@ class DatabaseView(DataView):
self.ds.update_with_inherited_metadata(metadata)
if request.args.get("sql"):
- if not self.ds.config("allow_sql"):
- raise DatasetteError("sql= is not allowed", status=400)
sql = request.args.get("sql")
validate_sql_select(sql)
return await QueryView(self.ds).data(
@@ -90,6 +88,9 @@ class DatabaseView(DataView):
"private": not await self.ds.permission_allowed(
None, "view-database", database
),
+ "allow_execute_sql": await self.ds.permission_allowed(
+ request.actor, "execute-sql", database, default=True
+ ),
},
{
"show_hidden": request.args.get("_show_hidden"),
@@ -289,6 +290,9 @@ class QueryView(DataView):
"columns": columns,
"query": {"sql": sql, "params": params},
"private": private,
+ "allow_execute_sql": await self.ds.permission_allowed(
+ request.actor, "execute-sql", database, default=True
+ ),
},
extra_template,
templates,
diff --git a/datasette/views/table.py b/datasette/views/table.py
index 4cec0cda..91245293 100644
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@@ -342,8 +342,10 @@ class TableView(RowTableShared):
extra_wheres_for_ui = []
# Add _where= from querystring
if "_where" in request.args:
- if not self.ds.config("allow_sql"):
- raise DatasetteError("_where= is not allowed", status=400)
+ if not await self.ds.permission_allowed(
+ request.actor, "execute-sql", resource=database, default=True,
+ ):
+ raise DatasetteError("_where= is not allowed", status=403)
else:
where_clauses.extend(request.args.getlist("_where"))
extra_wheres_for_ui = [
@@ -839,6 +841,9 @@ class TableView(RowTableShared):
"next": next_value and str(next_value) or None,
"next_url": next_url,
"private": private,
+ "allow_execute_sql": await self.ds.permission_allowed(
+ request.actor, "execute-sql", database, default=True
+ ),
},
extra_template,
(
diff --git a/docs/authentication.rst b/docs/authentication.rst
index 34d46511..f7281db4 100644
--- a/docs/authentication.rst
+++ b/docs/authentication.rst
@@ -176,7 +176,7 @@ This works for SQL views as well - you can treat them as if they are tables.
.. warning::
Restricting access to tables and views in this way will NOT prevent users from querying them using arbitrary SQL queries, `like this