From d21dbcf7c494c9bf56a3c323c3a9d9e160bf6398 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 1/3] feat: Implement team members endpoint for admin console --- CHANGELOG.rst | 19 ++ openedx_authz/api/data.py | 15 ++ openedx_authz/api/roles.py | 26 ++ openedx_authz/api/users.py | 74 ++++++ openedx_authz/api/utils.py | 77 ++++++ openedx_authz/engine/matcher.py | 13 +- openedx_authz/rest_api/utils.py | 50 +--- openedx_authz/rest_api/v1/fields.py | 12 + openedx_authz/rest_api/v1/filters.py | 23 ++ openedx_authz/rest_api/v1/serializers.py | 58 ++++- openedx_authz/rest_api/v1/urls.py | 1 + openedx_authz/rest_api/v1/views.py | 55 ++++- openedx_authz/tests/api/test_roles.py | 38 +++ openedx_authz/tests/rest_api/test_views.py | 262 +++++++++++++++++++++ openedx_authz/tests/test_enforcement.py | 146 +++++++++++- openedx_authz/utils.py | 26 ++ 16 files changed, 843 insertions(+), 52 deletions(-) create mode 100644 openedx_authz/api/utils.py create mode 100644 openedx_authz/rest_api/v1/filters.py create mode 100644 openedx_authz/utils.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3de9fd8d..5cc0060a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,14 +14,33 @@ Change Log Unreleased ********** +1.5.0 - 2026-04-09 +****************** + +Added +===== + +* Add ``users/`` endpoint to fetch all team members, with optional filters for orgs, scopes, search by username user full name or email, sorting and pagination. + +Fixed +===== + +* Fix enforcer ``is_admin_or_superuser_check`` that was not taking into account Org glob scopes. + 1.4.0 - 2026-04-09 ****************** +Added +===== + * Add ``orgs/`` endpoint to list and search orgs, with pagination, as required for filters in the Admin Console. 1.3.0 2026-04-08 **************** +Added +===== + * Add stub CCX_COACH role/ CCXCourseOverviewData scope to prevent errors when working with CCX courses. * Add ADR for global scope support for role assignments. diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 966c6ea4..86e54c7c 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -1207,3 +1207,18 @@ def __repr__(self): """Developer friendly string representation of the role assignment.""" role_keys = ", ".join(role.namespaced_key for role in self.roles) return f"{self.subject.namespaced_key} => [{role_keys}] @ {self.scope.namespaced_key}" + + +@define +class UserAssignments: + """A user with their role assignments""" + + user: "User" + assignments: list[RoleAssignmentData] + + +class UserAssignmentsFilter(Enum): + """Enum for the filters that can be applied over UserAssignments.""" + + SCOPES = "scopes" + ORGS = "orgs" diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index fa8d9448..1215b79b 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -31,6 +31,9 @@ "batch_unassign_role_from_subjects_in_scope", "get_all_roles_in_scope", "get_all_roles_names", + "get_subject_role_assignments_in_scope", + "get_subject_role_assignments_for_role_in_scope", + "get_all_subject_role_assignments", "get_all_subject_role_assignments_in_scope", "get_permissions_for_active_roles_in_scope", "get_permissions_for_roles", @@ -269,6 +272,29 @@ def batch_unassign_role_from_subjects_in_scope(subjects: list[SubjectData], role unassign_role_from_subject_in_scope(subject, role, scope) +def get_all_subject_role_assignments() -> list[RoleAssignmentData]: + """Get all the roles for every subject across all scopes. + + Returns: + list[RoleAssignmentData]: A list of role assignments for the subject. + """ + enforcer = AuthzEnforcer.get_enforcer() + role_assignments = [] + for policy in enforcer.get_grouping_policy(): + subject = SubjectData(namespaced_key=policy[GroupingPolicyIndex.SUBJECT.value]) + role = RoleData(namespaced_key=policy[GroupingPolicyIndex.ROLE.value]) + role.permissions = get_permissions_for_single_role(role) + + role_assignments.append( + RoleAssignmentData( + subject=subject, + roles=[role], + scope=ScopeData(namespaced_key=policy[GroupingPolicyIndex.SCOPE.value]), + ) + ) + return role_assignments + + def get_subject_role_assignments(subject: SubjectData) -> list[RoleAssignmentData]: """Get all the roles for a subject across all scopes. diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index 30cd851e..e2a4d20f 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -11,10 +11,16 @@ from openedx_authz.api.data import ( ActionData, + ContentLibraryData, + CourseOverviewData, + OrgContentLibraryGlobData, + OrgCourseOverviewGlobData, PermissionData, RoleAssignmentData, RoleData, ScopeData, + UserAssignments, + UserAssignmentsFilter, UserData, ) from openedx_authz.api.permissions import is_subject_allowed @@ -32,6 +38,8 @@ unassign_role_from_subject_in_scope, unassign_subject_from_all_roles, ) +from openedx_authz.api.utils import filter_user_assignments, get_user_assignment_map +from openedx_authz.constants.permissions import COURSES_MANAGE_COURSE_TEAM, MANAGE_LIBRARY_TEAM __all__ = [ "assign_role_to_user_in_scope", @@ -43,6 +51,7 @@ "get_user_role_assignments_for_role_in_scope", "get_user_role_assignments_filtered", "get_all_user_role_assignments_in_scope", + "get_visible_role_assignments_for_user", "is_user_allowed", "get_scopes_for_user_and_permission", "get_users_for_role_in_scope", @@ -205,6 +214,71 @@ def get_all_user_role_assignments_in_scope( return get_all_subject_role_assignments_in_scope(ScopeData(external_key=scope_external_key)) +def _filter_allowed_assignments( + user_external_key: str, assignments: list[RoleAssignmentData] +) -> list[RoleAssignmentData]: + """ + Filter the given role assignments to only include those that the user has permission to view. + """ + allowed_assignments: list[RoleAssignmentData] = [] + for assignment in assignments: + permission = None + + # For CourseOverviewData and ContentLibraryData, check for the view permission + if isinstance(assignment.scope, (CourseOverviewData, OrgCourseOverviewGlobData)): + permission = COURSES_MANAGE_COURSE_TEAM.identifier + elif isinstance(assignment.scope, (ContentLibraryData, OrgContentLibraryGlobData)): + permission = MANAGE_LIBRARY_TEAM.identifier + + if permission and is_user_allowed( + user_external_key=user_external_key, + action_external_key=permission, + scope_external_key=assignment.scope.external_key, + ): + allowed_assignments.append(assignment) + + return allowed_assignments + + +def get_visible_role_assignments_for_user( + orgs: list[str] = None, + scopes: list[str] = None, + allowed_for_user_external_key: str = None, +) -> list[UserAssignments]: + """ + Get all user role assignments filtered by orgs and/or scopes, and only include + assignments that the specified user has permission to view. + + Args: + orgs: Optional list of orgs to filter by (e.g., ['edX', 'MITx']). + scopes: Optional list of scopes to filter by (e.g., ['lib:DemoX:CSPROB']). + allowed_for_user_external_key: The username to check permissions against (e.g., 'john_doe'). + + Returns: + list[UserAssignments]: A list of users with their role assignments, filtered by orgs/scopes and permissions. + """ + user_role_assignments = get_user_role_assignments_filtered() + # 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, + ) + # Group assignments by user + users_with_assignments = get_user_assignment_map(user_role_assignments) + + users_with_assignments = filter_user_assignments( + users_with_assignments=users_with_assignments, + by=UserAssignmentsFilter.SCOPES, + values=scopes, + ) + users_with_assignments = filter_user_assignments( + users_with_assignments=users_with_assignments, + by=UserAssignmentsFilter.ORGS, + values=orgs, + ) + return users_with_assignments + + def is_user_allowed( user_external_key: str, action_external_key: str, diff --git a/openedx_authz/api/utils.py b/openedx_authz/api/utils.py new file mode 100644 index 00000000..8cc9aa57 --- /dev/null +++ b/openedx_authz/api/utils.py @@ -0,0 +1,77 @@ +"""Utility functions used on api""" + +from django.contrib.auth import get_user_model + +from openedx_authz.api.data import ( + RoleAssignmentData, + UserAssignments, + UserAssignmentsFilter, +) + +User = get_user_model() + + +def get_user_map(usernames: list[str]) -> dict[str, User]: + """ + Retrieve a dictionary mapping usernames to User objects for efficient batch lookups. + + This function performs a single optimized database query to fetch multiple users, + making it ideal for scenarios where we need to look up several users at once + (e.g., when serializing multiple user role assignments). + + Args: + usernames (list[str]): List of usernames to retrieve. Duplicates are automatically + handled by the database query. + + Returns: + dict[str, User]: Dictionary mapping each username to its corresponding User object. + Only users that exist in the database are included in the returned dictionary. + """ + users = User.objects.filter(username__in=usernames).select_related("profile") + return {user.username: user for user in users} + + +def get_user_assignment_map(role_assignments: list[RoleAssignmentData]) -> list[UserAssignments]: + """ + Group role assignments by user + """ + usernames = {assignment.subject.username for assignment in role_assignments} + user_map = get_user_map(usernames) + + users_with_assignments: list[UserAssignments] = [] + + for username, user in user_map.items(): + assignments = [a for a in role_assignments if a.subject.username == username] + users_with_assignments.append(UserAssignments(user=user, assignments=assignments)) + + return users_with_assignments + + +def filter_user_assignments( + users_with_assignments: list[UserAssignments], + by: UserAssignmentsFilter, + values: list[str], +) -> list[UserAssignments]: + """ + Filter user assignments by orgs or scopes. + """ + if not values: + return users_with_assignments + + def _get_value_to_filter(assignment: RoleAssignmentData) -> str: + if by == UserAssignmentsFilter.SCOPES: + return assignment.scope.external_key + elif by == UserAssignmentsFilter.ORGS: + return assignment.scope.org + else: + raise ValueError(f"Invalid filter: '{by}'. Must be one of {[f.value for f in UserAssignmentsFilter]}") + + filtered_users: list[UserAssignments] = [] + for uwa in users_with_assignments: + if any(_get_value_to_filter(a) in values for a in uwa.assignments): + # Also filter assignments to reflect the correct number of assignments + filtered_assignments = [a for a in uwa.assignments if _get_value_to_filter(a) in values] + filtered_users.append(UserAssignments(user=uwa.user, assignments=filtered_assignments)) + users_with_assignments = filtered_users + + return filtered_users diff --git a/openedx_authz/engine/matcher.py b/openedx_authz/engine/matcher.py index 0bc7e3d6..b1cde5bb 100644 --- a/openedx_authz/engine/matcher.py +++ b/openedx_authz/engine/matcher.py @@ -3,8 +3,15 @@ from django.contrib.auth import get_user_model from edx_django_utils.cache import RequestCache -from openedx_authz.api.data import ContentLibraryData, CourseOverviewData, ScopeData, UserData -from openedx_authz.rest_api.utils import get_user_by_username_or_email +from openedx_authz.api.data import ( + ContentLibraryData, + CourseOverviewData, + OrgContentLibraryGlobData, + OrgCourseOverviewGlobData, + ScopeData, + UserData, +) +from openedx_authz.utils import get_user_by_username_or_email User = get_user_model() @@ -12,6 +19,8 @@ SCOPES_WITH_ADMIN_OR_SUPERUSER_CHECK = { (ContentLibraryData.NAMESPACE, ContentLibraryData), (CourseOverviewData.NAMESPACE, CourseOverviewData), + (OrgContentLibraryGlobData.NAMESPACE, OrgContentLibraryGlobData), + (OrgCourseOverviewGlobData.NAMESPACE, OrgCourseOverviewGlobData), } diff --git a/openedx_authz/rest_api/utils.py b/openedx_authz/rest_api/utils.py index cbeaaa82..d7a6d9c1 100644 --- a/openedx_authz/rest_api/utils.py +++ b/openedx_authz/rest_api/utils.py @@ -1,13 +1,11 @@ """Utility functions for the Open edX AuthZ REST API.""" -from django.contrib.auth import get_user_model -from django.db.models import Q - -from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD, ScopeData +from openedx_authz.api.data import ( + GLOBAL_SCOPE_WILDCARD, + ScopeData, +) from openedx_authz.rest_api.data import SearchField, SortField, SortOrder -User = get_user_model() - def get_generic_scope(scope: ScopeData) -> ScopeData: """ @@ -31,46 +29,6 @@ def get_generic_scope(scope: ScopeData) -> ScopeData: return ScopeData(namespaced_key=f"{scope.NAMESPACE}{ScopeData.SEPARATOR}{GLOBAL_SCOPE_WILDCARD}") -def get_user_map(usernames: list[str]) -> dict[str, User]: - """ - Retrieve a dictionary mapping usernames to User objects for efficient batch lookups. - - This function performs a single optimized database query to fetch multiple users, - making it ideal for scenarios where we need to look up several users at once - (e.g., when serializing multiple user role assignments). - - Args: - usernames (list[str]): List of usernames to retrieve. Duplicates are automatically - handled by the database query. - - Returns: - dict[str, User]: Dictionary mapping each username to its corresponding User object. - Only users that exist in the database are included in the returned dictionary. - """ - users = User.objects.filter(username__in=usernames).select_related("profile") - return {user.username: user for user in users} - - -def get_user_by_username_or_email(username_or_email: str) -> User: - """ - Retrieve a user by their username or email address. - - Args: - username_or_email (str): The username or email address to search for. - - Returns: - User: The User object if found and not retired. - - Raises: - User.DoesNotExist: If no user matches the provided username or email, - or if the user has an associated retirement request. - """ - user = User.objects.get(Q(email=username_or_email) | Q(username=username_or_email)) - if hasattr(user, "userretirementrequest"): - raise User.DoesNotExist - return user - - def sort_users( users: list[dict], sort_by: SortField = SortField.USERNAME, diff --git a/openedx_authz/rest_api/v1/fields.py b/openedx_authz/rest_api/v1/fields.py index 89800788..2413a975 100644 --- a/openedx_authz/rest_api/v1/fields.py +++ b/openedx_authz/rest_api/v1/fields.py @@ -15,6 +15,18 @@ def to_representation(self, value): return ",".join(value).lower() +class CaseSensitiveCommaSeparatedListField(serializers.CharField): + """Serializer for a comma-separated list of strings, case-sensitive.""" + + def to_internal_value(self, data): + """Convert string separated by commas to list of unique items preserving order""" + return list(dict.fromkeys(item.strip() for item in data.split(",") if item.strip())) + + def to_representation(self, value): + """Convert list to string separated by commas""" + return ",".join(value) + + class LowercaseCharField(serializers.CharField): """Serializer for a lowercase string.""" diff --git a/openedx_authz/rest_api/v1/filters.py b/openedx_authz/rest_api/v1/filters.py new file mode 100644 index 00000000..fd0878c0 --- /dev/null +++ b/openedx_authz/rest_api/v1/filters.py @@ -0,0 +1,23 @@ +"""Custom DRF filter backends for the Open edX AuthZ REST API.""" + +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 + + +class TeamMemberSearchFilter(BaseFilterBackend): + """Filter team members by a search term.""" + + def filter_queryset(self, request, queryset, view): + search = request.query_params.get("search") + return filter_users(users=queryset, search=search, roles=None) + + +class TeamMemberOrderingFilter(BaseFilterBackend): + """Sort team members by a given field and order.""" + + 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) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 58831c57..551329ed 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -4,9 +4,14 @@ from rest_framework import serializers 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.utils import get_generic_scope -from openedx_authz.rest_api.v1.fields import CommaSeparatedListField, LowercaseCharField +from openedx_authz.rest_api.v1.fields import ( + CaseSensitiveCommaSeparatedListField, + CommaSeparatedListField, + LowercaseCharField, +) User = get_user_model() @@ -203,3 +208,54 @@ def get_email(self, obj) -> str: def get_roles(self, obj: api.RoleAssignmentData) -> list[str]: """Get the roles for the given role assignment.""" return [role.external_key for role in obj.roles] + + +class ListTeamMembersSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer for listing team members. + This serializer is TeamMembersAPIView, which is used in the Admin Console. + In this content, a team member is anyone with studio access. + """ + + 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) + + +class TeamMemberSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer for team members. + This serializer is APIs used by the Admin Console. + In this content, a team member is anyone with studio access. + """ + + username = serializers.SerializerMethodField() + full_name = serializers.SerializerMethodField() + email = serializers.SerializerMethodField() + assignation_count = serializers.SerializerMethodField() + + def get_username(self, obj: UserAssignments) -> str: + """Get the username for the given role assignment.""" + return getattr(obj.user, "username", "") if obj.user else "" + + def get_full_name(self, obj: UserAssignments) -> str: + """Get the full name for the given role assignment.""" + return obj.user.get_full_name() if obj.user else "" + + def get_email(self, obj: UserAssignments) -> str: + """Get the email for the given role assignment.""" + return getattr(obj.user, "email", "") if obj.user else "" + + def get_assignation_count(self, obj: UserAssignments) -> int: + """Get the assignation count for the given role assignment.""" + return len(obj.assignments) diff --git a/openedx_authz/rest_api/v1/urls.py b/openedx_authz/rest_api/v1/urls.py index 761e7f6f..dafdc1c5 100644 --- a/openedx_authz/rest_api/v1/urls.py +++ b/openedx_authz/rest_api/v1/urls.py @@ -13,4 +13,5 @@ path("roles/", views.RoleListView.as_view(), name="role-list"), path("roles/users/", views.RoleUserAPIView.as_view(), name="role-user-list"), path("orgs/", views.AdminConsoleOrgsAPIView.as_view(), name="orgs-list"), + path("users/", views.TeamMembersAPIView.as_view(), name="user-list"), ] diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index c6947967..bcf1bef1 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -19,28 +19,31 @@ from rest_framework.views import APIView from openedx_authz import api +from openedx_authz.api.utils import get_user_map from openedx_authz.constants import permissions from openedx_authz.rest_api.data import RoleOperationError, RoleOperationStatus from openedx_authz.rest_api.decorators import authz_permissions, view_auth_classes from openedx_authz.rest_api.utils import ( filter_users, get_generic_scope, - get_user_by_username_or_email, - get_user_map, sort_users, ) +from openedx_authz.rest_api.v1.filters import 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, + ListTeamMembersSerializer, ListUsersInRoleWithScopeSerializer, PermissionValidationResponseSerializer, PermissionValidationSerializer, RemoveUsersFromRoleWithScopeSerializer, + TeamMemberSerializer, UserRoleAssignmentSerializer, ) +from openedx_authz.utils import get_user_by_username_or_email logger = logging.getLogger(__name__) @@ -535,3 +538,51 @@ class AdminConsoleOrgsAPIView(generics.ListAPIView): filter_backends = [filters.SearchFilter] search_fields = ["name", "short_name"] permission_classes = [AnyScopePermission] + + +@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. + """ + + pagination_class = AuthZAPIViewPagination + filter_backends = [TeamMemberSearchFilter, TeamMemberOrderingFilter] + + @apidocs.schema( + parameters=[ + apidocs.query_parameter("scopes", str, description="The scopes to query assignments for"), + apidocs.query_parameter("orgs", str, description="The orgs to query assignments for"), + apidocs.query_parameter("search", str, description="The search query to filter users by"), + 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: ListRolesWithScopeResponseSerializer(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) -> Response: + """Retrieve all users that have at least one assignation according to the filtering fields.""" + serializer = ListTeamMembersSerializer(data=request.query_params) + serializer.is_valid(raise_exception=True) + query_params = serializer.validated_data + + users_with_assignments = api.get_visible_role_assignments_for_user( + orgs=query_params.get("orgs"), + scopes=query_params.get("scopes"), + allowed_for_user_external_key=request.user.username, + ) + + team_members = TeamMemberSerializer(users_with_assignments, many=True).data + for backend in self.filter_backends: + team_members = backend().filter_queryset(request, team_members, self) + + paginator = self.pagination_class() + paginated_response_data = paginator.paginate_queryset(team_members, request) + return paginator.get_paginated_response(paginated_response_data) diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index e5fa5686..ecdb80e2 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -27,6 +27,7 @@ _get_field_index_and_values, assign_role_to_subject_in_scope, batch_assign_role_to_subjects_in_scope, + get_all_subject_role_assignments, get_all_subject_role_assignments_in_scope, get_permissions_for_active_roles_in_scope, get_permissions_for_single_role, @@ -1110,6 +1111,43 @@ def test_get_all_role_assignments_in_scope(self, scope_name, expected_assignment for assignment in role_assignments: self.assertIn(assignment, expected_assignments) + def test_get_all_subject_role_assignments(self): + """Test retrieving all role assignments across all subjects and scopes. + + Expected result: + - Returns all role assignments present in the system. + - Each assignment includes subject, role, and scope. + - Known assignments from the test setup are present in the result. + """ + role_assignments = get_all_subject_role_assignments() + + self.assertGreater(len(role_assignments), 0) + + # Verify each assignment has the expected structure + for assignment in role_assignments: + self.assertIsNotNone(assignment.subject) + self.assertIsNotNone(assignment.scope) + self.assertGreater(len(assignment.roles), 0) + for role in assignment.roles: + self.assertIsNotNone(role.external_key) + + # Verify known assignments from setup are present + subject_scope_role_triples = { + (a.subject.external_key, a.scope.external_key, a.roles[0].external_key) for a in role_assignments + } + self.assertIn( + ("alice", "lib:Org1:math_101", roles.LIBRARY_ADMIN.external_key), + subject_scope_role_triples, + ) + self.assertIn( + ("eve", "lib:Org2:physics_401", roles.LIBRARY_ADMIN.external_key), + subject_scope_role_triples, + ) + self.assertIn( + ("liam", "lib:Org4:art_101", roles.LIBRARY_AUTHOR.external_key), + subject_scope_role_triples, + ) + def test_assign_role_creates_extended_casbin_rule(self): """Test that assigning a role creates an ExtendedCasbinRule record. diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index e3d12223..5d100b56 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -1004,6 +1004,7 @@ def test_get_orgs_permissions(self, username: str, expected_status: int): Expected result: - Returns appropriate status code based on user permissions + """ user = User.objects.get(username=username) self.client.force_authenticate(user=user) @@ -1040,6 +1041,7 @@ def test_get_orgs_unauthenticated(self): Expected result: - Returns 401 UNAUTHORIZED status + """ self.client.force_authenticate(user=None) @@ -1048,6 +1050,266 @@ def test_get_orgs_unauthenticated(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) +@ddt +class TestTeamMembersAPIView(ViewTestMixin): + """ + Test suite for TeamMembersAPIView. + + Setup summary (from ViewTestMixin.setUpClass): + lib:Org1:LIB1 → admin_1 (library_admin), regular_1 (library_user), regular_2 (library_user) [3 users] + lib:Org2:LIB2 → admin_2 (library_user), regular_3 (library_user), regular_4 (library_user) [3 users] + lib:Org3:LIB3 → admin_3 (library_admin), regular_5 (library_admin), regular_6 (library_author), + regular_7 (library_contributor), regular_8 (library_user) [5 users] + + Total unique users with assignments: 11 + (admin_1..3 are staff/superuser; regular_1..8 are plain users) + + Visibility via filter_allowed_assignments: + - Staff/superuser: sees all 11 users (is_admin_or_superuser_check grants MANAGE_LIBRARY_TEAM on lib scopes) + - regular_5 (library_admin in Org3:LIB3): MANAGE_LIBRARY_TEAM granted → sees Org3 members (5) + - regular_1 (library_user in Org1:LIB1): no MANAGE_LIBRARY_TEAM → sees 0 + - regular_3 (library_user in Org2:LIB2): no MANAGE_LIBRARY_TEAM → sees 0 + - 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", + side_effect=get_user_map_without_profile, + ) + self.get_user_map_patcher.start() + self.addCleanup(self.get_user_map_patcher.stop) + + # -------------------------------------------------------------------- # + # Visibility: calling user only sees assignments it has view access to # + # -------------------------------------------------------------------- # + + @data( + # Staff/superuser sees all users across all scopes + ("admin_1", 11), + # regular_5 has LIBRARY_ADMIN in lib:Org3:LIB3 (MANAGE_LIBRARY_TEAM granted) → sees only Org3 members + ("regular_5", 5), + # regular_1 has LIBRARY_USER in lib:Org1:LIB1 (no MANAGE_LIBRARY_TEAM) → sees nothing + ("regular_1", 0), + # regular_3 has LIBRARY_USER in lib:Org2:LIB2 (no MANAGE_LIBRARY_TEAM) → sees nothing + ("regular_3", 0), + # regular_9 has no assignments → sees nothing + ("regular_9", 0), + ) + @unpack + def test_visibility_limited_to_accessible_scopes(self, username: str, expected_count: int): + """Calling user only sees assignments for scopes it has MANAGE_*_TEAM access to. + + Expected result: + - Staff/superuser sees all users across all scopes. + - Regular users only see members of scopes they can manage the team for. + - Users without MANAGE_*_TEAM permission see no results. + + """ + user = User.objects.get(username=username) + self.client.force_authenticate(user=user) + + response = self.client.get(self.url) + 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) + + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + # -------------------------------------------------------------------- # + + # Filter by scopes # + # -------------------------------------------------------------------- # + + @data( + # Single scope + ("lib:Org1:LIB1", 3), + ("lib:Org2:LIB2", 3), + ("lib:Org3:LIB3", 5), + # Multiple scopes (users are unique per scope, no overlap) + ("lib:Org1:LIB1,lib:Org2:LIB2", 6), + ("lib:Org1:LIB1,lib:Org3:LIB3", 8), + ("lib:Org1:LIB1,lib:Org2:LIB2,lib:Org3:LIB3", 11), + # Non-existent scope returns no results + ("lib:Org99:NOLIB", 0), + ) + @unpack + def test_filter_by_scopes(self, scopes: str, expected_count: int): + """Results are filtered to the requested scopes. + + Expected result: + - Only users with assignments in the given scope(s) are returned. + - Multiple scopes are OR-combined. + """ + response = self.client.get(self.url, {"scopes": scopes}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], expected_count) + + # ------------------------------------------------------------------ # + # Filter by orgs # + # ------------------------------------------------------------------ # + + @data( + # Single org + ("Org1", 3), + ("Org2", 3), + ("Org3", 5), + # Multiple orgs + ("Org1,Org2", 6), + ("Org1,Org3", 8), + ("Org1,Org2,Org3", 11), + # Non-existent org returns no results + ("OrgX", 0), + ) + @unpack + def test_filter_by_orgs(self, orgs: str, expected_count: int): + """Results are filtered to the requested orgs. + + Expected result: + - Only users with assignments in the given org(s) are returned. + - Multiple orgs are OR-combined. + """ + response = self.client.get(self.url, {"orgs": orgs}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], expected_count) + + # ------------------------------------------------------------------ # + # Search (username, full_name, email) # + # ------------------------------------------------------------------ # + + @data( + # Exact username match + ("admin_1", 1), + # Partial username match + ("admin", 3), + ("regular", 8), + # Email match + ("admin_1@example.com", 1), + ("@example.com", 11), + # No match + ("nonexistent", 0), + ) + @unpack + def test_search(self, search: str, expected_count: int): + """Search filters by username, full_name, or email (case-insensitive). + + Expected result: + - Returns only users whose username, full_name, or email contains the search term. + """ + response = self.client.get(self.url, {"search": search}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], expected_count) + + # ------------------------------------------------------------------ # + # Sorting # + # ------------------------------------------------------------------ # + + @data( + ("username", "asc"), + ("username", "desc"), + ("email", "asc"), + ("email", "desc"), + ("full_name", "asc"), + ("full_name", "desc"), + ) + @unpack + def test_sorting(self, sort_by: str, order: str): + """Results can be sorted by username, full_name, or email in asc/desc order. + + Expected result: + - Returns 200 OK. + - Results are ordered according to the requested field and direction. + """ + response = self.client.get(self.url, {"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"}, + {"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, query_params) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + # ------------------------------------------------------------------ # + # Pagination # + # ------------------------------------------------------------------ # + + @data( + ({"page": 1, "page_size": 5}, 5, True), + ({"page": 2, "page_size": 5}, 5, True), + ({"page": 3, "page_size": 5}, 1, False), + ({"page": 1, "page_size": 11}, 11, False), + ({"page": 1, "page_size": 6}, 6, True), + ) + @unpack + def test_pagination(self, query_params: dict, expected_page_count: int, has_next: bool): + """Results are paginated correctly. + + Expected result: + - Returns 200 OK. + - Page contains the expected number of items. + - `next` link is present only when more pages exist. + """ + response = self.client.get(self.url, query_params) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 11) + 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. + + Expected result: + - Returns 200 OK. + - Each item has username, full_name, email, and assignation_count. + """ + response = self.client.get(self.url, {"scopes": "lib:Org1:LIB1"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + for item in response.data["results"]: + self.assertIn("username", item) + self.assertIn("full_name", item) + self.assertIn("email", item) + self.assertIn("assignation_count", item) + self.assertEqual(item["assignation_count"], 1) + + @ddt class TestRoleListView(ViewTestMixin): """Test suite for RoleListView.""" diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index 8afd7f2b..a79d3487 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -16,7 +16,13 @@ from django.contrib.auth import get_user_model from openedx_authz import ROOT_DIRECTORY -from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD, ContentLibraryData, CourseOverviewData +from openedx_authz.api.data import ( + GLOBAL_SCOPE_WILDCARD, + ContentLibraryData, + CourseOverviewData, + OrgContentLibraryGlobData, + OrgCourseOverviewGlobData, +) from openedx_authz.constants import roles from openedx_authz.constants.permissions import ( COURSES_CREATE_FILES, @@ -29,6 +35,7 @@ make_action_key, make_course_assignment, make_course_case, + make_course_key, make_library_assignment, make_library_case, make_library_key, @@ -668,6 +675,16 @@ def test_org_level_glob_enforcement(self, request: AuthRequest): self._test_enforcement(self.POLICIES + self.ASSIGNMENTS, request) +def make_org_library_glob_key(key: str) -> str: + """Create a namespaced org-level library glob key (e.g., 'lib^lib:DemoX:*').""" + return f"{OrgContentLibraryGlobData.NAMESPACE}{OrgContentLibraryGlobData.SEPARATOR}{key}" + + +def make_org_course_glob_key(key: str) -> str: + """Create a namespaced org-level course glob key (e.g., 'course-v1^course-v1:DemoX+*').""" + return f"{OrgCourseOverviewGlobData.NAMESPACE}{OrgCourseOverviewGlobData.SEPARATOR}{key}" + + @pytest.mark.django_db @ddt class StaffSuperuserAccessTests(CasbinEnforcementTestCase): @@ -753,3 +770,130 @@ def test_staff_superuser_guaranteed_permissions( """ request = {"subject": subject, "action": action, "scope": scope, "expected_result": expected_result} self._test_enforcement(self.POLICY, request) + + +@pytest.mark.django_db +@ddt +class AdminOrSuperuserMatcherTests(CasbinEnforcementTestCase): + """ + Tests for is_admin_or_superuser_check across all supported scope types. + + Verifies that staff and superuser flags grant access for every scope type + listed in SCOPES_WITH_ADMIN_OR_SUPERUSER_CHECK, and that regular users and + unsupported scope types are correctly denied. + """ + + POLICY = [] + + def setUp(self) -> None: + """Set up staff, superuser, and regular user.""" + super().setUp() + User.objects.create_user(username="staff_user", email="staff@example.com", password="test", is_staff=True) + User.objects.create_superuser(username="superuser", email="super@example.com", password="test") + User.objects.create_user(username="regular_user", email="regular@example.com", password="test") + + @data( + # ContentLibraryData scope + ( + make_user_key("staff_user"), + make_action_key("content_libraries.view_library"), + make_library_key("lib:TestOrg:TestLib"), + True, + ), + ( + make_user_key("superuser"), + make_action_key("content_libraries.view_library"), + make_library_key("lib:TestOrg:TestLib"), + True, + ), + ( + make_user_key("regular_user"), + make_action_key("content_libraries.view_library"), + make_library_key("lib:TestOrg:TestLib"), + False, + ), + # CourseOverviewData scope + ( + make_user_key("staff_user"), + make_action_key("courses.view_course"), + make_course_key("course-v1:TestOrg+TestCourse+2024"), + True, + ), + ( + make_user_key("superuser"), + make_action_key("courses.view_course"), + make_course_key("course-v1:TestOrg+TestCourse+2024"), + True, + ), + ( + make_user_key("regular_user"), + make_action_key("courses.view_course"), + make_course_key("course-v1:TestOrg+TestCourse+2024"), + False, + ), + # OrgContentLibraryGlobData scope + ( + make_user_key("staff_user"), + make_action_key("content_libraries.view_library"), + make_org_library_glob_key("lib:TestOrg:*"), + True, + ), + ( + make_user_key("superuser"), + make_action_key("content_libraries.view_library"), + make_org_library_glob_key("lib:TestOrg:*"), + True, + ), + ( + make_user_key("regular_user"), + make_action_key("content_libraries.view_library"), + make_org_library_glob_key("lib:TestOrg:*"), + False, + ), + # OrgCourseOverviewGlobData scope + ( + make_user_key("staff_user"), + make_action_key("courses.view_course"), + make_org_course_glob_key("course-v1:TestOrg+*"), + True, + ), + ( + make_user_key("superuser"), + make_action_key("courses.view_course"), + make_org_course_glob_key("course-v1:TestOrg+*"), + True, + ), + ( + make_user_key("regular_user"), + make_action_key("courses.view_course"), + make_org_course_glob_key("course-v1:TestOrg+*"), + False, + ), + # Unsupported scope type - no one is granted access via this matcher + (make_user_key("staff_user"), make_action_key("manage"), make_scope_key("org", "TestOrg"), False), + (make_user_key("superuser"), make_action_key("manage"), make_scope_key("org", "TestOrg"), False), + ) + @unpack + def test_is_admin_or_superuser_check( + self, + subject: str, + action: str, + scope: str, + expected_result: bool, + ): + """Test is_admin_or_superuser_check grants access to staff/superusers for all supported scopes. + + Verifies that: + - Staff users are always allowed for ContentLibraryData, CourseOverviewData, + OrgContentLibraryGlobData, and OrgCourseOverviewGlobData scopes. + - Superusers are always allowed for the same scopes. + - Regular users are denied when they have no role assignments. + - Unsupported scope types (e.g., org) are denied even for staff/superusers. + + Expected result: + - staff_user and superuser: True for all four supported scope types. + - regular_user: False for all scope types (no role assignments). + - staff_user and superuser: False for unsupported scope types. + """ + request = {"subject": subject, "action": action, "scope": scope, "expected_result": expected_result} + self._test_enforcement(self.POLICY, request) diff --git a/openedx_authz/utils.py b/openedx_authz/utils.py new file mode 100644 index 00000000..0e1dfe5f --- /dev/null +++ b/openedx_authz/utils.py @@ -0,0 +1,26 @@ +"""General utility functions for Open edX AuthZ.""" + +from django.contrib.auth import get_user_model +from django.db.models import Q + +User = get_user_model() + + +def get_user_by_username_or_email(username_or_email: str) -> User: + """ + Retrieve a user by their username or email address. + + Args: + username_or_email (str): The username or email address to search for. + + Returns: + User: The User object if found and not retired. + + Raises: + User.DoesNotExist: If no user matches the provided username or email, + or if the user has an associated retirement request. + """ + user = User.objects.get(Q(email=username_or_email) | Q(username=username_or_email)) + if hasattr(user, "userretirementrequest"): + raise User.DoesNotExist + return user From 026de9514221a391c0b9b363fac8ec2166a6e34e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Thu, 9 Apr 2026 10:25:39 -0600 Subject: [PATCH 2/3] squash!: Refactor to define admin view permissions in data classes --- openedx_authz/api/data.py | 236 ++++++--------------- openedx_authz/api/users.py | 12 +- openedx_authz/api/utils.py | 13 +- openedx_authz/constants/permissions.py | 2 +- openedx_authz/data.py | 115 ++++++++++ openedx_authz/tests/api/test_data.py | 4 + openedx_authz/tests/rest_api/test_views.py | 27 ++- 7 files changed, 211 insertions(+), 198 deletions(-) create mode 100644 openedx_authz/data.py diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 86e54c7c..e6e18f41 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -6,7 +6,7 @@ from abc import abstractmethod from enum import Enum from functools import cached_property -from typing import Any, ClassVar, Literal, Type +from typing import Any, ClassVar, Type from attrs import define from opaque_keys import InvalidKeyError @@ -14,6 +14,8 @@ from opaque_keys.edx.locator import LibraryLocatorV2 from organizations.models import Organization +from openedx_authz.constants.permissions import COURSES_VIEW_COURSE_TEAM, VIEW_LIBRARY_TEAM +from openedx_authz.data import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR, ActionData, AuthzBaseClass, AuthZData, PermissionData from openedx_authz.models.scopes import get_content_library_model, get_course_overview_model ContentLibrary = get_content_library_model() @@ -21,6 +23,8 @@ __all__ = [ "ActionData", + "AuthZData", + "AuthzBaseClass", "ContentLibraryData", "CourseOverviewData", "GroupingPolicyIndex", @@ -36,7 +40,6 @@ "UserData", ] -AUTHZ_POLICY_ATTRIBUTES_SEPARATOR = "^" EXTERNAL_KEY_SEPARATOR = ":" GLOBAL_SCOPE_WILDCARD = "*" NAMESPACED_KEY_PATTERN = rf"^.+{re.escape(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR)}.+$" @@ -86,65 +89,6 @@ class PolicyIndex(Enum): # The rest of the fields are optional and can be ignored for now -class AuthzBaseClass: - """Base class for all authz classes. - - Attributes: - SEPARATOR: The separator between the namespace and the identifier (default: '^'). - NAMESPACE: The namespace prefix for the data type (e.g., 'user', 'role', 'act', 'lib'). - """ - - SEPARATOR: ClassVar[str] = AUTHZ_POLICY_ATTRIBUTES_SEPARATOR - NAMESPACE: ClassVar[str] = None - - -@define -class AuthZData(AuthzBaseClass): - """Base class for all authz data classes. - - Attributes: - NAMESPACE: The namespace prefix for the data type (e.g., 'user', 'role', 'act', 'lib'). - SEPARATOR: The separator between the namespace and the identifier (default: '^'). - external_key: The ID for the object outside of the authz system (e.g., 'john_doe' for a user, - 'instructor' for a role, 'lib:DemoX:CSPROB' for a content library). - namespaced_key: The ID for the object within the authz system, combining namespace and external_key - (e.g., 'user^john_doe', 'role^instructor', 'lib^lib:DemoX:CSPROB'). - - Examples: - >>> user = UserData(external_key='john_doe') - >>> user.namespaced_key - 'user^john_doe' - >>> role = RoleData(namespaced_key='role^instructor') - >>> role.external_key - 'instructor' - """ - - external_key: str = "" - namespaced_key: str = "" - - def __attrs_post_init__(self): - """Post-initialization processing for attributes. - - This method ensures that either external_key or namespaced_key is provided, - and derives the other attribute based on the NAMESPACE and SEPARATOR. - """ - if not self.NAMESPACE: - # No namespace defined, nothing to do - return - - if not self.external_key and not self.namespaced_key: - raise ValueError("Either external_key or namespaced_key must be provided.") - - # Case 1: Initialized with external_key only, derive namespaced_key - if not self.namespaced_key: - self.namespaced_key = f"{self.NAMESPACE}{self.SEPARATOR}{self.external_key}" - - # Case 2: Initialized with namespaced_key only, derive external_key. Assume valid format for - # namespaced_key at this point. - if not self.external_key: - self.external_key = self.namespaced_key.split(self.SEPARATOR, 1)[1] - - class ScopeMeta(type): """Metaclass for ScopeData to handle dynamic subclass instantiation based on namespace.""" @@ -397,6 +341,20 @@ def validate_external_key(cls, _: str) -> bool: """ return True + @classmethod + @abstractmethod + def get_admin_view_permission(cls) -> PermissionData: + """Get the permission required to view this scope + + This method should be implemented on every ScopeData subclass to define + which permission to check against when a user tries to see assignations + related to this scope in the Admin Console. + + Returns: + PermissionData: The permission required to view this scope in the admin console. + """ + raise NotImplementedError("Subclasses must implement get_admin_view_permission method.") + @abstractmethod def get_object(self) -> Any | None: """Retrieve the underlying domain object that this scope represents. @@ -494,6 +452,15 @@ def validate_external_key(cls, external_key: str) -> bool: except InvalidKeyError: return False + @classmethod + def get_admin_view_permission(cls) -> PermissionData: + """Get the permission required to view this scope + + Returns: + PermissionData: The permission required to view this scope in the admin console. + """ + return VIEW_LIBRARY_TEAM + def get_object(self) -> ContentLibrary | None: """Retrieve the ContentLibrary instance associated with this scope. @@ -607,6 +574,15 @@ def validate_external_key(cls, external_key: str) -> bool: except InvalidKeyError: return False + @classmethod + def get_admin_view_permission(cls) -> PermissionData: + """Get the permission required to view this scope + + Returns: + PermissionData: The permission required to view this scope in the admin console. + """ + return COURSES_VIEW_COURSE_TEAM + def get_object(self) -> CourseOverview | None: """Retrieve the CourseOverview instance associated with this scope. @@ -710,6 +686,15 @@ def validate_external_key(cls, external_key: str) -> bool: return True + @classmethod + def get_admin_view_permission(cls) -> PermissionData: + """Get the permission required to view this scope + + Returns: + PermissionData: The permission required to view this scope in the admin console. + """ + raise NotImplementedError("Subclasses must implement get_admin_view_permission method.") + @classmethod def get_org(cls, external_key: str) -> str | None: """Extract the organization identifier from the glob pattern. @@ -799,6 +784,15 @@ class OrgContentLibraryGlobData(OrgGlobData): NAMESPACE: ClassVar[str] = "lib" ID_SEPARATOR: ClassVar[str] = ":" + @classmethod + def get_admin_view_permission(cls) -> PermissionData: + """Get the permission required to view this scope + + Returns: + PermissionData: The permission required to view this scope in the admin console. + """ + return VIEW_LIBRARY_TEAM + @define class OrgCourseOverviewGlobData(OrgGlobData): @@ -839,6 +833,15 @@ class OrgCourseOverviewGlobData(OrgGlobData): NAMESPACE: ClassVar[str] = "course-v1" ID_SEPARATOR: ClassVar[str] = "+" + @classmethod + def get_admin_view_permission(cls) -> PermissionData: + """Get the permission required to view this scope + + Returns: + PermissionData: The permission required to view this scope in the admin console. + """ + return COURSES_VIEW_COURSE_TEAM + class CCXCourseOverviewData(CourseOverviewData): """CCX course scope for authorization in the Open edX platform. @@ -994,117 +997,6 @@ def __repr__(self): return self.namespaced_key -@define -class ActionData(AuthZData): - """An action represents an operation that can be performed in the authorization system. - - Actions are the operations that can be allowed or denied in authorization policies. - - Attributes: - NAMESPACE: 'act' for actions. - external_key: The action identifier (e.g., 'content_libraries.view_library'). - namespaced_key: The action identifier with namespace (e.g., 'act^content_libraries.view_library'). - name: Property that returns a human-readable action name (e.g., 'Content Libraries > View Library'). - - Examples: - >>> action = ActionData(external_key='content_libraries.delete_library') - >>> action.namespaced_key - 'act^content_libraries.delete_library' - >>> action.name - 'Content Libraries > Delete Library' - """ - - NAMESPACE: ClassVar[str] = "act" - - @property - def name(self) -> str: - """The human-readable name of the action (e.g., 'Content Libraries > Delete Library'). - - This property transforms the external_key into a human-readable display name - by replacing dots with ' > ' and capitalizing each word. - - Returns: - str: The human-readable action name (e.g., 'Content Libraries > Delete Library'). - """ - parts = self.external_key.split(".") - return " > ".join(part.replace("_", " ").title() for part in parts) - - def __str__(self): - """Human readable string representation of the action.""" - return self.name - - def __repr__(self): - """Developer friendly string representation of the action.""" - return self.namespaced_key - - -@define -class PermissionData: - """A permission combines an action with an effect (allow or deny). - - Permissions define whether a specific action should be allowed or denied. - They are typically associated with roles in the authorization system. - - Attributes: - action: The action being permitted or denied (ActionData instance). - effect: The effect of the permission, either 'allow' or 'deny' (default: 'allow'). - - Examples: - >>> read_action = ActionData(external_key='read') - >>> permission = PermissionData(action=read_action, effect='allow') - >>> str(permission) - 'Read - allow' - >>> write_action = ActionData(external_key='write') - >>> deny_perm = PermissionData(action=write_action, effect='deny') - >>> str(deny_perm) - 'Write - deny' - """ - - action: ActionData = None - effect: Literal["allow", "deny"] = "allow" - - @property - def identifier(self) -> str: - """Get the permission identifier. - - Returns: - str: The permission identifier (e.g., 'content_libraries.delete_library'). - """ - return self.action.external_key - - def __eq__(self, other: "PermissionData") -> bool: - """Compare permissions based on their action identifier. - - Two PermissionData instances are considered equal if they have the same action's - external_key and effect. - - Args: - other: Another PermissionData instance or any object. - - Returns: - bool: True if the actions match, False otherwise. - - Example: - >>> perm1 = PermissionData(action=ActionData(external_key='view'), effect='allow') - >>> perm2 = PermissionData(action=ActionData(external_key='view'), effect='allow') - >>> perm1 == perm2 # True - same action and effect - True - >>> perm1 in [perm2] # Uses __eq__ - True - """ - if self.action is None or other.action is None: - return False - return self.action.external_key == other.action.external_key and self.effect == other.effect - - def __str__(self): - """Human readable string representation of the permission and its effect.""" - return f"{self.action} - {self.effect}" - - def __repr__(self): - """Developer friendly string representation of the permission.""" - return f"{self.action.namespaced_key} => {self.effect}" - - @define(eq=False) class RoleData(AuthZData): """A role is a named collection of permissions that can be assigned to subjects. diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index e2a4d20f..b2dd2654 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -11,10 +11,6 @@ from openedx_authz.api.data import ( ActionData, - ContentLibraryData, - CourseOverviewData, - OrgContentLibraryGlobData, - OrgCourseOverviewGlobData, PermissionData, RoleAssignmentData, RoleData, @@ -39,7 +35,6 @@ unassign_subject_from_all_roles, ) from openedx_authz.api.utils import filter_user_assignments, get_user_assignment_map -from openedx_authz.constants.permissions import COURSES_MANAGE_COURSE_TEAM, MANAGE_LIBRARY_TEAM __all__ = [ "assign_role_to_user_in_scope", @@ -224,11 +219,8 @@ def _filter_allowed_assignments( for assignment in assignments: permission = None - # For CourseOverviewData and ContentLibraryData, check for the view permission - if isinstance(assignment.scope, (CourseOverviewData, OrgCourseOverviewGlobData)): - permission = COURSES_MANAGE_COURSE_TEAM.identifier - elif isinstance(assignment.scope, (ContentLibraryData, OrgContentLibraryGlobData)): - permission = MANAGE_LIBRARY_TEAM.identifier + # Get the permission needed to view the specific scope in the admin console + permission = assignment.scope.get_admin_view_permission().identifier if permission and is_user_allowed( user_external_key=user_external_key, diff --git a/openedx_authz/api/utils.py b/openedx_authz/api/utils.py index 8cc9aa57..9ce10091 100644 --- a/openedx_authz/api/utils.py +++ b/openedx_authz/api/utils.py @@ -54,6 +54,17 @@ def filter_user_assignments( ) -> list[UserAssignments]: """ Filter user assignments by orgs or scopes. + + Returns a list of users that have at least one assignment matching the filters, + with only the matching assignments for each matching user. + + Args: + users_with_assignments (list[UserAssignments]): The list of users with their role assignments. + by (UserAssignmentsFilter): The filter type (by orgs or scopes). + values (list[str]): The list of orgs or scopes to filter by. + + Returns: + list[UserAssignments]: The filtered list of users with their role assignments. """ if not values: return users_with_assignments @@ -62,7 +73,7 @@ def _get_value_to_filter(assignment: RoleAssignmentData) -> str: if by == UserAssignmentsFilter.SCOPES: return assignment.scope.external_key elif by == UserAssignmentsFilter.ORGS: - return assignment.scope.org + return getattr(assignment.scope, "org", None) else: raise ValueError(f"Invalid filter: '{by}'. Must be one of {[f.value for f in UserAssignmentsFilter]}") diff --git a/openedx_authz/constants/permissions.py b/openedx_authz/constants/permissions.py index f9064cc1..ed1ba42c 100644 --- a/openedx_authz/constants/permissions.py +++ b/openedx_authz/constants/permissions.py @@ -2,7 +2,7 @@ Default permission constants. """ -from openedx_authz.api.data import ActionData, PermissionData +from openedx_authz.data import ActionData, PermissionData # Content Library Permissions diff --git a/openedx_authz/data.py b/openedx_authz/data.py new file mode 100644 index 00000000..2df5af37 --- /dev/null +++ b/openedx_authz/data.py @@ -0,0 +1,115 @@ +""" +Top-level data classes for actions and permissions. + +These are defined here (rather than in openedx_authz.api.data) to avoid a +circular import between openedx_authz.api.data and openedx_authz.constants.permissions. +""" + +from typing import ClassVar, Literal + +from attrs import define + +AUTHZ_POLICY_ATTRIBUTES_SEPARATOR = "^" + + +class AuthzBaseClass: + """Base class for all authz classes.""" + + SEPARATOR: ClassVar[str] = AUTHZ_POLICY_ATTRIBUTES_SEPARATOR + NAMESPACE: ClassVar[str] = None + + +@define +class AuthZData(AuthzBaseClass): + """Base class for all authz data classes.""" + + external_key: str = "" + namespaced_key: str = "" + + def __attrs_post_init__(self): + """Derive namespaced_key from external_key or vice versa after initialization.""" + if not self.NAMESPACE: + return + + if not self.external_key and not self.namespaced_key: + raise ValueError("Either external_key or namespaced_key must be provided.") + + if not self.namespaced_key: + self.namespaced_key = f"{self.NAMESPACE}{self.SEPARATOR}{self.external_key}" + + if not self.external_key: + self.external_key = self.namespaced_key.split(self.SEPARATOR, 1)[1] + + +@define +class ActionData(AuthZData): + """ + An action represents an operation that can be performed in the authorization system. + + Attributes: + NAMESPACE: 'act' for actions. + external_key: The action identifier (e.g., 'content_libraries.view_library'). + namespaced_key: The action identifier with namespace (e.g., 'act^content_libraries.view_library'). + + Examples: + >>> action = ActionData(external_key='content_libraries.delete_library') + >>> action.namespaced_key + 'act^content_libraries.delete_library' + >>> action.name + 'Content Libraries > Delete Library' + """ + + NAMESPACE: ClassVar[str] = "act" + + @property + def name(self) -> str: + """The human-readable name of the action (e.g., 'Content Libraries > Delete Library').""" + parts = self.external_key.split(".") + return " > ".join(part.replace("_", " ").title() for part in parts) + + def __str__(self): + """Human readable string representation of the action.""" + return self.name + + def __repr__(self): + """Developer friendly string representation of the action.""" + return self.namespaced_key + + +@define +class PermissionData: + """ + A permission combines an action with an effect (allow or deny). + + Attributes: + action: The action being permitted or denied (ActionData instance). + effect: The effect of the permission, either 'allow' or 'deny' (default: 'allow'). + + Examples: + >>> read_action = ActionData(external_key='read') + >>> permission = PermissionData(action=read_action, effect='allow') + >>> str(permission) + 'Read - allow' + """ + + action: ActionData = None + effect: Literal["allow", "deny"] = "allow" + + @property + def identifier(self) -> str: + """Get the permission identifier.""" + return self.action.external_key + + def __eq__(self, other: "PermissionData") -> bool: + """Compare permissions based on their action identifier and effect.""" + if self.action is None or other.action is None: + return False + return self.action.external_key == other.action.external_key and self.effect == other.effect + + def __str__(self): + """Human readable string representation of the permission and its effect.""" + return f"{self.action} - {self.effect}" + + def __repr__(self): + """Developer friendly string representation of the permission.""" + return f"{self.action.namespaced_key} => {self.effect}" diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 49b11eb8..81d312fd 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -404,6 +404,10 @@ def get_object(self): def exists(self) -> bool: return False + @classmethod + def get_admin_view_permission(cls): + raise NotImplementedError("Not implemented for TempScope") + # Metaclass should have recreated the registries on the class self.assertTrue(hasattr(TempScope, "scope_registry")) self.assertTrue(hasattr(TempScope, "glob_registry")) diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index 5d100b56..45aaa472 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -1065,10 +1065,10 @@ class TestTeamMembersAPIView(ViewTestMixin): (admin_1..3 are staff/superuser; regular_1..8 are plain users) Visibility via filter_allowed_assignments: - - Staff/superuser: sees all 11 users (is_admin_or_superuser_check grants MANAGE_LIBRARY_TEAM on lib scopes) - - regular_5 (library_admin in Org3:LIB3): MANAGE_LIBRARY_TEAM granted → sees Org3 members (5) - - regular_1 (library_user in Org1:LIB1): no MANAGE_LIBRARY_TEAM → sees 0 - - regular_3 (library_user in Org2:LIB2): no MANAGE_LIBRARY_TEAM → sees 0 + - Staff/superuser: sees all 11 users (is_admin_or_superuser_check grants VIEW_LIBRARY_TEAM on lib scopes) + - 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 """ @@ -1091,24 +1091,23 @@ def setUp(self): @data( # Staff/superuser sees all users across all scopes ("admin_1", 11), - # regular_5 has LIBRARY_ADMIN in lib:Org3:LIB3 (MANAGE_LIBRARY_TEAM granted) → sees only Org3 members - ("regular_5", 5), - # regular_1 has LIBRARY_USER in lib:Org1:LIB1 (no MANAGE_LIBRARY_TEAM) → sees nothing - ("regular_1", 0), - # regular_3 has LIBRARY_USER in lib:Org2:LIB2 (no MANAGE_LIBRARY_TEAM) → sees nothing - ("regular_3", 0), + # regular_1 has LIBRARY_USER in lib:Org1:LIB1 (VIEW_LIBRARY_TEAM granted) → sees only Org1 members + ("regular_1", 3), + # regular_3 has LIBRARY_USER in lib:Org2:LIB2 (VIEW_LIBRARY_TEAM granted) → sees only Org2 members + ("regular_3", 3), + # regular_6 has LIBRARY_AUTHOR in lib:Org3:LIB3 (VIEW_LIBRARY_TEAM granted) → sees only Org3 members + ("regular_6", 5), # regular_9 has no assignments → sees nothing ("regular_9", 0), ) @unpack def test_visibility_limited_to_accessible_scopes(self, username: str, expected_count: int): - """Calling user only sees assignments for scopes it has MANAGE_*_TEAM access to. + """Calling user only sees assignments for scopes it has VIEW_*_TEAM access to. Expected result: - Staff/superuser sees all users across all scopes. - - Regular users only see members of scopes they can manage the team for. - - Users without MANAGE_*_TEAM permission see no results. - + - Regular users only see members of scopes they have VIEW_*_TEAM permission for. + - Users with no assignments see no results. """ user = User.objects.get(username=username) self.client.force_authenticate(user=user) From ff21bfdd5b9037dff5e7d4d9f857b704b18d621b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Thu, 9 Apr 2026 12:11:20 -0600 Subject: [PATCH 3/3] squash!: Fix conflicts and bump version --- openedx_authz/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index f897dc2c..f729fdf5 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "1.4.0" +__version__ = "1.5.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))