Skip to content

Commit 3c0c7cf

Browse files
Eeshu-YadavDhanus3133nemesifier
authored
[feature] Close stale sessions on Accounting-On #617
When NAS devices are cold rebooted, they may not close active RADIUS sessions, which can cause issues with Simultaneous-Use. For this reason, when an Accounting-On request is detected, we make sure to close any lingering open RADIUS accounting session from the same NAS. Close #617 --------- Co-authored-by: Dhanus <dhanus3133@gmail.com> Co-authored-by: Federico Capoano <f.capoano@openwisp.io>
1 parent a685a70 commit 3c0c7cf

4 files changed

Lines changed: 94 additions & 5 deletions

File tree

openwisp_radius/api/freeradius_views.py

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@
4444
"^{0}[:-]{0}[:-]{0}[:-]{0}[:-]{0}[:-]{0}".format("[a-f0-9]{2}"), re.I
4545
)
4646
_TOKEN_AUTH_FAILED = _("Token authentication failed")
47-
# Accounting-On and Accounting-Off are not implemented and
48-
# hence ignored right now - may be implemented in the future
49-
UNSUPPORTED_STATUS_TYPES = ["Accounting-On", "Accounting-Off"]
47+
# Accounting-Off is not implemented and hence ignored right now
48+
# may be implemented in the future
49+
UNSUPPORTED_STATUS_TYPES = ["Accounting-Off"]
5050
logger = logging.getLogger(__name__)
5151

5252
RadiusToken = load_model("RadiusToken")
@@ -502,7 +502,11 @@ def post(self, request, *args, **kwargs):
502502
if request.user.is_anonymous and request.auth is None:
503503
return Response(status=status.HTTP_200_OK)
504504
data = request.data.copy()
505-
if data.get("status_type", None) in UNSUPPORTED_STATUS_TYPES:
505+
status_type = data.get("status_type", None)
506+
if status_type in UNSUPPORTED_STATUS_TYPES:
507+
return Response(None)
508+
if status_type == "Accounting-On":
509+
self._handle_accounting_on(data)
506510
return Response(None)
507511
# Create or Update
508512
try:
@@ -533,6 +537,27 @@ def post(self, request, *args, **kwargs):
533537
self.send_radius_accounting_signal(serializer.validated_data)
534538
return Response(None)
535539

540+
def _handle_accounting_on(self, data):
541+
"""
542+
When NAS devices are cold rebooted,
543+
they may not close active sessions,
544+
which can cause issues with Simultaneous-Use.
545+
For this reason, OpenWISP closes any open session from
546+
the same NAS when it receives an Accounting-On request.
547+
"""
548+
called_station_id = data.get("called_station_id")
549+
closed_count = RadiusAccounting._close_stale_sessions_on_nas_boot(
550+
called_station_id=called_station_id,
551+
organization_id=self.request.auth,
552+
)
553+
if closed_count:
554+
logger.info(
555+
f"Closed {closed_count} stale session(s) for device with "
556+
f"Called-Station-Id {called_station_id} in organization "
557+
f"{self.request.auth} due to receiving an "
558+
"Accounting-On packet."
559+
)
560+
536561
def _is_interim_update_corner_case(self, error, data):
537562
"""
538563
Handles "Interim-Updates" for RadiusAccounting sessions

openwisp_radius/api/serializers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class Meta:
139139
("Start", "Start"),
140140
("Interim-Update", "Interim-Update"),
141141
("Stop", "Stop"),
142-
("Accounting-On", "Accounting-On (ignored)"),
142+
("Accounting-On", "Accounting-On"),
143143
("Accounting-Off", "Accounting-Off (ignored)"),
144144
)
145145

openwisp_radius/base/models.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,23 @@ def close_stale_sessions(cls, days=None, hours=None):
557557
session.terminate_cause = "Session Timeout"
558558
session.save()
559559

560+
@classmethod
561+
def _close_stale_sessions_on_nas_boot(cls, called_station_id, organization_id):
562+
"""
563+
Called during RADIUS Accounting-On.
564+
"""
565+
if not called_station_id or not organization_id:
566+
return 0
567+
stale_sessions = cls.objects.filter(
568+
called_station_id=called_station_id,
569+
organization_id=organization_id,
570+
stop_time__isnull=True,
571+
)
572+
closed_count = stale_sessions.update(
573+
stop_time=now(), terminate_cause="NAS-Reboot"
574+
)
575+
return closed_count
576+
560577

561578
class AbstractNas(OrgMixin, TimeStampedEditableModel):
562579
name = models.CharField(

openwisp_radius/tests/test_api/test_freeradius_api.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,7 @@ def test_accounting_when_nas_using_pfsense_started(self):
12211221
response = self.client.post(
12221222
self._acct_url,
12231223
data=json.dumps(data),
1224+
HTTP_AUTHORIZATION=self.auth_header,
12241225
content_type="application/json",
12251226
)
12261227
self.assertEqual(response.status_code, 200)
@@ -1278,6 +1279,52 @@ def test_user_accounting_list_empty_diff_organization(self):
12781279
self.assertEqual(response.status_code, 200)
12791280
self.assertEqual(len(response.json()), 0)
12801281

1282+
@freeze_time(START_DATE)
1283+
def test_accounting_on_closes_stale_sessions(self):
1284+
stale_user = self._create_user(username="stale_user", email="stale@user.com")
1285+
self._create_org_user(user=stale_user)
1286+
stale_session_data = self.acct_post_data.copy()
1287+
stale_session_data.update(
1288+
{
1289+
"unique_id": "stale-session-1",
1290+
"called_station_id": "AA-BB-CC-DD-EE-FF",
1291+
"username": stale_user.username,
1292+
"stop_time": None,
1293+
}
1294+
)
1295+
stale_session = self._create_radius_accounting(**stale_session_data)
1296+
self.assertIsNone(stale_session.stop_time)
1297+
other_user = self._create_user(username="other_user", email="other@user.com")
1298+
self._create_org_user(user=other_user)
1299+
other_session_data = self.acct_post_data.copy()
1300+
other_session_data.update(
1301+
{
1302+
"unique_id": "other-session-1",
1303+
"called_station_id": "11-22-33-44-55-66",
1304+
"username": other_user.username,
1305+
"stop_time": None,
1306+
}
1307+
)
1308+
other_session = self._create_radius_accounting(**other_session_data)
1309+
self.assertIsNone(other_session.stop_time)
1310+
self.assertEqual(RadiusAccounting.objects.count(), 2)
1311+
# Simulate Accounting-On packet from the first NAS
1312+
accounting_on_data = {
1313+
"status_type": "Accounting-On",
1314+
"called_station_id": "AA-BB-CC-DD-EE-FF",
1315+
"unique_id": "accounting-on-packet-id",
1316+
}
1317+
response = self.post_json(accounting_on_data)
1318+
self.assertEqual(response.status_code, 200)
1319+
self.assertIsNone(response.data)
1320+
self.assertEqual(RadiusAccounting.objects.count(), 2)
1321+
stale_session.refresh_from_db()
1322+
self.assertIsNotNone(stale_session.stop_time)
1323+
self.assertEqual(stale_session.terminate_cause, "NAS-Reboot")
1324+
# Session from other NAS unaffected
1325+
other_session.refresh_from_db()
1326+
self.assertIsNone(other_session.stop_time)
1327+
12811328

12821329
class TestTransactionFreeradiusApi(
12831330
AcctMixin,

0 commit comments

Comments
 (0)