Tweaks and docs to object-level model permissions.

docs/api-guide/filtering.md

@@ -257,6 +257,49 @@ The `ordering` attribute may be either a string or a list/tuple of strings.
 
 ---
 
+## DjangoObjectPermissionsFilter
+
+The `DjangoObjectPermissionsFilter` is intended to be used together with the [`django-guardian`][guardian] package, with custom `'view'` permissions added.  The filter will ensure that querysets only returns objects for which the user has the appropriate view permission.
+
+This filter class must be used with views that provide either a `queryset` or a `model` attribute.
+
+If you're using `DjangoObjectPermissionsFilter`, you'll probably also want to add an appropriate object permissions class, to ensure that users can only operate on instances if they have the appropriate object permissions.  The easiest way to do this is to subclass `DjangoObjectPermissions` and add `'view'` permissions to the `perms_map` attribute.
+
+A complete example using both `DjangoObjectPermissionsFilter` and `DjangoObjectPermissions` might look something like this.
+
+**permissions.py**:
+
+    class CustomObjectPermissions(permissions.DjangoObjectPermissions):
+		"""
+		Similar to `DjangoObjectPermissions`, but adding 'view' permissions.
+		"""
+        perms_map = {
+            'GET': ['%(app_label)s.view_%(model_name)s'],
+            'OPTIONS': ['%(app_label)s.view_%(model_name)s'],
+            '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'],
+            'DELETE': ['%(app_label)s.delete_%(model_name)s'],
+        }
+
+**views.py**:
+
+    class EventViewSet(viewsets.ModelViewSet):
+    	"""
+    	Viewset that only lists events if user has 'view' permissions, and only
+    	allows operations on individual events if user has appropriate 'view', 'add',
+    	'change' or 'delete' permissions.
+		"""
+        queryset = Event.objects.all()
+        serializer = EventSerializer
+        filter_backends = (filters.DjangoObjectPermissionsFilter,)
+        permission_classes = (myapp.permissions.CustomObjectPermissions,)
+
+For more information on adding `'view'` permissions for models, see the [relevant section][view-permissions] of the `django-guardian` documentation, and [this blogpost][view-permissions-blogpost].
+
+---
+
 # Custom generic filtering
 
 You can also provide your own generic filtering backend, or write an installable app for other developers to use.
@@ -281,5 +324,8 @@ We could achieve the same behavior by overriding `get_queryset()` on the views,
 [cite]: https://docs.djangoproject.com/en/dev/topics/db/queries/#retrieving-specific-objects-with-filters
 [django-filter]: https://github.com/alex/django-filter
 [django-filter-docs]: https://django-filter.readthedocs.org/en/latest/index.html
+[guardian]: http://pythonhosted.org/django-guardian/
+[view-permissions]: http://pythonhosted.org/django-guardian/userguide/assign.html
+[view-permissions-blogpost]: http://blog.nyaruka.com/adding-a-view-permission-to-django-models
 [nullbooleanselect]: https://github.com/django/django/blob/master/django/forms/widgets.py
 [search-django-admin]: https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.search_fields

docs/api-guide/permissions.md

@@ -120,7 +120,21 @@ To use custom model permissions, override `DjangoModelPermissions` and set the `
 
 ## DjangoModelPermissionsOrAnonReadOnly
 
-Similar to `DjangoModelPermissions`, but also allows unauthenticated users to have  read-only access to the API.
+Similar to `DjangoModelPermissions`, but also allows unauthenticated users to have read-only access to the API.
+
+## DjangoObjectPermissions
+
+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].
+
+When applied to a view that has a `.model` property, 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.
+* `DELETE` requests require the user to have the `delete` permission on the model instance.
+
+Note that `DjangoObjectPermissions` **does not** require the `django-guardian` package, and should support other object-level backends equally well.
+
+As with `DjangoModelPermissions` you can use custom model permissions by overriding `DjangoModelPermissions` and setting the `.perms_map` property.  Refer to the source code for details.  Note that if you add a custom `view` permission for `GET`, `HEAD` and `OPTIONS` requests, you'll probably also want to consider adding the `DjangoObjectPermissionsFilter` class to ensure that list endpoints only return results including objects for which the user has appropriate view permissions.
 
 ## TokenHasReadWriteScope
 
@@ -220,7 +234,9 @@ The [Composed Permissions][composed-permissions] package provides a simple way t
 [authentication]: authentication.md
 [throttling]: throttling.md
 [contribauth]: https://docs.djangoproject.com/en/1.0/topics/auth/#permissions
+[objectpermissions]: https://docs.djangoproject.com/en/dev/topics/auth/customizing/#handling-object-permissions
 [guardian]: https://github.com/lukaszb/django-guardian
+[get_objects_for_user]: http://pythonhosted.org/django-guardian/api/guardian.shortcuts.html#get-objects-for-user
 [django-oauth-plus]: http://code.larlet.fr/django-oauth-plus
 [django-oauth2-provider]: https://github.com/caffeinehit/django-oauth2-provider
 [2.2-announcement]: ../topics/2.2-announcement.md

rest_framework/filters.py

@@ -23,22 +23,6 @@ class BaseFilterBackend(object):
         raise NotImplementedError(".filter_queryset() must be overridden.")
 
 
-class ObjectPermissionReaderFilter(BaseFilterBackend):
-    """
-    A filter backend that limits results to those where the requesting user
-    has read object level permissions.
-    """
-    def __init__(self):
-        assert guardian, 'Using ObjectPermissionReaderFilter, but django-guardian is not installed'
-
-    def filter_queryset(self, request, queryset, view):
-        user = request.user
-        model_cls = queryset.model
-        model_name = model_cls._meta.module_name
-        permission = 'read_' + model_name
-        return guardian.shortcuts.get_objects_for_user(user, permission, queryset)
-
-
 class DjangoFilterBackend(BaseFilterBackend):
     """
     A filter backend that uses django-filter.
@@ -156,3 +140,24 @@ class OrderingFilter(BaseFilterBackend):
             return queryset.order_by(*ordering)
 
         return queryset
+
+
+class DjangoObjectPermissionsFilter(BaseFilterBackend):
+    """
+    A filter backend that limits results to those where the requesting user
+    has read object level permissions.
+    """
+    def __init__(self):
+        assert guardian, 'Using DjangoObjectPermissionsFilter, but django-guardian is not installed'
+
+    perm_format = '%(app_label)s.view_%(model_name)s'
+
+    def filter_queryset(self, request, queryset, view):
+        user = request.user
+        model_cls = queryset.model
+        kwargs = {
+            'app_label': model_cls._meta.app_label,
+            'model_name': model_cls._meta.module_name
+        }
+        permission = self.perm_format % kwargs
+        return guardian.shortcuts.get_objects_for_user(user, permission, queryset)

rest_framework/permissions.py

@@ -152,10 +152,10 @@ class DjangoModelPermissionsOrAnonReadOnly(DjangoModelPermissions):
     authenticated_users_only = False
 
 
-class DjangoObjectLevelModelPermissions(DjangoModelPermissions):
+class DjangoObjectPermissions(DjangoModelPermissions):
     """
-    The request is authenticated using `django.contrib.auth` permissions.
-    See: https://docs.djangoproject.com/en/dev/topics/auth/#permissions
+    The request is authenticated using Django's object-level permissions.
+    It requires an object-permissions-enabled backend, such as Django Guardian.
 
     It ensures that the user is authenticated, and has the appropriate
     `add`/`change`/`delete` permissions on the object using .has_perms.
@@ -164,21 +164,22 @@ class DjangoObjectLevelModelPermissions(DjangoModelPermissions):
     provide a `.model` or `.queryset` attribute.
     """
 
-    actions_map = {
-        'GET': ['read_%(model_name)s'],
-        'OPTIONS': ['read_%(model_name)s'],
-        'HEAD': ['read_%(model_name)s'],
-        'POST': ['add_%(model_name)s'],
-        'PUT': ['change_%(model_name)s'],
-        'PATCH': ['change_%(model_name)s'],
-        'DELETE': ['delete_%(model_name)s'],
+    perms_map = {
+        'GET': [],
+        'OPTIONS': [],
+        'HEAD': [],
+        'POST': ['%(app_label)s.add_%(model_name)s'],
+        'PUT': ['%(app_label)s.change_%(model_name)s'],
+        'PATCH': ['%(app_label)s.change_%(model_name)s'],
+        'DELETE': ['%(app_label)s.delete_%(model_name)s'],
     }
 
     def get_required_object_permissions(self, method, model_cls):
         kwargs = {
+            'app_label': model_cls._meta.app_label,
             'model_name': model_cls._meta.module_name
         }
-        return [perm % kwargs for perm in self.actions_map[method]]
+        return [perm % kwargs for perm in self.perms_map[method]]
 
     def has_object_permission(self, request, view, obj):
         model_cls = getattr(view, 'model', None)
@@ -190,10 +191,24 @@ class DjangoObjectLevelModelPermissions(DjangoModelPermissions):
         perms = self.get_required_object_permissions(request.method, model_cls)
         user = request.user
 
-        check = user.has_perms(perms, obj)
-        if not check:
-            raise Http404
-        return user.has_perms(perms, obj)
+        if not user.has_perms(perms, obj):
+            # If the user does not have permissions we need to determine if
+            # they have read permissions to see 403, or not, and simply see
+            # a 404 reponse.
+
+            if request.method in ('GET', 'OPTIONS', 'HEAD'):
+                # Read permissions already checked and failed, no need
+                # to make another lookup.
+                raise Http404
+
+            read_perms = self.get_required_object_permissions('GET', model_cls)
+            if not user.has_perms(read_perms, obj):
+                raise Http404
+
+            # Has read permissions.
+            return False
+
+        return True
 
 
 class TokenHasReadWriteScope(BasePermission):

rest_framework/tests/test_permissions.py

@@ -2,9 +2,10 @@ from __future__ import unicode_literals
 from django.contrib.auth.models import User, Permission, Group
 from django.db import models
 from django.test import TestCase
+from django.utils import unittest
 from rest_framework import generics, status, permissions, authentication, HTTP_HEADER_ENCODING
 from rest_framework.compat import guardian
-from rest_framework.filters import ObjectPermissionReaderFilter
+from rest_framework.filters import DjangoObjectPermissionsFilter
 from rest_framework.test import APIRequestFactory
 from rest_framework.tests.models import BasicModel
 import base64
@@ -148,130 +149,152 @@ class BasicPermModel(models.Model):
     class Meta:
         app_label = 'tests'
         permissions = (
-            ('read_basicpermmodel', 'Can view basic perm model'),
+            ('view_basicpermmodel', 'Can view basic perm model'),
             # add, change, delete built in to django
         )
 
+# Custom object-level permission, that includes 'view' permissions
+class ViewObjectPermissions(permissions.DjangoObjectPermissions):
+    perms_map = {
+        'GET': ['%(app_label)s.view_%(model_name)s'],
+        'OPTIONS': ['%(app_label)s.view_%(model_name)s'],
+        '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'],
+        'DELETE': ['%(app_label)s.delete_%(model_name)s'],
+    }
+
+
 class ObjectPermissionInstanceView(generics.RetrieveUpdateDestroyAPIView):
     model = BasicPermModel
     authentication_classes = [authentication.BasicAuthentication]
-    permission_classes = [permissions.DjangoObjectLevelModelPermissions]
+    permission_classes = [ViewObjectPermissions]
 
 object_permissions_view = ObjectPermissionInstanceView.as_view()
 
+
 class ObjectPermissionListView(generics.ListAPIView):
     model = BasicPermModel
     authentication_classes = [authentication.BasicAuthentication]
-    permission_classes = [permissions.DjangoObjectLevelModelPermissions]
+    permission_classes = [ViewObjectPermissions]
 
 object_permissions_list_view = ObjectPermissionListView.as_view()
 
-if guardian:
-    from guardian.shortcuts import assign_perm
-
-    class ObjectPermissionsIntegrationTests(TestCase):
-        """
-        Integration tests for the object level permissions API.
-        """
-        @classmethod
-        def setUpClass(cls):
-            # create users
-            create = User.objects.create_user
-            users = {
-                'fullaccess': create('fullaccess', 'fullaccess@example.com', 'password'),
-                'readonly': create('readonly', 'readonly@example.com', 'password'),
-                'writeonly': create('writeonly', 'writeonly@example.com', 'password'),
-                'deleteonly': create('deleteonly', 'deleteonly@example.com', 'password'),
-            }
-
-            # give everyone model level permissions, as we are not testing those
-            everyone = Group.objects.create(name='everyone')
-            model_name = BasicPermModel._meta.module_name
-            app_label = BasicPermModel._meta.app_label
-            f = '{0}_{1}'.format
-            perms = {
-                'read':   f('read', model_name),
-                'change': f('change', model_name),
-                'delete': f('delete', model_name)
-            }
-            for perm in perms.values():
-                perm = '{0}.{1}'.format(app_label, perm)
-                assign_perm(perm, everyone)
-            everyone.user_set.add(*users.values())
-
-            cls.perms = perms
-            cls.users = users
-
-        def setUp(self):
-            perms = self.perms
-            users = self.users
-
-            # appropriate object level permissions
-            readers = Group.objects.create(name='readers')
-            writers = Group.objects.create(name='writers')
-            deleters = Group.objects.create(name='deleters')
-
-            model = BasicPermModel.objects.create(text='foo')
-            
-            assign_perm(perms['read'], readers, model)
-            assign_perm(perms['change'], writers, model)
-            assign_perm(perms['delete'], deleters, model)
-
-            readers.user_set.add(users['fullaccess'], users['readonly'])
-            writers.user_set.add(users['fullaccess'], users['writeonly'])
-            deleters.user_set.add(users['fullaccess'], users['deleteonly'])
-
-            self.credentials = {}
-            for user in users.values():
-                self.credentials[user.username] = basic_auth_header(user.username, 'password')
-
-        # Delete
-        def test_can_delete_permissions(self):
-            request = factory.delete('/1', HTTP_AUTHORIZATION=self.credentials['deleteonly'])
-            response = object_permissions_view(request, pk='1')
-            self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
-
-        def test_cannot_delete_permissions(self):
-            request = factory.delete('/1', HTTP_AUTHORIZATION=self.credentials['readonly'])
-            response = object_permissions_view(request, pk='1')
-            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-
-        # Update
-        def test_can_update_permissions(self):
-            request = factory.patch('/1', {'text': 'foobar'}, format='json',
-                HTTP_AUTHORIZATION=self.credentials['writeonly'])
-            response = object_permissions_view(request, pk='1')
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-            self.assertEqual(response.data.get('text'), 'foobar')
-
-        def test_cannot_update_permissions(self):
-            request = factory.patch('/1', {'text': 'foobar'}, format='json',
-                HTTP_AUTHORIZATION=self.credentials['deleteonly'])
-            response = object_permissions_view(request, pk='1')
-            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-
-        # Read
-        def test_can_read_permissions(self):
-            request = factory.get('/1', HTTP_AUTHORIZATION=self.credentials['readonly'])
-            response = object_permissions_view(request, pk='1')
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-
-        def test_cannot_read_permissions(self):
-            request = factory.get('/1', HTTP_AUTHORIZATION=self.credentials['writeonly'])
-            response = object_permissions_view(request, pk='1')
-            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-
-        # Read list
-        def test_can_read_list_permissions(self):
-            request = factory.get('/', HTTP_AUTHORIZATION=self.credentials['readonly'])
-            object_permissions_list_view.cls.filter_backends = (ObjectPermissionReaderFilter,)
-            response = object_permissions_list_view(request)
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-            self.assertEqual(response.data[0].get('id'), 1)
-
-        def test_cannot_read_list_permissions(self):
-            request = factory.get('/', HTTP_AUTHORIZATION=self.credentials['writeonly'])
-            object_permissions_list_view.cls.filter_backends = (ObjectPermissionReaderFilter,)
-            response = object_permissions_list_view(request)
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-            self.assertListEqual(response.data, [])
+
+@unittest.skipUnless(guardian, 'django-guardian not installed')
+class ObjectPermissionsIntegrationTests(TestCase):
+    """
+    Integration tests for the object level permissions API.
+    """
+    @classmethod
+    def setUpClass(cls):
+        from guardian.shortcuts import assign_perm
+
+        # create users
+        create = User.objects.create_user
+        users = {
+            'fullaccess': create('fullaccess', 'fullaccess@example.com', 'password'),
+            'readonly': create('readonly', 'readonly@example.com', 'password'),
+            'writeonly': create('writeonly', 'writeonly@example.com', 'password'),
+            'deleteonly': create('deleteonly', 'deleteonly@example.com', 'password'),
+        }
+
+        # give everyone model level permissions, as we are not testing those
+        everyone = Group.objects.create(name='everyone')
+        model_name = BasicPermModel._meta.module_name
+        app_label = BasicPermModel._meta.app_label
+        f = '{0}_{1}'.format
+        perms = {
+            'view':   f('view', model_name),
+            'change': f('change', model_name),
+            'delete': f('delete', model_name)
+        }
+        for perm in perms.values():
+            perm = '{0}.{1}'.format(app_label, perm)
+            assign_perm(perm, everyone)
+        everyone.user_set.add(*users.values())
+
+        cls.perms = perms
+        cls.users = users
+
+    def setUp(self):
+        from guardian.shortcuts import assign_perm
+        perms = self.perms
+        users = self.users
+
+        # appropriate object level permissions
+        readers = Group.objects.create(name='readers')
+        writers = Group.objects.create(name='writers')
+        deleters = Group.objects.create(name='deleters')
+
+        model = BasicPermModel.objects.create(text='foo')
+        
+        assign_perm(perms['view'], readers, model)
+        assign_perm(perms['change'], writers, model)
+        assign_perm(perms['delete'], deleters, model)
+
+        readers.user_set.add(users['fullaccess'], users['readonly'])
+        writers.user_set.add(users['fullaccess'], users['writeonly'])
+        deleters.user_set.add(users['fullaccess'], users['deleteonly'])
+
+        self.credentials = {}
+        for user in users.values():
+            self.credentials[user.username] = basic_auth_header(user.username, 'password')
+
+    # Delete
+    def test_can_delete_permissions(self):
+        request = factory.delete('/1', HTTP_AUTHORIZATION=self.credentials['deleteonly'])
+        response = object_permissions_view(request, pk='1')
+        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
+    def test_cannot_delete_permissions(self):
+        request = factory.delete('/1', HTTP_AUTHORIZATION=self.credentials['readonly'])
+        response = object_permissions_view(request, pk='1')
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    # Update
+    def test_can_update_permissions(self):
+        request = factory.patch('/1', {'text': 'foobar'}, format='json',
+            HTTP_AUTHORIZATION=self.credentials['writeonly'])
+        response = object_permissions_view(request, pk='1')
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data.get('text'), 'foobar')
+
+    def test_cannot_update_permissions(self):
+        request = factory.patch('/1', {'text': 'foobar'}, format='json',
+            HTTP_AUTHORIZATION=self.credentials['deleteonly'])
+        response = object_permissions_view(request, pk='1')
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    def test_cannot_update_permissions_non_existing(self):
+        request = factory.patch('/999', {'text': 'foobar'}, format='json',
+            HTTP_AUTHORIZATION=self.credentials['deleteonly'])
+        response = object_permissions_view(request, pk='999')
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    # Read
+    def test_can_read_permissions(self):
+        request = factory.get('/1', HTTP_AUTHORIZATION=self.credentials['readonly'])
+        response = object_permissions_view(request, pk='1')
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+    def test_cannot_read_permissions(self):
+        request = factory.get('/1', HTTP_AUTHORIZATION=self.credentials['writeonly'])
+        response = object_permissions_view(request, pk='1')
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    # Read list
+    def test_can_read_list_permissions(self):
+        request = factory.get('/', HTTP_AUTHORIZATION=self.credentials['readonly'])
+        object_permissions_list_view.cls.filter_backends = (DjangoObjectPermissionsFilter,)
+        response = object_permissions_list_view(request)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
\ No newline at end of file
+        self.assertEqual(response.data[0].get('id'), 1)
+
+    def test_cannot_read_list_permissions(self):
+        request = factory.get('/', HTTP_AUTHORIZATION=self.credentials['writeonly'])
+        object_permissions_list_view.cls.filter_backends = (DjangoObjectPermissionsFilter,)
+        response = object_permissions_list_view(request)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertListEqual(response.data, [])