debug-menu permission, closes #1068

Also added tests for navigation menu logic.
This commit is contained in:
Simon Willison 2020-10-30 08:41:57 -07:00
commit 222f79bb4c
6 changed files with 58 additions and 4 deletions

View file

@ -3,7 +3,10 @@ from datasette import hookimpl
@hookimpl @hookimpl
def menu_links(datasette, actor): 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 [ return [
{"href": datasette.urls.path("/-/databases"), "label": "Databases"}, {"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("/-/actor"), "label": "Debug actor"},
{"href": datasette.urls.path("/-/patterns"), "label": "Pattern portfolio"}, {"href": datasette.urls.path("/-/patterns"), "label": "Pattern portfolio"},
] ]
return inner

View file

@ -5,7 +5,7 @@ from datasette.utils import actor_matches_allow
@hookimpl(tryfirst=True) @hookimpl(tryfirst=True)
def permission_allowed(datasette, actor, action, resource): def permission_allowed(datasette, actor, action, resource):
async def inner(): async def inner():
if action == "permissions-debug": if action in ("permissions-debug", "debug-menu"):
if actor and actor.get("id") == "root": if actor and actor.get("id") == "root":
return True return True
elif action == "view-instance": elif action == "view-instance":

View file

@ -96,7 +96,8 @@ class PermissionsDebugView(BaseView):
return await self.render( return await self.render(
["permissions_debug.html"], ["permissions_debug.html"],
request, 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))},
) )

View file

@ -522,3 +522,12 @@ permissions-debug
Actor is allowed to view the ``/-/permissions`` debug page. Actor is allowed to view the ``/-/permissions`` debug page.
Default *deny*. Default *deny*.
.. _permissions_debug_menu:
debug-menu
----------
Controls if the various debug pages are displayed in the navigation menu.
Default *deny*.

View file

@ -1507,3 +1507,41 @@ def test_edit_sql_link_not_shown_if_user_lacks_permission(permission_allowed):
assert "Edit SQL" in response.text assert "Edit SQL" in response.text
else: else:
assert "Edit SQL" not in response.text 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)

View file

@ -310,10 +310,11 @@ def test_permissions_checked(app_client, path, permissions):
def test_permissions_debug(app_client): def test_permissions_debug(app_client):
app_client.ds._permission_checks.clear() 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 # With the cookie it should work
cookie = app_client.actor_cookie({"id": "root"}) cookie = app_client.actor_cookie({"id": "root"})
response = app_client.get("/-/permissions", cookies={"ds_actor": cookie}) response = app_client.get("/-/permissions", cookies={"ds_actor": cookie})
assert response.status == 200
# Should show one failure and one success # Should show one failure and one success
soup = Soup(response.body, "html.parser") soup = Soup(response.body, "html.parser")
check_divs = soup.findAll("div", {"class": "check"}) check_divs = soup.findAll("div", {"class": "check"})