Skip to content

Commit

Permalink
[change] Added support for device deactivation feature #251
Browse files Browse the repository at this point in the history
- Do not allow changing DeviceFirmwareImage of a deactivated device
- Disable API operations on deactivated devices

Closes #251

---------

Co-authored-by: Federico Capoano <[email protected]>
  • Loading branch information
pandafy and nemesifier authored Nov 19, 2024
1 parent c065323 commit 1aa3e54
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
pip install -U pip wheel setuptools
pip install -U -r requirements-test.txt
pip install -U -e .
pip install ${{ matrix.django-version }}
pip install -U ${{ matrix.django-version }}
sudo npm install -g jshint stylelint
- name: QA checks
Expand Down
6 changes: 4 additions & 2 deletions openwisp_firmware_upgrader/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from django.utils.translation import gettext_lazy as _
from reversion.admin import VersionAdmin

from openwisp_controller.config.admin import DeviceAdmin
from openwisp_controller.config.admin import DeactivatedDeviceReadOnlyMixin, DeviceAdmin
from openwisp_users.multitenancy import MultitenantAdminMixin, MultitenantOrgFilter
from openwisp_utils.admin import ReadOnlyAdmin, TimeReadonlyAdminMixin

Expand Down Expand Up @@ -402,7 +402,9 @@ def get_form_kwargs(self, index):
return kwargs


class DeviceFirmwareInline(MultitenantAdminMixin, admin.StackedInline):
class DeviceFirmwareInline(
MultitenantAdminMixin, DeactivatedDeviceReadOnlyMixin, admin.StackedInline
):
model = DeviceFirmware
formset = DeviceFormSet
form = DeviceFirmwareForm
Expand Down
8 changes: 7 additions & 1 deletion openwisp_firmware_upgrader/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django.http import Http404
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework import filters, generics, pagination, serializers, status
from rest_framework.exceptions import NotFound
from rest_framework.exceptions import NotFound, PermissionDenied
from rest_framework.request import clone_request
from rest_framework.response import Response
from rest_framework.utils.serializer_helpers import ReturnDict
Expand Down Expand Up @@ -257,6 +257,12 @@ class DeviceFirmwareDetailView(
lookup_url_kwarg = 'pk'
organization_field = 'device__organization'

def get_object(self):
obj = super().get_object()
if self.request.method not in ('GET', 'HEAD') and obj.device.is_deactivated():
raise PermissionDenied
return obj

def get_serializer_context(self):
context = super().get_serializer_context()
context.update({'device_id': self.kwargs['pk']})
Expand Down
30 changes: 30 additions & 0 deletions openwisp_firmware_upgrader/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,36 @@ def test_device_firmware_upgrade_without_device_connection(
mocked_func.assert_not_called()
self.assertEqual(response.status_code, 200)

def test_deactivated_firmware_image_inline(self):
self._login()
device = self._create_config(organization=self._get_org()).device
device.deactivate()
response = self.client.get(
reverse('admin:config_device_change', args=[device.id])
)
# Check that it is not possible to add a DeviceFirmwareImage to a
# deactivated device in the admin interface.
self.assertContains(
response,
'<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(
response,
'<div class="readonly">Test Category v0.1:'
' TP-Link WDR4300 v1 (OpenWrt 19.07 and later)</div>',
)
self.assertNotContains(
response,
'<select name="devicefirmware-0-image" id="id_devicefirmware-0-image">',
)


_mock_updrade = 'openwisp_firmware_upgrader.upgraders.openwrt.OpenWrt.upgrade'
_mock_connect = 'openwisp_controller.connection.models.DeviceConnection.connect'
Expand Down
21 changes: 21 additions & 0 deletions openwisp_firmware_upgrader/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,27 @@ def test_device_firmware_detail_400(self):
self.assertEqual(r.status_code, 400)
self.assertIn('Invalid pk', r.json()['image'][0])

def test_deactivated_device(self):
device_fw = self._create_device_firmware()
device_fw.device.deactivate()
url = reverse('upgrader:api_devicefirmware_detail', args=[device_fw.device.pk])

with self.subTest('Test retrieving DeviceFirmwareImage'):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)

with self.subTest('Test updating DeviceFirmwareImage'):
response = self.client.put(
url,
data={'image': device_fw.image.pk},
content_type='application/json',
)
self.assertEqual(response.status_code, 403)

with self.subTest('Test deleting DeviceFirmwareImage'):
response = self.client.delete(url)
self.assertEqual(response.status_code, 403)

def test_device_firmware_detail_delete(self):
device_fw = self._create_device_firmware()
self.assertEqual(DeviceFirmware.objects.count(), 1)
Expand Down
4 changes: 2 additions & 2 deletions openwisp_firmware_upgrader/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,15 +432,15 @@ def test_device_fw_not_created_on_device_connection_save(self):
self.assertEqual(DeviceConnection.objects.count(), 1)
self.assertEqual(Device.objects.count(), 1)
self.assertEqual(DeviceFirmware.objects.count(), 0)
d1.delete()
d1.delete(check_deactivated=False)
Credentials.objects.all().delete()

with self.subTest("Device doesn't define model"):
d1 = self._create_device_with_connection(os=self.os, model='')
self.assertEqual(DeviceConnection.objects.count(), 1)
self.assertEqual(Device.objects.count(), 1)
self.assertEqual(DeviceFirmware.objects.count(), 0)
d1.delete()
d1.delete(check_deactivated=False)
Credentials.objects.all().delete()

build1.os = None
Expand Down
4 changes: 3 additions & 1 deletion openwisp_firmware_upgrader/tests/test_selenium.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,16 @@ def test_restoring_deleted_device(self):
device = self._create_device(
os=self.os, model=image.boards[0], organization=org
)
self._create_config(device=device)
config = self._create_config(device=device)
self.assertEqual(Device.objects.count(), 1)
self.assertEqual(DeviceConnection.objects.count(), 1)
self.assertEqual(DeviceFirmware.objects.count(), 1)

call_command('createinitialrevisions')

self.login()
device.deactivate()
config.set_status_deactivated()
# Delete the device
self.open(
reverse(f'admin:{self.config_app_label}_device_delete', args=[device.id])
Expand Down

0 comments on commit 1aa3e54

Please sign in to comment.