Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions src/apps/api/tests/test_submissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ def setUp(self):
leaderboard=None
)

# add submission with that is on the leaderboard
# add submission with that is on the leaderboard (leaderboard submissions should always be finished)
self.leaderboard_submission = SubmissionFactory(
phase=self.phase,
owner=self.participant,
status=Submission.SUBMITTED,
status=Submission.FINISHED,
leaderboard=self.leaderboard
)

Expand Down Expand Up @@ -260,6 +260,24 @@ def test_who_can_see_detailed_result_when_visualization_is_true_and_competition_
resp = self.client.get(url)
assert resp.status_code == 200

def test_anonymous_cannot_list_or_retrieve_submissions(self):
"""
SubmissionViewSet's general list/retrieve endpoints must not leak submission
data to anonymous users, even for a finished submission on a leaderboard.
Public leaderboard data is meant to be served only through
PhaseViewSet.get_leaderboard, which uses a restricted serializer.
"""
# List: the leaderboard submission must not appear
resp = self.client.get(reverse('submission-list'))
assert resp.status_code == 200
results = resp.data.get('results', resp.data)
assert all(item['id'] != self.leaderboard_submission.pk for item in results)

# Retrieve: must 404, not leak the record
url = reverse('submission-detail', args=(self.leaderboard_submission.pk,))
resp = self.client.get(url)
assert resp.status_code == 404


class SubmissionGetDetailsAPITests(APITestCase):
def setUp(self):
Expand Down Expand Up @@ -373,23 +391,19 @@ def test_hidden_details_actually_stops_submission_creator_from_seeing_output(sel

def test_anonymous_cannot_get_details_of_finished_leaderboard_submission(self):
"""
Unlike the two tests above, uses a finished submission that IS on a leaderboard,
so it's reachable by anonymous users; expect it to still be denied to them.

SubmissionViewSet.get_queryset() in src/apps/api/views/submissions.py has a branch
for unauthenticated GET requests that filters on leaderboard__isnull=False (plus
status=FINISHED and is_soft_deleted=False). That's what lets an anonymous request
find this submission via get_details' super().get_object() call in the first place --
so this test's leaderboard_submission (leaderboard set, status=FINISHED in setUp)
is what actually exercises that branch, unlike existing_submission (leaderboard=None)
used above, which fails the leaderboard__isnull=False filter and 404s before
get_details' owner/admin check is ever reached.
Unlike the two tests above, uses a finished submission that IS on a leaderboard.
Being on a leaderboard must not make submission details reachable by anonymous users.

SubmissionViewSet.get_queryset() in src/apps/api/views/submissions.py returns an
empty queryset for unauthenticated GET requests, so get_details' super().get_object()
never finds the submission and we get a 404 (rather than a 403 confirming it exists).
Public leaderboard data is served separately by PhaseViewSet.get_leaderboard.
"""
url = reverse('submission-get-details', args=(self.leaderboard_submission.pk,))

# Anonymous: object is found via the leaderboard queryset, but must still be denied
# Anonymous: filtered out at the queryset level, existence is not leaked
resp = self.client.get(url)
assert resp.status_code == 403
assert resp.status_code == 404

# Non-owner, non-admin authenticated user: filtered out at the queryset level
self.client.force_login(self.other_user)
Expand Down
33 changes: 7 additions & 26 deletions src/apps/api/views/submissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,28 +114,12 @@ def get_queryset(self):
qs = super().get_queryset()
if self.request.method == 'GET':
if not self.request.user.is_authenticated:
# Show leaderboard submissions to unauthenticated users
return (
qs.filter(
leaderboard__isnull=False,
is_soft_deleted=False,
status=Submission.FINISHED,
)
.select_related(
'phase',
'phase__competition',
'participant',
'participant__user',
'owner',
'data',
)
.prefetch_related(
'children',
'scores',
'scores__column',
'task',
)
)
# Anonymous users get nothing here. This endpoint returns full
# submission records (filenames, status details, fact sheet
# answers, internal ids, ...); the public leaderboard view is
# served separately by PhaseViewSet.get_leaderboard, which uses
# a restricted serializer.
return qs.none()

# Check if admin is requesting to see soft-deleted submissions
show_is_soft_deleted = self.request.query_params.get('show_is_soft_deleted', 'false').lower() == 'true'
Expand Down Expand Up @@ -177,12 +161,9 @@ def get_queryset(self):
Q(phase__competition__created_by=self.request.user) |
Q(phase__competition__collaborators__in=[self.request.user.pk])
) is not qs:
ValidationError("Request Contained Submissions you don't have authorization for")
raise ValidationError("Request Contained Submissions you don't have authorization for")
if self.action in ['re_run_many_submissions']:
print(f'debug {qs}')
print(f'debug {qs.first().status}')
qs = qs.filter(status__in=[Submission.FINISHED, Submission.FAILED, Submission.CANCELLED])
print(f'debug {qs}')
return qs

def create(self, request, *args, **kwargs):
Expand Down