From 30a8132d58a89fed0e034e058b62fab5180fae0f Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Jun 2020 11:18:46 -0700 Subject: [PATCH 1/7] Docs for authentication + canned query permissions, refs #800 Closes #786 --- docs/authentication.rst | 108 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/docs/authentication.rst b/docs/authentication.rst index 0a9a4c0d..2c07f75a 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -4,14 +4,118 @@ Authentication and permissions ================================ -Datasette's authentication system is currently under construction. Follow `issue 699 `__ to track the development of this feature. +Datasette does not require authentication by default. Any visitor to a Datasette instance can explore the full data and execute SQL queries. + +Datasette's plugin system can be used to add many different styles of authentication, such as user accounts, single sign-on or API keys. + +.. _authentication_actor: + +Actors +====== + +Through plugins, Datasette can support both authenticated users (with cookies) and authenticated API agents (via authentication tokens). The word "actor" is used to cover both of these cases. + +Every request to Datasette has an associated actor value. This can be ``None`` for unauthenticated requests, or a JSON compatible Python dictionary for authenticated users or API agents. + +The only required field in an actor is ``"id"``, which must be a string. Plugins may decide to add any other fields to the actor dictionary. + +Plugins can use the :ref:`plugin_actor_from_request` hook to implement custom logic for authenticating an actor based on the incoming HTTP request. + +.. _authentication_root: + +Using the "root" actor +====================== + +Datasette currently leaves almost all forms of authentication to plugins - `datasette-auth-github `__ for example. + +The one exception is the "root" account, which you can sign into while using Datasette on your local machine. This provides access to a small number of debugging features. + +To sign in as root, start Datasette using the ``--root`` command-line option, like this:: + + $ datasette --root + http://127.0.0.1:8001/-/auth-token?token=786fc524e0199d70dc9a581d851f466244e114ca92f33aa3b42a139e9388daa7 + INFO: Started server process [25801] + INFO: Waiting for application startup. + INFO: Application startup complete. + INFO: Uvicorn running on http://127.0.0.1:8001 (Press CTRL+C to quit) + +The URL on the first line includes a one-use token which can be used to sign in as the "root" actor in your browser. Click on that link and then visit ``http://127.0.0.1:8001/-/actor`` to confirm that you are authenticated as an actor that looks like this: + +.. code-block:: json + + { + "id": "root" + } + + +.. _authentication_permissions_canned_queries: + +Setting permissions for canned queries +====================================== + +Datasette's :ref:`canned_queries` default to allowing any user to execute them. + +You can limit who is allowed to execute a specific query with the ``"allow"`` key in the :ref:`metadata` configuration for that query. + +Here's how to restrict access to a write query to just the "root" user: + +.. code-block:: json + + { + "databases": { + "mydatabase": { + "queries": { + "add_name": { + "sql": "INSERT INTO names (name) VALUES (:name)", + "write": true, + "allow": { + "id": ["root"] + } + } + } + } + } + } + +To allow any of the actors with an ``id`` matching a specific list of values, use this: + +.. code-block:: json + + { + "allow": { + "id": ["simon", "cleopaws"] + } + } + +This works for other keys as well. Imagine an actor that looks like this: + +.. code-block:: json + + { + "id": "simon", + "roles": ["staff", "developer"] + } + +You can provide access to any user that has "developer" as one of their roles like so: + +.. code-block:: json + + { + "allow": { + "roles": ["developer"] + } + } + +Note that "roles" is not a concept that is baked into Datasette - it's more of a convention that plugins can choose to implement and act on. + +These keys act as an "or" mechanism. A actor will be able to execute the query if any of their JSON properties match any of the values in the corresponding lists in the ``allow`` block. .. _PermissionsDebugView: Permissions Debug ================= -The debug tool at ``/-/permissions`` is only available to the root user. +The debug tool at ``/-/permissions`` is only available to the :ref:`authenticated root user ` (or any actor granted the ``permissions-debug`` action according to a plugin). It shows the thirty most recent permission checks that have been carried out by the Datasette instance. From d4c7b85f556230923d37ff327a068ed08aa9b62b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Jun 2020 11:23:54 -0700 Subject: [PATCH 2/7] Documentation for "id": "*", refs #800 --- docs/authentication.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/authentication.rst b/docs/authentication.rst index 2c07f75a..a90dcc41 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -108,6 +108,16 @@ You can provide access to any user that has "developer" as one of their roles li Note that "roles" is not a concept that is baked into Datasette - it's more of a convention that plugins can choose to implement and act on. +If you want to provide access to any actor with a value for a specific key, use ``"*"``. For example, to spceify that a query can be accessed by any logged-in user use this: + +.. code-block:: json + + { + "allow": { + "id": "*" + } + } + These keys act as an "or" mechanism. A actor will be able to execute the query if any of their JSON properties match any of the values in the corresponding lists in the ``allow`` block. .. _PermissionsDebugView: From 14f6b4d200f24940a795ddc0825319ab2891bde2 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Jun 2020 11:39:11 -0700 Subject: [PATCH 3/7] actor_matches_allow utility function, refs #800 --- datasette/utils/__init__.py | 19 +++++++++++++++++++ docs/authentication.rst | 18 ++++++++++++++++-- tests/test_utils.py | 27 +++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 059db184..eb118f38 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -854,3 +854,22 @@ def call_with_supported_arguments(fn, **kwargs): ) call_with.append(kwargs[parameter]) return fn(*call_with) + + +def actor_matches_allow(actor, allow): + if allow is None: + return True + for key, values in allow.items(): + if values == "*" and key in actor: + return True + if isinstance(values, str): + values = [values] + actor_values = actor.get(key) + if actor_values is None: + return False + if isinstance(actor_values, str): + actor_values = [actor_values] + actor_values = set(actor_values) + if actor_values.intersection(values): + return True + return False diff --git a/docs/authentication.rst b/docs/authentication.rst index a90dcc41..85bbbbbd 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -50,8 +50,8 @@ The URL on the first line includes a one-use token which can be used to sign in .. _authentication_permissions_canned_queries: -Setting permissions for canned queries -====================================== +Permissions for canned queries +============================== Datasette's :ref:`canned_queries` default to allowing any user to execute them. @@ -120,6 +120,20 @@ If you want to provide access to any actor with a value for a specific key, use These keys act as an "or" mechanism. A actor will be able to execute the query if any of their JSON properties match any of the values in the corresponding lists in the ``allow`` block. +.. _authentication_actor_matches_allow: + +actor_matches_allow() +===================== + +Plugins that wish to implement the same permissions scheme as canned queries can take advantage of the ``datasette.utils.actor_matches_allow(actor, allow)`` function: + +.. code-block:: python + + from datasette.utils import actor_matches_allow + + actor_matches_allow({"id": "root"}, {"id": "*"}) + # returns True + .. _PermissionsDebugView: Permissions Debug diff --git a/tests/test_utils.py b/tests/test_utils.py index 4931ef3b..7c24648a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -459,3 +459,30 @@ def test_multi_params(data, should_raise): p1 = utils.MultiParams(data) assert "bar" == p1["foo"] assert ["bar", "baz"] == list(p1.getlist("foo")) + + +@pytest.mark.parametrize( + "actor,allow,expected", + [ + ({"id": "root"}, None, True), + ({"id": "root"}, {}, False), + # Special "*" value for any key: + ({"id": "root"}, {"id": "*"}, True), + ({}, {"id": "*"}, False), + ({"name": "root"}, {"id": "*"}, False), + # Supports single strings or list of values: + ({"id": "root"}, {"id": "bob"}, False), + ({"id": "root"}, {"id": ["bob"]}, False), + ({"id": "root"}, {"id": "root"}, True), + ({"id": "root"}, {"id": ["root"]}, True), + # Any matching role will work: + ({"id": "garry", "roles": ["staff", "dev"]}, {"roles": ["staff"]}, True), + ({"id": "garry", "roles": ["staff", "dev"]}, {"roles": ["dev"]}, True), + ({"id": "garry", "roles": ["staff", "dev"]}, {"roles": ["otter"]}, False), + ({"id": "garry", "roles": ["staff", "dev"]}, {"roles": ["dev", "otter"]}, True), + ({"id": "garry", "roles": []}, {"roles": ["staff"]}, False), + ({"id": "garry"}, {"roles": ["staff"]}, False), + ], +) +def test_actor_matches_allow(actor, allow, expected): + assert expected == utils.actor_matches_allow(actor, allow) From 3f83d4632a643266f46ccd955d951be7aacbab99 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Jun 2020 12:05:22 -0700 Subject: [PATCH 4/7] Respect query permissions on database page, refs #800 --- datasette/templates/database.html | 2 +- datasette/utils/__init__.py | 1 + datasette/views/database.py | 13 ++++++++++++- tests/test_canned_write.py | 31 ++++++++++++++++++++++++++++++- tests/test_utils.py | 3 +++ 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/datasette/templates/database.html b/datasette/templates/database.html index e47b2418..fc88003c 100644 --- a/datasette/templates/database.html +++ b/datasette/templates/database.html @@ -60,7 +60,7 @@

Queries

{% endif %} diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index eb118f38..077728f4 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -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(): diff --git a/datasette/views/database.py b/datasette/views/database.py index 558dd0f0..abc7d3bb 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -2,6 +2,7 @@ import os import jinja2 from datasette.utils import ( + actor_matches_allow, to_css_class, validate_sql_select, is_url, @@ -53,6 +54,16 @@ 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)), + ) + 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, @@ -60,7 +71,7 @@ class DatabaseView(DataView): "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"), diff --git a/tests/test_canned_write.py b/tests/test_canned_write.py index be838063..5b5756b0 100644 --- a/tests/test_canned_write.py +++ b/tests/test_canned_write.py @@ -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", @@ -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( @@ -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 '' 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"] + ] diff --git a/tests/test_utils.py b/tests/test_utils.py index 7c24648a..975ed0fd 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -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), From 070838bfa19b177f59ef3bd8f0139266adecda90 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Jun 2020 12:26:19 -0700 Subject: [PATCH 5/7] Better test for Vary header --- tests/fixtures.py | 2 -- tests/test_canned_write.py | 6 ++++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 4ca7b10f..2268ef4d 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -132,8 +132,6 @@ class TestClient: if csrftoken_from is True: csrftoken_from = path token_response = await self._request(csrftoken_from) - # Check this had a Vary: Cookie header - assert "Cookie" == token_response.headers["vary"] csrftoken = token_response.cookies["ds_csrftoken"] cookies["ds_csrftoken"] = csrftoken post_data["csrftoken"] = csrftoken diff --git a/tests/test_canned_write.py b/tests/test_canned_write.py index 5b5756b0..aacc586f 100644 --- a/tests/test_canned_write.py +++ b/tests/test_canned_write.py @@ -100,6 +100,12 @@ def test_custom_params(canned_write_client): assert '' in response.text +def test_vary_header(canned_write_client): + # These forms embed a csrftoken so they should be served with Vary: Cookie + assert "vary" not in canned_write_client.get("/data").headers + assert "Cookie" == canned_write_client.get("/data/update_name").headers["vary"] + + def test_canned_query_permissions_on_database_page(canned_write_client): # Without auth only shows three queries query_names = [ From 966eec7f75d2e1b809b001bb7e82f35d477f77ea Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Jun 2020 12:27:00 -0700 Subject: [PATCH 6/7] Check permissions on canned query page, refs #800 --- datasette/views/database.py | 10 +++++++++- tests/test_canned_write.py | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/datasette/views/database.py b/datasette/views/database.py index abc7d3bb..4e9a6da7 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -9,7 +9,7 @@ from datasette.utils import ( path_with_added_args, path_with_removed_args, ) -from datasette.utils.asgi import AsgiFileDownload +from datasette.utils.asgi import AsgiFileDownload, Response from datasette.plugins import pm from .base import DatasetteError, DataView @@ -125,6 +125,14 @@ class QueryView(DataView): params.pop("sql") if "_shape" in params: params.pop("_shape") + + # Respect canned query permissions + if canned_query: + if not actor_matches_allow( + request.scope.get("actor", None), metadata.get("allow") + ): + return Response("Permission denied", status=403) + # Extract any :named parameters named_parameters = named_parameters or self.re_named_parameter.findall(sql) named_parameter_values = { diff --git a/tests/test_canned_write.py b/tests/test_canned_write.py index aacc586f..73b01e51 100644 --- a/tests/test_canned_write.py +++ b/tests/test_canned_write.py @@ -128,3 +128,11 @@ def test_canned_query_permissions_on_database_page(canned_write_client): {"name": q["name"], "requires_auth": q["requires_auth"]} for q in response.json["queries"] ] + + +def test_canned_query_permissions(canned_write_client): + assert 403 == canned_write_client.get("/data/delete_name").status + assert 200 == canned_write_client.get("/data/update_name").status + cookies = {"ds_actor": canned_write_client.ds.sign({"id": "root"}, "actor")} + assert 200 == canned_write_client.get("/data/delete_name", cookies=cookies).status + assert 200 == canned_write_client.get("/data/update_name", cookies=cookies).status From 3359d54a4eb9c9725c27a85437661b5180c4099a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Jun 2020 12:33:08 -0700 Subject: [PATCH 7/7] Use cookies when accessing csrftoken_from --- tests/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 2268ef4d..75bd6b94 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -131,7 +131,7 @@ class TestClient: if csrftoken_from is not None: if csrftoken_from is True: csrftoken_from = path - token_response = await self._request(csrftoken_from) + token_response = await self._request(csrftoken_from, cookies=cookies) csrftoken = token_response.cookies["ds_csrftoken"] cookies["ds_csrftoken"] = csrftoken post_data["csrftoken"] = csrftoken