[chores] Handled organization agnostic Accounting-On requests #617

Accounting-On/Off requests may not be tied to a specific organization.
Before we were just ignoring these requests.
Now we check the IP is coming from one of authorized IP addresses,
Accounting-Off requests are still ignored but Accounting-On
requests will close stale sessions, regardless of whether
the request is tied to a specific organization or not.

Related to #617

Author: nemesifier

openwisp_radius/api/freeradius_views.py

@@ -46,7 +46,8 @@
 _TOKEN_AUTH_FAILED = _("Token authentication failed")
 # Accounting-Off is not implemented and hence ignored right now
 # may be implemented in the future
-UNSUPPORTED_STATUS_TYPES = ["Accounting-Off"]
+# Accounting-On is used to close stale sessions
+SPECIAL_STATUS_TYPES = ["Accounting-On", "Accounting-Off"]
 logger = logging.getLogger(__name__)
 
 RadiusToken = load_model("RadiusToken")
@@ -78,16 +79,17 @@ class Meta:
 
 
 class FreeradiusApiAuthentication(BaseAuthentication):
-    def _get_ip_list(self, uuid):
-        if f"ip-{uuid}" in cache:
+    def _get_ip_list(self, uuid=None):
+        ip_list = None
+        if uuid and f"ip-{uuid}" in cache:
             ip_list = cache.get(f"ip-{uuid}")
-        else:
+        elif uuid:
             try:
                 ip_list = OrganizationRadiusSettings.objects.get(
                     organization__pk=uuid
                 ).freeradius_allowed_hosts_list
             except OrganizationRadiusSettings.DoesNotExist:
-                ip_list = None
+                pass
             else:
                 cache.set(f"ip-{uuid}", ip_list)
         return ip_list or app_settings.FREERADIUS_ALLOWED_HOSTS
@@ -168,8 +170,8 @@ def authenticate(self, request):
         self.check_organization(request)
         uuid, token = self.get_uuid_token(request)
         if not uuid and not token:
-            if request.data.get("status_type", None) in UNSUPPORTED_STATUS_TYPES:
-                return
+            if request.data.get("status_type", None) in SPECIAL_STATUS_TYPES:
+                return self._check_client_ip_and_return(request, uuid)
             username = request.data.get("username") or request.query_params.get(
                 "username"
             )
@@ -499,15 +501,15 @@ def post(self, request, *args, **kwargs):
         does not return any JSON response so that freeradius will avoid
         processing the response without generating warnings
         """
-        if request.user.is_anonymous and request.auth is None:
-            return Response(status=status.HTTP_200_OK)
         data = request.data.copy()
         status_type = data.get("status_type", None)
-        if status_type in UNSUPPORTED_STATUS_TYPES:
-            return Response(None)
-        if status_type == "Accounting-On":
-            self._handle_accounting_on(data)
-            return Response(None)
+        # Special Cases
+        if (
+            request.user.is_anonymous and request.auth is None
+        ) or status_type in SPECIAL_STATUS_TYPES:
+            if status_type == "Accounting-On":
+                self._handle_accounting_on(data)
+            return Response(status=status.HTTP_200_OK)
         # Create or Update
         try:
             instance = self.get_queryset().get(unique_id=data.get("unique_id"))
@@ -547,8 +549,7 @@ def _handle_accounting_on(self, data):
         """
         called_station_id = data.get("called_station_id")
         closed_count = RadiusAccounting._close_stale_sessions_on_nas_boot(
-            called_station_id=called_station_id,
-            organization_id=self.request.auth,
+            called_station_id=called_station_id
         )
         if closed_count:
             logger.info(

openwisp_radius/base/models.py

@@ -558,15 +558,14 @@ def close_stale_sessions(cls, days=None, hours=None):
             session.save()
 
     @classmethod
-    def _close_stale_sessions_on_nas_boot(cls, called_station_id, organization_id):
+    def _close_stale_sessions_on_nas_boot(cls, called_station_id):
         """
         Called during RADIUS Accounting-On.
         """
-        if not called_station_id or not organization_id:
+        if not called_station_id:
             return 0
         stale_sessions = cls.objects.filter(
             called_station_id=called_station_id,
-            organization_id=organization_id,
             stop_time__isnull=True,
         )
         closed_count = stale_sessions.update(

openwisp_radius/tests/test_api/test_freeradius_api.py

@@ -1221,7 +1221,6 @@ def test_accounting_when_nas_using_pfsense_started(self):
         response = self.client.post(
             self._acct_url,
             data=json.dumps(data),
-            HTTP_AUTHORIZATION=self.auth_header,
             content_type="application/json",
         )
         self.assertEqual(response.status_code, 200)
@@ -1311,10 +1310,29 @@ def test_accounting_on_closes_stale_sessions(self):
         # Simulate Accounting-On packet from the first NAS
         accounting_on_data = {
             "status_type": "Accounting-On",
+            "session_id": "c2bc87808f568e3c",
+            "unique_id": "2d124bdc1d269430629970d5f0bb7113",
+            "username": "",
+            "realm": "",
+            "nas_ip_address": "192.168.0.4",
+            "nas_port_id": "0",
+            "nas_port_type": "",
+            "session_time": "",
+            "authentication": "",
+            "input_octets": "",
+            "output_octets": "",
             "called_station_id": "AA-BB-CC-DD-EE-FF",
-            "unique_id": "accounting-on-packet-id",
+            "calling_station_id": "",
+            "terminate_cause": "",
+            "service_type": "",
+            "framed_protocol": "",
+            "framed_ip_address": "",
         }
-        response = self.post_json(accounting_on_data)
+        response = self.client.post(
+            self._acct_url,
+            data=json.dumps(accounting_on_data),
+            content_type="application/json",
+        )
         self.assertEqual(response.status_code, 200)
         self.assertIsNone(response.data)
         self.assertEqual(RadiusAccounting.objects.count(), 2)