diff --git a/src/common/project-helper.js b/src/common/project-helper.js index a2d5cec..0be5e9d 100644 --- a/src/common/project-helper.js +++ b/src/common/project-helper.js @@ -9,16 +9,15 @@ const errors = require("./errors"); const logger = require("./logger"); /** - * Normalizes billing-account markup to the decimal format persisted on + * Normalizes billing-account markup to the multiplier format persisted on * challenges. * - * Legacy project-service responses can return whole percentage points (for - * example `50` for 50%), while newer billing-account records use decimal - * fractions (for example `0.58` for 58%). This helper preserves modern values - * and converts only legacy percentages. + * Billing-account records use the direct multiplier consumed by challenge, + * finance, and billing-account ledger math. Values greater than `1` are valid + * and must not be converted to percentage form. * * @param {unknown} rawMarkup Markup value returned by Projects API. - * @returns {number|null} Decimal markup or `null` when the input is empty or invalid. + * @returns {number|null} Billing markup multiplier or `null` when the input is empty or invalid. */ function normalizeBillingMarkup(rawMarkup) { if (_.isNil(rawMarkup) || rawMarkup === "") { @@ -30,7 +29,7 @@ function normalizeBillingMarkup(rawMarkup) { return null; } - return markup > 1 ? markup / 100 : markup; + return markup; } /** @@ -261,7 +260,7 @@ class ProjectHelper { * @param {string|number} params.billingAccountId Billing-account identifier. * @param {string} params.challengeId Challenge id to use as the external reference. * @param {number|string} params.memberPaymentAmount Challenge member-payment amount before markup. - * @param {number|string|null|undefined} params.markup Billing-account markup as a decimal or percentage. + * @param {number|string|null|undefined} params.markup Billing-account markup multiplier. * @returns {Promise} Billing Accounts API lock response. * @throws {errors.BadRequestError} When required values are missing or invalid. * @throws {Error} When the Billing Accounts API rejects the lock request. diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 2bb4b00..074404e 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -1956,7 +1956,6 @@ async function searchChallenges(currentUser, criteria) { const requestedMemberId = !_.isNil(criteria.memberId) ? _.toString(criteria.memberId) : null; const currentUserMemberId = currentUser && !_hasAdminRole && !_isMachineToken ? _.toString(currentUser.userId) : null; - const memberIdForTaskFilter = requestedMemberId || currentUserMemberId; const isSelfMemberSearch = Boolean( requestedMemberId && currentUserMemberId && requestedMemberId === currentUserMemberId, ); @@ -2078,27 +2077,11 @@ async function searchChallenges(currentUser, criteria) { }); } - // FIXME: Tech Debt - let excludeTasks = true; - if (requestedMemberId) { - // When we already restrict the result set to a specific member, - // rerunning the generic task visibility filter is redundant. - excludeTasks = false; - } else if ( - currentUser && - (_hasAdminRole || _isMachineToken || hasProjectManagerAccessForSearch) - ) { - // if you're an admin or m2m, security rules wont be applied - excludeTasks = false; - } - /** - * For non-authenticated users: - * - Only unassigned tasks will be returned - * For authenticated users (non-admin): - * - Only unassigned tasks and tasks assigned to the logged in user will be returned - * For admins/m2m: - * - All tasks will be returned + * Task challenges are visible to ordinary authenticated users only when they + * have a resource on the task. Anonymous users never receive tasks. Admin, + * machine-token and project-manager searches retain their operational access. + * A requested memberId narrows results but never grants the caller task access. */ if (currentUser && (_hasAdminRole || _isMachineToken)) { // For admins/m2m, allow filtering based on task properties @@ -2117,28 +2100,16 @@ async function searchChallenges(currentUser, criteria) { taskMemberId: criteria.taskMemberId, }); } - } else if (excludeTasks) { - const taskFilter = []; - if (memberIdForTaskFilter) { + } else if (!hasProjectManagerAccessForSearch) { + const taskFilter = [{ taskIsTask: false }]; + if (currentUserMemberId) { taskFilter.push({ + taskIsTask: true, memberAccesses: { - some: { memberId: memberIdForTaskFilter }, + some: { memberId: currentUserMemberId }, }, }); } - taskFilter.push({ - taskIsTask: false, - }); - taskFilter.push({ - taskIsTask: true, - taskIsAssigned: false, - taskMemberId: null, - }); - if (currentUser && !_hasAdminRole && !_isMachineToken) { - taskFilter.push({ - taskMemberId: currentUser.userId, - }); - } prismaFilter.where.AND.push({ OR: taskFilter, }); diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index 6c5a47f..aa2a1ad 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -1006,6 +1006,71 @@ describe("challenge service unit tests", () => { should.equal(result.numOfRegistrants, 0); }); + it("search challenges hides unassigned tasks from anonymous users", async () => { + await prisma.challenge.update({ + where: { id: data.taskChallenge.id }, + data: { + taskIsAssigned: false, + taskMemberId: null, + }, + }); + + try { + const result = await service.searchChallenges(undefined, { + id: data.taskChallenge.id, + }); + + should.equal(result.total, 0); + should.equal(result.result.length, 0); + } finally { + await prisma.challenge.update({ + where: { id: data.taskChallenge.id }, + data: { taskIsAssigned: true }, + }); + } + }); + + it("search challenges uses the caller resource association for task visibility", async () => { + const originalGetCompleteUserGroupTreeIds = helper.getCompleteUserGroupTreeIds; + const originalMemberChallengeAccessFindMany = prisma.memberChallengeAccess.findMany; + const originalChallengeFindMany = prisma.challenge.findMany; + let capturedWhere; + + helper.getCompleteUserGroupTreeIds = async () => []; + prisma.memberChallengeAccess.findMany = async () => [{ challengeId: data.taskChallenge.id }]; + prisma.challenge.findMany = async (query) => { + capturedWhere = query.where; + return []; + }; + + try { + const result = await service.searchChallenges( + { roles: ["Topcoder User"], userId: "caller-resource-id" }, + { memberId: "different-member-id" }, + ); + + should.equal(result.total, 0); + const taskVisibilityFilter = capturedWhere.AND.find( + (filter) => filter.OR && filter.OR.some((condition) => condition.taskIsTask === false), + ); + taskVisibilityFilter.should.deep.equal({ + OR: [ + { taskIsTask: false }, + { + taskIsTask: true, + memberAccesses: { + some: { memberId: "caller-resource-id" }, + }, + }, + ], + }); + } finally { + helper.getCompleteUserGroupTreeIds = originalGetCompleteUserGroupTreeIds; + prisma.memberChallengeAccess.findMany = originalMemberChallengeAccessFindMany; + prisma.challenge.findMany = originalChallengeFindMany; + } + }); + it("search challenges sorts status alphabetically for member and non-member searches", async () => { const statusChallenges = [ { diff --git a/test/unit/project-helper.test.js b/test/unit/project-helper.test.js index 7be1a10..3fbe9a9 100644 --- a/test/unit/project-helper.test.js +++ b/test/unit/project-helper.test.js @@ -41,13 +41,13 @@ describe('project helper unit tests', () => { }) }) - it('converts legacy whole-percentage billing markup to decimal format', async () => { + it('preserves billing markup multipliers greater than one', async () => { m2mHelper.getM2MToken = async () => 'test-token' axios.get = async () => ({ status: 200, data: { tcBillingAccountId: '80004217', - markup: 58 + markup: 1.2229 } }) @@ -55,7 +55,7 @@ describe('project helper unit tests', () => { result.should.deep.equal({ billingAccountId: '80004217', - markup: 0.58 + markup: 1.2229 }) }) @@ -81,13 +81,13 @@ describe('project helper unit tests', () => { const result = await projectHelper.lockChallengeBillingAccountAmount({ billingAccountId: '80001012', challengeId: 'challenge-id', - memberPaymentAmount: 1000, - markup: 0.1 + memberPaymentAmount: 394, + markup: 1.2229 }) patchUrl.should.equal('http://localhost:4000/v6/billing-accounts/80001012/lock-amount') patchBody.should.deep.equal({ - amount: 1100, + amount: 875.8226, challengeId: 'challenge-id', externalId: 'challenge-id', externalType: 'CHALLENGE' @@ -95,7 +95,7 @@ describe('project helper unit tests', () => { patchHeaders.should.deep.equal({ Authorization: 'Bearer test-token' }) result.should.deep.equal({ externalId: 'challenge-id', - amount: 1100 + amount: 875.8226 }) }) })