Handle composition of unimplemented permission methods correctly

rest_framework/permissions.py

@@ -53,16 +53,18 @@ def __init__(self, op1, op2):
         self.op2 = op2
 
     def has_permission(self, request, view):
-        return (
-            self.op1.has_permission(request, view) and
-            self.op2.has_permission(request, view)
-        )
+        hasperm1 = self.op1.has_permission(request, view)
+        if not hasperm1:
+            return hasperm1
+        hasperm2 = self.op2.has_permission(request, view)
+        return hasperm1 if hasperm2 is NotImplemented else hasperm2
 
     def has_object_permission(self, request, view, obj):
-        return (
-            self.op1.has_object_permission(request, view, obj) and
-            self.op2.has_object_permission(request, view, obj)
-        )
+        hasperm1 = self.op1.has_object_permission(request, view, obj)
+        if not hasperm1:
+            return hasperm1
+        hasperm2 = self.op2.has_object_permission(request, view, obj)
+        return hasperm1 if hasperm2 is NotImplemented else hasperm2
 
 
 class OR:
@@ -71,27 +73,35 @@ def __init__(self, op1, op2):
         self.op2 = op2
 
     def has_permission(self, request, view):
-        return (
-            self.op1.has_permission(request, view) or
-            self.op2.has_permission(request, view)
-        )
+        hasperm1 = self.op1.has_permission(request, view)
+        if hasperm1 and hasperm1 is not NotImplemented:
+            return hasperm1
+        hasperm2 = self.op2.has_permission(request, view)
+        return hasperm2 or hasperm1
 
     def has_object_permission(self, request, view, obj):
-        return (
-            self.op1.has_object_permission(request, view, obj) or
-            self.op2.has_object_permission(request, view, obj)
-        )
+        hasperm1 = self.op1.has_object_permission(request, view, obj)
+        if hasperm1 is NotImplemented:
+            hasperm1 = self.op1.has_permission(request, view)
+        if hasperm1 and hasperm1 is not NotImplemented:
+            return hasperm1
+        hasperm2 = self.op2.has_object_permission(request, view, obj)
+        if hasperm2 is NotImplemented:
+            hasperm2 = self.op2.has_permission(request, view)
+        return hasperm2 or hasperm1
 
 
 class NOT:
     def __init__(self, op1):
         self.op1 = op1
 
     def has_permission(self, request, view):
-        return not self.op1.has_permission(request, view)
+        hasperm = self.op1.has_permission(request, view)
+        return hasperm if hasperm is NotImplemented else not hasperm
 
     def has_object_permission(self, request, view, obj):
-        return not self.op1.has_object_permission(request, view, obj)
+        hasperm = self.op1.has_object_permission(request, view, obj)
+        return hasperm if hasperm is NotImplemented else not hasperm
 
 
 class BasePermissionMetaclass(OperationHolderMixin, type):
@@ -107,13 +117,13 @@ def has_permission(self, request, view):
         """
         Return `True` if permission is granted, `False` otherwise.
         """
-        return True
+        return NotImplemented
 
     def has_object_permission(self, request, view, obj):
         """
         Return `True` if permission is granted, `False` otherwise.
         """
-        return True
+        return NotImplemented
 
 
 class AllowAny(BasePermission):
@@ -220,7 +230,7 @@ def has_permission(self, request, view):
         # Workaround to ensure DjangoModelPermissions are not applied
         # to the root view when using DefaultRouter.
         if getattr(view, '_ignore_model_permissions', False):
-            return True
+            return super().has_permission(request, view)
 
         if not request.user or (
            not request.user.is_authenticated and self.authenticated_users_only):

tests/test_permissions.py

@@ -528,6 +528,12 @@ def setUp(self):
             self.password
         )
         self.client.login(username=self.username, password=self.password)
+        self.admin_user = User.objects.create_user(
+            'paul',
+            '[email protected]',
+            'password',
+            is_staff=True,
+        )
 
     def test_and_false(self):
         request = factory.get('/1', format='json')
@@ -677,3 +683,30 @@ def test_object_and_lazyness(self):
                 assert hasperm is False
                 assert mock_deny.call_count == 1
                 mock_allow.assert_not_called()
+
+    def test_has_permission_not_implemented(self):
+        request = factory.get('/1', format='json')
+        request.user = self.user
+        composed_perm = ~BasicObjectPerm
+        assert composed_perm().has_permission(request, None) is NotImplemented
+        assert composed_perm().has_object_permission(request, None, None) is True
+
+    def test_has_object_permission_not_implemented_false(self):
+        request = factory.get('/1', format='json')
+        request.user = self.user
+        composed_perm = (
+            permissions.IsAdminUser |
+            BasicObjectPerm
+        )
+        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):
+        request = factory.get('/1', format='json')
+        request.user = self.admin_user
+        composed_perm = (
+            permissions.IsAdminUser |
+            BasicObjectPerm
+        )
+        assert composed_perm().has_permission(request, None) is True
+        assert composed_perm().has_object_permission(request, None, None) is True