From ad399ccd22379874260da28dc4c5eb7cbac920a6 Mon Sep 17 00:00:00 2001 From: Srinath0916 Date: Fri, 19 Dec 2025 23:37:52 +0530 Subject: [PATCH 01/17] [fix] Fixed MultiValueDictKeyError on empty device form submission #1057 Fixes #1057 --------- Co-authored-by: Gagan Deep Co-authored-by: Federico Capoano (cherry picked from commit 828dfb30dcc1378e7dc592a662f4d0aeeeb1ce04) --- openwisp_controller/config/admin.py | 15 ++++++++-- .../config/tests/test_admin.py | 30 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index b78d048f6..9c758fd06 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -352,10 +352,13 @@ def get_temp_model_instance(self, **options): config_model = self.Meta.model instance = config_model(**options) device_model = config_model.device.field.related_model - org = Organization.objects.get(pk=self.data["organization"]) + if not (org_id := self.data.get("organization")): + # We cannot validate the templates without an organization. + return + org = Organization.objects.get(pk=org_id) instance.device = device_model( - name=self.data["name"], - mac_address=self.data["mac_address"], + name=self.data.get("name", ""), + mac_address=self.data.get("mac_address", ""), organization=org, ) return instance @@ -369,6 +372,12 @@ def clean_templates(self): # when adding self.instance is empty, we need to create a # temporary instance that we'll use just for validation config = self.get_temp_model_instance(**data) + if not config: + # The request does not contain vaild data to create a temporary + # Device instance. Thus, we cannot validate the templates. + # The Device validation will be handled by DeviceAdmin. + # Therefore, we don't need to raise any error here. + return else: config = self.instance if config.backend and templates: diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index c33b402c4..3be6f448a 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -2279,6 +2279,36 @@ def test_templates_fetch_queries_10(self): config = self._create_config(organization=self._get_org()) self._verify_template_queries(config, 10) + def test_empty_device_form_with_config_inline(self): + org = self._get_org() + template = self._create_template(organization=org) + path = reverse(f"admin:{self.app_label}_device_add") + # Submit form without required device fields but with config inline + # This reproduces the scenario where user clicks "Add another Configuration" + # and submits without filling device details + params = { + "config-0-backend": "netjsonconfig.OpenWrt", + "config-0-templates": str(template.pk), + "config-0-config": json.dumps({}), + "config-0-context": "", + "config-TOTAL_FORMS": 1, + "config-INITIAL_FORMS": 0, + "config-MIN_NUM_FORMS": 0, + "config-MAX_NUM_FORMS": 1, + "deviceconnection_set-TOTAL_FORMS": 0, + "deviceconnection_set-INITIAL_FORMS": 0, + "deviceconnection_set-MIN_NUM_FORMS": 0, + "deviceconnection_set-MAX_NUM_FORMS": 1000, + "command_set-TOTAL_FORMS": 0, + "command_set-INITIAL_FORMS": 0, + "command_set-MIN_NUM_FORMS": 0, + "command_set-MAX_NUM_FORMS": 1000, + } + response = self.client.post(path, params) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "errorlist") + self.assertEqual(Device.objects.count(), 0) + class TestTransactionAdmin( CreateConfigTemplateMixin, From ce13840bdc9dbfafd6962785d9688bc6c32027b9 Mon Sep 17 00:00:00 2001 From: Sarthak Tyagi <142912014+stktyagi@users.noreply.github.com> Date: Fri, 16 Jan 2026 03:22:45 +0530 Subject: [PATCH 02/17] [fix] Fixed 500 FieldError in DeviceLocationView #1110 Fixes #1110 [backport 1.2] (cherry picked from commit 6697a3a8820c2bd10960edfb9ceec52fea6a8488) --- openwisp_controller/geo/api/views.py | 1 + openwisp_controller/geo/tests/test_api.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index b5514cf26..eb923abe4 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -115,6 +115,7 @@ class DeviceLocationView( lookup_field = "content_object" lookup_url_kwarg = "pk" organization_field = "content_object__organization" + organization_lookup = "organization__in" _device_field = "content_object" def get_queryset(self): diff --git a/openwisp_controller/geo/tests/test_api.py b/openwisp_controller/geo/tests/test_api.py index 4cc37519f..8c103784b 100644 --- a/openwisp_controller/geo/tests/test_api.py +++ b/openwisp_controller/geo/tests/test_api.py @@ -9,6 +9,7 @@ from django.urls import reverse from django.urls.exceptions import NoReverseMatch from PIL import Image +from rest_framework import status from rest_framework.authtoken.models import Token from swapper import load_model @@ -1036,3 +1037,20 @@ def test_deactivated_device(self): with self.subTest("Test deleting DeviceLocation"): response = self.client.delete(url) self.assertEqual(response.status_code, 403) + + def test_device_location_view_parent_permission(self): + org1 = self._create_org(name="Org One") + device1 = self._create_device(organization=org1) + org2 = self._create_org(name="Org Two") + manager_org2 = self._create_administrator( + organizations=[org2], + username="manager_org2", + password="test_password", + is_superuser=False, + is_staff=True, + ) + self.client.force_login(manager_org2) + url = reverse("geo_api:device_location", args=[device1.pk]) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.client.logout() From 4a44a940471726ccc88c81ed92aba17cebae1dab Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 24 Feb 2026 22:04:14 +0530 Subject: [PATCH 03/17] [fix] Use context variables in Vpn.auto_client for OpenVPN backend Bug: The `Vpn.auto_client` method was incorrectly using the Vpn.host field when generating the client configuration for the OpenVPN backend. This resulted in the `remote` directive being hardcoded in the `Template.config`, instead of being rendered from the provided context variables. Fix: The `Vpn.auto_client` method has been updated to use the correct context-based values. [backport 1.2] (cherry picked from commit dbf1a0573fc5064ed26d5f839d8353f14dc70e88) --- openwisp_controller/config/base/vpn.py | 2 +- openwisp_controller/config/tests/test_vpn.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/openwisp_controller/config/base/vpn.py b/openwisp_controller/config/base/vpn.py index 819cc1db7..5a1da28d5 100644 --- a/openwisp_controller/config/base/vpn.py +++ b/openwisp_controller/config/base/vpn.py @@ -648,7 +648,7 @@ def auto_client(self, auto_cert=True, template_backend_class=None): context_keys.pop("ip_address", None) context_keys.pop("vpn_subnet", None) auto = backend.auto_client( - host=self.host, + host=vpn_host, server=self.config[config_dict_key][0], **context_keys, ) diff --git a/openwisp_controller/config/tests/test_vpn.py b/openwisp_controller/config/tests/test_vpn.py index 4a0a5d300..69ac88afa 100644 --- a/openwisp_controller/config/tests/test_vpn.py +++ b/openwisp_controller/config/tests/test_vpn.py @@ -194,12 +194,13 @@ def test_auto_client(self): vpn = self._create_vpn() auto = vpn.auto_client() context_keys = vpn._get_auto_context_keys() - del context_keys["vpn_host"] del context_keys["vpn_port"] for key in context_keys.keys(): context_keys[key] = "{{%s}}" % context_keys[key] control = vpn.backend_class.auto_client( - host=vpn.host, server=self._vpn_config["openvpn"][0], **context_keys + host=context_keys.pop("vpn_host"), + server=self._vpn_config["openvpn"][0], + **context_keys, ) control["files"] = [ { @@ -224,14 +225,15 @@ def test_auto_client_auto_cert_False(self): vpn = self._create_vpn() auto = vpn.auto_client(auto_cert=False) context_keys = vpn._get_auto_context_keys() - del context_keys["vpn_host"] del context_keys["vpn_port"] for key in context_keys.keys(): context_keys[key] = "{{%s}}" % context_keys[key] for key in ["cert_path", "cert_contents", "key_path", "key_contents"]: del context_keys[key] control = vpn.backend_class.auto_client( - host=vpn.host, server=self._vpn_config["openvpn"][0], **context_keys + host=context_keys.pop("vpn_host"), + server=self._vpn_config["openvpn"][0], + **context_keys, ) control["files"] = [ { From 5aaf05ef3b46ab9d286f0055a4710ff232267457 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Tue, 24 Feb 2026 14:00:22 -0300 Subject: [PATCH 04/17] [chores:tests] Fixed mocking in test_update_vpn_server_configuration The first call to vpn.save() was generting a real HTTP request, which failed in the CI of ansible-openwisp2. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> (cherry picked from commit 582e6a714b14c7fdef32d84be9449095ed611ff0) # Conflicts: # openwisp_controller/config/tests/test_vpn.py --- openwisp_controller/config/tests/test_vpn.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/openwisp_controller/config/tests/test_vpn.py b/openwisp_controller/config/tests/test_vpn.py index 69ac88afa..091f32afd 100644 --- a/openwisp_controller/config/tests/test_vpn.py +++ b/openwisp_controller/config/tests/test_vpn.py @@ -830,23 +830,33 @@ def test_update_vpn_server_configuration(self): with self.subTest("Webhook endpoint and authentication endpoint is present"): vpn.webhook_endpoint = "https://example.com" vpn.auth_token = "super-secret-token" - vpn.save() - vpn_client.refresh_from_db() + success_response = mock.Mock(spec=requests.Response) + success_response.status_code = 200 + success_response.raise_for_status = mock.Mock() with mock.patch( "openwisp_controller.config.tasks.logger.info" ) as mocked_logger, mock.patch( "requests.post", return_value=HttpResponse() ): + vpn.save() + vpn_client.refresh_from_db() post_save.send( instance=vpn_client, sender=vpn_client._meta.model, created=False ) - mocked_logger.assert_called_once_with( + expected_call = mock.call( f"Triggered update webhook of VPN Server UUID: {vpn.pk}" ) + mocked_logger.assert_has_calls([expected_call, expected_call]) + self.assertEqual(mocked_logger.call_count, 2) - with mock.patch("logging.Logger.error") as mocked_logger, mock.patch( - "requests.post", return_value=HttpResponseNotFound() + fail_response = mock.Mock(spec=requests.Response) + fail_response.status_code = 404 + fail_response.raise_for_status.side_effect = requests.exceptions.HTTPError( + "Not Found" + ) + with mock.patch("logging.Logger.warning") as mocked_logger, mock.patch( + "requests.post", return_value=fail_response ): post_save.send( instance=vpn_client, sender=vpn_client._meta.model, created=False From bc2c0abf46b536613029c6001cbed44053f51b36 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Wed, 4 Mar 2026 17:47:44 -0300 Subject: [PATCH 05/17] 1.2.1 release --- CHANGES.rst | 12 ++++++++++++ openwisp_controller/__init__.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6d92ea169..7e3a6c238 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,18 @@ Changelog ========= +Version 1.2.1 [2026-03-04] +-------------------------- + +Bugfixes +~~~~~~~~ + +- Use context variables in Vpn.auto_client for OpenVPN backend +- Fixed 500 FieldError in DeviceLocationView `#1110 + `_ +- Fixed MultiValueDictKeyError on empty device form submission `#1057 + `_ + Version 1.2.0 [2025-10-24] -------------------------- diff --git a/openwisp_controller/__init__.py b/openwisp_controller/__init__.py index a8f01a0c1..cba3a1ce8 100644 --- a/openwisp_controller/__init__.py +++ b/openwisp_controller/__init__.py @@ -1,4 +1,4 @@ -VERSION = (1, 2, 0, "final") +VERSION = (1, 2, 1, "final") __version__ = VERSION # alias From a6b46df14191b35328a931e26a8ba6554b2e8aa4 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Fri, 6 Mar 2026 15:33:36 -0300 Subject: [PATCH 06/17] [ci:1.2] Updated CI build targets on 1.2 --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9cd16da5c..175cf78aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,12 +5,12 @@ on: push: branches: - master + - "1.2" pull_request: branches: - master - "1.1" - - gsoc25-map - - gsoc25-whois + - "1.2" jobs: build: From 33706ab77bf694537f0e4da0b0f396667afc201b Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Fri, 6 Mar 2026 15:15:20 -0300 Subject: [PATCH 07/17] [change:ux] Improved help text of configuration variable fields The previous help texts were confusing and too technical. [backport 1.2] (cherry picked from commit c540d38e4c5e7a6dd4d2a6ed26f662dcd9162a84) --- openwisp_controller/config/base/config.py | 8 ++--- .../config/base/device_group.py | 8 ++--- .../config/base/multitenancy.py | 3 +- openwisp_controller/config/base/template.py | 7 ++--- .../config/migrations/0023_update_context.py | 7 +++-- .../0028_template_default_values.py | 6 ++-- .../config/migrations/0036_device_group.py | 8 +++-- .../migrations/0049_devicegroup_context.py | 4 +-- ...0051_organizationconfigsettings_context.py | 4 +-- .../sample_config/migrations/0001_initial.py | 30 +++++++++++-------- 10 files changed, 46 insertions(+), 39 deletions(-) diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index 3333e4c59..139877344 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -94,10 +94,10 @@ class AbstractConfig(ChecksumCacheMixin, BaseConfig): blank=True, default=dict, help_text=_( - "Additional " - '' - "context (configuration variables) in JSON format" + "allows overriding " + '' + "configuration variables" ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, dump_kwargs={"indent": 4}, diff --git a/openwisp_controller/config/base/device_group.py b/openwisp_controller/config/base/device_group.py index 64fc77e5d..366d61667 100644 --- a/openwisp_controller/config/base/device_group.py +++ b/openwisp_controller/config/base/device_group.py @@ -42,8 +42,9 @@ class AbstractDeviceGroup(OrgMixin, TimeStampedEditableModel): load_kwargs={"object_pairs_hook": collections.OrderedDict}, dump_kwargs={"indent": 4}, help_text=_( - "Group meta data, use this field to store data which is related" - " to this group and can be retrieved via the REST API." + "Store custom metadata related to this group. This field is intended " + "for arbitrary data that does not affect device configuration and can " + "be retrieved via the REST API for integrations or external tools." ), verbose_name=_("Metadata"), ) @@ -53,8 +54,7 @@ class AbstractDeviceGroup(OrgMixin, TimeStampedEditableModel): load_kwargs={"object_pairs_hook": collections.OrderedDict}, dump_kwargs={"indent": 4}, help_text=_( - "This field can be used to add meta data for the group" - ' or to add "Configuration Variables" to the devices.' + "Define configuration variables available to all devices in this group" ), verbose_name=_("Configuration Variables"), ) diff --git a/openwisp_controller/config/base/multitenancy.py b/openwisp_controller/config/base/multitenancy.py index 9cdf15736..1352728bb 100644 --- a/openwisp_controller/config/base/multitenancy.py +++ b/openwisp_controller/config/base/multitenancy.py @@ -37,7 +37,8 @@ class AbstractOrganizationConfigSettings(UUIDModel): load_kwargs={"object_pairs_hook": collections.OrderedDict}, dump_kwargs={"indent": 4}, help_text=_( - 'This field can be used to add "Configuration Variables"' " to the devices." + "Define reusable configuration variables available " + "to all devices in this organization" ), verbose_name=_("Configuration Variables"), ) diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index d7b0a7dc9..25e89eee3 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -97,10 +97,9 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): default=dict, blank=True, help_text=_( - "A dictionary containing the default " - "values for the variables used by this " - "template; these default variables will " - "be used during schema validation." + "Define default values for the variables used in this template. " + "These values are used during validation and when a variable is " + "not provided by the device, group, or organization." ), load_kwargs={"object_pairs_hook": OrderedDict}, dump_kwargs={"indent": 4}, diff --git a/openwisp_controller/config/migrations/0023_update_context.py b/openwisp_controller/config/migrations/0023_update_context.py index 278ee8036..68edd30da 100644 --- a/openwisp_controller/config/migrations/0023_update_context.py +++ b/openwisp_controller/config/migrations/0023_update_context.py @@ -18,9 +18,10 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"ensure_ascii": False, "indent": 4}, help_text=( - 'Additional ' - "context (configuration variables) in JSON format" + "allows overriding " + '' + "configuration variables" ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, ), diff --git a/openwisp_controller/config/migrations/0028_template_default_values.py b/openwisp_controller/config/migrations/0028_template_default_values.py index 47a0909b6..13651f4dd 100644 --- a/openwisp_controller/config/migrations/0028_template_default_values.py +++ b/openwisp_controller/config/migrations/0028_template_default_values.py @@ -18,9 +18,9 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"ensure_ascii": False, "indent": 4}, help_text=( - "A dictionary containing the default values for " - "the variables used by this template; these default " - "variables will be used during schema validation." + "Define default values for the variables used in this template. " + "These values are used during validation and when a variable is " + "not provided by the device, group, or organization." ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, verbose_name="Default Values", diff --git a/openwisp_controller/config/migrations/0036_device_group.py b/openwisp_controller/config/migrations/0036_device_group.py index 6197bbb51..f3ca39ced 100644 --- a/openwisp_controller/config/migrations/0036_device_group.py +++ b/openwisp_controller/config/migrations/0036_device_group.py @@ -62,9 +62,11 @@ class Migration(migrations.Migration): dump_kwargs={"ensure_ascii": False, "indent": 4}, load_kwargs={"object_pairs_hook": collections.OrderedDict}, help_text=( - "Group meta data, use this field to store data which is" - " related to this group and can be retrieved via the" - " REST API." + "Store custom metadata related to this group. " + "This field is intended for arbitrary data that " + "does not affect device configuration and can " + "be retrieved via the REST API for integrations " + "or external tools." ), verbose_name="Metadata", ), diff --git a/openwisp_controller/config/migrations/0049_devicegroup_context.py b/openwisp_controller/config/migrations/0049_devicegroup_context.py index adb108d75..34733f4db 100644 --- a/openwisp_controller/config/migrations/0049_devicegroup_context.py +++ b/openwisp_controller/config/migrations/0049_devicegroup_context.py @@ -20,8 +20,8 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"ensure_ascii": False, "indent": 4}, help_text=( - "This field can be used to add meta data for the group" - ' or to add "Configuration Variables" to the devices.' + "Define configuration variables available " + "to all devices in this group" ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, verbose_name="Configuration Variables", diff --git a/openwisp_controller/config/migrations/0051_organizationconfigsettings_context.py b/openwisp_controller/config/migrations/0051_organizationconfigsettings_context.py index a72a6f173..b643e2e12 100644 --- a/openwisp_controller/config/migrations/0051_organizationconfigsettings_context.py +++ b/openwisp_controller/config/migrations/0051_organizationconfigsettings_context.py @@ -20,8 +20,8 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"indent": 4}, help_text=( - 'This field can be used to add "Configuration Variables"' - " to the devices." + "Define reusable configuration variables available " + "to all devices in this organization" ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, verbose_name="Configuration Variables", diff --git a/tests/openwisp2/sample_config/migrations/0001_initial.py b/tests/openwisp2/sample_config/migrations/0001_initial.py index d41f4fd89..c6bb22c89 100644 --- a/tests/openwisp2/sample_config/migrations/0001_initial.py +++ b/tests/openwisp2/sample_config/migrations/0001_initial.py @@ -121,9 +121,10 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"ensure_ascii": False, "indent": 4}, help_text=( - 'Additional ' - "context (configuration variables) in JSON format" + "allows overriding " + '' + "configuration variables" ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, ), @@ -579,9 +580,10 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"ensure_ascii": False, "indent": 4}, help_text=( - "A dictionary containing the default values for the " - "variables used by this template; these default variables " - "will be used during schema validation." + "Define default values for the variables used " + "in this template. These values are used during " + "validation and when a variable is not provided " + "by the device, group, or organization." ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, verbose_name="Default Values", @@ -709,8 +711,8 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"indent": 4}, help_text=( - 'This field can be used to add "Configuration Variables"' - " to the devices." + "Define reusable configuration variables " + "available to all devices in this organization" ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, verbose_name="Configuration Variables", @@ -765,9 +767,11 @@ class Migration(migrations.Migration): dump_kwargs={"ensure_ascii": False, "indent": 4}, load_kwargs={"object_pairs_hook": collections.OrderedDict}, help_text=( - "Group meta data, use this field to store data which is" - " related to this group and can be retrieved via the" - " REST API." + "Store custom metadata related to this group. " + "This field is intended for arbitrary data that " + "does not affect device configuration and can " + "be retrieved via the REST API for integrations " + "or external tools." ), verbose_name="Metadata", ), @@ -779,8 +783,8 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={"ensure_ascii": False, "indent": 4}, help_text=( - "This field can be used to add meta data for the group" - ' or to add "Configuration Variables" to the devices.' + "Define configuration variables available " + "to all devices in this group" ), load_kwargs={"object_pairs_hook": collections.OrderedDict}, verbose_name="Configuration Variables", From 92bb6a30aacdfbf9007af8528b3f2495bad6823c Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Fri, 6 Mar 2026 16:13:08 -0300 Subject: [PATCH 08/17] [chores] Fixed test_vpn for 1.2 branch --- openwisp_controller/config/tests/test_vpn.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openwisp_controller/config/tests/test_vpn.py b/openwisp_controller/config/tests/test_vpn.py index 091f32afd..0b9a52ad8 100644 --- a/openwisp_controller/config/tests/test_vpn.py +++ b/openwisp_controller/config/tests/test_vpn.py @@ -3,12 +3,13 @@ from subprocess import CalledProcessError, TimeoutExpired from unittest import mock +import requests from celery.exceptions import Retry, SoftTimeLimitExceeded from django.conf import settings from django.core.exceptions import ValidationError from django.db.models.signals import post_save from django.db.utils import IntegrityError -from django.http.response import HttpResponse, HttpResponseNotFound +from django.http.response import HttpResponse from django.test import TestCase, TransactionTestCase from requests.exceptions import ConnectionError, RequestException, Timeout from swapper import load_model @@ -855,7 +856,7 @@ def test_update_vpn_server_configuration(self): fail_response.raise_for_status.side_effect = requests.exceptions.HTTPError( "Not Found" ) - with mock.patch("logging.Logger.warning") as mocked_logger, mock.patch( + with mock.patch("logging.Logger.error") as mocked_logger, mock.patch( "requests.post", return_value=fail_response ): post_save.send( From 4af04a4ae0419a2998c7a054739eaa001c9ae06e Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Fri, 6 Mar 2026 16:36:42 -0300 Subject: [PATCH 09/17] [ci] Bound setuptools to <82 due to removal of pkg_resources --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 175cf78aa..db2eead63 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,7 +74,7 @@ jobs: libgeos-dev libspatialite-dev spatialite-bin \ libsqlite3-mod-spatialite sudo npm install -g prettier - pip install -U pip wheel setuptools + pip install -U pip wheel "setuptools<82" pip install -U -r requirements-test.txt pip install -U -e . pip install ${{ matrix.django-version }} From c0d8813c310a33b2204d92cb062dd55833066af5 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Fri, 6 Mar 2026 17:11:22 -0300 Subject: [PATCH 10/17] [release] Version 1.2.2 --- CHANGES.rst | 12 ++++++++++++ openwisp_controller/__init__.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 7e3a6c238..3a4993fa3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,18 @@ Changelog ========= +Version 1.2.2 [2026-03-06] +-------------------------- + +Changes +~~~~~~~ + +Other changes ++++++++++++++ + +- Improved help text of configuration variable fields +- Minor fixes in the test suite + Version 1.2.1 [2026-03-04] -------------------------- diff --git a/openwisp_controller/__init__.py b/openwisp_controller/__init__.py index cba3a1ce8..dfa212db3 100644 --- a/openwisp_controller/__init__.py +++ b/openwisp_controller/__init__.py @@ -1,4 +1,4 @@ -VERSION = (1, 2, 1, "final") +VERSION = (1, 2, 2, "final") __version__ = VERSION # alias From d6a4727c2f1cf72d120afa1c6fdb3032a32a3330 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 19 Mar 2026 03:04:22 +0530 Subject: [PATCH 11/17] [fix] Fixed concurrent update_config detection #1204 Fixed a bug in the configuration update task that caused Celery workers to incorrectly skip execution. The task would sometimes falsely detect a collision with another worker, leading to missed configuration updates. [backport 1.2] Fixes #1204 Co-authored-by: Piyush Bafna <130243298+piyushdev04@users.noreply.github.com> (cherry picked from commit 45b24b65386d6e28dcaf2b235ee161311510045c) --- openwisp_controller/connection/tasks.py | 15 ++-- .../connection/tests/test_models.py | 68 +++++++++++++++---- .../connection/tests/test_tasks.py | 50 ++++++++++++++ 3 files changed, 116 insertions(+), 17 deletions(-) diff --git a/openwisp_controller/connection/tasks.py b/openwisp_controller/connection/tasks.py index d75bbde20..56e6a7bb1 100644 --- a/openwisp_controller/connection/tasks.py +++ b/openwisp_controller/connection/tasks.py @@ -16,20 +16,25 @@ _TASK_NAME = "openwisp_controller.connection.tasks.update_config" -def _is_update_in_progress(device_id): +def _is_update_in_progress(device_id, current_task_id=None): active = current_app.control.inspect().active() if not active: return False # check if there's any other running task before adding it + # exclude the current task by comparing task IDs for task_list in active.values(): for task in task_list: - if task["name"] == _TASK_NAME and str(device_id) in task["args"]: + if ( + task["name"] == _TASK_NAME + and str(device_id) in task["args"] + and task["id"] != current_task_id + ): return True return False -@shared_task -def update_config(device_id): +@shared_task(bind=True) +def update_config(self, device_id): """ Launches the ``update_config()`` operation of a specific device in the background @@ -48,7 +53,7 @@ def update_config(device_id): except ObjectDoesNotExist as e: logger.warning(f'update_config("{device_id}") failed: {e}') return - if _is_update_in_progress(device_id): + if _is_update_in_progress(device_id, current_task_id=self.request.id): return try: device_conn = DeviceConnection.get_working_connection(device) diff --git a/openwisp_controller/connection/tests/test_models.py b/openwisp_controller/connection/tests/test_models.py index 14693dfbd..4766a3f7e 100644 --- a/openwisp_controller/connection/tests/test_models.py +++ b/openwisp_controller/connection/tests/test_models.py @@ -1,6 +1,7 @@ import socket from unittest import mock from unittest.mock import PropertyMock +from uuid import uuid4 import paramiko from django.contrib.auth.models import ContentType @@ -1026,20 +1027,56 @@ def _assert_applying_conf_test_command(mocked_exec): @mock.patch.object(DeviceConnection, "update_config") @mock.patch.object(DeviceConnection, "get_working_connection") def test_device_update_config_in_progress( - self, mocked_get_working_connection, update_config, mocked_sleep + self, mocked_get_working_connection, mocked_update_config, mocked_sleep ): conf = self._prepare_conf_object() - with mock.patch("celery.app.control.Inspect.active") as mocked_active: - mocked_active.return_value = { - "task": [{"name": _TASK_NAME, "args": [str(conf.device.pk)]}] - } - conf.config = {"general": {"timezone": "UTC"}} - conf.full_clean() - conf.save() - mocked_active.assert_called_once() - mocked_get_working_connection.assert_not_called() - update_config.assert_not_called() + with self.subTest("More than one update_config task active for the device"): + with mock.patch("celery.app.control.Inspect.active") as mocked_active: + mocked_active.return_value = { + "task": [ + { + "name": _TASK_NAME, + "args": [str(conf.device.pk)], + "id": str(uuid4()), + } + ] + } + conf.config = {"general": {"timezone": "UTC"}} + conf.full_clean() + conf.save() + mocked_active.assert_called_once() + mocked_get_working_connection.assert_not_called() + mocked_update_config.assert_not_called() + + Config.objects.update(status="applied") + mocked_get_working_connection.return_value = ( + conf.device.deviceconnection_set.first() + ) + with self.subTest("Only one task is active for the device"): + task_id = str(uuid4()) + with mock.patch( + "celery.app.control.Inspect.active" + ) as mocked_active, mock.patch( + "celery.app.task.Context.id", + new_callable=mock.PropertyMock, + return_value=task_id, + ): + mocked_active.return_value = { + "task": [ + { + "name": _TASK_NAME, + "args": [str(conf.device.pk)], + "id": task_id, + } + ] + } + conf.config = {"general": {"timezone": "Asia/Kolkata"}} + conf.full_clean() + conf.save() + mocked_active.assert_called_once() + mocked_get_working_connection.assert_called_once() + mocked_update_config.assert_called_once() @mock.patch("time.sleep") @mock.patch.object(DeviceConnection, "update_config") @@ -1053,8 +1090,15 @@ def test_device_update_config_not_in_progress( ) with mock.patch("celery.app.control.Inspect.active") as mocked_active: + # Mock a task running for a different device (args is different) mocked_active.return_value = { - "task": [{"name": _TASK_NAME, "args": ["..."]}] + "task": [ + { + "name": _TASK_NAME, + "args": ["another-device-id"], # Different device + "id": "different-task-id", + } + ] } conf.config = {"general": {"timezone": "UTC"}} conf.full_clean() diff --git a/openwisp_controller/connection/tests/test_tasks.py b/openwisp_controller/connection/tests/test_tasks.py index c2cdb7ff4..1f55bdb36 100644 --- a/openwisp_controller/connection/tests/test_tasks.py +++ b/openwisp_controller/connection/tests/test_tasks.py @@ -19,6 +19,56 @@ class TestTasks(CreateConnectionsMixin, TestCase): "openwisp_controller.connection.base.models.AbstractDeviceConnection.connect" ) + def _get_mocked_celery_active(self, device_id, task_id=None): + return { + "worker1": [ + { + "name": tasks._TASK_NAME, + "args": [device_id], + "id": task_id or str(uuid.uuid4()), + } + ] + } + + def test_is_update_in_progress_same_task(self): + device_id = str(uuid.uuid4()) + task_id = str(uuid.uuid4()) + with mock.patch( + "celery.app.control.Inspect.active", + return_value=self._get_mocked_celery_active(device_id, task_id), + ): + result = tasks._is_update_in_progress(device_id, current_task_id=task_id) + self.assertEqual(result, False) + + def test_is_update_in_progress_different_task(self): + device_id = str(uuid.uuid4()) + current_task_id = str(uuid.uuid4()) + other_task_id = str(uuid.uuid4()) + with mock.patch( + "celery.app.control.Inspect.active", + return_value=self._get_mocked_celery_active(device_id, other_task_id), + ): + result = tasks._is_update_in_progress( + device_id, current_task_id=current_task_id + ) + self.assertEqual(result, True) + + def test_is_update_in_progress_no_tasks(self): + device_id = str(uuid.uuid4()) + with mock.patch("celery.app.control.Inspect.active", return_value={}): + result = tasks._is_update_in_progress(device_id) + self.assertEqual(result, False) + + def test_is_update_in_progress_different_device(self): + device_id = str(uuid.uuid4()) + other_device_id = str(uuid.uuid4()) + with mock.patch( + "celery.app.control.Inspect.active", + return_value=self._get_mocked_celery_active(other_device_id), + ): + result = tasks._is_update_in_progress(device_id) + self.assertEqual(result, False) + @mock.patch("logging.Logger.warning") @mock.patch("time.sleep") def test_update_config_missing_config(self, mocked_sleep, mocked_warning): From 981c8906241cc05812f6a7eab30aadce25f4dee3 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 27 Mar 2026 04:26:24 +0530 Subject: [PATCH 12/17] [fix] Invalidate Config checksum after subnet provisioning Bug: Previously, the `post_provision_handler` in BaseSubnetDivisionRuleType did not update the associated Config checksum after subnets or IP addresses were provisioned. This could lead to inconsistent state where the Config's checksum and checksum_db diverged, especially when VPN templates were switched or updated, causing change detection and dependent logic to fail. Fix: - Converted `post_provision_handler` to a classmethod so subclasses can extend it safely. - Invalidate the backend instance cache for the Config. - Update `checksum_db` if the current checksum differs. - In VPNSubnetDivisionRuleType, call the superclass handler to ensure checksum and cache are updated automatically when a VPN template is provisioned. [backport 1.2] (cherry picked from commit 76e15278adf0979a63e3bd9b007ad73b8f33de77) --- .../subnet_division/rule_types/base.py | 36 ++++- .../subnet_division/rule_types/vpn.py | 5 +- .../subnet_division/tests/test_admin.py | 140 +++++++++++++++++- 3 files changed, 168 insertions(+), 13 deletions(-) diff --git a/openwisp_controller/subnet_division/rule_types/base.py b/openwisp_controller/subnet_division/rule_types/base.py index 826150cbb..2b51797b8 100644 --- a/openwisp_controller/subnet_division/rule_types/base.py +++ b/openwisp_controller/subnet_division/rule_types/base.py @@ -79,16 +79,36 @@ def _provision_receiver(): def destroyer_receiver(cls, instance, **kwargs): cls.destroy_provisioned_subnets_ips(instance, **kwargs) - @staticmethod - def post_provision_handler(instance, provisioned, **kwargs): + @classmethod + def post_provision_handler(cls, instance, provisioned, **kwargs): """ - This method should be overridden in inherited rule types to - perform any operation on provisioned subnets and IP addresses. - :param instance: object that triggered provisioning - :param provisioned: dictionary containing subnets and IP addresses - provisioned, None if nothing is provisioned + Hook for post-provisioning actions on subnets and IP addresses. + + This method is intended to be extended by subclasses of rule types + to perform custom operations after subnets and IPs are provisioned. + + Subnet provisioning is executed asynchronously in Celery workers. + If the device configuration references variables provided by the + subnet division rule, the current checksum may have been computed + using variable names instead of their provisioned values. In such cases, + `Config.checksum_db` (which tracks persisted configuration changes) + must be updated to reflect the actual provisioned values, and the + checksum cache invalidated to avoid stale data. + + :param instance: The object that triggered the provisioning. + :param provisioned: Dictionary containing provisioned subnets and IPs, + or None if no provisioning occurred. """ - pass + if not provisioned: + return + config = cls.get_config(instance) + config._invalidate_backend_instance_cache() + current_checksum = config.checksum + if current_checksum != config.checksum_db: + # Update checksum using the UPDATE query to avoid sending + # unnecessary signals that may be triggered by `save()` method. + config._update_checksum_db(current_checksum) + config.invalidate_checksum_cache() @staticmethod def subnet_provisioned_signal_emitter(instance, provisioned): diff --git a/openwisp_controller/subnet_division/rule_types/vpn.py b/openwisp_controller/subnet_division/rule_types/vpn.py index 4f21ad0e9..2680f89fb 100644 --- a/openwisp_controller/subnet_division/rule_types/vpn.py +++ b/openwisp_controller/subnet_division/rule_types/vpn.py @@ -41,8 +41,9 @@ def provision_for_existing_objects(cls, rule_obj): for vpn_client in qs: cls.provision_receiver(instance=vpn_client, created=True) - @staticmethod - def post_provision_handler(instance, provisioned, **kwargs): + @classmethod + def post_provision_handler(cls, instance, provisioned, **kwargs): + super().post_provision_handler(instance, provisioned, **kwargs) # Assign the first provisioned IP address to the VPNClient # only when subnets and IPs have been provisioned if provisioned and provisioned["ip_addresses"]: diff --git a/openwisp_controller/subnet_division/tests/test_admin.py b/openwisp_controller/subnet_division/tests/test_admin.py index e058aa3f4..793540ccc 100644 --- a/openwisp_controller/subnet_division/tests/test_admin.py +++ b/openwisp_controller/subnet_division/tests/test_admin.py @@ -1,16 +1,21 @@ from unittest.mock import patch -from django.test import TestCase +from django.test import TestCase, TransactionTestCase from django.urls import reverse from swapper import load_model -from openwisp_controller.config.tests.utils import TestWireguardVpnMixin +from openwisp_controller.config.tests.test_admin import TestDeviceAdminMixin +from openwisp_controller.config.tests.utils import ( + TestVpnX509Mixin, + TestWireguardVpnMixin, +) from openwisp_users.tests.utils import TestMultitenantAdminMixin -from .helpers import SubnetDivisionAdminTestMixin +from .helpers import SubnetDivisionAdminTestMixin, SubnetDivisionTestMixin Subnet = load_model("openwisp_ipam", "Subnet") Device = load_model("config", "Device") +Config = load_model("config", "Config") class TestSubnetAdmin( @@ -257,3 +262,132 @@ def test_delete_device(self): ) self.assertEqual(subnet_response.status_code, 200) self.assertContains(subnet_response, self.config.device.name, 1) + + +class TestTransactionDeviceAdmin( + SubnetDivisionTestMixin, + TestVpnX509Mixin, + TestDeviceAdminMixin, + TransactionTestCase, +): + ipam_label = "openwisp_ipam" + config_label = "config" + + def test_vpn_template_switch_checksum_db(self): + admin = self._create_admin() + self.client.force_login(admin) + org = self._get_org() + vpn1_subnet = self._get_master_subnet(organization=org, subnet="10.0.0.0/24") + self._get_vpn_subdivision_rule( + number_of_ips=1, + number_of_subnets=1, + organization=org, + master_subnet=vpn1_subnet, + label="VPN1", + ) + vpn1 = self._create_vpn(name="vpn1", organization=org, subnet=vpn1_subnet) + vpn2_subnet = self._get_master_subnet(organization=org, subnet="10.0.1.0/24") + self._get_vpn_subdivision_rule( + number_of_ips=1, + number_of_subnets=1, + organization=org, + master_subnet=vpn2_subnet, + label="VPN2", + ) + vpn2 = self._create_vpn(name="vpn2", organization=org, subnet=vpn2_subnet) + vpn1_template = self._create_template( + organization=org, + name="vpn1-template", + type="vpn", + vpn=vpn1, + default_values={ + "VPN1_subnet1_ip1": "10.0.0.1", + "VPN1_prefix": "24", + "ifname": "tun0", + }, + auto_cert=True, + config={}, + ) + vpn1_template.config["openvpn"][0]["dev"] = "{{ ifname }}" + vpn1_template.config.update( + { + "network": [ + { + "config_name": "interface", + "config_value": "lan", + "ipaddr": "{{ VPN1_subnet1_ip1 }}", + "netmask": "255.255.255.240", + } + ], + } + ) + vpn1_template.full_clean() + vpn1_template.save() + vpn2_template = self._create_template( + organization=org, + name="vpn2-template", + type="vpn", + vpn=vpn2, + default_values={ + "VPN2_subnet1_ip1": "10.0.1.1", + "VPN2_prefix": "32", + "ifname": "tun1", + }, + auto_cert=True, + config={}, + ) + vpn2_template.config["openvpn"][0]["dev"] = "{{ ifname }}" + vpn2_template.config.update( + { + "network": [ + { + "config_name": "interface", + "config_value": "lan", + "ipaddr": "{{ VPN2_subnet1_ip1 }}", + "netmask": "255.255.255.240", + } + ], + } + ) + vpn2_template.full_clean() + vpn2_template.save() + default_template = self._create_template( + name="default-template", + default=True, + ) + path = reverse(f"admin:{self.config_label}_device_add") + params = self._get_device_params(org=org) + params.update( + {"config-0-templates": f"{default_template.pk},{vpn1_template.pk}"} + ) + response = self.client.post(path, data=params, follow=True) + self.assertEqual(response.status_code, 200) + config = Config.objects.get(device__name=params["name"]) + config.refresh_from_db() + config._invalidate_backend_instance_cache() + initial_checksum = config.checksum + self.assertEqual(config.checksum_db, initial_checksum) + self.assertEqual(config.vpnclient_set.count(), 1) + self.assertEqual(config.vpnclient_set.first().vpn, vpn1) + + path = reverse( + f"admin:{self.config_label}_device_change", args=[config.device_id] + ) + params.update( + { + "config-0-templates": f"{default_template.pk},{vpn2_template.pk}", + "config-0-id": str(config.pk), + "config-0-device": str(config.device_id), + "config-INITIAL_FORMS": 1, + "_continue": True, + } + ) + response = self.client.post(path, data=params, follow=True) + self.assertEqual(response.status_code, 200) + config.refresh_from_db() + config._invalidate_backend_instance_cache() + self.assertEqual(config.status, "modified") + self.assertEqual(config.vpnclient_set.count(), 1) + self.assertEqual(config.vpnclient_set.first().vpn, vpn2) + self.assertNotEqual(config.checksum, initial_checksum) + self.assertEqual(config.checksum, config.checksum_db) From 9531b8b7aa3b3a8af356b46e5b8a6ff048a1f506 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 27 Mar 2026 10:15:01 +0530 Subject: [PATCH 13/17] [chores] Fixed tests to include organization in location add params (cherry picked from commit af0d99b2d3128fccf0c296601296de9030cf9e38) --- openwisp_controller/geo/tests/test_admin.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openwisp_controller/geo/tests/test_admin.py b/openwisp_controller/geo/tests/test_admin.py index 8d914dc8d..e333c4b1d 100644 --- a/openwisp_controller/geo/tests/test_admin.py +++ b/openwisp_controller/geo/tests/test_admin.py @@ -29,6 +29,12 @@ def setUp(self): """override TestAdminMixin.setUp""" pass + def _get_location_add_params(self, **kwargs): + params = super()._get_location_add_params(**kwargs) + if "organization" not in kwargs: + params["organization"] = self._get_org().id + return params + def _create_multitenancy_test_env(self, vpn=False): org1 = self._create_organization(name="test1org") org2 = self._create_organization(name="test2org") From 7c269011dbe714892f93fa7ceb94b197d0f53b92 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 27 Mar 2026 21:55:12 +0530 Subject: [PATCH 14/17] [fix] Fixed duplicate template entries in Device admin Bug: The JS logic in relevant_templates.js assumed that the last `ul.sortedm2m-items` element belonged to the empty inline form used by Django admin formsets. This assumption breaks when the user does not have permission to add Config objects: in that case the ConfigInlineAdmin does not render the empty form. As a result, both selectors ended up referencing the same list and the script appended template elements twice to the same `sortedm2m` list, causing duplicate entries and issues when saving the form. Fix: Select the empty form container explicitly using `#config-empty ul.sortedm2m-items` instead of relying on the last occurrence of `ul.sortedm2m-items`. This ensures the logic works correctly regardless of whether the empty inline form is rendered. [backport 1.2] (cherry picked from commit b73e83ad2d82dd64ea83717921cb05618db53769) --- .../static/config/js/relevant_templates.js | 2 +- .../config/tests/test_selenium.py | 100 +++++++++++++++++- .../geo/tests/test_selenium.py | 6 +- 3 files changed, 104 insertions(+), 4 deletions(-) diff --git a/openwisp_controller/config/static/config/js/relevant_templates.js b/openwisp_controller/config/static/config/js/relevant_templates.js index 9d7a1a766..72a1668ac 100644 --- a/openwisp_controller/config/static/config/js/relevant_templates.js +++ b/openwisp_controller/config/static/config/js/relevant_templates.js @@ -138,7 +138,7 @@ django.jQuery(function ($) { resetTemplateOptions(); var enabledTemplates = [], sortedm2mUl = $("ul.sortedm2m-items:first"), - sortedm2mPrefixUl = $("ul.sortedm2m-items:last"); + sortedm2mPrefixUl = $("#config-empty ul.sortedm2m-items"); // Adds "li" elements for templates Object.keys(data).forEach(function (templateId, index) { diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index ae9061d39..ace698c6f 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -1,5 +1,6 @@ import time +from django.contrib.auth.models import Group, Permission from django.contrib.staticfiles.testing import StaticLiveServerTestCase from django.test import tag from django.urls.base import reverse @@ -40,9 +41,17 @@ def _verify_templates_visibility(self, hidden=None, visible=None): hidden = hidden or [] visible = visible or [] for template in hidden: - self.wait_for_invisibility(By.XPATH, f'//*[@value="{template.id}"]') + self.wait_for_invisibility( + By.XPATH, + f'//ul[contains(@class,"sortedm2m-items")]' + f'//input[@value="{template.id}"]', + ) for template in visible: - self.wait_for_visibility(By.XPATH, f'//*[@value="{template.id}"]') + self.wait_for_visibility( + By.XPATH, + f'//ul[contains(@class,"sortedm2m-items")]' + f'//input[@value="{template.id}"]', + ) @tag("selenium_tests") @@ -373,6 +382,93 @@ def test_add_remove_templates(self): self.assertEqual(config.templates.count(), 0) self.assertEqual(config.status, "modified") + def test_relevant_templates_duplicates(self): + """ + Test that a user with specific permissions can see shared templates + properly. Verifies that: + 1. User with custom group permissions can access the admin + 2. Multiple shared templates are displayed correctly + 3. Each template appears only once in the sortedm2m list + """ + # Define permission codenames for the custom group + permission_codenames = [ + "view_group", + "change_config", + "view_config", + "add_device", + "change_device", + "delete_device", + "view_device", + "view_devicegroup", + "view_template", + ] + # Create a custom group with the specified permissions + permissions = Permission.objects.filter(codename__in=permission_codenames) + custom_group, _ = Group.objects.get_or_create(name="Custom Operator") + custom_group.permissions.set(permissions) + # Create a user and assign the custom group + user = self._create_user( + username="limited_user", + password="testpass123", + email="limited@test.com", + is_staff=True, + ) + user.groups.add(custom_group) + org = self._get_org() + self._create_org_user(user=user, organization=org, is_admin=True) + # Create multiple shared templates (organization=None) + template1 = self._create_template( + name="Shared Template 1", organization=None, default=True + ) + template2 = self._create_template(name="Shared Template 2", organization=None) + device = self._create_config(organization=org).device + # Login as the limited user + self.login(username="limited_user", password="testpass123") + # Navigate using Selenium + self.open( + reverse("admin:config_device_change", args=[device.id]) + "#config-group" + ) + self.hide_loading_overlay() + with self.subTest( + "Regression precondition: empty Config inline is not rendered" + ): + self.assertFalse(self.web_driver.find_elements(By.ID, "config-empty")) + + with self.subTest("All shared templates should be visible"): + self._verify_templates_visibility(visible=[template1, template2]) + + with self.subTest("Verify sortedm2m list has exactly 2 template items"): + # Check that ul.sortedm2m-items.sortedm2m.ui-sortable has exactly 2 children + # with .sortedm2m-item class + sortedm2m_items = self.find_elements( + by=By.CSS_SELECTOR, + value="ul.sortedm2m-items.sortedm2m.ui-sortable > li.sortedm2m-item", + ) + self.assertEqual( + len(sortedm2m_items), + 2, + ( + "Expected exactly 2 template items in sortedm2m list," + f" found {len(sortedm2m_items)}" + ), + ) + + with self.subTest( + "Verify checkbox inputs are rendered with expected attributes" + ): + for idx, template_id in enumerate([template1.id, template2.id]): + checkbox = self.find_element( + by=By.ID, value=f"id_config-templates_{idx}" + ) + self.assertEqual(checkbox.get_attribute("value"), str(template_id)) + self.assertEqual(checkbox.get_attribute("data-required"), "false") + + with self.subTest("Save operation completes successfully"): + # Scroll to the top of the page to ensure the save button is visible + self.web_driver.execute_script("window.scrollTo(0, 0);") + self.find_element(by=By.NAME, value="_save").click() + self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success", timeout=5) + @tag("selenium_tests") class TestDeviceGroupAdmin( diff --git a/openwisp_controller/geo/tests/test_selenium.py b/openwisp_controller/geo/tests/test_selenium.py index 03d1dcac0..1a24354c9 100644 --- a/openwisp_controller/geo/tests/test_selenium.py +++ b/openwisp_controller/geo/tests/test_selenium.py @@ -91,7 +91,11 @@ def setUp(self): def test_unsaved_changes_readonly(self): self.login() ol = self._create_object_location() - path = reverse("admin:config_device_change", args=[ol.device.id]) + path = reverse( + f"admin:{self.object_model._meta.app_label}_" + f"{self.object_model._meta.model_name}_change", + args=[ol.device.id], + ) with self.subTest("Alert should not be displayed without any change"): self.open(path) From adaa725850bad0043100f7fdc090c0b6e854eea1 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Fri, 27 Mar 2026 21:02:19 -0300 Subject: [PATCH 15/17] [fix] Corrected initial field value assignment in AbstractDevice class Fixed incorrect initialization of `_initial_` values when a Device instance is loaded with deferred fields, which could break change detection logic. [backport 1.2] --- openwisp_controller/config/base/device.py | 6 +++--- openwisp_controller/config/tests/test_device.py | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/openwisp_controller/config/base/device.py b/openwisp_controller/config/base/device.py index 003fe862e..bbd3d2039 100644 --- a/openwisp_controller/config/base/device.py +++ b/openwisp_controller/config/base/device.py @@ -325,9 +325,9 @@ def _get_initial_values_for_checked_fields(self): if not present_values: return self.refresh_from_db(fields=present_values.keys()) - for field in self._changed_checked_fields: - setattr(self, f"_initial_{field}", field) - setattr(self, field, present_values[field]) + for field, value in present_values.items(): + setattr(self, f"_initial_{field}", getattr(self, field)) + setattr(self, field, value) def _check_name_changed(self): if self._initial_name == models.DEFERRED: diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index eb053f237..350fc15e5 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -529,6 +529,20 @@ def test_device_field_changed_checks(self): with self.assertNumQueries(3): device._check_changed_fields() + def test_deferred_fields_populated_correctly(self): + device = self._create_device( + name="deferred-test", + management_ip="10.0.0.1", + ) + # Load the instance with deferred fields omitted + device = Device.objects.only("id").get(pk=device.pk) + device.management_ip = "10.0.0.55" + # Saving the device object will populate the deferred fields + device.save() + # Ensure `_initial_` contains the actual value, not the field name + self.assertEqual(getattr(device, "_initial_management_ip"), "10.0.0.55") + self.assertNotEqual(getattr(device, "_initial_management_ip"), "management_ip") + def test_exceed_organization_device_limit(self): org = self._get_org() org.config_limits.device_limit = 1 From b8086e017284a4cf80fc9b073d90cc46043e03fd Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Thu, 9 Apr 2026 14:16:18 -0300 Subject: [PATCH 16/17] 1.2.3 release --- CHANGES.rst | 12 ++++++++++++ openwisp_controller/__init__.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3a4993fa3..5fba2b293 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,18 @@ Changelog ========= +Version 1.2.3 [2026-04-09] +-------------------------- + +Bugfixes +~~~~~~~~ + +- Corrected initial field value assignment in ``AbstractDevice`` class +- Fixed duplicate template entries in Device admin +- Invalidate Config checksum after subnet provisioning +- Fixed concurrent ``update_config`` detection `#1204 + `_ + Version 1.2.2 [2026-03-06] -------------------------- diff --git a/openwisp_controller/__init__.py b/openwisp_controller/__init__.py index dfa212db3..a3e16c5d2 100644 --- a/openwisp_controller/__init__.py +++ b/openwisp_controller/__init__.py @@ -1,4 +1,4 @@ -VERSION = (1, 2, 2, "final") +VERSION = (1, 2, 3, "final") __version__ = VERSION # alias From 2c9091c656aeeffd6872cb706c9bce1d22e1d9d4 Mon Sep 17 00:00:00 2001 From: Oliver Kraitschy Date: Tue, 19 May 2026 15:54:12 +0200 Subject: [PATCH 17/17] [fix] Fix model access in migration for checksum_db We need to use the historical model for the data migration, otherwise the migration will fail. See https://docs.djangoproject.com/en/6.0/topics/migrations/#data-migrations [backport 1.2] Signed-off-by: Oliver Kraitschy --- .../config/migrations/0061_config_checksum_db.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openwisp_controller/config/migrations/0061_config_checksum_db.py b/openwisp_controller/config/migrations/0061_config_checksum_db.py index a572951cf..34319cae3 100644 --- a/openwisp_controller/config/migrations/0061_config_checksum_db.py +++ b/openwisp_controller/config/migrations/0061_config_checksum_db.py @@ -1,7 +1,6 @@ # Generated by Django for issue #1113 optimization from django.db import migrations, models -from swapper import load_model def populate_checksum_db(apps, schema_editor): @@ -13,7 +12,7 @@ def populate_checksum_db(apps, schema_editor): hence we use Config.objects.bulk_update() instead of Config.update_status_if_checksum_changed(). """ - Config = load_model("config", "Config") + Config = apps.get_model("config", "Config") chunk_size = 100 updated_configs = [] qs = (