From cffb66c8cc34eb4fdd6cbef07c06244eb839a998 Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Sat, 28 Jun 2025 22:04:35 +0500 Subject: [PATCH 1/3] Submission delete API bug fixed. More restrictions added --- src/apps/api/views/submissions.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/apps/api/views/submissions.py b/src/apps/api/views/submissions.py index afe14fb36..cca130321 100644 --- a/src/apps/api/views/submissions.py +++ b/src/apps/api/views/submissions.py @@ -179,11 +179,27 @@ def create(self, request, *args, **kwargs): return super(SubmissionViewSet, self).create(request, *args, **kwargs) def destroy(self, request, *args, **kwargs): + """ + - If a user is owner of a submission and submission is not on the leaderboard, user can delete the submission using the delete API + - If a user is either super user or admin of the competition of the submission, user can delete the submission + - If user is neither owner nor admin, user cannot delete the submission + """ submission = self.get_object() - if request.user != submission.owner and not self.has_admin_permission(request.user, submission): - raise PermissionDenied("Cannot interact with submission you did not make") + is_owner = request.user == submission.owner + is_super_user_or_competition_admin = self.has_admin_permission(request.user, submission) + + # If user is neither owner nor super user/admin + # return permission denied + if not is_owner and not is_super_user_or_competition_admin: + raise PermissionDenied("You do not have permission to delete this submission!") + + # If user is owner but submission is on the leaderboard + # return permission denied + if is_owner and submission.leaderboard: + raise PermissionDenied("You cannot delete a leaderboard submission!") + # Otherwise, delete the submission self.perform_destroy(submission) return Response(status=status.HTTP_204_NO_CONTENT) From 17e980defc31ce2a4f130789640ae8a1c5e7b75f Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Sun, 29 Jun 2025 14:04:47 +0500 Subject: [PATCH 2/3] tests updated and fixed --- src/apps/api/tests/test_submissions.py | 35 ++++++++++++++++++-------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/apps/api/tests/test_submissions.py b/src/apps/api/tests/test_submissions.py index fb5b3966f..b9ae428ab 100644 --- a/src/apps/api/tests/test_submissions.py +++ b/src/apps/api/tests/test_submissions.py @@ -22,6 +22,7 @@ def setUp(self): self.collaborator = UserFactory(username='collab', password='collab') self.comp = CompetitionFactory(created_by=self.creator, collaborators=[self.collaborator]) self.phase = PhaseFactory(competition=self.comp) + self.leaderboard = LeaderboardFactory() # Extra dummy user to test permissions, they shouldn't have access to many things self.other_user = UserFactory(username='other_user', password='other') @@ -41,7 +42,16 @@ def setUp(self): phase=self.phase, owner=self.participant, status=Submission.SUBMITTED, - secret='7df3600c-1234-5678-bbc8-bbe91f42d875' + secret='7df3600c-1234-5678-bbc8-bbe91f42d875', + leaderboard=None + ) + + # add submission with that is on the leaderboard + self.leaderboard_submission = SubmissionFactory( + phase=self.phase, + owner=self.participant, + status=Submission.SUBMITTED, + leaderboard=self.leaderboard ) def test_can_make_submission_checks_if_you_are_participant(self): @@ -95,27 +105,23 @@ def test_cannot_delete_submission_you_didnt_create(self): # As anonymous user resp = self.client.delete(url) assert resp.status_code == 403 - assert resp.data["detail"] == "Cannot interact with submission you did not make" + assert resp.data["detail"] == "You do not have permission to delete this submission!" # As regular user self.client.force_login(self.other_user) resp = self.client.delete(url) assert resp.status_code == 403 - assert resp.data["detail"] == "Cannot interact with submission you did not make" + assert resp.data["detail"] == "You do not have permission to delete this submission!" + def test_can_delete_submission_you_created(self): + url = reverse('submission-detail', args=(self.existing_submission.pk,)) # As user who made submission self.client.force_login(self.participant) resp = self.client.delete(url) assert resp.status_code == 204 assert not Submission.objects.filter(pk=self.existing_submission.pk).exists() - # As superuser (re-making submission since it has been destroyed) - self.existing_submission = SubmissionFactory( - phase=self.phase, - owner=self.participant, - status=Submission.SUBMITTED, - secret='7df3600c-1234-5678-90c8-bbe91f42d875' - ) + def test_super_user_can_delete_submission_you_created(self): url = reverse('submission-detail', args=(self.existing_submission.pk,)) self.client.force_login(self.superuser) @@ -123,6 +129,15 @@ def test_cannot_delete_submission_you_didnt_create(self): assert resp.status_code == 204 assert not Submission.objects.filter(pk=self.existing_submission.pk).exists() + def test_cannot_delete_leaderboard_submission_you_created(self): + url = reverse('submission-detail', args=(self.leaderboard_submission.pk,)) + + self.client.force_login(self.participant) + resp = self.client.delete(url) + + assert resp.status_code == 403 + assert resp.data["detail"] == "You cannot delete a leaderboard submission!" + def test_cannot_get_details_of_submission_unless_creator_collab_or_superuser(self): url = reverse('submission-get-details', args=(self.existing_submission.pk,)) From 317c77ae66c5556e5839ac1430b74469016fd7fd Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Mon, 30 Jun 2025 08:29:26 +0500 Subject: [PATCH 3/3] missing condition added, new test added for missing condition --- src/apps/api/tests/test_submissions.py | 8 ++++++++ src/apps/api/views/submissions.py | 12 +++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/apps/api/tests/test_submissions.py b/src/apps/api/tests/test_submissions.py index b9ae428ab..f33a4e83b 100644 --- a/src/apps/api/tests/test_submissions.py +++ b/src/apps/api/tests/test_submissions.py @@ -129,6 +129,14 @@ def test_super_user_can_delete_submission_you_created(self): assert resp.status_code == 204 assert not Submission.objects.filter(pk=self.existing_submission.pk).exists() + def test_super_user_can_delete_leaderboard_submission_you_created(self): + url = reverse('submission-detail', args=(self.leaderboard_submission.pk,)) + + self.client.force_login(self.superuser) + resp = self.client.delete(url) + assert resp.status_code == 204 + assert not Submission.objects.filter(pk=self.leaderboard_submission.pk).exists() + def test_cannot_delete_leaderboard_submission_you_created(self): url = reverse('submission-detail', args=(self.leaderboard_submission.pk,)) diff --git a/src/apps/api/views/submissions.py b/src/apps/api/views/submissions.py index cca130321..4bfd7b508 100644 --- a/src/apps/api/views/submissions.py +++ b/src/apps/api/views/submissions.py @@ -180,23 +180,21 @@ def create(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): """ - - If a user is owner of a submission and submission is not on the leaderboard, user can delete the submission using the delete API - - If a user is either super user or admin of the competition of the submission, user can delete the submission - If user is neither owner nor admin, user cannot delete the submission + - If a user is not admin and is owner of a submission and submission is on the leaderboard, user cannot delete the submission + - In rest of the cases i.e. user is admin/super user or user is owner of the submisison and submission is not on the leaderboard, user can delete the submisison """ submission = self.get_object() is_owner = request.user == submission.owner is_super_user_or_competition_admin = self.has_admin_permission(request.user, submission) - # If user is neither owner nor super user/admin - # return permission denied + # If user is neither owner nor super user/admin return permission denied if not is_owner and not is_super_user_or_competition_admin: raise PermissionDenied("You do not have permission to delete this submission!") - # If user is owner but submission is on the leaderboard - # return permission denied - if is_owner and submission.leaderboard: + # If user is not admin, is owner and submission is on the leaderboard return permission denied + if not is_super_user_or_competition_admin and is_owner and submission.leaderboard: raise PermissionDenied("You cannot delete a leaderboard submission!") # Otherwise, delete the submission