Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
*******************

Expand All @@ -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
=====
Expand Down
2 changes: 1 addition & 1 deletion openedx_authz/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

import os

__version__ = "1.10.0"
__version__ = "1.11.0"

ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))
82 changes: 65 additions & 17 deletions openedx_authz/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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


Expand All @@ -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,
Expand Down
37 changes: 19 additions & 18 deletions openedx_authz/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: what happens if the user is not activated? Should we assign them a role?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question, in this case, this logic already existed and we don't have a specific requirement for this, perhaps we should create a tech debt issue to review this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks

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)
Expand Down
89 changes: 89 additions & 0 deletions openedx_authz/tests/rest_api/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down