diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index bb9237ffd843..826429fce986 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -5127,13 +5127,21 @@ function canEditMoneyRequest( } // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 const reportPolicy = policy ?? getPolicy(moneyRequestReport?.policyID); - const isAdmin = reportPolicy?.role === CONST.POLICY.ROLE.ADMIN; - const isManager = deprecatedCurrentUserAccountID === moneyRequestReport?.managerID; + const isManagerOfReport = deprecatedCurrentUserAccountID === moneyRequestReport?.managerID; - if (isInvoiceReport(moneyRequestReport) && (isManager || isChatReportArchived)) { + if (isInvoiceReport(moneyRequestReport) && (isManagerOfReport || isChatReportArchived)) { return false; } + // Admin/manager rights only apply when the expense actually sits on a workspace report. Without this guard an + // unreported expense (self-DM track expense) is weighed against the caller's policy, which is the viewer's own + // default workspace rather than the expense's, so anyone who admins any workspace could edit someone else's + // expense. A self-DM also has no managerID, so an unresolved account ID would otherwise match it. Invoice reports + // stay included so a policy admin keeps the rights they had before this guard existed. + const isReportOnAWorkspace = isFinancialReportsForBusinesses(moneyRequestReport); + const isAdmin = isReportOnAWorkspace && reportPolicy?.role === CONST.POLICY.ROLE.ADMIN; + const isManager = isReportOnAWorkspace && isManagerOfReport; + // Admin & managers can always edit coding fields such as tag, category, billable, etc. if (isAdmin || isManager) { return true; diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 31f041489644..c1695e403a96 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -6165,6 +6165,116 @@ describe('ReportUtils', () => { expect(canEditRequest).toEqual(false); }); + it('should not let someone else edit an unreported expense just because they are an admin of their own workspace', async () => { + const otherUserAccountID = 99; + const selfDMReport: Report = { + reportID: '98099', + type: CONST.REPORT.TYPE.CHAT, + chatType: CONST.REPORT.CHAT_TYPE.SELF_DM, + ownerAccountID: otherUserAccountID, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + }; + // The workspace the viewer would move the expense to, which is their own - not the workspace the expense belongs to + const viewerOwnPolicy: Policy = { + ...createRandomPolicy(98099, CONST.POLICY.TYPE.TEAM), + role: CONST.POLICY.ROLE.ADMIN, + }; + const transaction = { + ...createRandomTransaction(98099), + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + // A managed card transaction is allowed through earlier on, so keep this a plain tracked expense + managedCard: false, + }; + const trackExpenseAction: ReportAction = { + ...createRandomReportAction(98099), + reportID: selfDMReport.reportID, + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + actorAccountID: otherUserAccountID, + message: [{type: CONST.REPORT.MESSAGE.TYPE.TEXT, text: ''}], + previousMessage: undefined, + originalMessage: { + IOUTransactionID: transaction.transactionID, + amount: 530, + currency: CONST.CURRENCY.USD, + type: CONST.IOU.REPORT_ACTION_TYPE.TRACK, + }, + }; + + const reportCollectionDataSet: ReportCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`]: selfDMReport, + }; + const transactionCollectionDataSet: TransactionCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]: transaction, + }; + + await Onyx.multiSet({ + [ONYXKEYS.PERSONAL_DETAILS_LIST]: participantsPersonalDetails, + [ONYXKEYS.SESSION]: {email: currentUserEmail, accountID: currentUserAccountID}, + ...reportCollectionDataSet, + ...transactionCollectionDataSet, + }); + await waitForBatchedUpdates(); + + expect(canEditMoneyRequest(trackExpenseAction, transaction, undefined, false, selfDMReport, viewerOwnPolicy)).toBe(false); + + // The person who tracked the expense keeps their own edit rights + expect(canEditMoneyRequest({...trackExpenseAction, actorAccountID: currentUserAccountID}, transaction, undefined, false, selfDMReport, viewerOwnPolicy)).toBe(true); + }); + + it('should still let a policy admin edit an expense on an unapproved invoice report', async () => { + // Invoice reports are workspace reports, so restricting admin rights to reported expenses must not strip + // them here. Dynamic External Workflow is set deliberately because only the admin check short-circuits + // ahead of the guard that blocks non-admins on reports that are no longer open. + const invoicePolicy: Policy = { + ...createRandomPolicy(98100, CONST.POLICY.TYPE.TEAM), + role: CONST.POLICY.ROLE.ADMIN, + approvalMode: CONST.POLICY.APPROVAL_MODE.DYNAMICEXTERNAL, + }; + const invoiceReport: Report = { + reportID: '98100', + type: CONST.REPORT.TYPE.INVOICE, + policyID: invoicePolicy.id, + ownerAccountID: 99, + // A different account, so the invoice-manager early return below does not apply + managerID: 8723, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + }; + const transaction = {...createRandomTransaction(98100), reportID: invoiceReport.reportID}; + const invoiceAction: ReportAction = { + ...createRandomReportAction(98100), + reportID: invoiceReport.reportID, + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + actorAccountID: 99, + message: [{type: CONST.REPORT.MESSAGE.TYPE.TEXT, text: ''}], + previousMessage: undefined, + originalMessage: { + IOUTransactionID: transaction.transactionID, + amount: 530, + currency: CONST.CURRENCY.USD, + type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, + }, + }; + + const reportCollectionDataSet: ReportCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT}${invoiceReport.reportID}`]: invoiceReport, + }; + const transactionCollectionDataSet: TransactionCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]: transaction, + }; + + await Onyx.multiSet({ + [ONYXKEYS.PERSONAL_DETAILS_LIST]: participantsPersonalDetails, + [ONYXKEYS.SESSION]: {email: currentUserEmail, accountID: currentUserAccountID}, + ...reportCollectionDataSet, + ...transactionCollectionDataSet, + }); + await waitForBatchedUpdates(); + + expect(canEditMoneyRequest(invoiceAction, transaction, undefined, false, invoiceReport, invoicePolicy)).toBe(true); + }); + it('should use the passed reportActions to determine whether the report was forwarded since the last submit', async () => { const reportID = '89015'; const transactionID = '89015-transaction';