[2.1.x] Fixed #29419 -- Allowed permissioning of admin actions.

Backport of 958c7b301ead79974db8edd5b9c6588a10a28ae7 from master

9 files changed, 192 insertions, 17 deletions

diff --git a/django/contrib/admin/actions.py b/django/contrib/admin/actions.py
index f64b89205e..1e1c3bd384 100644
--- a/django/contrib/admin/actions.py
+++ b/django/contrib/admin/actions.py
@@ -23,10 +23,6 @@ def delete_selected(modeladmin, request, queryset):
     opts = modeladmin.model._meta
     app_label = opts.app_label
 
-    # Check that the user has delete permission for the actual model
-    if not modeladmin.has_delete_permission(request):
-        raise PermissionDenied
-
     # Populate deletable_objects, a data structure of all related objects that
     # will also be deleted.
     deletable_objects, model_count, perms_needed, protected = modeladmin.get_deleted_objects(queryset, request)
@@ -79,4 +75,5 @@ def delete_selected(modeladmin, request, queryset):
     ], context)
 
 
+delete_selected.allowed_permissions = ('delete',)
 delete_selected.short_description = gettext_lazy("Delete selected %(verbose_name_plural)s")
diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py
index cf5b312369..8dd5a72256 100644
--- a/django/contrib/admin/checks.py
+++ b/django/contrib/admin/checks.py
@@ -579,6 +579,7 @@ class ModelAdminChecks(BaseModelAdminChecks):
             *self._check_list_editable(admin_obj),
             *self._check_search_fields(admin_obj),
             *self._check_date_hierarchy(admin_obj),
+            *self._check_action_permission_methods(admin_obj),
         ]
 
     def _check_save_as(self, obj):
@@ -891,6 +892,32 @@ class ModelAdminChecks(BaseModelAdminChecks):
                 else:
                     return []
 
+    def _check_action_permission_methods(self, obj):
+        """
+        Actions with an allowed_permission attribute require the ModelAdmin to
+        implement a has_<perm>_permission() method for each permission.
+        """
+        actions = obj._get_base_actions()
+        errors = []
+        for func, name, _ in actions:
+            if not hasattr(func, 'allowed_permissions'):
+                continue
+            for permission in func.allowed_permissions:
+                method_name = 'has_%s_permission' % permission
+                if not hasattr(obj, method_name):
+                    errors.append(
+                        checks.Error(
+                            '%s must define a %s() method for the %s action.' % (
+                                obj.__class__.__name__,
+                                method_name,
+                                func.__name__,
+                            ),
+                            obj=obj.__class__,
+                            id='admin.E129',
+                        )
+                    )
+        return errors
+
 
 class InlineModelAdminChecks(BaseModelAdminChecks):
 
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 62c881eb74..0f4dd93cb3 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -852,16 +852,8 @@ class ModelAdmin(BaseModelAdmin):
         return helpers.checkbox.render(helpers.ACTION_CHECKBOX_NAME, str(obj.pk))
     action_checkbox.short_description = mark_safe('<input type="checkbox" id="action-toggle">')
 
-    def get_actions(self, request):
-        """
-        Return a dictionary mapping the names of all actions for this
-        ModelAdmin to a tuple of (callable, name, description) for each action.
-        """
-        # If self.actions is explicitly set to None that means that we don't
-        # want *any* actions enabled on this page.
-        if self.actions is None or IS_POPUP_VAR in request.GET:
-            return OrderedDict()
-
+    def _get_base_actions(self):
+        """Return the list of actions, prior to any request-based filtering."""
         actions = []
 
         # Gather actions from the admin site first
@@ -876,8 +868,34 @@ class ModelAdmin(BaseModelAdmin):
             actions.extend(self.get_action(action) for action in class_actions)
 
         # get_action might have returned None, so filter any of those out.
-        actions = filter(None, actions)
+        return filter(None, actions)
+
+    def _filter_actions_by_permissions(self, request, actions):
+        """Filter out any actions that the user doesn't have access to."""
+        filtered_actions = []
+        for action in actions:
+            callable = action[0]
+            if not hasattr(callable, 'allowed_permissions'):
+                filtered_actions.append(action)
+                continue
+            permission_checks = (
+                getattr(self, 'has_%s_permission' % permission)
+                for permission in callable.allowed_permissions
+            )
+            if any(has_permission(request) for has_permission in permission_checks):
+                filtered_actions.append(action)
+        return filtered_actions
 
+    def get_actions(self, request):
+        """
+        Return a dictionary mapping the names of all actions for this
+        ModelAdmin to a tuple of (callable, name, description) for each action.
+        """
+        # If self.actions is set to None that means actions are disabled on
+        # this page.
+        if self.actions is None or IS_POPUP_VAR in request.GET:
+            return OrderedDict()
+        actions = self._filter_actions_by_permissions(request, self._get_base_actions())
         # Convert the actions into an OrderedDict keyed by name.
         return OrderedDict(
             (name, (func, name, desc))
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index a85c6b4e6e..b0d7dfc066 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -579,6 +579,8 @@ with the admin site:
   which does not refer to a Field.
 * **admin.E128**: The value of ``date_hierarchy`` must be a ``DateField`` or
   ``DateTimeField``.
+* **admin.E129**: ``<modeladmin>`` must define a ``has_<foo>_permission()``
+  method for the ``<action>`` action.
 
 ``InlineModelAdmin``
 ~~~~~~~~~~~~~~~~~~~~
diff --git a/docs/ref/contrib/admin/actions.txt b/docs/ref/contrib/admin/actions.txt
index 0eb6de5b11..82ce1e6f92 100644
--- a/docs/ref/contrib/admin/actions.txt
+++ b/docs/ref/contrib/admin/actions.txt
@@ -357,3 +357,52 @@ Conditionally enabling or disabling actions
                     if 'delete_selected' in actions:
                         del actions['delete_selected']
                 return actions
+
+.. _admin-action-permissions:
+
+Setting permissions for actions
+-------------------------------
+
+.. versionadded:: 2.1
+
+Actions may limit their availability to users with specific permissions by
+setting an ``allowed_permissions`` attribute on the action function::
+
+    def make_published(modeladmin, request, queryset):
+        queryset.update(status='p')
+    make_published.allowed_permissions = ('change',)
+
+The ``make_published()`` action will only be available to users that pass the
+:meth:`.ModelAdmin.has_change_permission` check.
+
+If ``allowed_permissions`` has more than one permission, the action will be
+available as long as the user passes at least one of the checks.
+
+Available values for ``allowed_permissions`` and the corresponding method
+checks are:
+
+- ``'add'``: :meth:`.ModelAdmin.has_add_permission`
+- ``'change'``: :meth:`.ModelAdmin.has_change_permission`
+- ``'delete'``: :meth:`.ModelAdmin.has_delete_permission`
+- ``'view'``: :meth:`.ModelAdmin.has_view_permission`
+
+You can specify any other value as long as you implement a corresponding
+``has_<value>_permission(self, request)`` method on the ``ModelAdmin``.
+
+For example::
+
+    from django.contrib import admin
+    from django.contrib.auth import get_permission_codename
+
+    class ArticleAdmin(admin.ModelAdmin):
+        actions = ['make_published']
+
+        def make_published(self, request, queryset):
+            queryset.update(status='p')
+        make_published.allowed_permissions = ('publish',)
+
+        def has_publish_permission(self, request):
+            """Does the user have the publish permission?"""
+            opts = self.opts
+            codename = get_permission_codename('publish', opts)
+            return request.user.has_perm('%s.%s' % (opts.app_label, codename))
diff --git a/docs/releases/2.1.txt b/docs/releases/2.1.txt
index 81f112b464..c68f673d84 100644
--- a/docs/releases/2.1.txt
+++ b/docs/releases/2.1.txt
@@ -82,6 +82,9 @@ Minor features
 * :meth:`.InlineModelAdmin.has_add_permission` is now passed the parent object
   as the second positional argument, ``obj``.
 
+* Admin actions may now :ref:`specify permissions <admin-action-permissions>`
+  to limit their availability to certain users.
+
 :mod:`django.contrib.auth`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/tests/admin_views/test_actions.py b/tests/admin_views/test_actions.py
index 423c2f927f..c0ea30fcf7 100644
--- a/tests/admin_views/test_actions.py
+++ b/tests/admin_views/test_actions.py
@@ -429,8 +429,11 @@ class AdminActionsPermissionTests(TestCase):
             ACTION_CHECKBOX_NAME: [self.s1.pk],
             'action': 'delete_selected',
         }
-        response = self.client.post(reverse('admin:admin_views_subscriber_changelist'), action_data)
-        self.assertEqual(response.status_code, 403)
+        url = reverse('admin:admin_views_subscriber_changelist')
+        response = self.client.post(url, action_data)
+        self.assertRedirects(response, url, fetch_redirect_response=False)
+        response = self.client.get(response.url)
+        self.assertContains(response, 'No action selected.')
 
     def test_model_admin_no_delete_permission_externalsubscriber(self):
         """
diff --git a/tests/modeladmin/test_actions.py b/tests/modeladmin/test_actions.py
new file mode 100644
index 0000000000..17b3c324d2
--- /dev/null
+++ b/tests/modeladmin/test_actions.py
@@ -0,0 +1,57 @@
+from django.contrib import admin
+from django.contrib.auth.models import Permission, User
+from django.contrib.contenttypes.models import ContentType
+from django.test import TestCase
+
+from .models import Band
+
+
+class AdminActionsTests(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.superuser = User.objects.create_superuser(username='super', password='secret', email='super@example.com')
+        content_type = ContentType.objects.get_for_model(Band)
+        Permission.objects.create(name='custom', codename='custom_band', content_type=content_type)
+        for user_type in ('view', 'add', 'change', 'delete', 'custom'):
+            username = '%suser' % user_type
+            user = User.objects.create_user(username=username, password='secret', is_staff=True)
+            permission = Permission.objects.get(codename='%s_band' % user_type, content_type=content_type)
+            user.user_permissions.add(permission)
+            setattr(cls, username, user)
+
+    def test_get_actions_respects_permissions(self):
+        class MockRequest:
+            pass
+
+        class BandAdmin(admin.ModelAdmin):
+            actions = ['custom_action']
+
+            def custom_action(modeladmin, request, queryset):
+                pass
+
+            def has_custom_permission(self, request):
+                return request.user.has_perm('%s.custom_band' % self.opts.app_label)
+
+        ma = BandAdmin(Band, admin.AdminSite())
+        mock_request = MockRequest()
+        mock_request.GET = {}
+        cases = [
+            (None, self.viewuser, ['custom_action']),
+            ('view', self.superuser, ['delete_selected', 'custom_action']),
+            ('view', self.viewuser, ['custom_action']),
+            ('add', self.adduser, ['custom_action']),
+            ('change', self.changeuser, ['custom_action']),
+            ('delete', self.deleteuser, ['delete_selected', 'custom_action']),
+            ('custom', self.customuser, ['custom_action']),
+        ]
+        for permission, user, expected in cases:
+            with self.subTest(permission=permission, user=user):
+                if permission is None:
+                    if hasattr(BandAdmin.custom_action, 'allowed_permissions'):
+                        del BandAdmin.custom_action.allowed_permissions
+                else:
+                    BandAdmin.custom_action.allowed_permissions = (permission,)
+                mock_request.user = user
+                actions = ma.get_actions(mock_request)
+                self.assertEqual(list(actions.keys()), expected)
diff --git a/tests/modeladmin/test_checks.py b/tests/modeladmin/test_checks.py
index 5a0433deb4..6a10441471 100644
--- a/tests/modeladmin/test_checks.py
+++ b/tests/modeladmin/test_checks.py
@@ -1290,3 +1290,22 @@ class AutocompleteFieldsTests(CheckTestCase):
         site = AdminSite()
         site.register(User, UserAdmin)
         self.assertIsValid(Admin, ValidationTestModel, admin_site=site)
+
+
+class ActionsCheckTests(CheckTestCase):
+
+    def test_custom_permissions_require_matching_has_method(self):
+        def custom_permission_action(modeladmin, request, queryset):
+            pass
+
+        custom_permission_action.allowed_permissions = ('custom',)
+
+        class BandAdmin(ModelAdmin):
+            actions = (custom_permission_action,)
+
+        self.assertIsInvalid(
+            BandAdmin, Band,
+            'BandAdmin must define a has_custom_permission() method for the '
+            'custom_permission_action action.',
+            id='admin.E129',
+        )