From 46746fce94bb6bfa3b233f0ca0ba1f345eee6d3c Mon Sep 17 00:00:00 2001 From: Mohammad Mahdi Mohajer Date: Fri, 7 Aug 2026 14:47:36 -0400 Subject: [PATCH] fix: use the permission's `message` for denial responses Permission denials on `adrf.views.APIView` always fell back to DRF's generic "You do not have permission to perform this action.", discarding any custom `message` set on the permission class. Two separate causes, across the four leaf checks: 1. All four read `permission.detail`, but DRF permission classes carry the denial text on `.message` (see `BasePermission.message` and `rest_framework.views.APIView.check_permissions`). `.detail` is the attribute on `APIException`, not on permissions, so the getattr always returned `None`. 2. `check_async_permissions` and `check_async_object_permissions` additionally read the attribute off the *boolean result* of `asyncio.gather` rather than off the permission object, so even the correct attribute name could never have resolved. The results are now zipped back with their permissions. The `code` attribute was affected by (2) in the same way. This makes async views report the same denial reason as the equivalent sync DRF view. Existing tests only asserted `status_code == 403` and never inspected the response body, which is how this went unnoticed; the added tests assert the message and code for the sync, async, object and non-object paths. --- adrf/views.py | 18 ++++---- tests/test_object_permissions.py | 58 ++++++++++++++++++++++++++ tests/test_permissions.py | 71 ++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 8 deletions(-) diff --git a/adrf/views.py b/adrf/views.py index 97b0836..90c9b7d 100755 --- a/adrf/views.py +++ b/adrf/views.py @@ -142,14 +142,14 @@ async def check_async_permissions( return_exceptions=True, ) - for has_permission in has_permissions: + for permission, has_permission in zip(permissions, has_permissions): if isinstance(has_permission, Exception): raise has_permission elif not has_permission: self.permission_denied( request, - message=getattr(has_permission, "detail", None), - code=getattr(has_permission, "code", None), + message=getattr(permission, "message", None), + code=getattr(permission, "code", None), ) def check_sync_permissions( @@ -164,7 +164,7 @@ def check_sync_permissions( if not permission.has_permission(request, self): self.permission_denied( request, - message=getattr(permission, "detail", None), + message=getattr(permission, "message", None), code=getattr(permission, "code", None), ) @@ -206,14 +206,16 @@ async def check_async_object_permissions( return_exceptions=True, ) - for has_object_permission in has_object_permissions: + for permission, has_object_permission in zip( + permissions, has_object_permissions + ): if isinstance(has_object_permission, Exception): raise has_object_permission elif not has_object_permission: self.permission_denied( request, - message=getattr(has_object_permission, "detail", None), - code=getattr(has_object_permission, "code", None), + message=getattr(permission, "message", None), + code=getattr(permission, "code", None), ) def check_sync_object_permissions( @@ -228,7 +230,7 @@ def check_sync_object_permissions( if not permission.has_object_permission(request, self, obj): self.permission_denied( request, - message=getattr(permission, "detail", None), + message=getattr(permission, "message", None), code=getattr(permission, "code", None), ) diff --git a/tests/test_object_permissions.py b/tests/test_object_permissions.py index 8d049d0..2fed828 100755 --- a/tests/test_object_permissions.py +++ b/tests/test_object_permissions.py @@ -73,3 +73,61 @@ async def test_sync_object_permission_reject(self): )(request) self.assertEqual(response.status_code, 403) + + +class AsyncMessageObjectPermission(BasePermission): + message = "Async object permission denied for a specific reason." + code = "async_obj_denied" + + async def has_permission(self, request, view): + return True + + async def has_object_permission(self, request, view, obj): + return False + + +class SyncMessageObjectPermission(BasePermission): + message = "Sync object permission denied for a specific reason." + code = "sync_obj_denied" + + def has_permission(self, request, view): + return True + + def has_object_permission(self, request, view, obj): + return False + + +class MessageObjectPermissionTestView(ObjectPermissionTestView): + # `permission_denied` short circuits to `NotAuthenticated` when the request + # carries authenticators but none succeeded, which would mask the message + # under test. + authentication_classes = () + + +@override_settings(ROOT_URLCONF=__name__) +class TestObjectPermissionDeniedMessage(TestCase): + """The denial message and code are read from the permission that denied.""" + + async def test_async_object_permission_denied_message(self): + request = factory.get("/async/reject") + + response = await MessageObjectPermissionTestView.as_view( + permission_classes=(AsyncMessageObjectPermission,) + )(request) + + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data["detail"], AsyncMessageObjectPermission.message) + self.assertEqual( + response.data["detail"].code, AsyncMessageObjectPermission.code + ) + + async def test_sync_object_permission_denied_message(self): + request = factory.get("/sync/reject") + + response = await MessageObjectPermissionTestView.as_view( + permission_classes=(SyncMessageObjectPermission,) + )(request) + + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data["detail"], SyncMessageObjectPermission.message) + self.assertEqual(response.data["detail"].code, SyncMessageObjectPermission.code) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 566e741..23002f1 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -214,3 +214,74 @@ async def test_sync_first_complex_mixed_permission( mock_async_accept.assert_awaited() mock_sync_reject.assert_called() self.assertEqual(response.status_code, 200) + + +class AsyncMessagePermission(AsyncBasePermission): + message = "Async permission denied for a specific reason." + code = "async_denied" + + async def has_permission(self, request, view): + return False + + +class SyncMessagePermission(BasePermission): + message = "Sync permission denied for a specific reason." + code = "sync_denied" + + def has_permission(self, request, view): + return False + + +class AsyncAllowPermission(AsyncBasePermission): + message = "This permission allowed the request, so its message must not be used." + + async def has_permission(self, request, view): + return True + + +class MessageView(APIView): + # `permission_denied` short circuits to `NotAuthenticated` when the request + # carries authenticators but none succeeded, which would mask the message + # under test. + authentication_classes = () + + async def get(self, request): + return HttpResponse("ok") + + +@override_settings(ROOT_URLCONF=__name__) +class TestPermissionDeniedMessage(TestCase): + """The denial message and code are read from the permission that denied.""" + + async def test_async_permission_denied_message(self): + request = factory.get("/view/async/reject/") + + response = await MessageView.as_view( + permission_classes=(AsyncMessagePermission,) + )(request) + + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data["detail"], AsyncMessagePermission.message) + self.assertEqual(response.data["detail"].code, AsyncMessagePermission.code) + + async def test_sync_permission_denied_message(self): + request = factory.get("/view/sync/reject/") + + response = await MessageView.as_view( + permission_classes=(SyncMessagePermission,) + )(request) + + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data["detail"], SyncMessagePermission.message) + self.assertEqual(response.data["detail"].code, SyncMessagePermission.code) + + async def test_async_permission_denied_message_of_the_denying_permission(self): + """The message must come from the permission that actually returned False.""" + request = factory.get("/view/async/reject/") + + response = await MessageView.as_view( + permission_classes=(AsyncAllowPermission, AsyncMessagePermission) + )(request) + + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data["detail"], AsyncMessagePermission.message)