From 0c7615b7332806205f0da894aecae33fcb712883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gregor=20M=C3=BCllegger?= Date: Thu, 23 May 2013 01:21:01 +0200 Subject: [PATCH] Implementing a class-based permission handling. --- djadmin2/permissions.py | 121 ++++++++++++++++++ djadmin2/viewmixins.py | 36 ++++-- djadmin2/views.py | 21 ++- example/blog/tests/test_apiviews.py | 51 +++++++- .../blog/tests/test_builtin_api_resources.py | 32 ++++- 5 files changed, 233 insertions(+), 28 deletions(-) create mode 100644 djadmin2/permissions.py diff --git a/djadmin2/permissions.py b/djadmin2/permissions.py new file mode 100644 index 0000000..9e31279 --- /dev/null +++ b/djadmin2/permissions.py @@ -0,0 +1,121 @@ +''' +djadmin2's permission handling. The permission classes have the same API as +the permission handling classes of the django-rest-framework. That way, we can +reuse them in the admin's REST API. + +The permission checks take place in callables that follow the following +interface: + +* They get passed in the current ``request``, an instance of the currently + active ``view`` and optionally the object that should be used for + object-level permission checking. +* Return ``True`` if the permission shall be granted, ``False`` otherwise. + +The permission classes are then just fancy wrappers of these basic checks of +which it can hold multiple. +''' + + +def is_authenticated(request, view, obj=None): + return request.user.is_authenticated() + + +def is_staff(request, view, obj=None): + return request.user.is_staff + + +def is_superuser(request, view, obj=None): + return request.user.is_superuser + + +def model_permission(permission): + def has_permission(request, view, obj=None): + model_class = getattr(view, 'model', None) + queryset = getattr(view, 'queryset', None) + + if model_class is None and queryset is not None: + model_class = queryset.model + + assert model_class, ( + 'Cannot apply DjangoModelPermissions on a view that does not ' + 'have `.model` or `.queryset` property.') + + kwargs = { + 'app_label': model_class._meta.app_label, + 'model_name': model_class._meta.module_name + } + permission_name = permission % kwargs + return request.user.has_perm(permission_name, obj) + return has_permission + + +class AdminPermission(object): + ''' + Provides a base class with a common API. It implements a compatible + interface to django-rest-framework permission backends. + ''' + permissions = [] + permissions_for_method = {} + + def get_permission_checks(self, request, view): + permission_checks = [] + permission_checks.extend(self.permissions) + method_permissions = self.permissions_for_method.get(request.method, ()) + permission_checks.extend(method_permissions) + return permission_checks + + # needs to be compatible to django-rest-framework + def has_permission(self, request, view, obj=None): + if request.user: + for permission_check in self.get_permission_checks(request, view): + if not permission_check(request, view, obj): + return False + return True + return False + + # needs to be compatible to django-rest-framework + def has_object_permission(self, request, view, obj): + return self.has_permission(request, view, obj) + + +class IsStaffPermission(AdminPermission): + ''' + It ensures that the user is authenticated and is a staff member. + ''' + permissions = ( + is_authenticated, + is_staff) + + +class ModelPermission(AdminPermission): + ''' + Checks if the necessary model permissions are set for the accessed object. + ''' + # Map methods into required permission codes. + # Override this if you need to also provide 'view' permissions, + # or if you want to provide custom permission checks. + permissions_for_method = { + 'GET': (), + 'OPTIONS': (), + 'HEAD': (), + 'POST': (model_permission('%(app_label)s.add_%(model_name)s'),), + 'PUT': (model_permission('%(app_label)s.change_%(model_name)s'),), + 'PATCH': (model_permission('%(app_label)s.change_%(model_name)s'),), + 'DELETE': (model_permission('%(app_label)s.delete_%(model_name)s'),), + } + + +class ModelViewPermission(AdminPermission): + permissions = (model_permission('%(app_label)s.view_%(model_name)s'),) + + +class ModelAddPermission(AdminPermission): + permissions = (model_permission('%(app_label)s.add_%(model_name)s'),) + + +class ModelChangePermission(AdminPermission): + permissions = (model_permission('%(app_label)s.change_%(model_name)s'),) + + +class ModelDeletePermission(AdminPermission): + permissions = (model_permission('%(app_label)s.delete_%(model_name)s'),) diff --git a/djadmin2/viewmixins.py b/djadmin2/viewmixins.py index 1fb9201..d200a98 100644 --- a/djadmin2/viewmixins.py +++ b/djadmin2/viewmixins.py @@ -7,14 +7,23 @@ from django.forms.models import modelform_factory from braces.views import AccessMixin -from . import constants +from . import constants, permissions from .utils import admin2_urlname, model_options class Admin2Mixin(object): + # are set in the ModelAdmin2 class when creating the view via + # .as_view(...) model_admin = None model_name = None app_label = None + permission_classes = (permissions.IsStaffPermission,) + + def __init__(self, **kwargs): + self.permissions = [ + permission_class() + for permission_class in self.permission_classes] + super(Admin2Mixin, self).__init__(**kwargs) def get_template_names(self): return [os.path.join(constants.ADMIN2_THEME_DIRECTORY, self.default_template_name)] @@ -30,24 +39,29 @@ class Admin2Mixin(object): return self.form_class return modelform_factory(self.get_model()) + def has_permission(self, obj=None): + ''' + Return ``True`` if the permission shall be granted, ``False`` + otherwise. + ''' + for backend in self.permissions: + if not backend.has_permission(self.request, self, obj): + return False + return True + class AdminModel2Mixin(Admin2Mixin, AccessMixin): model_admin = None - # Permission type to check for when a request is sent to this view. - permission_type = None def dispatch(self, request, *args, **kwargs): - # Check if user has necessary permissions. If the permission_type isn't specified then check for staff status. - has_permission = self.model_admin.has_permission(request, self.permission_type) \ - if self.permission_type else request.user.is_staff - # Raise exception or redirect to login if user doesn't have permissions. - if not has_permission: + # Raise exception or redirect to login if user doesn't have + # permissions. + if not self.has_permission(): if self.raise_exception: raise PermissionDenied # return a forbidden response else: return redirect_to_login(request.get_full_path(), self.get_login_url(), self.get_redirect_field_name()) - return super(AdminModel2Mixin, self).dispatch(request, *args, **kwargs) def get_context_data(self, **kwargs): @@ -55,9 +69,6 @@ class AdminModel2Mixin(Admin2Mixin, AccessMixin): model = self.get_model() model_meta = model_options(model) context.update({ - 'has_add_permission': self.model_admin.has_add_permission(self.request), - 'has_edit_permission': self.model_admin.has_edit_permission(self.request), - 'has_delete_permission': self.model_admin.has_delete_permission(self.request), 'app_label': model_meta.app_label, 'model_name': model_meta.verbose_name, 'model_name_pluralized': model_meta.verbose_name_plural @@ -77,7 +88,6 @@ class AdminModel2Mixin(Admin2Mixin, AccessMixin): class Admin2ModelFormMixin(object): - def get_success_url(self): if '_continue' in self.request.POST: view_name = admin2_urlname(self, 'update') diff --git a/djadmin2/views.py b/djadmin2/views.py index fae0c18..04fb5c8 100644 --- a/djadmin2/views.py +++ b/djadmin2/views.py @@ -4,6 +4,7 @@ from django.views import generic import extra_views +from . import permissions from .viewmixins import Admin2Mixin, AdminModel2Mixin, Admin2ModelFormMixin @@ -39,7 +40,9 @@ class AppIndexView(Admin2Mixin, generic.TemplateView): class ModelListView(AdminModel2Mixin, generic.ListView): default_template_name = "model_list.html" - permission_type = 'view' + permission_classes = ( + permissions.IsStaffPermission, + permissions.ModelViewPermission) def post(self, request): # This is where we handle actions @@ -69,13 +72,17 @@ class ModelListView(AdminModel2Mixin, generic.ListView): class ModelDetailView(AdminModel2Mixin, generic.DetailView): default_template_name = "model_detail.html" - permission_type = 'view' + permission_classes = ( + permissions.IsStaffPermission, + permissions.ModelViewPermission) class ModelEditFormView(AdminModel2Mixin, Admin2ModelFormMixin, extra_views.UpdateWithInlinesView): form_class = None default_template_name = "model_update_form.html" - permission_type = 'change' + permission_classes = ( + permissions.IsStaffPermission, + permissions.ModelChangePermission) def get_context_data(self, **kwargs): context = super(ModelEditFormView, self).get_context_data(**kwargs) @@ -87,7 +94,9 @@ class ModelEditFormView(AdminModel2Mixin, Admin2ModelFormMixin, extra_views.Upda class ModelAddFormView(AdminModel2Mixin, Admin2ModelFormMixin, extra_views.CreateWithInlinesView): form_class = None default_template_name = "model_update_form.html" - permission_type = 'add' + permission_classes = ( + permissions.IsStaffPermission, + permissions.ModelAddPermission) def get_context_data(self, **kwargs): context = super(ModelAddFormView, self).get_context_data(**kwargs) @@ -99,4 +108,6 @@ class ModelAddFormView(AdminModel2Mixin, Admin2ModelFormMixin, extra_views.Creat class ModelDeleteView(AdminModel2Mixin, generic.DeleteView): success_url = "../../" # TODO - fix this! default_template_name = "model_confirm_delete.html" - permission_type = 'delete' + permission_classes = ( + permissions.IsStaffPermission, + permissions.ModelDeletePermission) diff --git a/example/blog/tests/test_apiviews.py b/example/blog/tests/test_apiviews.py index b200194..737688f 100644 --- a/example/blog/tests/test_apiviews.py +++ b/example/blog/tests/test_apiviews.py @@ -1,6 +1,7 @@ +from django.contrib.auth.models import User +from django.core.urlresolvers import reverse from django.test import TestCase from django.test.client import RequestFactory -from django.core.urlresolvers import reverse from django.utils import simplejson as json @@ -10,49 +11,74 @@ from djadmin2.models import ModelAdmin2 from ..models import Post -class ViewTest(TestCase): +class APITestCase(TestCase): def setUp(self): self.factory = RequestFactory() + self.user = User( + username='admin', + is_staff=True) + self.user.set_password('admin') + self.user.save() def get_model_admin(self, model): return ModelAdmin2(model, default) -class IndexAPIViewTest(ViewTest): +class IndexAPIViewTest(APITestCase): def test_response_ok(self): request = self.factory.get(reverse('admin2:api-index')) + request.user = self.user view = apiviews.IndexAPIView.as_view(**default.get_api_index_kwargs()) response = view(request) self.assertEqual(response.status_code, 200) + def test_view_permission(self): + request = self.factory.get(reverse('admin2:api-index')) + view = apiviews.IndexAPIView.as_view(**default.get_api_index_kwargs()) + response = view(request) + self.assertEqual(response.status_code, 403) -class ListCreateAPIViewTest(ViewTest): +class ListCreateAPIViewTest(APITestCase): def test_response_ok(self): request = self.factory.get(reverse('admin2:blog_post_api-list')) + request.user = self.user model_admin = self.get_model_admin(Post) view = apiviews.ListCreateAPIView.as_view( **model_admin.get_api_list_kwargs()) response = view(request) self.assertEqual(response.status_code, 200) + def test_view_permission(self): + request = self.factory.get(reverse('admin2:blog_post_api-list')) + model_admin = self.get_model_admin(Post) + view = apiviews.ListCreateAPIView.as_view( + **model_admin.get_api_list_kwargs()) + response = view(request) + self.assertEqual(response.status_code, 403) + def test_list_includes_unicode_field(self): Post.objects.create(title='Foo', body='Bar') request = self.factory.get(reverse('admin2:blog_post_api-list')) + request.user = self.user model_admin = self.get_model_admin(Post) view = apiviews.ListCreateAPIView.as_view( **model_admin.get_api_list_kwargs()) response = view(request) response.render() + self.assertEqual(response.status_code, 200) self.assertIn('"__str__": "Foo"', response.content) def test_pagination(self): request = self.factory.get(reverse('admin2:blog_post_api-list')) + request.user = self.user model_admin = self.get_model_admin(Post) view = apiviews.ListCreateAPIView.as_view( **model_admin.get_api_list_kwargs()) response = view(request) response.render() + + self.assertEqual(response.status_code, 200) data = json.loads(response.content) self.assertEqual(data['count'], 0) # next and previous fields exist, but are null because we have no @@ -63,9 +89,20 @@ class ListCreateAPIViewTest(ViewTest): self.assertEqual(data['previous'], None) -class RetrieveUpdateDestroyAPIViewTest(ViewTest): - +class RetrieveUpdateDestroyAPIViewTest(APITestCase): def test_response_ok(self): + post = Post.objects.create(title='Foo', body='Bar') + request = self.factory.get( + reverse('admin2:blog_post_api-detail', + kwargs={'pk': post.pk})) + request.user = self.user + model_admin = self.get_model_admin(Post) + view = apiviews.RetrieveUpdateDestroyAPIView.as_view( + **model_admin.get_api_detail_kwargs()) + response = view(request, pk=post.pk) + self.assertEqual(response.status_code, 200) + + def test_view_permission(self): post = Post.objects.create(title='Foo', body='Bar') request = self.factory.get( reverse('admin2:blog_post_api-detail', @@ -74,4 +111,4 @@ class RetrieveUpdateDestroyAPIViewTest(ViewTest): view = apiviews.RetrieveUpdateDestroyAPIView.as_view( **model_admin.get_api_detail_kwargs()) response = view(request, pk=post.pk) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 403) diff --git a/example/blog/tests/test_builtin_api_resources.py b/example/blog/tests/test_builtin_api_resources.py index a3f9a73..5108dc4 100644 --- a/example/blog/tests/test_builtin_api_resources.py +++ b/example/blog/tests/test_builtin_api_resources.py @@ -1,14 +1,20 @@ from django.contrib.auth.models import Group, User from django.core.urlresolvers import reverse -from django.test import TestCase +from .test_apiviews import APITestCase -class UserAPITest(TestCase): +class UserAPITest(APITestCase): def test_list_response_ok(self): + self.client.login(username='admin', password='admin') response = self.client.get(reverse('admin2:auth_user_api-list')) self.assertEqual(response.status_code, 200) + def test_list_view_permission(self): + response = self.client.get(reverse('admin2:auth_user_api-list')) + self.assertEqual(response.status_code, 403) + def test_detail_response_ok(self): + self.client.login(username='admin', password='admin') user = User.objects.create_user( username='Foo', password='bar') @@ -16,14 +22,34 @@ class UserAPITest(TestCase): reverse('admin2:auth_user_api-detail', args=(user.pk,))) self.assertEqual(response.status_code, 200) + def test_detail_view_permission(self): + user = User.objects.create_user( + username='Foo', + password='bar') + response = self.client.get( + reverse('admin2:auth_user_api-detail', args=(user.pk,))) + self.assertEqual(response.status_code, 403) -class GroupAPITest(TestCase): + +class GroupAPITest(APITestCase): def test_list_response_ok(self): + self.client.login(username='admin', password='admin') response = self.client.get(reverse('admin2:auth_group_api-list')) self.assertEqual(response.status_code, 200) + def test_list_view_permission(self): + response = self.client.get(reverse('admin2:auth_group_api-list')) + self.assertEqual(response.status_code, 403) + def test_detail_response_ok(self): + self.client.login(username='admin', password='admin') group = Group.objects.create(name='group') response = self.client.get( reverse('admin2:auth_group_api-detail', args=(group.pk,))) self.assertEqual(response.status_code, 200) + + def test_detail_view_permission(self): + group = Group.objects.create(name='group') + response = self.client.get( + reverse('admin2:auth_group_api-detail', args=(group.pk,))) + self.assertEqual(response.status_code, 403)