You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by nm...@apache.org on 2020/03/04 20:19:08 UTC
[ofbiz-framework] branch trunk updated: Fixed: Refactoring
permission model call, alone permission service failed (OFBIZ-11440)
This is an automated email from the ASF dual-hosted git repository.
nmalin pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 9a50fa9 Fixed: Refactoring permission model call, alone permission service failed (OFBIZ-11440)
9a50fa9 is described below
commit 9a50fa907b041df6907a15d6742a5527fcbf610d
Author: Nicolas Malin <ni...@nereide.fr>
AuthorDate: Wed Mar 4 18:36:13 2020 +0100
Fixed: Refactoring permission model call, alone permission service failed
(OFBIZ-11440)
During refactoring (OFBIZ-7113), the service reader used two different methods to read a
permission service definition if is present alone or in a group. For the alone version a link
was missing between the service and the permission service to resolve correctly the permission.
To solve this problem I reorganized to use only one method to initialize a service model permission.
Other point, an attribute renaming was missed : action -> permissionMainAction, when service permission call was called
Last, these problems weren't detected before due to missing unit test on failure service permission, now cover.
---
.../org/apache/ofbiz/service/ModelPermission.java | 6 ++--
.../apache/ofbiz/service/ModelServiceReader.java | 42 ++++++++++++----------
.../ofbiz/service/test/ServicePermissionTests.java | 10 ++++++
3 files changed, 36 insertions(+), 22 deletions(-)
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java b/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java
index 0336490..b4f070d 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java
@@ -133,8 +133,8 @@ public class ModelPermission implements Serializable {
permission.auth = true;
Map<String, Object> ctx = permission.makeValid(context, ModelService.IN_PARAM);
- if (UtilValidate.isNotEmpty(action)) {
- ctx.put("mainAction", action);
+ if (UtilValidate.isNotEmpty(permissionMainAction)) {
+ ctx.put("mainAction", permissionMainAction);
}
if (UtilValidate.isNotEmpty(permissionResourceDesc)) {
ctx.put("resourceDescription", permissionResourceDesc);
@@ -154,7 +154,7 @@ public class ModelPermission implements Serializable {
Debug.logError(failMessage + e.getMessage(), module);
return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ServicePermissionErrorDefinitionProblem", locale));
}
- if (Debug.verboseOn()) Debug.logVerbose("Service permision result : hasPermission " + resp.get("hasPermission") + ", failMessage " + failMessage , module);
+ if (Debug.verboseOn()) Debug.logVerbose("Service permission result : hasPermission " + resp.get("hasPermission") + ", failMessage " + failMessage , module);
if (permissionReturnErrorOnFailure &&
(UtilValidate.isNotEmpty(failMessage) || ! ((Boolean) resp.get("hasPermission")).booleanValue())) {
if (UtilValidate.isEmpty(failMessage)) failMessage = UtilProperties.getMessage(resource, "ServicePermissionErrorRefused", locale);
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java b/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java
index 9ef2f6a..3b45ecb 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java
@@ -243,7 +243,7 @@ public class ModelServiceReader implements Serializable {
// construct the context
service.contextInfo = new HashMap<>();
createNotification(serviceElement, service);
- createPermission(serviceElement, service);
+ createAloneServicePermission(serviceElement, service);
createPermGroups(serviceElement, service);
createGroupDefs(serviceElement, service);
createImplDefs(serviceElement, service);
@@ -305,15 +305,25 @@ public class ModelServiceReader implements Serializable {
}
}
- private static void createPermission(Element baseElement, ModelService model) {
+ private static ModelPermission createServicePermission(Element e, ModelService model) {
+ ModelPermission modelPermission = new ModelPermission();
+ modelPermission.permissionType = ModelPermission.PERMISSION_SERVICE;
+ modelPermission.permissionServiceName = e.getAttribute("service-name");
+ modelPermission.permissionMainAction = e.getAttribute("main-action");
+ modelPermission.permissionResourceDesc = e.getAttribute("resource-description");
+ modelPermission.permissionRequireNewTransaction = !"false".equalsIgnoreCase(e.getAttribute("require-new-transaction"));
+ modelPermission.serviceModel = model;
+ return modelPermission;
+ }
+
+ private static void createAloneServicePermission(Element baseElement, ModelService model) {
Element e = UtilXml.firstChildElement(baseElement, "permission-service");
if (e != null) {
- ModelPermission modelPermission = new ModelPermission();
- modelPermission.permissionServiceName = e.getAttribute("service-name");
- modelPermission.permissionMainAction = e.getAttribute("main-action");
- modelPermission.permissionResourceDesc = e.getAttribute("resource-description");
- modelPermission.permissionRequireNewTransaction = !"false".equalsIgnoreCase(e.getAttribute("require-new-transaction"));
- model.auth = true; // auth is always required when permissions are set
+ ModelPermission modelPermission = createServicePermission(e, model);
+ model.modelPermission = modelPermission;
+
+ // auth is always required when permissions are set
+ model.auth = true;
}
}
@@ -343,17 +353,11 @@ public class ModelServiceReader implements Serializable {
// Create the permissions based on permission services
for (Element element : UtilXml.childElementList(baseElement, "permission-service")) {
- ModelPermission perm = new ModelPermission();
- if (baseElement != null) {
- perm.permissionType = ModelPermission.PERMISSION_SERVICE;
- perm.permissionServiceName = element.getAttribute("service-name");
- perm.action = element.getAttribute("main-action");
- perm.permissionResourceDesc = element.getAttribute("resource-description");
- perm.permissionRequireNewTransaction = !"false".equalsIgnoreCase(element.getAttribute("require-new-transaction"));
- perm.auth = true; // auth is always required when permissions are set
- perm.serviceModel = service;
- group.permissions.add(perm);
- }
+ group.permissions.add(createServicePermission(element, service));
+ }
+ if (UtilValidate.isNotEmpty(group.permissions)) {
+ // auth is always required when permissions are set
+ service.auth = true;
}
}
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java b/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java
index e400ebd..09d5fa8 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java
@@ -43,6 +43,16 @@ public class ServicePermissionTests extends OFBizTestCase {
assertTrue(ServiceUtil.isSuccess(results));
}
+ public void testServicePermissionError() throws Exception {
+ Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser1"), "givePermission", "N");
+ try {
+ Map<String, Object> results = dispatcher.runSync("testSimpleServicePermission", testingPermMap);
+ assertFalse("The testGroupPermission don't raise service exception", ServiceUtil.isError(results));
+ } catch (ServiceAuthException e) {
+ assertNotNull(e);
+ }
+ }
+
public void testGroupPermissionSuccess() throws Exception {
Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser1"), "givePermission", "Y");
Map<String, Object> results = dispatcher.runSync("testSimpleGroupAndPermission", testingPermMap);