[fix] Use counter's reply name for CoA

Also added error handling when the NAS name field does not contain a valid ip.

Author: pandafy

openwisp_radius/tasks.py

@@ -15,6 +15,7 @@
 from openwisp_utils.admin_theme.email import send_email
 from openwisp_utils.tasks import OpenwispCeleryTask
 
+from . import settings as app_settings
 from .radclient.client import RadClient
 from .utils import get_one_time_login_url, load_model
 
@@ -128,17 +129,28 @@ def get_radsecret_from_radacct(rad_acct):
             'name', 'secret'
         )
         for nas in qs.iterator():
-            if ipaddress.ip_address(rad_acct.nas_ip_address) in ipaddress.ip_network(
-                nas.name
-            ):
-                return nas.secret
+            try:
+                if ipaddress.ip_address(
+                    rad_acct.nas_ip_address
+                ) in ipaddress.ip_network(nas.name):
+                    return nas.secret
+            except ValueError:
+                logger.warning(
+                    f'Failed to parse NAS IP network for "{nas.id}" object. Skipping!'
+                )
+
+    def get_radius_reply_name(check):
+        try:
+            return app_settings.CHECK_ATTRIBUTE_COUNTERS_MAP[check.attribute].reply_name
+        except KeyError:
+            return check.attribute
 
     def get_radius_attributes():
         attributes = {}
         rad_group_checks = RadiusGroupCheck.objects.filter(group_id=new_group_id)
         if rad_group_checks:
             for check in rad_group_checks:
-                attributes[check.attribute] = f'{check.value}'
+                attributes[get_radius_reply_name(check)] = f'{check.value}'
         elif (
             not rad_group_checks
             and RadiusGroup.objects.filter(id=new_group_id).exists()
@@ -147,7 +159,7 @@ def get_radius_attributes():
             # Unset attributes set by the previous group.
             rad_group_checks = RadiusGroupCheck.objects.filter(group_id=old_group_id)
             for check in rad_group_checks:
-                attributes[check.attribute] = ''
+                attributes[get_radius_reply_name(check)] = ''
         return attributes
 
     try:

openwisp_radius/tests/test_models.py

@@ -975,14 +975,34 @@ def test_perform_change_of_authorization_celery_task_failures(
             )
         mocked_logger.reset_mock()
 
-        self._create_nas(
-            name='127.0.0.1',
+        nas = self._create_nas(
+            name='NAS',
             organization=org,
             short_name='test',
             type='Virtual',
             secret='testing123',
         )
 
+        with self.subTest('Test NAS name does not contain IP network'):
+            perform_change_of_authorization(
+                user_id=user.id,
+                old_group_id=user_group.id,
+                new_group_id=power_user_group.id,
+            )
+            self.assertEqual(
+                mocked_logger.call_args_list[0][0][0],
+                f'Failed to parse NAS IP network for "{nas.id}" object. Skipping!',
+            )
+            self.assertEqual(
+                mocked_logger.call_args_list[1][0][0],
+                f'Failed to find RADIUS secret for "{session.unique_id}"'
+                ' RadiusAccounting object. Skipping CoA operation'
+                ' for this session.',
+            )
+        mocked_logger.reset_mock()
+
+        nas.name = '127.0.0.1'
+        nas.save()
         with self.subTest('Test RadClient encountered error while sending CoA packet'):
             perform_change_of_authorization(
                 user_id=user.id,
@@ -1026,8 +1046,8 @@ def test_change_of_authorization(self, mocked_radclient, *args):
         mocked_radclient.assert_called_with(
             {
                 'User-Name': user.username,
-                'Max-Daily-Session': '',
-                'Max-Daily-Session-Traffic': '',
+                'Session-Timeout': '',
+                'ChilliSpot-Max-Total-Octets': '',
             }
         )
         rad_acct.refresh_from_db()
@@ -1042,8 +1062,8 @@ def test_change_of_authorization(self, mocked_radclient, *args):
         mocked_radclient.assert_called_with(
             {
                 'User-Name': user.username,
-                'Max-Daily-Session': '10800',
-                'Max-Daily-Session-Traffic': '3000000000',
+                'Session-Timeout': '10800',
+                'ChilliSpot-Max-Total-Octets': '3000000000',
             }
         )
         rad_acct.refresh_from_db()