This is an automated email from the ASF dual-hosted git repository. mridulpathak pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
commit f3207bdff6dc99f1205601a7ad910bdc6dad98e7 Author: Mridul Pathak <[email protected]> AuthorDate: Mon Oct 12 15:18:41 2026 +0530 Add permission checks to FinAccount create/update/delete services Several Financial Account services (createFinAccount, updateFinAccount, deleteFinAccount, createFinAccountTrans, createFinAccountRole, createFinAccountAuth, setFinAccountTransStatus, depositWithdrawPayments) had neither auth nor a permission-service, so any authenticated session could call them directly. Wire the existing (previously UI-only) acctgFinAcctPermissionCheck permission-service into each of these service definitions, matching the convention already used for sibling services in this file (updateFinAccountRole, deleteFinAccountRole). Several internal callers invoke these services as a trusted side effect of an already-validated operation (gift certificate purchase/reload/ redemption, financial-account payment authorization/capture, auto- replenish, and return/store-credit processing), using the initiating customer's or CSR's own userLogin. Elevate those specific calls to the system user, mirroring the existing pattern in GiftCertificateServices#createGiftCertificate, so the new permission checks don't block legitimate checkout and return flows: - FinAccountPaymentServices: financial-account auth/capture/withdraw/ deposit/replenish - GiftCertificateServices: gift certificate creation, reload, redemption, and authorization - OrderReturnServices: store-credit creation on return processing - FinAccountProductServices: financial-account purchase fulfillment --- .../accounting/servicedef/services_finaccount.xml | 24 ++++++++++----- .../finaccount/FinAccountPaymentServices.java | 34 +++++++++++++++++----- .../finaccount/FinAccountProductServices.java | 9 ++++-- .../payment/GiftCertificateServices.java | 17 +++++++++-- .../ofbiz/order/order/OrderReturnServices.java | 16 +++++++--- 5 files changed, 74 insertions(+), 26 deletions(-) diff --git a/applications/accounting/servicedef/services_finaccount.xml b/applications/accounting/servicedef/services_finaccount.xml index 305b46b201..31dc81f72e 100644 --- a/applications/accounting/servicedef/services_finaccount.xml +++ b/applications/accounting/servicedef/services_finaccount.xml @@ -25,18 +25,20 @@ under the License. <version>1.0</version> <!-- Financial Account --> - <service name="createFinAccount" engine="simple" default-entity-name="FinAccount" + <service name="createFinAccount" engine="simple" default-entity-name="FinAccount" auth="true" location="component://accounting/minilang/finaccount/FinAccountServices.xml" invoke="createFinAccount"> <description>Create a new Financial Account. If no finAccountId is provided, an auto-sequenced one will be used.</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="CREATE"/> <auto-attributes mode="INOUT" optional="true" include="pk"/> <auto-attributes mode="IN" optional="true" include="nonpk"> <exclude field-name="actualBalance"/> <exclude field-name="availableBalance"/> </auto-attributes> </service> - <service name="updateFinAccount" engine="simple" default-entity-name="FinAccount" + <service name="updateFinAccount" engine="simple" default-entity-name="FinAccount" auth="true" location="component://accounting/minilang/finaccount/FinAccountServices.xml" invoke="updateFinAccount"> <description>Update a Financial Account</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="UPDATE"/> <auto-attributes mode="INOUT" optional="false" include="pk"/> <auto-attributes mode="IN" optional="true" include="nonpk"> <exclude field-name="actualBalance"/> @@ -48,8 +50,9 @@ under the License. <attribute name="replenishPaymentId" type="String" mode="OUT" optional="true"/> <attribute name="replenishLevel" type="BigDecimal" mode="OUT" optional="true"/> </service> - <service name="deleteFinAccount" default-entity-name="FinAccount" engine="entity-auto" invoke="expire"> + <service name="deleteFinAccount" default-entity-name="FinAccount" engine="entity-auto" invoke="expire" auth="true"> <description>Delete a Financial Account</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="DELETE"/> <auto-attributes mode="IN" optional="false" include="pk"/> </service> <service name="updateFinAccountBalancesFromTrans" engine="simple" @@ -72,9 +75,10 @@ under the License. </service> <!-- Financial Account Transaction --> - <service name="createFinAccountTrans" engine="simple" default-entity-name="FinAccountTrans" + <service name="createFinAccountTrans" engine="simple" default-entity-name="FinAccountTrans" auth="true" location="component://accounting/minilang/finaccount/FinAccountServices.xml" invoke="createFinAccountTrans"> <description>Create a new Financial Account Transaction. Will use current timestamp for entryDate and trasanctionDate if none is provided.</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="CREATE"/> <auto-attributes mode="OUT" optional="false" include="pk"/> <auto-attributes mode="IN" optional="true" include="nonpk"> <exclude field-name="performedByPartyId"/> @@ -88,9 +92,10 @@ under the License. <attribute name="glAccountId" type="String" mode="IN" optional="false"/> </service> - <service name="createFinAccountRole" engine="simple" default-entity-name="FinAccountRole" + <service name="createFinAccountRole" engine="simple" default-entity-name="FinAccountRole" auth="true" location="component://accounting/minilang/finaccount/FinAccountServices.xml" invoke="createFinAccountRole"> <description>Create a new Financial Account Role</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="CREATE"/> <auto-attributes mode="IN" optional="false" include="pk"/> <auto-attributes mode="IN" optional="true" include="nonpk"/> <override name="fromDate" optional="true"/> @@ -105,9 +110,10 @@ under the License. <auto-attributes include="pk" mode="IN" optional="false"/> </service> - <service name="createFinAccountAuth" engine="simple" default-entity-name="FinAccountAuth" + <service name="createFinAccountAuth" engine="simple" default-entity-name="FinAccountAuth" auth="true" location="component://accounting/minilang/finaccount/FinAccountServices.xml" invoke="createFinAccountAuth"> <description>Lower level service for creating authorization against a fin account. Will use current time for authorizationDate and thruDate if not supplied.</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="CREATE"/> <auto-attributes mode="OUT" optional="false" include="pk"/> <auto-attributes mode="IN" optional="true" include="nonpk"/> </service> @@ -119,9 +125,10 @@ under the License. <attribute name="expireDateTime" type="Timestamp" mode="IN" optional="true"/> </service> - <service name="setFinAccountTransStatus" engine="simple" + <service name="setFinAccountTransStatus" engine="simple" auth="true" location="component://accounting/minilang/finaccount/FinAccountServices.xml" invoke="setFinAccountTransStatus"> <description>Set financial account transaction status</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="UPDATE"/> <attribute name="finAccountTransId" type="String" mode="IN" optional="true"/> <attribute name="statusId" type="String" mode="IN" optional="true"/> <attribute name="oldStatusId" type="String" mode="OUT" optional="true"/> @@ -447,9 +454,10 @@ under the License. <implements service="itemFulfillmentInterface"/> </service> - <service name="depositWithdrawPayments" engine="simple" + <service name="depositWithdrawPayments" engine="simple" auth="true" location="component://accounting/minilang/finaccount/FinAccountServices.xml" invoke="depositWithdrawPayments"> <description>Deposit withdraw payments</description> + <permission-service service-name="acctgFinAcctPermissionCheck" main-action="CREATE"/> <attribute name="paymentIds" type="List" mode="IN" optional="false"/> <attribute name="finAccountId" type="String" mode="IN" optional="false"/> <attribute name="groupInOneTransaction" type="String" mode="IN" optional="true"/> diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountPaymentServices.java b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountPaymentServices.java index a13febf8c5..775c27459f 100644 --- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountPaymentServices.java +++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountPaymentServices.java @@ -262,8 +262,11 @@ public class FinAccountPaymentServices { thruDate = UtilDateTime.getDayEnd(UtilDateTime.nowTimestamp(), 30L); // default 30 days for an auth } + // createFinAccountAuth requires accounting permission; run as the "system" user since this is a trusted + // side effect of an already-validated financial-account payment authorization + GenericValue systemUserLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system").cache().queryOne(); Map<String, Object> tmpResult = dispatcher.runSync("createFinAccountAuth", UtilMisc.<String, Object>toMap("finAccountId", - finAccountId, "amount", amount, "thruDate", thruDate, "userLogin", userLogin)); + finAccountId, "amount", amount, "thruDate", thruDate, "userLogin", systemUserLogin)); if (ServiceUtil.isError(tmpResult)) { return ServiceUtil.returnError(ServiceUtil.getErrorMessage(tmpResult)); @@ -712,10 +715,14 @@ public class FinAccountPaymentServices { // balance went below zero, set negative pending replenishment status so that no // more auths or captures will go through until it is replenished try { - Map<String, Object> rollbackCtx = UtilMisc.toMap("userLogin", userLogin, "finAccountId", + // updateFinAccount requires accounting permission; run as the "system" user since this is a + // trusted side effect of an already-validated financial-account deposit + GenericValue systemUserLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system") + .cache().queryOne(); + Map<String, Object> rollbackCtx = UtilMisc.toMap("userLogin", systemUserLogin, "finAccountId", finAccountId, "statusId", "FNACT_NEGPENDREPL"); dispatcher.addRollbackService("updateFinAccount", rollbackCtx, true); - } catch (GenericServiceException e) { + } catch (GenericEntityException | GenericServiceException e) { Debug.logError(e, MODULE); return ServiceUtil.returnError(e.getMessage()); } @@ -828,10 +835,14 @@ public class FinAccountPaymentServices { // configure rollback service to set status to Negative Pending Replenishment if ("FNACT_NEGPENDREPL".equals(statusId)) { try { - Map<String, Object> rollbackCtx = UtilMisc.toMap("userLogin", userLogin, "finAccountId", finAccountId, + // updateFinAccount requires accounting permission; run as the "system" user since this is a + // trusted side effect of an already-validated auto-replenish operation + GenericValue systemUserLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system") + .cache().queryOne(); + Map<String, Object> rollbackCtx = UtilMisc.toMap("userLogin", systemUserLogin, "finAccountId", finAccountId, "statusId", "FNACT_NEGPENDREPL"); dispatcher.addRollbackService("updateFinAccount", rollbackCtx, true); - } catch (GenericServiceException e) { + } catch (GenericEntityException | GenericServiceException e) { Debug.logError(e, MODULE); return ServiceUtil.returnError(e.getMessage()); } @@ -930,12 +941,16 @@ public class FinAccountPaymentServices { // say we are in good standing again if ("FNACT_NEGPENDREPL".equals(statusId)) { try { + // updateFinAccount requires accounting permission; run as the "system" user since this is a + // trusted side effect of an already-validated auto-replenish operation + GenericValue systemUserLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system") + .cache().queryOne(); Map<String, Object> ufaResp = dispatcher.runSync("updateFinAccount", - UtilMisc.<String, Object>toMap("finAccountId", finAccountId, "statusId", "FNACT_ACTIVE", "userLogin", userLogin)); + UtilMisc.<String, Object>toMap("finAccountId", finAccountId, "statusId", "FNACT_ACTIVE", "userLogin", systemUserLogin)); if (ServiceUtil.isError(ufaResp)) { return ServiceUtil.returnError(ServiceUtil.getErrorMessage(ufaResp)); } - } catch (GenericServiceException e) { + } catch (GenericEntityException | GenericServiceException e) { Debug.logError(e, MODULE); return ServiceUtil.returnError(e.getMessage()); } @@ -1058,7 +1073,10 @@ public class FinAccountPaymentServices { transCtx.put("orderItemSeqId", orderItemSeqId); transCtx.put("reasonEnumId", reasonEnumId); transCtx.put("amount", amount); - transCtx.put("userLogin", userLogin); + // createFinAccountTrans requires accounting permission; run as the "system" user since this transaction is + // a trusted side effect of an already-validated financial-account payment (the partyId above still records + // the real party the transaction is with) + transCtx.put("userLogin", EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system").cache().queryOne()); transCtx.put("paymentId", paymentId); Map<String, Object> transResult; diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountProductServices.java b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountProductServices.java index 40450d3a6f..edb0b177fb 100644 --- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountProductServices.java +++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountProductServices.java @@ -228,12 +228,15 @@ public class FinAccountProductServices { roleCtx.put("partyId", partyId); roleCtx.put("roleTypeId", "OWNER"); roleCtx.put("finAccountId", finAccountId); - roleCtx.put("userLogin", userLogin); - roleCtx.put("fromDate", UtilDateTime.nowTimestamp()); + // createFinAccountRole requires accounting permission; run as the "system" user since this is a trusted + // side effect of an already-validated financial-account purchase fulfillment Map<String, Object> roleResp; try { + GenericValue rolePermUserLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system").cache().queryOne(); + roleCtx.put("userLogin", rolePermUserLogin); + roleCtx.put("fromDate", UtilDateTime.nowTimestamp()); roleResp = dispatcher.runSync("createFinAccountRole", roleCtx); - } catch (GenericServiceException e) { + } catch (GenericEntityException | GenericServiceException e) { Debug.logError(e, MODULE); return ServiceUtil.returnError(e.getMessage()); } diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java index e5464a6f36..a1351aa958 100644 --- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java +++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java @@ -108,11 +108,15 @@ public class GiftCertificateServices { finAccountId = cardNumber; // create the FinAccount + // createFinAccount requires accounting permission; run as the "system" user since this is a + // trusted side effect of an already-validated gift certificate purchase + GenericValue createFaPermUserLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system") + .cache().queryOne(); Map<String, Object> acctCtx = UtilMisc.<String, Object>toMap("finAccountId", finAccountId); acctCtx.put("finAccountTypeId", FinAccountHelper.getGiftCertFinAccountTypeId()); acctCtx.put("finAccountName", accountName); acctCtx.put("finAccountCode", pinNumber); - acctCtx.put("userLogin", userLogin); + acctCtx.put("userLogin", createFaPermUserLogin); acctResult = dispatcher.runSync("createFinAccount", acctCtx); if (ServiceUtil.isError(acctResult)) { return ServiceUtil.returnError(ServiceUtil.getErrorMessage(acctResult)); @@ -544,10 +548,14 @@ public class GiftCertificateServices { if (giftCertSettings.getLong("authValidDays") != null) { thruDate = UtilDateTime.getDayEnd(UtilDateTime.nowTimestamp(), giftCertSettings.getLong("authValidDays")); } + // createFinAccountAuth requires accounting permission; run as the "system" user since this is a + // trusted side effect of an already-validated gift certificate authorization + GenericValue authPermUserLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system") + .cache().queryOne(); Map<String, Object> tmpResult = dispatcher.runSync("createFinAccountAuth", UtilMisc.<String, Object>toMap("finAccountId", finAccountId, "amount", amount, "currencyUomId", currency, - "thruDate", thruDate, "userLogin", userLogin)); + "thruDate", thruDate, "userLogin", authPermUserLogin)); if (ServiceUtil.isError(tmpResult)) { return ServiceUtil.returnError(ServiceUtil.getErrorMessage(tmpResult)); } else { @@ -1398,7 +1406,10 @@ public class GiftCertificateServices { Map<String, Object> transCtx = UtilMisc.<String, Object>toMap("finAccountTransTypeId", txType); transCtx.put("finAccountId", finAccountId); transCtx.put("partyId", userLogin.getString("partyId")); - transCtx.put("userLogin", userLogin); + // createFinAccountTrans requires accounting permission; run as the "system" user since this transaction is + // a trusted side effect of an already-validated gift certificate operation (the partyId above still records + // the real party the transaction is with) + transCtx.put("userLogin", EntityQuery.use(delegator).from("UserLogin").where("userLoginId", "system").cache().queryOne()); transCtx.put("paymentId", paymentId); transCtx.put("orderId", orderId); transCtx.put("amount", amount); diff --git a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java index 3a2844922e..015fc188c5 100644 --- a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java +++ b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java @@ -867,9 +867,13 @@ public class OrderReturnServices { Map<String, Object> finAccountRoleResult = null; try { + // createFinAccountRole requires accounting permission; run as the "system" user since this is a + // trusted side effect of an already-validated return/store-credit operation + GenericValue finAccountRolePermUserLogin = EntityQuery.use(delegator).from("UserLogin") + .where("userLoginId", "system").cache().queryOne(); finAccountRoleResult = dispatcher.runSync("createFinAccountRole", UtilMisc.toMap("finAccountId", - finAccountId, "partyId", fromPartyId, "roleTypeId", "OWNER", "userLogin", userLogin)); - } catch (GenericServiceException e) { + finAccountId, "partyId", fromPartyId, "roleTypeId", "OWNER", "userLogin", finAccountRolePermUserLogin)); + } catch (GenericEntityException | GenericServiceException e) { Debug.logError(e, "Problem running the createFinAccountRole service", MODULE); return ServiceUtil.returnError(UtilProperties.getMessage(RES_ERROR, "OrderProblemCreatingFinAccountRoleRecord", locale)); @@ -907,10 +911,14 @@ public class OrderReturnServices { if (finAccountId != null) { Map<String, Object> finAccountTransResult = null; try { + // createFinAccountTrans requires accounting permission; run as the "system" user since this is a + // trusted side effect of an already-validated return/store-credit operation + GenericValue finAccountTransPermUserLogin = EntityQuery.use(delegator).from("UserLogin") + .where("userLoginId", "system").cache().queryOne(); finAccountTransResult = dispatcher.runSync("createFinAccountTrans", UtilMisc.toMap("finAccountId", finAccountId, "finAccountTransTypeId", "DEPOSIT", "partyId", toPartyId, "amount", creditTotal, "reasonEnumId", "FATR_REFUND", - "userLogin", userLogin)); - } catch (GenericServiceException e) { + "userLogin", finAccountTransPermUserLogin)); + } catch (GenericEntityException | GenericServiceException e) { Debug.logError(e, "Problem creating FinAccountTrans record", MODULE); return ServiceUtil.returnError(UtilProperties.getMessage(RES_ERROR, "OrderProblemCreatingFinAccountTransRecord", locale));
