From 431f8fe3aef31e16e67d6bc26cb17149fcf05b8b Mon Sep 17 00:00:00 2001 From: Serhii Tereshchenko Date: Tue, 11 Feb 2025 11:07:34 +0200 Subject: [PATCH] refactor: Refactor permissions to allow list --- rest_framework/exceptions.py | 11 ++++ rest_framework/permissions.py | 95 +++++++++++++++-------------- tests/test_permissions.py | 109 ++++++++++++++-------------------- 3 files changed, 108 insertions(+), 107 deletions(-) diff --git a/rest_framework/exceptions.py b/rest_framework/exceptions.py index 09f111102..af1f4df1d 100644 --- a/rest_framework/exceptions.py +++ b/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: diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index dc4a3f0b8..b5b578971 100644 --- a/rest_framework/permissions.py +++ b/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 @@ class OperandHolder(OperationHolderMixin): 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 + + +class AND(OperatorBase): 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 - + 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: diff --git a/tests/test_permissions.py b/tests/test_permissions.py index dabef46e7..a9e727253 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -12,14 +12,16 @@ from rest_framework import ( 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 @@ denied_view_with_detail_and_1 = DeniedViewWithDetailAND1.as_view() 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 @@ denied_object_view_with_detail_and_1 = DeniedObjectViewWithDetailAND1.as_view() 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 @@ class CustomPermissionsTests(TestCase): User.objects.create_user('username', 'username@example.com', '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 @@ class CustomPermissionsTests(TestCase): 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 @@ class CustomPermissionsTests(TestCase): 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 @@ class CustomPermissionsTests(TestCase): 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)