Skip to content

Allow DjangoObjectPermissions to use views that define get_queryset #2864

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.

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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/api-guide/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ Similar to `DjangoModelPermissions`, but also allows unauthenticated users to ha

This permission class ties into Django's standard [object permissions framework][objectpermissions] that allows per-object permissions on models. In order to use this permission class, you'll also need to add a permission backend that supports object-level permissions, such as [django-guardian][guardian].

As with `DjangoModelPermissions`, this permission must only be applied to views that have a `.queryset` property. Authorization will only be granted if the user *is authenticated* and has the *relevant per-object permissions* and *relevant model permissions* assigned.
As with `DjangoModelPermissions`, 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 per-object permissions* and *relevant model permissions* assigned.

* `POST` requests require the user to have the `add` permission on the model instance.
* `PUT` and `PATCH` requests require the user to have the `change` permission on the model instance.
Expand Down
2 changes: 1 addition & 1 deletion rest_framework/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def get_required_object_permissions(self, method, model_cls):
return [perm % kwargs for perm in self.perms_map[method]]

def has_object_permission(self, request, view, obj):
model_cls = view.queryset.model
model_cls = view.get_queryset().model
Copy link
Member

Choose a reason for hiding this comment

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

Should we also fallback to view.queryset.model? That'd be consistent with the documentation above, and would be less likely to introduce any breakage to existing APIs.

Also then more consistent with #2863.

user = request.user

perms = self.get_required_object_permissions(request.method, model_cls)
Expand Down
21 changes: 21 additions & 0 deletions tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,18 @@ class ObjectPermissionListView(generics.ListAPIView):
object_permissions_list_view = ObjectPermissionListView.as_view()


class GetQuerysetObjectPermissionInstanceView(generics.RetrieveUpdateDestroyAPIView):
serializer_class = BasicPermSerializer
authentication_classes = [authentication.BasicAuthentication]
permission_classes = [ViewObjectPermissions]

def get_queryset(self):
return BasicPermModel.objects.all()


get_queryset_object_permissions_view = GetQuerysetObjectPermissionInstanceView.as_view()


@unittest.skipUnless(guardian, 'django-guardian not installed')
class ObjectPermissionsIntegrationTests(TestCase):
"""
Expand Down Expand Up @@ -326,6 +338,15 @@ def test_cannot_read_permissions(self):
response = object_permissions_view(request, pk='1')
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)

def test_can_read_get_queryset_permissions(self):
"""
same as ``test_can_read_permissions`` but with a view
that rely on ``.get_queryset()`` instead of ``.queryset``.
"""
request = factory.get('/1', HTTP_AUTHORIZATION=self.credentials['readonly'])
response = get_queryset_object_permissions_view(request, pk='1')
self.assertEqual(response.status_code, status.HTTP_200_OK)

# Read list
def test_can_read_list_permissions(self):
request = factory.get('/', HTTP_AUTHORIZATION=self.credentials['readonly'])
Expand Down