[feature] Added CIDR and subnet support to FREERADIUS_ALLOWED_HOSTS #229

docs/user/settings.rst

@@ -190,6 +190,10 @@ You can use subnets when freeradius is hosted on a variable IP, e.g.:
 - ``198.168.0.0/24`` to allow the entire LAN.
 - ``0.0.0.0/0`` to allow any address (useful for development / testing).
 
+Entries with host bits set (e.g. ``172.18.0.5/16``) and extra whitespace
+around comma-separated entries in the organization settings are handled
+gracefully.
+
 This value can be overridden per organization in the organization change
 page. You can skip setting this option if you intend to set it from
 organization change page for each organization.

openwisp_radius/api/freeradius_views.py

@@ -106,7 +106,9 @@ def _check_client_ip_and_return(self, request, uuid):
 
         for ip in ip_list:
             try:
-                if ipaddress.ip_address(client_ip) in ipaddress.ip_network(ip):
+                if ipaddress.ip_address(client_ip) in ipaddress.ip_network(
+                    (ip or "").strip(), strict=False
+                ):
                     return (AnonymousUser(), uuid)
             except ValueError:
                 invalid_addr_message = _(

openwisp_radius/base/models.py

@@ -1446,9 +1446,11 @@ def __str__(self):
 
     @property
     def freeradius_allowed_hosts_list(self):
-        addresses = []
-        if self.freeradius_allowed_hosts:
-            addresses = self.freeradius_allowed_hosts.split(",")
+        addresses = [
+            (ip or "").strip()
+            for ip in (self.freeradius_allowed_hosts or "").split(",")
+            if (ip or "").strip()
+        ]
         return addresses
 
     @property
@@ -1492,13 +1494,15 @@ def _clean_freeradius_allowed_hosts(self):
             else:
                 try:
                     for ip_address in allowed_hosts_set:
-                        ipaddress.ip_network(ip_address)
+                        ip_str = (ip_address or "").strip()
+                        if ip_str:
+                            ipaddress.ip_network(ip_str, strict=False)
                 except ValueError:
                     raise ValidationError(
                         {
                             "freeradius_allowed_hosts": _(
                                 "Invalid input. Please enter valid ip addresses "
-                                "or subnets separated by comma. (no spaces)"
+                                "or subnets separated by comma."
                             )
                         }
                     )

openwisp_radius/tests/test_admin.py

@@ -582,7 +582,7 @@ def test_organization_radsettings_freeradius_allowed_hosts(self):
                 response,
                 _(
                     "Invalid input. Please enter valid ip addresses "
-                    "or subnets separated by comma. (no spaces)"
+                    "or subnets separated by comma."
                 ),
             )
 

openwisp_radius/tests/test_api/test_freeradius_api.py

@@ -2480,6 +2480,47 @@ def test_ip_from_radsetting_invalid(self):
         self.assertEqual(response.status_code, 403)
         self.assertEqual(response.data["detail"], test_fail_msg)
 
+    def test_ip_from_radsetting_cidr_range_valid(self):
+        with mock.patch(self.freeradius_hosts_path, []):
+            radsetting = OrganizationRadiusSettings.objects.get(
+                organization=self._get_org()
+            )
+            radsetting.freeradius_allowed_hosts = "172.18.0.0/16"
+            radsetting.save()
+
+        with mock.patch(
+            "openwisp_radius.api.freeradius_views.get_client_ip",
+            return_value=("172.18.0.10", True),
+        ):
+            response = self.client.post(reverse("radius:authorize"), self.params)
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 200)
+        self.assertIn("control:Auth-Type", response.data)
+        self.assertEqual(response.data["control:Auth-Type"], "Accept")
+
+    @capture_any_output()
+    def test_ip_outside_cidr_range_rejected(self):
+        with mock.patch(self.freeradius_hosts_path, []):
+            radsetting = OrganizationRadiusSettings.objects.get(
+                organization=self._get_org()
+            )
+            radsetting.freeradius_allowed_hosts = "172.18.0.0/16"
+            radsetting.save()
+
+        with mock.patch(
+            "openwisp_radius.api.freeradius_views.get_client_ip",
+            return_value=("10.0.0.5", True),
+        ):
+            response = self.client.post(reverse("radius:authorize"), self.params)
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(
+            response.data["detail"],
+            "Request rejected: Client IP address (10.0.0.5) is not in "
+            "the list of IP addresses allowed to consume the freeradius API.",
+        )
+
 
 class TestTransactionClientIpApi(
     TestClientIpApiMixin, ApiTokenMixin, BaseTransactionTestCase
@@ -2519,6 +2560,67 @@ def test_ip_from_radsetting_valid(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, _AUTH_TYPE_ACCEPT_RESPONSE)
 
+    def test_ip_from_radsetting_cidr_range_valid(self):
+        with mock.patch(self.freeradius_hosts_path, []):
+            radsetting = OrganizationRadiusSettings.objects.get(
+                organization=self._get_org()
+            )
+        radsetting.freeradius_allowed_hosts = "172.18.0.0/16"
+        radsetting.save()
+
+        with mock.patch(
+            "openwisp_radius.api.freeradius_views.get_client_ip",
+            return_value=("172.18.0.10", True),
+        ):
+            response = self.client.post(reverse("radius:authorize"), self.params)
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data, _AUTH_TYPE_ACCEPT_RESPONSE)
+
+    def test_ip_from_radsetting_spaces_and_host_bits_valid(self):
+        org = self._get_org()
+        with mock.patch(self.freeradius_hosts_path, []):
+            radsetting = OrganizationRadiusSettings.objects.get(organization=org)
+
+        radsetting.freeradius_allowed_hosts = "127.0.0.1, 172.18.0.5/16"
+        radsetting.save()
+
+        self.assertEqual(
+            cache.get(f"ip-{org.pk}"),
+            ["127.0.0.1", "172.18.0.5/16"],
+        )
+
+        with mock.patch(
+            "openwisp_radius.api.freeradius_views.get_client_ip",
+            return_value=("172.18.0.10", True),
+        ):
+            response = self.client.post(reverse("radius:authorize"), self.params)
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data, _AUTH_TYPE_ACCEPT_RESPONSE)
+
+    @capture_any_output()
+    def test_ip_outside_cidr_range_rejected(self):
+        with mock.patch(self.freeradius_hosts_path, []):
+            radsetting = OrganizationRadiusSettings.objects.get(
+                organization=self._get_org()
+            )
+        radsetting.freeradius_allowed_hosts = "172.18.0.0/16"
+        radsetting.save()
+
+        with mock.patch(
+            "openwisp_radius.api.freeradius_views.get_client_ip",
+            return_value=("10.0.0.5", True),
+        ):
+            response = self.client.post(reverse("radius:authorize"), self.params)
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(
+            response.data["detail"],
+            "Request rejected: Client IP address (10.0.0.5) is not in "
+            "the list of IP addresses allowed to consume the freeradius API.",
+        )
+
     def test_ip_from_setting_valid(self):
         response = self.client.post(reverse("radius:authorize"), self.params)
         self.assertEqual(response.status_code, 200)