[ThevinuJ]

Description

Currently, users can post financial transactions to accounts belonging to sibling branches (e.g., a teller in one branch processing a repayment or withdrawal for a client in another branch), bypassing proper office-level isolation.

This PR enforces the office-hierarchy check across loan and savings operations, bringing them in line with existing client-level access controls.

Summary of Changes

• Loans: Added office hierarchy validation for repayments, disbursals, write-offs, charge-offs, refunds, waivers, adjustments, and loan charges.
• Savings: Added validation for deposits, withdrawals, reversals, adjustments, holds, account closures, and savings charges.
• Account Transfers: Updated transfer logic to ensure both source and destination accounts reside within the user's office hierarchy.

What Remains Unchanged

• Read Operations: Viewing accounts and reports is intentionally untouched to preserve existing lookups and reporting functionality.
• Scheduled Jobs: Background processes like interest posting, standing instructions, and overdue charges continue to run unaffected as the system user.

Testing

• Added unit and integration tests covering loan, savings, and transfer workflows.
• Verified that users attempting transactions from a sibling branch receive a 403 Forbidden, while users from the correct office hierarchy succeed.
• Tested locally against a Docker setup alongside existing account transfer test suites.

Checklist

• Write the commit message as per our guidelines
• Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
• Create/update unit or integration tests for verifying the changes made.
• Follow our coding conventions.
• Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
• This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
• If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.
• I followed the AI Policy.

[adamsaghy]

Findings

1. Collection-sheet savings deposits skip the check (confirmed by tracing the code)

• Path: CollectionSheetWritePlatformServiceJpaRepositoryImpl.updateBulkMandatorySavingsDuePayments calls DepositAccountWritePlatformServiceJpaRepositoryImpl.mandatorySavingsAccountDeposit, which loads the account with depositAccountAssembler.assembleFrom(...). There is no office check.
• No other guard: the savingsId comes straight from the bulkSavingsDueTransactions JSON (DepositAccountAssembler:471). Nothing checks that the account belongs to the group.
• Effect: a sibling-branch user can still deposit into another branch's savings account through saveCollectionSheet or saveIndividualCollectionSheet. The same request's loan repayments and disbursals are blocked, because lines 768 and 1207 were changed.
• Why it matters: the description says deposits are covered.

2. Fixed and recurring deposit accounts are not covered at all

• Every method in DepositAccountWritePlatformServiceJpaRepositoryImpl still uses assembleFrom. That includes depositToRDAccount, depositToFDAccount, withdrawal, undoFD/RDTransaction, adjustFD/RDTransaction, prematureCloseFD/RDAccount and closeFD/RDAccount.
• These are the same kinds of operation the PR blocks on plain savings. Either guard them or say in the description that they are out of scope.

3. Undoing a loan disbursal can now fail with 403 (plausible; traced, not run)

• undoLoanDisbursal calls reverseAllTransactions(loanId, LOAN) at line 935. That undoes every account transfer linked to the loan through savingsAccountWritePlatformService.undoTransaction, which now goes through assembleFromInUserScope (line 795).
• Two kinds of transfer can involve a savings account in another branch. Standing instructions accept a toOfficeId. Guarantors have no office restriction, so a guarantor recovery can pull from another branch.
• Effect: a branch-A user who can see and act on a branch-A loan gets a 403 on undo-disbursal, and the whole undo rolls back. Before this PR it worked.
• The original transfer ran unchecked, as the system user or through transferFunds. So the user can't reverse something they were never blocked from creating.
• Suggested fix: in internal reversal paths, check the account that triggered the reversal (the loan), not each counterparty account.

Smaller gaps against the stated scope (non-blocking)

• Loan transactions outside LoanWritePlatformServiceJpaRepositoryImpl: capitalized income, buy-down fee, re-age, re-amortize, contract termination and interest pause still use assembleFrom. Capitalized income and buy-down fee create transactions with an amount, so they count as monetary.
• Working-capital loans (fineract-working-capital-loan) have their own repayment, write-off and charge-off services. None of them is guarded.
• Transfers: the account-transfer API takes explicit fromOfficeId and toOfficeId parameters. Requiring both sides to be in scope is a deliberate breaking change for clients that do cross-branch transfers as non-head-office users. It should be called out in the release notes.

Tests

The integration tests give the sibling user the relevant permission (for example REPAYMENT_LOAN), so the 403 really comes from the hierarchy check and not from a missing permission.

There is no test for the collection-sheet path, FD/RD, or undo-disbursal across branches. Those are the three findings above.

[ThevinuJ]

Thanks @adamsaghy for the review, all three findings were valid. I've pushed fixes:

1. Collection sheet: mandatorySavingsAccountDeposit now uses a scoped loader. The savings loop swallowed every exception, so the check fired but the request still returned 200. It now re-throws only NoAuthorizationException, giving a 403 like the loan part of the same sheet. Other errors behave as before.

2. FD/RD: deposit, withdrawal, post interest, undo, adjust, close and premature close now check the account's office.

3. Undo disbursal: internal transfer reversals (allowAccountTransferModification = true) no longer re-check the counterparty account, because the loan that triggered them is already checked. The user-facing transfer undo now checks both sides, matching create.

Tests: integration tests for all three, plus a regression test where undo-disbursal succeeds with a transfer from another branch's savings account.

Not covered in this PR - account activation (savings/FD/RD), capitalized income, buy-down fee, re-age/re-amortize, interest pause, contract termination, and working-capital loans. and cross-branch transfers now need access to both offices. Happy to open a follow-up JIRA for these and work on it next.