[change] Enforce deactivated device state across modules #1338

This change introduces a generic enforcement of the deactivated device
state across OpenWISP Controller. When a device is deactivated, it
is treated as non-operational. This means no new configuration,
management, or connectivity operations will target it. However,
controller state, security, and cleanup processes will continue to run
to maintain system consistency.

Specifically, device registration and re-registration are blocked for
deactivated devices. Any workflow that would implicitly restore
configuration state or make a deactivated device appear operational
again is also blocked. Automatic template assignment and propagation
from groups will not run for deactivated configurations.

Active management traffic such as remote command execution is blocked.
Configuration pushes and reachability checks are allowed only during the
transitional state of deactivation. Once a configuration is fully
deactivated, no further controller-initiated communication will occur.

State-only and cleanup operations, like certificate revocation and cache
invalidation, remain allowed.

Closes #1338

Author: pandafy

docs/user/device-config-status.rst

@@ -35,7 +35,9 @@ scheduled to be removed from the device.
 
 The device has been deactivated. The configuration applied through
 OpenWISP has been removed, and any other operation to manage the device
-will be prevented or rejected.
+will be prevented or rejected. This also includes network operations which
+would normally push configuration updates, for example changes to shared
+templates.
 
 .. note::
 

openwisp_controller/config/admin.py

@@ -679,6 +679,7 @@ def change_group(self, request, queryset):
             form = ChangeDeviceGroupForm(data=request.POST, org_id=org_id)
             if form.is_valid():
                 group = form.cleaned_data["device_group"]
+                queryset = queryset.filter(_is_deactivated=False)
                 # Evaluate queryset to store old group id
                 old_group_qs = list(queryset)
                 queryset.update(group=group or None)

openwisp_controller/config/base/config.py

@@ -848,13 +848,15 @@ def activate(self):
         self._invalidate_backend_instance_cache()
         old_checksum = self.checksum
         self.add_default_templates()
-        if self.device._get_group():
-            self.device.manage_devices_group_templates(
-                device_ids=self.device.id,
-                old_group_ids=None,
-                group_id=self.device.group_id,
+        if self.device._has_group():
+            # Call manage_group_templates directly rather than going through
+            # manage_devices_group_templates, which would skip the device because
+            # it is still marked as deactivated at this point in the activation flow.
+            self.manage_group_templates(
+                self.device.group.templates.all(),
+                self.get_template_model().objects.none(),
             )
-        del self.backend_instance
+        self._invalidate_backend_instance_cache()
         if old_checksum == self.checksum:
             # Accelerate activation if the configuration remains
             # unchanged (i.e. empty configuration)
@@ -987,6 +989,11 @@ def manage_backend_changed(cls, instance_id, old_backend, backend, **kwargs):
         Config = load_model("config", "Config")
         Template = load_model("config", "Template")
         config = Config.objects.get(pk=instance_id)
+        # All modification operations are blocked on deactivated devices.
+        # Thus, a user cannot edit the backend for device when it is deactivating.
+        # Therefore, it will be safe to block this operation here.
+        if config.is_deactivating_or_deactivated():
+            return
         device_group = config.device.group
         if not device_group:
             return

openwisp_controller/config/base/device.py

@@ -183,8 +183,15 @@ def _get_organization__config_settings(self):
         )
 
     def is_deactivated(self):
+        """Return whether deactivation has been initiated."""
         return self._is_deactivated
 
+    def is_fully_deactivated(self):
+        """Return whether the device and its configuration are fully deactivated."""
+        return self.is_deactivated() and (
+            not self._has_config() or self.config.is_deactivated()
+        )
+
     def deactivate(self):
         if self.is_deactivated():
             # The device has already been deactivated.
@@ -299,11 +306,10 @@ def save(self, *args, **kwargs):
             self._check_changed_fields()
 
     def delete(self, using=None, keep_parents=False, check_deactivated=True):
-        if check_deactivated and (
-            not self.is_deactivated()
-            or (self._has_config() and not self.config.is_deactivated())
-        ):
-            raise PermissionDenied("The device must be deactivated prior to deletion")
+        if check_deactivated and (not self.is_fully_deactivated()):
+            raise PermissionDenied(
+                _("The device must be deactivated prior to deletion")
+            )
         return super().delete(using, keep_parents)
 
     def _check_changed_fields(self):
@@ -479,6 +485,10 @@ def create_default_config(self, **options):
         creates a new config instance to apply group templates
         if group has templates.
         """
+        if self.is_deactivated():
+            # All modification operations are blocked on deactivated devices.
+            # Hence, default config should not be created for deactivated devices.
+            return
         if not (self.group and self.group.templates.exists()):
             return
         config = self.get_temp_config_instance(
@@ -499,6 +509,11 @@ def manage_devices_group_templates(cls, device_ids, old_group_ids, group_id):
             old_group_ids = [old_group_ids]
         for device_id, old_group_id in zip(device_ids, old_group_ids):
             device = Device.objects.get(pk=device_id)
+            if device.is_deactivated():
+                # Skip deactivated devices: their configuration is intentionally
+                # emptied during deactivation, so re-applying group templates
+                # would break that state and trigger a push to the device.
+                continue
             if not hasattr(device, "config"):
                 device.create_default_config()
             config_created = hasattr(device, "config")

openwisp_controller/config/base/device_group.py

@@ -110,7 +110,7 @@ def manage_group_templates(cls, group_id, old_template_ids, template_ids):
         device_group = DeviceGroup.objects.get(id=group_id)
         templates = Template.objects.filter(pk__in=template_ids)
         old_templates = Template.objects.filter(pk__in=old_template_ids)
-        for device in device_group.device_set.iterator():
+        for device in device_group.device_set.exclude(_is_deactivated=True).iterator():
             if not hasattr(device, "config"):
                 device.create_default_config()
             device.config.manage_group_templates(templates, old_templates)

openwisp_controller/config/controller/views.py

@@ -230,6 +230,12 @@ def post(self, request, *args, **kwargs):
         bad_request = forbid_unallowed(request, "POST", "key", device.key)
         if bad_request:
             return bad_request
+        if device.is_deactivated():
+            # Inventory metadata must not be recorded for deactivated devices.
+            # GetDeviceView already returns 404 once the config is fully
+            # "deactivated"; this additionally rejects the transient
+            # "deactivating" state, where the device flag is already set.
+            return ControllerResponse("error: device deactivated", status=403)
         # update device information
         for attr in self.UPDATABLE_FIELDS:
             if attr in request.POST:
@@ -408,7 +414,9 @@ def post(self, request, *args, **kwargs):
         # (key is not None only if CONSISTENT_REGISTRATION is enabled)
         new = False
         try:
-            device = self.model.objects.get(key=key)
+            device = self.model.objects.select_related("config").get(key=key)
+            if device.is_deactivated():
+                return ControllerResponse("error: device deactivated", status=403)
             # update device info
             for attr in self.UPDATABLE_FIELDS:
                 if attr in request.POST:

openwisp_controller/config/tests/test_admin.py

@@ -1,3 +1,4 @@
+import csv
 import io
 import json
 import os
@@ -653,6 +654,68 @@ def test_change_device_group_action_perms(self):
             extra_payload={"device_group": group.pk, "apply": True},
         )
 
+    def test_device_export_includes_deactivated_config_status(self):
+        device = self._create_device_config()
+        device.deactivate()
+        response = self.client.post(
+            reverse(f"admin:{self.app_label}_device_export"), {"format": "0"}
+        )
+        self.assertNotContains(response, "error")
+        rows = list(csv.reader(response.content.decode().splitlines()))
+        header = rows[0]
+        status_col = header.index("config_status")
+        self.assertEqual(rows[1][status_col], "deactivated")
+
+    def test_device_import_deactivated_config_status_ignored(self):
+        # config_status is readonly=True in DeviceResource, so importing a
+        # row with config_status=deactivated must NOT create a deactivated device.
+        org = self._get_org(org_name="default")
+        contents = (
+            "name,mac_address,organization,group,model,os,system,notes,last_ip,"
+            "management_ip,config_status,config_backend,config_data,config_context,"
+            "config_templates,created,modified,id,key,organization_id,group_id\n"
+            "test-deactivated,00:11:22:33:44:66,{org_name},,,,,,,,"
+            "deactivated,netjsonconfig.OpenWrt,,,,"
+            "2022-10-17 15:26:51,2022-10-17 15:26:51,"
+            "559871c5-ce3d-4c7e-9176-fb6623d562f3,934d0799b1ce3a454bbb585cda1d7a49,"
+            "{org_id},"
+        ).strip()
+        contents = contents.format(org_name=org.name, org_id=org.id)
+        csv_file = ContentFile(contents)
+        response = self.client.post(
+            reverse(f"admin:{self.app_label}_device_import"),
+            {"format": "0", "import_file": csv_file, "file_name": "test.csv"},
+        )
+        self.assertFalse(response.context["result"].has_errors())
+        confirm_form = response.context["confirm_form"]
+        data = confirm_form.initial
+        response = self.client.post(
+            reverse(f"admin:{self.app_label}_device_process_import"), data, follow=True
+        )
+        self.assertEqual(response.status_code, 200)
+        device = Device.objects.get(name="test-deactivated")
+        self.assertFalse(device._is_deactivated)
+        self.assertTrue(device._has_config())
+        self.assertNotEqual(device.config.status, "deactivated")
+
+    def test_change_group_action_skips_deactivated_device(self):
+        path = reverse(f"admin:{self.app_label}_device_changelist")
+        org = self._get_org(org_name="default")
+        group = self._create_device_group(name="test-group", organization=org)
+        device = self._create_device(organization=org)
+        device.deactivate()
+        post_data = {
+            "_selected_action": [device.pk],
+            "action": "change_group",
+            "csrfmiddlewaretoken": "test",
+            "apply": True,
+            "device_group": group.pk,
+        }
+        response = self.client.post(path, post_data, follow=True)
+        self.assertEqual(response.status_code, 200)
+        device.refresh_from_db()
+        self.assertIsNone(device.group)
+
     def test_device_import_with_group_apply_templates(self):
         org = self._get_org(org_name="default")
         template = self._create_template(name="template")
@@ -3038,6 +3101,26 @@ def test_change_device_group_action_changes_templates(self):
                 self.assertEqual(response.status_code, 200)
                 self.assertEqual(len(mocked.call_args_list), 2)
 
+        with self.subTest("Change group skips deactivated devices in signals"):
+            device3 = self._create_device(
+                organization=org1,
+                group=dg1,
+                name="default.test.device3",
+                mac_address="AA:BB:CC:DD:EE:01",
+            )
+            device3.deactivate()
+            data = post_data.copy()
+            data["_selected_action"] = [device1.pk, device3.pk]
+            data["device_group"] = str(dg2.pk)
+            with patch.object(Device, "_send_device_group_changed_signal") as mocked:
+                response = self.client.post(path, data, follow=True)
+                self.assertEqual(response.status_code, 200)
+                self.assertEqual(len(mocked.call_args_list), 1)
+            device1.refresh_from_db()
+            device3.refresh_from_db()
+            self.assertEqual(device1.group_id, dg2.id)
+            self.assertEqual(device3.group_id, dg1.id)
+
         device2.organization = org2
         device2.save()
 

openwisp_controller/config/tests/test_config.py

@@ -564,6 +564,24 @@ def test_auto_cert_not_deleted_on_device_deactivation(self):
         self.assertEqual(config.templates.count(), 0)
         self.assertEqual(cert.revoked, True)
 
+    def test_certificate_updated_skipped_for_deactivated_config(self):
+        self._create_template(type="vpn", vpn=self._create_vpn(), default=True)
+        config = self._create_config(organization=self._get_org())
+        cert = config.vpnclient_set.first().cert
+        config.deactivate()
+        config.refresh_from_db()
+        self.assertEqual(config.status, "deactivating")
+        # VpnClient is deleted on deactivation; cert is auto-revoked.
+        self.assertEqual(config.vpnclient_set.count(), 0)
+        # Un-revoke the cert so certificate_updated() bypasses the early
+        # "if revoked: return" guard and hits the ObjectDoesNotExist path.
+        cert.revoked = False
+        cert.save()
+        # Config status must not change: certificate_updated() returns early
+        # because the VpnClient was deleted during deactivation.
+        config.refresh_from_db()
+        self.assertEqual(config.status, "deactivating")
+
     def _get_vpn_context(self):
         self.test_create_cert()
         c = Config.objects.get(device__name="test-create-cert")

openwisp_controller/config/tests/test_controller.py

@@ -786,6 +786,22 @@ def test_device_consistent_registration_exists_no_config(self):
         d.refresh_from_db()
         self.assertIsNotNone(d.config)
 
+    @capture_any_output()
+    def test_register_deactivated_device(self):
+        device = self._create_device_config()
+        device.key = TEST_CONSISTENT_KEY
+        device.save()
+        device.deactivate()
+        original_os = device.os
+        params = self._get_reregistration_payload(
+            device, name=device.name, os="OpenWrt 22.03"
+        )
+        response = self.client.post(self.register_url, params)
+        self.assertContains(response, "error: device deactivated", status_code=403)
+        device.refresh_from_db()
+        self.assertEqual(device.os, original_os)
+        self.assertEqual(device.config.templates.count(), 0)
+
     def test_device_registration_update_hw_info(self):
         d = self._create_device_config()
         d.key = TEST_CONSISTENT_KEY
@@ -1075,9 +1091,37 @@ def test_device_update_info(self):
             self.assertEqual(d.model, params["model"])
 
     def test_deactivated_device_update_info(self):
-        self._test_deactivating_deactivated_device_view(
-            "device_update_info", method="post", data={}
-        )
+        # Inventory metadata updates must be rejected for deactivated devices,
+        # including the transient "deactivating" state (which GetDeviceView does
+        # not exclude on its own).
+        self._create_template(required=True)
+        device = self._create_device_config()
+        url = reverse("controller:device_update_info", args=[device.pk])
+        params = {
+            "key": device.key,
+            "model": "TP-Link TL-WDR4300 v2",
+            "os": "OpenWrt 18.06-SNAPSHOT r7312-e60be11330",
+            "system": "Atheros AR9344 rev 3",
+        }
+        device.deactivate()
+        device.refresh_from_db()
+        self.assertEqual(device.config.status, "deactivating")
+        initial = (device.os, device.model, device.system)
+        # payload must differ from current values so a missed block is detectable
+        self.assertNotEqual((params["os"], params["model"], params["system"]), initial)
+
+        with self.subTest("rejected while deactivating"):
+            response = self.client.post(url, params)
+            self.assertEqual(response.status_code, 403)
+            self._check_header(response)
+            device.refresh_from_db()
+            # metadata must be left untouched
+            self.assertEqual((device.os, device.model, device.system), initial)
+
+        with self.subTest("not found once fully deactivated"):
+            device.config.set_status_deactivated()
+            response = self.client.post(url, params)
+            self.assertEqual(response.status_code, 404)
 
     def test_device_update_info_bad_uuid(self):
         d = self._create_device_config()

openwisp_controller/config/tests/test_device.py

@@ -512,6 +512,27 @@ def test_device_group_changed_not_emitted_on_creation(self):
             self._create_device(name="test", organization=org, group=device_group)
         handler.assert_not_called()
 
+    def test_manage_devices_group_templates_skips_deactivated_devices(self):
+        org = self._get_org()
+        old_template = self._create_template(name="old-template", organization=org)
+        new_template = self._create_template(name="new-template", organization=org)
+        old_group = self._create_device_group(name="old-group", organization=org)
+        new_group = self._create_device_group(name="new-group", organization=org)
+        old_group.templates.add(old_template)
+        new_group.templates.add(new_template)
+        device = self._create_device(name="test", organization=org, group=old_group)
+        device.deactivate()
+        device.config.refresh_from_db()
+        self.assertEqual(device.config.templates.count(), 0)
+        Device.manage_devices_group_templates(device.pk, old_group.pk, new_group.pk)
+        device.config.refresh_from_db()
+        # Deactivated device config is skipped: adding templates to a cleared
+        # config would misrepresent what has been applied to the device.
+        self.assertEqual(device.config.templates.count(), 0)
+        self.assertNotIn(new_template, device.config.templates.all())
+        # Status must remain "deactivated" — no config push is initiated.
+        self.assertEqual(device.config.status, "deactivated")
+
     def test_device_field_changed_checks(self):
         self._create_device()
         device_group = self._create_device_group()