[change] Close all previous open RADIUS session #437

- Whenever a new session is started, all previous open session
from the same user and calling-station-id is closed. In the previous implementation,
only sessions with the same called-station-id were closed.
- Avoid unnecessary update query in close previous session receiver. The bulk update
  query was being executed also  when there was no open session to close.
- Use datetime.now() for closing Radius session if update_time is None

Related to #437

openwisp_radius/receivers.py

@@ -5,6 +5,7 @@
 
 from celery.exceptions import OperationalError
 from django.db import transaction
+from django.utils.timezone import now
 
 from openwisp_radius.tasks import send_login_email
 
@@ -100,21 +101,23 @@ def close_previous_radius_accounting_sessions(instance, created, **kwargs):
     if not created or not instance.called_station_id:
         return
     RadiusAccounting = load_model('RadiusAccounting')
-    closed_session = []
+    closed_sessions = []
     open_sessions = RadiusAccounting.objects.exclude(
         unique_id=instance.unique_id
     ).filter(
         stop_time__isnull=True,
-        called_station_id=instance.called_station_id,
         calling_station_id=instance.calling_station_id,
         username=instance.username,
     )
     for session in open_sessions:
-        session.stop_time = session.update_time
+        session.stop_time = session.update_time or now()
         session.terminate_cause = 'Session-Timeout'
-        closed_session.append(session)
+        closed_sessions.append(session)
+    # avoid bulk update if not necessary
+    if not closed_sessions:
+        return
     RadiusAccounting.objects.bulk_update(
-        closed_session, fields=['stop_time', 'terminate_cause']
+        closed_sessions, fields=['stop_time', 'terminate_cause']
     )
 
 

openwisp_radius/tests/test_api/test_freeradius_api.py

@@ -1293,15 +1293,17 @@ def test_accounting_filter_stop_time(self):
         item = response.data[0]
         self.assertEqual(parser.parse(item['stop_time']), ra.stop_time)
 
+    @freeze_time('2023-10-25T22:14:09+01:00')
     def test_accounting_filter_is_open(self):
         data1 = self.acct_post_data
         data1.update(dict(stop_time=None, unique_id='99144d60'))
-        self._create_radius_accounting(**data1)
+        ra = self._create_radius_accounting(**data1)
         data2 = self.acct_post_data
-        data2.update(
-            dict(stop_time='2018-03-02T00:43:24.020460+01:00', unique_id='85144d60')
-        )
-        ra = self._create_radius_accounting(**data2)
+        data2.update(dict(unique_id='85144d60'))
+        self._create_radius_accounting(**data2)
+        # The old session gets closed automatically, fetch the
+        # updated object from the database.
+        ra.refresh_from_db(fields=['stop_time'])
         response = self.client.get(
             f'{self._acct_url}?is_open=true',
             HTTP_AUTHORIZATION=self.auth_header,
@@ -1545,7 +1547,7 @@ def test_mac_addr_roaming_accounting_view(self):
             self.assertEqual(response.status_code, 200)
             self.assertEqual(response.data, None)
 
-        self._create_radius_accounting(update_time=now(), **acct_post_data)
+        self._create_radius_accounting(**acct_post_data)
 
         with self.subTest('Test mac address roaming is disabled'):
             response = response = self.client.post(

openwisp_radius/tests/test_models.py

@@ -149,19 +149,6 @@ def test_multiple_accounting_sessions(self):
             self.assertEqual(radiusaccounting1.stop_time, radiusaccounting1.update_time)
             self.assertEqual(radiusaccounting2.stop_time, None)
 
-        with self.subTest('Test different called_session_id'):
-            self.assertEqual(
-                RadiusAccounting.objects.filter(stop_time__isnull=True).count(), 1
-            )
-            radiusaccounting_options['called_station_id'] = 'AA-AA-AA-AA-AA-0B'
-            self._create_radius_accounting(
-                unique_id='113', **radiusaccounting_options, update_time=timezone.now()
-            )
-            radiusaccounting2.refresh_from_db()
-            self.assertEqual(RadiusAccounting.objects.filter(stop_time=None).count(), 2)
-            self.assertEqual(radiusaccounting2.terminate_cause, None)
-            self.assertEqual(radiusaccounting2.stop_time, None)
-
     @capture_any_output()
     @mock.patch.object(app_settings, 'OPENVPN_DATETIME_FORMAT', u'%Y-%m-%d %H:%M:%S')
     @mock.patch.object(app_settings, 'CONVERT_CALLED_STATION_ON_CREATE', True)

setup.cfg

@@ -21,4 +21,5 @@ exclude = *.egg-info,
           .git,
           ./tests/*settings*.py,
           docs/*
+        ./tests/openwisp2/saml/*
 max-line-length = 88