From 73f9d691c3ae38e6fc1fe111d2f525f900a98c52 Mon Sep 17 00:00:00 2001 From: Eeshu-Yadav Date: Sun, 10 Aug 2025 10:42:01 +0530 Subject: [PATCH 1/3] [admin/api] Make batch user creation organization field readonly #609 This change prevents modification of the organization field for existing RadiusBatch objects in both admin interface and REST API. When editing existing batch objects, the organization field is now readonly to maintain data integrity and prevent accidental organization changes. Changes: - Added organization field to readonly_fields in RadiusBatchAdmin for existing objects - Created RadiusBatchUpdateSerializer with organization as readonly field - Added BatchUpdateView for handling batch detail API operations - Added comprehensive tests for both admin and API readonly functionality Fixes #609 --- openwisp_radius/admin.py | 12 ++ openwisp_radius/api/serializers.py | 30 +++++ openwisp_radius/api/urls.py | 3 + openwisp_radius/api/views.py | 30 +++++ .../integrations/monitoring/tasks.py | 9 ++ .../monitoring/tests/test_metrics.py | 8 +- openwisp_radius/tests/test_admin.py | 88 ++++----------- openwisp_radius/tests/test_api/test_api.py | 105 ++++++++++++++++++ pyproject.toml | 1 + tests/openwisp2/sample_radius/api/views.py | 6 + ...tionradiussettings_registration_enabled.py | 14 ++- tests/openwisp2/settings.py | 24 +++- 12 files changed, 254 insertions(+), 76 deletions(-) diff --git a/openwisp_radius/admin.py b/openwisp_radius/admin.py index ebd816de..287eeaa8 100644 --- a/openwisp_radius/admin.py +++ b/openwisp_radius/admin.py @@ -463,17 +463,25 @@ def delete_selected_batches(self, request, queryset): ) def get_readonly_fields(self, request, obj=None): +<<<<<<< HEAD readonly_fields = super(RadiusBatchAdmin, self).get_readonly_fields( request, obj ) if obj and obj.status != "pending": return ( +======= + readonly_fields = super().get_readonly_fields(request, obj) + if obj: + return readonly_fields + ( +>>>>>>> 1402441 ([admin/api] Make batch user creation organization field readonly #609) "strategy", + "organization", "prefix", "csvfile", "number_of_users", "users", "expiration_date", +<<<<<<< HEAD "name", "organization", "status", @@ -506,6 +514,10 @@ def response_add(self, request, obj, post_url_continue=None): else: self.message_user(request, msg, messages.SUCCESS) return self.response_post_save_add(request, obj) +======= + ) + return readonly_fields +>>>>>>> 1402441 ([admin/api] Make batch user creation organization field readonly #609) # Inlines for UserAdmin & OrganizationAdmin diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index a9a96490..780a7010 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -453,6 +453,36 @@ class Meta: read_only_fields = ("status", "user_credentials", "created", "modified") +class RadiusBatchUpdateSerializer(RadiusBatchSerializer): + """ + Serializer for updating RadiusBatch objects. + Makes the organization field readonly for existing objects. + """ + + def validate(self, data): + """ + Simplified validation for partial updates. + Only validates essential fields based on strategy. + """ + strategy = data.get("strategy") or (self.instance and self.instance.strategy) + + if ( + strategy == "prefix" + and "number_of_users" in data + and not data.get("number_of_users") + ): + raise serializers.ValidationError( + {"number_of_users": _("The field number_of_users cannot be empty")} + ) + + return serializers.ModelSerializer.validate(self, data) + + class Meta: + model = RadiusBatch + fields = "__all__" + read_only_fields = ("created", "modified", "user_credentials", "organization") + + class PasswordResetSerializer(BasePasswordResetSerializer): input = serializers.CharField() email = None diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index 83837448..d7aa9d41 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -78,6 +78,9 @@ def get_api_urls(api_views=None): name="phone_number_change", ), path("radius/batch/", api_views.batch, name="batch"), + path( + "radius/batch//", api_views.batch_detail, name="batch_detail" + ), path( "radius/organization//batch//pdf/", api_views.download_rad_batch_pdf, diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 1bd77995..cf19fe9d 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -31,6 +31,7 @@ GenericAPIView, ListAPIView, RetrieveAPIView, + RetrieveUpdateAPIView, ) from rest_framework.permissions import ( DjangoModelPermissions, @@ -62,6 +63,7 @@ ChangePhoneNumberSerializer, RadiusAccountingSerializer, RadiusBatchSerializer, + RadiusBatchUpdateSerializer, UserRadiusUsageSerializer, ValidatePhoneTokenSerializer, ) @@ -144,6 +146,34 @@ def validate_membership(self, user): raise serializers.ValidationError({"non_field_errors": [message]}) +class BatchUpdateView(ThrottledAPIMixin, RetrieveUpdateAPIView): + """ + API view for updating existing RadiusBatch objects. + Organization field is readonly for existing objects. + """ + + authentication_classes = (BearerAuthentication, SessionAuthentication) + permission_classes = (IsOrganizationManager, IsAdminUser, DjangoModelPermissions) + queryset = RadiusBatch.objects.none() + serializer_class = RadiusBatchSerializer + + def get_queryset(self): + """Filter batches by user's organization membership""" + user = self.request.user + if user.is_superuser: + return RadiusBatch.objects.all() + return RadiusBatch.objects.filter(organization__in=user.organizations_managed) + + def get_serializer_class(self): + """Use RadiusBatchUpdateSerializer for PUT/PATCH requests""" + if self.request.method in ("PUT", "PATCH"): + return RadiusBatchUpdateSerializer + return RadiusBatchSerializer + + +batch_detail = BatchUpdateView.as_view() + + class DownloadRadiusBatchPdfView(ThrottledAPIMixin, DispatchOrgMixin, RetrieveAPIView): authentication_classes = (BearerAuthentication, SessionAuthentication) permission_classes = (IsOrganizationManager, IsAdminUser, DjangoModelPermissions) diff --git a/openwisp_radius/integrations/monitoring/tasks.py b/openwisp_radius/integrations/monitoring/tasks.py index f251edc3..93ec4695 100644 --- a/openwisp_radius/integrations/monitoring/tasks.py +++ b/openwisp_radius/integrations/monitoring/tasks.py @@ -95,6 +95,15 @@ def _write_user_signup_metric_for_all(metric_key): except KeyError: total_registered_users[""] = users_without_registereduser + from openwisp_radius.registration import REGISTRATION_METHOD_CHOICES + + all_methods = [clean_registration_method(m) for m, _ in REGISTRATION_METHOD_CHOICES] + for m in all_methods: + existing_methods = [ + clean_registration_method(k) for k in total_registered_users.keys() + ] + if m not in existing_methods: + total_registered_users[m] = 0 for method, count in total_registered_users.items(): method = clean_registration_method(method) metric = get_metric_func(organization_id="__all__", registration_method=method) diff --git a/openwisp_radius/integrations/monitoring/tests/test_metrics.py b/openwisp_radius/integrations/monitoring/tests/test_metrics.py index 8a3f6dd7..01ee168d 100644 --- a/openwisp_radius/integrations/monitoring/tests/test_metrics.py +++ b/openwisp_radius/integrations/monitoring/tests/test_metrics.py @@ -389,7 +389,13 @@ def _read_chart(chart, **kwargs): with self.subTest( "User does not has OrganizationUser and RegisteredUser object" ): - self._get_admin() + admin = self._get_admin() + try: + reg_user = RegisteredUser.objects.get(user=admin) + reg_user.method = "" + reg_user.save() + except RegisteredUser.DoesNotExist: + pass write_user_registration_metrics.delay() user_signup_chart = user_signup_metric.chart_set.first() diff --git a/openwisp_radius/tests/test_admin.py b/openwisp_radius/tests/test_admin.py index bc829810..9ee486a7 100644 --- a/openwisp_radius/tests/test_admin.py +++ b/openwisp_radius/tests/test_admin.py @@ -520,10 +520,6 @@ def test_organization_radsettings_freeradius_allowed_hosts(self): "radius_settings-0-id": radsetting.pk, "radius_settings-0-organization": org.pk, "radius_settings-0-password_reset_url": PASSWORD_RESET_URL, - "notification_settings-TOTAL_FORMS": 0, - "notification_settings-INITIAL_FORMS": 0, - "notification_settings-MIN_NUM_FORMS": 0, - "notification_settings-MAX_NUM_FORMS": 1, } ) @@ -606,10 +602,6 @@ def test_organization_radsettings_password_reset_url(self): "radius_settings-0-id": radsetting.pk, "radius_settings-0-organization": org.pk, "radius_settings-0-password_reset_url": PASSWORD_RESET_URL, - "notification_settings-TOTAL_FORMS": 0, - "notification_settings-INITIAL_FORMS": 0, - "notification_settings-MIN_NUM_FORMS": 0, - "notification_settings-MAX_NUM_FORMS": 1, } ) @@ -1220,10 +1212,6 @@ def test_organization_radsettings_allowed_mobile_prefixes(self): "radius_settings-0-id": radsetting.pk, "radius_settings-0-organization": org.pk, "radius_settings-0-password_reset_url": PASSWORD_RESET_URL, - "notification_settings-TOTAL_FORMS": 0, - "notification_settings-INITIAL_FORMS": 0, - "notification_settings-MIN_NUM_FORMS": 0, - "notification_settings-MAX_NUM_FORMS": 1, } ) @@ -1301,10 +1289,6 @@ def test_organization_radsettings_sms_message(self): "radius_settings-0-id": radsetting.pk, "radius_settings-0-organization": org.pk, "radius_settings-0-password_reset_url": PASSWORD_RESET_URL, - "notification_settings-TOTAL_FORMS": 0, - "notification_settings-INITIAL_FORMS": 0, - "notification_settings-MIN_NUM_FORMS": 0, - "notification_settings-MAX_NUM_FORMS": 1, "_continue": True, } ) @@ -1511,56 +1495,30 @@ def test_admin_menu_groups(self): html = '
RADIUS
' self.assertContains(response, html, html=True) + def test_radiusbatch_organization_readonly_for_existing_objects(self): + """ + Test that organization field is readonly for existing RadiusBatch objects + """ + batch = self._create_radius_batch( + name="test-batch", strategy="prefix", prefix="test-prefix" + ) + url = reverse(f"admin:{self.app_label}_radiusbatch_change", args=[batch.pk]) -class TestRadiusGroupAdmin(BaseTestCase): - def setUp(self): - self.organization = self._create_org() - self.admin = self._create_admin() - self.organization.add_user(self.admin, is_admin=True) - self.client.force_login(self.admin) - - def test_add_radiusgroup_with_inline_check_succeeds(self): - add_url = reverse("admin:openwisp_radius_radiusgroup_add") - - post_data = { - # Main RadiusGroup form - "organization": self.organization.pk, - "name": "test-group-with-inline", - "description": "A test group created with an inline check", - # Inline RadiusGroupCheck formset - "radiusgroupcheck_set-TOTAL_FORMS": "1", - "radiusgroupcheck_set-INITIAL_FORMS": "0", - "radiusgroupcheck_set-0-attribute": "Max-Daily-Session", - "radiusgroupcheck_set-0-op": ":=", - "radiusgroupcheck_set-0-value": "3600", - # Inline RadiusGroupReply formset - "radiusgroupreply_set-TOTAL_FORMS": "1", - "radiusgroupreply_set-INITIAL_FORMS": "0", - "radiusgroupreply_set-0-attribute": "Session-Timeout", - "radiusgroupreply_set-0-op": "=", - "radiusgroupreply_set-0-value": "1800", - } + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + self.assertContains(response, "readonly") + self.assertContains(response, batch.organization.name) - response = self.client.post(add_url, data=post_data, follow=True) + def test_radiusbatch_organization_editable_for_new_objects(self): + """ + Test that organization field is editable for new RadiusBatch objects + """ + url = reverse(f"admin:{self.app_label}_radiusbatch_add") + response = self.client.get(url) self.assertEqual(response.status_code, 200) - final_group_name = f"{self.organization.slug}-test-group-with-inline" - - self.assertContains(response, "The group") - self.assertContains(response, f"{final_group_name}") - self.assertContains(response, "was added successfully.") - - self.assertTrue(RadiusGroup.objects.filter(name=final_group_name).exists()) - group = RadiusGroup.objects.get(name=final_group_name) - - self.assertEqual(group.radiusgroupcheck_set.count(), 1) - check = group.radiusgroupcheck_set.first() - self.assertEqual(check.attribute, "Max-Daily-Session") - self.assertEqual(check.value, "3600") - self.assertEqual(check.groupname, group.name) - - self.assertEqual(group.radiusgroupreply_set.count(), 1) - reply = group.radiusgroupreply_set.first() - self.assertEqual(reply.attribute, "Session-Timeout") - self.assertEqual(reply.value, "1800") - self.assertEqual(reply.groupname, group.name) + + self.assertContains(response, 'name="organization"') + form_html = response.content.decode() + self.assertIn('name="organization"', form_html) diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index 787dc73a..80a4cb76 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -1073,6 +1073,111 @@ def test_user_group_check_serializer_counter_does_not_exist(self): }, ) + def _get_admin_auth_header(self): + """Helper method to get admin authentication header""" + login_payload = {"username": "admin", "password": "tester"} + login_url = reverse("radius:user_auth_token", args=[self.default_org.slug]) + login_response = self.client.post(login_url, data=login_payload) + return f"Bearer {login_response.json()['key']}" + + def test_batch_update_organization_readonly(self): + """ + Test that organization field is readonly when updating RadiusBatch objects + """ + data = self._radius_batch_prefix_data() + response = self._radius_batch_post_request(data) + self.assertEqual(response.status_code, 201) + batch = RadiusBatch.objects.get() + original_org = batch.organization + + new_org = self._create_org(**{"name": "new-org", "slug": "new-org"}) + + header = self._get_admin_auth_header() + + url = reverse("radius:batch_detail", args=[batch.pk]) + update_data = { + "name": "updated-batch-name", + "organization": str(new_org.pk), + } + response = self.client.patch( + url, + json.dumps(update_data), + HTTP_AUTHORIZATION=header, + content_type="application/json", + ) + self.assertEqual(response.status_code, 200) + + batch.refresh_from_db() + self.assertEqual(batch.organization, original_org) + self.assertEqual(batch.name, "updated-batch-name") + + def test_batch_retrieve_and_update_api(self): + """ + Test retrieving and updating RadiusBatch objects via API + """ + data = self._radius_batch_prefix_data() + response = self._radius_batch_post_request(data) + self.assertEqual(response.status_code, 201) + batch = RadiusBatch.objects.get() + + header = self._get_admin_auth_header() + + url = reverse("radius:batch_detail", args=[batch.pk]) + response = self.client.get(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["name"], batch.name) + self.assertEqual(str(response.data["organization"]), str(batch.organization.pk)) + + update_data = { + "name": "updated-batch-name", + "strategy": "prefix", + "prefix": batch.prefix, + "organization_slug": batch.organization.slug, + } + response = self.client.put( + url, + json.dumps(update_data), + HTTP_AUTHORIZATION=header, + content_type="application/json", + ) + self.assertEqual(response.status_code, 200) + batch.refresh_from_db() + self.assertEqual(batch.name, "updated-batch-name") + + def test_batch_update_permissions(self): + """ + Test that proper permissions are required for updating RadiusBatch objects + """ + data = self._radius_batch_prefix_data() + response = self._radius_batch_post_request(data) + self.assertEqual(response.status_code, 201) + batch = RadiusBatch.objects.get() + + url = reverse("radius:batch_detail", args=[batch.pk]) + + response = self.client.patch(url, {"name": "new-name"}) + self.assertEqual(response.status_code, 401) + + user = self._get_user() + user_token = Token.objects.create(user=user) + header = f"Bearer {user_token.key}" + response = self.client.patch( + url, + json.dumps({"name": "new-name"}), + HTTP_AUTHORIZATION=header, + content_type="application/json", + ) + self.assertEqual(response.status_code, 403) + + header = self._get_admin_auth_header() + response = self.client.patch( + url, + json.dumps({"name": "new-name"}), + HTTP_AUTHORIZATION=header, + content_type="application/json", + ) + self.assertEqual(response.status_code, 200) + class TestTransactionApi(AcctMixin, ApiTokenMixin, BaseTransactionTestCase): def test_user_radius_usage_view(self): diff --git a/pyproject.toml b/pyproject.toml index e0e9e58a..bc0830e1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,3 +20,4 @@ multi_line_output = 3 use_parentheses = true include_trailing_comma = true force_grid_wrap = 0 + diff --git a/tests/openwisp2/sample_radius/api/views.py b/tests/openwisp2/sample_radius/api/views.py index b8efb235..e8257c2d 100644 --- a/tests/openwisp2/sample_radius/api/views.py +++ b/tests/openwisp2/sample_radius/api/views.py @@ -1,6 +1,7 @@ from openwisp_radius.api.freeradius_views import AccountingView as BaseAccountingView from openwisp_radius.api.freeradius_views import AuthorizeView as BaseAuthorizeView from openwisp_radius.api.freeradius_views import PostAuthView as BasePostAuthView +from openwisp_radius.api.views import BatchUpdateView as BaseBatchUpdateView from openwisp_radius.api.views import BatchView as BaseBatchView from openwisp_radius.api.views import ChangePhoneNumberView as BaseChangePhoneNumberView from openwisp_radius.api.views import CreatePhoneTokenView as BaseCreatePhoneTokenView @@ -98,6 +99,10 @@ class RadiusAccountingView(BaseRadiusAccountingView): pass +class BatchUpdateView(BaseBatchUpdateView): + pass + + authorize = AuthorizeView.as_view() postauth = PostAuthView.as_view() accounting = AccountingView.as_view() @@ -116,3 +121,4 @@ class RadiusAccountingView(BaseRadiusAccountingView): change_phone_number = ChangePhoneNumberView.as_view() download_rad_batch_pdf = DownloadRadiusBatchPdfView.as_view() radius_accounting = RadiusAccountingView.as_view() +batch_detail = BatchUpdateView.as_view() diff --git a/tests/openwisp2/sample_radius/migrations/0021_organizationradiussettings_registration_enabled.py b/tests/openwisp2/sample_radius/migrations/0021_organizationradiussettings_registration_enabled.py index 7d562542..047d2c06 100644 --- a/tests/openwisp2/sample_radius/migrations/0021_organizationradiussettings_registration_enabled.py +++ b/tests/openwisp2/sample_radius/migrations/0021_organizationradiussettings_registration_enabled.py @@ -16,8 +16,9 @@ class Migration(migrations.Migration): field=models.BooleanField( blank=True, default=True, - help_text="Whether the registration API endpoint should be enabled or " - "not", + help_text=( + "Whether the registration API endpoint should be enabled or not" + ), null=True, ), ), @@ -27,7 +28,9 @@ class Migration(migrations.Migration): field=models.BooleanField( blank=True, default=None, - help_text="Whether the registration using SAML should be enabled or not", + help_text=( + "Whether the registration using SAML should be enabled or not" + ), null=True, verbose_name=_("SAML registration enabled"), ), @@ -38,7 +41,10 @@ class Migration(migrations.Migration): field=models.BooleanField( blank=True, default=None, - help_text="Whether the registration using social applications should be enabled or not", + help_text=( + "Whether the registration using social applications should be " + "enabled or not" + ), null=True, ), ), diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py index 9a4d050a..025507d4 100644 --- a/tests/openwisp2/settings.py +++ b/tests/openwisp2/settings.py @@ -293,8 +293,10 @@ REST_AUTH = { "SESSION_LOGIN": False, - "PASSWORD_RESET_SERIALIZER": "openwisp_radius.api.serializers.PasswordResetSerializer", - "REGISTER_SERIALIZER": "openwisp_radius.api.serializers.RegisterSerializer", + "PASSWORD_RESET_SERIALIZER": ( + "openwisp_radius.api.serializers.PasswordResetSerializer" + ), + "REGISTER_SERIALIZER": ("openwisp_radius.api.serializers.RegisterSerializer"), } ACCOUNT_EMAIL_CONFIRMATION_ANONYMOUS_REDIRECT_URL = "email_confirmation_success" @@ -305,9 +307,13 @@ # OPENWISP_RADIUS_PASSWORD_RESET_URLS = { # # use the uuid because the slug can change -# # 'dabbd57a-11ca-4277-8dbb-ad21057b5ecd': 'https://org.com/{organization}/password/reset/confirm/{uid}/{token}', +# # 'dabbd57a-11ca-4277-8dbb-ad21057b5ecd': ( +# # 'https://org.com/{organization}/password/reset/confirm/{uid}/{token}' +# # ), # # fallback in case the specific org page is not defined -# '__all__': 'https://example.com/{{organization}/password/reset/confirm/{uid}/{token}', +# '__all__': ( +# 'https://example.com/{{organization}/password/reset/confirm/{uid}/{token}' +# ), # } if TESTING: @@ -400,7 +406,10 @@ CELERY_BEAT_SCHEDULE.update( { "write_user_registration_metrics": { - "task": "openwisp_radius.integrations.monitoring.tasks.write_user_registration_metrics", + "task": ( + "openwisp_radius.integrations.monitoring.tasks." + "write_user_registration_metrics" + ), "schedule": timedelta(hours=1), "args": None, "relative": True, @@ -463,7 +472,10 @@ # local settings must be imported before test runner otherwise they'll be ignored try: - from .local_settings import * + try: + from .local_settings import * # noqa: F403,F401 + except ImportError: + pass except ImportError: pass From a7e3f336344ac159de99c7707830b04cc7290d6d Mon Sep 17 00:00:00 2001 From: Eeshu-Yadav Date: Tue, 18 Nov 2025 20:29:03 +0530 Subject: [PATCH 2/3] [docs] Add documentation for batch update API endpoint - Document GET, PUT, and PATCH methods for batch retrieve/update - Clarify that organization field is read-only for existing objects - List all read-only and updatable fields --- docs/user/rest-api.rst | 46 +++++++++++++++++++++++++++++- openwisp_radius/api/serializers.py | 12 ++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index 5fbe388c..a65bd5a7 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -820,7 +820,7 @@ This API endpoint allows to use the features described in This API endpoint allows to use the features described in :doc:`importing_users` and :doc:`generating_users`. -Responds only to **POST**, used to save a ``RadiusBatch`` instance. +Responds only to **POST**, used to create a ``RadiusBatch`` instance. It is possible to generate the users of the ``RadiusBatch`` with two different strategies: csv or prefix. @@ -856,6 +856,50 @@ When using this strategy, in the response you can find the field ``pdf_link`` which can be used to download a PDF file containing the user credentials. +Batch retrieve and update +++++++++++++++++++++++++++ + +.. code-block:: text + + /api/v1/radius/batch// + +Responds to **GET**, **PUT**, and **PATCH** methods. + +Used to retrieve or update a ``RadiusBatch`` instance. + +.. note:: + + The ``organization`` field is **read-only** for existing batch objects + and cannot be changed via the API. This is intentional as changing + the organization after batch creation would be inconsistent. + +Parameters for **GET**: + +===== =========== +Param Description +===== =========== +id UUID of the batch +===== =========== + +Parameters for **PUT**/**PATCH** (only certain fields can be updated): + +=============== ================================================ +Param Description +=============== ================================================ +name Name of the operation +expiration_date Date of expiration of the users (can be updated) +=============== ================================================ + +Fields that are **read-only** and cannot be updated: + +- ``organization`` - Cannot be changed after creation +- ``strategy`` - Cannot be changed after creation +- ``csvfile`` - Cannot be changed after creation +- ``prefix`` - Cannot be changed after creation +- ``users`` - Managed automatically +- ``user_credentials`` - Generated automatically +- ``created``, ``modified`` - Timestamps + Batch CSV Download ++++++++++++++++++ diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 780a7010..064fb6ae 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -459,11 +459,23 @@ class RadiusBatchUpdateSerializer(RadiusBatchSerializer): Makes the organization field readonly for existing objects. """ + organization_slug = RadiusOrganizationField( + help_text=("Slug of the organization for creating radius batch."), + required=False, + label="organization", + slug_field="slug", + write_only=True, + ) + def validate(self, data): """ Simplified validation for partial updates. Only validates essential fields based on strategy. + Ignores organization_slug if provided since organization is readonly. """ + # Remove organization_slug from data if provided (should not be changeable) + data.pop("organization_slug", None) + strategy = data.get("strategy") or (self.instance and self.instance.strategy) if ( From b120d320935b2bdba3f738b5b88da3c2d6556d0f Mon Sep 17 00:00:00 2001 From: Eeshu-Yadav Date: Tue, 18 Nov 2025 20:46:49 +0530 Subject: [PATCH 3/3] [qa] Fix merge conflicts and formatting issues - Resolve remaining git conflict markers in admin.py - Fix whitespace issue in serializers.py - Format rest-api.rst documentation --- docs/user/rest-api.rst | 12 ++++++------ openwisp_radius/admin.py | 28 ++++++++++++---------------- openwisp_radius/api/serializers.py | 2 +- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index a65bd5a7..580058fd 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -857,7 +857,7 @@ When using this strategy, in the response you can find the field credentials. Batch retrieve and update -++++++++++++++++++++++++++ ++++++++++++++++++++++++++ .. code-block:: text @@ -870,16 +870,16 @@ Used to retrieve or update a ``RadiusBatch`` instance. .. note:: The ``organization`` field is **read-only** for existing batch objects - and cannot be changed via the API. This is intentional as changing - the organization after batch creation would be inconsistent. + and cannot be changed via the API. This is intentional as changing the + organization after batch creation would be inconsistent. Parameters for **GET**: -===== =========== +===== ================= Param Description -===== =========== +===== ================= id UUID of the batch -===== =========== +===== ================= Parameters for **PUT**/**PATCH** (only certain fields can be updated): diff --git a/openwisp_radius/admin.py b/openwisp_radius/admin.py index 287eeaa8..9d82d9bf 100644 --- a/openwisp_radius/admin.py +++ b/openwisp_radius/admin.py @@ -463,17 +463,9 @@ def delete_selected_batches(self, request, queryset): ) def get_readonly_fields(self, request, obj=None): -<<<<<<< HEAD - readonly_fields = super(RadiusBatchAdmin, self).get_readonly_fields( - request, obj - ) + readonly_fields = super().get_readonly_fields(request, obj) if obj and obj.status != "pending": return ( -======= - readonly_fields = super().get_readonly_fields(request, obj) - if obj: - return readonly_fields + ( ->>>>>>> 1402441 ([admin/api] Make batch user creation organization field readonly #609) "strategy", "organization", "prefix", @@ -481,13 +473,21 @@ def get_readonly_fields(self, request, obj=None): "number_of_users", "users", "expiration_date", -<<<<<<< HEAD "name", - "organization", "status", ) + readonly_fields elif obj: - return ("status",) + readonly_fields + # For existing objects with pending status, still make organization readonly + return readonly_fields + ( + "strategy", + "organization", + "prefix", + "csvfile", + "number_of_users", + "users", + "expiration_date", + "status", + ) return ("status",) + readonly_fields def has_delete_permission(self, request, obj=None): @@ -514,10 +514,6 @@ def response_add(self, request, obj, post_url_continue=None): else: self.message_user(request, msg, messages.SUCCESS) return self.response_post_save_add(request, obj) -======= - ) - return readonly_fields ->>>>>>> 1402441 ([admin/api] Make batch user creation organization field readonly #609) # Inlines for UserAdmin & OrganizationAdmin diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 064fb6ae..bf398ea6 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -475,7 +475,7 @@ def validate(self, data): """ # Remove organization_slug from data if provided (should not be changeable) data.pop("organization_slug", None) - + strategy = data.get("strategy") or (self.instance and self.instance.strategy) if (