From 194e4f6c3fffde69eb196f8535ca45386b40ec2d Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 21 Mar 2022 11:41:56 -0700 Subject: [PATCH] Removed check_permission() from BaseView, closes #1677 Refs #1660 --- datasette/app.py | 1 + datasette/views/base.py | 10 ---------- datasette/views/database.py | 2 +- datasette/views/index.py | 2 +- datasette/views/special.py | 10 +++++----- tests/test_permissions.py | 13 ++++++++----- 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 9e509e961a..22ae211fca 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -639,6 +639,7 @@ async def ensure_permissions( Raises datasette.Forbidden() if any of the checks fail """ + assert actor is None or isinstance(actor, dict) for permission in permissions: if isinstance(permission, str): action = permission diff --git a/datasette/views/base.py b/datasette/views/base.py index d1e684a2b0..221e1882ce 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -66,16 +66,6 @@ async def head(self, *args, **kwargs): response.body = b"" return response - async def check_permission(self, request, action, resource=None): - ok = await self.ds.permission_allowed( - request.actor, - action, - resource=resource, - default=True, - ) - if not ok: - raise Forbidden(action) - def database_color(self, database): return "ff0000" diff --git a/datasette/views/database.py b/datasette/views/database.py index 69ed1233c1..31a1839f97 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -229,7 +229,7 @@ async def data( None, "view-query", (database, canned_query), default=True ) else: - await self.check_permission(request, "execute-sql", database) + await self.ds.ensure_permissions(request.actor, [("execute-sql", database)]) # Extract any :named parameters named_parameters = named_parameters or await derive_named_parameters( diff --git a/datasette/views/index.py b/datasette/views/index.py index f5e3118125..1c391e2670 100644 --- a/datasette/views/index.py +++ b/datasette/views/index.py @@ -20,7 +20,7 @@ class IndexView(BaseView): async def get(self, request): as_format = request.url_vars["format"] - await self.check_permission(request, "view-instance") + await self.ds.ensure_permissions(request.actor, ["view-instance"]) databases = [] for name, db in self.ds.databases.items(): visible, database_private = await check_visibility( diff --git a/datasette/views/special.py b/datasette/views/special.py index 395ee58789..dd8345288b 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -16,7 +16,7 @@ def __init__(self, datasette, filename, data_callback, needs_request=False): async def get(self, request): as_format = request.url_vars["format"] - await self.check_permission(request, "view-instance") + await self.ds.ensure_permissions(request.actor, ["view-instance"]) if self.needs_request: data = self.data_callback(request) else: @@ -47,7 +47,7 @@ class PatternPortfolioView(BaseView): has_json_alternate = False async def get(self, request): - await self.check_permission(request, "view-instance") + await self.ds.ensure_permissions(request.actor, ["view-instance"]) return await self.render(["patterns.html"], request=request) @@ -95,7 +95,7 @@ class PermissionsDebugView(BaseView): has_json_alternate = False async def get(self, request): - await self.check_permission(request, "view-instance") + await self.ds.ensure_permissions(request.actor, ["view-instance"]) if not await self.ds.permission_allowed(request.actor, "permissions-debug"): raise Forbidden("Permission denied") return await self.render( @@ -146,11 +146,11 @@ class MessagesDebugView(BaseView): has_json_alternate = False async def get(self, request): - await self.check_permission(request, "view-instance") + await self.ds.ensure_permissions(request.actor, ["view-instance"]) return await self.render(["messages_debug.html"], request) async def post(self, request): - await self.check_permission(request, "view-instance") + await self.ds.ensure_permissions(request.actor, ["view-instance"]) post = await request.post_vars() message = post.get("message", "") message_type = post.get("message_type") or "INFO" diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 788523b010..f4169dbe12 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -321,17 +321,20 @@ def test_permissions_debug(app_client): checks = [ { "action": div.select_one(".check-action").text, - "result": bool(div.select(".check-result-true")), + # True = green tick, False = red cross, None = gray None + "result": None + if div.select(".check-result-no-opinion") + else bool(div.select(".check-result-true")), "used_default": bool(div.select(".check-used-default")), } for div in check_divs ] - assert [ + assert checks == [ {"action": "permissions-debug", "result": True, "used_default": False}, - {"action": "view-instance", "result": True, "used_default": True}, + {"action": "view-instance", "result": None, "used_default": True}, {"action": "permissions-debug", "result": False, "used_default": True}, - {"action": "view-instance", "result": True, "used_default": True}, - ] == checks + {"action": "view-instance", "result": None, "used_default": True}, + ] @pytest.mark.parametrize(