Skip to content

Commit

Permalink
[fix] Fix accounting stop handling for empty octets
Browse files Browse the repository at this point in the history
Certain NAS devices, such as hostapd, may omit input/output octets
in accounting-stop requests. Consequently, FreeRADIUS represents
these fields as empty strings in API requests to this API. Django
REST Framework's validation rejects these requests as the fields
are defined as Integer fields and do not accept empty strings.

This patch addresses this issue by gracefully handling empty strings,
making the API more tolerant of varying system behaviors and
enhancing compatibility across heterogeneous systems.
  • Loading branch information
nemesifier committed Apr 6, 2024
1 parent 1a7cb9a commit 6a2d988
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 6 deletions.
27 changes: 21 additions & 6 deletions openwisp_radius/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,13 @@ class RadiusAccountingSerializer(serializers.ModelSerializer):
framed_ipv6_address = serializers.IPAddressField(
required=False, allow_blank=True, protocol='IPv6'
)
session_time = serializers.IntegerField(required=False, default=0)
session_time = serializers.IntegerField(required=False)
stop_time = serializers.DateTimeField(required=False)
update_time = serializers.DateTimeField(required=False)
input_octets = serializers.IntegerField(required=False, default=0)
output_octets = serializers.IntegerField(required=False, default=0)
# this is needed otherwise serialize will ignore status_type from accounting packet
# as it's not a model field
input_octets = serializers.IntegerField(required=False)
output_octets = serializers.IntegerField(required=False)
# this is needed otherwise serializer will ignore status_type
# from the accounting request because it's not a model field
status_type = serializers.ChoiceField(
write_only=True, required=True, choices=STATUS_TYPE_CHOICES
)
Expand Down Expand Up @@ -191,9 +191,24 @@ def is_valid(self, raise_exception=False):
raise error

def run_validation(self, data):
"""
Custom validation to handle empty strings in:
- session_time
- input_octets
- output_octets
"""
for field in ['session_time', 'input_octets', 'output_octets']:
if data.get('status_type', None) == 'Start' and data[field] == '':
# missing data in accounting start,
# let's set zero as default value
if data.get('status_type', None) == 'Start' and data.get(field) == '':
data[field] = 0
# missing data in accounting stop,
# let's remove the empty string to
# prevent the API from failing
# the existing values stored in previous
# interim-updates won't be changed
if data.get('status_type', None) == 'Stop' and data.get(field) == '':
del data[field]
return super().run_validation(data)

def validate(self, data):
Expand Down
35 changes: 35 additions & 0 deletions openwisp_radius/tests/test_api/test_freeradius_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,41 @@ def test_accounting_stop_200(self):
self.assertEqual(ra.start_time, start_time)
self.assertAcctData(ra, data)

@freeze_time(START_DATE)
def test_accounting_stop_empty_octets(self):
self.assertEqual(RadiusAccounting.objects.count(), 0)
data = self.acct_post_data
data.update(
dict(
input_octets=9900909,
output_octets=1513075509,
)
)
ra = self._create_radius_accounting(**data)
ra.refresh_from_db()
start_time = ra.start_time
data = self.acct_post_data
data.update(
{
'status_type': 'Stop',
'terminate_cause': '',
'input_octets': '',
'output_octets': '',
}
)
data = self._get_accounting_params(**data)
response = self.post_json(data)
self.assertEqual(response.status_code, 200)
self.assertIsNone(response.data)
self.assertEqual(RadiusAccounting.objects.count(), 1)
ra.refresh_from_db()
self.assertEqual(ra.update_time.timetuple(), now().timetuple())
self.assertEqual(ra.stop_time.timetuple(), now().timetuple())
self.assertEqual(ra.start_time, start_time)
self.assertEqual(ra.input_octets, 9900909)
self.assertEqual(ra.output_octets, 1513075509)
self.assertEqual(ra.session_time, 261)

@freeze_time(START_DATE)
@capture_any_output()
def test_accounting_stop_201(self):
Expand Down

0 comments on commit 6a2d988

Please sign in to comment.