From ae566dc6ebe21cd0a4002ff1d2350b6d2e39f813 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 6 Feb 2026 18:59:34 +0530 Subject: [PATCH 01/10] [feature] Added API endpoint to manage RADIUS groups of users #676 Closes #676 --- docs/user/rest-api.rst | 63 +++++++ openwisp_radius/api/serializers.py | 24 +++ openwisp_radius/api/urls.py | 10 ++ openwisp_radius/api/views.py | 103 ++++++++++- openwisp_radius/tests/test_api/test_api.py | 200 +++++++++++++++++++++ tests/openwisp2/sample_radius/api/views.py | 9 +- 6 files changed, 407 insertions(+), 2 deletions(-) diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index 79528090..667afe54 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -961,3 +961,66 @@ DELETE ^^^^^^ Deletes a RADIUS group identified by its UUID. + +RADIUS User Groups +++++++++++++++++++ + +.. code-block:: text + + /api/v1/users/user//radius-group/ + +GET +^^^ + +Returns the list of RADIUS group assignments for the specified user. +Pagination is provided using page number pagination; default page size is +20 and can be overridden with the ``page_size`` parameter (maximum 100). + +.. code-block:: text + + GET /api/v1/users/user//radius-group/ + +POST +^^^^ + +Creates a RADIUS user group assignment for the specified user. + +======== ============================================== +Param Description +======== ============================================== +group UUID of the RADIUS group to assign (required) +priority Priority of the assignment (optional, integer) +======== ============================================== + +.. note:: + + The provided ``group`` must belong to the same organization as the + user; attempting to assign a group from another organization will + return a ``400`` error with ``does_not_exist`` for the ``group`` + field. + +.. code-block:: text + + /api/v1/users/user//radius-group// + +GET (detail) +^^^^^^^^^^^^ + +Returns a single RADIUS user group assignment by its UUID. + +PATCH +^^^^^ + +Partially updates a RADIUS user group assignment. + +======== ============================================== +Param Description +======== ============================================== +group UUID of the RADIUS group to assign (optional) +priority Priority of the assignment (optional, integer) +======== ============================================== + +DELETE +^^^^^^ + +Deletes the RADIUS user group assignment identified by the UUID. diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index ae7cc3e2..65d3e653 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -365,6 +365,30 @@ def validate(self, data): return data +class RadiusUserGroupSerializer(FilterSerializerByOrgManaged, ValidatedModelSerializer): + class Meta: + model = RadiusUserGroup + fields = ("id", "group", "priority", "created", "modified") + read_only_fields = ("id", "created", "modified") + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + view = self.context.get("view") + self._user = view.get_parent_queryset().first() + if self._user: + self.fields["group"].queryset = self.fields["group"].queryset.filter( + organization_id__in=self._user.organizations_dict.keys() + ) + + def validate(self, data): + if self._user: + if "username" not in data: + data["username"] = self._user.username + if "user" not in data: + data["user"] = self._user + return super().validate(data) + + class GroupSerializer(serializers.ModelSerializer): class Meta: model = Group diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index dfeeab65..88d02572 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -98,6 +98,16 @@ def get_api_urls(api_views=None): api_views.radius_group_detail, name="radius_group_detail", ), + path( + "users/user//radius-group/", + api_views.radius_user_group_list, + name="radius_user_group_list", + ), + path( + "users/user//radius-group//", + api_views.radius_user_group_detail, + name="radius_user_group_detail", + ), ] else: return [] diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index fba092e9..dedb0c46 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -48,7 +48,11 @@ from openwisp_radius.api.serializers import RadiusUserSerializer from openwisp_users.api.authentication import BearerAuthentication, SesameAuthentication from openwisp_users.api.filters import OrganizationManagedFilter -from openwisp_users.api.mixins import FilterByOrganizationManaged, ProtectedAPIMixin +from openwisp_users.api.mixins import ( + FilterByOrganizationManaged, + FilterByParentManaged, + ProtectedAPIMixin, +) from openwisp_users.api.permissions import IsOrganizationManager from openwisp_users.api.views import ChangePasswordView as BasePasswordChangeView from openwisp_users.backends import UsersAuthenticationBackend @@ -69,6 +73,7 @@ RadiusAccountingSerializer, RadiusBatchSerializer, RadiusGroupSerializer, + RadiusUserGroupSerializer, UserRadiusUsageSerializer, ValidatePhoneTokenSerializer, ) @@ -936,3 +941,99 @@ class RadiusGroupDetailView( radius_group_detail = RadiusGroupDetailView.as_view() + + +class BaseRadiusUserGroupView(ProtectedAPIMixin, FilterByParentManaged): + """ + Base view for RadiusUserGroup management. + Provides user parent filtering and queryset logic. + """ + + serializer_class = RadiusUserGroupSerializer + queryset = RadiusUserGroup.objects.select_related("group", "user").order_by( + "-created" + ) + + def get_queryset(self): + qs = super().get_queryset() + if getattr(self, "swagger_fake_view", False): + return super().get_queryset() + return qs.filter(user_id=self.kwargs["user_pk"]) + + def get_parent_queryset(self): + """Get the parent user from the URL.""" + return User.objects.filter(pk=self.kwargs["user_pk"]) + + def get_organization_queryset(self, qs): + """Filter users by organizations the request user manages.""" + orgs = self.request.user.organizations_managed + app_label = User._meta.app_config.label + filter_kwargs = { + # exclude superusers + "is_superuser": False, + # ensure user is member of the org + f"{app_label}_organizationuser__organization_id__in": orgs, + } + return qs.filter(**filter_kwargs).distinct() + + +@method_decorator( + name="get", + decorator=swagger_auto_schema( + operation_description=""" + Returns the list of RADIUS user groups for a specific user. + """, + ), +) +@method_decorator( + name="post", + decorator=swagger_auto_schema( + operation_description=""" + Creates a new RADIUS user group assignment for the user. + """, + ), +) +class RadiusUserGroupListCreateView(BaseRadiusUserGroupView, ListCreateAPIView): + pagination_class = RadiusGroupPaginator + + +radius_user_group_list = RadiusUserGroupListCreateView.as_view() + + +@method_decorator( + name="get", + decorator=swagger_auto_schema( + operation_description=""" + Returns a single RADIUS user group by its UUID. + """, + ), +) +@method_decorator( + name="put", + decorator=swagger_auto_schema( + operation_description=""" + Updates a RADIUS user group identified by its UUID. + """, + ), +) +@method_decorator( + name="patch", + decorator=swagger_auto_schema( + operation_description=""" + Partially updates a RADIUS user group identified by its UUID. + """, + ), +) +@method_decorator( + name="delete", + decorator=swagger_auto_schema( + operation_description=""" + Deletes a RADIUS user group identified by its UUID. + """, + ), +) +class RadiusUserGroupDetailView(BaseRadiusUserGroupView, RetrieveUpdateDestroyAPIView): + organization_field = "group__organization" + + +radius_user_group_detail = RadiusUserGroupDetailView.as_view() diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index a0148b21..13559110 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -1292,6 +1292,206 @@ def test_radius_group_detail(self): response = self.client.delete(url_org2) self.assertEqual(response.status_code, 404) + def test_radius_user_group_list(self): + org1 = self._create_org(name="Org 1") + org2 = self._create_org(name="Org 2") + admin_user = self._create_user(username="admin_user", email="admin@test.com") + self._create_org_user(user=admin_user, organization=org1, is_admin=True) + target_user = self._create_user(username="target_user", email="target@test.com") + self._create_org_user(user=target_user, organization=org1) + # Create a user in org2 that admin_user should not be able to access + org2_user = self._create_user(username="org2_user", email="org2@test.com") + self._create_org_user(user=org2_user, organization=org2) + org1_group = RadiusGroup.objects.get(organization=org1, name="org-1-users") + rug = self._create_radius_usergroup( + user=target_user, group=org1_group, priority=1 + ) + url = reverse("radius:radius_user_group_list", args=[target_user.pk]) + org2_user_url = reverse("radius:radius_user_group_list", args=[org2_user.pk]) + + with self.subTest("Unauthenticated access"): + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + self.client.force_login(user=admin_user) + with self.subTest("Access without permission"): + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self.client.post( + url, {"group": str(org1_group.pk)}, content_type="application/json" + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + self._add_model_permission(admin_user, RadiusUserGroup, ["view", "add"]) + with self.subTest("List RadiusUserGroups"): + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + data = response.data + self.assertEqual(data["count"], 1) + self.assertEqual(data["results"][0]["id"], str(rug.pk)) + + with self.subTest("Create RadiusUserGroup"): + org1_power_users = RadiusGroup.objects.get( + organization=org1, name="org-1-power-users" + ) + response = self.client.post( + url, + {"group": str(org1_power_users.pk), "priority": 2}, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual( + RadiusUserGroup.objects.filter(user=target_user).count(), 2 + ) + + with self.subTest("Cannot access user from other organization"): + response = self.client.get(org2_user_url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + with self.subTest("Cannot create for user not in admin's organization"): + org2_user2 = self._create_user( + username="org2_user2", email="org2user2@test.com" + ) + self._create_org_user(user=org2_user2, organization=org2) + org2_user2_url = reverse( + "radius:radius_user_group_list", args=[org2_user2.pk] + ) + org1_group = RadiusGroup.objects.get( + organization=org1, name="org-1-power-users" + ) + response = self.client.post( + org2_user2_url, + {"group": str(org1_group.pk)}, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + with self.subTest("Cannot create RadiusUserGroup with group from other org"): + self._create_org_user(user=admin_user, organization=org2, is_admin=True) + org2_group = RadiusGroup.objects.get( + organization=org2, name="org-2-power-users" + ) + response = self.client.post( + url, + {"group": str(org2_group.pk)}, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("group", response.data) + self.assertEqual(response.data["group"][0].code, "does_not_exist") + + with self.subTest("Superuser can access any user"): + superuser = self._get_admin() + self.client.force_login(user=superuser) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self.client.get(org2_user_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_radius_user_group_detail(self): + org1 = self._create_org(name="Org 1") + org2 = self._create_org(name="Org 2") + admin_user = self._create_user(username="admin_user", email="admin@test.com") + self._create_org_user(user=admin_user, organization=org1, is_admin=True) + target_user = self._create_user(username="target_user", email="target@test.com") + self._create_org_user(user=target_user, organization=org1) + org1_default_group = RadiusGroup.objects.get( + organization=org1, name="org-1-users" + ) + org1_power_users_group = RadiusGroup.objects.get( + organization=org1, name="org-1-power-users" + ) + rug = self._create_radius_usergroup( + user=target_user, group=org1_default_group, priority=1 + ) + url = reverse("radius:radius_user_group_detail", args=[target_user.pk, rug.pk]) + + with self.subTest("Unauthenticated access"): + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + response = self.client.patch( + url, {"priority": 5}, content_type="application/json" + ) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + response = self.client.delete(url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + self.client.force_login(user=admin_user) + with self.subTest("Access without permission"): + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + self._add_model_permission( + admin_user, RadiusUserGroup, ["view", "change", "delete"] + ) + + with self.subTest("GET operation"): + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + data = response.json() + self.assertEqual(data["id"], str(rug.pk)) + self.assertEqual(data["group"], str(org1_default_group.pk)) + + with self.subTest("PATCH operation"): + response = self.client.patch( + url, {"priority": 10}, content_type="application/json" + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + rug.refresh_from_db() + self.assertEqual(rug.priority, 10) + + with self.subTest("PUT operation"): + response = self.client.put( + url, + {"group": str(org1_power_users_group.pk), "priority": 3}, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + rug.refresh_from_db() + self.assertEqual(rug.group, org1_power_users_group) + self.assertEqual(rug.priority, 3) + + with self.subTest("DELETE operation"): + response = self.client.delete(url) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertEqual(RadiusUserGroup.objects.filter(pk=rug.pk).count(), 0) + + with self.subTest("GET not found"): + fake_uuid = "00000000-0000-0000-0000-000000000000" + fake_url = reverse( + "radius:radius_user_group_detail", args=[target_user.pk, fake_uuid] + ) + response = self.client.get(fake_url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + with self.subTest("Cannot access RadiusUserGroup of user from other org"): + org2_user = self._create_user(username="org2_user", email="org2@test.com") + self._create_org_user(user=org2_user, organization=org2) + org2_rug = self._create_radius_usergroup( + user=org2_user, + group=RadiusGroup.objects.get(organization=org2, name="org-2-users"), + priority=1, + ) + org2_url = reverse( + "radius:radius_user_group_detail", args=[org2_user.pk, org2_rug.pk] + ) + response = self.client.get(org2_url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + with self.subTest("Superuser can access any RadiusUserGroup"): + superuser = self._get_admin() + self.client.force_login(user=superuser) + response = self.client.get(org2_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Patch RadiusUserGroup of org2_user + response = self.client.patch( + org2_url, {"priority": 7}, content_type="application/json" + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + org2_rug.refresh_from_db() + self.assertEqual(org2_rug.priority, 7) + class TestTransactionApi(AcctMixin, ApiTokenMixin, BaseTransactionTestCase): def test_user_radius_usage_view(self): diff --git a/tests/openwisp2/sample_radius/api/views.py b/tests/openwisp2/sample_radius/api/views.py index f5338f4c..6bb68b99 100644 --- a/tests/openwisp2/sample_radius/api/views.py +++ b/tests/openwisp2/sample_radius/api/views.py @@ -17,7 +17,12 @@ ) from openwisp_radius.api.views import PasswordResetView as BasePasswordResetView from openwisp_radius.api.views import RadiusAccountingView as BaseRadiusAccountingView -from openwisp_radius.api.views import RadiusGroupDetailView, RadiusGroupListView +from openwisp_radius.api.views import ( + RadiusGroupDetailView, + RadiusGroupListView, + RadiusUserGroupDetailView, + RadiusUserGroupListCreateView, +) from openwisp_radius.api.views import RegisterView as BaseRegisterView from openwisp_radius.api.views import UserAccountingView as BaseUserAccountingView from openwisp_radius.api.views import UserRadiusUsageView as BaseUserRadiusUsageView @@ -119,3 +124,5 @@ class RadiusAccountingView(BaseRadiusAccountingView): radius_accounting = RadiusAccountingView.as_view() radius_group_list = RadiusGroupListView.as_view() radius_group_detail = RadiusGroupDetailView.as_view() +radius_user_group_list = RadiusUserGroupListCreateView.as_view() +radius_user_group_detail = RadiusUserGroupDetailView.as_view() From 15b1d880a7d09229f3f0ecd2811f957eef6a2bc7 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 10 Feb 2026 20:12:08 +0530 Subject: [PATCH 02/10] [fix] Made requested changes by coderabbitai --- openwisp_radius/api/serializers.py | 2 ++ openwisp_radius/api/views.py | 2 +- openwisp_radius/tests/test_api/test_api.py | 19 ++++++++++++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 65d3e653..7ffe2a38 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -379,6 +379,8 @@ def __init__(self, *args, **kwargs): self.fields["group"].queryset = self.fields["group"].queryset.filter( organization_id__in=self._user.organizations_dict.keys() ) + else: + self.fields["group"].queryset = self.fields["group"].queryset.none() def validate(self, data): if self._user: diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index dedb0c46..44e822dc 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -957,7 +957,7 @@ class BaseRadiusUserGroupView(ProtectedAPIMixin, FilterByParentManaged): def get_queryset(self): qs = super().get_queryset() if getattr(self, "swagger_fake_view", False): - return super().get_queryset() + return qs return qs.filter(user_id=self.kwargs["user_pk"]) def get_parent_queryset(self): diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index 13559110..e72b7a1b 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -1299,6 +1299,7 @@ def test_radius_user_group_list(self): self._create_org_user(user=admin_user, organization=org1, is_admin=True) target_user = self._create_user(username="target_user", email="target@test.com") self._create_org_user(user=target_user, organization=org1) + self._create_org_user(user=target_user, organization=org2) # Create a user in org2 that admin_user should not be able to access org2_user = self._create_user(username="org2_user", email="org2@test.com") self._create_org_user(user=org2_user, organization=org2) @@ -1367,7 +1368,8 @@ def test_radius_user_group_list(self): self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) with self.subTest("Cannot create RadiusUserGroup with group from other org"): - self._create_org_user(user=admin_user, organization=org2, is_admin=True) + # target_user is member of org2, + # but admin_user can only manage org1 org2_group = RadiusGroup.objects.get( organization=org2, name="org-2-power-users" ) @@ -1380,6 +1382,21 @@ def test_radius_user_group_list(self): self.assertIn("group", response.data) self.assertEqual(response.data["group"][0].code, "does_not_exist") + # target_user is only member of org1, + # admin_user can manage both org1 and org2 + OrganizationUser.objects.filter( + user=target_user, organization=org2 + ).delete() + self._create_org_user(user=target_user, organization=org2, is_admin=True) + response = self.client.post( + url, + {"group": str(org2_group.pk)}, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("group", response.data) + self.assertEqual(response.data["group"][0].code, "does_not_exist") + with self.subTest("Superuser can access any user"): superuser = self._get_admin() self.client.force_login(user=superuser) From e1676989b6ded454177e1bfae1950e05eb394f59 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Sat, 28 Feb 2026 01:45:17 +0530 Subject: [PATCH 03/10] [fix] Requested changes by @coderabbitai --- docs/user/rest-api.rst | 12 ++++++++++++ openwisp_radius/api/serializers.py | 8 ++++++-- openwisp_radius/api/views.py | 14 ++++++++++++++ openwisp_radius/tests/test_api/test_api.py | 10 ++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index 667afe54..1f6ec5bf 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -980,6 +980,18 @@ Pagination is provided using page number pagination; default page size is GET /api/v1/users/user//radius-group/ +It supports sorting by organization id and organization slug. + +Filters +""""""" + +========================= ============================ +Filter Parameter Description +========================= ============================ +group__organization Filter organizations by id +group__organization__slug Filter organizations by slug +========================= ============================ + POST ^^^^ diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 7ffe2a38..906b9e90 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -373,8 +373,12 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - view = self.context.get("view") - self._user = view.get_parent_queryset().first() + if self.context.get("view") and getattr( + self.context["view"], "get_parent_queryset", None + ): + self._user = self.context["view"].get_parent_queryset().first() + else: + self._user = None if self._user: self.fields["group"].queryset = self.fields["group"].queryset.filter( organization_id__in=self._user.organizations_dict.keys() diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 44e822dc..4b4e0b6f 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -977,6 +977,18 @@ def get_organization_queryset(self, qs): return qs.filter(**filter_kwargs).distinct() +class RadiusUserGroupFilter(OrganizationManagedFilter, filters.FilterSet): + """ + Filter RADIUS groups by organizations managed by the user. + """ + + organization_slug = None + + class Meta(OrganizationManagedFilter.Meta): + model = RadiusUserGroup + fields = ["group__organization", "group__organization__slug"] + + @method_decorator( name="get", decorator=swagger_auto_schema( @@ -995,6 +1007,8 @@ def get_organization_queryset(self, qs): ) class RadiusUserGroupListCreateView(BaseRadiusUserGroupView, ListCreateAPIView): pagination_class = RadiusGroupPaginator + filter_backends = [DjangoFilterBackend] + filterset_class = RadiusUserGroupFilter radius_user_group_list = RadiusUserGroupListCreateView.as_view() diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index e72b7a1b..e16e2e2b 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -1331,6 +1331,16 @@ def test_radius_user_group_list(self): self.assertEqual(data["count"], 1) self.assertEqual(data["results"][0]["id"], str(rug.pk)) + with self.subTest("Filter by organization query parameter"): + # requesting the same organization that the existing group belongs to + response = self.client.get(url + f"?group__organization={org1.pk}") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 1) + # a different organization should yield no results + response = self.client.get(url + f"?group__organization={org2.pk}") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("group__organization", response.data) + with self.subTest("Create RadiusUserGroup"): org1_power_users = RadiusGroup.objects.get( organization=org1, name="org-1-power-users" From 50572dcccd810aab3668f5059103ddf931819c29 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Sat, 28 Feb 2026 02:43:39 +0530 Subject: [PATCH 04/10] [fix] Made requested changes by coderabbitai --- docs/user/rest-api.rst | 12 ++++++++++++ openwisp_radius/api/serializers.py | 6 ++++-- openwisp_radius/api/views.py | 4 ++-- openwisp_radius/tests/test_api/test_api.py | 8 +++++++- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index 1f6ec5bf..673350f0 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -1020,6 +1020,18 @@ GET (detail) Returns a single RADIUS user group assignment by its UUID. +PUT +^^^ + +Fully updates the RADIUS user group assignment. + +======== ============================================== +Param Description +======== ============================================== +group UUID of the RADIUS group to assign (optional) +priority Priority of the assignment (optional, integer) +======== ============================================== + PATCH ^^^^^ diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 906b9e90..f74d5d43 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -373,8 +373,10 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - if self.context.get("view") and getattr( - self.context["view"], "get_parent_queryset", None + if ( + self.context.get("view") + and getattr(self.context["view"], "get_parent_queryset", None) + and not getattr(self.context["view"], "swagger_fake_view", False) ): self._user = self.context["view"].get_parent_queryset().first() else: diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 4b4e0b6f..5a2a1058 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -955,9 +955,9 @@ class BaseRadiusUserGroupView(ProtectedAPIMixin, FilterByParentManaged): ) def get_queryset(self): - qs = super().get_queryset() if getattr(self, "swagger_fake_view", False): - return qs + return self.queryset.none() + qs = super().get_queryset() return qs.filter(user_id=self.kwargs["user_pk"]) def get_parent_queryset(self): diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index e16e2e2b..35e43bd8 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -26,6 +26,7 @@ from openwisp_radius import settings as app_settings from openwisp_radius.api.serializers import ( + RadiusUserGroupSerializer, RadiusUserSerializer, UserGroupCheckSerializer, ) @@ -1397,7 +1398,7 @@ def test_radius_user_group_list(self): OrganizationUser.objects.filter( user=target_user, organization=org2 ).delete() - self._create_org_user(user=target_user, organization=org2, is_admin=True) + self._create_org_user(user=admin_user, organization=org2, is_admin=True) response = self.client.post( url, {"group": str(org2_group.pk)}, @@ -1519,6 +1520,11 @@ def test_radius_user_group_detail(self): org2_rug.refresh_from_db() self.assertEqual(org2_rug.priority, 7) + def test_radius_user_group_serializer_without_view_context(self): + serializer = RadiusUserGroupSerializer(context={}) + self.assertEqual(serializer._user, None) + self.assertEqual(serializer.fields["group"].queryset.count(), 0) + class TestTransactionApi(AcctMixin, ApiTokenMixin, BaseTransactionTestCase): def test_user_radius_usage_view(self): From 93a80275f25d9f0ace7f09e65eac902746d4a1bc Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 2 Mar 2026 15:25:41 +0530 Subject: [PATCH 05/10] [docs] Made fixes suggested by @coderabbitai --- docs/user/rest-api.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index 673350f0..0efc1882 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -980,7 +980,7 @@ Pagination is provided using page number pagination; default page size is GET /api/v1/users/user//radius-group/ -It supports sorting by organization id and organization slug. +It supports filtering by organization id and organization slug. Filters """"""" From 0aabd6181dd49d21913e5cdaa8fb315af96fc451 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 2 Mar 2026 20:36:02 +0530 Subject: [PATCH 06/10] [docs] Made fixes suggested by @coderabbitai --- openwisp_radius/api/views.py | 14 ++++++++++++-- openwisp_radius/tests/test_api/test_api.py | 7 ++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 5a2a1058..827f7323 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -957,8 +957,18 @@ class BaseRadiusUserGroupView(ProtectedAPIMixin, FilterByParentManaged): def get_queryset(self): if getattr(self, "swagger_fake_view", False): return self.queryset.none() - qs = super().get_queryset() - return qs.filter(user_id=self.kwargs["user_pk"]) + qs = ( + super() + .get_queryset() + .filter( + user_id=self.kwargs["user_pk"], + ) + ) + if self.request.user.is_superuser: + return qs + return qs.filter( + group__organization__in=self.request.user.organizations_managed + ) def get_parent_queryset(self): """Get the parent user from the URL.""" diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index 35e43bd8..c15c7a9c 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -1308,6 +1308,8 @@ def test_radius_user_group_list(self): rug = self._create_radius_usergroup( user=target_user, group=org1_group, priority=1 ) + org2_group = RadiusGroup.objects.get(organization=org2, name="org-2-users") + self._create_radius_usergroup(user=target_user, group=org2_group, priority=1) url = reverse("radius:radius_user_group_list", args=[target_user.pk]) org2_user_url = reverse("radius:radius_user_group_list", args=[org2_user.pk]) @@ -1353,7 +1355,10 @@ def test_radius_user_group_list(self): ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual( - RadiusUserGroup.objects.filter(user=target_user).count(), 2 + RadiusUserGroup.objects.filter( + user=target_user, group__organization=org1 + ).count(), + 2, ) with self.subTest("Cannot access user from other organization"): From b6cf69708868d3581473fff99f973b9dbddbb7d9 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 6 Mar 2026 13:06:08 +0530 Subject: [PATCH 07/10] [fix] Made requested changes by @coderabbitai --- openwisp_radius/api/views.py | 2 +- openwisp_radius/tests/test_api/test_api.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 827f7323..07c4bd37 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -971,7 +971,6 @@ def get_queryset(self): ) def get_parent_queryset(self): - """Get the parent user from the URL.""" return User.objects.filter(pk=self.kwargs["user_pk"]) def get_organization_queryset(self, qs): @@ -992,6 +991,7 @@ class RadiusUserGroupFilter(OrganizationManagedFilter, filters.FilterSet): Filter RADIUS groups by organizations managed by the user. """ + # Disable parent's organization_slug; use group__organization__slug instead organization_slug = None class Meta(OrganizationManagedFilter.Meta): diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index c15c7a9c..00906de0 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -1484,6 +1484,17 @@ def test_radius_user_group_detail(self): self.assertEqual(rug.group, org1_power_users_group) self.assertEqual(rug.priority, 3) + with self.subTest("PUT without group field"): + response = self.client.put( + url, + {"priority": 4}, + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + rug.refresh_from_db() + self.assertEqual(rug.group, org1_power_users_group) + self.assertEqual(rug.priority, 4) + with self.subTest("DELETE operation"): response = self.client.delete(url) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) From 81714c685346c948d2e4a45a3b8728fe6d7d35a8 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 9 Mar 2026 15:29:32 +0530 Subject: [PATCH 08/10] [fix] Made requested changes --- openwisp_radius/api/serializers.py | 14 ++++++++------ openwisp_radius/tests/test_api/test_api.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index f74d5d43..de8b85ae 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -373,17 +373,19 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + view = self.context.get("view") if ( - self.context.get("view") - and getattr(self.context["view"], "get_parent_queryset", None) - and not getattr(self.context["view"], "swagger_fake_view", False) + view + and getattr(view, "get_parent_queryset", None) + and not getattr(view, "swagger_fake_view", False) ): - self._user = self.context["view"].get_parent_queryset().first() + self._user = view.get_parent_queryset().first() else: self._user = None - if self._user: + if self._user and view and getattr(view.request, "user", None): + orgs = view.request.user.organizations_managed self.fields["group"].queryset = self.fields["group"].queryset.filter( - organization_id__in=self._user.organizations_dict.keys() + organization__in=orgs ) else: self.fields["group"].queryset = self.fields["group"].queryset.none() diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index 00906de0..d0a6f3d5 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -1495,6 +1495,21 @@ def test_radius_user_group_detail(self): self.assertEqual(rug.group, org1_power_users_group) self.assertEqual(rug.priority, 4) + with self.subTest("Org manager cannot assign group from another org"): + self._create_org_user(user=target_user, organization=org2) + org2_group = RadiusGroup.objects.get(organization=org2, name="org-2-users") + response = self.client.put( + url, + {"group": str(org2_group.pk), "priority": 8}, + content_type="application/json", + ) + self.assertEqual( + response.status_code, + status.HTTP_400_BAD_REQUEST, + ) + rug.refresh_from_db() + self.assertEqual(rug.group, org1_power_users_group) + with self.subTest("DELETE operation"): response = self.client.delete(url) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) From 33f55482383a348b6b2344126429c2079d0bccd2 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 9 Mar 2026 23:23:27 +0530 Subject: [PATCH 09/10] [fix] Fixed failing test --- openwisp_radius/api/serializers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index de8b85ae..9123bf29 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -384,8 +384,10 @@ def __init__(self, *args, **kwargs): self._user = None if self._user and view and getattr(view.request, "user", None): orgs = view.request.user.organizations_managed - self.fields["group"].queryset = self.fields["group"].queryset.filter( - organization__in=orgs + self.fields["group"].queryset = ( + self.fields["group"] + .queryset.filter(organization__in=orgs) + .filter(organization__in=self._user.organizations_dict.keys()) ) else: self.fields["group"].queryset = self.fields["group"].queryset.none() From 783bf3586f9fd3f35c12a7389bd96ee5b679db97 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 10 Mar 2026 13:30:40 +0530 Subject: [PATCH 10/10] [fix] Added comments --- openwisp_radius/api/serializers.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 9123bf29..b9b01165 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -383,11 +383,12 @@ def __init__(self, *args, **kwargs): else: self._user = None if self._user and view and getattr(view.request, "user", None): - orgs = view.request.user.organizations_managed - self.fields["group"].queryset = ( - self.fields["group"] - .queryset.filter(organization__in=orgs) - .filter(organization__in=self._user.organizations_dict.keys()) + # Restrict available groups to organizations that the request user manages + # and that the edited user belongs to. This prevents assigning groups from + # organizations outside the request user's management scope. + self.fields["group"].queryset = self.fields["group"].queryset.filter( + Q(organization__in=view.request.user.organizations_managed) + & Q(organization__in=self._user.organizations_dict.keys()) ) else: self.fields["group"].queryset = self.fields["group"].queryset.none()