From ca302ae469729946231cb088e7a7789fe1c73ff9 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 9 Jan 2024 19:04:12 +0530 Subject: [PATCH] [change] Limit actions on disabled organizations #815 Controller views return 404 response for disabled organizations. Closes #815 --- openwisp_controller/config/apps.py | 13 ++++- openwisp_controller/config/handlers.py | 16 ++++++ openwisp_controller/config/tasks.py | 11 ++++ .../config/tests/test_controller.py | 52 ++++++++++++++++++- .../config/tests/test_handlers.py | 30 +++++++++++ 5 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 openwisp_controller/config/tests/test_handlers.py diff --git a/openwisp_controller/config/apps.py b/openwisp_controller/config/apps.py index f614fb901..846725c2e 100644 --- a/openwisp_controller/config/apps.py +++ b/openwisp_controller/config/apps.py @@ -2,7 +2,13 @@ from django.conf import settings from django.core.exceptions import ImproperlyConfigured from django.db.models import Case, Count, When -from django.db.models.signals import m2m_changed, post_delete, post_save, pre_delete +from django.db.models.signals import ( + m2m_changed, + post_delete, + post_save, + pre_delete, + pre_save, +) from django.utils.translation import gettext_lazy as _ from openwisp_notifications.types import ( register_notification_type, @@ -127,6 +133,11 @@ def connect_signals(self): sender=self.config_model, dispatch_uid='devicegroup_templates_change_handler.backend_changed', ) + pre_save.connect( + handlers.organization_disabled_handler, + sender=self.org_model, + dispatch_uid='organization_disabled_pre_save_clear_device_checksum_cache', + ) post_save.connect( self.org_limits.post_save_handler, sender=self.org_model, diff --git a/openwisp_controller/config/handlers.py b/openwisp_controller/config/handlers.py index c07ecb654..744789051 100644 --- a/openwisp_controller/config/handlers.py +++ b/openwisp_controller/config/handlers.py @@ -136,3 +136,19 @@ def devicegroup_templates_change_handler(instance, **kwargs): backend=kwargs.get('backend'), old_backend=kwargs.get('old_backend'), ) + + +def organization_disabled_handler(instance, **kwargs): + """ + Asynchronously invalidates DeviceCheckView.get_device cache + """ + if instance.is_active: + return + try: + db_instance = Organization.objects.only('is_active').get(id=instance.id) + except Organization.DoesNotExist: + return + if instance.is_active == db_instance.is_active: + # No change in is_active + return + tasks.invalidate_device_checksum_view_cache.delay(str(instance.id)) diff --git a/openwisp_controller/config/tasks.py b/openwisp_controller/config/tasks.py index 16f53d1ab..b9fb05915 100644 --- a/openwisp_controller/config/tasks.py +++ b/openwisp_controller/config/tasks.py @@ -146,3 +146,14 @@ def change_devices_templates(instance_id, model_name, **kwargs): def bulk_invalidate_config_get_cached_checksum(query_params): Config = load_model('config', 'Config') Config.bulk_invalidate_get_cached_checksum(query_params) + + +@shared_task(base=OpenwispCeleryTask) +def invalidate_device_checksum_view_cache(organization_id): + from .controller.views import DeviceChecksumView + + Device = load_model('config', 'Device') + for device in ( + Device.objects.filter(organization_id=organization_id).only('id').iterator() + ): + DeviceChecksumView.invalidate_get_device_cache(device) diff --git a/openwisp_controller/config/tests/test_controller.py b/openwisp_controller/config/tests/test_controller.py index a82a27361..32b055de4 100644 --- a/openwisp_controller/config/tests/test_controller.py +++ b/openwisp_controller/config/tests/test_controller.py @@ -60,6 +60,21 @@ def _create_org(self, shared_secret=TEST_ORG_SHARED_SECRET, **kwargs): def _check_header(self, response): self.assertEqual(response['X-Openwisp-Controller'], 'true') + def _test_view_organization_disabled( + self, obj, url, http_method='get', org=None, data=None + ): + method = getattr(self.client, http_method) + data = data or {} + payload = {'key': obj.key, **data} + response = method(url, payload) + self.assertEqual(response.status_code, 200) + # Disable organization + org = org or getattr(obj, 'organization') + org.is_active = False + org.save() + response = method(url, {'key': obj.key}) + self.assertEqual(response.status_code, 404) + def test_device_checksum(self): d = self._create_device_config() c = d.config @@ -356,6 +371,12 @@ def test_vpn_checksum_405(self): ) self.assertEqual(response.status_code, 405) + def test_vpn_checksum_org_disabled(self): + vpn = self._create_vpn(organization=self._get_org()) + self._test_view_organization_disabled( + vpn, reverse('controller:vpn_checksum', args=[vpn.pk]) + ) + def test_vpn_download_config(self): v = self._create_vpn() url = reverse('controller:vpn_download_config', args=[v.pk]) @@ -397,6 +418,13 @@ def test_vpn_download_config_405(self): ) self.assertEqual(response.status_code, 405) + def test_vpn_download_config_org_disabled(self): + vpn = self._create_vpn(organization=self._get_org()) + self._test_view_organization_disabled( + vpn, + reverse('controller:vpn_download_config', args=[vpn.pk]), + ) + def test_register(self, **kwargs): options = { 'hardware_id': '1234', @@ -890,6 +918,20 @@ def test_device_update_info_405(self): ) self.assertEqual(response.status_code, 405) + def test_device_update_info_org_disabled(self): + device = self._create_device_config() + self._test_view_organization_disabled( + device, + reverse('controller:device_update_info', args=[device.pk]), + http_method='post', + data={ + 'key': device.key, + 'model': 'TP-Link TL-WDR4300 v2', + 'os': 'OpenWrt 18.06-SNAPSHOT r7312-e60be11330', + 'system': 'Atheros AR9344 rev 3', + }, + ) + def test_device_checksum_no_config(self): d = self._create_device() response = self.client.get( @@ -1140,8 +1182,16 @@ def test_register_403_disabled_org(self): self.assertContains(response, 'error: unrecognized secret', status_code=403) def test_checksum_404_disabled_org(self): - org = self._create_org(is_active=False) + org = self._create_org() c = self._create_config(organization=org) + # Cache checksum + response = self.client.get( + reverse('controller:device_checksum', args=[c.device.pk]), + {'key': c.device.key}, + ) + self.assertEqual(response.status_code, 200) + org.is_active = False + org.save() response = self.client.get( reverse('controller:device_checksum', args=[c.device.pk]), {'key': c.device.key}, diff --git a/openwisp_controller/config/tests/test_handlers.py b/openwisp_controller/config/tests/test_handlers.py new file mode 100644 index 000000000..3e3d24919 --- /dev/null +++ b/openwisp_controller/config/tests/test_handlers.py @@ -0,0 +1,30 @@ +from unittest.mock import patch + +from django.test import TestCase + +from openwisp_users.tests.utils import TestOrganizationMixin + +from .. import tasks + + +class TestHandlers(TestOrganizationMixin, TestCase): + @patch.object(tasks.invalidate_device_checksum_view_cache, 'delay') + def test_organization_disabled_handler(self, mocked_task): + with self.subTest('Test task not executed on creating active orgs'): + org = self._create_org() + mocked_task.assert_not_called() + + with self.subTest('Test task executed on changing active to inactive org'): + org.is_active = False + org.save() + mocked_task.assert_called_once() + + mocked_task.reset_mock() + with self.subTest('Test task not executed on saving inactive org'): + org.name = 'Changed named' + org.save() + mocked_task.assert_not_called() + + with self.subTest('Test task not executed on creating inactive org'): + self._create_org(is_active=False, name='inactive', slug='inactive') + mocked_task.assert_not_called()