diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index 95af2154d..43fc6b7e9 100644 --- a/openwisp_controller/config/admin.py +++ b/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[^/]+)/$', - 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): diff --git a/openwisp_controller/config/static/config/js/default_templates.js b/openwisp_controller/config/static/config/js/default_templates.js index d7bf4286e..b3e981b63 100644 --- a/openwisp_controller/config/static/config/js/default_templates.js +++ b/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,7 +18,7 @@ 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 @@ -26,7 +26,7 @@ django.jQuery(function ($) { 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); }); }); } diff --git a/openwisp_controller/config/templates/admin/config/change_form.html b/openwisp_controller/config/templates/admin/config/change_form.html index 6c98aa652..e662c04ce 100644 --- a/openwisp_controller/config/templates/admin/config/change_form.html +++ b/openwisp_controller/config/templates/admin/config/change_form.html @@ -56,12 +56,12 @@ {% endfor %} {% endif %} - {% if default_template_urls %} + {% if default_template_url %} diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index fe998dc5d..89f7fe12e 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -986,7 +986,7 @@ 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'}} @@ -994,7 +994,7 @@ def test_get_template_default_values(self): 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'}} diff --git a/openwisp_controller/config/tests/test_views.py b/openwisp_controller/config/tests/test_views.py index 8a61297f8..703c8b5e5 100644 --- a/openwisp_controller/config/tests/test_views.py +++ b/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) diff --git a/openwisp_controller/config/views.py b/openwisp_controller/config/views.py index 7926415d3..fa04ec1e5 100644 --- a/openwisp_controller/config/views.py +++ b/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)