Fix Respect can_read_model permission in DjangoModelPermissions (#8009)

ManishShah120 · web-flow · commit 0618fa88e1a8 · 2023-01-13T14:52:54.000+06:00
* Fix Respect `can_read_model` permission in DjangoModelPermissions FIXES: #6324 * Updated documentation and simplified code
diff --git 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
@@ -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 @@ def has_permission(self, request, view):
 
         queryset = self._queryset(view)
         perms = self.get_required_permissions(request.method, queryset.model)
+        change_perm = self.get_required_permissions('PUT', queryset.model)
+
+        user = request.user
+        if request.method == 'GET':
+            return user.has_perms(perms) or user.has_perms(change_perm)
 
-        return request.user.has_perms(perms)
+        return user.has_perms(perms)
 
 
 class DjangoModelPermissionsOrAnonReadOnly(DjangoModelPermissions):
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
@@ -80,7 +80,8 @@ def setUp(self):
         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 @@ def test_get_queryset_has_create_permissions(self):
         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 @@ def test_does_not_have_create_permissions(self):
         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)
