fix(current-account): stop account-id collisions between savings and current accounts from misrouting money and losing GL entries

m_current_account and m_savings_account each keep their own identity
sequence, so the same numeric id can be a real row in both -- an id-only
lookup used to decide "is this a current account?" once sent a real
deposit into an unrelated client's current account instead of the
savings account a request actually named, across every entry point that
did this: cashier deposit approval/reprocess, clerk withdrawal approval,
and clerk disbursement collection/disbursement/bank-transfer. All of
those now also verify the account_no before trusting an id match.

Separately, GL posting for current-account transactions has been
silently failing since acc_product_mapping had two rows for the same
(product_id, product_type, financial_account_type) slot -- current
products share PortfolioProductType.SAVING's id space with real savings
products, and a NULL payment_type doesn't collide under that column's
own unique constraint, so duplicates went unnoticed. The single-result
repository lookup that read this mapping threw on the duplicate, which
was swallowed by a try/catch, so account balances kept updating while no
journal entry was ever posted. Both current-account GL lookups now take
the most recently created mapping instead of failing outright.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

fineract-provider/src/main/java/org/apache/fineract/portfolio/cashierdeposit/api/CashierDepositRequestApiResource.java

@@ -423,7 +423,13 @@ public class CashierDepositRequestApiResource {
             // even for current accounts (submitted before the frontend fix), and FD/RD accounts share
             // the very same m_savings_account table as a plain savings account, distinguished only by
             // deposit_type_enum -- so this is looked up fresh rather than trusted from the request.
-            final String depositCategory = resolveDepositAccountCategory(savingsAccountId);
+            // The account_no is also checked -- m_current_account and m_savings_account each keep
+            // their own identity sequence, so the same numeric id can legitimately belong to a
+            // current account AND an unrelated savings account at once. An id match alone once sent
+            // a real deposit into a stranger's current account instead of the savings account the
+            // request actually named; account_no is the one value that can't collide between them.
+            final String requestAccountNo = (String) record.get("account_no");
+            final String depositCategory = resolveDepositAccountCategory(savingsAccountId, requestAccountNo);
             final boolean isCurrentAccount = "CURRENT".equals(depositCategory);
             txnId = switch (depositCategory) {
                 case "CURRENT" -> executeCurrentAccountDeposit(savingsAccountId, amount, ptId, txnDateStr, description);
@@ -578,7 +584,8 @@ public class CashierDepositRequestApiResource {
         final String txnDateStr = txnDateObj != null ? Iterables.get(Splitter.on(' ').split(txnDateObj.toString()), 0) : null;
         final String description = (String) record.get("description");
 
-        if (savingsAccountId == null || !isCurrentAccountById(savingsAccountId)) {
+        final String requestAccountNo = (String) record.get("account_no");
+        if (savingsAccountId == null || !isCurrentAccountById(savingsAccountId, requestAccountNo)) {
             throw apiError(400, "This account is not a current account. Reprocess is only needed for current account deposits.");
         }
 
@@ -863,11 +870,30 @@ public class CashierDepositRequestApiResource {
         return result.getResourceId() != null ? result.getResourceId().toString() : null;
     }
 
-    /** Returns true if this account ID belongs to m_current_account (not m_savings_account). */
-    private boolean isCurrentAccountById(final Long accountId) {
+    /**
+     * Returns true if this account ID belongs to m_current_account (not m_savings_account).
+     *
+     * m_current_account and m_savings_account each keep their own identity sequence, so id=1 (say)
+     * can be a real row in BOTH tables at once — two completely unrelated accounts, possibly two
+     * different clients, sharing nothing but a coincidental number. An id-only check here once sent
+     * a savings deposit into a stranger's current account: id=1 existed in m_current_account, so
+     * this returned true, even though the request was for m_savings_account id=1.
+     *
+     * expectedAccountNo — the account_no the caller actually meant — closes that gap: an id match is
+     * only trusted once the account_no on that current-account row confirms it is truly the same
+     * account, not a same-numbered stranger. Pass null to skip the check (only safe where the caller
+     * has no account_no to compare against, e.g. an id already known to be unambiguous).
+     */
+    private boolean isCurrentAccountById(final Long accountId, final String expectedAccountNo) {
         try {
+            if (expectedAccountNo == null || expectedAccountNo.isBlank()) {
+                final Integer count = jdbcTemplate.queryForObject(
+                    "SELECT COUNT(*) FROM m_current_account WHERE id = ?", Integer.class, accountId);
+                return count != null && count > 0;
+            }
             final Integer count = jdbcTemplate.queryForObject(
-                "SELECT COUNT(*) FROM m_current_account WHERE id = ?", Integer.class, accountId);
+                "SELECT COUNT(*) FROM m_current_account WHERE id = ? AND account_no = ?",
+                Integer.class, accountId, expectedAccountNo);
             return count != null && count > 0;
         } catch (Exception e) {
             return false;
@@ -880,12 +906,15 @@ public class CashierDepositRequestApiResource {
      * m_savings_account table as a plain savings account (deposit_type_enum 200/300 vs 100), so a
      * plain "not current → savings" check would post an FD/RD deposit through the generic savings
      * deposit command, which does not accept a term-deposit account id.
+     *
+     * expectedAccountNo guards the current-account check against an id collision with
+     * m_savings_account — see {@link #isCurrentAccountById}.
      */
-    private String resolveDepositAccountCategory(final Long accountId) {
+    private String resolveDepositAccountCategory(final Long accountId, final String expectedAccountNo) {
         if (accountId == null) {
             return "SAVINGS";
         }
-        if (isCurrentAccountById(accountId)) {
+        if (isCurrentAccountById(accountId, expectedAccountNo)) {
             return "CURRENT";
         }
         final List<Integer> depositTypes = jdbcTemplate.queryForList(

fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkdisbursement/api/ClerkDisbursementRequestApiResource.java

@@ -551,8 +551,11 @@ public class ClerkDisbursementRequestApiResource {
                     final BigDecimal entryAmt = new BigDecimal(entry.get("amount").getAsString());
                     final String acNo = entry.has("accountNo") ? entry.get("accountNo").getAsString() : "?";
                     final Long accountId = entry.get("savingsAccountId").getAsLong();
-                    final boolean isCurrent = "CURRENT".equals(
-                        entry.has("accountType") ? entry.get("accountType").getAsString() : "SAVINGS");
+                    // Not trusted from entry.accountType alone -- see isCurrentAccountById: an id can
+                    // exist in both m_current_account and m_savings_account at once, and a bank
+                    // transfer posts and moves money immediately, with no maker-checker step to catch
+                    // a wrong label before it does.
+                    final boolean isCurrent = isCurrentAccountById(accountId, acNo);
 
                     final Long ctrlGl = isCurrent
                         ? getCurrentControlGlId(accountId) : getSavingsControlGlId(accountId);
@@ -660,7 +663,7 @@ public class ClerkDisbursementRequestApiResource {
                     }
 
                     final Long accountId = entry.get("savingsAccountId").getAsLong();
-                    final boolean isCurrentAccount = isCurrentAccountById(accountId);
+                    final boolean isCurrentAccount = isCurrentAccountById(accountId, acNo);
                     String txnId = null;
 
                     if ("Cr".equals(dbCr)) {
@@ -959,8 +962,11 @@ public class ClerkDisbursementRequestApiResource {
                 final Long accountId = e.get("savingsAccountId").getAsLong();
                 final BigDecimal amount = new BigDecimal(e.get("amount").getAsString());
                 final String acNo = e.has("accountNo") ? e.get("accountNo").getAsString() : "?";
-                final String acTyp = e.has("accountType") ? e.get("accountType").getAsString() : "SAVINGS";
-                final boolean isCurrent = "CURRENT".equals(acTyp);
+                // Not trusted from entry.accountType alone -- see isCurrentAccountById: an id can
+                // exist in both m_current_account and m_savings_account at once, and a bank transfer
+                // posts and moves money immediately, with no maker-checker step to catch a wrong
+                // label before it does.
+                final boolean isCurrent = isCurrentAccountById(accountId, acNo);
 
                 final Long ctrlGl = isCurrent
                     ? getCurrentControlGlId(accountId) : getSavingsControlGlId(accountId);
@@ -986,8 +992,8 @@ public class ClerkDisbursementRequestApiResource {
                 final Long accountId = e.get("savingsAccountId").getAsLong();
                 final BigDecimal amount = new BigDecimal(e.get("amount").getAsString());
                 final String acNo = e.has("accountNo") ? e.get("accountNo").getAsString() : "?";
-                final String acTyp = e.has("accountType") ? e.get("accountType").getAsString() : "SAVINGS";
-                final boolean isCurrent = "CURRENT".equals(acTyp);
+                // Not trusted from entry.accountType alone -- see isCurrentAccountById.
+                final boolean isCurrent = isCurrentAccountById(accountId, acNo);
 
                 final Long ctrlGl = isCurrent
                     ? getCurrentControlGlId(accountId) : getSavingsControlGlId(accountId);
@@ -1361,10 +1367,25 @@ public class ClerkDisbursementRequestApiResource {
         commandsSourceWritePlatformService.logCommandSource(cmd);
     }
 
-    private boolean isCurrentAccountById(final Long accountId) {
+    /**
+     * m_current_account and m_savings_account each keep their own identity sequence, so the same
+     * numeric id can be a real row in both at once -- two unrelated accounts, possibly two different
+     * clients, sharing nothing but a coincidental number. An id-only check here has already sent a
+     * real deposit into a stranger's current account instead of the savings account the entry
+     * actually named. expectedAccountNo -- the entry's own accountNo -- closes that gap: an id match
+     * is only trusted once the account_no on that current-account row confirms it. Pass null to skip
+     * the check where the caller has no account_no to compare against.
+     */
+    private boolean isCurrentAccountById(final Long accountId, final String expectedAccountNo) {
         try {
+            if (expectedAccountNo == null || expectedAccountNo.isBlank() || "?".equals(expectedAccountNo)) {
+                final Integer count = jdbcTemplate.queryForObject(
+                    "SELECT COUNT(*) FROM m_current_account WHERE id = ?", Integer.class, accountId);
+                return count != null && count > 0;
+            }
             final Integer count = jdbcTemplate.queryForObject(
-                "SELECT COUNT(*) FROM m_current_account WHERE id = ?", Integer.class, accountId);
+                "SELECT COUNT(*) FROM m_current_account WHERE id = ? AND account_no = ?",
+                Integer.class, accountId, expectedAccountNo);
             return count != null && count > 0;
         } catch (Exception e) {
             return false;

fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkWithdrawalRequestApiResource.java

@@ -470,7 +470,7 @@ public class ClerkWithdrawalRequestApiResource {
 
         String txnId = null;
         if (savingsAccountId != null) {
-            final boolean isCurrentAccount = isCurrentAccountById(savingsAccountId);
+            final boolean isCurrentAccount = isCurrentAccountById(savingsAccountId, (String) record.get("account_no"));
             txnId = isCurrentAccount
                 ? executeCurrentAccountWithdrawal(savingsAccountId, amount, ptId, txnDateStr, description)
                 : executeSavingsWithdrawal(savingsAccountId, amount, ptId, txnDateStr, description);
@@ -872,10 +872,25 @@ public class ClerkWithdrawalRequestApiResource {
         return result.getResourceId() != null ? result.getResourceId().toString() : null;
     }
 
-    private boolean isCurrentAccountById(final Long accountId) {
+    /**
+     * m_current_account and m_savings_account each keep their own identity sequence, so the same
+     * numeric id can be a real row in both at once -- two unrelated accounts, possibly two different
+     * clients, sharing nothing but a coincidental number. An id-only check here can send a real
+     * withdrawal against a stranger's current account instead of the savings account the request
+     * actually named. expectedAccountNo -- the request's own account_no -- closes that gap: an id
+     * match is only trusted once the account_no on that current-account row confirms it. Pass null
+     * to skip the check where the caller has no account_no to compare against.
+     */
+    private boolean isCurrentAccountById(final Long accountId, final String expectedAccountNo) {
         try {
+            if (expectedAccountNo == null || expectedAccountNo.isBlank()) {
+                final Integer count = jdbcTemplate.queryForObject(
+                    "SELECT COUNT(*) FROM m_current_account WHERE id = ?", Integer.class, accountId);
+                return count != null && count > 0;
+            }
             final Integer count = jdbcTemplate.queryForObject(
-                "SELECT COUNT(*) FROM m_current_account WHERE id = ?", Integer.class, accountId);
+                "SELECT COUNT(*) FROM m_current_account WHERE id = ? AND account_no = ?",
+                Integer.class, accountId, expectedAccountNo);
             return count != null && count > 0;
         } catch (Exception e) {
             return false;