[firmware-upgrader] Prevent upgrade operations on deactivated devices #382

Added comprehensive validation to prevent firmware upgrade operations
on deactivated devices across all application layers:

- Added model validation in AbstractDeviceFirmware.clean() and
  AbstractUpgradeOperation.clean() to reject operations on deactivated devices
- Enhanced API validation in DeviceFirmwareSerializer to validate device state
- Updated batch operation filtering in _find_related_device_firmwares() and
  _find_firmwareless_devices() to exclude deactivated devices
- Added comprehensive tests for model, API, and admin validation

This ensures defense-in-depth protection where deactivated devices cannot
be targeted for firmware upgrades through any interface (UI, API, or batch
operations), preventing potential security issues.

Fixes #382

Author: SAMurai-16

.gitignore

@@ -8,6 +8,7 @@ __pycache__/
 # Distribution / packaging
 .Python
 env/
+.venv/
 build/
 develop-eggs/
 dist/

openwisp_firmware_upgrader/api/serializers.py

@@ -150,8 +150,12 @@ def validate(self, data):
             device_id = self.context.get("device_id")
             device = self._get_device_object(device_id)
             data.update({"device": device})
-        image = data.get("image")
         device = data.get("device")
+        if device and device.is_deactivated():
+            raise ValidationError(
+                _("Cannot create firmware object for deactivated device")
+            )
+        image = data.get("image")
         if (
             image
             and device

openwisp_firmware_upgrader/base/models.py

@@ -206,6 +206,7 @@ def _find_related_device_firmwares(
             .select_related(*related)
             .filter(image__build__category_id=self.category_id)
             .exclude(image__build=self, installed=True)
+            .exclude(device___is_deactivated=True)
             .order_by("-created")
         )
         if group:
@@ -227,7 +228,7 @@ def _find_firmwareless_devices(self, boards=None, group=None, location=None):
         qs = Device.objects.filter(
             devicefirmware__isnull=True,
             model__in=boards,
-        )
+        ).exclude(_is_deactivated=True)
         if self.category.organization_id:
             qs = qs.filter(organization_id=self.category.organization_id)
         if group:
@@ -391,6 +392,10 @@ class Meta:
     def clean(self):
         if not hasattr(self, "image") or not hasattr(self, "device"):
             return
+        if self.device.is_deactivated():
+            raise ValidationError(
+                _("Cannot create firmware object for deactivated device")
+            )
         if (
             self.image.build.category.organization is not None
             and self.image.build.category.organization != self.device.organization
@@ -827,6 +832,13 @@ class AbstractUpgradeOperation(UpgradeOptionsMixin, TimeStampedEditableModel):
     class Meta:
         abstract = True
 
+    def clean(self):
+        super().clean()
+        if hasattr(self, "device") and self.device and self.device.is_deactivated():
+            raise ValidationError(
+                _("Cannot create upgrade operation for deactivated device")
+            )
+
     def log_line(self, line, save=True):
         if self.log:
             self.log += f"\n{line}"

openwisp_firmware_upgrader/tests/test_admin.py

@@ -7,6 +7,7 @@
 from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Permission
+from django.core.exceptions import ValidationError
 from django.test import RequestFactory, TestCase, TransactionTestCase
 from django.urls import reverse
 from django.utils.timezone import localtime
@@ -392,6 +393,9 @@ def test_device_firmware_upgrade_without_device_connection(
     def test_deactivated_firmware_image_inline(self):
         self._login()
         device = self._create_config(organization=self._get_org()).device
+        # Create device firmware BEFORE deactivating device
+        self._create_device_firmware(device=device)
+        # Now deactivate the device
         device.deactivate()
         response = self.client.get(
             reverse("admin:config_device_change", args=[device.id])
@@ -403,10 +407,7 @@ def test_deactivated_firmware_image_inline(self):
             '<input type="hidden" name="devicefirmware-MAX_NUM_FORMS"'
             ' value="0" id="id_devicefirmware-MAX_NUM_FORMS">',
         )
-        self._create_device_firmware(device=device)
-        response = self.client.get(
-            reverse("admin:config_device_change", args=[device.id])
-        )
+
         # Ensure that a deactivated device's existing DeviceFirmwareImage
         # is displayed as readonly in the admin interface.
         self.assertContains(
@@ -1464,5 +1465,32 @@ def test_batch_upgrade_operation_list_location_filter(self, *args):
             self.assertNotContains(response, str(batch2.pk))
             self.assertNotContains(response, str(batch3.pk))
 
+    def test_device_firmware_inline_deactivated_device(self, *args):
+        """Test that DeviceFirmware inline shows validation error for deactivated device"""
+        device_fw = self._create_device_firmware()
+        device = device_fw.device
+        device.deactivate()
+
+        # Test that trying to create a new DeviceFirmware for deactivated device fails
+        with self.assertRaises(ValidationError) as cm:
+            new_device_fw = DeviceFirmware(device=device, image=device_fw.image)
+            new_device_fw.full_clean()
+
+        self.assertIn(
+            "Cannot create firmware object for deactivated device", str(cm.exception)
+        )
+
+    @mock.patch("openwisp_firmware_upgrader.tasks.upgrade_firmware.delay")
+    def test_basic_deactivated_device_admin_validation(self, *args):
+        """Test basic admin validation for deactivated devices"""
+        device_fw = self._create_device_firmware()
+        device = device_fw.device
+
+        # Test that validation works at the model level
+        device.deactivate()
+        with self.assertRaises(ValidationError):
+            new_device_fw = DeviceFirmware(device=device, image=device_fw.image)
+            new_device_fw.full_clean()
+
 
 del TestConfigAdmin

openwisp_firmware_upgrader/tests/test_api.py

@@ -3,6 +3,7 @@
 
 import swapper
 from django.contrib.auth import get_user_model
+from django.core.exceptions import ValidationError
 from django.test import Client, TestCase
 from django.urls import reverse
 from packaging.version import parse as parse_version
@@ -2115,3 +2116,22 @@ def get_download_url(self):
             "upgrader:api_firmware_download",
             args=[self.image.build.pk, self.image.pk],
         )
+
+
+class TestDeactivatedDeviceAPI(TestAPIUpgraderMixin, TestCase):
+    """Test API behavior with deactivated devices"""
+
+    def test_device_firmware_basic_deactivated_validation(self):
+        """Test that DeviceFirmware creation is rejected for deactivated devices"""
+        device_fw = self._create_device_firmware()
+        device = device_fw.device
+        device.deactivate()
+
+        # Test that we cannot create new device firmware for deactivated device
+        with self.assertRaises(ValidationError) as cm:
+            new_device_fw = DeviceFirmware(device=device, image=device_fw.image)
+            new_device_fw.full_clean()
+
+        self.assertIn(
+            "Cannot create firmware object for deactivated device", str(cm.exception)
+        )

openwisp_firmware_upgrader/tests/test_models.py

@@ -992,3 +992,77 @@ def test_batch_upgrade_with_group_and_location_filtering(self, *_args):
         self.assertEqual(len(upgrade_ops), 1)
         batch.refresh_from_db()
         self.assertEqual(batch.status, "success")
+
+    def test_device_firmware_deactivated_device_validation(self):
+        """Test that DeviceFirmware clean method raises ValidationError for deactivated device"""
+        device_fw = self._create_device_firmware(upgrade=False)
+        device_fw.device.deactivate()
+
+        with self.assertRaises(ValidationError) as ctx:
+            device_fw.clean()
+
+        self.assertIn(
+            "Cannot create firmware object for deactivated device", str(ctx.exception)
+        )
+
+    def test_upgrade_operation_deactivated_device_validation(self):
+        """Test that UpgradeOperation clean method raises ValidationError for deactivated device"""
+        device_fw = self._create_device_firmware(upgrade=False)
+        device_fw.device.deactivate()
+
+        upgrade_op = UpgradeOperation(device=device_fw.device, image=device_fw.image)
+
+        with self.assertRaises(ValidationError) as ctx:
+            upgrade_op.clean()
+
+        self.assertIn(
+            "Cannot create upgrade operation for deactivated device", str(ctx.exception)
+        )
+
+    def test_device_firmware_save_deactivated_device(self):
+        """Test that DeviceFirmware save fails for deactivated device"""
+        device_fw = self._create_device_firmware(upgrade=False)
+        device_fw.device.deactivate()
+
+        # Create new DeviceFirmware instance for deactivated device
+        new_device_fw = DeviceFirmware(device=device_fw.device, image=device_fw.image)
+
+        with self.assertRaises(ValidationError):
+            new_device_fw.full_clean()
+
+    @mock.patch("openwisp_firmware_upgrader.tasks.upgrade_firmware.delay")
+    def test_batch_upgrade_excludes_deactivated_devices(self, *args):
+        """Test that batch upgrades exclude deactivated devices"""
+        env = self._create_upgrade_env()
+        # Deactivate one device
+        env["d1"].deactivate()
+
+        batch = env["build2"].batch_upgrade(firmwareless=False)
+        ops = UpgradeOperation.objects.filter(batch=batch)
+
+        # Should only have operations for non-deactivated devices
+        device_ids = [op.device.pk for op in ops]
+        self.assertNotIn(env["d1"].pk, device_ids)  # deactivated device excluded
+        self.assertIn(env["d2"].pk, device_ids)  # active device included
+
+    def test_deactivated_device_validation(self):
+        """Test that model validation prevents operations on deactivated devices"""
+        device_fw = self._create_device_firmware()
+        device = device_fw.device
+
+        # Test DeviceFirmware validation
+        device.deactivate()
+        with self.assertRaises(ValidationError) as cm:
+            new_device_fw = DeviceFirmware(device=device, image=device_fw.image)
+            new_device_fw.full_clean()
+        self.assertIn(
+            "Cannot create firmware object for deactivated device", str(cm.exception)
+        )
+
+        # Test UpgradeOperation validation
+        with self.assertRaises(ValidationError) as cm:
+            operation = UpgradeOperation(device=device, image=device_fw.image)
+            operation.full_clean()
+        self.assertIn(
+            "Cannot create upgrade operation for deactivated device", str(cm.exception)
+        )