[change] Limit actions on disabled organizations #815

Controller views return 404 response for disabled organizations.

Closes #815

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,

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))

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)

openwisp_controller/config/tests/test_controller.py

@@ -59,6 +59,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
@@ -355,6 +370,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])
@@ -396,6 +417,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',
@@ -875,6 +903,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(
@@ -1125,8 +1167,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},

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()