From 504c89e8008c557a1e83c45535b549f77a3503b2 Mon Sep 17 00:00:00 2001 From: Maxime Turcotte Date: Wed, 28 May 2014 12:07:27 -0400 Subject: [PATCH] Fixed #6327 -- Added has_module_permission method to BaseModelAdmin Thanks chrj for the suggestion. --- django/contrib/admin/options.py | 13 +++++ django/contrib/admin/sites.py | 11 ++-- docs/ref/contrib/admin/index.txt | 14 +++++ docs/releases/1.8.txt | 4 +- tests/admin_ordering/tests.py | 3 ++ tests/admin_views/admin.py | 8 +++ tests/admin_views/tests.py | 64 +++++++++++++++++++++++ tests/admin_views/urls.py | 1 + tests/modeladmin/tests.py | 90 ++++++++++++++++++++++++++++++++ 9 files changed, 201 insertions(+), 7 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 28c7ac4761..e01cdd051c 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -473,6 +473,19 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): codename = get_permission_codename('delete', opts) return request.user.has_perm("%s.%s" % (opts.app_label, codename)) + def has_module_permission(self, request): + """ + Returns True if the given request has any permission in the given + app label. + + Can be overridden by the user in subclasses. In such case it should + return True if the given request has permission to view the module on + the admin index page and access the module's index page. Overriding it + does not restrict access to the add, change or delete views. Use + `ModelAdmin.has_(add|change|delete)_permission` for that. + """ + return request.user.has_module_perms(self.opts.app_label) + @python_2_unicode_compatible class ModelAdmin(BaseModelAdmin): diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 8480c08891..84778849e9 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -367,10 +367,9 @@ class AdminSite(object): apps that have been registered in this site. """ app_dict = {} - user = request.user for model, model_admin in self._registry.items(): app_label = model._meta.app_label - has_module_perms = user.has_module_perms(app_label) + has_module_perms = model_admin.has_module_permission(request) if has_module_perms: perms = model_admin.get_model_perms(request) @@ -424,14 +423,14 @@ class AdminSite(object): current_app=self.name) def app_index(self, request, app_label, extra_context=None): - user = request.user app_name = apps.get_app_config(app_label).verbose_name - has_module_perms = user.has_module_perms(app_label) - if not has_module_perms: - raise PermissionDenied app_dict = {} for model, model_admin in self._registry.items(): if app_label == model._meta.app_label: + has_module_perms = model_admin.has_module_permission(request) + if not has_module_perms: + raise PermissionDenied + perms = model_admin.get_model_perms(request) # Check whether user has any perm for this module. diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 876bf290c3..295ff12aa4 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -1631,6 +1631,19 @@ templates used by the :class:`ModelAdmin` views: be interpreted as meaning that the current user is not permitted to delete any object of this type). +.. method:: ModelAdmin.has_module_permission(request) + + .. versionadded:: 1.8 + + Should return ``True`` if displaying the module on the admin index page and + accessing the module's index page is permitted, ``False`` otherwise. + Uses :meth:`User.has_module_perms() + ` by default. Overriding + it does not restrict access to the add, change or delete views, + :meth:`~ModelAdmin.has_add_permission`, + :meth:`~ModelAdmin.has_change_permission`, and + :meth:`~ModelAdmin.has_delete_permission` should be used for that. + .. method:: ModelAdmin.get_queryset(request) The ``get_queryset`` method on a ``ModelAdmin`` returns a @@ -1909,6 +1922,7 @@ adds some of its own (the shared features are actually defined in the - :meth:`~ModelAdmin.has_add_permission` - :meth:`~ModelAdmin.has_change_permission` - :meth:`~ModelAdmin.has_delete_permission` +- :meth:`~ModelAdmin.has_module_permission` The ``InlineModelAdmin`` class adds: diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index b4f385529d..f4098fa08a 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -31,7 +31,9 @@ Minor features :mod:`django.contrib.admin` ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -* ... +* :class:`~django.contrib.admin.ModelAdmin` now has a + :meth:`~django.contrib.admin.ModelAdmin.has_module_permission` + method to allow limiting access to the module on the admin index page. :mod:`django.contrib.auth` ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/admin_ordering/tests.py b/tests/admin_ordering/tests.py index 7a6efb0b4d..bc08adee12 100644 --- a/tests/admin_ordering/tests.py +++ b/tests/admin_ordering/tests.py @@ -17,6 +17,9 @@ class MockSuperUser(object): def has_perm(self, perm): return True + def has_module_perms(self, module): + return True + request = MockRequest() request.user = MockSuperUser() diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index db186f6027..22ff4536fa 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -124,6 +124,12 @@ class ArticleAdmin(admin.ModelAdmin): return super(ArticleAdmin, self).save_model(request, obj, form, change) +class ArticleAdmin2(admin.ModelAdmin): + + def has_module_permission(self, request): + return False + + class RowLevelChangePermissionModelAdmin(admin.ModelAdmin): def has_change_permission(self, request, obj=None): """ Only allow changing objects with even id number """ @@ -923,3 +929,5 @@ site.register(Group, GroupAdmin) site2 = admin.AdminSite(name="namespaced_admin") site2.register(User, UserAdmin) site2.register(Group, GroupAdmin) +site7 = admin.AdminSite(name="admin7") +site7.register(Article, ArticleAdmin2) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 185f252335..c8ae5e743c 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -1493,6 +1493,70 @@ class AdminViewPermissionsTest(TestCase): self.assertEqual(response.status_code, 302) self.assertEqual(response.url, 'http://example.com/dummy/foo/') + def test_has_module_permission(self): + """ + Ensure that has_module_permission() returns True for all users who + have any permission for that module (add, change, or delete), so that + the module is displayed on the admin index page. + """ + login_url = reverse('admin:login') + '?next=/test_admin/admin/' + + self.client.post(login_url, self.super_login) + response = self.client.get('/test_admin/admin/') + self.assertContains(response, 'admin_views') + self.assertContains(response, 'Articles') + self.client.get('/test_admin/admin/logout/') + + self.client.post(login_url, self.adduser_login) + response = self.client.get('/test_admin/admin/') + self.assertContains(response, 'admin_views') + self.assertContains(response, 'Articles') + self.client.get('/test_admin/admin/logout/') + + self.client.post(login_url, self.changeuser_login) + response = self.client.get('/test_admin/admin/') + self.assertContains(response, 'admin_views') + self.assertContains(response, 'Articles') + self.client.get('/test_admin/admin/logout/') + + self.client.post(login_url, self.deleteuser_login) + response = self.client.get('/test_admin/admin/') + self.assertContains(response, 'admin_views') + self.assertContains(response, 'Articles') + self.client.get('/test_admin/admin/logout/') + + def test_overriding_has_module_permission(self): + """ + Ensure that overriding has_module_permission() has the desired effect. + In this case, it always returns False, so the module should not be + displayed on the admin index page for any users. + """ + login_url = reverse('admin:login') + '?next=/test_admin/admin7/' + + self.client.post(login_url, self.super_login) + response = self.client.get('/test_admin/admin7/') + self.assertNotContains(response, 'admin_views') + self.assertNotContains(response, 'Articles') + self.client.get('/test_admin/admin7/logout/') + + self.client.post(login_url, self.adduser_login) + response = self.client.get('/test_admin/admin7/') + self.assertNotContains(response, 'admin_views') + self.assertNotContains(response, 'Articles') + self.client.get('/test_admin/admin7/logout/') + + self.client.post(login_url, self.changeuser_login) + response = self.client.get('/test_admin/admin7/') + self.assertNotContains(response, 'admin_views') + self.assertNotContains(response, 'Articles') + self.client.get('/test_admin/admin7/logout/') + + self.client.post(login_url, self.deleteuser_login) + response = self.client.get('/test_admin/admin7/') + self.assertNotContains(response, 'admin_views') + self.assertNotContains(response, 'Articles') + self.client.get('/test_admin/admin7/logout/') + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), ROOT_URLCONF="admin_views.urls") diff --git a/tests/admin_views/urls.py b/tests/admin_views/urls.py index c0c59d34fc..a9fa71040c 100644 --- a/tests/admin_views/urls.py +++ b/tests/admin_views/urls.py @@ -11,4 +11,5 @@ urlpatterns = [ url(r'^test_admin/admin3/', include(admin.site.urls), dict(form_url='pony')), url(r'^test_admin/admin4/', include(customadmin.simple_site.urls)), url(r'^test_admin/admin5/', include(admin.site2.urls)), + url(r'^test_admin/admin7/', include(admin.site7.urls)), ] diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 3f5f378818..93cb58a42c 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -1542,3 +1542,93 @@ class ListDisplayEditableTests(CheckTestCase): list_editable = ['name', 'slug'] list_display_links = ['pub_date'] self.assertIsValid(ProductAdmin, ValidationTestModel) + + +class ModelAdminPermissionTests(TestCase): + + class MockUser(object): + def has_module_perms(self, app_label): + if app_label == "modeladmin": + return True + return False + + class MockAddUser(MockUser): + def has_perm(self, perm): + if perm == "modeladmin.add_band": + return True + return False + + class MockChangeUser(MockUser): + def has_perm(self, perm): + if perm == "modeladmin.change_band": + return True + return False + + class MockDeleteUser(MockUser): + def has_perm(self, perm): + if perm == "modeladmin.delete_band": + return True + return False + + def test_has_add_permission(self): + """ + Ensure that has_add_permission returns True for users who can add + objects and False for users who can't. + """ + ma = ModelAdmin(Band, AdminSite()) + request = MockRequest() + request.user = self.MockAddUser() + self.assertTrue(ma.has_add_permission(request)) + request.user = self.MockChangeUser() + self.assertFalse(ma.has_add_permission(request)) + request.user = self.MockDeleteUser() + self.assertFalse(ma.has_add_permission(request)) + + def test_has_change_permission(self): + """ + Ensure that has_change_permission returns True for users who can edit + objects and False for users who can't. + """ + ma = ModelAdmin(Band, AdminSite()) + request = MockRequest() + request.user = self.MockAddUser() + self.assertFalse(ma.has_change_permission(request)) + request.user = self.MockChangeUser() + self.assertTrue(ma.has_change_permission(request)) + request.user = self.MockDeleteUser() + self.assertFalse(ma.has_change_permission(request)) + + def test_has_delete_permission(self): + """ + Ensure that has_delete_permission returns True for users who can delete + objects and False for users who can't. + """ + ma = ModelAdmin(Band, AdminSite()) + request = MockRequest() + request.user = self.MockAddUser() + self.assertFalse(ma.has_delete_permission(request)) + request.user = self.MockChangeUser() + self.assertFalse(ma.has_delete_permission(request)) + request.user = self.MockDeleteUser() + self.assertTrue(ma.has_delete_permission(request)) + + def test_has_module_permission(self): + """ + Ensure that has_module_permission returns True for users who have any + permission for the module and False for users who don't. + """ + ma = ModelAdmin(Band, AdminSite()) + request = MockRequest() + request.user = self.MockAddUser() + self.assertTrue(ma.has_module_permission(request)) + request.user = self.MockChangeUser() + self.assertTrue(ma.has_module_permission(request)) + request.user = self.MockDeleteUser() + self.assertTrue(ma.has_module_permission(request)) + ma.opts.app_label = "anotherapp" + request.user = self.MockAddUser() + self.assertFalse(ma.has_module_permission(request)) + request.user = self.MockChangeUser() + self.assertFalse(ma.has_module_permission(request)) + request.user = self.MockDeleteUser() + self.assertFalse(ma.has_module_permission(request))