From 3470f7d868f7edb9d7495d1441af2620ac3a19c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Tue, 31 Mar 2026 12:24:06 -0600 Subject: [PATCH 01/11] feat: Implemented team member assignments endpoint --- CHANGELOG.rst | 8 + openedx_authz/__init__.py | 2 +- openedx_authz/api/data.py | 14 + openedx_authz/api/users.py | 69 +++++ openedx_authz/rest_api/data.py | 8 + openedx_authz/rest_api/utils.py | 37 ++- openedx_authz/rest_api/v1/filters.py | 13 +- openedx_authz/rest_api/v1/serializers.py | 65 ++++- openedx_authz/rest_api/v1/urls.py | 1 + openedx_authz/rest_api/v1/views.py | 62 ++++- openedx_authz/tests/rest_api/test_utils.py | 26 ++ openedx_authz/tests/rest_api/test_views.py | 308 ++++++++++++++++++++- 12 files changed, 603 insertions(+), 10 deletions(-) create mode 100644 openedx_authz/tests/rest_api/test_utils.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1dc83133..bf590705 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,14 @@ Change Log Unreleased ********** +1.8.0 - 2026-04-14 +****************** + +Added +===== + +* Add the ``/api/authz/v1/users//assignments`` endpoint to get a list of role assignations for a user. + 1.7.0 - 2026-04-14 ****************** diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index 2a978b1f..1beebf98 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "1.7.0" +__version__ = "1.8.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 8caa450f..6b173248 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -37,6 +37,7 @@ "RoleData", "ScopeData", "SubjectData", + "SuperAdminAssignmentData", "UserData", ] @@ -1119,6 +1120,19 @@ def __repr__(self): return f"{self.subject.namespaced_key} => [{role_keys}] @ {self.scope.namespaced_key}" +@define +class SuperAdminAssignmentData: + """Represents a superadmin entry in a team member assignment list. + + Used alongside RoleAssignmentData in serializer contexts where a user is a + staff/superuser and their access is not derived from a specific role assignment. + """ + + subject: SubjectData = None + is_staff: bool = False + is_superuser: bool = False + + @define class UserAssignments: """A user with their role assignments""" diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index 6edd736d..c1d9fcc9 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -17,6 +17,7 @@ RoleAssignmentData, RoleData, ScopeData, + SuperAdminAssignmentData, UserAssignments, UserAssignmentsFilter, UserData, @@ -39,6 +40,9 @@ from openedx_authz.api.utils import filter_user_assignments, get_user_assignment_map from openedx_authz.utils import get_user_by_username_or_email +User = get_user_model() + + __all__ = [ "assign_role_to_user_in_scope", "batch_assign_role_to_users_in_scope", @@ -47,6 +51,7 @@ "get_user_role_assignments", "get_user_role_assignments_in_scope", "get_user_role_assignments_for_role_in_scope", + "get_user_role_assignments_for_user_filtered", "get_user_role_assignments_filtered", "get_all_user_role_assignments_in_scope", "get_visible_role_assignments_for_user", @@ -55,6 +60,7 @@ "get_users_for_role_in_scope", "unassign_all_roles_from_user", "validate_users", + "get_superadmins", ] @@ -172,6 +178,42 @@ def get_user_role_assignments_for_role_in_scope( ) +def get_user_role_assignments_for_user_filtered( + user_external_key: str, + orgs: list[str] = None, + roles: list[str] = None, + allowed_for_user_external_key: str = None, +) -> list[RoleAssignmentData]: + """ + Get role assignments for a specific user, filtered by orgs and/or roles, + and only include assignments that the specified user has permission to view. + + Args: + user_external_key: The user to get assignments for (e.g., 'john_doe'). + orgs: Optional list of orgs to filter by (e.g., ['edX', 'MITx']). + roles: Optional list of roles to filter by (e.g., ['library_admin']). + allowed_for_user_external_key: The username to check permissions against (e.g., 'john_doe'). + + Returns: + list[RoleAssignmentData]: A list of role assignments for the user, filtered by orgs/roles and permissions. + """ + user_role_assignments = get_user_role_assignments(user_external_key=user_external_key) + # Filter assignments based on the user's permissions + user_role_assignments = _filter_allowed_assignments( + user_external_key=allowed_for_user_external_key, + assignments=user_role_assignments, + ) + if orgs: + # Filter by orgs + user_role_assignments = [a for a in user_role_assignments if a.scope.org in orgs] + if roles: + # Filter by roles + user_role_assignments = [ + a for a in user_role_assignments if any(role.external_key in roles for role in a.roles) + ] + return user_role_assignments + + def get_user_role_assignments_filtered( *, user_external_key: str | None = None, @@ -369,3 +411,30 @@ def validate_users(user_identifiers: list[str]) -> tuple[list[str], list[str]]: invalid_users.append(user_identifier) return valid_users, invalid_users + + +def get_superadmins(user_external_keys: list[str] | None = None) -> list[SuperAdminAssignmentData]: + """Returns all superadmins as SuperAdminAssignmentData. + + A superadmin is a User with a Django staff or superuser role. + Superadmins automatically are allowed to do any action. + + Args: + user_external_keys (list[str] or None): To filter by usernames + + Returns: + list[SuperAdminAssignmentData]: The superadmin data + """ + # Retrieve user data to check if they are a superusers + requested_users = User.objects.filter(username__in=user_external_keys, is_active=True) + superadmin_assignments: list[SuperAdminAssignmentData] = [] + for requested_user in requested_users: + if requested_user.is_staff or requested_user.is_superuser: + superadmin_assignments.append( + SuperAdminAssignmentData( + subject=UserData(external_key=requested_user.username), + is_staff=requested_user.is_staff, + is_superuser=requested_user.is_superuser, + ) + ) + return superadmin_assignments diff --git a/openedx_authz/rest_api/data.py b/openedx_authz/rest_api/data.py index 5af8afe1..74eb4432 100644 --- a/openedx_authz/rest_api/data.py +++ b/openedx_authz/rest_api/data.py @@ -20,6 +20,14 @@ class SortField(BaseEnum): EMAIL = "email" +class AssignmentSortField(BaseEnum): + """Enum for the role assignment fields to sort by.""" + + ROLE = "role" + ORG = "org" + SCOPE = "scope" + + class SortOrder(BaseEnum): """Enum for the order to sort by.""" diff --git a/openedx_authz/rest_api/utils.py b/openedx_authz/rest_api/utils.py index d7a6d9c1..3b8308ae 100644 --- a/openedx_authz/rest_api/utils.py +++ b/openedx_authz/rest_api/utils.py @@ -2,9 +2,10 @@ from openedx_authz.api.data import ( GLOBAL_SCOPE_WILDCARD, + RoleAssignmentData, ScopeData, ) -from openedx_authz.rest_api.data import SearchField, SortField, SortOrder +from openedx_authz.rest_api.data import AssignmentSortField, SearchField, SortField, SortOrder def get_generic_scope(scope: ScopeData) -> ScopeData: @@ -93,3 +94,37 @@ def filter_users(users: list[dict], search: str | None, roles: list[str] | None) filtered_users.append(user) return filtered_users + + +def sort_assignments( + assignments: list[RoleAssignmentData], + sort_by: AssignmentSortField = AssignmentSortField.ROLE, + order: SortOrder = SortOrder.ASC, +) -> list[RoleAssignmentData]: + """ + Sort role assignments by a given field and order. + + Args: + assignments (list[RoleAssignmentData]): The assignments to sort. + sort_by (SortField, optional): The field to sort by. Defaults to AssignmentSortField.ROLE. + order (SortOrder, optional): The order to sort by. Defaults to SortOrder.ASC. + + Raises: + ValueError: If the sort field is invalid. + ValueError: If the sort order is invalid. + + Returns: + list[RoleAssignmentData]: The sorted assignments. + """ + if sort_by not in AssignmentSortField.values(): + raise ValueError(f"Invalid field: '{sort_by}'. Must be one of {AssignmentSortField.values()}") + + if order not in SortOrder.values(): + raise ValueError(f"Invalid order: '{order}'. Must be one of {SortOrder.values()}") + + sorted_assignments = sorted( + assignments, + key=lambda assignment: (assignment.get(sort_by) or "").lower(), + reverse=order == SortOrder.DESC, + ) + return sorted_assignments diff --git a/openedx_authz/rest_api/v1/filters.py b/openedx_authz/rest_api/v1/filters.py index fd0878c0..ebf95396 100644 --- a/openedx_authz/rest_api/v1/filters.py +++ b/openedx_authz/rest_api/v1/filters.py @@ -2,8 +2,8 @@ from rest_framework.filters import BaseFilterBackend -from openedx_authz.rest_api.data import SortField, SortOrder -from openedx_authz.rest_api.utils import filter_users, sort_users +from openedx_authz.rest_api.data import AssignmentSortField, SortField, SortOrder +from openedx_authz.rest_api.utils import filter_users, sort_assignments, sort_users class TeamMemberSearchFilter(BaseFilterBackend): @@ -21,3 +21,12 @@ def filter_queryset(self, request, queryset, view): sort_by = request.query_params.get("sort_by", SortField.USERNAME) order = request.query_params.get("order", SortOrder.ASC) return sort_users(users=queryset, sort_by=sort_by, order=order) + + +class TeamMemberAssignmentsOrderingFilter(BaseFilterBackend): + """Sort team member assignments by a given field and order.""" + + def filter_queryset(self, request, queryset, view): + sort_by = request.query_params.get("sort_by", AssignmentSortField.ROLE) + order = request.query_params.get("order", SortOrder.ASC) + return sort_assignments(assignments=queryset, sort_by=sort_by, order=order) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 3f6c8b39..0ae93bae 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -5,7 +5,7 @@ from openedx_authz import api from openedx_authz.api.data import UserAssignments -from openedx_authz.rest_api.data import SortField, SortOrder +from openedx_authz.rest_api.data import AssignmentSortField, SortField, SortOrder from openedx_authz.rest_api.utils import get_generic_scope from openedx_authz.rest_api.v1.fields import ( CaseSensitiveCommaSeparatedListField, @@ -292,3 +292,66 @@ class UserValidationAPIViewResponseSerializer(serializers.Serializer): # pylint help_text="List of user identifiers that were not found or are invalid", ) summary = UserValidationSummarySerializer(help_text="Summary statistics for the validation operation") + + +class ListTeamMemberAssignmentsSerializer(serializers.Serializer): # pylint: disable=abstract-method + """Serializer for listing team member assignments.""" + + orgs = CaseSensitiveCommaSeparatedListField(required=False, default=[]) + roles = CaseSensitiveCommaSeparatedListField(required=False, default=[]) + sort_by = serializers.ChoiceField( + required=False, + choices=[(e.value, e.name) for e in AssignmentSortField], + default=AssignmentSortField.ROLE, + ) + order = serializers.ChoiceField( + required=False, + choices=[(e.value, e.name) for e in SortOrder], + default=SortOrder.ASC, + ) + + +class TeamMemberAssignmentSerializer(serializers.Serializer): # pylint: disable=abstract-method + """Serializer for team member assignments.""" + + is_superadmin = serializers.SerializerMethodField() + role = serializers.SerializerMethodField() + org = serializers.SerializerMethodField() + scope = serializers.SerializerMethodField() + permission_count = serializers.SerializerMethodField() + + def get_is_superadmin(self, obj: api.RoleAssignmentData | api.SuperAdminAssignmentData) -> bool: + """Get whether this assignment entry is for a superadmin.""" + return isinstance(obj, api.SuperAdminAssignmentData) + + def get_role(self, obj: api.RoleAssignmentData | api.SuperAdminAssignmentData) -> str: + """Get the role for the given role assignment.""" + match obj: + case api.SuperAdminAssignmentData(): + return "django.superuser" if obj.is_superuser else "django.staff" + case api.RoleAssignmentData(): + return obj.roles[0].external_key if obj.roles else "" + + def get_org(self, obj: api.RoleAssignmentData | api.SuperAdminAssignmentData) -> str: + """Get the org for the given role assignment.""" + match obj: + case api.SuperAdminAssignmentData(): + return "*" + case api.RoleAssignmentData(): + return getattr(obj.scope, "org", None) + + def get_scope(self, obj: api.RoleAssignmentData | api.SuperAdminAssignmentData) -> str: + """Get the scope for the given role assignment.""" + match obj: + case api.SuperAdminAssignmentData(): + return "*" + case api.RoleAssignmentData(): + return obj.scope.external_key + + def get_permission_count(self, obj: api.RoleAssignmentData | api.SuperAdminAssignmentData) -> int | None: + """Get the permission count for the given role assignment.""" + match obj: + case api.SuperAdminAssignmentData(): + return None + case api.RoleAssignmentData(): + return len(obj.roles[0].permissions) if obj.roles else 0 diff --git a/openedx_authz/rest_api/v1/urls.py b/openedx_authz/rest_api/v1/urls.py index 8dbca5c5..40f846a6 100644 --- a/openedx_authz/rest_api/v1/urls.py +++ b/openedx_authz/rest_api/v1/urls.py @@ -15,4 +15,5 @@ path("orgs/", views.AdminConsoleOrgsAPIView.as_view(), name="orgs-list"), path("users/", views.TeamMembersAPIView.as_view(), name="user-list"), path("users/validate/", views.UserValidationAPIView.as_view(), name="user-validation"), + path("users//assignments", views.TeamMemberAssignmentsAPIView.as_view(), name="user-assignment-list"), ] diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index 26bdd2a4..b3459212 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -20,6 +20,8 @@ from rest_framework.views import APIView from openedx_authz import api +from openedx_authz.api.data import RoleAssignmentData, SuperAdminAssignmentData +from openedx_authz.api.users import get_superadmins, get_user_role_assignments_for_user_filtered from openedx_authz.api.utils import get_user_map from openedx_authz.constants import permissions from openedx_authz.rest_api.data import RoleOperationError, RoleOperationStatus @@ -29,18 +31,24 @@ get_generic_scope, sort_users, ) -from openedx_authz.rest_api.v1.filters import TeamMemberOrderingFilter, TeamMemberSearchFilter +from openedx_authz.rest_api.v1.filters import ( + TeamMemberAssignmentsOrderingFilter, + TeamMemberOrderingFilter, + TeamMemberSearchFilter, +) from openedx_authz.rest_api.v1.paginators import AuthZAPIViewPagination from openedx_authz.rest_api.v1.permissions import AnyScopePermission, DynamicScopePermission from openedx_authz.rest_api.v1.serializers import ( AddUsersToRoleWithScopeSerializer, ListRolesWithScopeResponseSerializer, ListRolesWithScopeSerializer, + ListTeamMemberAssignmentsSerializer, ListTeamMembersSerializer, ListUsersInRoleWithScopeSerializer, PermissionValidationResponseSerializer, PermissionValidationSerializer, RemoveUsersFromRoleWithScopeSerializer, + TeamMemberAssignmentSerializer, TeamMemberSerializer, UserRoleAssignmentSerializer, UserValidationAPIViewResponseSerializer, @@ -675,3 +683,55 @@ def post(self, request: HttpRequest) -> Response: } response_serializer = UserValidationAPIViewResponseSerializer(response_data) return Response(response_serializer.data, status=status.HTTP_200_OK) + + +@view_auth_classes() +class TeamMemberAssignmentsAPIView(APIView): + """ + API view for listing user role assignments + """ + + pagination_class = AuthZAPIViewPagination + filter_backends = [TeamMemberAssignmentsOrderingFilter] + + @apidocs.schema( + parameters=[ + apidocs.query_parameter("orgs", str, description="The orgs to query assignations for"), + apidocs.query_parameter("roles", str, description="The roles to query assignations for"), + apidocs.query_parameter("sort_by", str, description="The field to sort by"), + apidocs.query_parameter("order", str, description="The order to sort by"), + apidocs.query_parameter("page", int, description="Page number for pagination"), + apidocs.query_parameter("page_size", int, description="Number of items per page"), + ], + responses={ + status.HTTP_200_OK: TeamMemberAssignmentSerializer(many=True), + status.HTTP_400_BAD_REQUEST: "The request parameters are invalid", + status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", + }, + ) + def get(self, request: HttpRequest, username: str) -> Response: + """Retrieve all user role assignments.""" + serializer = ListTeamMemberAssignmentsSerializer(data=request.query_params) + serializer.is_valid(raise_exception=True) + query_params = serializer.validated_data + + user_role_assignments: list[RoleAssignmentData | SuperAdminAssignmentData] = [] + + # Retrieve superadmin assignments (django staff or superuser users), as they always have access to everything + user_role_assignments += get_superadmins(user_external_keys=[username]) + + user_role_assignments += get_user_role_assignments_for_user_filtered( + user_external_key=username, + orgs=query_params.get("orgs"), + roles=query_params.get("roles"), + allowed_for_user_external_key=request.user.username, + ) + + assignments = TeamMemberAssignmentSerializer(user_role_assignments, many=True).data + for backend in self.filter_backends: + assignments = backend().filter_queryset(request, assignments, self) + + # Paginate + paginator = self.pagination_class() + paginated_response_data = paginator.paginate_queryset(assignments, request) + return paginator.get_paginated_response(paginated_response_data) diff --git a/openedx_authz/tests/rest_api/test_utils.py b/openedx_authz/tests/rest_api/test_utils.py new file mode 100644 index 00000000..1678eaec --- /dev/null +++ b/openedx_authz/tests/rest_api/test_utils.py @@ -0,0 +1,26 @@ +"""Unit tests for openedx_authz.rest_api.utils.""" + +from django.test import TestCase + +from openedx_authz.rest_api.data import AssignmentSortField +from openedx_authz.rest_api.utils import sort_assignments + + +class TestSortAssignments(TestCase): + """Tests for sort_assignments.""" + + def test_invalid_sort_field_raises_value_error(self): + """Passing an unrecognised sort_by value raises ValueError.""" + with self.assertRaises(ValueError) as ctx: + sort_assignments(assignments=[], sort_by="invalid_field") + + self.assertIn("invalid_field", str(ctx.exception)) + self.assertIn("Invalid field", str(ctx.exception)) + + def test_invalid_sort_order_raises_value_error(self): + """Passing an unrecognised order value raises ValueError.""" + with self.assertRaises(ValueError) as ctx: + sort_assignments(assignments=[], sort_by=AssignmentSortField.ROLE, order="invalid_order") + + self.assertIn("invalid_order", str(ctx.exception)) + self.assertIn("Invalid order", str(ctx.exception)) diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index bd97aa60..5be7061a 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -1070,13 +1070,11 @@ class TestTeamMembersAPIView(ViewTestMixin): - regular_1 (library_user in Org1:LIB1): VIEW_LIBRARY_TEAM granted → sees Org1 members (3) - regular_3 (library_user in Org2:LIB2): VIEW_LIBRARY_TEAM granted → sees Org2 members (3) - regular_6 (library_author in Org3:LIB3): VIEW_LIBRARY_TEAM granted → sees Org3 members (5) - - regular_9 (no assignments): sees 0 users """ def setUp(self): """Set up test fixtures.""" super().setUp() - self.url = reverse("openedx_authz:user-list") self.get_user_map_patcher = patch( "openedx_authz.api.utils.get_user_map", @@ -1129,8 +1127,7 @@ def test_unauthenticated_returns_401(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - # -------------------------------------------------------------------- # - + # -------------------------------------------------------------------- # # Filter by scopes # # -------------------------------------------------------------------- # @@ -1310,6 +1307,309 @@ def test_response_shape(self): self.assertEqual(item["assignation_count"], 1) +@ddt +class TestTeamMemberAssignmentsAPIView(ViewTestMixin): + """ + Test suite for TeamMemberAssignmentsAPIView. + + Setup summary (from ViewTestMixin.setUpClass): + lib:Org1:LIB1 → admin_1 (library_admin), regular_1 (library_user), regular_2 (library_user) + lib:Org2:LIB2 → admin_2 (library_user), regular_3 (library_user), regular_4 (library_user) + lib:Org3:LIB3 → admin_3 (library_admin), regular_5 (library_admin), regular_6 (library_author), + regular_7 (library_contributor), regular_8 (library_user) + + URL: /authz/v1/users//assignments + Response fields per item: is_superadmin, role, org, scope, permission_count + + Superadmin entry: + admin_1..3 are staff/superusers. Querying any of them always prepends one + SuperAdminAssignmentData entry: role="django.superuser" (or "django.staff"), + org="*", scope="*", permission_count=None, is_superadmin=True. + This entry is always included regardless of org/role filters, since those + filters are applied only to the role assignments, not to the superadmin entry. + + Visibility via filter_allowed_assignments: + - Staff/superuser: sees all assignments for any user + - regular_1 (library_user in Org1:LIB1): sees only Org1:LIB1 assignments + - regular_9 (no assignments): sees nothing for any user + """ + + def setUp(self): + """Set up test fixtures.""" + super().setUp() + self.get_user_map_patcher = patch( + "openedx_authz.api.utils.get_user_map", + side_effect=get_user_map_without_profile, + ) + self.get_user_map_patcher.start() + self.addCleanup(self.get_user_map_patcher.stop) + + def _url(self, username: str) -> str: + return reverse("openedx_authz:user-assignment-list", kwargs={"username": username}) + + # -------------------------------------------------------------------- # + # Visibility: calling user only sees assignments it has view access to # + # -------------------------------------------------------------------- # + + @data( + # Staff/superuser targets get 1 superadmin entry + their role assignment(s) + ("admin_1", "admin_1", 2), # superadmin entry + library_admin in Org1 + ("admin_1", "admin_2", 2), # superadmin entry + library_user in Org2 + ("admin_1", "admin_3", 2), # superadmin entry + library_admin in Org3 + # Regular user targets get only their role assignments (no superadmin entry) + ("admin_1", "regular_5", 1), + # The superadmin entry is always included for superadmin targets, visible to all callers + ("regular_1", "admin_1", 2), # superadmin entry + library_admin in Org1 (visible via Org1 access) + # regular_1 cannot see admin_2's Org2 role assignment, but superadmin entry is still included + ("regular_1", "admin_2", 1), # superadmin entry only + # regular_9 has no assignments but superadmin entry is still included for admin targets + ("regular_9", "admin_1", 1), # superadmin entry only + ) + @unpack + def test_visibility_limited_to_accessible_scopes(self, caller: str, target: str, expected_count: int): + """Calling user only sees role assignments for scopes it has view access to. + + The superadmin entry is always included when the target is a superadmin, + regardless of the calling user's permissions. + + Expected result: + - Superadmin targets always include the superadmin entry. + - Role assignments are filtered by the calling user's permissions. + - Regular user targets return only their visible role assignments. + """ + self.client.force_authenticate(user=User.objects.get(username=caller)) + + response = self.client.get(self._url(target)) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], expected_count) + + def test_unauthenticated_returns_401(self): + """Unauthenticated requests are rejected. + + Expected result: + - Returns 401 UNAUTHORIZED. + """ + self.client.force_authenticate(user=None) + + response = self.client.get(self._url("admin_1")) + + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_unknown_user_returns_empty(self): + """Requesting assignments for a non-existent user returns an empty list. + + Expected result: + - Returns 200 OK with count 0. + """ + response = self.client.get(self._url("nonexistent_user")) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 0) + + # ------------------------------------------------------------------ # + # Filter by orgs # + # ------------------------------------------------------------------ # + + @data( + # admin_3 has library_admin in lib:Org3:LIB3; superadmin entry is always included + ("admin_3", "Org3", 2), # superadmin entry + Org3 role assignment + ("admin_3", "Org1", 1), # superadmin entry only (no Org1 role assignment) + # regular_5 has library_admin in lib:Org3:LIB3 (no superadmin entry) + ("regular_5", "Org3", 1), + ("regular_5", "Org1", 0), + # non-existent org: superadmin entry still included for admin targets + ("admin_1", "OrgX", 1), # superadmin entry only + ) + @unpack + def test_filter_by_orgs(self, target: str, orgs: str, expected_count: int): + """Results are filtered to the requested orgs. + + Expected result: + - Only assignments in the given org(s) are returned. + - Multiple orgs are OR-combined. + """ + response = self.client.get(self._url(target), {"orgs": orgs}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], expected_count) + + def test_filter_by_multiple_orgs(self): + """Multiple orgs are OR-combined. + + Expected result: + - Returns assignments matching any of the given orgs. + """ + # regular_6 has library_author in lib:Org3:LIB3 + # regular_7 has library_contributor in lib:Org3:LIB3 + # Use admin_1 (staff) to see all of regular_8's assignments + # regular_8 has library_user in lib:Org3:LIB3 only + response = self.client.get(self._url("regular_8"), {"orgs": "Org1,Org3"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 1) + + # ------------------------------------------------------------------ # + # Filter by roles # + # ------------------------------------------------------------------ # + + @data( + # role filter applies only to role assignments; superadmin entry is always included for admin targets + ("admin_1", roles.LIBRARY_ADMIN.external_key, 2), # superadmin entry + library_admin + ("admin_1", roles.LIBRARY_USER.external_key, 1), # superadmin entry only + ("regular_5", roles.LIBRARY_ADMIN.external_key, 1), + ("regular_5", roles.LIBRARY_USER.external_key, 0), + ("regular_6", roles.LIBRARY_AUTHOR.external_key, 1), + ("regular_6", roles.LIBRARY_ADMIN.external_key, 0), + ("admin_1", "non_existent_role", 1), # superadmin entry only + ) + @unpack + def test_filter_by_roles(self, target: str, role_filter: str, expected_count: int): + """Results are filtered to the requested roles. + + Expected result: + - Only assignments with the given role(s) are returned. + """ + response = self.client.get(self._url(target), {"roles": role_filter}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], expected_count) + + def test_filter_by_multiple_roles(self): + """Multiple roles are OR-combined for role assignments; superadmin entry always included. + + Expected result: + - Returns assignments matching any of the given roles, plus the superadmin entry. + """ + # admin_3 has library_admin in Org3:LIB3; filter for admin + author returns + # 1 role assignment + 1 superadmin entry = 2 + response = self.client.get( + self._url("admin_3"), + {"roles": f"{roles.LIBRARY_ADMIN.external_key},{roles.LIBRARY_AUTHOR.external_key}"}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 2) + + # ------------------------------------------------------------------ # + # Sorting # + # ------------------------------------------------------------------ # + + @data( + ("role", "asc"), + ("role", "desc"), + ("org", "asc"), + ("org", "desc"), + ("scope", "asc"), + ("scope", "desc"), + ) + @unpack + def test_sorting(self, sort_by: str, order: str): + """Results can be sorted by role, org, or scope in asc/desc order. + + Uses regular_3 and regular_4 who both have library_user in Org2:LIB2, + and admin_2 who also has library_user in Org2:LIB2 — but we need a user + with multiple assignments to verify ordering. Use admin_1 (staff) viewing + regular_5 who has a single assignment; sorting still returns 200 OK. + + Expected result: + - Returns 200 OK. + - Results are ordered according to the requested field and direction. + """ + # admin_1 is staff so sees all assignments; regular_5 has 1 assignment + response = self.client.get(self._url("regular_5"), {"sort_by": sort_by, "order": order}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + values = [item[sort_by] for item in response.data["results"]] + expected = sorted(values, key=lambda v: (v or "").lower(), reverse=order == "desc") + self.assertEqual(values, expected) + + @data( + {"sort_by": "invalid"}, + {"sort_by": "username"}, + {"order": "ascending"}, + {"order": "descending"}, + ) + def test_sorting_invalid_params(self, query_params: dict): + """Invalid sort_by or order values return 400. + + Expected result: + - Returns 400 BAD REQUEST. + """ + response = self.client.get(self._url("admin_1"), query_params) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + # ------------------------------------------------------------------ # + # Pagination # + # ------------------------------------------------------------------ # + + @data( + ({"page": 1, "page_size": 1}, 1, True), + ({"page": 2, "page_size": 1}, 1, False), + ({"page": 1, "page_size": 2}, 2, False), + ) + @unpack + def test_pagination(self, query_params: dict, expected_page_count: int, has_next: bool): + """Results are paginated correctly. + + Assigns regular_8 a second role (library_admin in Org1:LIB1) so it has + 2 assignments visible to admin_1 (staff). + + Expected result: + - Returns 200 OK. + - Page contains the expected number of items. + - `next` link is present only when more pages exist. + """ + assign_role_to_user_in_scope("regular_8", roles.LIBRARY_ADMIN.external_key, "lib:Org1:LIB1") + + response = self.client.get(self._url("regular_8"), query_params) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data["results"]), expected_page_count) + if has_next: + self.assertIsNotNone(response.data["next"]) + else: + self.assertIsNone(response.data["next"]) + + # ------------------------------------------------------------------ # + # Response shape # + # ------------------------------------------------------------------ # + + def test_response_shape(self): + """Each result item contains the expected fields. + + admin_1 is a superuser, so the response contains two items: + - A superadmin entry with role="django.superuser", org="*", scope="*", + permission_count=None, is_superadmin=True + - A regular role assignment entry with concrete values and is_superadmin=False + + Expected result: + - Returns 200 OK. + - Each item has is_superadmin, role, org, scope, and permission_count. + """ + response = self.client.get(self._url("admin_1")) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 2) + + superadmin_item = next(item for item in response.data["results"] if item["is_superadmin"]) + self.assertIn(superadmin_item["role"], ("django.superuser", "django.staff")) + self.assertEqual(superadmin_item["org"], "*") + self.assertEqual(superadmin_item["scope"], "*") + self.assertIsNone(superadmin_item["permission_count"]) + + role_item = next(item for item in response.data["results"] if not item["is_superadmin"]) + self.assertIn("role", role_item) + self.assertIn("org", role_item) + self.assertIn("scope", role_item) + self.assertIn("permission_count", role_item) + self.assertEqual(role_item["role"], roles.LIBRARY_ADMIN.external_key) + self.assertEqual(role_item["org"], "Org1") + self.assertEqual(role_item["scope"], "lib:Org1:LIB1") + self.assertGreater(role_item["permission_count"], 0) + + @ddt class TestRoleListView(ViewTestMixin): """Test suite for RoleListView.""" From 73bc75e66b55ecd0cbab6167681e6178db35dd17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 10 Apr 2026 16:26:28 -0600 Subject: [PATCH 02/11] squash!: Improve namings --- openedx_authz/api/users.py | 8 ++++---- openedx_authz/rest_api/v1/views.py | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index c1d9fcc9..98f7b07d 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -51,16 +51,16 @@ "get_user_role_assignments", "get_user_role_assignments_in_scope", "get_user_role_assignments_for_role_in_scope", - "get_user_role_assignments_for_user_filtered", "get_user_role_assignments_filtered", "get_all_user_role_assignments_in_scope", "get_visible_role_assignments_for_user", + "get_visible_specific_user_role_assignments_for_user", "is_user_allowed", "get_scopes_for_user_and_permission", "get_users_for_role_in_scope", "unassign_all_roles_from_user", "validate_users", - "get_superadmins", + "get_superadmin_assignments", ] @@ -178,7 +178,7 @@ def get_user_role_assignments_for_role_in_scope( ) -def get_user_role_assignments_for_user_filtered( +def get_visible_specific_user_role_assignments_for_user( user_external_key: str, orgs: list[str] = None, roles: list[str] = None, @@ -413,7 +413,7 @@ def validate_users(user_identifiers: list[str]) -> tuple[list[str], list[str]]: return valid_users, invalid_users -def get_superadmins(user_external_keys: list[str] | None = None) -> list[SuperAdminAssignmentData]: +def get_superadmin_assignments(user_external_keys: list[str] | None = None) -> list[SuperAdminAssignmentData]: """Returns all superadmins as SuperAdminAssignmentData. A superadmin is a User with a Django staff or superuser role. diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index b3459212..65ba5fee 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -21,7 +21,7 @@ from openedx_authz import api from openedx_authz.api.data import RoleAssignmentData, SuperAdminAssignmentData -from openedx_authz.api.users import get_superadmins, get_user_role_assignments_for_user_filtered +from openedx_authz.api.users import get_superadmin_assignments, get_visible_specific_user_role_assignments_for_user from openedx_authz.api.utils import get_user_map from openedx_authz.constants import permissions from openedx_authz.rest_api.data import RoleOperationError, RoleOperationStatus @@ -718,9 +718,9 @@ def get(self, request: HttpRequest, username: str) -> Response: user_role_assignments: list[RoleAssignmentData | SuperAdminAssignmentData] = [] # Retrieve superadmin assignments (django staff or superuser users), as they always have access to everything - user_role_assignments += get_superadmins(user_external_keys=[username]) + user_role_assignments += get_superadmin_assignments(user_external_keys=[username]) - user_role_assignments += get_user_role_assignments_for_user_filtered( + user_role_assignments += get_visible_specific_user_role_assignments_for_user( user_external_key=username, orgs=query_params.get("orgs"), roles=query_params.get("roles"), From 76f3a658f1f867516cdbc1a61edf6158084cea79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 10 Apr 2026 16:53:35 -0600 Subject: [PATCH 03/11] squash!: Apply suggestions --- openedx_authz/api/users.py | 8 ++++++-- openedx_authz/rest_api/v1/urls.py | 4 +++- openedx_authz/tests/rest_api/test_views.py | 16 ++++++++-------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index 98f7b07d..6ad6313e 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -205,7 +205,7 @@ def get_visible_specific_user_role_assignments_for_user( ) if orgs: # Filter by orgs - user_role_assignments = [a for a in user_role_assignments if a.scope.org in orgs] + user_role_assignments = [a for a in user_role_assignments if getattr(a.scope, "org", None) in orgs] if roles: # Filter by roles user_role_assignments = [ @@ -426,7 +426,11 @@ def get_superadmin_assignments(user_external_keys: list[str] | None = None) -> l list[SuperAdminAssignmentData]: The superadmin data """ # Retrieve user data to check if they are a superusers - requested_users = User.objects.filter(username__in=user_external_keys, is_active=True) + if user_external_keys is None: + requested_users = User.objects.filter(is_active=True) + else: + requested_users = User.objects.filter(username__in=user_external_keys, is_active=True) + superadmin_assignments: list[SuperAdminAssignmentData] = [] for requested_user in requested_users: if requested_user.is_staff or requested_user.is_superuser: diff --git a/openedx_authz/rest_api/v1/urls.py b/openedx_authz/rest_api/v1/urls.py index 40f846a6..c2608145 100644 --- a/openedx_authz/rest_api/v1/urls.py +++ b/openedx_authz/rest_api/v1/urls.py @@ -15,5 +15,7 @@ path("orgs/", views.AdminConsoleOrgsAPIView.as_view(), name="orgs-list"), path("users/", views.TeamMembersAPIView.as_view(), name="user-list"), path("users/validate/", views.UserValidationAPIView.as_view(), name="user-validation"), - path("users//assignments", views.TeamMemberAssignmentsAPIView.as_view(), name="user-assignment-list"), + path( + "users//assignments/", views.TeamMemberAssignmentsAPIView.as_view(), name="user-assignment-list" + ), ] diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index 5be7061a..2b535f81 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -1322,7 +1322,7 @@ class TestTeamMemberAssignmentsAPIView(ViewTestMixin): Response fields per item: is_superadmin, role, org, scope, permission_count Superadmin entry: - admin_1..3 are staff/superusers. Querying any of them always prepends one + admin_1..3 are staff/superusers. Querying any of them adds one entry SuperAdminAssignmentData entry: role="django.superuser" (or "django.staff"), org="*", scope="*", permission_count=None, is_superadmin=True. This entry is always included regardless of org/role filters, since those @@ -1505,21 +1505,21 @@ def test_filter_by_multiple_roles(self): ) @unpack def test_sorting(self, sort_by: str, order: str): - """Results can be sorted by role, org, or scope in asc/desc order. + """Results are sorted by role, org, or scope in asc/desc order. - Uses regular_3 and regular_4 who both have library_user in Org2:LIB2, - and admin_2 who also has library_user in Org2:LIB2 — but we need a user - with multiple assignments to verify ordering. Use admin_1 (staff) viewing - regular_5 who has a single assignment; sorting still returns 200 OK. + Uses admin_3, who has 2 items in the response: a superadmin entry + (role="django.superuser", org="*", scope="*") and a role assignment + (role="library_admin", org="Org3", scope="lib:Org3:LIB3"). With two + distinct values per field the sort order is non-trivial and verifiable. Expected result: - Returns 200 OK. - Results are ordered according to the requested field and direction. """ - # admin_1 is staff so sees all assignments; regular_5 has 1 assignment - response = self.client.get(self._url("regular_5"), {"sort_by": sort_by, "order": order}) + response = self.client.get(self._url("admin_3"), {"sort_by": sort_by, "order": order}) self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertGreater(len(response.data["results"]), 1) values = [item[sort_by] for item in response.data["results"]] expected = sorted(values, key=lambda v: (v or "").lower(), reverse=order == "desc") self.assertEqual(values, expected) From aaa9f288ee46e3be84366d7607417b8e8d2dce49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 10 Apr 2026 17:19:13 -0600 Subject: [PATCH 04/11] squash!: Apply suggestions --- CHANGELOG.rst | 2 +- openedx_authz/api/users.py | 28 +++++++++++++----------- openedx_authz/rest_api/v1/serializers.py | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bf590705..8736370a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,7 +20,7 @@ Unreleased Added ===== -* Add the ``/api/authz/v1/users//assignments`` endpoint to get a list of role assignations for a user. +* Add the ``/api/authz/v1/users//assignments/`` endpoint to get a list of role assignations for a user. 1.7.0 - 2026-04-14 ****************** diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index 6ad6313e..23a9ca01 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -10,6 +10,7 @@ """ from django.contrib.auth import get_user_model +from django.db.models import Q from openedx_authz.api.data import ( ActionData, @@ -256,11 +257,14 @@ def get_all_user_role_assignments_in_scope( def _filter_allowed_assignments( - user_external_key: str, assignments: list[RoleAssignmentData] + assignments: list[RoleAssignmentData], user_external_key: str = None ) -> list[RoleAssignmentData]: """ Filter the given role assignments to only include those that the user has permission to view. """ + if not user_external_key: + # If no user is specified, return all assignments + return assignments allowed_assignments: list[RoleAssignmentData] = [] for assignment in assignments: permission = None @@ -425,20 +429,18 @@ def get_superadmin_assignments(user_external_keys: list[str] | None = None) -> l Returns: list[SuperAdminAssignmentData]: The superadmin data """ - # Retrieve user data to check if they are a superusers - if user_external_keys is None: - requested_users = User.objects.filter(is_active=True) - else: - requested_users = User.objects.filter(username__in=user_external_keys, is_active=True) + superadmin_filter = Q(is_active=True) & (Q(is_staff=True) | Q(is_superuser=True)) + if user_external_keys is not None: + superadmin_filter &= Q(username__in=user_external_keys) + requested_users = User.objects.filter(superadmin_filter) superadmin_assignments: list[SuperAdminAssignmentData] = [] for requested_user in requested_users: - if requested_user.is_staff or requested_user.is_superuser: - superadmin_assignments.append( - SuperAdminAssignmentData( - subject=UserData(external_key=requested_user.username), - is_staff=requested_user.is_staff, - is_superuser=requested_user.is_superuser, - ) + superadmin_assignments.append( + SuperAdminAssignmentData( + subject=UserData(external_key=requested_user.username), + is_staff=requested_user.is_staff, + is_superuser=requested_user.is_superuser, ) + ) return superadmin_assignments diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 0ae93bae..fb00d179 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -338,7 +338,7 @@ def get_org(self, obj: api.RoleAssignmentData | api.SuperAdminAssignmentData) -> case api.SuperAdminAssignmentData(): return "*" case api.RoleAssignmentData(): - return getattr(obj.scope, "org", None) + return getattr(obj.scope, "org", "") def get_scope(self, obj: api.RoleAssignmentData | api.SuperAdminAssignmentData) -> str: """Get the scope for the given role assignment.""" From eab726967738c177b26b77122400086b4d8e2a6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 10 Apr 2026 17:38:18 -0600 Subject: [PATCH 05/11] squash!: Apply suggestions --- openedx_authz/rest_api/utils.py | 9 ++++----- openedx_authz/tests/rest_api/test_views.py | 13 ++++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/openedx_authz/rest_api/utils.py b/openedx_authz/rest_api/utils.py index 3b8308ae..19904731 100644 --- a/openedx_authz/rest_api/utils.py +++ b/openedx_authz/rest_api/utils.py @@ -2,7 +2,6 @@ from openedx_authz.api.data import ( GLOBAL_SCOPE_WILDCARD, - RoleAssignmentData, ScopeData, ) from openedx_authz.rest_api.data import AssignmentSortField, SearchField, SortField, SortOrder @@ -97,15 +96,15 @@ def filter_users(users: list[dict], search: str | None, roles: list[str] | None) def sort_assignments( - assignments: list[RoleAssignmentData], + assignments: list[dict], sort_by: AssignmentSortField = AssignmentSortField.ROLE, order: SortOrder = SortOrder.ASC, -) -> list[RoleAssignmentData]: +) -> list[dict]: """ Sort role assignments by a given field and order. Args: - assignments (list[RoleAssignmentData]): The assignments to sort. + assignments (list[dict]): The assignments to sort. sort_by (SortField, optional): The field to sort by. Defaults to AssignmentSortField.ROLE. order (SortOrder, optional): The order to sort by. Defaults to SortOrder.ASC. @@ -114,7 +113,7 @@ def sort_assignments( ValueError: If the sort order is invalid. Returns: - list[RoleAssignmentData]: The sorted assignments. + list[dict]: The sorted assignments. """ if sort_by not in AssignmentSortField.values(): raise ValueError(f"Invalid field: '{sort_by}'. Must be one of {AssignmentSortField.values()}") diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index 2b535f81..3cbcb538 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -1318,20 +1318,23 @@ class TestTeamMemberAssignmentsAPIView(ViewTestMixin): lib:Org3:LIB3 → admin_3 (library_admin), regular_5 (library_admin), regular_6 (library_author), regular_7 (library_contributor), regular_8 (library_user) - URL: /authz/v1/users//assignments + URL: /api/authz/v1/users//assignments/ Response fields per item: is_superadmin, role, org, scope, permission_count Superadmin entry: - admin_1..3 are staff/superusers. Querying any of them adds one entry + admin_1..3 are staff/superusers. Querying any of them always adds one SuperAdminAssignmentData entry: role="django.superuser" (or "django.staff"), org="*", scope="*", permission_count=None, is_superadmin=True. This entry is always included regardless of org/role filters, since those filters are applied only to the role assignments, not to the superadmin entry. Visibility via filter_allowed_assignments: - - Staff/superuser: sees all assignments for any user - - regular_1 (library_user in Org1:LIB1): sees only Org1:LIB1 assignments - - regular_9 (no assignments): sees nothing for any user + - Staff/superuser: sees all role assignments for any user, plus the superadmin + entry when the target is a superadmin. + - regular_1 (library_user in Org1:LIB1): sees only Org1:LIB1 role assignments, + plus the superadmin entry when the target is a superadmin. + - regular_9 (no assignments): sees no role assignments for any user, but still + sees the superadmin entry when the target is a superadmin. """ def setUp(self): From 59600eb3e361ff7f622c525e1d19f9076d2ca981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 10 Apr 2026 17:47:35 -0600 Subject: [PATCH 06/11] squash!: Fix docstring --- openedx_authz/rest_api/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_authz/rest_api/utils.py b/openedx_authz/rest_api/utils.py index 19904731..06b04462 100644 --- a/openedx_authz/rest_api/utils.py +++ b/openedx_authz/rest_api/utils.py @@ -105,7 +105,7 @@ def sort_assignments( Args: assignments (list[dict]): The assignments to sort. - sort_by (SortField, optional): The field to sort by. Defaults to AssignmentSortField.ROLE. + sort_by (AssignmentSortField, optional): The field to sort by. Defaults to AssignmentSortField.ROLE. order (SortOrder, optional): The order to sort by. Defaults to SortOrder.ASC. Raises: From acd632fbac72f5c51cbffa6605b0b99a3e8ac10c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Mon, 13 Apr 2026 09:53:43 -0600 Subject: [PATCH 07/11] squash!: Apply PR suggestions --- openedx_authz/api/data.py | 2 +- openedx_authz/api/users.py | 6 +-- openedx_authz/rest_api/v1/serializers.py | 47 ++++++++++-------------- openedx_authz/rest_api/v1/views.py | 7 +++- 4 files changed, 28 insertions(+), 34 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 6b173248..3220ce15 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -1128,7 +1128,7 @@ class SuperAdminAssignmentData: staff/superuser and their access is not derived from a specific role assignment. """ - subject: SubjectData = None + user: "User" = None is_staff: bool = False is_superuser: bool = False diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index 23a9ca01..d5c0f350 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -55,7 +55,7 @@ "get_user_role_assignments_filtered", "get_all_user_role_assignments_in_scope", "get_visible_role_assignments_for_user", - "get_visible_specific_user_role_assignments_for_user", + "get_visible_user_role_assignments_filtered_by_current_user", "is_user_allowed", "get_scopes_for_user_and_permission", "get_users_for_role_in_scope", @@ -179,7 +179,7 @@ def get_user_role_assignments_for_role_in_scope( ) -def get_visible_specific_user_role_assignments_for_user( +def get_visible_user_role_assignments_filtered_by_current_user( user_external_key: str, orgs: list[str] = None, roles: list[str] = None, @@ -438,7 +438,7 @@ def get_superadmin_assignments(user_external_keys: list[str] | None = None) -> l for requested_user in requested_users: superadmin_assignments.append( SuperAdminAssignmentData( - subject=UserData(external_key=requested_user.username), + user=requested_user, is_staff=requested_user.is_staff, is_superuser=requested_user.is_superuser, ) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index fb00d179..30c931e7 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -34,6 +34,21 @@ class ActionMixin(serializers.Serializer): # pylint: disable=abstract-method action = serializers.CharField(max_length=255) +class OrderMixin(serializers.Serializer): # pylint: disable=abstract-method + """Mixin providing ordering field functionality.""" + + sort_by = serializers.ChoiceField( + required=False, + choices=[(e.value, e.name) for e in SortField], + default=SortField.USERNAME, + ) + order = serializers.ChoiceField( + required=False, + choices=[(e.value, e.name) for e in SortOrder], + default=SortOrder.ASC, + ) + + class PermissionValidationSerializer(ActionMixin, ScopeMixin): # pylint: disable=abstract-method """Serializer for permission validation request.""" @@ -111,20 +126,10 @@ class RemoveUsersFromRoleWithScopeSerializer( users = CommaSeparatedListField(allow_blank=False) -class ListUsersInRoleWithScopeSerializer(ScopeMixin): # pylint: disable=abstract-method +class ListUsersInRoleWithScopeSerializer(ScopeMixin, OrderMixin): # pylint: disable=abstract-method """Serializer for listing users in a role with a scope.""" roles = CommaSeparatedListField(required=False, default=[]) - sort_by = serializers.ChoiceField( - required=False, - choices=[(e.value, e.name) for e in SortField], - default=SortField.USERNAME, - ) - order = serializers.ChoiceField( - required=False, - choices=[(e.value, e.name) for e in SortOrder], - default=SortOrder.ASC, - ) search = LowercaseCharField(required=False, default=None) @@ -210,7 +215,7 @@ def get_roles(self, obj: api.RoleAssignmentData) -> list[str]: return [role.external_key for role in obj.roles] -class ListTeamMembersSerializer(serializers.Serializer): # pylint: disable=abstract-method +class ListTeamMembersSerializer(OrderMixin): # pylint: disable=abstract-method """ Serializer for listing team members. This serializer is TeamMembersAPIView, which is used in the Admin Console. @@ -219,16 +224,6 @@ class ListTeamMembersSerializer(serializers.Serializer): # pylint: disable=abst scopes = CaseSensitiveCommaSeparatedListField(required=False, default=[]) orgs = CaseSensitiveCommaSeparatedListField(required=False, default=[]) - sort_by = serializers.ChoiceField( - required=False, - choices=[(e.value, e.name) for e in SortField], - default=SortField.USERNAME, - ) - order = serializers.ChoiceField( - required=False, - choices=[(e.value, e.name) for e in SortOrder], - default=SortOrder.ASC, - ) search = LowercaseCharField(required=False, default=None) @@ -294,21 +289,17 @@ class UserValidationAPIViewResponseSerializer(serializers.Serializer): # pylint summary = UserValidationSummarySerializer(help_text="Summary statistics for the validation operation") -class ListTeamMemberAssignmentsSerializer(serializers.Serializer): # pylint: disable=abstract-method +class ListTeamMemberAssignmentsSerializer(OrderMixin): # pylint: disable=abstract-method """Serializer for listing team member assignments.""" orgs = CaseSensitiveCommaSeparatedListField(required=False, default=[]) roles = CaseSensitiveCommaSeparatedListField(required=False, default=[]) + # Overriding sort_by from OrderMixin due to different choices and default value sort_by = serializers.ChoiceField( required=False, choices=[(e.value, e.name) for e in AssignmentSortField], default=AssignmentSortField.ROLE, ) - order = serializers.ChoiceField( - required=False, - choices=[(e.value, e.name) for e in SortOrder], - default=SortOrder.ASC, - ) class TeamMemberAssignmentSerializer(serializers.Serializer): # pylint: disable=abstract-method diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index 65ba5fee..5df98e8a 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -21,7 +21,10 @@ from openedx_authz import api from openedx_authz.api.data import RoleAssignmentData, SuperAdminAssignmentData -from openedx_authz.api.users import get_superadmin_assignments, get_visible_specific_user_role_assignments_for_user +from openedx_authz.api.users import ( + get_superadmin_assignments, + get_visible_user_role_assignments_filtered_by_current_user, +) from openedx_authz.api.utils import get_user_map from openedx_authz.constants import permissions from openedx_authz.rest_api.data import RoleOperationError, RoleOperationStatus @@ -720,7 +723,7 @@ def get(self, request: HttpRequest, username: str) -> Response: # Retrieve superadmin assignments (django staff or superuser users), as they always have access to everything user_role_assignments += get_superadmin_assignments(user_external_keys=[username]) - user_role_assignments += get_visible_specific_user_role_assignments_for_user( + user_role_assignments += get_visible_user_role_assignments_filtered_by_current_user( user_external_key=username, orgs=query_params.get("orgs"), roles=query_params.get("roles"), From f13a62aaf76cec34c3dfc72759514b1249e74d2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Tue, 14 Apr 2026 12:22:33 -0600 Subject: [PATCH 08/11] squash!: Fix conflicts --- openedx_authz/api/roles.py | 3 +-- openedx_authz/api/users.py | 1 - openedx_authz/engine/utils.py | 21 +++++++++------------ openedx_authz/tests/test_migrations.py | 7 ++----- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index 809e0ddb..bc7ca1e4 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -571,6 +571,5 @@ def get_all_role_assignments_per_scope_type(scope_types: tuple[type[ScopeData], list[RoleAssignmentData]: All assignments whose scope is an instance of any of the given scope types. """ return [ - role_assignment for role_assignment in get_role_assignments() - if isinstance(role_assignment.scope, scope_types) + role_assignment for role_assignment in get_role_assignments() if isinstance(role_assignment.scope, scope_types) ] diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index d5c0f350..f5018920 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -400,7 +400,6 @@ def validate_users(user_identifiers: list[str]) -> tuple[list[str], list[str]]: Returns: tuple: (valid_users, invalid_users) lists """ - User = get_user_model() valid_users = [] invalid_users = [] diff --git a/openedx_authz/engine/utils.py b/openedx_authz/engine/utils.py index e9091d6f..8603934b 100644 --- a/openedx_authz/engine/utils.py +++ b/openedx_authz/engine/utils.py @@ -264,8 +264,7 @@ def migrate_legacy_course_roles_to_authz(course_access_role_model, course_id_lis # Permission applied to individual user logger.info( - f"Migrating permission for User: {permission.user.username} " - f"to Role: {role} in Scope: {scope_external_key}" + f"Migrating permission for User: {permission.user.username} to Role: {role} in Scope: {scope_external_key}" ) is_user_added = assign_role_to_user_in_scope( @@ -322,7 +321,10 @@ def migrate_authz_to_legacy_course_roles( _validate_migration_input(course_id_list, org_id) role_assignments = get_all_role_assignments_per_scope_type( - scope_types=(CourseOverviewData, OrgCourseOverviewGlobData,) + scope_types=( + CourseOverviewData, + OrgCourseOverviewGlobData, + ) ) # Two cases here: @@ -330,17 +332,15 @@ def migrate_authz_to_legacy_course_roles( # 2. only course_id_list provided: filter by course_id — org-level glob scopes are excluded (no course_id). if org_id: role_assignments = [ - role_assignment - for role_assignment in role_assignments - if role_assignment.scope.org == org_id + role_assignment for role_assignment in role_assignments if role_assignment.scope.org == org_id ] if course_id_list and not org_id: role_assignments = [ role_assignment for role_assignment in role_assignments - if isinstance(role_assignment.scope, CourseOverviewData) and - role_assignment.scope.course_id in course_id_list + if isinstance(role_assignment.scope, CourseOverviewData) + and role_assignment.scope.course_id in course_id_list ] roles_with_errors = [] @@ -350,13 +350,10 @@ def migrate_authz_to_legacy_course_roles( user_external_keys = {assignment.subject.external_key for assignment in role_assignments} users_by_username = { subject.user.username: subject.user - for subject in user_subject_model.objects.filter( - user__username__in=user_external_keys - ).select_related("user") + for subject in user_subject_model.objects.filter(user__username__in=user_external_keys).select_related("user") } for role_assignment in role_assignments: - # Per valid role assignment, create corresponding CourseAccessRole entry # depending on whether the scope is course-level or org-level glob try: diff --git a/openedx_authz/tests/test_migrations.py b/openedx_authz/tests/test_migrations.py index 508ef1dc..fb82908e 100644 --- a/openedx_authz/tests/test_migrations.py +++ b/openedx_authz/tests/test_migrations.py @@ -1301,9 +1301,7 @@ def test_migrate_org_level_scope_creates_org_glob_assignment(self): AuthzEnforcer.get_enforcer().load_policy() org_scope = OrgCourseOverviewGlobData.build_external_key(org_short_name_new) - assignments = get_user_role_assignments_in_scope( - user_external_key=user.username, scope_external_key=org_scope - ) + assignments = get_user_role_assignments_in_scope(user_external_key=user.username, scope_external_key=org_scope) self.assertEqual(len(assignments), 1) self.assertEqual(assignments[0].roles[0], COURSE_ADMIN) @@ -1404,8 +1402,7 @@ def test_course_id_list_filter_excludes_glob_and_other_courses(self): AuthzEnforcer.get_enforcer().load_policy() errors, successes = migrate_authz_to_legacy_course_roles( - CourseAccessRole, UserSubject, - course_id_list=[self.course_id], org_id=None, delete_after_migration=False + CourseAccessRole, UserSubject, course_id_list=[self.course_id], org_id=None, delete_after_migration=False ) migrated_users = {assignment.subject.external_key for assignment in successes} From f9aafe7ea7d868cb09a59cbad9b47b6054de301c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Tue, 14 Apr 2026 12:41:31 -0600 Subject: [PATCH 09/11] squash!: Updated docstrings --- openedx_authz/rest_api/v1/views.py | 141 +++++++++++++++++++++++++++-- 1 file changed, 132 insertions(+), 9 deletions(-) diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index 5df98e8a..db9baca4 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -560,9 +560,63 @@ def get_queryset(self) -> QuerySet: @view_auth_classes() class TeamMembersAPIView(APIView): """ - API view for listing users in relation to role assignments - This API is used in the Team Members section in Admin Console. - In this content, a team member is anyone with studio access. + API view for listing users in relation to role assignments. + This API is used in the Team Members section in the Admin Console. + In this context, a team member is anyone with studio access. + + **Endpoints** + + - GET: Retrieve all users that have at least one role assignment + + **Query Parameters** + + - scopes (Optional): Comma-separated list of scopes to filter by (e.g., 'lib:Org1:LIB1') + - orgs (Optional): Comma-separated list of orgs to filter by (e.g., 'Org1,Org2') + - search (Optional): Search term to filter users by username, full name, or email + - sort_by (Optional): Field to sort by. Options: username, full_name, email. Defaults to username + - order (Optional): Sort order, 'asc' or 'desc'. Defaults to asc + - page (Optional): Page number for pagination + - page_size (Optional): Number of items per page + + **Response Format** + + Returns a paginated list of team member objects, each containing: + + - username: The user's username + - full_name: The user's full name + - email: The user's email address + - assignation_count: The number of role assignments the user has + + **Authentication and Permissions** + + - Requires authenticated user. + - Results are filtered according to calling user's scope-level view permissions. + + **Example Request** + + GET /api/authz/v1/users/?orgs=Org1&search=john&sort_by=username&order=asc&page=1&page_size=10 + + **Example Response**:: + + { + "count": 2, + "next": null, + "previous": null, + "results": [ + { + "username": "jane_doe", + "full_name": "Jane Doe", + "email": "jane_doe@example.com", + "assignation_count": 3 + }, + { + "username": "john_doe", + "full_name": "John Doe", + "email": "john_doe@example.com", + "assignation_count": 1 + } + ] + } """ pagination_class = AuthZAPIViewPagination @@ -691,7 +745,70 @@ def post(self, request: HttpRequest) -> Response: @view_auth_classes() class TeamMemberAssignmentsAPIView(APIView): """ - API view for listing user role assignments + API view for listing role assignments for a specific user. + This API is used in the Team Member detail view in the Admin Console. + + **Endpoints** + + - GET: Retrieve all role assignments for a specific user + + **URL Parameters** + + - username (Required): The username of the user to retrieve assignments for + + **Query Parameters** + + - orgs (Optional): Comma-separated list of orgs to filter assignments by (e.g., 'Org1,Org2') + - roles (Optional): Comma-separated list of roles to filter assignments by (e.g., 'library_admin,library_user') + - sort_by (Optional): Field to sort by. Options: role, org, scope. Defaults to role + - order (Optional): Sort order, 'asc' or 'desc'. Defaults to asc + - page (Optional): Page number for pagination + - page_size (Optional): Number of items per page + + **Response Format** + + Returns a paginated list of assignment objects, each containing: + + - is_superadmin: Whether this entry denotes a superadmin (staff/superuser) + - role: The role name (e.g., 'library_admin', 'django.superuser') + - org: The org over which this role is applied ('*' for superadmins) + - scope: The scope over which this role is applied ('*' for superadmins) + - permission_count: The number of permissions that apply to this role (null for superadmins) + + **Authentication and Permissions** + + - Requires authenticated user. + - Results are filtered according to calling user's scope-level view permissions. + - Superadmin entries are always included when the target user is a staff/superuser. + + **Example Request** + + GET + /api/authz/v1/users/john_doe/assignments/?orgs=Org1&roles=library_admin&sort_by=role&order=asc&page=1&page_size=10 + + **Example Response**:: + + { + "count": 2, + "next": null, + "previous": null, + "results": [ + { + "is_superadmin": true, + "role": "django.superuser", + "org": "*", + "scope": "*", + "permission_count": null + }, + { + "is_superadmin": false, + "role": "library_admin", + "org": "Org1", + "scope": "lib:Org1:LIB1", + "permission_count": 11 + } + ] + } """ pagination_class = AuthZAPIViewPagination @@ -699,17 +816,23 @@ class TeamMemberAssignmentsAPIView(APIView): @apidocs.schema( parameters=[ - apidocs.query_parameter("orgs", str, description="The orgs to query assignations for"), - apidocs.query_parameter("roles", str, description="The roles to query assignations for"), - apidocs.query_parameter("sort_by", str, description="The field to sort by"), - apidocs.query_parameter("order", str, description="The order to sort by"), + apidocs.query_parameter("orgs", str, description="Comma-separated list of orgs to filter assignments by"), + apidocs.query_parameter("roles", str, description="Comma-separated list of roles to filter assignments by"), + apidocs.query_parameter( + "sort_by", + str, + description="The field to sort by. Options: role, org, scope. Defaults to role", + ), + apidocs.query_parameter( + "order", str, description="The order to sort by. Options: asc, desc. Defaults to asc" + ), apidocs.query_parameter("page", int, description="Page number for pagination"), apidocs.query_parameter("page_size", int, description="Number of items per page"), ], responses={ status.HTTP_200_OK: TeamMemberAssignmentSerializer(many=True), status.HTTP_400_BAD_REQUEST: "The request parameters are invalid", - status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", + status.HTTP_401_UNAUTHORIZED: "The user is not authenticated", }, ) def get(self, request: HttpRequest, username: str) -> Response: From ce614a59ac4a8e1552d2e85220337037dc8dbb3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Tue, 14 Apr 2026 13:25:59 -0600 Subject: [PATCH 10/11] squash!: Attend PR comment --- openedx_authz/rest_api/v1/serializers.py | 2 +- openedx_authz/rest_api/v1/views.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 30c931e7..822f813a 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -289,7 +289,7 @@ class UserValidationAPIViewResponseSerializer(serializers.Serializer): # pylint summary = UserValidationSummarySerializer(help_text="Summary statistics for the validation operation") -class ListTeamMemberAssignmentsSerializer(OrderMixin): # pylint: disable=abstract-method +class ListTeamMemberAssignmentsQuerySerializer(OrderMixin): # pylint: disable=abstract-method """Serializer for listing team member assignments.""" orgs = CaseSensitiveCommaSeparatedListField(required=False, default=[]) diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index db9baca4..661a4b54 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -45,7 +45,7 @@ AddUsersToRoleWithScopeSerializer, ListRolesWithScopeResponseSerializer, ListRolesWithScopeSerializer, - ListTeamMemberAssignmentsSerializer, + ListTeamMemberAssignmentsQuerySerializer, ListTeamMembersSerializer, ListUsersInRoleWithScopeSerializer, PermissionValidationResponseSerializer, @@ -837,7 +837,7 @@ class TeamMemberAssignmentsAPIView(APIView): ) def get(self, request: HttpRequest, username: str) -> Response: """Retrieve all user role assignments.""" - serializer = ListTeamMemberAssignmentsSerializer(data=request.query_params) + serializer = ListTeamMemberAssignmentsQuerySerializer(data=request.query_params) serializer.is_valid(raise_exception=True) query_params = serializer.validated_data From aede0dc7388154d98c09b76ece72fde63e5bf2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Wed, 15 Apr 2026 12:35:51 -0600 Subject: [PATCH 11/11] squash!: Filter for active users only --- openedx_authz/api/users.py | 11 +++++++ openedx_authz/tests/api/test_users.py | 46 +++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index f5018920..fa67f122 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -204,6 +204,17 @@ def get_visible_user_role_assignments_filtered_by_current_user( user_external_key=allowed_for_user_external_key, assignments=user_role_assignments, ) + + # Only include assignments whose subject corresponds to an active user, + # consistent with get_superadmin_assignments which filters by is_active=True. + active_usernames = set( + User.objects.filter( + username__in=[a.subject.username for a in user_role_assignments], + is_active=True, + ).values_list("username", flat=True) + ) + user_role_assignments = [a for a in user_role_assignments if a.subject.username in active_usernames] + if orgs: # Filter by orgs user_role_assignments = [a for a in user_role_assignments if getattr(a.scope, "org", None) in orgs] diff --git a/openedx_authz/tests/api/test_users.py b/openedx_authz/tests/api/test_users.py index 08b89327..da0b3b75 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -14,6 +14,7 @@ get_user_role_assignments, get_user_role_assignments_for_role_in_scope, get_user_role_assignments_in_scope, + get_visible_user_role_assignments_filtered_by_current_user, is_user_allowed, unassign_all_roles_from_user, unassign_role_from_user, @@ -570,3 +571,48 @@ def test_validate_users_user_does_not_exist_handling(self, mock_get_user): self.assertEqual(valid_users, []) self.assertEqual(invalid_users, ["nonexistent_user"]) + + +class TestGetVisibleUserRoleAssignmentsFilteredByCurrentUserActiveFilter(UserAssignmentsSetupMixin): + """Test that get_visible_user_role_assignments_filtered_by_current_user excludes inactive users.""" + + def test_active_user_assignments_are_returned(self): + """Test that assignments for an active user are returned.""" + User = get_user_model() + User.objects.create_user(username="alice", email="alice@example.com", is_active=True) + + assignments = get_visible_user_role_assignments_filtered_by_current_user( + user_external_key="alice", + ) + + usernames = {a.subject.username for a in assignments} + self.assertIn("alice", usernames) + + def test_inactive_user_assignments_are_excluded(self): + """Test that assignments for an inactive user are filtered out.""" + User = get_user_model() + User.objects.create_user(username="alice", email="alice@example.com", is_active=False) + + assignments = get_visible_user_role_assignments_filtered_by_current_user( + user_external_key="alice", + ) + + self.assertEqual(assignments, []) + + def test_mixed_active_inactive_subjects_in_assignments(self): + """Test that only active users' assignments are returned when multiple subjects exist.""" + User = get_user_model() + # eve has roles in lib:Org2:physics_401, lib:Org2:chemistry_501, lib:Org2:biology_601 + # grace has a role in lib:Org1:math_advanced + User.objects.create_user(username="eve", email="eve@example.com", is_active=True) + User.objects.create_user(username="grace", email="grace@example.com", is_active=False) + + eve_assignments = get_visible_user_role_assignments_filtered_by_current_user( + user_external_key="eve", + ) + grace_assignments = get_visible_user_role_assignments_filtered_by_current_user( + user_external_key="grace", + ) + + self.assertGreater(len(eve_assignments), 0) + self.assertEqual(grace_assignments, [])