From f830f34a5be065a327e80bdec33d4584d3f062c1 Mon Sep 17 00:00:00 2001 From: "aya.abdallah" Date: Mon, 13 Jul 2026 13:29:52 +0300 Subject: [PATCH] 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 Co-Authored-By: Claude Sonnet 4.6 --- .../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);