Skip to content

Commit 44476de

Browse files
committed
PM-5747: require passing Design F2F submissions for winner downloads
What was broken A registered submitter whose Design First2Finish submission failed iterative review could download the winning submission when all-registrant winner downloads were disabled. Root cause The legacy First2Finish authorization branch treated owning any submission as sufficient and returned before the passing-submission eligibility check, including for Design challenges. What was changed Limited the legacy any-submission First2Finish eligibility rule to non-Design challenges. Design First2Finish submitters now require a passing submission unless all-registrant winner downloads are enabled. Updated the endpoint documentation and service JSDoc to describe the narrowed rule. Any added/updated tests Added a regression test that models a Design First2Finish member with an owned submission but no passing review summation and verifies that the winning submission download is denied before storage access. The focused download authorization suite, lint, and build pass. The full test suite still reports eight unrelated failures that reproduce unchanged on origin/develop.
1 parent cb6f78e commit 44476de

3 files changed

Lines changed: 57 additions & 6 deletions

File tree

src/api/submission/submission.controller.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ export class SubmissionController {
405405
@ApiOperation({
406406
summary: 'Download the submission',
407407
description:
408-
'Roles: Copilot, Admin, User, Reviewer. After challenge completion, the exact metadata value allowAllRegistrantsToDownloadWinningSubmissions=true lets every registered Submitter download only an exact final winning submission and denies non-winners without legacy fallback. Other values retain legacy passing or First2Finish eligibility. Design challenges also require submissionsViewable. | Scopes: read:submission',
408+
'Roles: Copilot, Admin, User, Reviewer. After challenge completion, the exact metadata value allowAllRegistrantsToDownloadWinningSubmissions=true lets every registered Submitter download only an exact final winning submission and denies non-winners without legacy fallback. Other values require passing-submission eligibility, except non-Design First2Finish challenges retain legacy submitter eligibility. Design challenges also require submissionsViewable. | Scopes: read:submission',
409409
})
410410
@ApiParam({
411411
name: 'submissionId',

src/api/submission/submission.service.spec.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,6 +1421,52 @@ describe('SubmissionService', () => {
14211421
expect(s3Send).not.toHaveBeenCalled();
14221422
});
14231423

1424+
it('denies a failed Design First2Finish submitter when the new flag is disabled', async () => {
1425+
resourceApiService.getMemberResourcesRoles.mockResolvedValue([
1426+
{ roleName: 'Submitter' },
1427+
]);
1428+
challengeApiServiceMock.getChallengeDetail.mockResolvedValue({
1429+
status: ChallengeStatus.COMPLETED,
1430+
type: 'First2Finish',
1431+
track: 'Design',
1432+
metadata: {
1433+
submissionsViewable: 'true',
1434+
},
1435+
winners: [{ userId: 'owner-user', placement: 1 }],
1436+
});
1437+
prismaMock.submission.findFirst.mockImplementation(({ where }) =>
1438+
Promise.resolve(
1439+
where.reviewSummation ? null : { id: 'failed-own-submission' },
1440+
),
1441+
);
1442+
1443+
await expect(
1444+
service.getSubmissionFileStream(
1445+
{
1446+
userId: 'failed-first2finish-submitter',
1447+
isMachine: false,
1448+
roles: [],
1449+
} as any,
1450+
'sub-123',
1451+
),
1452+
).rejects.toBeInstanceOf(ForbiddenException);
1453+
1454+
expect(prismaMock.submission.findFirst).toHaveBeenCalledTimes(1);
1455+
expect(prismaMock.submission.findFirst).toHaveBeenCalledWith({
1456+
where: {
1457+
challengeId: 'challenge-xyz',
1458+
memberId: 'failed-first2finish-submitter',
1459+
reviewSummation: {
1460+
some: {
1461+
isPassing: true,
1462+
},
1463+
},
1464+
},
1465+
select: { id: true },
1466+
});
1467+
expect(s3Send).not.toHaveBeenCalled();
1468+
});
1469+
14241470
it('preserves manager access when Design submissions are not viewable', async () => {
14251471
resourceApiService.getMemberResourcesRoles.mockResolvedValue([
14261472
{ roleName: 'Manager' },

src/api/submission/submission.service.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,8 +1286,9 @@ export class SubmissionService {
12861286
* eligible reviewers, copilots, or managers.
12871287
* - When `allowAllRegistrantsToDownloadWinningSubmissions` is exactly
12881288
* `"true"`, every registered Submitter may download only an exact final
1289-
* winning submission after completion. Other metadata values retain legacy
1290-
* passing or First2Finish eligibility. Design challenges additionally
1289+
* winning submission after completion. Other metadata values require
1290+
* passing-submission eligibility, except non-Design First2Finish challenges
1291+
* retain legacy submitter eligibility. Design challenges additionally
12911292
* require `submissionsViewable` to be true.
12921293
*
12931294
* The file is always fetched from the configured clean bucket, never from DMZ.
@@ -1509,8 +1510,9 @@ export class SubmissionService {
15091510
* passes, exact `"true"` makes the requested target decisive: it must be the
15101511
* exact canonical result for a recorded placement winner (or carry matching
15111512
* legacy placement data), and a non-winner fails closed without legacy
1512-
* fallback. Other metadata values retain the legacy First2Finish or
1513-
* passing-submission eligibility behavior.
1513+
* fallback. Other metadata values require passing-submission eligibility,
1514+
* except non-Design First2Finish challenges retain legacy submitter
1515+
* eligibility.
15141516
*
15151517
* @param challengeId - Challenge containing the requested submission.
15161518
* @param requesterMemberId - Registered Submitter requesting the download.
@@ -1544,7 +1546,10 @@ export class SubmissionService {
15441546
return this.isWinningSubmission(challengeId, challenge, submission);
15451547
}
15461548

1547-
if (this.isFirst2FinishChallenge(challenge)) {
1549+
if (
1550+
this.isFirst2FinishChallenge(challenge) &&
1551+
!this.isDesignChallenge(challenge)
1552+
) {
15481553
const memberSubmission = await this.prisma.submission.findFirst({
15491554
where: {
15501555
challengeId,

0 commit comments

Comments
 (0)