You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by mr...@apache.org on 2026/10/13 07:59:33 UTC

(ofbiz-framework) branch trunk updated (98e498f9a2 -> 3b5423b760)

This is an automated email from the ASF dual-hosted git repository.

mridulpathak pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


    from 98e498f9a2 Fixed: createReturnItem() does not verify OrderItem ownership before attaching it to a ReturnHeader
     new f3207bdff6 Add permission checks to FinAccount create/update/delete services
     new 3b5423b760 Elevate createAccountAndCredit's internal FinAccount writes to the system user

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../accounting/servicedef/services_finaccount.xml  | 24 ++++++++++-----
 .../finaccount/FinAccountPaymentServices.java      | 34 +++++++++++++++++-----
 .../finaccount/FinAccountProductServices.java      |  9 ++++--
 .../accounting/finaccount/FinAccountServices.java  | 19 ++++++++++--
 .../payment/GiftCertificateServices.java           | 17 +++++++++--
 .../ofbiz/order/order/OrderReturnServices.java     | 16 +++++++---
 6 files changed, 90 insertions(+), 29 deletions(-)


(ofbiz-framework) 02/02: Elevate createAccountAndCredit's internal FinAccount writes to the system user

Posted by mr...@apache.org.
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 3b5423b760753cf7bf49c471e93bffe8c985c258
Author: Mridul Pathak <mr...@gmail.com>
AuthorDate: Mon Oct 12 17:16:11 2026 +0530

    Elevate createAccountAndCredit's internal FinAccount writes to the system user
    
    createAccountAndCredit (shared by createServiceCredit and createFinAccountAndCredit) was the one caller in this change not elevated to the system user, so a caller without accounting permission now gets rejected by createFinAccount/createFinAccountRole/createFinAccountTrans.
---
 .../accounting/finaccount/FinAccountServices.java     | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountServices.java b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountServices.java
index 86795f19ff..b3d4616671 100644
--- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountServices.java
+++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/finaccount/FinAccountServices.java
@@ -121,7 +121,11 @@ public class FinAccountServices {
                 createAccountContext.put("finAccountTypeId", finAccountTypeId);
                 createAccountContext.put("finAccountName", accountName);
                 createAccountContext.put("ownerPartyId", partyId);
-                createAccountContext.put("userLogin", userLogin);
+                // createFinAccount requires accounting permission; run as the "system" user since this is a trusted
+                // side effect of an already-validated service-credit/financial-account operation
+                GenericValue createAccountPermUserLogin = EntityQuery.use(delegator).from("UserLogin")
+                        .where("userLoginId", "system").cache().queryOne();
+                createAccountContext.put("userLogin", createAccountPermUserLogin);
 
                 Map<String, Object> createAccountResult = dispatcher.runSync(createAccountServiceName, createAccountContext);
                 if (ServiceUtil.isError(createAccountResult) || ServiceUtil.isFailure(createAccountResult)) {
@@ -139,7 +143,11 @@ public class FinAccountServices {
                         roleCtx.put("partyId", partyId);
                         roleCtx.put("roleTypeId", "OWNER");
                         roleCtx.put("finAccountId", creditAccountId);
-                        roleCtx.put("userLogin", userLogin);
+                        // createFinAccountRole requires accounting permission; run as the "system" user since this is
+                        // a trusted side effect of an already-validated service-credit/financial-account operation
+                        GenericValue rolePermUserLogin = EntityQuery.use(delegator).from("UserLogin")
+                                .where("userLoginId", "system").cache().queryOne();
+                        roleCtx.put("userLogin", rolePermUserLogin);
                         roleCtx.put("fromDate", UtilDateTime.nowTimestamp());
                         Map<String, Object> roleResp;
                         try {
@@ -167,7 +175,12 @@ public class FinAccountServices {
             transactionMap.put("amount", context.get("amount"));
             transactionMap.put("reasonEnumId", context.get("reasonEnumId"));
             transactionMap.put("comments", context.get("comments"));
-            transactionMap.put("userLogin", userLogin);
+            // createFinAccountTrans requires accounting permission; run as the "system" user since this transaction
+            // is a trusted side effect of an already-validated service-credit/financial-account operation (the
+            // partyId above still records the real party the transaction is with)
+            GenericValue transPermUserLogin = EntityQuery.use(delegator).from("UserLogin")
+                    .where("userLoginId", "system").cache().queryOne();
+            transactionMap.put("userLogin", transPermUserLogin);
 
             Map<String, Object> creditTransResult = dispatcher.runSync("createFinAccountTrans", transactionMap);
             if (ServiceUtil.isError(creditTransResult) || ServiceUtil.isFailure(creditTransResult)) {


(ofbiz-framework) 01/02: Add permission checks to FinAccount create/update/delete services

Posted by mr...@apache.org.
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 <mr...@gmail.com>
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));