From 0a57aa20c9dbc4cdbbf927c4f8f71a689732a065 Mon Sep 17 00:00:00 2001 From: carlos cantillo Date: Mon, 13 Apr 2026 09:11:22 -0500 Subject: [PATCH 1/5] feat: add bulk scope support to PUT /api/authz/v1/roles/users/ Accept a new list field alongside the existing string, allowing role assignment across multiple scopes in one request while keeping full backward compatibility. Each response entry now includes a field. --- openedx_authz/rest_api/v1/serializers.py | 60 ++++++++++++++- openedx_authz/rest_api/v1/views.py | 37 ++++----- openedx_authz/tests/rest_api/test_views.py | 89 ++++++++++++++++++++++ 3 files changed, 166 insertions(+), 20 deletions(-) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 1e484036..d93a13f4 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -117,18 +117,74 @@ def validate(self, attrs) -> dict: class AddUsersToRoleWithScopeSerializer( - RoleScopeValidationMixin, RoleMixin, ScopeMixin, ): # pylint: disable=abstract-method - """Serializer for adding users to a role with a scope.""" + """Serializer for adding users to a role with one or more scopes. + + Accepts either a single ``scope`` string (backward-compatible) or a + ``scopes`` list for bulk assignment. Exactly one of the two must be + provided per request. + """ + scope = serializers.CharField(max_length=255, required=False, default=None, allow_null=True) + scopes = serializers.ListField( + child=serializers.CharField(max_length=255), + required=False, + default=None, + ) users = serializers.ListField(child=serializers.CharField(max_length=255), allow_empty=False) def validate_users(self, value) -> list[str]: """Eliminate duplicates preserving order""" return list(dict.fromkeys(value)) + def validate(self, attrs) -> dict: + """Validate that exactly one of 'scope'/'scopes' is provided and that every + scope exists in the registry, exists in the system, and supports the role. + Returns validated data with a unified ``scopes`` list of strings. + """ + validated_data = super().validate(attrs) + scope = validated_data.get("scope") + scopes = validated_data.get("scopes") + role_value = validated_data["role"] + + if scope and scopes is not None: + raise serializers.ValidationError( + "Provide either 'scope' or 'scopes', not both." + ) + + scopes_list = scopes if scopes is not None else ([scope] if scope else None) + if not scopes_list: + raise serializers.ValidationError( + "Either 'scope' or 'scopes' must be provided." + ) + + validated_scopes = [] + for scope_value in scopes_list: + try: + scope_obj = api.ScopeData(external_key=scope_value) + except ValueError as exc: + raise serializers.ValidationError({"scope": str(exc)}) from exc + + if not scope_obj.exists(): + raise serializers.ValidationError({"scope": f"Scope '{scope_value}' does not exist"}) + + role_obj = api.RoleData(external_key=role_value) + generic_scope = get_generic_scope(scope_obj) + role_definitions = api.get_role_definitions_in_scope(generic_scope) + + if role_obj not in role_definitions: + raise serializers.ValidationError( + {"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"} + ) + + validated_scopes.append(scope_value) + + validated_data.pop("scope", None) + validated_data["scopes"] = validated_scopes + return validated_data + class RemoveUsersFromRoleWithScopeSerializer( RoleScopeValidationMixin, diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index a2e43cb8..7bf4f832 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -321,30 +321,31 @@ def get(self, request: HttpRequest) -> Response: ) @authz_permissions([permissions.MANAGE_LIBRARY_TEAM.identifier]) def put(self, request: HttpRequest) -> Response: - """Assign multiple users to a specific role within a scope.""" + """Assign multiple users to a specific role within one or more scopes.""" serializer = AddUsersToRoleWithScopeSerializer(data=request.data) serializer.is_valid(raise_exception=True) data = serializer.validated_data completed, errors = [], [] - for user_identifier in data["users"]: - response_dict = {"user_identifier": user_identifier} - try: - user = get_user_by_username_or_email(user_identifier) - result = api.assign_role_to_user_in_scope(user.username, data["role"], data["scope"]) - if result: - response_dict["status"] = RoleOperationStatus.ROLE_ADDED - completed.append(response_dict) - else: - response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE + for scope_value in data["scopes"]: + for user_identifier in data["users"]: + response_dict = {"user_identifier": user_identifier, "scope": scope_value} + try: + user = get_user_by_username_or_email(user_identifier) + result = api.assign_role_to_user_in_scope(user.username, data["role"], scope_value) + if result: + response_dict["status"] = RoleOperationStatus.ROLE_ADDED + completed.append(response_dict) + else: + response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE + errors.append(response_dict) + except User.DoesNotExist: + response_dict["error"] = RoleOperationError.USER_NOT_FOUND + errors.append(response_dict) + except Exception as e: # pylint: disable=broad-exception-caught + logger.error(f"Error assigning role to user {user_identifier} in scope {scope_value}: {e}") + response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR errors.append(response_dict) - except User.DoesNotExist: - response_dict["error"] = RoleOperationError.USER_NOT_FOUND - errors.append(response_dict) - except Exception as e: # pylint: disable=broad-exception-caught - logger.error(f"Error assigning role to user {user_identifier}: {e}") - response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR - errors.append(response_dict) response_data = {"completed": completed, "errors": errors} return Response(response_data, status=status.HTTP_207_MULTI_STATUS) diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index e721dc16..5ad5055a 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -542,6 +542,95 @@ def test_add_users_to_role_invalid_data(self, request_data: dict): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + @data( + # Single scope in 'scopes' list - one user, success + (["admin_1"], ["lib:Org1:LIB3"], 1, 0), + # Single scope in 'scopes' list - multiple users, all success + (["admin_1", "regular_1"], ["lib:Org1:LIB3"], 2, 0), + # Two scopes - one user, success in both + (["admin_1"], ["lib:Org1:LIB3", "lib:Org2:LIB4"], 2, 0), + # Two scopes - two users, all succeed (2 scopes * 2 users = 4 completed) + (["admin_1", "regular_1"], ["lib:Org1:LIB3", "lib:Org2:LIB4"], 4, 0), + # Three scopes - one user, success in all + (["admin_1"], ["lib:Org1:LIB3", "lib:Org2:LIB4", "lib:Org3:LIB5"], 3, 0), + ) + @unpack + def test_add_users_to_role_multi_scope_success( + self, + users: list[str], + scopes: list[str], + expected_completed: int, + expected_errors: int, + ): + """Test adding users to a role using the new 'scopes' list field. + + Expected result: + - Returns 207 MULTI-STATUS + - Completed count equals len(users) * len(scopes) when all succeed + - Each completed entry contains a 'scope' field + """ + role = roles.LIBRARY_ADMIN.external_key + request_data = {"role": role, "scopes": scopes, "users": users} + + with patch.object(api.ContentLibraryData, "exists", return_value=True): + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS) + self.assertEqual(len(response.data["completed"]), expected_completed) + self.assertEqual(len(response.data["errors"]), expected_errors) + for entry in response.data["completed"]: + self.assertIn("scope", entry) + self.assertIn(entry["scope"], scopes) + + def test_add_users_to_role_backward_compat_response_includes_scope(self): + """Test that the old single-'scope' payload still works and response includes scope. + + Expected result: + - Returns 207 MULTI-STATUS + - Each completed entry contains a 'scope' field matching the requested scope + """ + scope = "lib:Org1:LIB3" + request_data = { + "role": roles.LIBRARY_ADMIN.external_key, + "scope": scope, + "users": ["admin_1", "regular_1"], + } + + with patch.object(api.ContentLibraryData, "exists", return_value=True): + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS) + self.assertEqual(len(response.data["completed"]), 2) + for entry in response.data["completed"]: + self.assertIn("scope", entry) + self.assertEqual(entry["scope"], scope) + + @data( + # Both 'scope' and 'scopes' provided + { + "role": roles.LIBRARY_ADMIN.external_key, + "scope": "lib:Org1:LIB1", + "scopes": ["lib:Org2:LIB2"], + "users": ["admin_1"], + }, + # 'scopes' as empty list + { + "role": roles.LIBRARY_ADMIN.external_key, + "scopes": [], + "users": ["admin_1"], + }, + ) + def test_add_users_to_role_invalid_scopes_field(self, request_data: dict): + """Test that invalid combinations of scope/scopes fields return 400. + + Expected result: + - Returns 400 BAD REQUEST status + """ + with patch.object(DynamicScopePermission, "has_permission", return_value=True): + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + @data( # Unauthenticated (None, status.HTTP_401_UNAUTHORIZED), From 39b4a9caa58d39b5ff54b13e60260fe1780683c0 Mon Sep 17 00:00:00 2001 From: carlos cantillo Date: Tue, 14 Apr 2026 10:32:22 -0500 Subject: [PATCH 2/5] refactor: simplify validated_scopes list in AddUsersToRoleWithScopeSerializer --- openedx_authz/rest_api/v1/serializers.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index d93a13f4..65893ecd 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -160,7 +160,6 @@ def validate(self, attrs) -> dict: "Either 'scope' or 'scopes' must be provided." ) - validated_scopes = [] for scope_value in scopes_list: try: scope_obj = api.ScopeData(external_key=scope_value) @@ -179,10 +178,8 @@ def validate(self, attrs) -> dict: {"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"} ) - validated_scopes.append(scope_value) - validated_data.pop("scope", None) - validated_data["scopes"] = validated_scopes + validated_data["scopes"] = scopes_list return validated_data From 9d7c26be0b6e38610fc6568f4f2d2e176bd41d43 Mon Sep 17 00:00:00 2001 From: carlos cantillo Date: Thu, 16 Apr 2026 11:55:38 -0500 Subject: [PATCH 3/5] chore: bump version to 1.6.0 and update CHANGELOG --- CHANGELOG.rst | 10 +++++++++- openedx_authz/__init__.py | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 58b24951..19f333b5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,14 @@ Change Log Unreleased ********** +1.11.0 - 2026-04-13 +******************* + +Added +===== + +* Add bulk scope support to ``PUT /api/authz/v1/roles/users/``: accept a ``scopes`` list field to assign a role across multiple scopes in a single request, while keeping backward compatibility with the existing single ``scope`` field. + 1.10.0 - 2026-04-16 ******************* @@ -23,7 +31,7 @@ Added * Add ``scopes/`` endpoint to list all scopes (courses and libraries), sorted by org, with search and pagination support. 1.9.0 - 2026-04-14 -****************** +******************* Added ===== diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index 762138e2..c6f61fe9 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "1.10.0" +__version__ = "1.11.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) From 2d5ab84a1597a88109700e2a6b7e6190ae8d6455 Mon Sep 17 00:00:00 2001 From: carlos cantillo Date: Thu, 16 Apr 2026 12:27:43 -0500 Subject: [PATCH 4/5] refactor: extract scope/role validation into reusable helper in RoleScopeValidationMixin --- openedx_authz/rest_api/v1/serializers.py | 61 +++++++++++------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 65893ecd..fd85064e 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -76,28 +76,17 @@ class PermissionValidationResponseSerializer(PermissionValidationSerializer): # class RoleScopeValidationMixin(serializers.Serializer): # pylint: disable=abstract-method """Mixin providing role and scope validation logic.""" - def validate(self, attrs) -> dict: - """Validate that the specified role and scope are valid and that the role exists in the scope. - - This method performs the following validations: - 1. Validates that the scope is registered in the scope registry - 2. Validates that the scope exists in the system - 3. Validates that the role is defined into the roles assigned to the scope + def _validate_scope_and_role(self, scope_value: str, role_value: str) -> None: + """Validate that a single scope exists and the role is defined in it. Args: - attrs: Dictionary containing 'role' and 'scope' keys with their string values. - - Returns: - dict: The validated data dictionary with 'role' and 'scope' keys. + scope_value: The scope string to validate. + role_value: The role string to validate against the scope. Raises: serializers.ValidationError: If the scope is not registered, doesn't exist, or if the role is not defined in the scope. """ - validated_data = super().validate(attrs) - scope_value = validated_data["scope"] - role_value = validated_data["role"] - try: scope = api.ScopeData(external_key=scope_value) except ValueError as exc: @@ -113,10 +102,31 @@ def validate(self, attrs) -> dict: if role not in role_definitions: raise serializers.ValidationError({"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"}) + def validate(self, attrs) -> dict: + """Validate that the specified role and scope are valid and that the role exists in the scope. + + This method performs the following validations: + 1. Validates that the scope is registered in the scope registry + 2. Validates that the scope exists in the system + 3. Validates that the role is defined into the roles assigned to the scope + + Args: + attrs: Dictionary containing 'role' and 'scope' keys with their string values. + + Returns: + dict: The validated data dictionary with 'role' and 'scope' keys. + + Raises: + serializers.ValidationError: If the scope is not registered, doesn't exist, + or if the role is not defined in the scope. + """ + validated_data = super().validate(attrs) + self._validate_scope_and_role(validated_data["scope"], validated_data["role"]) return validated_data class AddUsersToRoleWithScopeSerializer( + RoleScopeValidationMixin, RoleMixin, ScopeMixin, ): # pylint: disable=abstract-method @@ -136,7 +146,7 @@ class AddUsersToRoleWithScopeSerializer( users = serializers.ListField(child=serializers.CharField(max_length=255), allow_empty=False) def validate_users(self, value) -> list[str]: - """Eliminate duplicates preserving order""" + """Eliminate duplicates preserving order.""" return list(dict.fromkeys(value)) def validate(self, attrs) -> dict: @@ -144,7 +154,7 @@ def validate(self, attrs) -> dict: scope exists in the registry, exists in the system, and supports the role. Returns validated data with a unified ``scopes`` list of strings. """ - validated_data = super().validate(attrs) + validated_data = super(RoleScopeValidationMixin, self).validate(attrs) scope = validated_data.get("scope") scopes = validated_data.get("scopes") role_value = validated_data["role"] @@ -161,22 +171,7 @@ def validate(self, attrs) -> dict: ) for scope_value in scopes_list: - try: - scope_obj = api.ScopeData(external_key=scope_value) - except ValueError as exc: - raise serializers.ValidationError({"scope": str(exc)}) from exc - - if not scope_obj.exists(): - raise serializers.ValidationError({"scope": f"Scope '{scope_value}' does not exist"}) - - role_obj = api.RoleData(external_key=role_value) - generic_scope = get_generic_scope(scope_obj) - role_definitions = api.get_role_definitions_in_scope(generic_scope) - - if role_obj not in role_definitions: - raise serializers.ValidationError( - {"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"} - ) + self._validate_scope_and_role(scope_value, role_value) validated_data.pop("scope", None) validated_data["scopes"] = scopes_list From f47d3612ca827bec3b6a9ef9cf12d9017638d39b Mon Sep 17 00:00:00 2001 From: ccantillo <43839760+ccantillo@users.noreply.github.com> Date: Thu, 16 Apr 2026 13:16:41 -0500 Subject: [PATCH 5/5] chore: update CHANGELOG for bulk scope role assignment Made-with: Cursor --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 19f333b5..49f07b03 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,7 +14,7 @@ Change Log Unreleased ********** -1.11.0 - 2026-04-13 +1.11.0 - 2026-04-16 ******************* Added