refactor: Refactor permissions to allow list

Author: last-partizan

rest_framework/exceptions.py

@@ -106,6 +106,17 @@ class APIException(Exception):
     default_code = 'error'
 
     def __init__(self, detail=None, code=None):
+        if (
+            isinstance(detail, tuple)
+            and isinstance(code, tuple)
+            and len(detail) == len(code)
+        ):
+            self.detail = [
+                _get_error_details(d or self.default_detail, c or self.default_code)
+                for d, c in zip(detail, code)
+            ]
+            return
+
         if detail is None:
             detail = self.default_detail
         if code is None:

rest_framework/permissions.py

@@ -2,7 +2,6 @@
 Provides a set of pluggable permission policies.
 """
 from django.http import Http404
-from django.utils.translation import gettext_lazy as _
 
 from rest_framework import exceptions
 
@@ -59,61 +58,69 @@ def __hash__(self):
         return hash((self.operator_class, self.op1_class, self.op2_class))
 
 
-class AND:
-    def __init__(self, op1, op2):
-        self.op1 = op1
-        self.op2 = op2
-        self.message = None
+class OperatorBase:
+    def __init__(self, *permissions):
+        self._permissions = permissions
 
-    def has_permission(self, request, view):
-        if not self.op1.has_permission(request, view):
-            self.message = getattr(self.op1, 'message', None)
-            return False
 
-        if not self.op2.has_permission(request, view):
-            self.message = getattr(self.op2, 'message', None)
-            return False
+class AND(OperatorBase):
 
+    def has_permission(self, request, view):
+        for perm in self._permissions:
+            if not perm.has_permission(request, view):
+                self._set_message_and_code(perm)
+                return False
         return True
 
     def has_object_permission(self, request, view, obj):
-        if not self.op1.has_object_permission(request, view, obj):
-            self.message = getattr(self.op1, 'message', None)
-            return False
-
-        if not self.op2.has_object_permission(request, view, obj):
-            self.message = getattr(self.op2, 'message', None)
-            return False
-
+        for perm in self._permissions:
+            if not perm.has_object_permission(request, view, obj):
+                self._set_message_and_code(perm)
+                return False
         return True
 
+    def _set_message_and_code(self, perm):
+        self.message = getattr(perm, 'message', None)
+        self.code = getattr(perm, 'code', None)
 
-class OR:
-    def __init__(self, op1, op2):
-        self.op1 = op1
-        self.op2 = op2
-        self.message1 = getattr(op1, 'message', None)
-        self.message2 = getattr(op2, 'message', None)
-        self.message = self.message1 or self.message2
-        if self.message1 and self.message2:
-            self.message = '"{0}" {1} "{2}"'.format(
-                self.message1, _('OR'), self.message2,
-            )
+
+class OR(OperatorBase):
 
     def has_permission(self, request, view):
-        return (
-            self.op1.has_permission(request, view) or
-            self.op2.has_permission(request, view)
-        )
+        collector = ResultCollector()
+        for perm in self._permissions:
+            if perm.has_permission(request, view):
+                return True
+            else:
+                collector.add_message_and_code(perm)
+        collector.finalize(self)
+        return False
 
     def has_object_permission(self, request, view, obj):
-        return (
-            self.op1.has_permission(request, view)
-            and self.op1.has_object_permission(request, view, obj)
-        ) or (
-            self.op2.has_permission(request, view)
-            and self.op2.has_object_permission(request, view, obj)
-        )
+        collector = ResultCollector()
+        for perm in self._permissions:
+            if perm.has_permission(request, view) and perm.has_object_permission(request, view, obj):
+                return True
+            else:
+                collector.add_message_and_code(perm)
+        collector.finalize(self)
+        return False
+
+
+class ResultCollector:
+    def __init__(self):
+        self.messages = ()
+        self.codes = ()
+
+    def add_message_and_code(self, perm):
+        message = getattr(perm, 'message', None)
+        code = getattr(perm, 'code', None)
+        self.messages += (message,)
+        self.codes += (code,)
+
+    def finalize(self, perm):
+        perm.message = self.messages
+        perm.code = self.codes
 
 
 class NOT:

tests/test_permissions.py

@@ -12,14 +12,16 @@
     HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers,
     status, views
 )
+from rest_framework.exceptions import ErrorDetail
 from rest_framework.routers import DefaultRouter
 from rest_framework.test import APIRequestFactory
 from tests.models import BasicModel
 
 factory = APIRequestFactory()
 
-CUSTOM_MESSAGE_1 = 'Custom: You cannot access this resource'
-CUSTOM_MESSAGE_2 = 'Custom: You do not have permission to view this resource'
+DEFAULT_MESSAGE = ErrorDetail('You do not have permission to perform this action.', 'permission_denied')
+CUSTOM_MESSAGE_1 = ErrorDetail('Custom: You cannot access this resource', 'permission_denied_custom')
+CUSTOM_MESSAGE_2 = ErrorDetail('Custom: You do not have permission to view this resource', 'permission_denied_custom')
 INVERTED_MESSAGE = 'Inverted: Your account already active'
 
 
@@ -519,18 +521,6 @@ class DeniedViewWithDetailAND3(PermissionInstanceView):
     permission_classes = (BasicPermWithDetail & AnotherBasicPermWithDetail,)
 
 
-class DeniedViewWithDetailOR1(PermissionInstanceView):
-    permission_classes = (BasicPerm | BasicPermWithDetail,)
-
-
-class DeniedViewWithDetailOR2(PermissionInstanceView):
-    permission_classes = (BasicPermWithDetail | BasicPerm,)
-
-
-class DeniedViewWithDetailOR3(PermissionInstanceView):
-    permission_classes = (BasicPermWithDetail | AnotherBasicPermWithDetail,)
-
-
 class DeniedViewWithDetailNOT(PermissionInstanceView):
     permission_classes = (~BasicPermWithDetail,)
 
@@ -548,23 +538,11 @@ class DeniedObjectViewWithDetailAND1(PermissionInstanceView):
 
 
 class DeniedObjectViewWithDetailAND2(PermissionInstanceView):
-    permission_classes = (permissions.AllowAny & AnotherBasicObjectPermWithDetail)
+    permission_classes = (permissions.AllowAny & AnotherBasicObjectPermWithDetail,)
 
 
 class DeniedObjectViewWithDetailAND3(PermissionInstanceView):
-    permission_classes = (AnotherBasicObjectPermWithDetail & BasicObjectPermWithDetail)
-
-
-class DeniedObjectViewWithDetailOR1(PermissionInstanceView):
-    permission_classes = (BasicObjectPerm | BasicObjectPermWithDetail)
-
-
-class DeniedObjectViewWithDetailOR2(PermissionInstanceView):
-    permission_classes = (BasicObjectPermWithDetail | BasicObjectPerm,)
-
-
-class DeniedObjectViewWithDetailOR3(PermissionInstanceView):
-    permission_classes = (BasicObjectPermWithDetail | AnotherBasicObjectPermWithDetail,)
+    permission_classes = (AnotherBasicObjectPermWithDetail & BasicObjectPermWithDetail,)
 
 
 class DeniedObjectViewWithDetailNOT(PermissionInstanceView):
@@ -579,10 +557,6 @@ class DeniedObjectViewWithDetailNOT(PermissionInstanceView):
 denied_view_with_detail_and_2 = DeniedViewWithDetailAND2.as_view()
 denied_view_with_detail_and_3 = DeniedViewWithDetailAND3.as_view()
 
-denied_view_with_detail_or_1 = DeniedViewWithDetailOR1.as_view()
-denied_view_with_detail_or_2 = DeniedViewWithDetailOR2.as_view()
-denied_view_with_detail_or_3 = DeniedViewWithDetailOR3.as_view()
-
 denied_view_with_detail_not = DeniedObjectViewWithDetailNOT.as_view()
 
 denied_object_view = DeniedObjectView.as_view()
@@ -593,10 +567,6 @@ class DeniedObjectViewWithDetailNOT(PermissionInstanceView):
 denied_object_view_with_detail_and_2 = DeniedObjectViewWithDetailAND2.as_view()
 denied_object_view_with_detail_and_3 = DeniedObjectViewWithDetailAND3.as_view()
 
-denied_object_view_with_detail_or_1 = DeniedObjectViewWithDetailOR1.as_view()
-denied_object_view_with_detail_or_2 = DeniedObjectViewWithDetailOR2.as_view()
-denied_object_view_with_detail_or_3 = DeniedObjectViewWithDetailOR3.as_view()
-
 denied_object_view_with_detail_not = DeniedObjectViewWithDetailNOT.as_view()
 
 
@@ -606,21 +576,18 @@ def setUp(self):
         User.objects.create_user('username', '[email protected]', 'password')
         credentials = basic_auth_header('username', 'password')
         self.request = factory.get('/1', format='json', HTTP_AUTHORIZATION=credentials)
-        self.custom_code = 'permission_denied_custom'
 
     def test_permission_denied(self):
         response = denied_view(self.request, pk=1)
         detail = response.data.get('detail')
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertNotEqual(detail, CUSTOM_MESSAGE_1)
-        self.assertNotEqual(detail.code, self.custom_code)
+        self.assertEqual(detail, DEFAULT_MESSAGE)
 
     def test_permission_denied_with_custom_detail(self):
         response = denied_view_with_detail(self.request, pk=1)
         detail = response.data.get('detail')
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
         self.assertEqual(detail, CUSTOM_MESSAGE_1)
-        self.assertEqual(detail.code, self.custom_code)
 
     def test_permission_denied_with_custom_detail_and_1(self):
         response = denied_view_with_detail_and_1(self.request, pk=1)
@@ -641,23 +608,31 @@ def test_permission_denied_with_custom_detail_and_3(self):
         self.assertEqual(detail, CUSTOM_MESSAGE_1)
 
     def test_permission_denied_with_custom_detail_or_1(self):
-        response = denied_view_with_detail_or_1(self.request, pk=1)
-        detail = response.data.get('detail')
+        view = PermissionInstanceView.as_view(
+            permission_classes=(BasicPerm | BasicPermWithDetail,),
+        )
+        response = view(self.request, pk=1)
+        detail = response.data
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertEqual(detail, CUSTOM_MESSAGE_1)
+        self.assertEqual(detail, [DEFAULT_MESSAGE, CUSTOM_MESSAGE_1])
 
     def test_permission_denied_with_custom_detail_or_2(self):
-        response = denied_view_with_detail_or_2(self.request, pk=1)
-        detail = response.data.get('detail')
+        view = PermissionInstanceView.as_view(
+            permission_classes=(BasicPermWithDetail | BasicPerm,),
+        )
+        response = view(self.request, pk=1)
+        detail = response.data
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertEqual(detail, CUSTOM_MESSAGE_1)
+        self.assertEqual(detail, [CUSTOM_MESSAGE_1, DEFAULT_MESSAGE])
 
     def test_permission_denied_with_custom_detail_or_3(self):
-        response = denied_view_with_detail_or_3(self.request, pk=1)
-        detail = response.data.get('detail')
+        view = PermissionInstanceView.as_view(
+            permission_classes=(BasicPermWithDetail | AnotherBasicPermWithDetail,),
+        )
+        response = view(self.request, pk=1)
+        detail = response.data
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        expected_message = '"{0}" OR "{1}"'.format(CUSTOM_MESSAGE_1, CUSTOM_MESSAGE_2)
-        self.assertEqual(detail, expected_message)
+        self.assertEqual(detail, [CUSTOM_MESSAGE_1, CUSTOM_MESSAGE_2])
 
     def test_permission_denied_with_custom_detail_not(self):
         response = denied_view_with_detail_not(self.request, pk=1)
@@ -669,15 +644,13 @@ def test_permission_denied_for_object(self):
         response = denied_object_view(self.request, pk=1)
         detail = response.data.get('detail')
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertNotEqual(detail, CUSTOM_MESSAGE_1)
-        self.assertNotEqual(detail.code, self.custom_code)
+        self.assertEqual(detail, DEFAULT_MESSAGE)
 
     def test_permission_denied_for_object_with_custom_detail(self):
         response = denied_object_view_with_detail(self.request, pk=1)
         detail = response.data.get('detail')
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
         self.assertEqual(detail, CUSTOM_MESSAGE_1)
-        self.assertEqual(detail.code, self.custom_code)
 
     def test_permission_denied_for_object_with_custom_detail_and_1(self):
         response = denied_object_view_with_detail_and_1(self.request, pk=1)
@@ -698,23 +671,33 @@ def test_permission_denied_for_object_with_custom_detail_and_3(self):
         self.assertEqual(detail, CUSTOM_MESSAGE_2)
 
     def test_permission_denied_for_object_with_custom_detail_or_1(self):
-        response = denied_object_view_with_detail_or_1(self.request, pk=1)
-        detail = response.data.get('detail')
+        view = PermissionInstanceView.as_view(
+            permission_classes=(BasicObjectPerm | BasicObjectPermWithDetail,),
+        )
+        response = view(self.request, pk=1)
+        detail = response.data
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertEqual(detail, CUSTOM_MESSAGE_1)
+        self.assertEqual(detail, [DEFAULT_MESSAGE, CUSTOM_MESSAGE_1])
 
     def test_permission_denied_for_object_with_custom_detail_or_2(self):
-        response = denied_object_view_with_detail_or_2(self.request, pk=1)
-        detail = response.data.get('detail')
+        view = PermissionInstanceView.as_view(
+            permission_classes=(BasicObjectPermWithDetail | BasicObjectPerm,),
+        )
+        response = view(self.request, pk=1)
+        detail = response.data
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertEqual(detail, CUSTOM_MESSAGE_1)
+        self.assertEqual(detail, [CUSTOM_MESSAGE_1, DEFAULT_MESSAGE])
 
     def test_permission_denied_for_object_with_custom_detail_or_3(self):
-        response = denied_object_view_with_detail_or_3(self.request, pk=1)
-        detail = response.data.get('detail')
+        view = PermissionInstanceView.as_view(
+            permission_classes=(
+                BasicObjectPermWithDetail | AnotherBasicObjectPermWithDetail,
+            ),
+        )
+        response = view(self.request, pk=1)
+        detail = response.data
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        expected_message = '"{0}" OR "{1}"'.format(CUSTOM_MESSAGE_1, CUSTOM_MESSAGE_2)
-        self.assertEqual(detail, expected_message)
+        self.assertEqual(detail, [CUSTOM_MESSAGE_1, CUSTOM_MESSAGE_2])
 
     def test_permission_denied_for_object_with_custom_detail_not(self):
         response = denied_object_view_with_detail_not(self.request, pk=1)