Skip to content

Commit 161d276

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 a936a66 commit 161d276

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
@@ -103,18 +103,74 @@ def validate(self, attrs) -> dict:
103103

104104

105105
class AddUsersToRoleWithScopeSerializer(
106-
RoleScopeValidationMixin,
107106
RoleMixin,
108107
ScopeMixin,
109108
): # pylint: disable=abstract-method
110-
"""Serializer for adding users to a role with a scope."""
109+
"""Serializer for adding users to a role with one or more scopes.
110+
111+
Accepts either a single ``scope`` string (backward-compatible) or a
112+
``scopes`` list for bulk assignment. Exactly one of the two must be
113+
provided per request.
114+
"""
111115

116+
scope = serializers.CharField(max_length=255, required=False, default=None, allow_null=True)
117+
scopes = serializers.ListField(
118+
child=serializers.CharField(max_length=255),
119+
required=False,
120+
default=None,
121+
)
112122
users = serializers.ListField(child=serializers.CharField(max_length=255), allow_empty=False)
113123

114124
def validate_users(self, value) -> list[str]:
115125
"""Eliminate duplicates preserving order"""
116126
return list(dict.fromkeys(value))
117127

128+
def validate(self, attrs) -> dict:
129+
"""Validate that exactly one of 'scope'/'scopes' is provided and that every
130+
scope exists in the registry, exists in the system, and supports the role.
131+
Returns validated data with a unified ``scopes`` list of strings.
132+
"""
133+
validated_data = super().validate(attrs)
134+
scope = validated_data.get("scope")
135+
scopes = validated_data.get("scopes")
136+
role_value = validated_data["role"]
137+
138+
if scope and scopes is not None:
139+
raise serializers.ValidationError(
140+
"Provide either 'scope' or 'scopes', not both."
141+
)
142+
143+
scopes_list = scopes if scopes is not None else ([scope] if scope else None)
144+
if not scopes_list:
145+
raise serializers.ValidationError(
146+
"Either 'scope' or 'scopes' must be provided."
147+
)
148+
149+
validated_scopes = []
150+
for scope_value in scopes_list:
151+
try:
152+
scope_obj = api.ScopeData(external_key=scope_value)
153+
except ValueError as exc:
154+
raise serializers.ValidationError({"scope": str(exc)}) from exc
155+
156+
if not scope_obj.exists():
157+
raise serializers.ValidationError({"scope": f"Scope '{scope_value}' does not exist"})
158+
159+
role_obj = api.RoleData(external_key=role_value)
160+
generic_scope = get_generic_scope(scope_obj)
161+
role_definitions = api.get_role_definitions_in_scope(generic_scope)
162+
163+
if role_obj not in role_definitions:
164+
raise serializers.ValidationError(
165+
{"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"}
166+
)
167+
168+
validated_scopes.append(scope_value)
169+
170+
validated_data.pop("scope", None)
171+
validated_data["scopes"] = validated_scopes
172+
return validated_data
173+
118174

119175
class RemoveUsersFromRoleWithScopeSerializer(
120176
RoleScopeValidationMixin,

‎openedx_authz/rest_api/v1/views.py‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -304,30 +304,31 @@ def get(self, request: HttpRequest) -> Response:
304304
)
305305
@authz_permissions([permissions.MANAGE_LIBRARY_TEAM.identifier])
306306
def put(self, request: HttpRequest) -> Response:
307-
"""Assign multiple users to a specific role within a scope."""
307+
"""Assign multiple users to a specific role within one or more scopes."""
308308
serializer = AddUsersToRoleWithScopeSerializer(data=request.data)
309309
serializer.is_valid(raise_exception=True)
310310
data = serializer.validated_data
311311

312312
completed, errors = [], []
313-
for user_identifier in data["users"]:
314-
response_dict = {"user_identifier": user_identifier}
315-
try:
316-
user = get_user_by_username_or_email(user_identifier)
317-
result = api.assign_role_to_user_in_scope(user.username, data["role"], data["scope"])
318-
if result:
319-
response_dict["status"] = RoleOperationStatus.ROLE_ADDED
320-
completed.append(response_dict)
321-
else:
322-
response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE
313+
for scope_value in data["scopes"]:
314+
for user_identifier in data["users"]:
315+
response_dict = {"user_identifier": user_identifier, "scope": scope_value}
316+
try:
317+
user = get_user_by_username_or_email(user_identifier)
318+
result = api.assign_role_to_user_in_scope(user.username, data["role"], scope_value)
319+
if result:
320+
response_dict["status"] = RoleOperationStatus.ROLE_ADDED
321+
completed.append(response_dict)
322+
else:
323+
response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE
324+
errors.append(response_dict)
325+
except User.DoesNotExist:
326+
response_dict["error"] = RoleOperationError.USER_NOT_FOUND
327+
errors.append(response_dict)
328+
except Exception as e: # pylint: disable=broad-exception-caught
329+
logger.error(f"Error assigning role to user {user_identifier} in scope {scope_value}: {e}")
330+
response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR
323331
errors.append(response_dict)
324-
except User.DoesNotExist:
325-
response_dict["error"] = RoleOperationError.USER_NOT_FOUND
326-
errors.append(response_dict)
327-
except Exception as e: # pylint: disable=broad-exception-caught
328-
logger.error(f"Error assigning role to user {user_identifier}: {e}")
329-
response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR
330-
errors.append(response_dict)
331332

332333
response_data = {"completed": completed, "errors": errors}
333334
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
@@ -533,6 +533,95 @@ def test_add_users_to_role_invalid_data(self, request_data: dict):
533533

534534
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
535535

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

0 commit comments

Comments
 (0)