From 9b42e1a4f5902fb7d6ad0111189900e2656ffda3 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 7 Jun 2020 20:50:37 -0700 Subject: [PATCH] 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 ee182c85c4..40be8d343c 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 fc88003c64..eaebfdf749 100644 --- a/datasette/templates/database.html +++ b/datasette/templates/database.html @@ -60,7 +60,7 @@

Views

Queries

{% endif %} diff --git a/datasette/templates/index.html b/datasette/templates/index.html index b394564a84..3b8568b3c3 100644 --- a/datasette/templates/index.html +++ b/datasette/templates/index.html @@ -10,7 +10,7 @@

{{ metadata.title or "Datasette" }}

{% 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 961ab61e6e..4804b2a930 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -58,8 +58,7 @@ 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)), + 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 5f903474f1..7b88028beb 100644 --- a/datasette/views/index.py +++ b/datasette/views/index.py @@ -2,7 +2,7 @@ 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 @@ async def get(self, request, as_format): 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 @@ async def get(self, request, as_format): ), "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 c217be8ff5..dc3fba3f35 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 e05640d7ce..3f6dc4df97 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 bf66bc9c3a..21014a2502 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