[feature] Allowed device to update name on re-registration

(cherry picked from commit 21002f3)

Author: pandafy

openwisp_controller/config/base/device.py

@@ -435,6 +435,29 @@ def get_temp_config_instance(self, **options):
         config.device = self
         return config
 
+    def should_skip_push_update(self):
+        """
+        Returns True if the device should skip push update.
+        Can be overridden with custom logic if needed.
+        """
+        return getattr(self, "_skip_push_update", False)
+
+    def skip_push_update_on_save(self):
+        """
+        Marks the device to skip push update on the next save.
+        This is useful when updating device properties during re-registration
+        to avoid triggering unnecessary push operations.
+        """
+        self._skip_push_update = True
+
+    def clear_skip_push_update(self):
+        """
+        Clears the skip push update flag.
+        This should be called after the flag has been checked and consumed.
+        """
+        if hasattr(self, "_skip_push_update"):
+            delattr(self, "_skip_push_update")
+
     def can_be_updated(self):
         """
         returns True if the device can and should be updated

openwisp_controller/config/controller/views.py

@@ -406,6 +406,10 @@ def post(self, request, *args, **kwargs):
             for attr in self.UPDATABLE_FIELDS:
                 if attr in request.POST:
                     setattr(device, attr, request.POST.get(attr))
+            # update hostname if it's not a default/generic value
+            if self._should_update_hostname(request, device):
+                device.name = request.POST.get("name")
+                device.skip_push_update_on_save()
             config = device.config
         # if get queryset fails, instantiate a new Device and Config
         except self.model.DoesNotExist:
@@ -457,6 +461,34 @@ def post(self, request, *args, **kwargs):
             s.format(**attributes), content_type="text/plain", status=201
         )
 
+    def _should_update_hostname(self, request, device):
+        """
+        Determines whether the hostname should be updated during re-registration.
+
+        Returns False if the hostname equals the MAC address (agents send MAC address
+        as hostname when hostname is OpenWrt or if default_hostname is set to *).
+
+        This prevents overwriting user configured hostnames with default/generic values.
+        """
+        if "name" not in request.POST:
+            return False
+
+        name = request.POST.get("name")
+        mac_address = device.mac_address
+        # normalize MAC address format (strip colons/dashes and lowercase)
+        normalized_mac = (
+            mac_address.replace(":", "").replace("-", "").lower()
+            if mac_address
+            else None
+        )
+        # normalize hostname for comparison
+        normalized_name = name.replace(":", "").replace("-", "").lower()
+        # don't update if hostname equals the MAC address
+        if normalized_name == normalized_mac:
+            return False
+
+        return True
+
 
 class GetVpnView(SingleObjectMixin, View):
     """

openwisp_controller/config/tests/test_controller.py

@@ -820,6 +820,87 @@ def test_device_registration_update_hw_info_no_config(self):
         self.assertEqual(d.system, params["system"])
         self.assertEqual(d.model, params["model"])
 
+    def test_device_registration_update_hostname(self):
+        """
+        Test that hostname is updated when the name in payload
+        is not the MAC address stored in OpenWISP
+        """
+        device = self._create_device_config(
+            device_opts={
+                "name": "old-hostname",
+                "mac_address": TEST_MACADDR,
+                "key": TEST_CONSISTENT_KEY,
+            }
+        )
+        params = {
+            "secret": TEST_ORG_SHARED_SECRET,
+            "name": "new-custom-hostname",
+            "mac_address": TEST_MACADDR,
+            "key": TEST_CONSISTENT_KEY,
+            "backend": "netjsonconfig.OpenWrt",
+            "model": "TP-Link TL-WDR4300 v2",
+            "os": "OpenWrt 18.06-SNAPSHOT r7312-e60be11330",
+            "system": "Atheros AR9344 rev 3",
+        }
+        self.assertNotEqual(device.name, params["name"])
+        response = self.client.post(self.register_url, params)
+        self.assertEqual(response.status_code, 201)
+        device.refresh_from_db()
+        self.assertEqual(device.name, "new-custom-hostname")
+
+    def test_device_registration_hostname_not_updated_when_mac_address(self):
+        """
+        Test that hostname is not updated when the name in payload
+        equals the MAC address (agents send MAC address as hostname
+        when hostname is OpenWrt or if default_hostname is set to *)
+        """
+        device = self._create_device_config(
+            device_opts={
+                "name": "meaningful-hostname",
+                "key": TEST_CONSISTENT_KEY,
+            }
+        )
+        params = {
+            "secret": TEST_ORG_SHARED_SECRET,
+            "name": TEST_MACADDR,
+            "mac_address": TEST_MACADDR,
+            "key": TEST_CONSISTENT_KEY,
+            "backend": "netjsonconfig.OpenWrt",
+            "model": "TP-Link TL-WDR4300 v2",
+            "os": "OpenWrt 18.06-SNAPSHOT r7312-e60be11330",
+            "system": "Atheros AR9344 rev 3",
+        }
+        response = self.client.post(self.register_url, params)
+        self.assertEqual(response.status_code, 201)
+        device.refresh_from_db()
+        self.assertEqual(device.name, "meaningful-hostname")
+
+    def test_device_registration_hostname_comparison_case_insensitive(self):
+        """
+        Test that MAC address comparison is case-insensitive and works
+        with different formats (colons, dashes, no separators)
+        """
+        mac_address = "00:11:22:33:aa:BB"
+        d = self._create_device_config(
+            device_opts={
+                "mac_address": mac_address,
+                "name": "configured-hostname",
+                "key": TEST_CONSISTENT_KEY,
+            }
+        )
+        params = {
+            "secret": TEST_ORG_SHARED_SECRET,
+            "name": "00-11-22-33-AA-bb",
+            "mac_address": mac_address,
+            "key": TEST_CONSISTENT_KEY,
+            "backend": "netjsonconfig.OpenWrt",
+        }
+        response = self.client.post(self.register_url, params)
+        self.assertEqual(response.status_code, 201)
+        d.refresh_from_db()
+        # Hostname should not be changed
+        self.assertEqual(d.name, "configured-hostname")
+
     def test_device_report_status_running(self):
         """
         maintained for backward compatibility

openwisp_controller/connection/apps.py

@@ -64,7 +64,7 @@ def ready(self):
 
     @classmethod
     def config_modified_receiver(cls, **kwargs):
-        transaction.on_commit(lambda: cls._launch_update_config(kwargs["device"].pk))
+        transaction.on_commit(lambda: cls._launch_update_config(kwargs["device"]))
 
     @classmethod
     def command_save_receiver(cls, sender, created, instance, **kwargs):
@@ -81,14 +81,18 @@ def command_save_receiver(cls, sender, created, instance, **kwargs):
         )
 
     @classmethod
-    def _launch_update_config(cls, device_id):
+    def _launch_update_config(cls, device):
         """
         Calls the background task update_config only if
         no other tasks are running for the same device
         """
         from .tasks import update_config
 
-        update_config.delay(device_id)
+        # Check if push update should be skipped
+        if device.should_skip_push_update():
+            device.clear_skip_push_update()
+            return
+        update_config.delay(str(device.pk))
 
     @classmethod
     def is_working_changed_receiver(

openwisp_controller/connection/tests/test_tasks.py

@@ -4,13 +4,15 @@
 from unittest import mock
 
 from celery.exceptions import SoftTimeLimitExceeded
-from django.test import TestCase
+from django.test import TestCase, TransactionTestCase
+from django.urls import reverse
 from swapper import load_model
 
 from .. import tasks
 from .utils import CreateConnectionsMixin
 
 Command = load_model("connection", "Command")
+OrganizationConfigSettings = load_model("config", "OrganizationConfigSettings")
 
 
 class TestTasks(CreateConnectionsMixin, TestCase):
@@ -85,3 +87,30 @@ def test_launch_command_exception(self, *args):
         command.refresh_from_db()
         self.assertEqual(command.status, "failed")
         self.assertEqual(command.output, "Internal system error: test error\n")
+
+
+class TestTransactionTasks(CreateConnectionsMixin, TransactionTestCase):
+    @mock.patch.object(tasks.update_config, "delay")
+    def test_update_config_hostname_changed_on_reregister(self, mocked_update_config):
+        org = self._get_org()
+        config_settings = OrganizationConfigSettings.objects.create(
+            organization=org, context={}, shared_secret="secret"
+        )
+        device = self._create_device(
+            name="AA-BB-CC-DD-EE-FF", organization=org, key="test-key"
+        )
+        self._create_config(device=device)
+        self._create_device_connection(device=device)
+        # Trigger re-registration with new hostname
+        response = self.client.post(
+            reverse("controller:device_register"),
+            {
+                "name": "new-hostname",
+                "secret": str(config_settings.shared_secret),
+                "key": str(device.key),
+                "mac_address": device.mac_address,
+                "backend": "netjsonconfig.OpenWrt",
+            },
+        )
+        self.assertEqual(response.status_code, 201)
+        mocked_update_config.assert_not_called()