From 4a9dcfa76089143bbeb5cd43fa3a406365d89e96 Mon Sep 17 00:00:00 2001 From: bwreilly Date: Fri, 6 Sep 2013 11:01:31 -0500 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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)