From a224c6c67b8ffb0692d3bc3d0eaa4edb0ce980cf Mon Sep 17 00:00:00 2001 From: Ben Buchwald Date: Fri, 7 May 2021 11:56:56 -0400 Subject: [PATCH] object-level only permissions must pass has_permission Handling permissions with OR is tricky. If a permission only implements has_object_permission, we wanted has_permission to return NotImplemented instead of True, so that inverting it does what you would expect (invert the object-level logic, don't invert the default has_permission True so the object-level logic is never checked). However, having OR ignore NotImplemented means that if the thing it is ORed with returns False, an object-level only permission will never get the chance to check its object-level logic. Rather than being ignored, NotImplemented should be treated as true logic-wise (it is in Python) but with lower precedence than any other truthy value. --- rest_framework/permissions.py | 4 ++-- tests/test_permissions.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 437e9036d..974715ae1 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -77,7 +77,7 @@ class OR: if hasperm1 and hasperm1 is not NotImplemented: return hasperm1 hasperm2 = self.op2.has_permission(request, view) - return hasperm1 if hasperm2 is NotImplemented else hasperm2 + return hasperm2 or hasperm1 def has_object_permission(self, request, view, obj): hasperm1 = self.op1.has_object_permission(request, view, obj) @@ -88,7 +88,7 @@ class OR: hasperm2 = self.op2.has_object_permission(request, view, obj) if hasperm2 is NotImplemented: hasperm2 = self.op2.has_permission(request, view) - return hasperm1 if hasperm2 is NotImplemented else hasperm2 + return hasperm2 or hasperm1 class NOT: diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 6ca3566c9..33bc6cf4a 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -698,7 +698,7 @@ class PermissionsCompositionTests(TestCase): permissions.IsAdminUser | BasicObjectPerm ) - assert composed_perm().has_permission(request, None) is False + assert composed_perm().has_permission(request, None) is NotImplemented assert composed_perm().has_object_permission(request, None, None) is False def test_has_object_permission_not_implemented_true(self):