From 5581509c4c3bbf3067880c57fa2d09a123e9887b Mon Sep 17 00:00:00 2001 From: Akis Kesoglou Date: Sun, 22 Jul 2018 16:25:30 +0300 Subject: [PATCH] Ensure compatibility with Django v2.1 --- README.rst | 14 +++++++++++--- rules/contrib/admin.py | 10 ++++++++++ rules/permissions.py | 2 +- tests/testsuite/contrib/test_views.py | 8 +++++--- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/README.rst b/README.rst index 946f8ba..c814d08 100644 --- a/README.rst +++ b/README.rst @@ -507,11 +507,14 @@ set to also use ``rules`` to authorize any add/change/delete actions in the Admin. The Admin asks for *four* different permissions, depending on action: - ``.add_`` +- ``.view_`` - ``.change_`` - ``.delete_`` - ```` -The first three are obvious. The fourth is the required permission for an app +*Note:* view permission is new in Django v2.1. + +The first four are obvious. The fifth is the required permission for an app to be displayed in the Admin's "dashboard". Here's some rules for our imaginary ``books`` app as an example: @@ -519,6 +522,7 @@ imaginary ``books`` app as an example: >>> rules.add_perm('books', rules.always_allow) >>> rules.add_perm('books.add_book', is_staff) + >>> rules.add_perm('books.view_book', is_staff | has_secret_access_code) >>> rules.add_perm('books.change_book', is_staff) >>> rules.add_perm('books.delete_book', is_staff) @@ -530,11 +534,10 @@ If you'd like to tell Django whether a user has permissions on a specific object, you'd have to override the following methods of a model's ``ModelAdmin``: +- ``has_view_permission(user, obj=None)`` - ``has_change_permission(user, obj=None)`` - ``has_delete_permission(user, obj=None)`` -**Note:** There's also ``has_add_permission(user)`` but is not relevant here. - ``rules`` comes with a custom ``ModelAdmin`` subclass, ``rules.contrib.admin.ObjectPermissionsModelAdmin``, that overrides these methods to pass on the edited model instance to the authorization backends, @@ -561,6 +564,11 @@ Now this allows you to specify permissions like this: >>> rules.add_perm('books.change_book', is_book_author_or_editor) >>> rules.add_perm('books.delete_book', is_book_author) +To preserve backwards compatibility, Django will ask for either *view* or +*change* permission. For maximum flexibility, ``rules`` behaves subtly +different: ``rules`` will ask for the change permission if and only if no rule +exists for the view permission. + Advanced features ================= diff --git a/rules/contrib/admin.py b/rules/contrib/admin.py index 0ba4745..a3c27bb 100644 --- a/rules/contrib/admin.py +++ b/rules/contrib/admin.py @@ -1,8 +1,18 @@ from django.contrib import admin from django.contrib.auth import get_permission_codename +from ..permissions import perm_exists class ObjectPermissionsModelAdminMixin(object): + def has_view_permission(self, request, obj=None): + opts = self.opts + codename = get_permission_codename('view', opts) + perm = '%s.%s' % (opts.app_label, codename) + if perm_exists(perm): + return request.user.has_perm(perm, obj) + else: + return self.has_change_permission(request, obj) + def has_change_permission(self, request, obj=None): opts = self.opts codename = get_permission_codename('change', opts) diff --git a/rules/permissions.py b/rules/permissions.py index d2f1cdf..b9705cd 100644 --- a/rules/permissions.py +++ b/rules/permissions.py @@ -25,7 +25,7 @@ def has_perm(name, *args, **kwargs): class ObjectPermissionBackend(object): - def authenticate(self, username, password): + def authenticate(self, *args, **kwargs): return None def has_perm(self, user, perm, *args, **kwargs): diff --git a/tests/testsuite/contrib/test_views.py b/tests/testsuite/contrib/test_views.py index 15a6af3..59e6901 100644 --- a/tests/testsuite/contrib/test_views.py +++ b/tests/testsuite/contrib/test_views.py @@ -114,10 +114,11 @@ def test_permission_required_mixin(self): self.assertEqual(response.status_code, 200) self.assertEqual(force_str(response.content), 'OK') - # Martin can *not* delete Adrian's book and is redirected to login + # Martin can *not* delete Adrian's book + # Up to Django v2.1, the response was a redirect to login self.assertTrue(self.client.login(username='martin', password='secr3t')) response = self.client.get(reverse('cbv.delete_book', args=(1,))) - self.assertEqual(response.status_code, 302) + self.assertIn(response.status_code, [302, 403]) # Martin can *not* delete Adrian's book and an PermissionDenied is raised self.assertTrue(self.client.login(username='martin', password='secr3t')) @@ -133,6 +134,7 @@ def test_permission_required_mixin(self): self.assertEqual(force_str(response.content), 'OK') # Martin does not have delete permission + # Up to Django v2.1, the response was a redirect to login self.assertTrue(self.client.login(username='martin', password='secr3t')) response = self.client.get(reverse('cbv.view_with_permission_list', args=(1,))) - self.assertEqual(response.status_code, 302) + self.assertIn(response.status_code, [302, 403])