Skip to content

Commit 0a59946

Browse files
ccantilloccantillo
committed
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.
1 parent 218e294 commit 0a59946

3 files changed

Lines changed: 166 additions & 20 deletions

File tree

‎openedx_authz/rest_api/v1/serializers.py‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,74 @@ def validate(self, attrs) -> dict:
8888

8989

9090
class AddUsersToRoleWithScopeSerializer(
91-
RoleScopeValidationMixin,
9291
RoleMixin,
9392
ScopeMixin,
9493
): # pylint: disable=abstract-method
95-
"""Serializer for adding users to a role with a scope."""
94+
"""Serializer for adding users to a role with one or more scopes.
95+
96+
Accepts either a single ``scope`` string (backward-compatible) or a
97+
``scopes`` list for bulk assignment. Exactly one of the two must be
98+
provided per request.
99+
"""
96100

101+
scope = serializers.CharField(max_length=255, required=False, default=None, allow_null=True)
102+
scopes = serializers.ListField(
103+
child=serializers.CharField(max_length=255),
104+
required=False,
105+
default=None,
106+
)
97107
users = serializers.ListField(child=serializers.CharField(max_length=255), allow_empty=False)
98108

99109
def validate_users(self, value) -> list[str]:
100110
"""Eliminate duplicates preserving order"""
101111
return list(dict.fromkeys(value))
102112

113+
def validate(self, attrs) -> dict:
114+
"""Validate that exactly one of 'scope'/'scopes' is provided and that every
115+
scope exists in the registry, exists in the system, and supports the role.
116+
Returns validated data with a unified ``scopes`` list of strings.
117+
"""
118+
validated_data = super().validate(attrs)
119+
scope = validated_data.get("scope")
120+
scopes = validated_data.get("scopes")
121+
role_value = validated_data["role"]
122+
123+
if scope and scopes is not None:
124+
raise serializers.ValidationError(
125+
"Provide either 'scope' or 'scopes', not both."
126+
)
127+
128+
scopes_list = scopes if scopes is not None else ([scope] if scope else None)
129+
if not scopes_list:
130+
raise serializers.ValidationError(
131+
"Either 'scope' or 'scopes' must be provided."
132+
)
133+
134+
validated_scopes = []
135+
for scope_value in scopes_list:
136+
try:
137+
scope_obj = api.ScopeData(external_key=scope_value)
138+
except ValueError as exc:
139+
raise serializers.ValidationError({"scope": str(exc)}) from exc
140+
141+
if not scope_obj.exists():
142+
raise serializers.ValidationError({"scope": f"Scope '{scope_value}' does not exist"})
143+
144+
role_obj = api.RoleData(external_key=role_value)
145+
generic_scope = get_generic_scope(scope_obj)
146+
role_definitions = api.get_role_definitions_in_scope(generic_scope)
147+
148+
if role_obj not in role_definitions:
149+
raise serializers.ValidationError(
150+
{"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"}
151+
)
152+
153+
validated_scopes.append(scope_value)
154+
155+
validated_data.pop("scope", None)
156+
validated_data["scopes"] = validated_scopes
157+
return validated_data
158+
103159

104160
class RemoveUsersFromRoleWithScopeSerializer(
105161
RoleScopeValidationMixin,

‎openedx_authz/rest_api/v1/views.py‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -287,30 +287,31 @@ def get(self, request: HttpRequest) -> Response:
287287
)
288288
@authz_permissions([permissions.MANAGE_LIBRARY_TEAM.identifier])
289289
def put(self, request: HttpRequest) -> Response:
290-
"""Assign multiple users to a specific role within a scope."""
290+
"""Assign multiple users to a specific role within one or more scopes."""
291291
serializer = AddUsersToRoleWithScopeSerializer(data=request.data)
292292
serializer.is_valid(raise_exception=True)
293293
data = serializer.validated_data
294294

295295
completed, errors = [], []
296-
for user_identifier in data["users"]:
297-
response_dict = {"user_identifier": user_identifier}
298-
try:
299-
user = get_user_by_username_or_email(user_identifier)
300-
result = api.assign_role_to_user_in_scope(user.username, data["role"], data["scope"])
301-
if result:
302-
response_dict["status"] = RoleOperationStatus.ROLE_ADDED
303-
completed.append(response_dict)
304-
else:
305-
response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE
296+
for scope_value in data["scopes"]:
297+
for user_identifier in data["users"]:
298+
response_dict = {"user_identifier": user_identifier, "scope": scope_value}
299+
try:
300+
user = get_user_by_username_or_email(user_identifier)
301+
result = api.assign_role_to_user_in_scope(user.username, data["role"], scope_value)
302+
if result:
303+
response_dict["status"] = RoleOperationStatus.ROLE_ADDED
304+
completed.append(response_dict)
305+
else:
306+
response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE
307+
errors.append(response_dict)
308+
except User.DoesNotExist:
309+
response_dict["error"] = RoleOperationError.USER_NOT_FOUND
310+
errors.append(response_dict)
311+
except Exception as e: # pylint: disable=broad-exception-caught
312+
logger.error(f"Error assigning role to user {user_identifier} in scope {scope_value}: {e}")
313+
response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR
306314
errors.append(response_dict)
307-
except User.DoesNotExist:
308-
response_dict["error"] = RoleOperationError.USER_NOT_FOUND
309-
errors.append(response_dict)
310-
except Exception as e: # pylint: disable=broad-exception-caught
311-
logger.error(f"Error assigning role to user {user_identifier}: {e}")
312-
response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR
313-
errors.append(response_dict)
314315

315316
response_data = {"completed": completed, "errors": errors}
316317
return Response(response_data, status=status.HTTP_207_MULTI_STATUS)

‎openedx_authz/tests/rest_api/test_views.py‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,95 @@ def test_add_users_to_role_invalid_data(self, request_data: dict):
548548

549549
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
550550

551+
@data(
552+
# Single scope in 'scopes' list - one user, success
553+
(["admin_1"], ["lib:Org1:LIB3"], 1, 0),
554+
# Single scope in 'scopes' list - multiple users, all success
555+
(["admin_1", "regular_1"], ["lib:Org1:LIB3"], 2, 0),
556+
# Two scopes - one user, success in both
557+
(["admin_1"], ["lib:Org1:LIB3", "lib:Org2:LIB4"], 2, 0),
558+
# Two scopes - two users, all succeed (2 scopes * 2 users = 4 completed)
559+
(["admin_1", "regular_1"], ["lib:Org1:LIB3", "lib:Org2:LIB4"], 4, 0),
560+
# Three scopes - one user, success in all
561+
(["admin_1"], ["lib:Org1:LIB3", "lib:Org2:LIB4", "lib:Org3:LIB5"], 3, 0),
562+
)
563+
@unpack
564+
def test_add_users_to_role_multi_scope_success(
565+
self,
566+
users: list[str],
567+
scopes: list[str],
568+
expected_completed: int,
569+
expected_errors: int,
570+
):
571+
"""Test adding users to a role using the new 'scopes' list field.
572+
573+
Expected result:
574+
- Returns 207 MULTI-STATUS
575+
- Completed count equals len(users) * len(scopes) when all succeed
576+
- Each completed entry contains a 'scope' field
577+
"""
578+
role = roles.LIBRARY_ADMIN.external_key
579+
request_data = {"role": role, "scopes": scopes, "users": users}
580+
581+
with patch.object(api.ContentLibraryData, "exists", return_value=True):
582+
response = self.client.put(self.url, data=request_data, format="json")
583+
584+
self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS)
585+
self.assertEqual(len(response.data["completed"]), expected_completed)
586+
self.assertEqual(len(response.data["errors"]), expected_errors)
587+
for entry in response.data["completed"]:
588+
self.assertIn("scope", entry)
589+
self.assertIn(entry["scope"], scopes)
590+
591+
def test_add_users_to_role_backward_compat_response_includes_scope(self):
592+
"""Test that the old single-'scope' payload still works and response includes scope.
593+
594+
Expected result:
595+
- Returns 207 MULTI-STATUS
596+
- Each completed entry contains a 'scope' field matching the requested scope
597+
"""
598+
scope = "lib:Org1:LIB3"
599+
request_data = {
600+
"role": roles.LIBRARY_ADMIN.external_key,
601+
"scope": scope,
602+
"users": ["admin_1", "regular_1"],
603+
}
604+
605+
with patch.object(api.ContentLibraryData, "exists", return_value=True):
606+
response = self.client.put(self.url, data=request_data, format="json")
607+
608+
self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS)
609+
self.assertEqual(len(response.data["completed"]), 2)
610+
for entry in response.data["completed"]:
611+
self.assertIn("scope", entry)
612+
self.assertEqual(entry["scope"], scope)
613+
614+
@data(
615+
# Both 'scope' and 'scopes' provided
616+
{
617+
"role": roles.LIBRARY_ADMIN.external_key,
618+
"scope": "lib:Org1:LIB1",
619+
"scopes": ["lib:Org2:LIB2"],
620+
"users": ["admin_1"],
621+
},
622+
# 'scopes' as empty list
623+
{
624+
"role": roles.LIBRARY_ADMIN.external_key,
625+
"scopes": [],
626+
"users": ["admin_1"],
627+
},
628+
)
629+
def test_add_users_to_role_invalid_scopes_field(self, request_data: dict):
630+
"""Test that invalid combinations of scope/scopes fields return 400.
631+
632+
Expected result:
633+
- Returns 400 BAD REQUEST status
634+
"""
635+
with patch.object(DynamicScopePermission, "has_permission", return_value=True):
636+
response = self.client.put(self.url, data=request_data, format="json")
637+
638+
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
639+
551640
@data(
552641
# Unauthenticated
553642
(None, status.HTTP_401_UNAUTHORIZED),

0 commit comments

Comments
 (0)