From 425ac4357ffb722a6ca86d08faba02ee38ad8689 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 15 Dec 2022 14:18:40 -0800 Subject: [PATCH] Ported app_client to ds_client where possible in test_auth.py, refs #1959 --- datasette/app.py | 4 ++ datasette/utils/testing.py | 7 -- tests/plugins/my_plugin_2.py | 4 +- tests/test_auth.py | 132 ++++++++++++++++++++--------------- tests/test_messages.py | 3 +- tests/utils.py | 8 +++ 6 files changed, 91 insertions(+), 67 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index b770b469..04e26a46 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1718,6 +1718,10 @@ class DatasetteClient: self.ds = ds self.app = ds.app() + def actor_cookie(self, actor): + # Utility method, mainly for tests + return self.ds.sign({"a": actor}, "actor") + def _fix(self, path, avoid_path_rewrites=False): if not isinstance(path, PrefixedUrlString) and not avoid_path_rewrites: path = self.ds.urls.path(path) diff --git a/datasette/utils/testing.py b/datasette/utils/testing.py index 4f76a799..cabe2e5c 100644 --- a/datasette/utils/testing.py +++ b/datasette/utils/testing.py @@ -28,13 +28,6 @@ class TestResponse: def cookies(self): return dict(self.httpx_response.cookies) - def cookie_was_deleted(self, cookie): - return any( - h - for h in self.httpx_response.headers.get_list("set-cookie") - if h.startswith(f'{cookie}="";') - ) - @property def json(self): return json.loads(self.text) diff --git a/tests/plugins/my_plugin_2.py b/tests/plugins/my_plugin_2.py index cee80703..4f7bf08c 100644 --- a/tests/plugins/my_plugin_2.py +++ b/tests/plugins/my_plugin_2.py @@ -135,7 +135,9 @@ def prepare_jinja2_environment(env, datasette): @hookimpl def startup(datasette): async def inner(): - result = await datasette.get_database().execute("select 1 + 1") + # Run against _internal so tests that use the ds_client fixture + # (which has no databases yet on startup) do not fail: + result = await datasette.get_database("_internal").execute("select 1 + 1") datasette._startup_hook_calculation = result.first()[0] return inner diff --git a/tests/test_auth.py b/tests/test_auth.py index dd1b61e3..bc5c6a2b 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,5 +1,6 @@ from bs4 import BeautifulSoup as Soup from .fixtures import app_client +from .utils import cookie_was_deleted from click.testing import CliRunner from datasette.utils import baseconv from datasette.cli import cli @@ -7,46 +8,47 @@ import pytest import time -def test_auth_token(app_client): +@pytest.mark.ds_client +@pytest.mark.asyncio +async def test_auth_token(ds_client): """The /-/auth-token endpoint sets the correct cookie""" - assert app_client.ds._root_token is not None - path = f"/-/auth-token?token={app_client.ds._root_token}" - response = app_client.get( - path, - ) - assert 302 == response.status + assert ds_client.ds._root_token is not None + path = f"/-/auth-token?token={ds_client.ds._root_token}" + response = await ds_client.get(path) + assert response.status_code == 302 assert "/" == response.headers["Location"] - assert {"a": {"id": "root"}} == app_client.ds.unsign( + assert {"a": {"id": "root"}} == ds_client.ds.unsign( response.cookies["ds_actor"], "actor" ) # Check that a second with same token fails - assert app_client.ds._root_token is None - assert ( - 403 - == app_client.get( - path, - ).status - ) + assert ds_client.ds._root_token is None + assert (await ds_client.get(path)).status_code == 403 -def test_actor_cookie(app_client): +@pytest.mark.ds_client +@pytest.mark.asyncio +async def test_actor_cookie(ds_client): """A valid actor cookie sets request.scope['actor']""" - cookie = app_client.actor_cookie({"id": "test"}) - response = app_client.get("/", cookies={"ds_actor": cookie}) - assert {"id": "test"} == app_client.ds._last_request.scope["actor"] + cookie = ds_client.actor_cookie({"id": "test"}) + await ds_client.get("/", cookies={"ds_actor": cookie}) + assert ds_client.ds._last_request.scope["actor"] == {"id": "test"} -def test_actor_cookie_invalid(app_client): - cookie = app_client.actor_cookie({"id": "test"}) +@pytest.mark.ds_client +@pytest.mark.asyncio +async def test_actor_cookie_invalid(ds_client): + cookie = ds_client.actor_cookie({"id": "test"}) # Break the signature - response = app_client.get("/", cookies={"ds_actor": cookie[:-1] + "."}) - assert None == app_client.ds._last_request.scope["actor"] + await ds_client.get("/", cookies={"ds_actor": cookie[:-1] + "."}) + assert ds_client.ds._last_request.scope["actor"] is None # Break the cookie format - cookie = app_client.ds.sign({"b": {"id": "test"}}, "actor") - response = app_client.get("/", cookies={"ds_actor": cookie}) - assert None == app_client.ds._last_request.scope["actor"] + cookie = ds_client.ds.sign({"b": {"id": "test"}}, "actor") + await ds_client.get("/", cookies={"ds_actor": cookie}) + assert ds_client.ds._last_request.scope["actor"] is None +@pytest.mark.ds_client +@pytest.mark.asyncio @pytest.mark.parametrize( "offset,expected", [ @@ -54,16 +56,17 @@ def test_actor_cookie_invalid(app_client): (-(24 * 60 * 60), None), ], ) -def test_actor_cookie_that_expires(app_client, offset, expected): +async def test_actor_cookie_that_expires(ds_client, offset, expected): expires_at = int(time.time()) + offset - cookie = app_client.ds.sign( + cookie = ds_client.ds.sign( {"a": {"id": "test"}, "e": baseconv.base62.encode(expires_at)}, "actor" ) - response = app_client.get("/", cookies={"ds_actor": cookie}) - assert expected == app_client.ds._last_request.scope["actor"] + response = await ds_client.get("/", cookies={"ds_actor": cookie}) + assert ds_client.ds._last_request.scope["actor"] == expected def test_logout(app_client): + # Keeping app_client for the moment because of csrftoken_from response = app_client.get( "/-/logout", cookies={"ds_actor": app_client.actor_cookie({"id": "test"})} ) @@ -88,18 +91,20 @@ def test_logout(app_client): cookies={"ds_actor": app_client.actor_cookie({"id": "test"})}, ) # The ds_actor cookie should have been unset - assert response4.cookie_was_deleted("ds_actor") + assert cookie_was_deleted(response4, "ds_actor") # Should also have set a message messages = app_client.ds.unsign(response4.cookies["ds_messages"], "messages") assert [["You are now logged out", 2]] == messages +@pytest.mark.ds_client +@pytest.mark.asyncio @pytest.mark.parametrize("path", ["/", "/fixtures", "/fixtures/facetable"]) -def test_logout_button_in_navigation(app_client, path): - response = app_client.get( - path, cookies={"ds_actor": app_client.actor_cookie({"id": "test"})} +async def test_logout_button_in_navigation(ds_client, path): + response = await ds_client.get( + path, cookies={"ds_actor": ds_client.actor_cookie({"id": "test"})} ) - anon_response = app_client.get(path) + anon_response = await ds_client.get(path) for fragment in ( "test", '
', @@ -108,9 +113,11 @@ def test_logout_button_in_navigation(app_client, path): assert fragment not in anon_response.text +@pytest.mark.ds_client +@pytest.mark.asyncio @pytest.mark.parametrize("path", ["/", "/fixtures", "/fixtures/facetable"]) -def test_no_logout_button_in_navigation_if_no_ds_actor_cookie(app_client, path): - response = app_client.get(path + "?_bot=1") +async def test_no_logout_button_in_navigation_if_no_ds_actor_cookie(ds_client, path): + response = await ds_client.get(path + "?_bot=1") assert "bot" in response.text assert '' not in response.text @@ -205,25 +212,33 @@ def test_auth_create_token( assert response3.json["actor"]["id"] == "test" -def test_auth_create_token_not_allowed_for_tokens(app_client): - ds_tok = app_client.ds.sign({"a": "test", "token": "dstok"}, "token") - response = app_client.get( +@pytest.mark.ds_client +@pytest.mark.asyncio +async def test_auth_create_token_not_allowed_for_tokens(ds_client): + ds_tok = ds_client.ds.sign({"a": "test", "token": "dstok"}, "token") + response = await ds_client.get( "/-/create-token", headers={"Authorization": "Bearer dstok_{}".format(ds_tok)}, ) - assert response.status == 403 + assert response.status_code == 403 -def test_auth_create_token_not_allowed_if_allow_signed_tokens_off(app_client): - app_client.ds._settings["allow_signed_tokens"] = False +@pytest.mark.ds_client +@pytest.mark.asyncio +async def test_auth_create_token_not_allowed_if_allow_signed_tokens_off(ds_client): + ds_client.ds._settings["allow_signed_tokens"] = False try: - ds_actor = app_client.actor_cookie({"id": "test"}) - response = app_client.get("/-/create-token", cookies={"ds_actor": ds_actor}) - assert response.status == 403 + ds_actor = ds_client.actor_cookie({"id": "test"}) + response = await ds_client.get( + "/-/create-token", cookies={"ds_actor": ds_actor} + ) + assert response.status_code == 403 finally: - app_client.ds._settings["allow_signed_tokens"] = True + ds_client.ds._settings["allow_signed_tokens"] = True +@pytest.mark.ds_client +@pytest.mark.asyncio @pytest.mark.parametrize( "scenario,should_work", ( @@ -236,31 +251,32 @@ def test_auth_create_token_not_allowed_if_allow_signed_tokens_off(app_client): ("valid_expiring_token", True), ), ) -def test_auth_with_dstok_token(app_client, scenario, should_work): +async def test_auth_with_dstok_token(ds_client, scenario, should_work): token = None _time = int(time.time()) if scenario in ("valid_unlimited_token", "allow_signed_tokens_off"): - token = app_client.ds.sign({"a": "test", "t": _time}, "token") + token = ds_client.ds.sign({"a": "test", "t": _time}, "token") elif scenario == "valid_expiring_token": - token = app_client.ds.sign({"a": "test", "t": _time - 50, "d": 1000}, "token") + token = ds_client.ds.sign({"a": "test", "t": _time - 50, "d": 1000}, "token") elif scenario == "expired_token": - token = app_client.ds.sign({"a": "test", "t": _time - 2000, "d": 1000}, "token") + token = ds_client.ds.sign({"a": "test", "t": _time - 2000, "d": 1000}, "token") elif scenario == "no_timestamp": - token = app_client.ds.sign({"a": "test"}, "token") + token = ds_client.ds.sign({"a": "test"}, "token") elif scenario == "invalid_token": token = "invalid" if token: token = "dstok_{}".format(token) if scenario == "allow_signed_tokens_off": - app_client.ds._settings["allow_signed_tokens"] = False + ds_client.ds._settings["allow_signed_tokens"] = False headers = {} if token: headers["Authorization"] = "Bearer {}".format(token) - response = app_client.get("/-/actor.json", headers=headers) + response = await ds_client.get("/-/actor.json", headers=headers) try: if should_work: - assert response.json.keys() == {"actor"} - actor = response.json["actor"] + data = response.json() + assert data.keys() == {"actor"} + actor = data["actor"] expected_keys = {"id", "token"} if scenario != "valid_unlimited_token": expected_keys.add("token_expires") @@ -270,9 +286,9 @@ def test_auth_with_dstok_token(app_client, scenario, should_work): if scenario != "valid_unlimited_token": assert isinstance(actor["token_expires"], int) else: - assert response.json == {"actor": None} + assert response.json() == {"actor": None} finally: - app_client.ds._settings["allow_signed_tokens"] = True + ds_client.ds._settings["allow_signed_tokens"] = True @pytest.mark.parametrize("expires", (None, 1000, -1000)) diff --git a/tests/test_messages.py b/tests/test_messages.py index 3af5439a..6fbff066 100644 --- a/tests/test_messages.py +++ b/tests/test_messages.py @@ -1,4 +1,5 @@ from .fixtures import app_client +from .utils import cookie_was_deleted import pytest @@ -25,4 +26,4 @@ def test_messages_are_displayed_and_cleared(app_client): # Messages should be in that HTML assert "xmessagex" in response.text # Cookie should have been set that clears messages - assert response.cookie_was_deleted("ds_messages") + assert cookie_was_deleted(response, "ds_messages") diff --git a/tests/utils.py b/tests/utils.py index 191ead9b..84d5b1df 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -30,3 +30,11 @@ def inner_html(soup): def has_load_extension(): conn = sqlite3.connect(":memory:") return hasattr(conn, "enable_load_extension") + + +def cookie_was_deleted(response, cookie): + return any( + h + for h in response.headers.get_list("set-cookie") + if h.startswith(f'{cookie}="";') + )