Skip to content

Commit 81bc0d3

Browse files
committed
PM-4026: allow challenge-wide roles to view challenge payments
What was broken: Challenge payment lookups in the review flow were forced to winner-only results for non-privileged users, so challenge managers reviewing completed challenges could see "No payments found" even when payments existed. Root cause: Challenge-wide access expansion only checked copilot roles. Manager/full-write challenge resource roles were not treated as challenge-wide readers. What was changed: Updated challenge-payments access checks to allow full challenge visibility for users with challenge-wide roles (fullWriteAccess roles plus manager/copilot role names). Kept winner-based filtering for users without challenge-wide access. Hardened role claim parsing to only stringify primitive claim values. Extended the challenge resource-role model with optional fullWriteAccess for this access check. Any added/updated tests: Added unit tests for ChallengePaymentsService to verify manager access returns all challenge payments and non-challenge-wide users remain winner-filtered.
1 parent 54a1e4e commit 81bc0d3

3 files changed

Lines changed: 127 additions & 13 deletions

File tree

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
jest.mock('src/shared/global', () => ({
2+
Logger: class {
3+
debug = jest.fn();
4+
5+
error = jest.fn();
6+
7+
info = jest.fn();
8+
9+
log = jest.fn();
10+
11+
warn = jest.fn();
12+
},
13+
}));
14+
15+
jest.mock('src/config', () => ({
16+
ENV_CONFIG: {
17+
TOPCODER_API_V6_BASE_URL: 'https://api.topcoder-dev.com/v6',
18+
},
19+
}));
20+
21+
import { ChallengePaymentsService } from './challenge-payments.service';
22+
23+
describe('ChallengePaymentsService', () => {
24+
let findManyMock: jest.Mock;
25+
let m2mFetchMock: jest.Mock;
26+
let service: ChallengePaymentsService;
27+
28+
beforeEach(() => {
29+
findManyMock = jest.fn().mockResolvedValue([]);
30+
m2mFetchMock = jest.fn();
31+
32+
service = new ChallengePaymentsService(
33+
{
34+
winnings: {
35+
findMany: findManyMock,
36+
},
37+
} as any,
38+
{
39+
m2mFetch: m2mFetchMock,
40+
} as any,
41+
);
42+
});
43+
44+
it('returns all challenge payments for managers', async () => {
45+
m2mFetchMock
46+
.mockResolvedValueOnce([
47+
{
48+
memberHandle: 'manager-user',
49+
memberId: '123',
50+
roleId: 'manager-role-id',
51+
},
52+
])
53+
.mockResolvedValueOnce([
54+
{
55+
fullWriteAccess: false,
56+
id: 'manager-role-id',
57+
name: 'Manager',
58+
},
59+
]);
60+
61+
await service.listChallengePayments({
62+
auth0User: { roles: ['Topcoder User'] },
63+
challengeId: 'challenge-id',
64+
isMachineToken: false,
65+
requestUserId: '123',
66+
winnerOnly: false,
67+
});
68+
69+
const where = findManyMock.mock.calls[0][0].where;
70+
expect(where.winner_id).toBeUndefined();
71+
});
72+
73+
it('keeps winner filtering for users without challenge-wide access', async () => {
74+
m2mFetchMock
75+
.mockResolvedValueOnce([
76+
{
77+
memberHandle: 'reviewer-user',
78+
memberId: '456',
79+
roleId: 'reviewer-role-id',
80+
},
81+
])
82+
.mockResolvedValueOnce([
83+
{
84+
fullWriteAccess: false,
85+
id: 'reviewer-role-id',
86+
name: 'Reviewer',
87+
},
88+
]);
89+
90+
await service.listChallengePayments({
91+
auth0User: { roles: ['Topcoder User'] },
92+
challengeId: 'challenge-id',
93+
isMachineToken: false,
94+
requestUserId: '456',
95+
winnerOnly: false,
96+
});
97+
98+
const where = findManyMock.mock.calls[0][0].where;
99+
expect(where.winner_id).toBe('456');
100+
});
101+
});

src/api/challenge-payments/challenge-payments.service.ts

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,17 @@ export class ChallengePaymentsService {
101101
}
102102

103103
try {
104-
const isCopilot = await this.isCopilotForChallenge(
105-
challengeId,
106-
requestUserId,
107-
);
108-
if (isCopilot) {
104+
const hasChallengeWideAccess =
105+
await this.hasChallengeWideAccessForChallenge(
106+
challengeId,
107+
requestUserId,
108+
);
109+
if (hasChallengeWideAccess) {
109110
allowAllForChallenge = true;
110111
}
111112
} catch (error) {
112113
this.logger.warn(
113-
`Failed to verify copilot status for user ${requestUserId} on challenge ${challengeId}`,
114+
`Failed to verify challenge-wide payment access for user ${requestUserId} on challenge ${challengeId}`,
114115
error instanceof Error ? error.message : error,
115116
);
116117
}
@@ -150,7 +151,11 @@ export class ChallengePaymentsService {
150151
roles.push(String(role));
151152
}
152153
});
153-
} else if (value) {
154+
} else if (
155+
typeof value === 'string' ||
156+
typeof value === 'number' ||
157+
typeof value === 'boolean'
158+
) {
154159
roles.push(String(value));
155160
}
156161
});
@@ -165,7 +170,7 @@ export class ChallengePaymentsService {
165170
return roles;
166171
}
167172

168-
private async isCopilotForChallenge(
173+
private async hasChallengeWideAccessForChallenge(
169174
challengeId: string,
170175
userId: string,
171176
): Promise<boolean> {
@@ -181,19 +186,26 @@ export class ChallengePaymentsService {
181186
return false;
182187
}
183188

184-
const copilotRoleIds = new Set(
189+
const challengeWideRoleIds = new Set(
185190
resourceRoles
186-
?.filter((role) =>
187-
role?.name ? role.name.toLowerCase().includes('copilot') : false,
191+
?.filter(
192+
(role) =>
193+
role?.fullWriteAccess === true ||
194+
(role?.name
195+
? role.name.toLowerCase().includes('copilot') ||
196+
role.name.toLowerCase().includes('manager')
197+
: false),
188198
)
189199
.map((role) => role.id),
190200
);
191201

192-
if (copilotRoleIds.size === 0) {
202+
if (challengeWideRoleIds.size === 0) {
193203
return false;
194204
}
195205

196-
return resources.some((resource) => copilotRoleIds.has(resource.roleId));
206+
return resources.some((resource) =>
207+
challengeWideRoleIds.has(resource.roleId),
208+
);
197209
}
198210

199211
private async fetchWinnings(

src/api/challenges/models/challenge.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ export interface ChallengeResource {
148148
export interface ResourceRole {
149149
id: string;
150150
name: string;
151+
fullWriteAccess?: boolean;
151152
}
152153

153154
export interface ChallengeReview {

0 commit comments

Comments
 (0)