diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 58b24951..49f07b03 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,14 @@ Change Log Unreleased ********** +1.11.0 - 2026-04-16 +******************* + +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__)) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 1e484036..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,6 +102,26 @@ 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 @@ -121,14 +130,53 @@ class AddUsersToRoleWithScopeSerializer( 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""" + """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(RoleScopeValidationMixin, self).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." + ) + + for scope_value in scopes_list: + self._validate_scope_and_role(scope_value, role_value) + + validated_data.pop("scope", None) + validated_data["scopes"] = scopes_list + 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),