Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions openwisp_radius/base/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import csv
import ipaddress
import json
import logging
import os
import string
Expand All @@ -17,13 +16,13 @@
from django.core.cache import cache
from django.core.exceptions import ObjectDoesNotExist, ValidationError
from django.core.mail import send_mail
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models, transaction
from django.db.models import ProtectedError, Q
from django.db.models import JSONField, ProtectedError, Q
from django.utils import timezone
from django.utils.crypto import get_random_string
from django.utils.timezone import now
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from model_utils.fields import AutoLastModifiedField
from openwisp_notifications.signals import notify
from phonenumber_field.modelfields import PhoneNumberField
Expand Down Expand Up @@ -928,6 +927,7 @@ class AbstractRadiusBatch(OrgMixin, TimeStampedEditableModel):
null=True,
blank=True,
verbose_name="PDF",
encoder=DjangoJSONEncoder,
)
expiration_date = models.DateField(
verbose_name=_("expiration date"),
Expand Down Expand Up @@ -1023,7 +1023,7 @@ def prefix_add(self, prefix, n, password_length=BATCH_DEFAULT_PASSWORD_LENGTH):
for user in users_list:
user.full_clean()
self.save_user(user)
self.user_credentials = json.dumps(user_credentials)
self.user_credentials = user_credentials
self.full_clean()
self.save()

Expand Down Expand Up @@ -1247,6 +1247,7 @@ class AbstractOrganizationRadiusSettings(UUIDModel):
" (optional, leave blank if unsure)"
),
verbose_name=_("SMS meta data"),
encoder=DjangoJSONEncoder,
)
freeradius_allowed_hosts = FallbackTextField(
help_text=_GET_IP_LIST_HELP_TEXT,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 5.2.5 on 2025-10-22 18:34

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("openwisp_radius", "0042_set_existing_batches_completed"),
]

operations = [
migrations.AlterField(
model_name="organizationradiussettings",
name="sms_meta_data",
field=models.JSONField(
blank=True,
help_text=(
"Additional configuration for SMS backend in JSON format "
"(optional, leave blank if unsure)"
),
null=True,
verbose_name="SMS meta data",
),
),
migrations.AlterField(
model_name="radiusbatch",
name="user_credentials",
field=models.JSONField(blank=True, null=True, verbose_name="PDF"),
),
]
70 changes: 70 additions & 0 deletions openwisp_radius/migrations/0044_convert_user_credentials_data.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Generated by Django 5.2.9 on 2026-02-13 18:15

import json
import logging

import django.core.serializers.json
from django.db import migrations, models

logger = logging.getLogger(__name__)


def convert_user_credentials_data(apps, schema_editor):
"""
Convert existing double-encoded JSON strings in user_credentials field
to proper JSON objects for Django's built-in JSONField.
"""
db_alias = schema_editor.connection.alias
RadiusBatch = apps.get_model("openwisp_radius", "RadiusBatch")
for batch in (
RadiusBatch.objects.using(db_alias)
.exclude(user_credentials__isnull=True)
.iterator()
):
if isinstance(batch.user_credentials, str):
try:
batch.user_credentials = json.loads(batch.user_credentials)
batch.save(using=db_alias, update_fields=["user_credentials"])
except Exception as e:
logger.exception(f"Encountered error while processing {batch}: {e}")
print(f"Encountered error while processing {batch}: {e}")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: Logic error - batch.save() is called even when json.loads() fails. When the exception is caught, batch.user_credentials still contains the unconverted string, which gets saved back to the database. Move batch.save() inside the try block to ensure only successfully converted data is saved.

Suggested change
print(f"Encountered error while processing {batch}: {e}")
if isinstance(batch.user_credentials, str):
try:
batch.user_credentials = json.loads(batch.user_credentials)
batch.save(using=db_alias, update_fields=["user_credentials"])
except Exception as e:
logger.exception(f"Encountered error while processing {batch}: {e}")
print(f"Encountered error while processing {batch}: {e}")



class Migration(migrations.Migration):

dependencies = [
(
"openwisp_radius",
"0043_alter_organizationradiussettings_sms_meta_data_and_more",
),
]

operations = [
migrations.AlterField(
model_name="organizationradiussettings",
name="sms_meta_data",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
help_text=(
"Additional configuration for SMS backend in JSON format "
"(optional, leave blank if unsure)"
),
null=True,
verbose_name="SMS meta data",
),
),
migrations.AlterField(
model_name="radiusbatch",
name="user_credentials",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
verbose_name="PDF",
),
),
migrations.RunPython(
convert_user_credentials_data, reverse_code=migrations.RunPython.noop
),
]
53 changes: 53 additions & 0 deletions openwisp_radius/tests/test_migrations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import importlib
import json
from unittest.mock import MagicMock

from django.db import connection
from django.test import TestCase

from openwisp_radius.models import RadiusBatch

migration_module = importlib.import_module(
"openwisp_radius.migrations.0044_convert_user_credentials_data"
)
convert_user_credentials_data = migration_module.convert_user_credentials_data


class Test0044Migration(TestCase):
def test_convert_user_credentials_data(self):
batch = RadiusBatch.objects.create(
name="test_batch_migration", strategy="prefix", prefix="test"
)
RadiusBatch.objects.filter(pk=batch.pk).update(
user_credentials=json.dumps({"user1": "pass1"})
)

apps = MagicMock()
apps.get_model.return_value = RadiusBatch

schema_editor = MagicMock()
schema_editor.connection = connection

convert_user_credentials_data(apps, schema_editor)

batch.refresh_from_db()
self.assertEqual(batch.user_credentials, {"user1": "pass1"})

def test_convert_user_credentials_data_invalid_json(self):
batch = RadiusBatch.objects.create(
name="test_batch_invalid", strategy="prefix", prefix="test2"
)
RadiusBatch.objects.filter(pk=batch.pk).update(
user_credentials="invalid_json_string"
)

apps = MagicMock()
apps.get_model.return_value = RadiusBatch

schema_editor = MagicMock()
schema_editor.connection = connection

convert_user_credentials_data(apps, schema_editor)

batch.refresh_from_db()
self.assertEqual(batch.user_credentials, "invalid_json_string")
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"weasyprint>=65,<68",
"dj-rest-auth>=6.0,<7.2",
"django-sendsms~=0.5.0",
"jsonfield~=3.1.0",
"django-private-storage~=3.1.0",
"django-ipware>=5.0,<7.1",
"pyrad~=2.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import django.core.validators
import django.db.models.deletion
import django.utils.timezone
import jsonfield.fields
import model_utils.fields
import private_storage.fields
import private_storage.storage.files
Expand Down Expand Up @@ -543,7 +542,7 @@ class Migration(migrations.Migration):
),
(
"sms_meta_data",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
help_text=(
"Additional configuration for SMS backend in JSON format"
Expand Down Expand Up @@ -695,9 +694,7 @@ class Migration(migrations.Migration):
),
(
"user_credentials",
jsonfield.fields.JSONField(
blank=True, null=True, verbose_name="PDF"
),
models.JSONField(blank=True, null=True, verbose_name="PDF"),
),
(
"expiration_date",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Generated by Django 5.2.9 on 2026-02-13 18:15

import django.core.serializers.json
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("sample_radius", "0031_radiusbatch_status"),
]

operations = [
migrations.AlterField(
model_name="organizationradiussettings",
name="sms_meta_data",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
help_text=(
"Additional configuration for SMS backend in JSON format "
"(optional, leave blank if unsure)"
),
null=True,
verbose_name="SMS meta data",
),
),
migrations.AlterField(
model_name="radiusbatch",
name="user_credentials",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
verbose_name="PDF",
),
),
]
Loading