[fix] Fixed authorization bug in get_default_templates view #420

openwisp_controller/config/admin.py

@@ -467,34 +467,29 @@ def _get_preview_instance(self, request):
             c.device.hardware_id = request.POST.get('hardware_id')
         return c
 
-    def _get_default_template_urls(self):
-        """
-        returns URLs to get default templates
-        used in change_form.html template
-        """
-        organizations = Organization.active.all()
-        urls = {}
-        for org in organizations:
-            urls[str(org.pk)] = reverse('admin:get_default_templates', args=[org.pk])
-        return json.dumps(urls)
-
     def get_urls(self):
         return [
             url(
                 r'^config/get-default-templates/(?P<organization_id>[^/]+)/$',
-                get_default_templates,
+                self.admin_site.admin_view(get_default_templates),
                 name='get_default_templates',
             ),
             url(
                 r'^get-template-default-values/$',
-                get_template_default_values,
+                self.admin_site.admin_view(get_template_default_values),
                 name='get_template_default_values',
             ),
         ] + super().get_urls()
 
     def get_extra_context(self, pk=None):
         ctx = super().get_extra_context(pk)
-        ctx.update({'default_template_urls': self._get_default_template_urls()})
+        ctx.update(
+            {
+                'default_template_url': reverse(
+                    'admin:get_default_templates', args=['org_id']
+                )
+            }
+        )
         return ctx
 
     def add_view(self, request, form_url='', extra_context=None):

openwisp_controller/config/static/config/js/default_templates.js

@@ -1,13 +1,13 @@
 'use strict';
 django.jQuery(function ($) {
     var firstRun = true,
-        addChangeEventToBackend = function (urls) {
+        addChangeEventToBackend = function (urlSchema) {
             $('#id_config-0-backend').change(function () {
                 setTimeout(function () {
                     // ensures getDefaultTemplates execute only after other
                     // onChange event handlers attached this field has been
                     // executed.
-                    getDefaultTemplates(urls);
+                    getDefaultTemplates(urlSchema);
                 });
             });
         },
@@ -18,15 +18,15 @@ django.jQuery(function ($) {
                 window.updateContext();
             }
         },
-        getDefaultTemplates = function (urls) {
+        getDefaultTemplates = function (urlSchema) {
             var orgID = $('#id_organization').val(),
                 backend = $('#id_config-0-backend').val();
             // proceed only if an organization and a backend have been selected
             if (orgID.length === 0 || backend.length === 0) {
                 unCheckInputs();
                 return;
             }
-            var url = urls[orgID],
+            var url = urlSchema.replace('org_id', orgID),
                 isNew = $('#id_config-0-id').length == 0;
             // if device is not new, do not execute on page load
             if (!isNew && firstRun) {
@@ -46,20 +46,20 @@ django.jQuery(function ($) {
                 });
             });
         },
-        bindDefaultTemplateLoading = function (urls) {
+        bindDefaultTemplateLoading = function (urlSchema) {
             var backendField = $('#id_config-0-backend');
             $('#id_organization').change(function () {
                 if ($('#id_config-0-backend').length > 0) {
-                    getDefaultTemplates(urls);
+                    getDefaultTemplates(urlSchema);
                 }
             });
             if (backendField.length > 0) {
-                addChangeEventToBackend(urls);
+                addChangeEventToBackend(urlSchema);
             } else {
                 $('#config-group > fieldset.module').ready(function () {
                     $('div.add-row > a').click(function () {
-                        addChangeEventToBackend(urls);
-                        getDefaultTemplates(urls);
+                        addChangeEventToBackend(urlSchema);
+                        getDefaultTemplates(urlSchema);
                     });
                 });
             }

openwisp_controller/config/templates/admin/config/change_form.html

@@ -56,12 +56,12 @@
         {% endfor %}
     </script>
     {% endif %}
-    {% if default_template_urls %}
+    {% if default_template_url %}
     <script>
         // enable default templates - do not remove this comment
         (function ($) {
             $(document).ready( function () {
-                window.bindDefaultTemplateLoading({{ default_template_urls| safe }});
+                window.bindDefaultTemplateLoading("{{ default_template_url }}");
             })
         }) (django.jQuery);
     </script>

openwisp_controller/config/tests/test_admin.py

@@ -986,15 +986,15 @@ def test_get_template_default_values(self):
         path = reverse('admin:get_template_default_values')
 
         with self.subTest('get default values for one template'):
-            with self.assertNumQueries(1):
+            with self.assertNumQueries(3):
                 r = self.client.get(path, {'pks': f'{t1.pk}'})
                 self.assertEqual(r.status_code, 200)
                 expected = {'default_values': {'name1': 'test1'}}
                 self.assertEqual(r.json(), expected)
 
         with self.subTest('get default values for multiple templates'):
             t2 = self._create_template(name='t2', default_values={'name2': 'test2'})
-            with self.assertNumQueries(1):
+            with self.assertNumQueries(3):
                 r = self.client.get(path, {'pks': f'{t1.pk},{t2.pk}'})
                 self.assertEqual(r.status_code, 200)
                 expected = {'default_values': {'name1': 'test1', 'name2': 'test2'}}

openwisp_controller/config/tests/test_views.py

@@ -103,12 +103,46 @@ def test_get_default_templates_with_backend_filtering(self):
         self.assertIn(str(t1.pk), templates)
         self.assertNotIn(str(t2.pk), templates)
 
-    def test_get_default_templates_403(self):
+    def test_get_default_templates_authorization(self):
         org1 = self._create_org(name='org1')
-        response = self.client.get(
-            reverse('admin:get_default_templates', args=[org1.pk])
-        )
-        self.assertEqual(response.status_code, 403)
+        with self.subTest('Unauthenticated user'):
+            # Unauthenticated users will be redirected to login page
+            response = self.client.get(
+                reverse('admin:get_default_templates', args=[org1.pk])
+            )
+            self.assertEqual(response.status_code, 302)
+
+        with self.subTest('Authenticated non-staff user'):
+            # Non-staff users will be redirected to login page of admin
+            # and will be asked to login with a staff account
+            user = self._create_user()
+            self.client.force_login(user)
+            response = self.client.get(
+                reverse('admin:get_default_templates', args=[org1.pk])
+            )
+            self.assertEqual(response.status_code, 302)
+            response = self.client.get(
+                reverse('admin:get_default_templates', args=[org1.pk]), follow=True
+            )
+            self.assertContains(response, 'not authorized')
+
+        with self.subTest('User requests data of other organization'):
+            org_owner = self._create_org_owner()
+            user = org_owner.organization_user.user
+            user.is_staff = True
+            user.save()
+            self.client.force_login(user)
+            response = self.client.get(
+                reverse('admin:get_default_templates', args=[org1.pk])
+            )
+            self.assertEqual(response.status_code, 403)
+
+        with self.subTest('Superuser requests data for any organization'):
+            self._login()
+            response = self.client.get(
+                reverse('admin:get_default_templates', args=[org1.pk])
+            )
+            self.assertEqual(response.status_code, 200)
 
     def test_get_default_templates_404(self):
         self._login()
@@ -141,3 +175,57 @@ def test_get_default_templates_400(self):
             reverse('admin:get_default_templates', args=['wrong'])
         )
         self.assertEqual(response.status_code, 404)
+
+    def get_template_default_values_authorization(self):
+        org1 = self._get_org()
+        org1_template = self._create_template(
+            organization=org1, default_values={'org1': 'secret1'}
+        )
+        org2 = self._create_org(name='org2')
+        org2_template = self._create_template(
+            organization=org2, default_values={'org2': 'secret2'}
+        )
+        shared_template = self._create_template(
+            name='shared-template', default_values={'key': 'value'}
+        )
+        url = (
+            reverse('admin:get_template_default_values')
+            + f'?pks={org1_template.pk},{org2_template.pk},{shared_template.pk}'
+        )
+
+        with self.subTest('Unauthenticated user'):
+            # Unauthenticated users will be redirected to login page
+            response = self.client.get(url)
+            self.assertEqual(response.status_code, 302)
+
+        with self.subTest('Authenticated non-staff user'):
+            # Non-staff users will be redirected to login page of admin
+            # and will be asked to login with a staff account
+            user = self._create_user()
+            self.client.force_login(user)
+            response = self.client.get(url)
+            self.assertEqual(response.status_code, 302)
+            response = self.client.get(
+                reverse('admin:get_default_templates', args=[org1.pk]), follow=True
+            )
+            self.assertContains(response, 'not authorized')
+
+        with self.subTest('Org admin requests data of other organization'):
+            org1_admin = self._create_org_user(organization=org1, is_admin=True)
+            org1_user = org1_admin.user
+            org1_user.is_staff = True
+            org1_user.save()
+            self.client.force_login(org1_user)
+            expected_response = {'default_values': {'org1': 'secret1', 'key': 'value'}}
+            response = self.client.get(url)
+            self.assertEqual(response.status_code, 200)
+            self.assertJSONEqual(response.content, expected_response)
+
+        with self.subTest('Superuser requests data for any organization'):
+            self._login()
+            response = self.client.get(url)
+            expected_response = {
+                'default_values': {'org1': 'secret1', 'org2': 'secret2', 'key': 'value'}
+            }
+            self.assertEqual(response.status_code, 200)
+            self.assertJSONEqual(response.content, expected_response)

openwisp_controller/config/views.py

@@ -2,6 +2,7 @@
 from copy import deepcopy
 from uuid import UUID
 
+from django.db.models import Q
 from django.http import HttpResponse, JsonResponse
 from django.utils import timezone
 from django.utils.module_loading import import_string
@@ -22,8 +23,7 @@ def get_default_templates(request, organization_id):
     """
     backend = request.GET.get("backend", None)
     user = request.user
-    authenticated = user.is_authenticated
-    if not authenticated and not user.is_staff:
+    if not user.is_superuser and not user.is_manager(organization_id):
         return HttpResponse(status=403)
     org = get_object_or_404(Organization, pk=organization_id, is_active=True)
     templates = get_default_templates_queryset(org.pk, backend, model=Template).only(
@@ -82,6 +82,7 @@ def get_template_default_values(request):
     """
     returns default_values for one or more templates
     """
+    user = request.user
     pk_list = []
     for pk in request.GET.get('pks', '').split(','):
         try:
@@ -92,9 +93,12 @@ def get_template_default_values(request):
             )
         else:
             pk_list.append(pk)
-    values = Template.objects.filter(pk__in=pk_list).values_list(
-        'default_values', flat=True
-    )
+    where = Q(pk__in=pk_list)
+    if not user.is_superuser:
+        where = where & (
+            Q(organization=None) | Q(organization__in=user.organizations_managed)
+        )
+    values = Template.objects.filter(where).values_list('default_values', flat=True)
     default_values = {}
     for item in values:
         default_values.update(item)