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.
This commit is contained in:
Ben Buchwald 2021-05-07 11:56:56 -04:00
parent 91f12360d0
commit a224c6c67b
2 changed files with 3 additions and 3 deletions

View File

@ -77,7 +77,7 @@ class OR:
if hasperm1 and hasperm1 is not NotImplemented: if hasperm1 and hasperm1 is not NotImplemented:
return hasperm1 return hasperm1
hasperm2 = self.op2.has_permission(request, view) 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): def has_object_permission(self, request, view, obj):
hasperm1 = self.op1.has_object_permission(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) hasperm2 = self.op2.has_object_permission(request, view, obj)
if hasperm2 is NotImplemented: if hasperm2 is NotImplemented:
hasperm2 = self.op2.has_permission(request, view) hasperm2 = self.op2.has_permission(request, view)
return hasperm1 if hasperm2 is NotImplemented else hasperm2 return hasperm2 or hasperm1
class NOT: class NOT:

View File

@ -698,7 +698,7 @@ class PermissionsCompositionTests(TestCase):
permissions.IsAdminUser | permissions.IsAdminUser |
BasicObjectPerm 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 assert composed_perm().has_object_permission(request, None, None) is False
def test_has_object_permission_not_implemented_true(self): def test_has_object_permission_not_implemented_true(self):