Skip to content

Commit 0a57aa2

Browse files
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 2663982 commit 0a57aa2

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

118118

119119
class AddUsersToRoleWithScopeSerializer(
120-
RoleScopeValidationMixin,
121120
RoleMixin,
122121
ScopeMixin,
123122
): # pylint: disable=abstract-method
124-
"""Serializer for adding users to a role with a scope."""
123+
"""Serializer for adding users to a role with one or more scopes.
124+
125+
Accepts either a single ``scope`` string (backward-compatible) or a
126+
``scopes`` list for bulk assignment. Exactly one of the two must be
127+
provided per request.
128+
"""
125129

130+
scope = serializers.CharField(max_length=255, required=False, default=None, allow_null=True)
131+
scopes = serializers.ListField(
132+
child=serializers.CharField(max_length=255),
133+
required=False,
134+
default=None,
135+
)
126136
users = serializers.ListField(child=serializers.CharField(max_length=255), allow_empty=False)
127137

128138
def validate_users(self, value) -> list[str]:
129139
"""Eliminate duplicates preserving order"""
130140
return list(dict.fromkeys(value))
131141

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

133189
class RemoveUsersFromRoleWithScopeSerializer(
134190
RoleScopeValidationMixin,

openedx_authz/rest_api/v1/views.py

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -321,30 +321,31 @@ def get(self, request: HttpRequest) -> Response:
321321
)
322322
@authz_permissions([permissions.MANAGE_LIBRARY_TEAM.identifier])
323323
def put(self, request: HttpRequest) -> Response:
324-
"""Assign multiple users to a specific role within a scope."""
324+
"""Assign multiple users to a specific role within one or more scopes."""
325325
serializer = AddUsersToRoleWithScopeSerializer(data=request.data)
326326
serializer.is_valid(raise_exception=True)
327327
data = serializer.validated_data
328328

329329
completed, errors = [], []
330-
for user_identifier in data["users"]:
331-
response_dict = {"user_identifier": user_identifier}
332-
try:
333-
user = get_user_by_username_or_email(user_identifier)
334-
result = api.assign_role_to_user_in_scope(user.username, data["role"], data["scope"])
335-
if result:
336-
response_dict["status"] = RoleOperationStatus.ROLE_ADDED
337-
completed.append(response_dict)
338-
else:
339-
response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE
330+
for scope_value in data["scopes"]:
331+
for user_identifier in data["users"]:
332+
response_dict = {"user_identifier": user_identifier, "scope": scope_value}
333+
try:
334+
user = get_user_by_username_or_email(user_identifier)
335+
result = api.assign_role_to_user_in_scope(user.username, data["role"], scope_value)
336+
if result:
337+
response_dict["status"] = RoleOperationStatus.ROLE_ADDED
338+
completed.append(response_dict)
339+
else:
340+
response_dict["error"] = RoleOperationError.USER_ALREADY_HAS_ROLE
341+
errors.append(response_dict)
342+
except User.DoesNotExist:
343+
response_dict["error"] = RoleOperationError.USER_NOT_FOUND
344+
errors.append(response_dict)
345+
except Exception as e: # pylint: disable=broad-exception-caught
346+
logger.error(f"Error assigning role to user {user_identifier} in scope {scope_value}: {e}")
347+
response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR
340348
errors.append(response_dict)
341-
except User.DoesNotExist:
342-
response_dict["error"] = RoleOperationError.USER_NOT_FOUND
343-
errors.append(response_dict)
344-
except Exception as e: # pylint: disable=broad-exception-caught
345-
logger.error(f"Error assigning role to user {user_identifier}: {e}")
346-
response_dict["error"] = RoleOperationError.ROLE_ASSIGNMENT_ERROR
347-
errors.append(response_dict)
348349

349350
response_data = {"completed": completed, "errors": errors}
350351
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
@@ -542,6 +542,95 @@ def test_add_users_to_role_invalid_data(self, request_data: dict):
542542

543543
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
544544

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

0 commit comments

Comments
 (0)