From 4a9dcfa76089143bbeb5cd43fa3a406365d89e96 Mon Sep 17 00:00:00 2001 From: bwreilly Date: Fri, 6 Sep 2013 11:01:31 -0500 Subject: [PATCH 01/16] added guardian as optional requirement, stubbed out object-level permission class --- docs/index.md | 1 + rest_framework/compat.py | 6 ++++++ rest_framework/permissions.py | 7 ++++++- rest_framework/tests/test_permissions.py | 1 + 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index e0a2e911b..d83fbff1f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -42,6 +42,7 @@ The following packages are optional: * [django-filter][django-filter] (0.5.4+) - Filtering support. * [django-oauth-plus][django-oauth-plus] (2.0+) and [oauth2][oauth2] (1.5.211+) - OAuth 1.0a support. * [django-oauth2-provider][django-oauth2-provider] (0.2.3+) - OAuth 2.0 support. +* [django-guardian][django-guardian] (1.1.1+) - Object level permissions support. **Note**: The `oauth2` Python package is badly misnamed, and actually provides OAuth 1.0a support. Also note that packages required for both OAuth 1.0a, and OAuth 2.0 are not yet Python 3 compatible. diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 6f7447add..b9d1dae6b 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -47,6 +47,12 @@ try: except ImportError: django_filters = None +# guardian is optional +try: + import guardian +except ImportError: + guardian = None + # cStringIO only if it's available, otherwise StringIO try: diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 1036663e0..6d213ba12 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -7,7 +7,7 @@ import warnings SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS'] -from rest_framework.compat import oauth2_provider_scope, oauth2_constants +from rest_framework.compat import oauth2_provider_scope, oauth2_constants, guardian class BasePermission(object): @@ -151,6 +151,11 @@ class DjangoModelPermissionsOrAnonReadOnly(DjangoModelPermissions): authenticated_users_only = False +class DjangoObjectLevelModelPermissions(DjangoModelPermissions): + def __init__(self): + assert guardian, 'Using DjangoObjectLevelModelPermissions, but guardian is not installed' + + class TokenHasReadWriteScope(BasePermission): """ The request is authenticated as a user and the token used has the right scope diff --git a/rest_framework/tests/test_permissions.py b/rest_framework/tests/test_permissions.py index e2cca3808..d1171cce5 100644 --- a/rest_framework/tests/test_permissions.py +++ b/rest_framework/tests/test_permissions.py @@ -4,6 +4,7 @@ from django.db import models from django.test import TestCase from rest_framework import generics, status, permissions, authentication, HTTP_HEADER_ENCODING from rest_framework.test import APIRequestFactory +from rest_framework.compat import guardian import base64 factory = APIRequestFactory() From b07de86ad372a185c05c1dd77ccd7bee3801996e Mon Sep 17 00:00:00 2001 From: bwreilly Date: Fri, 6 Sep 2013 12:35:06 -0500 Subject: [PATCH 02/16] some properly failing tests, set up for standard permissions --- rest_framework/permissions.py | 2 +- rest_framework/runtests/settings.py | 11 +++ rest_framework/tests/test_permissions.py | 99 ++++++++++++++---------- tox.ini | 8 ++ 4 files changed, 79 insertions(+), 41 deletions(-) diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 6d213ba12..b67be4141 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -153,7 +153,7 @@ class DjangoModelPermissionsOrAnonReadOnly(DjangoModelPermissions): class DjangoObjectLevelModelPermissions(DjangoModelPermissions): def __init__(self): - assert guardian, 'Using DjangoObjectLevelModelPermissions, but guardian is not installed' + assert guardian, 'Using DjangoObjectLevelModelPermissions, but django-guardian is not installed' class TokenHasReadWriteScope(BasePermission): diff --git a/rest_framework/runtests/settings.py b/rest_framework/runtests/settings.py index b3702d0bf..6750376f0 100644 --- a/rest_framework/runtests/settings.py +++ b/rest_framework/runtests/settings.py @@ -123,6 +123,17 @@ else: 'provider.oauth2', ) +# guardian is optional +try: + import guardian +except ImportError: + pass +else: + ANONYMOUS_USER_ID = -1 + INSTALLED_APPS += ( + 'guardian', + ) + STATIC_URL = '/static/' PASSWORD_HASHERS = ( diff --git a/rest_framework/tests/test_permissions.py b/rest_framework/tests/test_permissions.py index d1171cce5..dcdb4eea6 100644 --- a/rest_framework/tests/test_permissions.py +++ b/rest_framework/tests/test_permissions.py @@ -3,17 +3,14 @@ from django.contrib.auth.models import User, Permission from django.db import models from django.test import TestCase from rest_framework import generics, status, permissions, authentication, HTTP_HEADER_ENCODING -from rest_framework.test import APIRequestFactory from rest_framework.compat import guardian +from rest_framework.test import APIRequestFactory +from rest_framework.tests.models import BasicModel +from rest_framework.settings import api_settings import base64 factory = APIRequestFactory() - -class BasicModel(models.Model): - text = models.CharField(max_length=100) - - class RootView(generics.ListCreateAPIView): model = BasicModel authentication_classes = [authentication.BasicAuthentication] @@ -145,45 +142,67 @@ class ModelPermissionsIntegrationTests(TestCase): self.assertEqual(list(response.data['actions'].keys()), ['PUT']) -class OwnerModel(models.Model): - text = models.CharField(max_length=100) - owner = models.ForeignKey(User) +class BasicPermModel(BasicModel): + class Meta: + app_label = 'tests' + permissions = ( + ('read_basicpermmodel', "Can view basic perm model"), + # add, change, delete built in to django + ) -class IsOwnerPermission(permissions.BasePermission): - def has_object_permission(self, request, view, obj): - return request.user == obj.owner - - -class OwnerInstanceView(generics.RetrieveUpdateDestroyAPIView): - model = OwnerModel +class ObjectPermissionInstanceView(generics.RetrieveUpdateDestroyAPIView): + model = BasicModel authentication_classes = [authentication.BasicAuthentication] - permission_classes = [IsOwnerPermission] + permission_classes = [permissions.DjangoObjectLevelModelPermissions] -owner_instance_view = OwnerInstanceView.as_view() +object_permissions_view = ObjectPermissionInstanceView.as_view() + +if guardian: + class ObjectPermissionsIntegrationTests(TestCase): + """ + Integration tests for the object level permissions API. + """ + + def setUp(self): + # create users + User.objects.create_user('no_permission', 'no_permission@example.com', 'password') + reader = User.objects.create_user('reader', 'reader@example.com', 'password') + writer = User.objects.create_user('writer', 'writer@example.com', 'password') + full_access = User.objects.create_user('full_access', 'full_access@example.com', 'password') + + model = BasicPermModel.objects.create(text='foo') + + # assign permissions appropriately + from guardian.shortcuts import assign_perm + + read = "read_basicpermmodel" + write = "change_basicpermmodel" + delete = "delete_basicpermmodel" + app_label = 'tests.' + # model level permissions + assign_perm(app_label + delete, full_access, obj=model) + (assign_perm(app_label + write, user, obj=model) for user in (writer, full_access)) + (assign_perm(app_label + read, user, obj=model) for user in (reader, writer, full_access)) + + # object level permissions + assign_perm(delete, full_access, obj=model) + (assign_perm(write, user, obj=model) for user in (writer, full_access)) + (assign_perm(read, user, obj=model) for user in (reader, writer, full_access)) + + self.no_permission_credentials = basic_auth_header('no_permission', 'password') + self.reader_credentials = basic_auth_header('reader', 'password') + self.writer_credentials = basic_auth_header('writer', 'password') + self.full_access_credentials = basic_auth_header('full_access', 'password') -class ObjectPermissionsIntegrationTests(TestCase): - """ - Integration tests for the object level permissions API. - """ + def test_has_delete_permissions(self): + request = factory.delete('/1', HTTP_AUTHORIZATION=self.full_access_credentials) + response = object_permissions_view(request, pk='1') + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - def setUp(self): - User.objects.create_user('not_owner', 'not_owner@example.com', 'password') - user = User.objects.create_user('owner', 'owner@example.com', 'password') - - self.not_owner_credentials = basic_auth_header('not_owner', 'password') - self.owner_credentials = basic_auth_header('owner', 'password') - - OwnerModel(text='foo', owner=user).save() - - def test_owner_has_delete_permissions(self): - request = factory.delete('/1', HTTP_AUTHORIZATION=self.owner_credentials) - response = owner_instance_view(request, pk='1') - self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - - def test_non_owner_does_not_have_delete_permissions(self): - request = factory.delete('/1', HTTP_AUTHORIZATION=self.not_owner_credentials) - response = owner_instance_view(request, pk='1') - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_no_delete_permissions(self): + request = factory.delete('/1', HTTP_AUTHORIZATION=self.writer_credentials) + response = object_permissions_view(request, pk='1') + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) diff --git a/tox.ini b/tox.ini index aa97fd350..6e3b8e0a8 100644 --- a/tox.ini +++ b/tox.ini @@ -25,6 +25,7 @@ deps = https://www.djangoproject.com/download/1.6a1/tarball/ django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.4 + django-guardian==1.1.1 [testenv:py2.6-django1.6] basepython = python2.6 @@ -34,6 +35,7 @@ deps = https://www.djangoproject.com/download/1.6a1/tarball/ django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.4 + django-guardian==1.1.1 [testenv:py3.3-django1.5] basepython = python3.3 @@ -55,6 +57,7 @@ deps = django==1.5 django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.3 + django-guardian==1.1.1 [testenv:py2.6-django1.5] basepython = python2.6 @@ -64,6 +67,7 @@ deps = django==1.5 django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.3 + django-guardian==1.1.1 [testenv:py2.7-django1.4] basepython = python2.7 @@ -73,6 +77,7 @@ deps = django==1.4.3 django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.3 + django-guardian==1.1.1 [testenv:py2.6-django1.4] basepython = python2.6 @@ -82,6 +87,7 @@ deps = django==1.4.3 django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.3 + django-guardian==1.1.1 [testenv:py2.7-django1.3] basepython = python2.7 @@ -91,6 +97,7 @@ deps = django==1.3.5 django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.3 + django-guardian==1.1.1 [testenv:py2.6-django1.3] basepython = python2.6 @@ -100,3 +107,4 @@ deps = django==1.3.5 django-oauth-plus==2.0 oauth2==1.5.211 django-oauth2-provider==0.2.3 + django-guardian==1.1.1 From 57d6b5fb7c2652bb4c68edd1bcc95be736b06b31 Mon Sep 17 00:00:00 2001 From: bwreilly Date: Sat, 7 Sep 2013 23:16:43 -0500 Subject: [PATCH 03/16] necessary test settings for guardian --- rest_framework/runtests/settings.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rest_framework/runtests/settings.py b/rest_framework/runtests/settings.py index 6750376f0..be7216580 100644 --- a/rest_framework/runtests/settings.py +++ b/rest_framework/runtests/settings.py @@ -130,6 +130,10 @@ except ImportError: pass else: ANONYMOUS_USER_ID = -1 + AUTHENTICATION_BACKENDS = ( + 'django.contrib.auth.backends.ModelBackend', # default + 'guardian.backends.ObjectPermissionBackend', + ) INSTALLED_APPS += ( 'guardian', ) From 118645e4806effaa35726012a983676b2c55b6dd Mon Sep 17 00:00:00 2001 From: bwreilly Date: Sat, 7 Sep 2013 23:18:52 -0500 Subject: [PATCH 04/16] first pass at object level permissions and tests --- rest_framework/permissions.py | 46 +++++++ rest_framework/tests/test_permissions.py | 148 +++++++++++++++++------ 2 files changed, 157 insertions(+), 37 deletions(-) diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index b67be4141..2d8a30e91 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -7,6 +7,7 @@ import warnings SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS'] +from django.http import Http404 from rest_framework.compat import oauth2_provider_scope, oauth2_constants, guardian @@ -152,9 +153,54 @@ class DjangoModelPermissionsOrAnonReadOnly(DjangoModelPermissions): class DjangoObjectLevelModelPermissions(DjangoModelPermissions): + """ + Basic object level permissions utilizing django-guardian. + """ + def __init__(self): assert guardian, 'Using DjangoObjectLevelModelPermissions, but django-guardian is not installed' + action_perm_map = { + 'list': 'read', + 'retrieve': 'read', + 'create': 'add', + 'partial_update': 'change', + 'update': 'change', + 'destroy': 'delete', + } + + def _get_names(self, view): + model_cls = getattr(view, 'model', None) + queryset = getattr(view, 'queryset', None) + + if model_cls is None and queryset is not None: + model_cls = queryset.model + if not model_cls: # no model, no model based permissions + return None + model_name = model_cls._meta.module_name + return model_name + + def has_permission(self, request, view): + if view.action == 'list': + user = request.user + queryset = view.get_queryset() + model_name = self._get_names(view) + view.queryset = guardian.shortcuts.get_objects_for_user(user, 'read_' + model_name, queryset) #TODO: move to filter + return super(DjangoObjectLevelModelPermissions, self).has_permission(request, view) + + def has_object_permission(self, request, view, obj): + user = request.user + model_name = self._get_names(view) + action = self.action_perm_map.get(view.action) + + assert action, "Tried to determine object permissions but no action specified in view" + + perm = "{action}_{model_name}".format(action=action, model_name=model_name) + check = user.has_perm(perm, obj) + if not check: + raise Http404 + return user.has_perm(perm, obj) + class TokenHasReadWriteScope(BasePermission): """ diff --git a/rest_framework/tests/test_permissions.py b/rest_framework/tests/test_permissions.py index dcdb4eea6..d64ab04eb 100644 --- a/rest_framework/tests/test_permissions.py +++ b/rest_framework/tests/test_permissions.py @@ -1,12 +1,11 @@ from __future__ import unicode_literals -from django.contrib.auth.models import User, Permission +from django.contrib.auth.models import User, Permission, Group from django.db import models from django.test import TestCase from rest_framework import generics, status, permissions, authentication, HTTP_HEADER_ENCODING from rest_framework.compat import guardian from rest_framework.test import APIRequestFactory from rest_framework.tests.models import BasicModel -from rest_framework.settings import api_settings import base64 factory = APIRequestFactory() @@ -142,67 +141,142 @@ class ModelPermissionsIntegrationTests(TestCase): self.assertEqual(list(response.data['actions'].keys()), ['PUT']) -class BasicPermModel(BasicModel): +class BasicPermModel(models.Model): + text = models.CharField(max_length=100) class Meta: app_label = 'tests' permissions = ( - ('read_basicpermmodel', "Can view basic perm model"), + ('read_basicpermmodel', 'Can view basic perm model'), # add, change, delete built in to django ) class ObjectPermissionInstanceView(generics.RetrieveUpdateDestroyAPIView): - model = BasicModel + model = BasicPermModel authentication_classes = [authentication.BasicAuthentication] permission_classes = [permissions.DjangoObjectLevelModelPermissions] - object_permissions_view = ObjectPermissionInstanceView.as_view() +class ObjectPermissionListView(generics.ListAPIView): + model = BasicPermModel + authentication_classes = [authentication.BasicAuthentication] + permission_classes = [permissions.DjangoObjectLevelModelPermissions] + +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): - # create users - User.objects.create_user('no_permission', 'no_permission@example.com', 'password') - reader = User.objects.create_user('reader', 'reader@example.com', 'password') - writer = User.objects.create_user('writer', 'writer@example.com', 'password') - full_access = User.objects.create_user('full_access', 'full_access@example.com', 'password') - + 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) - # assign permissions appropriately - from guardian.shortcuts import assign_perm + readers.user_set.add(users['fullaccess'], users['readonly']) + writers.user_set.add(users['fullaccess'], users['writeonly']) + deleters.user_set.add(users['fullaccess'], users['deleteonly']) - read = "read_basicpermmodel" - write = "change_basicpermmodel" - delete = "delete_basicpermmodel" - app_label = 'tests.' - # model level permissions - assign_perm(app_label + delete, full_access, obj=model) - (assign_perm(app_label + write, user, obj=model) for user in (writer, full_access)) - (assign_perm(app_label + read, user, obj=model) for user in (reader, writer, full_access)) + self.credentials = {} + for user in users.values(): + self.credentials[user.username] = basic_auth_header(user.username, 'password') - # object level permissions - assign_perm(delete, full_access, obj=model) - (assign_perm(write, user, obj=model) for user in (writer, full_access)) - (assign_perm(read, user, obj=model) for user in (reader, writer, full_access)) - - self.no_permission_credentials = basic_auth_header('no_permission', 'password') - self.reader_credentials = basic_auth_header('reader', 'password') - self.writer_credentials = basic_auth_header('writer', 'password') - self.full_access_credentials = basic_auth_header('full_access', 'password') - - - def test_has_delete_permissions(self): - request = factory.delete('/1', HTTP_AUTHORIZATION=self.full_access_credentials) + # Delete + def test_can_delete_permissions(self): + request = factory.delete('/1', HTTP_AUTHORIZATION=self.credentials['deleteonly']) + object_permissions_view.cls.action = 'destroy' response = object_permissions_view(request, pk='1') self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - def test_no_delete_permissions(self): - request = factory.delete('/1', HTTP_AUTHORIZATION=self.writer_credentials) + def test_cannot_delete_permissions(self): + request = factory.delete('/1', HTTP_AUTHORIZATION=self.credentials['readonly']) + object_permissions_view.cls.action = 'destroy' 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']) + object_permissions_view.cls.action = 'partial_update' + 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']) + object_permissions_view.cls.action = 'partial_update' + 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']) + object_permissions_view.cls.action = 'retrieve' + 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']) + object_permissions_view.cls.action = 'retrieve' + 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.action = 'list' + 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.action = 'list' + response = object_permissions_list_view(request) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data, []) \ No newline at end of file From 9ff0f6d3bff3c1d02d2ccaf4f1500e25cb97620d Mon Sep 17 00:00:00 2001 From: bwreilly Date: Sat, 7 Sep 2013 23:48:03 -0500 Subject: [PATCH 05/16] switch to a dedicated filter for read list object permissions --- rest_framework/filters.py | 18 +++++++++++++++++- rest_framework/permissions.py | 13 ++++++------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/rest_framework/filters.py b/rest_framework/filters.py index 4079e1bd5..6d46ad233 100644 --- a/rest_framework/filters.py +++ b/rest_framework/filters.py @@ -4,7 +4,7 @@ returned by list views. """ from __future__ import unicode_literals from django.db import models -from rest_framework.compat import django_filters, six +from rest_framework.compat import django_filters, six, guardian from functools import reduce import operator @@ -23,6 +23,22 @@ 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. diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 2d8a30e91..0d5e0e787 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -9,6 +9,7 @@ SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS'] from django.http import Http404 from rest_framework.compat import oauth2_provider_scope, oauth2_constants, guardian +from rest_framework.filters import ObjectPermissionReaderFilter class BasePermission(object): @@ -169,7 +170,7 @@ class DjangoObjectLevelModelPermissions(DjangoModelPermissions): 'destroy': 'delete', } - def _get_names(self, view): + def _get_model_name(self, view): model_cls = getattr(view, 'model', None) queryset = getattr(view, 'queryset', None) @@ -182,19 +183,17 @@ class DjangoObjectLevelModelPermissions(DjangoModelPermissions): def has_permission(self, request, view): if view.action == 'list': - user = request.user queryset = view.get_queryset() - model_name = self._get_names(view) - view.queryset = guardian.shortcuts.get_objects_for_user(user, 'read_' + model_name, queryset) #TODO: move to filter + view.queryset = ObjectPermissionReaderFilter().filter_queryset(request, queryset, view) return super(DjangoObjectLevelModelPermissions, self).has_permission(request, view) def has_object_permission(self, request, view, obj): - user = request.user - model_name = self._get_names(view) action = self.action_perm_map.get(view.action) - assert action, "Tried to determine object permissions but no action specified in view" + user = request.user + model_name = self._get_model_name(view) + perm = "{action}_{model_name}".format(action=action, model_name=model_name) check = user.has_perm(perm, obj) if not check: From 0183c69538de7b6dc4e9b0602fc364e789e0cab6 Mon Sep 17 00:00:00 2001 From: bwreilly Date: Mon, 9 Sep 2013 08:39:09 -0700 Subject: [PATCH 06/16] removed unnecessary guardian req and view.action parsing --- rest_framework/permissions.py | 52 +++++++++++------------- rest_framework/tests/test_permissions.py | 11 ++--- 2 files changed, 26 insertions(+), 37 deletions(-) diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 0d5e0e787..61a33bdd3 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -8,8 +8,7 @@ import warnings SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS'] from django.http import Http404 -from rest_framework.compat import oauth2_provider_scope, oauth2_constants, guardian -from rest_framework.filters import ObjectPermissionReaderFilter +from rest_framework.compat import oauth2_provider_scope, oauth2_constants class BasePermission(object): @@ -158,47 +157,42 @@ class DjangoObjectLevelModelPermissions(DjangoModelPermissions): Basic object level permissions utilizing django-guardian. """ - def __init__(self): - assert guardian, 'Using DjangoObjectLevelModelPermissions, but django-guardian is not installed' - - action_perm_map = { - 'list': 'read', - 'retrieve': 'read', - 'create': 'add', - 'partial_update': 'change', - 'update': 'change', - 'destroy': 'delete', + 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'], } - def _get_model_name(self, view): - model_cls = getattr(view, 'model', None) - queryset = getattr(view, 'queryset', None) - - if model_cls is None and queryset is not None: - model_cls = queryset.model - if not model_cls: # no model, no model based permissions - return None - model_name = model_cls._meta.module_name - return model_name + def get_required_object_permissions(self, method, model_cls): + kwargs = { + 'model_name': model_cls._meta.module_name + } + return [perm % kwargs for perm in self.actions_map[method]] def has_permission(self, request, view): - if view.action == 'list': + if getattr(view, 'action', None) == 'list': queryset = view.get_queryset() view.queryset = ObjectPermissionReaderFilter().filter_queryset(request, queryset, view) return super(DjangoObjectLevelModelPermissions, self).has_permission(request, view) def has_object_permission(self, request, view, obj): - action = self.action_perm_map.get(view.action) - assert action, "Tried to determine object permissions but no action specified in view" + model_cls = getattr(view, 'model', None) + queryset = getattr(view, 'queryset', None) + if model_cls is None and queryset is not None: + model_cls = queryset.model + + perms = self.get_required_object_permissions(request.method, model_cls) user = request.user - model_name = self._get_model_name(view) - perm = "{action}_{model_name}".format(action=action, model_name=model_name) - check = user.has_perm(perm, obj) + check = user.has_perms(perms, obj) if not check: raise Http404 - return user.has_perm(perm, obj) + return user.has_perms(perms, obj) class TokenHasReadWriteScope(BasePermission): diff --git a/rest_framework/tests/test_permissions.py b/rest_framework/tests/test_permissions.py index d64ab04eb..2d866cd09 100644 --- a/rest_framework/tests/test_permissions.py +++ b/rest_framework/tests/test_permissions.py @@ -4,6 +4,7 @@ from django.db import models from django.test import TestCase 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.test import APIRequestFactory from rest_framework.tests.models import BasicModel import base64 @@ -227,13 +228,11 @@ if guardian: # Delete def test_can_delete_permissions(self): request = factory.delete('/1', HTTP_AUTHORIZATION=self.credentials['deleteonly']) - object_permissions_view.cls.action = 'destroy' 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']) - object_permissions_view.cls.action = 'destroy' response = object_permissions_view(request, pk='1') self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) @@ -241,7 +240,6 @@ if guardian: def test_can_update_permissions(self): request = factory.patch('/1', {'text': 'foobar'}, format='json', HTTP_AUTHORIZATION=self.credentials['writeonly']) - object_permissions_view.cls.action = 'partial_update' response = object_permissions_view(request, pk='1') self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data.get('text'), 'foobar') @@ -249,34 +247,31 @@ if guardian: def test_cannot_update_permissions(self): request = factory.patch('/1', {'text': 'foobar'}, format='json', HTTP_AUTHORIZATION=self.credentials['deleteonly']) - object_permissions_view.cls.action = 'partial_update' 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']) - object_permissions_view.cls.action = 'retrieve' 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']) - object_permissions_view.cls.action = 'retrieve' 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.action = 'list' + 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.action = 'list' + 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, []) \ No newline at end of file From 23fc9dd53fcd9cc25e2c77e5ffae395f04d4990d Mon Sep 17 00:00:00 2001 From: bwreilly Date: Mon, 9 Sep 2013 09:32:29 -0700 Subject: [PATCH 07/16] better doc for object permissions, drop redundant has_permission call --- rest_framework/permissions.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 61a33bdd3..70bf9c61e 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -154,7 +154,14 @@ class DjangoModelPermissionsOrAnonReadOnly(DjangoModelPermissions): class DjangoObjectLevelModelPermissions(DjangoModelPermissions): """ - Basic object level permissions utilizing django-guardian. + The request is authenticated using `django.contrib.auth` permissions. + See: https://docs.djangoproject.com/en/dev/topics/auth/#permissions + + It ensures that the user is authenticated, and has the appropriate + `add`/`change`/`delete` permissions on the object using .has_perms. + + This permission can only be applied against view classes that + provide a `.model` or `.queryset` attribute. """ actions_map = { @@ -173,12 +180,6 @@ class DjangoObjectLevelModelPermissions(DjangoModelPermissions): } return [perm % kwargs for perm in self.actions_map[method]] - def has_permission(self, request, view): - if getattr(view, 'action', None) == 'list': - queryset = view.get_queryset() - view.queryset = ObjectPermissionReaderFilter().filter_queryset(request, queryset, view) - return super(DjangoObjectLevelModelPermissions, self).has_permission(request, view) - def has_object_permission(self, request, view, obj): model_cls = getattr(view, 'model', None) queryset = getattr(view, 'queryset', None) From 222c1d1122b13940fa6072a1dfb89b25491ee6fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ociepka?= Date: Tue, 10 Sep 2013 12:02:14 +0200 Subject: [PATCH 08/16] Add order_by to the AutoFilterSet `AutoFilterSet` should contains `order_by` set to all by default. --- rest_framework/filters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rest_framework/filters.py b/rest_framework/filters.py index 4079e1bd5..1e58d173d 100644 --- a/rest_framework/filters.py +++ b/rest_framework/filters.py @@ -53,6 +53,7 @@ class DjangoFilterBackend(BaseFilterBackend): class Meta: model = queryset.model fields = filter_fields + order_by = True return AutoFilterSet return None From 5970baa20112921217ae4f2c2a9f175df25922db Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 10 Sep 2013 21:00:13 +0100 Subject: [PATCH 09/16] Tweaks and docs to object-level model permissions. --- docs/api-guide/filtering.md | 46 +++++ docs/api-guide/permissions.md | 18 +- rest_framework/filters.py | 37 ++-- rest_framework/permissions.py | 47 +++-- rest_framework/tests/test_permissions.py | 217 +++++++++++++---------- 5 files changed, 235 insertions(+), 130 deletions(-) diff --git a/docs/api-guide/filtering.md b/docs/api-guide/filtering.md index 649462da7..859e8d52a 100644 --- a/docs/api-guide/filtering.md +++ b/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 diff --git a/docs/api-guide/permissions.md b/docs/api-guide/permissions.md index a7bf15556..871de84ec 100644 --- a/docs/api-guide/permissions.md +++ b/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 diff --git a/rest_framework/filters.py b/rest_framework/filters.py index 6d46ad233..1693bcd24 100644 --- a/rest_framework/filters.py +++ b/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) diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 70bf9c61e..531847988 100644 --- a/rest_framework/permissions.py +++ b/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): diff --git a/rest_framework/tests/test_permissions.py b/rest_framework/tests/test_permissions.py index 2d866cd09..d08124f48 100644 --- a/rest_framework/tests/test_permissions.py +++ b/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'), - } +@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 - # 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()) + # 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'), + } - cls.perms = perms - cls.users = users + # 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()) - def setUp(self): - perms = self.perms - users = self.users + cls.perms = perms + cls.users = users - # appropriate object level permissions - readers = Group.objects.create(name='readers') - writers = Group.objects.create(name='writers') - deleters = Group.objects.create(name='deleters') + def setUp(self): + from guardian.shortcuts import assign_perm + perms = self.perms + users = self.users - model = BasicPermModel.objects.create(text='foo') - - assign_perm(perms['read'], readers, model) - assign_perm(perms['change'], writers, model) - assign_perm(perms['delete'], deleters, model) + # appropriate object level permissions + readers = Group.objects.create(name='readers') + writers = Group.objects.create(name='writers') + deleters = Group.objects.create(name='deleters') - readers.user_set.add(users['fullaccess'], users['readonly']) - writers.user_set.add(users['fullaccess'], users['writeonly']) - deleters.user_set.add(users['fullaccess'], users['deleteonly']) + model = BasicPermModel.objects.create(text='foo') + + assign_perm(perms['view'], readers, model) + assign_perm(perms['change'], writers, model) + assign_perm(perms['delete'], deleters, model) - self.credentials = {} - for user in users.values(): - self.credentials[user.username] = basic_auth_header(user.username, 'password') + readers.user_set.add(users['fullaccess'], users['readonly']) + writers.user_set.add(users['fullaccess'], users['writeonly']) + deleters.user_set.add(users['fullaccess'], users['deleteonly']) - # 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) + self.credentials = {} + for user in users.values(): + self.credentials[user.username] = basic_auth_header(user.username, 'password') - 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) + # 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) - # 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_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) - 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) + # 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') - # 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_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_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) + 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 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) + # 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_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, []) \ No newline at end of file + 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) + 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, []) From 101da4581083d75636b24c50638e7f288d1fe240 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 10 Sep 2013 21:06:42 +0100 Subject: [PATCH 10/16] Updated release notes --- docs/topics/release-notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index 1f363310d..ff3dae097 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -42,6 +42,7 @@ You can determine your currently installed version using `pip freeze`: ### Master +* Added `DjangoObjectPermissions`, and `DjangoObjectPermissionsFilter`. * Support customizable exception handling, using the `EXCEPTION_HANDLER` setting. * Support customizable view name and description functions, using the `VIEW_NAME_FUNCTION` and `VIEW_DESCRIPTION_FUNCTION` settings. * Added `MAX_PAGINATE_BY` setting and `max_paginate_by` generic view attribute. From a1d7ed20d256730492659f6ba6193faf1f12a581 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 10 Sep 2013 21:06:53 +0100 Subject: [PATCH 11/16] Add Django Guardian to travis testing --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 6a4532411..d12479e9e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,6 +18,7 @@ install: - "if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install oauth2==1.5.211 --use-mirrors; fi" - "if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install django-oauth-plus==2.0 --use-mirrors; fi" - "if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install django-oauth2-provider==0.2.4 --use-mirrors; fi" + - "if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install django-guardian==1.1.1 --use-mirrors; fi" - "if [[ ${DJANGO::11} == 'django==1.3' ]]; then pip install django-filter==0.5.4 --use-mirrors; fi" - "if [[ ${DJANGO::11} != 'django==1.3' ]]; then pip install django-filter==0.6 --use-mirrors; fi" - export PYTHONPATH=. From e021472a1667c4902000bb40e0c19f64160b1584 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 10 Sep 2013 21:07:20 +0100 Subject: [PATCH 12/16] Added @bwreilly for awesome work on #1093. Thanks!!! --- docs/topics/credits.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/topics/credits.md b/docs/topics/credits.md index 07e2ec47d..8269580ef 100644 --- a/docs/topics/credits.md +++ b/docs/topics/credits.md @@ -167,6 +167,7 @@ The following people have helped make REST framework great. * Andrey Antukh - [niwibe] * Mathieu Pillard - [diox] * Edmond Wong - [edmondwong] +* Ben Reilly - [bwreilly] Many thanks to everyone who's contributed to the project. @@ -370,3 +371,4 @@ You can also contact [@_tomchristie][twitter] directly on twitter. [niwibe]: https://github.com/niwibe [diox]: https://github.com/diox [edmondwong]: https://github.com/edmondwong +[bwreilly]: https://github.com/bwreilly From 195790e60b117eff421eb8f04a9f9f3527e797b8 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 11 Sep 2013 09:09:30 +0100 Subject: [PATCH 13/16] Version 2.3.8 --- docs/topics/release-notes.md | 4 +++- rest_framework/__init__.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index ff3dae097..3b35d9ed6 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -40,7 +40,9 @@ You can determine your currently installed version using `pip freeze`: ## 2.3.x series -### Master +### 2.3.8 + +**Date**: 11th September 2013 * Added `DjangoObjectPermissions`, and `DjangoObjectPermissionsFilter`. * Support customizable exception handling, using the `EXCEPTION_HANDLER` setting. diff --git a/rest_framework/__init__.py b/rest_framework/__init__.py index 087808e0b..2bd2991ba 100644 --- a/rest_framework/__init__.py +++ b/rest_framework/__init__.py @@ -1,4 +1,4 @@ -__version__ = '2.3.7' +__version__ = '2.3.8' VERSION = __version__ # synonym From 2a6a2013df4fcb8e09425e9fa758b91b3a23b751 Mon Sep 17 00:00:00 2001 From: Diego Ponciano Date: Wed, 11 Sep 2013 17:25:57 -0300 Subject: [PATCH 14/16] small typo correction on ViewSet example code --- docs/api-guide/viewsets.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-guide/viewsets.md b/docs/api-guide/viewsets.md index 2e65b7a43..1062cb32c 100644 --- a/docs/api-guide/viewsets.md +++ b/docs/api-guide/viewsets.md @@ -23,7 +23,7 @@ Let's define a simple viewset that can be used to list or retrieve all the users from django.shortcuts import get_object_or_404 from myapps.serializers import UserSerializer from rest_framework import viewsets - from rest_framewor.responses import Response + from rest_framework.response import Response class UserViewSet(viewsets.ViewSet): """ From dfc430cabaf76a1b3382a614cc692e4a52b09bcd Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 12 Sep 2013 20:27:23 +0100 Subject: [PATCH 15/16] Fix django guardian link --- docs/index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/index.md b/docs/index.md index d83fbff1f..bb2129f6f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -251,6 +251,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. [oauth2]: https://github.com/simplegeo/python-oauth2 [django-oauth-plus]: https://bitbucket.org/david/django-oauth-plus/wiki/Home [django-oauth2-provider]: https://github.com/caffeinehit/django-oauth2-provider +[django-guardian]: https://github.com/lukaszb/django-guardian [0.4]: https://github.com/tomchristie/django-rest-framework/tree/0.4.X [image]: img/quickstart.png [index]: . From 895beb89c60cea534f85b8a7749615755c4d43b5 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 12 Sep 2013 21:41:21 +0100 Subject: [PATCH 16/16] Note on '.model' as default only, with 'serializer_class', and 'queryset' attributes prefered. Closes #1100 --- docs/api-guide/generic-views.md | 2 +- docs/topics/writable-nested-serializers.md | 47 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 docs/topics/writable-nested-serializers.md diff --git a/docs/api-guide/generic-views.md b/docs/api-guide/generic-views.md index 7185b6b68..dc0076dff 100755 --- a/docs/api-guide/generic-views.md +++ b/docs/api-guide/generic-views.md @@ -69,7 +69,7 @@ The following attributes control the basic view behavior. **Shortcuts**: -* `model` - This shortcut may be used instead of setting either (or both) of the `queryset`/`serializer_class` attributes, although using the explicit style is generally preferred. If used instead of `serializer_class`, then then `DEFAULT_MODEL_SERIALIZER_CLASS` setting will determine the base serializer class. +* `model` - This shortcut may be used instead of setting either (or both) of the `queryset`/`serializer_class` attributes, although using the explicit style is generally preferred. If used instead of `serializer_class`, then then `DEFAULT_MODEL_SERIALIZER_CLASS` setting will determine the base serializer class. Note that `model` is only ever used for generating a default queryset or serializer class - the `queryset` and `serializer_class` attributes are always preferred if provided. **Pagination**: diff --git a/docs/topics/writable-nested-serializers.md b/docs/topics/writable-nested-serializers.md new file mode 100644 index 000000000..66ea78150 --- /dev/null +++ b/docs/topics/writable-nested-serializers.md @@ -0,0 +1,47 @@ +> To save HTTP requests, it may be convenient to send related documents along with the request. +> +> — [JSON API specification for Ember Data][cite]. + +# Writable nested serializers + +Although flat data structures serve to properly delineate between the individual entities in your service, there are cases where it may be more appropriate or convenient to use nested data structures. + +Nested data structures are easy enough to work with if they're read-only - simply nest your serializer classes and you're good to go. However, there are a few more subtleties to using writable nested serializers, due to the dependancies between the various model instances, and the need to save or delete multiple instances in a single action. + +## One-to-many data structures + +*Example of a **read-only** nested serializer. Nothing complex to worry about here.* + + class ToDoItemSerializer(serializers.ModelSerializer): + class Meta: + model = ToDoItem + fields = ('text', 'is_completed') + + class ToDoListSerializer(serializers.ModelSerializer): + items = ToDoItemSerializer(many=True, read_only=True) + + class Meta: + model = ToDoList + fields = ('title', 'items') + +Some example output from our serializer. + + { + 'title': 'Leaving party preperations', + 'items': { + {'text': 'Compile playlist', 'is_completed': True}, + {'text': 'Send invites', 'is_completed': False}, + {'text': 'Clean house', 'is_completed': False} + } + } + +Let's take a look at updating our nested one-to-many data structure. + +### Validation errors + +### Adding and removing items + +### Making PATCH requests + + +[cite]: http://jsonapi.org/format/#url-based-json-api \ No newline at end of file