Skip to content

Commit 7264aba

Browse files
committed
fix: improve error handling in permission validation and role assignment
1 parent 13a1114 commit 7264aba

1 file changed

Lines changed: 13 additions & 3 deletions

File tree

openedx_authz/rest_api/v1/views.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ def post(self, request: HttpRequest) -> Response:
104104
)
105105
except Exception as e: # pylint: disable=broad-exception-caught
106106
logger.error(f"Error validating permission for user {username}: {e}")
107+
return Response(
108+
data={"message": "An error occurred while validating permissions"},
109+
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
110+
)
107111

108112
serializer = PermissionValidationResponseSerializer(response_data, many=True)
109113
return Response(serializer.data, status=status.HTTP_200_OK)
@@ -218,13 +222,16 @@ def put(self, request: HttpRequest) -> Response:
218222
for user_identifier in serializer.validated_data["users"]:
219223
try:
220224
user = get_user_by_username_or_email(user_identifier)
221-
api.assign_role_to_user_in_scope(user.username, role_name, scope)
225+
result = api.assign_role_to_user_in_scope(user.username, role_name, scope)
226+
if not result:
227+
errors.append({"user_identifier": user_identifier, "error": "user_already_has_role"})
228+
continue
222229
completed.append({"user_identifier": user_identifier, "status": "role_added"})
223230
except User.DoesNotExist:
224231
errors.append({"user_identifier": user_identifier, "error": "user_not_found"})
225232
except Exception as e: # pylint: disable=broad-exception-caught
226233
logger.error(f"Error assigning role to user {user_identifier}: {e}")
227-
errors.append({"user_identifier": user_identifier, "error": "assignment_failed"})
234+
errors.append({"user_identifier": user_identifier, "error": "role_assignment_failed"})
228235

229236
response_data = {"completed": completed, "errors": errors}
230237
return Response(response_data, status=status.HTTP_207_MULTI_STATUS)
@@ -256,7 +263,10 @@ def delete(self, request: HttpRequest) -> Response:
256263
for user_identifier in user_identifiers:
257264
try:
258265
user = get_user_by_username_or_email(user_identifier)
259-
api.unassign_role_from_user(user.username, role_name, scope)
266+
result = api.unassign_role_from_user(user.username, role_name, scope)
267+
if not result:
268+
errors.append({"user_identifier": user_identifier, "error": "user_does_not_have_role"})
269+
continue
260270
completed.append({"user_identifier": user_identifier, "status": "role_removed"})
261271
except User.DoesNotExist:
262272
errors.append({"user_identifier": user_identifier, "error": "user_not_found"})

0 commit comments

Comments
 (0)