Merge pull request #1900 from codalab/submission_deletion

Submission delete API bug fixed. More restrictions added

Author: ObadaS

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,34 +105,47 @@ 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)
         resp = self.client.delete(url)
         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,))
+
+        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,))
 

src/apps/api/views/submissions.py

@@ -198,11 +198,25 @@ def create(self, request, *args, **kwargs):
         return super(SubmissionViewSet, self).create(request, *args, **kwargs)
 
     def destroy(self, request, *args, **kwargs):
+        """
+        - 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()
 
-        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 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
         self.perform_destroy(submission)
         return Response(status=status.HTTP_204_NO_CONTENT)