From 0618fa88e1a8c2cf8a2aab29ef6de66b49e5f7ed Mon Sep 17 00:00:00 2001 From: Manish Kumar Shah <45564404+ManishShah120@users.noreply.github.com> Date: Fri, 13 Jan 2023 14:22:54 +0530 Subject: [PATCH] Fix Respect `can_read_model` permission in DjangoModelPermissions (#8009) * Fix Respect `can_read_model` permission in DjangoModelPermissions FIXES: #6324 * Updated documentation and simplified code --- docs/api-guide/permissions.md | 3 ++- rest_framework/permissions.py | 11 ++++++++--- tests/test_permissions.py | 21 ++++++++++++++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/api-guide/permissions.md b/docs/api-guide/permissions.md index 34cbd0b73..27f7c5adb 100644 --- a/docs/api-guide/permissions.md +++ b/docs/api-guide/permissions.md @@ -173,11 +173,12 @@ This permission is suitable if you want to your API to allow read permissions to This permission class ties into Django's standard `django.contrib.auth` [model permissions][contribauth]. This permission must only be applied to views that have a `.queryset` property or `get_queryset()` method. Authorization will only be granted if the user *is authenticated* and has the *relevant model permissions* assigned. The appropriate model is determined by checking `get_queryset().model` or `queryset.model`. +* `GET` requests require the user to have the `view` or `change` permission on the model * `POST` requests require the user to have the `add` permission on the model. * `PUT` and `PATCH` requests require the user to have the `change` permission on the model. * `DELETE` requests require the user to have the `delete` permission on the model. -The default behavior can also be overridden to support custom model permissions. For example, you might want to include a `view` model permission for `GET` requests. +The default behaviour can also be overridden to support custom model permissions. To use custom model permissions, override `DjangoModelPermissions` and set the `.perms_map` property. Refer to the source code for details. diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 71de226f9..8fb4569cb 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -186,9 +186,9 @@ class DjangoModelPermissions(BasePermission): # Override this if you need to also provide 'view' permissions, # or if you want to provide custom permission codes. perms_map = { - 'GET': [], + 'GET': ['%(app_label)s.view_%(model_name)s'], 'OPTIONS': [], - 'HEAD': [], + 'HEAD': ['%(app_label)s.view_%(model_name)s'], 'POST': ['%(app_label)s.add_%(model_name)s'], 'PUT': ['%(app_label)s.change_%(model_name)s'], 'PATCH': ['%(app_label)s.change_%(model_name)s'], @@ -239,8 +239,13 @@ class DjangoModelPermissions(BasePermission): queryset = self._queryset(view) perms = self.get_required_permissions(request.method, queryset.model) + change_perm = self.get_required_permissions('PUT', queryset.model) - return request.user.has_perms(perms) + user = request.user + if request.method == 'GET': + return user.has_perms(perms) or user.has_perms(change_perm) + + return user.has_perms(perms) class DjangoModelPermissionsOrAnonReadOnly(DjangoModelPermissions): diff --git a/tests/test_permissions.py b/tests/test_permissions.py index aefff981e..428480dc7 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -80,7 +80,8 @@ class ModelPermissionsIntegrationTests(TestCase): user.user_permissions.set([ Permission.objects.get(codename='add_basicmodel'), Permission.objects.get(codename='change_basicmodel'), - Permission.objects.get(codename='delete_basicmodel') + Permission.objects.get(codename='delete_basicmodel'), + Permission.objects.get(codename='view_basicmodel') ]) user = User.objects.create_user('updateonly', 'updateonly@example.com', 'password') @@ -139,6 +140,15 @@ class ModelPermissionsIntegrationTests(TestCase): response = get_queryset_list_view(request, pk=1) self.assertEqual(response.status_code, status.HTTP_201_CREATED) + def test_has_get_permissions(self): + request = factory.get('/', HTTP_AUTHORIZATION=self.permitted_credentials) + response = root_view(request) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + request = factory.get('/1', HTTP_AUTHORIZATION=self.updateonly_credentials) + response = root_view(request, pk=1) + self.assertEqual(response.status_code, status.HTTP_200_OK) + def test_has_put_permissions(self): request = factory.put('/1', {'text': 'foobar'}, format='json', HTTP_AUTHORIZATION=self.permitted_credentials) @@ -156,6 +166,15 @@ class ModelPermissionsIntegrationTests(TestCase): response = root_view(request, pk=1) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_does_not_have_get_permissions(self): + request = factory.get('/', HTTP_AUTHORIZATION=self.disallowed_credentials) + response = root_view(request) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + request = factory.get('/1', HTTP_AUTHORIZATION=self.disallowed_credentials) + response = root_view(request, pk=1) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_does_not_have_put_permissions(self): request = factory.put('/1', {'text': 'foobar'}, format='json', HTTP_AUTHORIZATION=self.disallowed_credentials)