From 5821c6ac820423b7f431bdf06f6ae8da78f43753 Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Mon, 13 Jul 2026 13:29:52 +0300 Subject: [PATCH 1/9] FINERACT-2688: Fix loan disbursement for checker-only users via maker-checker permission check When maker-checker is enabled, a user holding only the _CHECKER permission (e.g. DISBURSE_LOAN_CHECKER) was rejected with a permission error instead of being routed to the pending approval queue. - logCommandSource(): detect checker-only users, look up the pending AWAITING_APPROVAL command for the same action/entity/resource, and auto-approve it; block makers from duplicate pending submissions - validateMakerChecker(): broaden approval condition from isCheckerSuperUser() to also accept role-specific _CHECKER permissions - Add CommandSourceRepository.findPendingByActionAndEntityAndResource() native query (searches all resource-id columns) - Add MakerCheckerCheckerOnlyInitiationException and MakerCheckerDuplicatePendingSubmissionException Signed-off-by: aya.abdallah --- .../domain/CommandSourceRepository.java | 23 +++++++ ...CheckerCheckerOnlyInitiationException.java | 34 ++++++++++ ...erDuplicatePendingSubmissionException.java | 34 ++++++++++ .../service/CommandSourceService.java | 3 +- ...CommandSourceWritePlatformServiceImpl.java | 65 +++++++++++++++++-- 5 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerCheckerOnlyInitiationException.java create mode 100644 fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerDuplicatePendingSubmissionException.java diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java index ca0de426e7b..937f5025a89 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java @@ -19,6 +19,7 @@ package org.apache.fineract.commands.domain; import java.time.OffsetDateTime; +import java.util.List; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.jpa.repository.Modifying; @@ -29,6 +30,28 @@ public interface CommandSourceRepository extends JpaRepository findPendingByActionAndEntityAndResource(@Param("actionName") String actionName, + @Param("entityName") String entityName, @Param("resourceId") Long resourceId, @Param("status") Integer status); + @Modifying(flushAutomatically = true) @Query("delete from CommandSource c where c.status = :status and c.madeOnDate is not null and c.madeOnDate <= :dateForPurgeCriteria") void deleteOlderEventsWithStatus(@Param("status") Integer status, @Param("dateForPurgeCriteria") OffsetDateTime dateForPurgeCriteria); diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerCheckerOnlyInitiationException.java b/fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerCheckerOnlyInitiationException.java new file mode 100644 index 00000000000..08aa18ca873 --- /dev/null +++ b/fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerCheckerOnlyInitiationException.java @@ -0,0 +1,34 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.commands.exception; + +import org.apache.fineract.infrastructure.core.exception.AbstractPlatformDomainRuleException; + +/** + * Thrown when a checker-only user (has _CHECKER permission but not the base permission) attempts to initiate an action + * directly, with no pending maker submission to approve. + */ +public class MakerCheckerCheckerOnlyInitiationException extends AbstractPlatformDomainRuleException { + + public MakerCheckerCheckerOnlyInitiationException(final String permissionCode) { + super("error.msg.maker.checker.checker.only.cannot.initiate", + "You have checker-only permission for this action. You cannot initiate it. Use the maker-checker approval flow to approve a pending submission.", + permissionCode); + } +} diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerDuplicatePendingSubmissionException.java b/fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerDuplicatePendingSubmissionException.java new file mode 100644 index 00000000000..e269f14ae43 --- /dev/null +++ b/fineract-core/src/main/java/org/apache/fineract/commands/exception/MakerCheckerDuplicatePendingSubmissionException.java @@ -0,0 +1,34 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.commands.exception; + +import org.apache.fineract.infrastructure.core.exception.AbstractPlatformDomainRuleException; + +/** + * Thrown when a maker attempts to submit an action that already has a pending checker approval entry for the same + * action, entity, and resource. + */ +public class MakerCheckerDuplicatePendingSubmissionException extends AbstractPlatformDomainRuleException { + + public MakerCheckerDuplicatePendingSubmissionException(final String actionName, final String entityName) { + super("error.msg.maker.checker.duplicate.pending.submission", + "This action is already pending checker approval. Please wait for it to be approved or rejected before resubmitting.", + actionName, entityName); + } +} diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java b/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java index 7127be695f9..a90e193eb6f 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java @@ -128,7 +128,8 @@ private void validateMakerChecker(CommandSource commandSource, AppUser user, boo String permission = commandSource.getPermissionCode(); boolean isMakerChecker = configurationDomainService.isMakerCheckerEnabledForTask(permission); if (isMakerChecker || result.isRollbackTransaction()) { - if (isApprovedByChecker || user.isCheckerSuperUser()) { + boolean userHasCheckerPermission = !user.hasNotPermissionForAnyOf("CHECKER_SUPER_USER", permission + "_CHECKER"); + if (isApprovedByChecker || userHasCheckerPermission) { commandSource.markAsChecked(user); } else { if (commandSource.isSanitized()) { diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java b/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java index 604a07a7a4d..f89439fcc58 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java @@ -23,11 +23,14 @@ import java.util.Objects; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.apache.fineract.commands.domain.CommandProcessingResultType; import org.apache.fineract.commands.domain.CommandSource; import org.apache.fineract.commands.domain.CommandSourceRepository; import org.apache.fineract.commands.domain.CommandWrapper; import org.apache.fineract.commands.exception.CommandNotAwaitingApprovalException; import org.apache.fineract.commands.exception.CommandNotFoundException; +import org.apache.fineract.commands.exception.MakerCheckerCheckerOnlyInitiationException; +import org.apache.fineract.commands.exception.MakerCheckerDuplicatePendingSubmissionException; import org.apache.fineract.commands.exception.UnsupportedCommandException; import org.apache.fineract.infrastructure.configuration.domain.ConfigurationDomainService; import org.apache.fineract.infrastructure.core.api.JsonCommand; @@ -56,18 +59,45 @@ public class PortfolioCommandSourceWritePlatformServiceImpl implements Portfolio @Override public CommandProcessingResult logCommandSource(final CommandWrapper wrapper) { boolean isApprovedByChecker = false; + final AppUser currentUser = this.context.authenticatedUser(wrapper); // check if is update of own account details - if (wrapper.isChangeOfOwnUserDetails(this.context.authenticatedUser(wrapper).getId())) { + if (wrapper.isChangeOfOwnUserDetails(currentUser.getId())) { // then allow this operation to proceed. // maker checker doesnt mean anything here. isApprovedByChecker = true; // set to true in case permissions have // been maker-checker enabled by // accident. } else { - // if not user changing their own details - check user has - // permission to perform specific task. - this.context.authenticatedUser(wrapper).validateHasPermissionTo(wrapper.getTaskPermissionName()); + final String taskPermission = wrapper.getTaskPermissionName(); + final boolean hasBasePermission = !currentUser.hasNotPermissionForAnyOf(taskPermission); + final boolean hasCheckerPermission = !currentUser.hasNotPermissionForAnyOf("CHECKER_SUPER_USER", taskPermission + "_CHECKER"); + + if (!hasBasePermission && hasCheckerPermission) { + // Checker-only user: find and approve the pending entry for this action+entity+resource + final Long resourceId = resolveResourceId(wrapper); + final List pendingCommands = findPendingCommandsByResource(wrapper, resourceId); + if (!pendingCommands.isEmpty()) { + final CommandSource pendingCommand = pendingCommands.get(0); + log.debug("Checker-only user {} auto-approving pending command id={} for {}/{}", currentUser.getUsername(), + pendingCommand.getId(), wrapper.entityName(), wrapper.actionName()); + return approveEntry(pendingCommand.getId()); + } else { + throw new MakerCheckerCheckerOnlyInitiationException(taskPermission); + } + } else { + currentUser.validateHasPermissionTo(taskPermission); + + if (!hasCheckerPermission && configurationService.isMakerCheckerEnabledForTask(taskPermission)) { + final Long resourceId = resolveResourceId(wrapper); + final List pendingCommands = findPendingCommandsByResource(wrapper, resourceId); + if (!pendingCommands.isEmpty()) { + log.warn("Maker {} attempted duplicate submission for {}/{} - pending id={}", currentUser.getUsername(), + wrapper.entityName(), wrapper.actionName(), pendingCommands.get(0).getId()); + throw new MakerCheckerDuplicatePendingSubmissionException(wrapper.actionName(), wrapper.entityName()); + } + } + } } validateIsUpdateAllowed(); @@ -142,6 +172,33 @@ private void validateIsUpdateAllowed() { this.schedulerJobRunnerReadService.isUpdatesAllowed(); } + private List findPendingCommandsByResource(final CommandWrapper wrapper, final Long resourceId) { + if (resourceId == null) { + return List.of(); + } + return this.commandSourceRepository.findPendingByActionAndEntityAndResource(wrapper.actionName(), wrapper.entityName(), resourceId, + CommandProcessingResultType.AWAITING_APPROVAL.getValue()); + } + + private Long resolveResourceId(final CommandWrapper wrapper) { + if (wrapper.getEntityId() != null) { + return wrapper.getEntityId(); + } + if (wrapper.getLoanId() != null) { + return wrapper.getLoanId(); + } + if (wrapper.getSavingsId() != null) { + return wrapper.getSavingsId(); + } + if (wrapper.getClientId() != null) { + return wrapper.getClientId(); + } + if (wrapper.getGroupId() != null) { + return wrapper.getGroupId(); + } + return null; + } + @Override public Long rejectEntry(final Long makerCheckerId) { final CommandSource commandSourceInput = validateMakerCheckerTransaction(makerCheckerId); From a2835e531631c41f0da5945ad30df8ac5c1289e4 Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Tue, 14 Jul 2026 09:34:51 +0300 Subject: [PATCH 2/9] FINERACT-2688: Revert validateMakerChecker to isCheckerSuperUser to fix MakercheckerTest The previous change replaced user.isCheckerSuperUser() with a broader userHasCheckerPermission check that also matched task-specific _CHECKER permissions. This caused makers who hold both base and _CHECKER permissions to have their commands auto-approved instead of routed to AWAITING_APPROVAL, breaking MakercheckerTest across all three databases. The checker-only flow introduced by FINERACT-2688 is handled upstream in logCommandSource, so validateMakerChecker never sees checker-only users and needs no change. Signed-off-by: aya.abdallah --- .../apache/fineract/commands/service/CommandSourceService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java b/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java index a90e193eb6f..7127be695f9 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java @@ -128,8 +128,7 @@ private void validateMakerChecker(CommandSource commandSource, AppUser user, boo String permission = commandSource.getPermissionCode(); boolean isMakerChecker = configurationDomainService.isMakerCheckerEnabledForTask(permission); if (isMakerChecker || result.isRollbackTransaction()) { - boolean userHasCheckerPermission = !user.hasNotPermissionForAnyOf("CHECKER_SUPER_USER", permission + "_CHECKER"); - if (isApprovedByChecker || userHasCheckerPermission) { + if (isApprovedByChecker || user.isCheckerSuperUser()) { commandSource.markAsChecked(user); } else { if (commandSource.isSanitized()) { From 7809abaded3c9048a40ea6909762edce69725dff Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Tue, 14 Jul 2026 13:17:07 +0300 Subject: [PATCH 3/9] FINERACT-2688: Add integration test for checker-only loan disbursement maker-checker flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers all five scenarios: - Maker submits disbursement → queued as AWAITING_APPROVAL - Maker submits again → HTTP 403 duplicate pending submission - Checker-only user with no pending entry → HTTP 403 cannot initiate - Checker-only user auto-approves pending entry and disburses - Traditional /makerchecker/{id}?command=approve flow unaffected Signed-off-by: aya.abdallah --- ...sbursementMakerCheckerIntegrationTest.java | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java new file mode 100644 index 00000000000..c805eebd910 --- /dev/null +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java @@ -0,0 +1,202 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.integrationtests; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import io.restassured.builder.RequestSpecBuilder; +import io.restassured.builder.ResponseSpecBuilder; +import io.restassured.http.ContentType; +import io.restassured.specification.RequestSpecification; +import io.restassured.specification.ResponseSpecification; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.fineract.client.models.PutGlobalConfigurationsRequest; +import org.apache.fineract.client.models.PutPermissionsRequest; +import org.apache.fineract.infrastructure.configuration.api.GlobalConfigurationConstants; +import org.apache.fineract.integrationtests.common.ClientHelper; +import org.apache.fineract.integrationtests.common.CommonConstants; +import org.apache.fineract.integrationtests.common.GlobalConfigurationHelper; +import org.apache.fineract.integrationtests.common.Utils; +import org.apache.fineract.integrationtests.common.commands.MakercheckersHelper; +import org.apache.fineract.integrationtests.common.loans.LoanApplicationTestBuilder; +import org.apache.fineract.integrationtests.common.loans.LoanProductTestBuilder; +import org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper; +import org.apache.fineract.integrationtests.common.organisation.StaffHelper; +import org.apache.fineract.integrationtests.useradministration.roles.RolesHelper; +import org.apache.fineract.integrationtests.useradministration.users.UserHelper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Integration test for the CBS-509 / FINERACT-2688 fix: maker-checker support for checker-only users on loan + * disbursement. + * + *

+ * Covers the complete flow: + *

    + *
  1. Maker submits disbursement → queued as AWAITING_APPROVAL (HTTP 200, loanId null)
    2. + *
  2. Maker submits again → HTTP 403 duplicate.pending.submission
    3. + *
  3. Checker-only user submits for loan with no pending entry → HTTP 403 checker.only.cannot.initiate
    4. + *
  4. Checker-only user submits for loan A (has pending entry) → auto-approves and disburses (loanId not null)
    5. + *
  5. Traditional /makerchecker/{id}?command=approve flow still works correctly
    6. + *
+ */ +public class LoanDisbursementMakerCheckerIntegrationTest { + + private static final String DISBURSAL_DATE = "01 January 2024"; + private static final String LOAN_SUBMISSION_DATE = "01 January 2024"; + private static final String LOAN_TERM_FREQUENCY = "12"; + private static final String LOAN_REPAYMENT_PERIOD = "1"; + private static final String LOAN_PRINCIPAL = "10000.00"; + private static final String DISBURSE_LOAN_PERMISSION = "DISBURSE_LOAN"; + + private ResponseSpecification responseSpec; + private RequestSpecification requestSpec; + private LoanTransactionHelper loanTransactionHelper; + private GlobalConfigurationHelper globalConfigurationHelper; + private MakercheckersHelper makercheckersHelper; + private RolesHelper rolesHelper; + + @BeforeEach + public void setup() { + Utils.initializeRESTAssured(); + this.requestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build(); + this.requestSpec.header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey()); + this.responseSpec = new ResponseSpecBuilder().expectStatusCode(200).build(); + this.loanTransactionHelper = new LoanTransactionHelper(this.requestSpec, this.responseSpec); + this.globalConfigurationHelper = new GlobalConfigurationHelper(); + this.makercheckersHelper = new MakercheckersHelper(this.requestSpec, this.responseSpec); + this.rolesHelper = new RolesHelper(); + } + + @Test + public void testLoanDisbursementMakerCheckerFlow() { + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(true)); + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.ENABLE_SAME_MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(false)); + + try { + // Enable maker-checker for loan disbursement + rolesHelper.updatePermissions(new PutPermissionsRequest().putPermissionsItem(DISBURSE_LOAN_PERMISSION, true)); + + // Maker role: base permission only (no checker permission) + Integer makerRoleId = RolesHelper.createRole(requestSpec, responseSpec); + RolesHelper.addPermissionsToRole(requestSpec, responseSpec, makerRoleId, Map.of(DISBURSE_LOAN_PERMISSION, true)); + + // Checker-only role: checker permission only (no base permission) + Integer checkerRoleId = RolesHelper.createRole(requestSpec, responseSpec); + RolesHelper.addPermissionsToRole(requestSpec, responseSpec, checkerRoleId, Map.of(DISBURSE_LOAN_PERMISSION + "_CHECKER", true)); + + Integer staffId = StaffHelper.createStaff(requestSpec, responseSpec); + String makerUsername = Utils.uniqueRandomStringGenerator("mkr", 8); + Integer makerUserId = (Integer) UserHelper.createUser(requestSpec, responseSpec, makerRoleId, staffId, makerUsername, + "A1b2c3d4e5f$", "resourceId"); + String checkerUsername = Utils.uniqueRandomStringGenerator("ckr", 8); + UserHelper.createUser(requestSpec, responseSpec, checkerRoleId, staffId, checkerUsername, "A1b2c3d4e5f$", "resourceId"); + + RequestSpecification makerRequestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build().header( + "Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey(makerUsername, "A1b2c3d4e5f$")); + RequestSpecification checkerRequestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build().header( + "Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey(checkerUsername, "A1b2c3d4e5f$")); + + // Admin creates product, client, and three approved loans + Integer clientId = ClientHelper.createClient(requestSpec, responseSpec); + assertNotNull(clientId); + Integer loanProductId = loanTransactionHelper.getLoanProductId(new LoanProductTestBuilder().withPrincipal(LOAN_PRINCIPAL) + .withRepaymentTypeAsMonth().withRepaymentAfterEvery(LOAN_REPAYMENT_PERIOD).withNumberOfRepayments(LOAN_TERM_FREQUENCY) + .withinterestRatePerPeriod("0").withInterestRateFrequencyTypeAsMonths().withAmortizationTypeAsEqualPrincipalPayment() + .withInterestTypeAsFlat().build(null)); + + Integer loanIdA = createAndApproveLoan(clientId, loanProductId); + Integer loanIdB = createAndApproveLoan(clientId, loanProductId); + Integer loanIdC = createAndApproveLoan(clientId, loanProductId); + + LoanTransactionHelper makerLoanHelper = new LoanTransactionHelper(makerRequestSpec, responseSpec); + LoanTransactionHelper checkerLoanHelper = new LoanTransactionHelper(checkerRequestSpec, responseSpec); + ResponseSpecification forbiddenSpec = new ResponseSpecBuilder().expectStatusCode(403).build(); + + // Scenario 1: Maker submits disbursement for loan A → queued as AWAITING_APPROVAL + HashMap makerResponseA = makerLoanHelper.disburseLoan(DISBURSAL_DATE, loanIdA, LOAN_PRINCIPAL, null); + assertNull(makerResponseA.get("loanId"), "Disbursement should be queued for checker approval, not immediately processed"); + + List> pendingEntries = makercheckersHelper + .getMakerCheckerList(Map.of("actionName", "DISBURSE", "entityName", "LOAN", "makerId", makerUserId.toString())); + assertEquals(1, pendingEntries.size(), "Exactly one pending DISBURSE_LOAN command should exist"); + + // Scenario 2: Maker submits again for loan A → 403 duplicate pending submission + Object duplicateResult = makerLoanHelper.disburseLoanWithTransactionAmount(DISBURSAL_DATE, loanIdA, LOAN_PRINCIPAL, + forbiddenSpec); + List> duplicateErrors = (List>) duplicateResult; + assertEquals("error.msg.maker.checker.duplicate.pending.submission", + duplicateErrors.get(0).get(CommonConstants.RESPONSE_ERROR_MESSAGE_CODE), "Duplicate submission should be rejected"); + + // Scenario 3: Checker-only user submits for loan B (no pending entry) → 403 cannot initiate + Object noEntryResult = checkerLoanHelper.disburseLoanWithTransactionAmount(DISBURSAL_DATE, loanIdB, LOAN_PRINCIPAL, + forbiddenSpec); + List> noEntryErrors = (List>) noEntryResult; + assertEquals("error.msg.maker.checker.checker.only.cannot.initiate", + noEntryErrors.get(0).get(CommonConstants.RESPONSE_ERROR_MESSAGE_CODE), + "Checker-only user cannot initiate a new disbursement"); + + // Scenario 4: Checker-only user submits for loan A (has pending entry) → auto-approves and disburses + HashMap checkerResponseA = checkerLoanHelper.disburseLoan(DISBURSAL_DATE, loanIdA, LOAN_PRINCIPAL, null); + assertNotNull(checkerResponseA.get("loanId"), "Loan A should be disbursed after checker-only user auto-approval"); + + // Scenario 5: Traditional /makerchecker/{id}?command=approve flow is unaffected + // Maker queues disbursement for loan C + HashMap makerResponseC = makerLoanHelper.disburseLoan(DISBURSAL_DATE, loanIdC, LOAN_PRINCIPAL, null); + assertNull(makerResponseC.get("loanId"), "Loan C disbursement should be queued"); + + List> pendingEntriesC = makercheckersHelper.getMakerCheckerList(Map.of("actionName", "DISBURSE", + "entityName", "LOAN", "makerId", makerUserId.toString(), "resourceId", loanIdC.toString())); + assertEquals(1, pendingEntriesC.size(), "Exactly one pending entry for loan C"); + Long pendingCommandIdC = ((Double) pendingEntriesC.get(0).get("id")).longValue(); + + // Admin approves via the existing /makerchecker/{id}?command=approve endpoint + HashMap approveResult = MakercheckersHelper.approveMakerCheckerEntry(requestSpec, responseSpec, pendingCommandIdC); + assertNotNull(approveResult); + assertNotNull(approveResult.get("loanId"), "Loan C should be disbursed after traditional checker approval"); + + } finally { + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(false)); + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.ENABLE_SAME_MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(true)); + rolesHelper.updatePermissions(new PutPermissionsRequest().putPermissionsItem(DISBURSE_LOAN_PERMISSION, false)); + } + } + + private Integer createAndApproveLoan(Integer clientId, Integer loanProductId) { + String loanJson = new LoanApplicationTestBuilder().withPrincipal(LOAN_PRINCIPAL).withLoanTermFrequency(LOAN_TERM_FREQUENCY) + .withLoanTermFrequencyAsMonths().withNumberOfRepayments(LOAN_TERM_FREQUENCY).withRepaymentEveryAfter(LOAN_REPAYMENT_PERIOD) + .withRepaymentFrequencyTypeAsMonths().withInterestRatePerPeriod("0").withInterestTypeAsFlatBalance() + .withAmortizationTypeAsEqualPrincipalPayments().withInterestCalculationPeriodTypeSameAsRepaymentPeriod() + .withExpectedDisbursementDate(DISBURSAL_DATE).withSubmittedOnDate(LOAN_SUBMISSION_DATE).withLoanType("individual") + .build(clientId.toString(), loanProductId.toString(), null); + Integer loanId = loanTransactionHelper.getLoanId(loanJson); + assertNotNull(loanId); + loanTransactionHelper.approveLoan(DISBURSAL_DATE, loanId); + return loanId; + } +} From cd65b81e8e90e9ff5502ccf8cdd73d7460e53c1b Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Wed, 15 Jul 2026 10:19:54 +0300 Subject: [PATCH 4/9] FINERACT-2688: Fix intermittent integration test failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes for the flaky CI failures in MySQL/MariaDB shards: 1. Move global config updates (MAKER_CHECKER, ENABLE_SAME_MAKER_CHECKER) inside the try block so the finally cleanup is guaranteed even if the second update throws — preventing MAKER_CHECKER=true from leaking to subsequent tests in the same shard and causing unexpected maker-checker queuing. 2. Narrow findPendingByActionAndEntityAndResource to match only resource_id, loan_id, and savings_account_id. The previous query also matched client_id, office_id, group_id, product_id, and other contextual columns. When a loan ID coincidentally equalled a contextual ID stored on another pending entry, the wrong command was returned — causing Scenario 3 (checker-only no-entry → 403) to auto-approve the wrong loan instead of throwing, breaking the assertion. --- .../commands/domain/CommandSourceRepository.java | 7 ------- .../LoanDisbursementMakerCheckerIntegrationTest.java | 9 ++++----- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java index 937f5025a89..5ce6692032a 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java @@ -36,15 +36,8 @@ where upper(c.action_name) = upper(?1) and upper(c.entity_name) = upper(?2) and ( c.resource_id = ?3 - or c.subresource_id = ?3 - or c.client_id = ?3 or c.loan_id = ?3 or c.savings_account_id = ?3 - or c.group_id = ?3 - or c.office_id = ?3 - or c.product_id = ?3 - or c.creditbureau_id = ?3 - or c.organisation_creditbureau_id = ?3 ) and c.status = ?4 order by c.made_on_date_utc desc diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java index c805eebd910..5e6eb4c3627 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDisbursementMakerCheckerIntegrationTest.java @@ -91,12 +91,11 @@ public void setup() { @Test public void testLoanDisbursementMakerCheckerFlow() { - globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.MAKER_CHECKER, - new PutGlobalConfigurationsRequest().enabled(true)); - globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.ENABLE_SAME_MAKER_CHECKER, - new PutGlobalConfigurationsRequest().enabled(false)); - try { + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(true)); + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.ENABLE_SAME_MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(false)); // Enable maker-checker for loan disbursement rolesHelper.updatePermissions(new PutPermissionsRequest().putPermissionsItem(DISBURSE_LOAN_PERMISSION, true)); From a473151d48eb2392bc00e8dbcda8a492c2ca45a8 Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Fri, 24 Jul 2026 10:15:28 +0300 Subject: [PATCH 5/9] FINERACT-2688: Use named parameters in native @Query for readability --- .../commands/domain/CommandSourceRepository.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java index 5ce6692032a..116ca4e4d52 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java @@ -32,14 +32,14 @@ public interface CommandSourceRepository extends JpaRepository findPendingByActionAndEntityAndResource(@Param("actionName") String actionName, From 0a1c26bd71285c3ff0d264a329e06ffb62267e6a Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Mon, 27 Jul 2026 10:43:19 +0300 Subject: [PATCH 6/9] FINERACT-2688: Fix shard test pollution and convert query to JPQL LoanNextPaymentDueAmountIntegrationTest enabled three business events (LoanApprovedBusinessEvent, LoanBalanceChangedBusinessEvent, LoanDisbursalBusinessEvent) in each test method but never disabled them. Adding LoanDisbursementMakerCheckerIntegrationTest alphabetically before it caused it to shift shards, exposing the leaked event state to subsequent tests and causing intermittent CI failures. Add @AfterEach cleanup to reliably disable the events after each method. Also convert the native SQL @Query to JPQL so it works identically on MariaDB, MySQL, and PostgreSQL without dialect-specific column names. --- .../domain/CommandSourceRepository.java | 18 +++++++----------- ...oanNextPaymentDueAmountIntegrationTest.java | 8 ++++++++ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java index 116ca4e4d52..60a09dac7f6 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java @@ -30,18 +30,14 @@ public interface CommandSourceRepository extends JpaRepository findPendingByActionAndEntityAndResource(@Param("actionName") String actionName, @Param("entityName") String entityName, @Param("resourceId") Long resourceId, @Param("status") Integer status); diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanNextPaymentDueAmountIntegrationTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanNextPaymentDueAmountIntegrationTest.java index 540a76c812a..ee0ebcd3884 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanNextPaymentDueAmountIntegrationTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanNextPaymentDueAmountIntegrationTest.java @@ -31,6 +31,7 @@ import org.apache.fineract.integrationtests.client.feign.FeignLoanTestBase; import org.apache.fineract.integrationtests.client.feign.modules.LoanTestData.InterestCalculationPeriodType; import org.apache.fineract.integrationtests.client.feign.modules.LoanTestData.RecalculationRestFrequencyType; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -40,6 +41,13 @@ public class LoanNextPaymentDueAmountIntegrationTest extends FeignLoanTestBase { ObjectMapper objectMapper = new ObjectMapper(); private static final String LOAN_ACCOUNT_DATA_V_1 = "org.apache.fineract.avro.loan.v1.LoanAccountDataV1"; + @AfterEach + void disableBusinessEvents() { + externalEventHelper.disableBusinessEvent("LoanApprovedBusinessEvent"); + externalEventHelper.disableBusinessEvent("LoanBalanceChangedBusinessEvent"); + externalEventHelper.disableBusinessEvent("LoanDisbursalBusinessEvent"); + } + @Test void test_progressive_interest_noRecalculation() { externalEventHelper.enableBusinessEvent("LoanApprovedBusinessEvent"); From a2bec2b79ea00e16bed70373e08af53eccd5ae4f Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Mon, 27 Jul 2026 12:13:06 +0300 Subject: [PATCH 7/9] FINERACT-2688: Revert JPQL to native SQL to fix shard 9 test failures --- .../domain/CommandSourceRepository.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java index 60a09dac7f6..116ca4e4d52 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java @@ -30,14 +30,18 @@ public interface CommandSourceRepository extends JpaRepository findPendingByActionAndEntityAndResource(@Param("actionName") String actionName, @Param("entityName") String entityName, @Param("resourceId") Long resourceId, @Param("status") Integer status); From a09b110c0fb4d0752549fac9dca9acbfe405d525 Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Tue, 28 Jul 2026 10:50:52 +0300 Subject: [PATCH 8/9] FINERACT-2688: Fix cross-DB query by switching to JPQL scalar id projection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the native SQL query (SELECT DISTINCT c.* with nativeQuery=true) with a plain JPQL scalar query that returns only the command source IDs. The native SQL had two failure modes: 1. MySQL/MariaDB: SELECT DISTINCT c.* triggers internal key creation for the TEXT column command_as_json, which MySQL cannot index without an explicit prefix length — resulting in a runtime SQL error. 2. The previous JPQL used DISTINCT on the entity type, causing Hibernate to JOIN-fetch eager @ManyToOne relationships and then apply DISTINCT over those columns, hitting the same TEXT-column restriction on MariaDB. Returning only c.id avoids both issues: no TEXT columns, no JOIN, no DISTINCT. The callers only needed the ID anyway (they reload the full entity via findById in approveEntry/validateMakerCheckerTransaction). --- .../domain/CommandSourceRepository.java | 20 +++++++---------- ...CommandSourceWritePlatformServiceImpl.java | 22 +++++++++---------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java index 116ca4e4d52..3fef504f8cc 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java @@ -30,19 +30,15 @@ public interface CommandSourceRepository extends JpaRepository findPendingByActionAndEntityAndResource(@Param("actionName") String actionName, + order by c.madeOnDate desc + """) + List findPendingIdsByActionAndEntityAndResource(@Param("actionName") String actionName, @Param("entityName") String entityName, @Param("resourceId") Long resourceId, @Param("status") Integer status); @Modifying(flushAutomatically = true) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java b/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java index f89439fcc58..fc6a79c2d25 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java @@ -76,12 +76,12 @@ public CommandProcessingResult logCommandSource(final CommandWrapper wrapper) { if (!hasBasePermission && hasCheckerPermission) { // Checker-only user: find and approve the pending entry for this action+entity+resource final Long resourceId = resolveResourceId(wrapper); - final List pendingCommands = findPendingCommandsByResource(wrapper, resourceId); - if (!pendingCommands.isEmpty()) { - final CommandSource pendingCommand = pendingCommands.get(0); + final List pendingIds = findPendingCommandIdsByResource(wrapper, resourceId); + if (!pendingIds.isEmpty()) { + final Long pendingCommandId = pendingIds.get(0); log.debug("Checker-only user {} auto-approving pending command id={} for {}/{}", currentUser.getUsername(), - pendingCommand.getId(), wrapper.entityName(), wrapper.actionName()); - return approveEntry(pendingCommand.getId()); + pendingCommandId, wrapper.entityName(), wrapper.actionName()); + return approveEntry(pendingCommandId); } else { throw new MakerCheckerCheckerOnlyInitiationException(taskPermission); } @@ -90,10 +90,10 @@ public CommandProcessingResult logCommandSource(final CommandWrapper wrapper) { if (!hasCheckerPermission && configurationService.isMakerCheckerEnabledForTask(taskPermission)) { final Long resourceId = resolveResourceId(wrapper); - final List pendingCommands = findPendingCommandsByResource(wrapper, resourceId); - if (!pendingCommands.isEmpty()) { + final List pendingIds = findPendingCommandIdsByResource(wrapper, resourceId); + if (!pendingIds.isEmpty()) { log.warn("Maker {} attempted duplicate submission for {}/{} - pending id={}", currentUser.getUsername(), - wrapper.entityName(), wrapper.actionName(), pendingCommands.get(0).getId()); + wrapper.entityName(), wrapper.actionName(), pendingIds.get(0)); throw new MakerCheckerDuplicatePendingSubmissionException(wrapper.actionName(), wrapper.entityName()); } } @@ -172,12 +172,12 @@ private void validateIsUpdateAllowed() { this.schedulerJobRunnerReadService.isUpdatesAllowed(); } - private List findPendingCommandsByResource(final CommandWrapper wrapper, final Long resourceId) { + private List findPendingCommandIdsByResource(final CommandWrapper wrapper, final Long resourceId) { if (resourceId == null) { return List.of(); } - return this.commandSourceRepository.findPendingByActionAndEntityAndResource(wrapper.actionName(), wrapper.entityName(), resourceId, - CommandProcessingResultType.AWAITING_APPROVAL.getValue()); + return this.commandSourceRepository.findPendingIdsByActionAndEntityAndResource(wrapper.actionName(), wrapper.entityName(), + resourceId, CommandProcessingResultType.AWAITING_APPROVAL.getValue()); } private Long resolveResourceId(final CommandWrapper wrapper) { From b7e7783bc52159df97926b58dc19e08a06464113 Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Tue, 28 Jul 2026 14:01:04 +0300 Subject: [PATCH 9/9] FINERACT-2688: Fix spotless formatting in CommandSourceRepository --- .../fineract/commands/domain/CommandSourceRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java index 3fef504f8cc..1a14f809819 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSourceRepository.java @@ -38,8 +38,8 @@ and upper(c.entityName) = upper(:entityName) and c.status = :status order by c.madeOnDate desc """) - List findPendingIdsByActionAndEntityAndResource(@Param("actionName") String actionName, - @Param("entityName") String entityName, @Param("resourceId") Long resourceId, @Param("status") Integer status); + List findPendingIdsByActionAndEntityAndResource(@Param("actionName") String actionName, @Param("entityName") String entityName, + @Param("resourceId") Long resourceId, @Param("status") Integer status); @Modifying(flushAutomatically = true) @Query("delete from CommandSource c where c.status = :status and c.madeOnDate is not null and c.madeOnDate <= :dateForPurgeCriteria")