diff --git a/datasette/default_menu_links.py b/datasette/default_menu_links.py index 11374fb5..0b135410 100644 --- a/datasette/default_menu_links.py +++ b/datasette/default_menu_links.py @@ -3,7 +3,10 @@ from datasette import hookimpl @hookimpl def menu_links(datasette, actor): - if actor and actor.get("id") == "root": + async def inner(): + if not await datasette.permission_allowed(actor, "debug-menu"): + return [] + return [ {"href": datasette.urls.path("/-/databases"), "label": "Databases"}, { @@ -38,3 +41,5 @@ def menu_links(datasette, actor): {"href": datasette.urls.path("/-/actor"), "label": "Debug actor"}, {"href": datasette.urls.path("/-/patterns"), "label": "Pattern portfolio"}, ] + + return inner diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index ddd45940..9f1d9c62 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -5,7 +5,7 @@ from datasette.utils import actor_matches_allow @hookimpl(tryfirst=True) def permission_allowed(datasette, actor, action, resource): async def inner(): - if action == "permissions-debug": + if action in ("permissions-debug", "debug-menu"): if actor and actor.get("id") == "root": return True elif action == "view-instance": diff --git a/datasette/views/special.py b/datasette/views/special.py index a9fc59b7..397dbc8c 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -96,7 +96,8 @@ class PermissionsDebugView(BaseView): return await self.render( ["permissions_debug.html"], request, - {"permission_checks": reversed(self.ds._permission_checks)}, + # list() avoids error if check is performed during template render: + {"permission_checks": list(reversed(self.ds._permission_checks))}, ) diff --git a/docs/authentication.rst b/docs/authentication.rst index f6c5d801..62ed7e8b 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -522,3 +522,12 @@ permissions-debug Actor is allowed to view the ``/-/permissions`` debug page. Default *deny*. + +.. _permissions_debug_menu: + +debug-menu +---------- + +Controls if the various debug pages are displayed in the navigation menu. + +Default *deny*. diff --git a/tests/test_html.py b/tests/test_html.py index 95b5128a..fed643a9 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -1507,3 +1507,41 @@ def test_edit_sql_link_not_shown_if_user_lacks_permission(permission_allowed): assert "Edit SQL" in response.text else: assert "Edit SQL" not in response.text + + +@pytest.mark.parametrize( + "actor_id,should_have_links,should_not_have_links", + [ + (None, None, None), + ("test", None, ["/-/permissions"]), + ("root", ["/-/permissions", "/-/allow-debug", "/-/metadata"], None), + ], +) +def test_navigation_menu_links( + app_client, actor_id, should_have_links, should_not_have_links +): + cookies = {} + if actor_id: + cookies = {"ds_actor": app_client.actor_cookie({"id": actor_id})} + html = app_client.get("/", cookies=cookies).text + soup = Soup(html, "html.parser") + details = soup.find("nav").find("details") + if not actor_id: + # Should not show a menu + assert details is None + return + # They are logged in: should show a menu + assert details is not None + # And a rogout form + assert details.find("form") is not None + if should_have_links: + for link in should_have_links: + assert ( + details.find("a", {"href": link}) is not None + ), "{} expected but missing from nav menu".format(link) + + if should_not_have_links: + for link in should_not_have_links: + assert ( + details.find("a", {"href": link}) is None + ), "{} found but should not have been in nav menu".format(link) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 4d1b09b8..60883eef 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -310,10 +310,11 @@ def test_permissions_checked(app_client, path, permissions): def test_permissions_debug(app_client): app_client.ds._permission_checks.clear() - assert 403 == app_client.get("/-/permissions").status + assert app_client.get("/-/permissions").status == 403 # With the cookie it should work cookie = app_client.actor_cookie({"id": "root"}) response = app_client.get("/-/permissions", cookies={"ds_actor": cookie}) + assert response.status == 200 # Should show one failure and one success soup = Soup(response.body, "html.parser") check_divs = soup.findAll("div", {"class": "check"})