diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index f51194af5..b6aad80ae 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -73,6 +73,20 @@ class BaseAdmin(TimeReadonlyAdminMixin, ModelAdmin): history_latest_first = True +class DeactivatedDeviceReadOnlyMixin(object): + def has_add_permission(self, request, obj): + perm = super().has_add_permission(request, obj) + if not obj: + return perm + return perm and not obj.is_deactivated() + + def has_change_permission(self, request, obj=None): + perm = super().has_change_permission(request) + if not obj: + return perm + return perm and not obj.is_deactivated() + + class BaseConfigAdmin(BaseAdmin): change_form_template = 'admin/config/change_form.html' preview_template = None @@ -390,6 +404,7 @@ class Meta(BaseForm.Meta): class ConfigInline( + DeactivatedDeviceReadOnlyMixin, MultitenantAdminMixin, TimeReadonlyAdminMixin, SystemDefinedVariableMixin, @@ -425,10 +440,6 @@ def _error_reason_field_conditional(self, obj, fields): fields.insert(fields.index('status') + 1, 'error_reason') return fields - def get_readonly_fields(self, request, obj): - fields = super().get_readonly_fields(request, obj) - return self._error_reason_field_conditional(obj, fields) - def get_fields(self, request, obj): fields = super().get_fields(request, obj) return self._error_reason_field_conditional(obj, fields) @@ -499,7 +510,7 @@ class DeviceAdmin(MultitenantAdminMixin, BaseConfigAdmin, UUIDAdmin): ] inlines = [ConfigInline] conditional_inlines = [] - actions = ['change_group'] + actions = ['deactivate_device', 'change_group', 'activate_device'] org_position = 1 if not app_settings.HARDWARE_ID_ENABLED else 2 list_display.insert(org_position, 'organization') _state_adding = False @@ -520,6 +531,12 @@ class Media(BaseConfigAdmin.Media): f'{prefix}js/relevant_templates.js', ] + def has_change_permission(self, request, obj=None): + perm = super().has_change_permission(request) + if not obj: + return perm + return perm and not obj.is_deactivated() + def save_form(self, request, form, change): self._state_adding = form.instance._state.adding return super().save_form(request, form, change) @@ -624,6 +641,34 @@ def change_group(self, request, queryset): request, 'admin/config/change_device_group.html', context ) + def _change_device_status(self, request, queryset, method): + """ + This helper method provides re-usability of code for + device activation and deactivation actions. + """ + # Validate selected devices can be managed by the user + if not request.user.is_superuser: + # There could be multiple devices selected by the user. + # Validate that all the devices can be managed by the user. + for org_id in set(queryset.values_list('organization_id', flat=True)): + if not request.user.is_manager(str(org_id)): + logger.warning( + f'{request.user} attempted to deactivate device of "{org_id}"' + ' organization which they do not manage.' + ' The operation was rejected.' + ) + return HttpResponseForbidden() + for device in queryset.iterator(): + getattr(device, method)() + + @admin.actions(description=_('Deactivate selected devices'), permissions=['change']) + def deactivate_device(self, request, queryset): + self._change_device_status(request, queryset, 'deactivate') + + @admin.actions(description=_('Activate selected devices'), permissions=['change']) + def activate_device(self, request, queryset): + self._change_device_status(request, queryset, 'activate') + def get_fields(self, request, obj=None): """ Do not show readonly fields in add form diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index 181cc566a..132f35e7f 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -62,7 +62,7 @@ class AbstractConfig(BaseConfig): blank=True, ) - STATUS = Choices('modified', 'applied', 'error') + STATUS = Choices('modified', 'applied', 'error', 'deactivating', 'deactivated') status = StatusField( _('configuration status'), help_text=_( @@ -331,6 +331,8 @@ def enforce_required_templates( """ if action not in ['pre_remove', 'post_clear']: return False + if instance.is_deactivating_or_deactivated(): + return raw_data = raw_data or {} template_query = models.Q(required=True, backend=instance.backend) # trying to remove a required template will raise PermissionDenied @@ -483,6 +485,15 @@ def save(self, *args, **kwargs): self._initial_status = self.status return result + def is_deactivating_or_deactivated(self): + return self.status in ['deactivating', 'deactivated'] + + def is_deactivating(self): + return self.status == 'deactivating' + + def is_deactivated(self): + return self.status == 'deactivated' + def _check_changes(self): current = self._meta.model.objects.only( 'backend', 'config', 'context', 'status' @@ -539,9 +550,10 @@ def _send_config_status_changed_signal(self): """ config_status_changed.send(sender=self.__class__, instance=self) - def _set_status(self, status, save=True, reason=None): + def _set_status(self, status, save=True, reason=None, extra_update_fields=None): self._send_config_status_changed = True - update_fields = ['status'] + extra_update_fields = extra_update_fields or [] + update_fields = ['status'] + extra_update_fields # The error reason should be updated when # 1. the configuration is in "error" status # 2. the configuration has changed from error status @@ -563,6 +575,18 @@ def set_status_applied(self, save=True): def set_status_error(self, save=True, reason=None): self._set_status('error', save, reason) + def set_status_deactivating(self, save=True): + """ + Set Config status as deactivating and + clears configuration and templates. + """ + self.config = {} + self._set_status('deactivating', save, extra_update_fields=['config']) + self.templates.clear() + + def set_status_deactivated(self, save=True): + self._set_status('deactivated', save) + def _has_device(self): return hasattr(self, 'device') diff --git a/openwisp_controller/config/base/device.py b/openwisp_controller/config/base/device.py index 95cf065f9..8042767d9 100644 --- a/openwisp_controller/config/base/device.py +++ b/openwisp_controller/config/base/device.py @@ -1,7 +1,7 @@ from hashlib import md5 from django.core.exceptions import ObjectDoesNotExist, ValidationError -from django.db import models +from django.db import models, transaction from django.db.models import Q from django.utils.translation import gettext_lazy as _ from swapper import get_model_name, load_model @@ -96,6 +96,10 @@ class AbstractDevice(OrgMixin, BaseModel): ), ) hardware_id = models.CharField(**(app_settings.HARDWARE_ID_OPTIONS)) + # This is an internal field which is used to track if + # the device has been deactivated. This field should not be changed + # directly, use the deactivate() method instead. + _is_deactivated = models.BooleanField(default=False) class Meta: unique_together = ( @@ -163,6 +167,33 @@ def _get_organization__config_settings(self): organization=self.organization if hasattr(self, 'organization') else None ) + def is_deactivated(self): + return self._is_deactivated + + def deactivate(self): + if self.is_deactivated(): + # The device has already been deactivated. + # No further operation is required. + return + with transaction.atomic(): + if self._has_config(): + self.config.set_status_deactivating() + self._is_deactivated = True + self.save() + + def activate(self): + if not self.is_deactivated(): + # The device is already active. + # No further operation is required. + return + with transaction.atomic(): + if self._has_config(): + self.config.set_status_modified() + # Trigger enforcing of required templates + self.config.templates.clear() + self._is_deactivated = False + self.save() + def get_context(self): config = self._get_config() return config.get_context() diff --git a/openwisp_controller/config/migrations/0054_device__is_deactivated.py b/openwisp_controller/config/migrations/0054_device__is_deactivated.py new file mode 100644 index 000000000..2becdbfd9 --- /dev/null +++ b/openwisp_controller/config/migrations/0054_device__is_deactivated.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.20 on 2024-02-29 11:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('config', '0053_vpnclient_secret'), + ] + + operations = [ + migrations.AddField( + model_name='device', + name='_is_deactivated', + field=models.BooleanField(default=False), + ), + ] diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index 17e38a1f4..675ef331d 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -2,7 +2,7 @@ from unittest import mock from django.core.exceptions import ValidationError -from django.test import TestCase +from django.test import TestCase, TransactionTestCase from swapper import load_model from openwisp_utils.tests import AssertNumQueriesSubTestMixin, catch_signal @@ -589,3 +589,46 @@ def test_create_default_config_existing(self): device.config.refresh_from_db() self.assertEqual(device.config.context, {'ssid': 'test'}) self.assertEqual(device.config.config, {'general': {}}) + + +class TestTransactionDevice( + CreateConfigTemplateMixin, + TestOrganizationMixin, + AssertNumQueriesSubTestMixin, + CreateDeviceGroupMixin, + TransactionTestCase, +): + def test_deactivating_device_with_config(self): + self._create_template(required=True) + config = self._create_config(organization=self._get_org()) + device = config.device + self.assertEqual(config.templates.count(), 1) + + device.deactivate() + device.refresh_from_db() + config.refresh_from_db() + self.assertEqual(device.is_deactivated(), True) + self.assertEqual(config.status, 'deactivating') + self.assertEqual(config.config, {}) + self.assertEqual(config.templates.count(), 0) + + device.activate() + device.refresh_from_db() + config.refresh_from_db() + self.assertEqual(device.is_deactivated(), False) + self.assertEqual(config.status, 'modified') + # Required templates are automatically added + self.assertEqual(config.templates.count(), 1) + + def test_deactivating_device_without_config(self): + device = self._create_device() + self.assertEqual(device._has_config(), False) + device.deactivate() + device.refresh_from_db() + self.assertEqual(device._has_config(), False) + self.assertEqual(device.is_deactivated(), True) + + device.activate() + device.refresh_from_db() + self.assertEqual(device._has_config(), False) + self.assertEqual(device.is_deactivated(), False) diff --git a/openwisp_controller/connection/admin.py b/openwisp_controller/connection/admin.py index 6e02cd895..f2114657f 100644 --- a/openwisp_controller/connection/admin.py +++ b/openwisp_controller/connection/admin.py @@ -14,7 +14,7 @@ from openwisp_utils.admin import TimeReadonlyAdminMixin from ..admin import MultitenantAdminMixin -from ..config.admin import DeviceAdmin +from ..config.admin import DeactivatedDeviceReadOnlyMixin, DeviceAdmin from .schema import schema from .widgets import CommandSchemaWidget, CredentialsSchemaWidget @@ -73,7 +73,9 @@ def schema_view(self, request): return JsonResponse(schema) -class DeviceConnectionInline(MultitenantAdminMixin, admin.StackedInline): +class DeviceConnectionInline( + MultitenantAdminMixin, DeactivatedDeviceReadOnlyMixin, admin.StackedInline +): model = DeviceConnection verbose_name = _('Credentials') verbose_name_plural = verbose_name diff --git a/openwisp_controller/geo/admin.py b/openwisp_controller/geo/admin.py index 2ca4b8c9d..da58e2e55 100644 --- a/openwisp_controller/geo/admin.py +++ b/openwisp_controller/geo/admin.py @@ -15,7 +15,11 @@ from openwisp_users.multitenancy import MultitenantOrgFilter from ..admin import MultitenantAdminMixin -from ..config.admin import DeviceAdminExportable +from ..config.admin import ( + DeactivatedDeviceReadOnlyMixin, + DeviceAdmin, + DeviceAdminExportable, +) from .exportable import GeoDeviceResource DeviceLocation = load_model('geo', 'DeviceLocation') @@ -72,7 +76,9 @@ class LocationAdmin(MultitenantAdminMixin, AbstractLocationAdmin): LocationAdmin.list_filter.insert(0, MultitenantOrgFilter) -class DeviceLocationInline(ObjectLocationMixin, admin.StackedInline): +class DeviceLocationInline( + ObjectLocationMixin, DeactivatedDeviceReadOnlyMixin, admin.StackedInline +): model = DeviceLocation form = ObjectLocationForm verbose_name = _('Map')