diff --git a/server/openslides/agenda/views.py b/server/openslides/agenda/views.py index 3c5b83410..ef1cbc0ab 100644 --- a/server/openslides/agenda/views.py +++ b/server/openslides/agenda/views.py @@ -346,6 +346,9 @@ class ListOfSpeakersViewSet( raise ValidationError({"detail": "The list of speakers is closed."}) user = self.request.user else: + if not isinstance(user_id, int): + raise ValidationError({"detail": "user_id has to be an int."}) + point_of_order = False # not for someone else # Add someone else. if not has_perm( @@ -353,8 +356,8 @@ class ListOfSpeakersViewSet( ): self.permission_denied(request) try: - user = get_user_model().objects.get(pk=int(user_id)) - except (ValueError, get_user_model().DoesNotExist): + user = get_user_model().objects.get(pk=user_id) + except get_user_model().DoesNotExist: raise ValidationError({"detail": "User does not exist."}) # Try to add the user. This ensurse that a user is not twice in the diff --git a/server/openslides/assignments/views.py b/server/openslides/assignments/views.py index 73790b9ad..3ed5c6bdd 100644 --- a/server/openslides/assignments/views.py +++ b/server/openslides/assignments/views.py @@ -126,46 +126,29 @@ class AssignmentViewSet(ModelViewSet): assignment.remove_candidate(request.user) return "You have withdrawn your candidature successfully." - def get_user_from_request_data(self, request): - """ - Helper method to get a specific user from request data (not the - request.user) so that the view self.candidature_other can play with it. - """ - if not isinstance(request.data, dict): - raise ValidationError( - { - "detail": "Invalid data. Expected dictionary, got {0}.", - "args": [type(request.data)], - } - ) - user_str = request.data.get("user", "") - try: - user_pk = int(user_str) - except ValueError: - raise ValidationError( - {"detail": 'Invalid data. Expected something like {"user": }.'} - ) - try: - user = get_user_model().objects.get(pk=user_pk) - except get_user_model().DoesNotExist: - raise ValidationError( - {"detail": "Invalid data. User {0} does not exist.", "args": [user_pk]} - ) - return user - @detail_route(methods=["post", "delete"]) def candidature_other(self, request, pk=None): """ View to nominate other users (POST) or delete their candidature status (DELETE). The client has to send {'user': }. """ - user = self.get_user_from_request_data(request) + user_id = request.data.get("user") + if not isinstance(user_id, int): + raise ValidationError({"detail": "user_id must be an int."}) + + try: + user = get_user_model().objects.get(pk=user_id) + except get_user_model().DoesNotExist: + raise ValidationError( + {"detail": "Invalid data. User {0} does not exist.", "args": [user_id]} + ) + assignment = self.get_object() if request.method == "POST": return self.nominate_other(request, user, assignment) else: # request.method == 'DELETE' - return self.delete_other(request, user, assignment) + return self.withdraw_other(request, user, assignment) def nominate_other(self, request, user, assignment): if assignment.phase == assignment.PHASE_FINISHED: @@ -191,7 +174,7 @@ class AssignmentViewSet(ModelViewSet): {"detail": "User {0} was nominated successfully.", "args": [str(user)]} ) - def delete_other(self, request, user, assignment): + def withdraw_other(self, request, user, assignment): # To delete candidature status you have to be a manager. if not has_perm(request.user, "assignments.can_manage"): self.permission_denied(request) diff --git a/server/tests/integration/agenda/test_viewset.py b/server/tests/integration/agenda/test_viewset.py index a6c457131..ab30b4cdd 100644 --- a/server/tests/integration/agenda/test_viewset.py +++ b/server/tests/integration/agenda/test_viewset.py @@ -356,6 +356,13 @@ class ManageSpeaker(TestCase): ).exists() ) + def test_add_someone_else_no_id(self): + response = self.client.post( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"user": ["not valid"]}, + ) + self.assertEqual(response.status_code, 400) + def test_point_of_order_single(self): config["agenda_enable_point_of_order_speakers"] = True self.assertEqual(Speaker.objects.all().count(), 0) diff --git a/server/tests/integration/assignments/test_viewset.py b/server/tests/integration/assignments/test_viewset.py index 71579a7a1..2b76114e3 100644 --- a/server/tests/integration/assignments/test_viewset.py +++ b/server/tests/integration/assignments/test_viewset.py @@ -279,6 +279,14 @@ class CandidatureOther(TestCase): .exists() ) + def test_nominate_other_invalid_id(self): + response = self.client.post( + reverse("assignment-candidature-other", args=[self.assignment.pk]), + {"user": ["not valid"]}, + ) + + self.assertEqual(response.status_code, 400) + def test_nominate_other_twice(self): self.assignment.add_candidate( get_user_model().objects.get(username="test_user_eeheekai4Phue6cahtho") diff --git a/server/tests/unit/agenda/test_views.py b/server/tests/unit/agenda/test_views.py index df182bdcc..8f3bb8310 100644 --- a/server/tests/unit/agenda/test_views.py +++ b/server/tests/unit/agenda/test_views.py @@ -46,7 +46,7 @@ class ListOfSpeakersViewSetManageSpeaker(TestCase): self.request.method = "POST" self.request.user = 1 self.request.data = { - "user": "2" + "user": 2 } # It is assumed that the request user has pk!=2. mock_get_user_model.return_value = MockUser = MagicMock() MockUser.objects.get.return_value = mock_user = MagicMock()