Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
11a0d502be | ||
|
|
2afd4aaf83 | ||
|
|
e5dd6af366 | ||
|
|
9436eab129 | ||
|
|
1408bddf58 | ||
|
|
3a37a5f90f | ||
|
|
e7417d16a6 |
+10
-20
@@ -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 <TASK>_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<String> sanitizeKeys) {
|
||||
if (sanitizeKeys == null || sanitizeKeys.isEmpty()) {
|
||||
return;
|
||||
|
||||
+20
-8
@@ -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 <TASK>_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;
|
||||
}
|
||||
|
||||
+5
-3
@@ -165,9 +165,11 @@ public class SecurityConfig {
|
||||
// envelope's RSA signature/encryption (NpciCryptoUtil) inside the handler.
|
||||
.requestMatchers(API_MATCHER.matcher(HttpMethod.POST, "/api/*/npci/**")).permitAll()
|
||||
// Client account summary, third-party callers — a partner integration calls this
|
||||
// with an X-Api-Key header instead of a Fineract username/password (issued via
|
||||
// POST /v1/apiclients, super-user only). Same posture as IMPS/NPCI above: the
|
||||
// outer gate stays open for this one narrow GET path, and
|
||||
// with an X-Api-Key header instead of a Fineract username/password. The key itself
|
||||
// is issued by inserting directly into m_api_client in the tenant-store database
|
||||
// (see ApiClientAdminApiResource — its POST /v1/apiclients is deliberately disabled,
|
||||
// since any bank's own super user reaching it could mint a platform-wide key). Same
|
||||
// posture as IMPS/NPCI above: the outer gate stays open for this one narrow GET path, and
|
||||
// ClientAllAccountsApiResource#retrieveByClientNo is what actually authenticates
|
||||
// the caller (ApiClientCredentialService checks the key's hash, active flag and
|
||||
// IP allowlist) — a request with no key and no Fineract login still gets nothing,
|
||||
|
||||
+20
-14
@@ -20,7 +20,6 @@ package org.apache.fineract.infrastructure.security.apiclient;
|
||||
|
||||
import com.google.gson.Gson;
|
||||
import com.google.gson.GsonBuilder;
|
||||
import com.google.gson.JsonElement;
|
||||
import io.swagger.v3.oas.annotations.tags.Tag;
|
||||
import jakarta.ws.rs.Consumes;
|
||||
import jakarta.ws.rs.DELETE;
|
||||
@@ -35,17 +34,26 @@ import jakarta.ws.rs.core.Response;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
|
||||
import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
|
||||
import org.apache.fineract.useradministration.domain.AppUser;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
/**
|
||||
* Admin-only management of the third-party API keys {@link ApiClientCredentialService} authenticates.
|
||||
* This resource itself requires a normal Fineract login, same as everything else -- ordinary
|
||||
* username/password, restricted to a super user (ALL_FUNCTIONS). Only the endpoints that use a minted
|
||||
* key to skip login (e.g. GET /v1/clientaccounts) are the ones exempted in SecurityConfig; issuing that
|
||||
* key in the first place is not something to hand to just anyone.
|
||||
* List/deactivate/reactivate still require a normal Fineract login restricted to a super user
|
||||
* (ALL_FUNCTIONS) -- but minting a NEW key ({@link #create}) is disabled here entirely, on purpose.
|
||||
*
|
||||
* {@code ALL_FUNCTIONS} is a per-TENANT role grant -- and {@code TenantMenuAccessApiResource}'s whole
|
||||
* point is that a bank's own super user (or a role simply misconfigured to carry ALL_FUNCTIONS, as the
|
||||
* "Clerk" role was found to in two tenants this session) must never be able to lift its own menu-access
|
||||
* ceiling. If this endpoint minted keys for any ALL_FUNCTIONS holder, that same bank super user could
|
||||
* call {@code POST /v1/apiclients} and hand themselves a brand-new platform-operator credential -- one
|
||||
* that authenticates against every tenant, not just their own -- completely bypassing the ceiling this
|
||||
* key mechanism exists to enforce. A super user being able to read/deactivate the client list is fine
|
||||
* (visibility and revocation are not privilege escalation); being able to mint a fresh one is exactly
|
||||
* the escalation path being closed. New keys are issued the same way this fix issued the first one: a
|
||||
* direct insert into {@code m_api_client} (see {@link ApiClientCredentialService#createClient}), by
|
||||
* whoever already has direct database access -- never over HTTP.
|
||||
*/
|
||||
@Path("/v1/apiclients")
|
||||
@Component
|
||||
@@ -55,20 +63,18 @@ public class ApiClientAdminApiResource {
|
||||
|
||||
private final PlatformSecurityContext context;
|
||||
private final ApiClientCredentialService credentialService;
|
||||
private final FromJsonHelper fromJsonHelper;
|
||||
private final Gson gson = new GsonBuilder().serializeNulls().create();
|
||||
|
||||
/** Disabled -- see the class comment. Minting a key is a direct-database operation only. */
|
||||
@POST
|
||||
@Consumes(MediaType.APPLICATION_JSON)
|
||||
@Produces(MediaType.APPLICATION_JSON)
|
||||
public String create(final String body) {
|
||||
final AppUser user = requireSuperUser();
|
||||
final JsonElement root = fromJsonHelper.parse(body);
|
||||
final String clientName = fromJsonHelper.extractStringNamed("clientName", root);
|
||||
final String allowedIps = fromJsonHelper.extractStringNamed("allowedIps", root);
|
||||
|
||||
final Map<String, Object> result = credentialService.createClient(clientName, allowedIps, user.getId());
|
||||
return gson.toJson(result);
|
||||
requireSuperUser();
|
||||
throw new WebApplicationException(Response.status(403)
|
||||
.entity("{\"message\":\"Minting a new platform API key over HTTP is disabled. "
|
||||
+ "Insert directly into m_api_client in the tenant-store database instead.\"}")
|
||||
.type(MediaType.APPLICATION_JSON).build());
|
||||
}
|
||||
|
||||
@GET
|
||||
|
||||
+10
-2
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+10
-2
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+10
-2
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+10
-2
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+10
-2
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+12
-2
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+10
-2
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+1
@@ -32,4 +32,5 @@
|
||||
<include file="parts/0009_set_and_encrypt_ro_if_not_exists.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0010_set_datetime_precision.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0011_create_api_client.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0012_seed_default_platform_api_client.xml" relativeToChangelogFile="true"/>
|
||||
</databaseChangeLog>
|
||||
|
||||
+62
@@ -0,0 +1,62 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!--
|
||||
|
||||
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.
|
||||
|
||||
-->
|
||||
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
|
||||
|
||||
<!-- Seeds ONE fixed, well-known platform-operator API key on every deployment, so
|
||||
#/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
|
||||
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
|
||||
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
|
||||
ApiClientCredentialService#sha256Hex. -->
|
||||
<changeSet id="0012_seed_default_platform_api_client" author="cbs-nitesh">
|
||||
<preConditions onFail="MARK_RAN">
|
||||
<and>
|
||||
<tableExists tableName="m_api_client"/>
|
||||
<sqlCheck expectedResult="0">
|
||||
SELECT COUNT(*) FROM m_api_client
|
||||
WHERE api_key_hash = '9d6d4dfa7a1953e33e0acde4fd775aad11689ec540340c7224796447118a3edd'
|
||||
</sqlCheck>
|
||||
</and>
|
||||
</preConditions>
|
||||
<insert tableName="m_api_client">
|
||||
<column name="client_name" value="Default Platform Operator (rotate before real production use)"/>
|
||||
<column name="api_key_hash" value="9d6d4dfa7a1953e33e0acde4fd775aad11689ec540340c7224796447118a3edd"/>
|
||||
<column name="api_key_prefix" value="cbsx_default"/>
|
||||
<column name="allowed_ips"/>
|
||||
<column name="created_by"/>
|
||||
<column name="created_by_tenant" value="ALL"/>
|
||||
</insert>
|
||||
</changeSet>
|
||||
|
||||
</databaseChangeLog>
|
||||
@@ -145,7 +145,6 @@
|
||||
<include file="parts/0583_save_rbi_form_a_return_job.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0584_form9_slr_assets.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0582_create_tds_config.xml" relativeToChangelogFile="true"/>
|
||||
<<<<<<< HEAD
|
||||
<!-- Our local 0575 Loan Originator checker-permission migration collided with dev-latest's own
|
||||
0575_rbi_reserve_custom_roles.xml once merged (it was only stashed/uncommitted locally, so it
|
||||
never appeared on either side of this merge as a real conflict - the collision only became
|
||||
@@ -156,7 +155,6 @@
|
||||
<include file="parts/0584_add_manage_survey_permissions.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0585_add_loan_cob_catchup_permission.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0586_add_system_module_checker_permissions.xml" relativeToChangelogFile="true"/>
|
||||
=======
|
||||
<include file="parts/0585_fix_current_product_od_scheme_pending_columns.xml" relativeToChangelogFile="true"/>
|
||||
>>>>>>> dev-latest
|
||||
<include file="parts/0587_fix_current_product_od_scheme_pending_columns.xml" relativeToChangelogFile="true"/>
|
||||
<include file="parts/0588_persist_manually_enabled_maker_checker_permissions.xml" relativeToChangelogFile="true"/>
|
||||
</databaseChangeLog>
|
||||
|
||||
+2
-2
@@ -31,7 +31,7 @@
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
|
||||
|
||||
<changeSet author="CBS" id="0585-1-fix-saving-product-temp-od-min-limit">
|
||||
<changeSet author="CBS" id="0587-1-fix-saving-product-temp-od-min-limit">
|
||||
<preConditions onFail="MARK_RAN">
|
||||
<and>
|
||||
<tableExists tableName="m_saving_product_temp"/>
|
||||
@@ -46,7 +46,7 @@
|
||||
</addColumn>
|
||||
</changeSet>
|
||||
|
||||
<changeSet author="CBS" id="0585-2-fix-saving-product-temp-od-excess-drawing">
|
||||
<changeSet author="CBS" id="0587-2-fix-saving-product-temp-od-excess-drawing">
|
||||
<preConditions onFail="MARK_RAN">
|
||||
<and>
|
||||
<tableExists tableName="m_saving_product_temp"/>
|
||||
+73
@@ -0,0 +1,73 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
|
||||
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
|
||||
|
||||
<!-- Converts manually-applied Configure Maker Checker Tasks changes into a repeatable migration.
|
||||
During Admin -> Organization and Admin -> System testing, can_maker_checker was switched on
|
||||
(via System -> Configure Maker Checker Tasks) for the 28 permissions below, directly in the
|
||||
dev database - none of that was ever captured by a migration, so a fresh/other environment
|
||||
restored from this changelog history would come up with every one of them still at its
|
||||
originally-seeded false value. Every code here was independently confirmed, via read-only
|
||||
inspection of both its origin changeset and the live database, to be seeded can_maker_checker
|
||||
= false and currently true in the running dev DB with no existing changeset already covering
|
||||
the flip (the one precedent for this pattern, DELETE_OFFICE, was flipped by changeset
|
||||
0538_enable_maker_checker_for_delete_office and is deliberately left untouched here).
|
||||
|
||||
This changeset only ever moves can_maker_checker from false to true for these exact codes -
|
||||
entity_name, action_name, grouping and every other column are left exactly as their origin
|
||||
changeset defined them (including the pre-existing DELETE_ACCOUNTNUMBERFORMAT entity_name
|
||||
drift, which is out of scope for this change and is not corrected here). No _CHECKER row is
|
||||
touched: those already exist for every code below (added earlier by 0583/0586, or by base
|
||||
Fineract for the stock codes), and can_maker_checker is not a column on a _CHECKER row.
|
||||
|
||||
Grouped by the screen each permission belongs to:
|
||||
- Currency Configuration: UPDATE_CURRENCY
|
||||
- Manage Funds: CREATE_FUND
|
||||
- Working Days: UPDATE_WORKINGDAYS
|
||||
- Payment Type: CREATE_PAYMENTTYPE, UPDATE_PAYMENTTYPE, DELETE_PAYMENTTYPE
|
||||
- Sponsor Banks: CREATE_SPONSORBANK, UPDATE_SPONSORBANK
|
||||
- Branch Profile: UPDATE_BRANCHPROFILE
|
||||
- Manage Data Tables: CREATE_DATATABLE, UPDATE_DATATABLE, DELETE_DATATABLE
|
||||
- Manage Codes: CREATE_CODE, UPDATE_CODE, DELETE_CODE,
|
||||
CREATE_CODEVALUE, UPDATE_CODEVALUE, DELETE_CODEVALUE
|
||||
- Manage Reports: CREATE_REPORT, UPDATE_REPORT, DELETE_REPORT
|
||||
- Manage Hooks: CREATE_HOOK, UPDATE_HOOK, DELETE_HOOK
|
||||
- Account Number Preferences: CREATE_ACCOUNTNUMBERFORMAT, DELETE_ACCOUNTNUMBERFORMAT
|
||||
- Manage User Agreement: CREATE_USERAGREEMENT
|
||||
- Manage Jobs (COB Workflow Step Reorder): UPDATE_BATCH_BUSINESS_STEP
|
||||
|
||||
Deliberately NOT included (confirmed still false, or out of scope, and left untouched):
|
||||
- UPDATE_PERMISSION, UPDATE_CONFIGURATION (the Configure Maker Checker Tasks self-lockout
|
||||
exclusion added by PermissionReadPlatformServiceImpl stays in effect; these remain false)
|
||||
- UPDATE_SPONSORDDGLMAPPING, all Entity-to-Entity Mapping, External Services and Manage
|
||||
Survey permissions (confirmed still false - never actually toggled on)
|
||||
- CREATE_ROLE/UPDATE_ROLE/DELETE_ROLE/etc., CREATE_GLACCOUNT, CREATE_CHARGE,
|
||||
CREATE_LOANPRODUCT, CREATE_SAVINGSPRODUCT and other permissions observed to already be
|
||||
true in the live DB but belonging to Accounting/Products/Roles modules, not Admin ->
|
||||
Organization or Admin -> System - out of scope for this change and left as pre-existing
|
||||
drift for a separate decision. -->
|
||||
<changeSet id="0588_persist_manually_enabled_maker_checker_permissions" author="cbs-anshul">
|
||||
<update tableName="m_permission">
|
||||
<column name="can_maker_checker" valueBoolean="true"/>
|
||||
<where>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'
|
||||
)</where>
|
||||
</update>
|
||||
</changeSet>
|
||||
|
||||
</databaseChangeLog>
|
||||
+12
-6
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user