[fix] Ensure admin actions are executed only if user has permission

openwisp_controller/config/admin.py

@@ -564,13 +564,16 @@ def construct_change_message(self, request, form, formsets, add=False):
             formsets[0] = self._config_formset
         return super().construct_change_message(request, form, formsets, add)
 
+    @admin.action(
+        description=_('Change group of selected Devices'), permissions=['change']
+    )
     def change_group(self, request, queryset):
         # Validate all selected devices belong to the same organization
         # which is managed by the user.
         org_id = None
         if queryset:
             org_id = queryset[0].organization_id
-        if not request.user.is_superuser and request.user.is_manager(org_id):
+        if not request.user.is_superuser and not request.user.is_manager(org_id):
             logger.warning(f'{request.user} does not manage "{org_id}" organization.')
             return HttpResponseForbidden()
         if len(queryset) != queryset.filter(organization_id=org_id).count():
@@ -621,8 +624,6 @@ def change_group(self, request, queryset):
             request, 'admin/config/change_device_group.html', context
         )
 
-    change_group.short_description = _('Change group of selected Devices')
-
     def get_fields(self, request, obj=None):
         """
         Do not show readonly fields in add form
@@ -820,6 +821,7 @@ class TemplateAdmin(MultitenantAdminMixin, BaseConfigAdmin, SystemDefinedVariabl
     readonly_fields = ['system_context']
     autocomplete_fields = ['vpn']
 
+    @admin.action(permissions=['add'])
     def clone_selected_templates(self, request, queryset):
         selectable_orgs = None
         user = request.user

openwisp_controller/config/tests/test_admin.py

@@ -13,7 +13,7 @@
 from django.urls import reverse
 from swapper import load_model
 
-from openwisp_utils.tests import catch_signal
+from openwisp_utils.tests import AdminActionPermTestMixin, catch_signal
 
 from ...geo.tests.utils import TestGeoMixin
 from ...tests.utils import TestAdminMixin
@@ -223,6 +223,7 @@ def test_device_import_config_not_templates(self):
 
 
 class TestAdmin(
+    AdminActionPermTestMixin,
     TestImportExportMixin,
     TestGeoMixin,
     CreateDeviceGroupMixin,
@@ -600,6 +601,22 @@ def test_change_device_group_action(self):
             response, 'What group do you want to assign to the selected devices?'
         )
 
+    def test_change_device_group_action_perms(self):
+        org = self._get_org()
+        user = self._create_user(is_staff=True)
+        self._create_org_user(is_admin=True, organization=org, user=user)
+        device = self._create_device(organization=org)
+        group = self._create_device_group(name='default', organization=org)
+        self._test_action_permission(
+            path=reverse(f'admin:{self.app_label}_device_changelist'),
+            action='change_group',
+            user=user,
+            obj=device,
+            message='Successfully changed group of selected devices.',
+            required_perms=['change'],
+            extra_payload={'device_group': group.pk, 'apply': True},
+        )
+
     def test_device_import_with_group_apply_templates(self):
         org = self._get_org(org_name='default')
         template = self._create_template(name='template')
@@ -991,6 +1008,20 @@ def test_clone_templates_org_errors(self):
         with self.subTest('template count should not change'):
             self.assertEqual(Template.objects.count(), count)
 
+    def test_clone_selected_templates_action_perms(self):
+        org = self._get_org()
+        user = self._create_user(is_staff=True)
+        self._create_org_user(is_admin=True, organization=org, user=user)
+        template = self._create_template(organization=org)
+        self._test_action_permission(
+            path=reverse(f'admin:{self.app_label}_template_changelist'),
+            action='clone_selected_templates',
+            user=user,
+            obj=template,
+            message='Successfully cloned selected templates.',
+            required_perms=['add'],
+        )
+
     def test_change_device_clean_templates(self):
         o = self._get_org()
         t = Template.objects.first()