mirror of
https://github.com/django/django.git
synced 2024-12-22 17:16:24 +00:00
Fixed #29419 -- Allowed permissioning of admin actions.
This commit is contained in:
parent
6dd4edb1b4
commit
958c7b301e
@ -23,10 +23,6 @@ def delete_selected(modeladmin, request, queryset):
|
|||||||
opts = modeladmin.model._meta
|
opts = modeladmin.model._meta
|
||||||
app_label = opts.app_label
|
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
|
# Populate deletable_objects, a data structure of all related objects that
|
||||||
# will also be deleted.
|
# will also be deleted.
|
||||||
deletable_objects, model_count, perms_needed, protected = modeladmin.get_deleted_objects(queryset, request)
|
deletable_objects, model_count, perms_needed, protected = modeladmin.get_deleted_objects(queryset, request)
|
||||||
@ -79,4 +75,5 @@ def delete_selected(modeladmin, request, queryset):
|
|||||||
], context)
|
], context)
|
||||||
|
|
||||||
|
|
||||||
|
delete_selected.allowed_permissions = ('delete',)
|
||||||
delete_selected.short_description = gettext_lazy("Delete selected %(verbose_name_plural)s")
|
delete_selected.short_description = gettext_lazy("Delete selected %(verbose_name_plural)s")
|
||||||
|
@ -579,6 +579,7 @@ class ModelAdminChecks(BaseModelAdminChecks):
|
|||||||
*self._check_list_editable(admin_obj),
|
*self._check_list_editable(admin_obj),
|
||||||
*self._check_search_fields(admin_obj),
|
*self._check_search_fields(admin_obj),
|
||||||
*self._check_date_hierarchy(admin_obj),
|
*self._check_date_hierarchy(admin_obj),
|
||||||
|
*self._check_action_permission_methods(admin_obj),
|
||||||
]
|
]
|
||||||
|
|
||||||
def _check_save_as(self, obj):
|
def _check_save_as(self, obj):
|
||||||
@ -891,6 +892,32 @@ class ModelAdminChecks(BaseModelAdminChecks):
|
|||||||
else:
|
else:
|
||||||
return []
|
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):
|
class InlineModelAdminChecks(BaseModelAdminChecks):
|
||||||
|
|
||||||
|
@ -852,16 +852,8 @@ class ModelAdmin(BaseModelAdmin):
|
|||||||
return helpers.checkbox.render(helpers.ACTION_CHECKBOX_NAME, str(obj.pk))
|
return helpers.checkbox.render(helpers.ACTION_CHECKBOX_NAME, str(obj.pk))
|
||||||
action_checkbox.short_description = mark_safe('<input type="checkbox" id="action-toggle">')
|
action_checkbox.short_description = mark_safe('<input type="checkbox" id="action-toggle">')
|
||||||
|
|
||||||
def get_actions(self, request):
|
def _get_base_actions(self):
|
||||||
"""
|
"""Return the list of actions, prior to any request-based filtering."""
|
||||||
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()
|
|
||||||
|
|
||||||
actions = []
|
actions = []
|
||||||
|
|
||||||
# Gather actions from the admin site first
|
# 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)
|
actions.extend(self.get_action(action) for action in class_actions)
|
||||||
|
|
||||||
# get_action might have returned None, so filter any of those out.
|
# 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.
|
# Convert the actions into an OrderedDict keyed by name.
|
||||||
return OrderedDict(
|
return OrderedDict(
|
||||||
(name, (func, name, desc))
|
(name, (func, name, desc))
|
||||||
|
@ -579,6 +579,8 @@ with the admin site:
|
|||||||
which does not refer to a Field.
|
which does not refer to a Field.
|
||||||
* **admin.E128**: The value of ``date_hierarchy`` must be a ``DateField`` or
|
* **admin.E128**: The value of ``date_hierarchy`` must be a ``DateField`` or
|
||||||
``DateTimeField``.
|
``DateTimeField``.
|
||||||
|
* **admin.E129**: ``<modeladmin>`` must define a ``has_<foo>_permission()``
|
||||||
|
method for the ``<action>`` action.
|
||||||
|
|
||||||
``InlineModelAdmin``
|
``InlineModelAdmin``
|
||||||
~~~~~~~~~~~~~~~~~~~~
|
~~~~~~~~~~~~~~~~~~~~
|
||||||
|
@ -357,3 +357,52 @@ Conditionally enabling or disabling actions
|
|||||||
if 'delete_selected' in actions:
|
if 'delete_selected' in actions:
|
||||||
del actions['delete_selected']
|
del actions['delete_selected']
|
||||||
return actions
|
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))
|
||||||
|
@ -82,6 +82,9 @@ Minor features
|
|||||||
* :meth:`.InlineModelAdmin.has_add_permission` is now passed the parent object
|
* :meth:`.InlineModelAdmin.has_add_permission` is now passed the parent object
|
||||||
as the second positional argument, ``obj``.
|
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`
|
:mod:`django.contrib.auth`
|
||||||
~~~~~~~~~~~~~~~~~~~~~~~~~~
|
~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||||
|
|
||||||
|
@ -429,8 +429,11 @@ class AdminActionsPermissionTests(TestCase):
|
|||||||
ACTION_CHECKBOX_NAME: [self.s1.pk],
|
ACTION_CHECKBOX_NAME: [self.s1.pk],
|
||||||
'action': 'delete_selected',
|
'action': 'delete_selected',
|
||||||
}
|
}
|
||||||
response = self.client.post(reverse('admin:admin_views_subscriber_changelist'), action_data)
|
url = reverse('admin:admin_views_subscriber_changelist')
|
||||||
self.assertEqual(response.status_code, 403)
|
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):
|
def test_model_admin_no_delete_permission_externalsubscriber(self):
|
||||||
"""
|
"""
|
||||||
|
57
tests/modeladmin/test_actions.py
Normal file
57
tests/modeladmin/test_actions.py
Normal file
@ -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)
|
@ -1290,3 +1290,22 @@ class AutocompleteFieldsTests(CheckTestCase):
|
|||||||
site = AdminSite()
|
site = AdminSite()
|
||||||
site.register(User, UserAdmin)
|
site.register(User, UserAdmin)
|
||||||
self.assertIsValid(Admin, ValidationTestModel, admin_site=site)
|
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',
|
||||||
|
)
|
||||||
|
Loading…
Reference in New Issue
Block a user