Skip to content

Modify permission operands to use custom messages #9649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Modify permission operands to use custom messages #9649

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e5162e1
Modify `AND` operand to have the ability to use custom messages
samitnuk Mar 8, 2019
42976d5
Modify `OR` operand to have the ability to use custom messages
samitnuk Mar 9, 2019
a3ac11e
Modify `NOT` operand to have the ability to use custom message
samitnuk Apr 13, 2020
2856523
Update tests/test_permissions.py
auvipy Apr 12, 2023
8ffa898
Update tests/test_permissions.py
auvipy Apr 12, 2023
dbdcb20
Update tests/test_permissions.py
auvipy Apr 12, 2023
431f8fe
refactor: Refactor permissions to allow list
last-partizan Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 28 additions & 8 deletions rest_framework/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
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

Expand Down Expand Up @@ -62,24 +63,42 @@ class AND:
def __init__(self, op1, op2):
self.op1 = op1
self.op2 = op2
self.message = None

def has_permission(self, request, view):
return (
self.op1.has_permission(request, view) and
self.op2.has_permission(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

return True

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)
)
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

return True


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,
)
Copy link
Author

@last-partizan last-partizan Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started working on getting this into proper shape, and I'm having trouble with this part.

And I need some help

Imagine a case:

class IsWhitelistedPath(BasePermission):
    message = "Path not whitelisted"
    ...

...
permission_classess = [IsAuthenticated | IsWhitelistedPath]

Here, in case of error, message will be "Path not whitelisted", completely ignoring the first one. This is wrong. And we do not have message available at the init time, because "AND" (and some of my permission classes) are setting self.message dynamically in has_permission call.

I come up with something like this:

class OR(OperatorBase):

    def has_permission(self, 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
   ...

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

Each failed permission adds it's message to a list, and we can handle this later, to produce a list of errors.

In this example, that will be:

[
    ErrorDetail('You do not have permission to perform this action.', 'permission_denied'),
    ErrorDetail('Path not whitelisted.'),
]

Maybe we should wrap it into additional container, something like {"detail": "Any of the listed perms required", "errors": [{"message": ..., "code": ...}]`... to make it clearer...

What do you think? @auvipy @tomchristie @samitnuk

Copy link
Collaborator

@auvipy auvipy Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest make the CI green first

Copy link
Author

@last-partizan last-partizan Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

And, now it includes my proposed changes.


def has_permission(self, request, view):
return (
Expand All @@ -100,6 +119,7 @@ def has_object_permission(self, request, view, obj):
class NOT:
def __init__(self, op1):
self.op1 = op1
self.message = getattr(self.op1, 'message_inverted', None)

def has_permission(self, request, view):
return not self.op1.has_permission(request, view)
Expand Down
195 changes: 188 additions & 7 deletions tests/test_permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@

factory = APIRequestFactory()

CUSTOM_MESSAGE_1 = 'Custom: You cannot access this resource'
CUSTOM_MESSAGE_2 = 'Custom: You do not have permission to view this resource'
INVERTED_MESSAGE = 'Inverted: Your account already active'


class BasicSerializer(serializers.ModelSerializer):
class Meta:
Expand Down Expand Up @@ -454,26 +458,42 @@ def has_permission(self, request, view):


class BasicPermWithDetail(permissions.BasePermission):
message = 'Custom: You cannot access this resource'
message = CUSTOM_MESSAGE_1
message_inverted = INVERTED_MESSAGE
code = 'permission_denied_custom'

def has_permission(self, request, view):
return False


class AnotherBasicPermWithDetail(permissions.BasePermission):
message = CUSTOM_MESSAGE_2

def has_permission(self, request, view):
return False


class BasicObjectPerm(permissions.BasePermission):
def has_object_permission(self, request, view, obj):
return False


class BasicObjectPermWithDetail(permissions.BasePermission):
message = 'Custom: You cannot access this resource'
message = CUSTOM_MESSAGE_1
message_inverted = INVERTED_MESSAGE
code = 'permission_denied_custom'

def has_object_permission(self, request, view, obj):
return False


class AnotherBasicObjectPermWithDetail(permissions.BasePermission):
message = CUSTOM_MESSAGE_2

def has_object_permission(self, request, view, obj):
return False


class PermissionInstanceView(generics.RetrieveUpdateDestroyAPIView):
queryset = BasicModel.objects.all()
serializer_class = BasicSerializer
Expand All @@ -487,6 +507,34 @@ class DeniedViewWithDetail(PermissionInstanceView):
permission_classes = (BasicPermWithDetail,)


class DeniedViewWithDetailAND1(PermissionInstanceView):
permission_classes = (BasicPermWithDetail & permissions.AllowAny,)


class DeniedViewWithDetailAND2(PermissionInstanceView):
permission_classes = (permissions.AllowAny & AnotherBasicPermWithDetail,)


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,)


class DeniedObjectView(PermissionInstanceView):
permission_classes = (BasicObjectPerm,)

Expand All @@ -495,52 +543,185 @@ class DeniedObjectViewWithDetail(PermissionInstanceView):
permission_classes = (BasicObjectPermWithDetail,)


class DeniedObjectViewWithDetailAND1(PermissionInstanceView):
permission_classes = (BasicObjectPermWithDetail & permissions.AllowAny,)


class DeniedObjectViewWithDetailAND2(PermissionInstanceView):
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,)


class DeniedObjectViewWithDetailNOT(PermissionInstanceView):
permission_classes = (~BasicObjectPermWithDetail,)


denied_view = DeniedView.as_view()

denied_view_with_detail = DeniedViewWithDetail.as_view()

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()

denied_object_view_with_detail = DeniedObjectViewWithDetail.as_view()

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()


class CustomPermissionsTests(TestCase):
def setUp(self):
BasicModel(text='foo').save()
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_message = 'Custom: You cannot access this resource'
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, self.custom_message)
self.assertNotEqual(detail, CUSTOM_MESSAGE_1)
self.assertNotEqual(detail.code, self.custom_code)

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, self.custom_message)
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)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, CUSTOM_MESSAGE_1)

def test_permission_denied_with_custom_detail_and_2(self):
response = denied_view_with_detail_and_2(self.request, pk=1)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, CUSTOM_MESSAGE_2)

def test_permission_denied_with_custom_detail_and_3(self):
response = denied_view_with_detail_and_3(self.request, pk=1)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
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')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, 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')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, CUSTOM_MESSAGE_1)

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')
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)

def test_permission_denied_with_custom_detail_not(self):
response = denied_view_with_detail_not(self.request, pk=1)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, INVERTED_MESSAGE)

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, self.custom_message)
self.assertNotEqual(detail, CUSTOM_MESSAGE_1)
self.assertNotEqual(detail.code, self.custom_code)

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, self.custom_message)
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)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, CUSTOM_MESSAGE_1)

def test_permission_denied_for_object_with_custom_detail_and_2(self):
response = denied_object_view_with_detail_and_2(self.request, pk=1)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, CUSTOM_MESSAGE_2)

def test_permission_denied_for_object_with_custom_detail_and_3(self):
response = denied_object_view_with_detail_and_3(self.request, pk=1)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
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')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, 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')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, CUSTOM_MESSAGE_1)

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')
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)

def test_permission_denied_for_object_with_custom_detail_not(self):
response = denied_object_view_with_detail_not(self.request, pk=1)
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, INVERTED_MESSAGE)


class PermissionsCompositionTests(TestCase):

Expand Down
Loading