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 <[email protected]>
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);
