From 2afd4aaf839c8e10ec917d746221fbf480c8af26 Mon Sep 17 00:00:00 2001 From: anshul Date: Wed, 30 Sep 2026 17:07:16 +0530 Subject: [PATCH 1/2] Enforce same-user maker-checker restriction --- .../service/CommandSourceService.java | 30 +++----- ...CommandSourceWritePlatformServiceImpl.java | 28 +++++-- ...WritePlatformServiceJpaRepositoryImpl.java | 12 ++- ...WritePlatformServiceJpaRepositoryImpl.java | 12 ++- ...WritePlatformServiceJpaRepositoryImpl.java | 12 ++- ...WritePlatformServiceJpaRepositoryImpl.java | 12 ++- ...WritePlatformServiceJpaRepositoryImpl.java | 12 ++- ...WritePlatformServiceJpaRepositoryImpl.java | 14 +++- ...WritePlatformServiceJpaRepositoryImpl.java | 12 ++- .../tenant/final-changelog-tenant.xml | 1 + ...ally_enabled_maker_checker_permissions.xml | 73 +++++++++++++++++++ .../service/CommandSourceServiceTest.java | 18 +++-- 12 files changed, 188 insertions(+), 48 deletions(-) create mode 100644 fineract-provider/src/main/resources/db/changelog/tenant/parts/0588_persist_manually_enabled_maker_checker_permissions.xml 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 591e46473..1f8ef69fa 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 @@ -137,7 +137,16 @@ public class CommandSourceService { String permission = commandSource.getPermissionCode(); boolean isMakerChecker = configurationDomainService.isMakerCheckerEnabledForTask(permission); if (isMakerChecker || result.isRollbackTransaction()) { - if (isApprovedByChecker || isSuperUserBypassAllowed(commandSource, user)) { + // isApprovedByChecker is only ever true here for a genuine checker approval + // (PortfolioCommandSourceWritePlatformServiceImpl.approveEntry(), which has already run + // validateMakerCheckerTransaction() and confirmed the approver is not the maker) or a user + // editing their own account details (CommandWrapper.isChangeOfOwnUserDetails() - maker-checker + // genuinely does not apply there). On the ORIGINAL submission this is always false and `user` + // here is always the maker themselves, so there is deliberately no CHECKER_SUPER_USER (or any + // other) bypass at this point: holding CHECKER_SUPER_USER lets a user approve someone ELSE's + // request without needing the specific _CHECKER permission, it must never let a maker + // auto-approve their own submission just by holding it. + if (isApprovedByChecker) { commandSource.markAsChecked(user); } else { if (commandSource.isSanitized()) { @@ -151,25 +160,6 @@ public class CommandSourceService { return result; } - private boolean isSuperUserBypassAllowed(CommandSource commandSource, AppUser user) { - if (!user.isCheckerSuperUser()) { - return false; - } - if ("GLACCOUNT".equalsIgnoreCase(commandSource.getEntityName()) - || "CBSBASEINTERESTRATE".equalsIgnoreCase(commandSource.getEntityName()) - || "CHARGE".equalsIgnoreCase(commandSource.getEntityName()) - || "LOANPRODUCT".equalsIgnoreCase(commandSource.getEntityName()) - || "SAVINGSPRODUCT".equalsIgnoreCase(commandSource.getEntityName()) - || "CURRENTPRODUCT".equalsIgnoreCase(commandSource.getEntityName()) - || "SHAREPRODUCT".equalsIgnoreCase(commandSource.getEntityName()) - || "FIXEDDEPOSITPRODUCT".equalsIgnoreCase(commandSource.getEntityName()) - || "USER".equalsIgnoreCase(commandSource.getEntityName()) - || "ROLE".equalsIgnoreCase(commandSource.getEntityName())) { - return false; - } - return true; - } - private void sanitizeJson(@NonNull CommandSource commandSource, Set sanitizeKeys) { if (sanitizeKeys == null || sanitizeKeys.isEmpty()) { return; 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 5406ac5b4..f4c7926ba 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 @@ -48,6 +48,12 @@ public class PortfolioCommandSourceWritePlatformServiceImpl implements Portfolio private final FromJsonHelper fromApiJsonHelper; private final CommandProcessingService processAndLogCommandService; private final SchedulerJobRunnerReadService schedulerJobRunnerReadService; + // No longer consulted by validateMakerCheckerTransaction() below: the maker-identity rule (a user + // can never approve their own submission) is now unconditional and isSameMakerCheckerEnabled() no + // longer overrides it. Left injected rather than removed - enable-same-maker-checker is still a + // live, independently-configurable global switch (System -> Configurations); deleting this field, + // and by extension deciding that switch should stop existing altogether, is a separate decision + // from closing this specific self-approval loophole. private final ConfigurationDomainService configurationService; @Override @@ -123,14 +129,20 @@ public class PortfolioCommandSourceWritePlatformServiceImpl implements Portfolio AppUser appUser = this.context.authenticatedUser(); String permissionCode = commandSource.getPermissionCode(); appUser.validateHasCheckerPermissionTo(permissionCode); - if (!configurationService.isSameMakerCheckerEnabled() && !appUser.isCheckerSuperUser()) { - AppUser maker = commandSource.getMaker(); - if (maker == null) { - throw new UnsupportedCommandException(permissionCode, "Maker user is missing."); - } - if (Objects.equals(appUser.getId(), maker.getId())) { - throw new UnsupportedCommandException(permissionCode, "Can not be checked by the same user."); - } + // Maker-identity rule: the user who submitted a command may never be the one who approves + // (or rejects) it - this is a fixed platform rule, not overridable by CHECKER_SUPER_USER (that + // permission still lets its holder action a DIFFERENT user's request without needing the + // specific _CHECKER permission - see validateHasCheckerPermissionTo() above - it just no + // longer lets them action their own) and not overridable by the enable-same-maker-checker + // global switch (isSameMakerCheckerEnabled() is deliberately no longer consulted here; that + // configuration is left in place and unused for this decision rather than removed, since + // deleting a still-configurable global setting is a separate decision from this one). + AppUser maker = commandSource.getMaker(); + if (maker == null) { + throw new UnsupportedCommandException(permissionCode, "Maker user is missing."); + } + if (Objects.equals(appUser.getId(), maker.getId())) { + throw new UnsupportedCommandException(permissionCode, "Can not be checked by the same user."); } return commandSource; } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/CurrentAccountEditWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/CurrentAccountEditWritePlatformServiceJpaRepositoryImpl.java index d0f945f84..cbb1c1bc8 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/CurrentAccountEditWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/CurrentAccountEditWritePlatformServiceJpaRepositoryImpl.java @@ -250,7 +250,11 @@ public class CurrentAccountEditWritePlatformServiceJpaRepositoryImpl implements .findByCurrentAccountIdAndStatus(currentAccountId, CurrentAccountPendingEdit.STATUS_PENDING) .orElseThrow(() -> new CurrentAccountNoPendingEditException(currentAccountId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see ShareAccountEditWritePlatformServiceJpaRepositoryImpl and + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement elsewhere. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new CurrentAccountEditSelfApprovalNotAllowedException(currentAccountId); } @@ -306,7 +310,11 @@ public class CurrentAccountEditWritePlatformServiceJpaRepositoryImpl implements .findByCurrentAccountIdAndStatus(currentAccountId, CurrentAccountPendingEdit.STATUS_PENDING) .orElseThrow(() -> new CurrentAccountNoPendingEditException(currentAccountId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see ShareAccountEditWritePlatformServiceJpaRepositoryImpl and + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement elsewhere. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new CurrentAccountEditSelfApprovalNotAllowedException(currentAccountId); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/FixedDepositProductWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/FixedDepositProductWritePlatformServiceJpaRepositoryImpl.java index 053998bf9..9cd21255f 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/FixedDepositProductWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/FixedDepositProductWritePlatformServiceJpaRepositoryImpl.java @@ -260,7 +260,11 @@ public class FixedDepositProductWritePlatformServiceJpaRepositoryImpl implements .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_FD, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsProductSelfApprovalNotAllowedException(productId); } @@ -350,7 +354,11 @@ public class FixedDepositProductWritePlatformServiceJpaRepositoryImpl implements .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_FD, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsProductSelfApprovalNotAllowedException(productId); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/RecurringDepositProductWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/RecurringDepositProductWritePlatformServiceJpaRepositoryImpl.java index 3abffe3ad..5fda9a977 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/RecurringDepositProductWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/RecurringDepositProductWritePlatformServiceJpaRepositoryImpl.java @@ -264,7 +264,11 @@ public class RecurringDepositProductWritePlatformServiceJpaRepositoryImpl implem .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_RD, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsProductSelfApprovalNotAllowedException(productId); } @@ -357,7 +361,11 @@ public class RecurringDepositProductWritePlatformServiceJpaRepositoryImpl implem .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_RD, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsProductSelfApprovalNotAllowedException(productId); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountEditWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountEditWritePlatformServiceJpaRepositoryImpl.java index ca4b88631..8d617f6f4 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountEditWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountEditWritePlatformServiceJpaRepositoryImpl.java @@ -203,7 +203,11 @@ public class SavingsAccountEditWritePlatformServiceJpaRepositoryImpl implements .findBySavingsAccountIdAndStatus(savingsId, SavingsAccountPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsAccountNoPendingEditException(savingsId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsAccountEditSelfApprovalNotAllowedException(savingsId); } @@ -265,7 +269,11 @@ public class SavingsAccountEditWritePlatformServiceJpaRepositoryImpl implements .findBySavingsAccountIdAndStatus(savingsId, SavingsAccountPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsAccountNoPendingEditException(savingsId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsAccountEditSelfApprovalNotAllowedException(savingsId); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsProductWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsProductWritePlatformServiceJpaRepositoryImpl.java index 54d4f96b7..0f94f146a 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsProductWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsProductWritePlatformServiceJpaRepositoryImpl.java @@ -420,7 +420,11 @@ public class SavingsProductWritePlatformServiceJpaRepositoryImpl implements Savi .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_SAVINGS, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsProductSelfApprovalNotAllowedException(productId); } @@ -524,7 +528,11 @@ public class SavingsProductWritePlatformServiceJpaRepositoryImpl implements Savi .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_SAVINGS, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new SavingsProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new SavingsProductSelfApprovalNotAllowedException(productId); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountEditWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountEditWritePlatformServiceJpaRepositoryImpl.java index 77d7290c9..7eecc2e66 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountEditWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountEditWritePlatformServiceJpaRepositoryImpl.java @@ -172,7 +172,12 @@ public class ShareAccountEditWritePlatformServiceJpaRepositoryImpl implements Sh .findByShareAccountIdAndStatus(shareAccountId, ShareAccountPendingEdit.STATUS_PENDING) .orElseThrow(() -> new ShareAccountNoPendingEditException(shareAccountId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - that permission still lets them approve someone ELSE's request, + // it just no longer lets them bypass the hold on their own (matches + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction(), the + // equivalent rule for the classic command pipeline). + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new ShareAccountEditSelfApprovalNotAllowedException(shareAccountId); } @@ -214,7 +219,12 @@ public class ShareAccountEditWritePlatformServiceJpaRepositoryImpl implements Sh .findByShareAccountIdAndStatus(shareAccountId, ShareAccountPendingEdit.STATUS_PENDING) .orElseThrow(() -> new ShareAccountNoPendingEditException(shareAccountId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - that permission still lets them approve someone ELSE's request, + // it just no longer lets them bypass the hold on their own (matches + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction(), the + // equivalent rule for the classic command pipeline). + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new ShareAccountEditSelfApprovalNotAllowedException(shareAccountId); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java index b2ce9c6b8..a135ebead 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java @@ -882,7 +882,11 @@ public class ShareProductWritePlatformServiceJpaRepositoryImpl implements ShareP .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_SHARE, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new ShareProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new ShareProductSelfApprovalNotAllowedException(productId); } @@ -968,7 +972,11 @@ public class ShareProductWritePlatformServiceJpaRepositoryImpl implements ShareP .findByProductTypeAndProductIdAndStatus(ProductPendingEdit.TYPE_SHARE, productId, ProductPendingEdit.STATUS_PENDING) .orElseThrow(() -> new ShareProductNoPendingEditException(productId)); - if (pendingEdit.getMaker().getId().equals(checker.getId()) && !checker.isCheckerSuperUser()) { + // Maker-identity rule: the same user can never approve their own submission, even when they + // hold CHECKER_SUPER_USER - see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction() for the + // equivalent enforcement in the classic command pipeline. + if (pendingEdit.getMaker().getId().equals(checker.getId())) { throw new ShareProductSelfApprovalNotAllowedException(productId); } diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml b/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml index d1385c84d..34bf62897 100644 --- a/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml +++ b/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml @@ -156,4 +156,5 @@ + diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/parts/0588_persist_manually_enabled_maker_checker_permissions.xml b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0588_persist_manually_enabled_maker_checker_permissions.xml new file mode 100644 index 000000000..bbee551d9 --- /dev/null +++ b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0588_persist_manually_enabled_maker_checker_permissions.xml @@ -0,0 +1,73 @@ + + + + + + + + code IN ( + 'UPDATE_CURRENCY', + 'CREATE_FUND', + 'UPDATE_WORKINGDAYS', + 'CREATE_PAYMENTTYPE', 'UPDATE_PAYMENTTYPE', 'DELETE_PAYMENTTYPE', + 'CREATE_SPONSORBANK', 'UPDATE_SPONSORBANK', + 'UPDATE_BRANCHPROFILE', + 'CREATE_DATATABLE', 'UPDATE_DATATABLE', 'DELETE_DATATABLE', + 'CREATE_CODE', 'UPDATE_CODE', 'DELETE_CODE', + 'CREATE_CODEVALUE', 'UPDATE_CODEVALUE', 'DELETE_CODEVALUE', + 'CREATE_REPORT', 'UPDATE_REPORT', 'DELETE_REPORT', + 'CREATE_HOOK', 'UPDATE_HOOK', 'DELETE_HOOK', + 'CREATE_ACCOUNTNUMBERFORMAT', 'DELETE_ACCOUNTNUMBERFORMAT', + 'CREATE_USERAGREEMENT', + 'UPDATE_BATCH_BUSINESS_STEP' + ) + + + + diff --git a/fineract-provider/src/test/java/org/apache/fineract/commands/service/CommandSourceServiceTest.java b/fineract-provider/src/test/java/org/apache/fineract/commands/service/CommandSourceServiceTest.java index c4db9dadf..ee634c61c 100644 --- a/fineract-provider/src/test/java/org/apache/fineract/commands/service/CommandSourceServiceTest.java +++ b/fineract-provider/src/test/java/org/apache/fineract/commands/service/CommandSourceServiceTest.java @@ -116,7 +116,11 @@ public class CommandSourceServiceTest { } @Test - public void testProcessCommand_NonGLAccount_SuperUser_BypassesMakerChecker() { + public void testProcessCommand_NonGLAccount_SuperUser_OwnSubmission_DoesNotBypassMakerChecker() { + // A maker who happens to hold CHECKER_SUPER_USER must not auto-approve their own submission - + // CHECKER_SUPER_USER only ever lets its holder approve a DIFFERENT user's request (see + // PortfolioCommandSourceWritePlatformServiceImpl.validateMakerCheckerTransaction()). This + // replaces the old bypass this test used to assert. NewCommandSourceHandler handler = Mockito.mock(NewCommandSourceHandler.class); JsonCommand jsonCommand = Mockito.mock(JsonCommand.class); CommandSource commandSource = Mockito.mock(CommandSource.class); @@ -125,6 +129,8 @@ public class CommandSourceServiceTest { Mockito.when(commandSource.getPermissionCode()).thenReturn("WITHDRAW_SAVINGSACCOUNT"); Mockito.when(commandSource.getEntityName()).thenReturn("SAVINGSACCOUNT"); Mockito.when(commandSource.getActionName()).thenReturn("WITHDRAW"); + Mockito.when(commandSource.getId()).thenReturn(300L); + Mockito.when(commandSource.getResourceId()).thenReturn(400L); Mockito.when(configurationDomainService.isMakerCheckerEnabledForTask("WITHDRAW_SAVINGSACCOUNT")).thenReturn(true); Mockito.when(superUser.isCheckerSuperUser()).thenReturn(true); @@ -132,12 +138,12 @@ public class CommandSourceServiceTest { .withCommandId(1L).build(); Mockito.when(handler.processCommand(jsonCommand)).thenReturn(dummyResult); - org.apache.fineract.infrastructure.core.data.CommandProcessingResult result = underTest.processCommand(handler, jsonCommand, - commandSource, superUser, false); + Assertions.assertThrows(org.apache.fineract.commands.exception.RollbackTransactionNotApprovedException.class, () -> { + underTest.processCommand(handler, jsonCommand, commandSource, superUser, false); + }); - Assertions.assertNotNull(result); - Mockito.verify(commandSource).markAsChecked(superUser); - Mockito.verify(commandSource, Mockito.never()).markAsAwaitingApproval(); + Mockito.verify(commandSource).markAsAwaitingApproval(); + Mockito.verify(commandSource, Mockito.never()).markAsChecked(any()); } @Test From 11a0d502be34f95a58f6694d3f2b2eda94510d95 Mon Sep 17 00:00:00 2001 From: anshul Date: Thu, 1 Oct 2026 12:03:32 +0530 Subject: [PATCH 2/2] Enforce same-user maker-checker restriction --- .../parts/0012_seed_default_platform_api_client.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fineract-provider/src/main/resources/db/changelog/tenant-store/parts/0012_seed_default_platform_api_client.xml b/fineract-provider/src/main/resources/db/changelog/tenant-store/parts/0012_seed_default_platform_api_client.xml index a75be07a2..b10999153 100644 --- a/fineract-provider/src/main/resources/db/changelog/tenant-store/parts/0012_seed_default_platform_api_client.xml +++ b/fineract-provider/src/main/resources/db/changelog/tenant-store/parts/0012_seed_default_platform_api_client.xml @@ -27,17 +27,17 @@ #/platform-admin/menu-access works out of the box the first time the app is stood up, without anyone needing shell/DB access to mint the first key by hand. It is deliberately the SAME raw value on every deployment/tenant (requested behaviour) rather than a random - one generated per install -- the trade-off, on purpose: this value is visible to anyone who + one generated per install - the trade-off, on purpose: this value is visible to anyone who can read this source file, exactly like a default admin password shipped in a seed script. It is a bootstrap convenience, not a secret. Rotate it for any deployment that is not purely local/dev: insert a new row with a real random key's hash (see ApiClientCredentialService's - hashing) and either deactivate or DELETE this row -- POST /v1/apiclients is intentionally + hashing) and either deactivate or DELETE this row - POST /v1/apiclients is intentionally disabled (see ApiClientAdminApiResource), so rotation is a direct m_api_client write, same as this seed. Raw key (only shown here, since it is fixed and not a secret to rotate away from): cbsx_default_platform_operator_key_2026 - api_key_hash below is SHA-256 of that exact string, hex-encoded, lowercase -- matching + api_key_hash below is SHA-256 of that exact string, hex-encoded, lowercase - matching ApiClientCredentialService#sha256Hex. -->