[feature] Added Called/Calling-Station-Id to authorize API to fix Simultaneous-Use #648

Closes #648

---------

Co-authored-by: Federico Capoano <f.capoano@openwisp.io>
Co-authored-by: Gagan Deep <pandafy.dev@gmail.com>

Author: 3 people

docs/deploy/freeradius.rst

@@ -140,7 +140,7 @@ for the available configuration values.
         uri = "${..connect_uri}/api/v1/freeradius/authorize/"
         method = 'post'
         body = 'json'
-        data = '{"username": "%{User-Name}", "password": "%{User-Password}"}'
+        data = '{"username": "%{User-Name}", "password": "%{User-Password}", "called_station_id": "%{Called-Station-ID}", "calling_station_id": "%{Calling-Station-ID}"}'
         tls = ${..tls}
     }
 

openwisp_radius/api/freeradius_views.py

@@ -268,9 +268,16 @@ def post(self, request, *args, **kwargs):
         serializer.is_valid(raise_exception=True)
         username = serializer.validated_data.get("username")
         password = serializer.validated_data.get("password")
+        called_station_id = serializer.validated_data.get("called_station_id")
+        calling_station_id = serializer.validated_data.get("calling_station_id")
         user = self.get_user(request, username, password)
         if user and self.authenticate_user(request, user, password):
-            data, status = self.get_replies(user, organization_id=request.auth)
+            data, status = self.get_replies(
+                user,
+                organization_id=request.auth,
+                called_station_id=called_station_id,
+                calling_station_id=calling_station_id,
+            )
             return Response(data, status=status)
         if app_settings.API_AUTHORIZE_REJECT:
             return Response(self.reject_attributes, status=self.reject_status)
@@ -296,7 +303,9 @@ def get_user(self, request, username, password):
             return user
         return None
 
-    def get_replies(self, user, organization_id):
+    def get_replies(
+        self, user, organization_id, called_station_id=None, calling_station_id=None
+    ):
         """
         Returns user group replies and executes counter checks
         """
@@ -312,7 +321,12 @@ def get_replies(self, user, organization_id):
 
             # Validate simultaneous use
             simultaneous_use = self._check_simultaneous_use(
-                data, user, group_checks, organization_id
+                data,
+                user,
+                group_checks,
+                organization_id,
+                called_station_id=called_station_id,
+                calling_station_id=calling_station_id,
             )
             if simultaneous_use is not None:
                 return simultaneous_use
@@ -326,7 +340,15 @@ def get_replies(self, user, organization_id):
 
         return data, self.accept_status
 
-    def _check_simultaneous_use(self, data, user, group_checks, organization_id):
+    def _check_simultaneous_use(
+        self,
+        data,
+        user,
+        group_checks,
+        organization_id,
+        called_station_id=None,
+        calling_station_id=None,
+    ):
         """
         Check if user has exceeded simultaneous use limit
 
@@ -338,11 +360,22 @@ def _check_simultaneous_use(self, data, user, group_checks, organization_id):
             # Exit early if the `Simultaneous-Use` check is not defined
             # in the RadiusGroup or if it permits unlimited concurrent sessions.
             return None
+
         open_sessions = RadiusAccounting.objects.filter(
             username=user.username,
             organization_id=organization_id,
             stop_time__isnull=True,
-        ).count()
+        )
+        # In some corner cases, RADIUS accounting sessions
+        # can remain open even though the user is not authenticated
+        # on the NAS anymore, for this reason, we shall allow re-authentication
+        # of the same client on the same NAS.
+        if called_station_id and calling_station_id:
+            open_sessions = open_sessions.exclude(
+                called_station_id=called_station_id,
+                calling_station_id=calling_station_id,
+            )
+        open_sessions = open_sessions.count()
         if open_sessions >= max_simultaneous:
             data.update(self.reject_attributes.copy())
             if "Reply-Message" not in data:

openwisp_radius/api/serializers.py

@@ -111,6 +111,12 @@ class AuthorizeSerializer(serializers.Serializer):
         max_length=User._meta.get_field("username").max_length, write_only=True
     )
     password = serializers.CharField(style={"input_type": "password"}, write_only=True)
+    called_station_id = serializers.CharField(
+        max_length=253, required=False, allow_blank=False, write_only=True
+    )
+    calling_station_id = serializers.CharField(
+        max_length=253, required=False, allow_blank=False, write_only=True
+    )
 
 
 class RadiusPostAuthSerializer(serializers.ModelSerializer):

openwisp_radius/tests/mixins.py

@@ -187,17 +187,17 @@ def _radius_batch_prefix_data(self, **kwargs):
         options.update(**kwargs)
         return options
 
-    def _authorize_user(self, username="tester", password="tester", auth_header=None):
+    def _authorize_user(
+        self, username="tester", password="tester", extra_payload=None, auth_header=None
+    ):
+        extra_payload = extra_payload or {}
+        opts = {
+            "path": reverse("radius:authorize"),
+            "data": {"username": username, "password": password, **extra_payload},
+        }
         if auth_header:
-            return self.client.post(
-                reverse("radius:authorize"),
-                {"username": username, "password": password},
-                HTTP_AUTHORIZATION=auth_header,
-            )
-        return self.client.post(
-            reverse("radius:authorize"),
-            {"username": username, "password": password},
-        )
+            opts["HTTP_AUTHORIZATION"] = auth_header
+        return self.client.post(**opts)
 
     def _login_and_obtain_auth_token(
         self, username="tester", password="tester", organization=None

openwisp_radius/tests/test_api/test_freeradius_api.py

@@ -1561,6 +1561,52 @@ def test_authorize_simultaneous_use(self):
                 {"op": "=", "value": "240"},
             )
 
+        simultaneous_use_check.refresh_from_db(fields=["value"])
+        with self.subTest("Same device reauth on same NAS is allowed"):
+            # User already has one open session (at the limit of 1)
+            self.assertEqual(simultaneous_use_check.value, "1")
+            self.assertEqual(
+                RadiusAccounting.objects.filter(
+                    username=user.username, stop_time=None
+                ).count(),
+                1,
+            )
+
+            # Re-authentication from the same device and NAS should be allowed
+            response = self._authorize_user(
+                auth_header=self.auth_header,
+                extra_payload={
+                    "called_station_id": self.acct_post_data["called_station_id"],
+                    "calling_station_id": self.acct_post_data["calling_station_id"],
+                },
+            )
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.data["control:Auth-Type"], "Accept")
+
+        simultaneous_use_check.refresh_from_db(fields=["value"])
+        with self.subTest("Authorization from different device on same NAS is denied"):
+            # User already has one open session (at the limit of 1)
+            self.assertEqual(simultaneous_use_check.value, "1")
+            self.assertEqual(
+                RadiusAccounting.objects.filter(
+                    username=user.username, stop_time=None
+                ).count(),
+                1,
+            )
+
+            response = self._authorize_user(
+                auth_header=self.auth_header,
+                extra_payload={
+                    "calling_station_id": "11:22:33:44:55:66",
+                    "called_station_id": self.acct_post_data["called_station_id"],
+                },
+            )
+            self.assertEqual(response.status_code, 401)
+            self.assertEqual(
+                response.data["control:Auth-Type"],
+                "Reject",
+            )
+
         with self.subTest("Closed sessions are ignored"):
             # Keep limit at 1 and Close all previously open sessions
             RadiusAccounting.objects.filter(stop_time=None).update(
@@ -1733,7 +1779,9 @@ def test_authorize_with_user_auth(self):
             self._test_authorize_with_user_auth_helper(user.phone_number, "tester")
 
         with self.subTest("Test authorization failure"):
-            r = self._authorize_user("thisuserdoesnotexist", "tester", self.auth_header)
+            r = self._authorize_user(
+                "thisuserdoesnotexist", "tester", auth_header=self.auth_header
+            )
             self.assertEqual(r.status_code, 200)
             self.assertEqual(r.data, None)