Skip to content

Commit

Permalink
Fix Respect can_read_model permission in DjangoModelPermissions (#8009
Browse files Browse the repository at this point in the history
)

* Fix Respect `can_read_model` permission in DjangoModelPermissions

FIXES: #6324

* Updated documentation and simplified code
  • Loading branch information
ManishShah120 authored Jan 13, 2023
1 parent 2d19f23 commit 0618fa8
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 5 deletions.
3 changes: 2 additions & 1 deletion docs/api-guide/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
11 changes: 8 additions & 3 deletions rest_framework/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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):
Expand Down
21 changes: 20 additions & 1 deletion tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 0618fa8

Please sign in to comment.