Skip to content

Commit

Permalink
[fix] Set called_station_id max-length to 253 chars #467
Browse files Browse the repository at this point in the history
- Set max_length attribute of called_station_id to 253 according to best practices
- Ensured serializers enforce this constraint in the API to avoid uncaught exceptions

Fixes #467
  • Loading branch information
dee077 authored Jan 21, 2025
1 parent 6e2dada commit 02c05dd
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 6 deletions.
2 changes: 0 additions & 2 deletions openwisp_radius/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ class RadiusPostAuthSerializer(serializers.ModelSerializer):
allow_blank=True,
style={'input_type': 'password'},
)
called_station_id = serializers.CharField(required=False, allow_blank=True)
calling_station_id = serializers.CharField(required=False, allow_blank=True)

def validate(self, data):
# do not save correct passwords in clear text
Expand Down
8 changes: 4 additions & 4 deletions openwisp_radius/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,15 +434,15 @@ class AbstractRadiusAccounting(OrgMixin, models.Model):
)
called_station_id = models.CharField(
verbose_name=_('called station ID'),
max_length=50,
max_length=253,
db_column='calledstationid',
db_index=True,
blank=True,
null=True,
)
calling_station_id = models.CharField(
verbose_name=_('calling station ID'),
max_length=50,
max_length=253,
db_column='callingstationid',
db_index=True,
blank=True,
Expand Down Expand Up @@ -791,14 +791,14 @@ class AbstractRadiusPostAuth(OrgMixin, UUIDModel):
reply = models.CharField(verbose_name=_('reply'), max_length=32)
called_station_id = models.CharField(
verbose_name=_('called station ID'),
max_length=50,
max_length=253,
db_column='calledstationid',
blank=True,
null=True,
)
calling_station_id = models.CharField(
verbose_name=_('calling station ID'),
max_length=50,
max_length=253,
db_column='callingstationid',
blank=True,
null=True,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Generated by Django 4.2.17 on 2024-12-23 15:24

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("openwisp_radius", "0038_clean_fallbackfields"),
]

operations = [
migrations.AlterField(
model_name="radiusaccounting",
name="called_station_id",
field=models.CharField(
blank=True,
db_column="calledstationid",
db_index=True,
max_length=253,
null=True,
verbose_name="called station ID",
),
),
migrations.AlterField(
model_name="radiusaccounting",
name="calling_station_id",
field=models.CharField(
blank=True,
db_column="callingstationid",
db_index=True,
max_length=253,
null=True,
verbose_name="calling station ID",
),
),
migrations.AlterField(
model_name="radiuspostauth",
name="called_station_id",
field=models.CharField(
blank=True,
db_column="calledstationid",
max_length=253,
null=True,
verbose_name="called station ID",
),
),
migrations.AlterField(
model_name="radiuspostauth",
name="calling_station_id",
field=models.CharField(
blank=True,
db_column="callingstationid",
max_length=253,
null=True,
verbose_name="calling station ID",
),
),
]
19 changes: 19 additions & 0 deletions openwisp_radius/tests/test_api/test_freeradius_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,25 @@ def test_postauth_400(self):
self.assertEqual(RadiusPostAuth.objects.all().count(), 0)
self.assertEqual(response.status_code, 400)

def test_postauth_station_id_max_length_253_exceed_400(self):
params = {
'called_station_id': 'C0-4A-00-EE-D1-0D:' + 'A' * 253,
'calling_station_id': '00:26:b9:20:5f:10' + 'A' * 253,
}
params = self._get_postauth_params(**params)
response = self.client.post(
reverse('radius:postauth'), params, HTTP_AUTHORIZATION=self.auth_header
)
self.assertEqual(response.status_code, 400)
self.assertEqual(
response.data['called_station_id'][0],
'Ensure this field has no more than 253 characters.',
)
self.assertEqual(
response.data['calling_station_id'][0],
'Ensure this field has no more than 253 characters.',
)

@capture_any_output()
def test_postauth_no_token_403(self):
response = self.client.post(reverse('radius:postauth'), {'username': 'tester'})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Generated by Django 4.2.17 on 2025-01-08 15:05

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
(
"sample_radius",
"0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more",
),
]

operations = [
migrations.AlterField(
model_name="radiusaccounting",
name="called_station_id",
field=models.CharField(
blank=True,
db_column="calledstationid",
db_index=True,
max_length=253,
null=True,
verbose_name="called station ID",
),
),
migrations.AlterField(
model_name="radiusaccounting",
name="calling_station_id",
field=models.CharField(
blank=True,
db_column="callingstationid",
db_index=True,
max_length=253,
null=True,
verbose_name="calling station ID",
),
),
migrations.AlterField(
model_name="radiuspostauth",
name="called_station_id",
field=models.CharField(
blank=True,
db_column="calledstationid",
max_length=253,
null=True,
verbose_name="called station ID",
),
),
migrations.AlterField(
model_name="radiuspostauth",
name="calling_station_id",
field=models.CharField(
blank=True,
db_column="callingstationid",
max_length=253,
null=True,
verbose_name="calling station ID",
),
),
]

0 comments on commit 02c05dd

Please sign in to comment.