Skip to content
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

Allow DjangoObjectPermissions to use views that define get_queryset #2905

Merged
merged 1 commit into from
May 13, 2015
Merged
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
29 changes: 18 additions & 11 deletions rest_framework/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,20 @@ def get_required_permissions(self, method, model_cls):
return [perm % kwargs for perm in self.perms_map[method]]

def has_permission(self, request, view):
# Workaround to ensure DjangoModelPermissions are not applied
# to the root view when using DefaultRouter.
if getattr(view, '_ignore_model_permissions', False):
return True

try:
queryset = view.get_queryset()
except AttributeError:
queryset = getattr(view, 'queryset', None)
except AssertionError:
# view.get_queryset() didn't find .queryset
queryset = None

# Workaround to ensure DjangoModelPermissions are not applied
# to the root view when using DefaultRouter.
if queryset is None and getattr(view, '_ignore_model_permissions', False):
return True

assert queryset is not None, (
'Cannot apply DjangoModelPermissions on a view that '
'does not have `.queryset` property nor redefines `.get_queryset()`.'
)
'does not have `.queryset` property or overrides the '
'`.get_queryset()` method.')

perms = self.get_required_permissions(request.method, queryset.model)

Expand Down Expand Up @@ -172,7 +169,17 @@ 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
try:
queryset = view.get_queryset()
except AttributeError:
queryset = getattr(view, 'queryset', None)

assert queryset is not None, (
'Cannot apply DjangoObjectPermissions on a view that '
'does not have `.queryset` property or overrides the '
'`.get_queryset()` method.')

model_cls = queryset.model
user = request.user

perms = self.get_required_object_permissions(request.method, model_cls)
Expand Down
36 changes: 36 additions & 0 deletions tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
from django.utils import unittest
from rest_framework import generics, serializers, status, permissions, authentication, HTTP_HEADER_ENCODING
from rest_framework.compat import guardian, get_model_name
from django.core.urlresolvers import ResolverMatch
from rest_framework.filters import DjangoObjectPermissionsFilter
from rest_framework.routers import DefaultRouter
from rest_framework.test import APIRequestFactory
from tests.models import BasicModel
import base64
Expand Down Expand Up @@ -49,6 +51,7 @@ class EmptyListView(generics.ListCreateAPIView):


root_view = RootView.as_view()
api_root_view = DefaultRouter().get_api_root_view()
instance_view = InstanceView.as_view()
get_queryset_list_view = GetQuerySetListView.as_view()
empty_list_view = EmptyListView.as_view()
Expand Down Expand Up @@ -86,6 +89,18 @@ def test_has_create_permissions(self):
response = root_view(request, pk=1)
self.assertEqual(response.status_code, status.HTTP_201_CREATED)

def test_api_root_view_discard_default_django_model_permission(self):
"""
We check that DEFAULT_PERMISSION_CLASSES can
apply to APIRoot view. More specifically we check expected behavior of
``_ignore_model_permissions`` attribute support.
"""
request = factory.get('/', format='json',
HTTP_AUTHORIZATION=self.permitted_credentials)
request.resolver_match = ResolverMatch('get', (), {})
response = api_root_view(request)
self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_get_queryset_has_create_permissions(self):
request = factory.post('/', {'text': 'foobar'}, format='json',
HTTP_AUTHORIZATION=self.permitted_credentials)
Expand Down Expand Up @@ -227,6 +242,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 +353,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