Author: nmalin
Date: Fri Aug 30 08:47:48 2019 New Revision: 1866137 URL: http://svn.apache.org/viewvc?rev=1866137&view=rev Log: Improved: Refactoring permission model call (OFBIZ-7113) With the problem to have a permission service not on the same transaction that the related service [1] I realized the following improvement to * unified call evalPermission * move all related field for permission service on ModelService to ModelPermission * Remove deprecated code * add labelized error message * call as same transaction the permission service by defautl with possibility to overide this rule * add new attributes on permission model: require-new-transaction and return-error-on-failure Thanks to Jacques Leroux for the review Added: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java (with props) ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java (with props) ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml (with props) Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/payment/PaymentServices.xml ofbiz/ofbiz-framework/trunk/applications/commonext/servicedef/services.xml ofbiz/ofbiz-framework/trunk/framework/service/config/ServiceErrorUiLabels.xml ofbiz/ofbiz-framework/trunk/framework/service/dtd/services.xsd ofbiz/ofbiz-framework/trunk/framework/service/servicedef/services_test_se.xml ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermGroup.java ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ServiceDispatcher.java ofbiz/ofbiz-framework/trunk/framework/service/testdef/servicetests.xml Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml (original) +++ ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml Fri Aug 30 08:47:48 2019 @@ -2482,7 +2482,6 @@ under the License. <set-service-fields service-name="createAcctgTrans" map="newAcctgTrans" to-map="createAcctgTransInMap"/> <now-timestamp field="nowTimestamp"/> <set field="createAcctgTransInMap.transactionDate" from-field="nowTimestamp"/> - <set field="createAcctgTransInMap.isPosted" value="N"/> <set field="originalAcctgTransId" from-field="parameters.fromAcctgTransId"/> <field-to-result field="originalAcctgTransId" result-name="acctgTransId"/> Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/payment/PaymentServices.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/payment/PaymentServices.xml?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/payment/PaymentServices.xml (original) +++ ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/payment/PaymentServices.xml Fri Aug 30 08:47:48 2019 @@ -547,7 +547,7 @@ under the License. <iterate list="paymentApplications" entry="paymentApplication"> <get-related-one relation-name="Invoice" value-field="paymentApplication" to-value-field="updateInvoiceCtx"/> <if-compare field="updateInvoiceCtx.statusId" operator="equals" value="INVOICE_PAID"> - <set-service-fields service-name="updateInvoice" map="updateInvoiceCtx" to-map="invoiceStatusCtx"/> + <set-service-fields service-name="setInvoiceStatus" map="updateInvoiceCtx" to-map="invoiceStatusCtx"/> <set field="invoiceStatusCtx.paidDate" type="Timestamp" value=""/> <set field="invoiceStatusCtx.statusId" value="INVOICE_READY"/> <call-service service-name="setInvoiceStatus" in-map-name="invoiceStatusCtx"/> Modified: ofbiz/ofbiz-framework/trunk/applications/commonext/servicedef/services.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/commonext/servicedef/services.xml?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/commonext/servicedef/services.xml (original) +++ ofbiz/ofbiz-framework/trunk/applications/commonext/servicedef/services.xml Fri Aug 30 08:47:48 2019 @@ -19,7 +19,7 @@ under the License. --> <services xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" - xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/services.xsd"> + xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/services.xsd"> <description>Commonext Component Services</description> <vendor>OFBiz</vendor> <version>1.0</version> Modified: ofbiz/ofbiz-framework/trunk/framework/service/config/ServiceErrorUiLabels.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/config/ServiceErrorUiLabels.xml?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/config/ServiceErrorUiLabels.xml (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/config/ServiceErrorUiLabels.xml Fri Aug 30 08:47:48 2019 @@ -67,7 +67,7 @@ <value xml:lang="ja">åå¼å IDãæ£ããããã¾ãã</value> <value xml:lang="nl">Relatie nummer van deze partij is niet gevonden</value> <value xml:lang="ro">Cod Subiect Lipseste</value> - <value xml:lang="th">à¹à¸¡à¹à¹à¸à¹à¹à¸ªà¹à¸£à¸«à¸±à¸ªà¸à¸¥à¸¸à¹à¸¡à¸à¸¹à¹à¹à¸à¹</value> + <value xml:lang="th">aà¹à¸¡à¹à¹à¸à¹à¹à¸ªà¹à¸£à¸«à¸±à¸ªà¸à¸¥à¸¸à¹à¸¡à¸à¸¹à¹à¹à¸à¹</value> <value xml:lang="zh">æ¾ä¸å°ä¼åæ è¯</value> <value xml:lang="zh-TW">ç¡è©²åé«èå¥</value> </property> @@ -114,6 +114,26 @@ <value xml:lang="zh">åæ° ${parameterName} ä¸çæ è¯ï¼IDï¼å¼æ æï¼${errorDetails}</value> <value xml:lang="zh-TW">åæ¸ ${parameterName} ä¸çèå¥(èå¥)å¼ç¡æ:${errorDetails}</value> </property> + <property key="ServicePermissionError"> + <value xml:lang="en">You haven't the permission for the service ${serviceName}, reason : ${failMessage}</value> + <value xml:lang="fr">Vous n'avez pas la permission sur le service ${serviceName}, raison : ${failMessage}</value> + </property> + <property key="ServicePermissionErrorDefinitionProblem"> + <value xml:lang="en">Problem on permission service definition, please see with your integrator</value> + <value xml:lang="fr">Problème dans la définition d'un service de validation de permission, merci d'en informer votre intégrateur</value> + </property> + <property key="ServicePermissionErrorInvalidPermissionType"> + <value xml:lang="en">The permission type call by the service isn't valid. Please see with your integrator</value> + <value xml:lang="fr">Le type de permission défini sur le service appelé n'est pas conforme, veuiller prendre contact avec votre intégrateur !</value> + </property> + <property key="ServicePermissionErrorRefused"> + <value xml:lang="en">Access refused</value> + <value xml:lang="fr">Accés refusé</value> + </property> + <property key="ServicePermissionErrorUserLoginMissing"> + <value xml:lang="en">User login is missing</value> + <value xml:lang="fr">Absence d'utilisateur identifié</value> + </property> <property key="ServiceTestDeadLockError"> <value xml:lang="en">Error running deadlock test services: ${errorString}</value> <value xml:lang="it">Errore durante il test del servizio deadlock: ${errorString}</value> Modified: ofbiz/ofbiz-framework/trunk/framework/service/dtd/services.xsd URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/dtd/services.xsd?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/dtd/services.xsd (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/dtd/services.xsd Fri Aug 30 08:47:48 2019 @@ -153,12 +153,28 @@ under the License. </xs:restriction> </xs:simpleType> </xs:attribute> + <xs:attribute name="require-new-transaction" default="false"> + <xs:simpleType> + <xs:restriction base="xs:token"> + <xs:enumeration value="true"/> + <xs:enumeration value="false"/> + </xs:restriction> + </xs:simpleType> + </xs:attribute> + <xs:attribute name="return-error-on-failure" default="true"> + <xs:annotation><xs:documentation>If set to false, when the permissions failed return the failMessage as error, else continue the service and give the hand to origin service to resolve the problem.</xs:documentation></xs:annotation> + <xs:simpleType> + <xs:restriction base="xs:token"> + <xs:enumeration value="true"/> + <xs:enumeration value="false"/> + </xs:restriction> + </xs:simpleType> + </xs:attribute> </xs:attributeGroup> <xs:element name="required-permissions"> <xs:complexType> <xs:sequence> <xs:element minOccurs="0" maxOccurs="unbounded" ref="check-permission"/> - <xs:element minOccurs="0" maxOccurs="unbounded" ref="check-role-member"/> <xs:element minOccurs="0" maxOccurs="unbounded" ref="permission-service"/> </xs:sequence> <xs:attributeGroup ref="attlist.required-permissions"/> @@ -183,19 +199,6 @@ under the License. <xs:attribute name="permission" type="xs:string" use="required"/> <xs:attribute name="action" type="xs:string"/> </xs:attributeGroup> - <xs:element name="check-role-member"> - <xs:annotation> - <xs:documentation> - This is deprecated - </xs:documentation> - </xs:annotation> - <xs:complexType> - <xs:attributeGroup ref="attlist.check-role-member"/> - </xs:complexType> - </xs:element> - <xs:attributeGroup name="attlist.check-role-member"> - <xs:attribute name="role-type" type="xs:string" use="required"/> - </xs:attributeGroup> <xs:element name="service-security"> <xs:complexType> <xs:attributeGroup ref="attlist.service-security"/> Modified: ofbiz/ofbiz-framework/trunk/framework/service/servicedef/services_test_se.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/servicedef/services_test_se.xml?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/servicedef/services_test_se.xml (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/servicedef/services_test_se.xml Fri Aug 30 08:47:48 2019 @@ -166,4 +166,50 @@ under the License. <service name="testXmlRpcClientAdd" engine="java" auth="false" location="org.apache.ofbiz.service.test.XmlRpcTests" invoke="testXmlRpcClientAdd"> <implements service="testServiceInterface"/> </service> + + <!--Test permission--> + <service name="testSimplePermission" engine="java" auth="true" + location="org.apache.ofbiz.service.test.ServiceEngineTestPermissionServices" invoke="genericTestService"> + <required-permissions join-type="AND"> + <check-permission permission="TESTPERM" action="_UPDATE"/> + </required-permissions> + </service> + <service name="testSimpleServicePermission" engine="java" auth="true" + location="org.apache.ofbiz.service.test.ServiceEngineTestPermissionServices" invoke="genericTestService"> + <permission-service service-name="testPermissionPing"/> + <attribute name="givePermission" type="String" mode="IN"/> + </service> + <service name="testSimpleGroupAndPermission" engine="java" auth="true" + location="org.apache.ofbiz.service.test.ServiceEngineTestPermissionServices" invoke="genericTestService"> + <required-permissions join-type="AND"> + <check-permission permission="TESTPERM" action="_UPDATE"/> + <permission-service service-name="testPermissionPing"/> + </required-permissions> + <attribute name="givePermission" type="String" mode="IN"/> + </service> + <service name="testSimpleGroupOrPermission" engine="java" auth="true" + location="org.apache.ofbiz.service.test.ServiceEngineTestPermissionServices" invoke="genericTestService"> + <required-permissions join-type="OR"> + <check-permission permission="TESTPERM" action="_UPDATE"/> + <permission-service service-name="testPermissionPing"/> + </required-permissions> + <attribute name="givePermission" type="String" mode="IN"/> + </service> + <service name="testServicePermissionWithTransaction" engine="java" auth="true" + location="org.apache.ofbiz.service.test.ServiceEngineTestPermissionServices" invoke="genericTestService"> + <permission-service service-name="testPermissionPing" require-new-transaction="true"/> + <attribute name="testingId" type="String" mode="IN"/> + </service> + <service name="testPermissionPing" engine="java" auth="true" + location="org.apache.ofbiz.service.test.ServiceEngineTestPermissionServices" invoke="testPermissionPing"> + <implements service="permissionInterface"/> + <attribute name="givePermission" type="String" mode="IN"/> + </service> + <service name="testPermissionExistingTesting" engine="java" auth="true" + location="org.apache.ofbiz.service.test.ServiceEngineTestPermissionServices" invoke="testPermissionExistingTesting"> + <implements service="permissionInterface"/> + <attribute name="testingId" type="String" mode="IN"/> + </service> + + </services> Modified: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermGroup.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermGroup.java?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermGroup.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermGroup.java Fri Aug 30 08:47:48 2019 @@ -19,6 +19,7 @@ package org.apache.ofbiz.service; import java.io.Serializable; +import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -38,21 +39,26 @@ public class ModelPermGroup implements S public List<ModelPermission> permissions = new LinkedList<>(); public String joinType; - - public boolean evalPermissions(DispatchContext dctx, Map<String, ? extends Object> context) { - if (UtilValidate.isNotEmpty(permissions)) { + public Map<String, Object> evalPermissions(DispatchContext dctx, Map<String, ? extends Object> context) { + List<String> permissionErrors = new ArrayList<String>(); + if (UtilValidate.isNotEmpty(permissions)) { boolean foundOne = false; for (ModelPermission perm: permissions) { - if (perm.evalPermission(dctx, context)) { + Map<String, Object> permResult = perm.evalPermission(dctx, context); + if (ServiceUtil.isSuccess(permResult)) { foundOne = true; } else { + ServiceUtil.addErrors(permissionErrors, null, permResult); if (joinType.equals(PERM_JOIN_AND)) { - return false; + return permResult; } } } - return foundOne; + if (! foundOne) { + return ServiceUtil.returnError(permissionErrors); + } } - return true; + return ServiceUtil.returnSuccess(); } } + Modified: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelPermission.java Fri Aug 30 08:47:48 2019 @@ -19,9 +19,11 @@ package org.apache.ofbiz.service; import java.io.Serializable; +import java.util.Locale; import java.util.Map; import org.apache.ofbiz.base.util.Debug; +import org.apache.ofbiz.base.util.UtilProperties; import org.apache.ofbiz.base.util.UtilValidate; import org.apache.ofbiz.entity.GenericValue; import org.apache.ofbiz.security.Security; @@ -43,28 +45,56 @@ public class ModelPermission implements public String nameOrRole = null; public String action = null; public String permissionServiceName = null; + public String permissionMainAction = null; public String permissionResourceDesc = null; + public boolean permissionRequireNewTransaction = false; + public boolean permissionReturnErrorOnFailure = true; public Boolean auth; - public String clazz = null; - public boolean evalPermission(DispatchContext dctx, Map<String, ? extends Object> context) { + public static final String resource = "ServiceErrorUiLabels"; + + @Override + public String toString() { + StringBuilder buf = new StringBuilder(); + buf.append(serviceModel.name).append("::"); + buf.append(permissionType).append("::"); + buf.append(nameOrRole).append("::"); + buf.append(action).append("::"); + buf.append(permissionServiceName).append("::"); + buf.append(permissionMainAction).append("::"); + buf.append(permissionResourceDesc).append("::"); + buf.append(permissionRequireNewTransaction).append("::"); + buf.append(permissionReturnErrorOnFailure).append("::"); + return buf.toString(); + } + + public Map<String, Object> evalPermission(DispatchContext dctx, Map<String, ? extends Object> context) { GenericValue userLogin = (GenericValue) context.get("userLogin"); + Locale locale = (Locale) context.get("locale"); Security security = dctx.getSecurity(); if (userLogin == null) { Debug.logInfo("Secure service requested with no userLogin object", module); - return false; + return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ServicePermissionErrorUserLoginMissing", locale)); } + boolean hasPermission = false; + if (Debug.verboseOn()) Debug.logVerbose(" Permission : Analyse " + this.toString(), module); switch (permissionType) { case PERMISSION: - return evalSimplePermission(security, userLogin); + hasPermission = evalSimplePermission(security, userLogin); + break; case ENTITY_PERMISSION: - return evalEntityPermission(security, userLogin); + hasPermission = evalEntityPermission(security, userLogin); + break; case PERMISSION_SERVICE: return evalPermissionService(serviceModel, dctx, context); default: Debug.logWarning("Invalid permission type [" + permissionType + "] for permission named : " + nameOrRole + " on service : " + serviceModel.name, module); - return false; + return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ServicePermissionErrorInvalidPermissionType", locale)); } + if (! hasPermission) { + return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ServicePermissionErrorRefused", locale)); + } + return ServiceUtil.returnSuccess(); } private boolean evalSimplePermission(Security security, GenericValue userLogin) { @@ -77,7 +107,7 @@ public class ModelPermission implements private boolean evalEntityPermission(Security security, GenericValue userLogin) { if (nameOrRole == null) { - Debug.logWarning("Null permission name passed for evaluation", module); + Debug.logError("Null permission name passed for evaluation", module); return false; } if (action == null) { @@ -86,18 +116,21 @@ public class ModelPermission implements return security.hasEntityPermission(nameOrRole, action, userLogin); } - private boolean evalPermissionService(ModelService origService, DispatchContext dctx, Map<String, ? extends Object> context) { + private Map<String, Object> evalPermissionService(ModelService origService, DispatchContext dctx, Map<String, ? extends Object> context) { + LocalDispatcher dispatcher = dctx.getDispatcher(); ModelService permission; + Locale locale = (Locale) context.get("locale"); if (permissionServiceName == null) { Debug.logWarning("No ModelService found; no service name specified!", module); - return false; + return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ServicePermissionErrorDefinitionProblem", locale)); } try { permission = dctx.getModelService(permissionServiceName); } catch (GenericServiceException e) { Debug.logError(e, "Failed to get ModelService: " + e.toString(), module); - return false; + return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ServicePermissionErrorDefinitionProblem", locale)); } + permission.auth = true; Map<String, Object> ctx = permission.makeValid(context, ModelService.IN_PARAM); if (UtilValidate.isNotEmpty(action)) { @@ -108,20 +141,25 @@ public class ModelPermission implements } else if (origService != null) { ctx.put("resourceDescription", origService.name); } - LocalDispatcher dispatcher = dctx.getDispatcher(); Map<String, Object> resp; String failMessage = null; try { - resp = dispatcher.runSync(permission.name, ctx, 300, true); + if (permissionRequireNewTransaction) { + resp = dispatcher.runSync(permission.name, ctx, 300, true); + } else { + resp = dispatcher.runSync(permission.name, ctx); + } failMessage = (String) resp.get("failMessage"); } catch (GenericServiceException e) { - Debug.logError(null + e.getMessage(), module); - return false; + Debug.logError(failMessage + e.getMessage(), module); + return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ServicePermissionErrorDefinitionProblem", locale)); } - if (ServiceUtil.isError(resp) || ServiceUtil.isFailure(resp)) { - Debug.logError(failMessage, module); - return false; + if (Debug.verboseOn()) Debug.logVerbose("Service permision 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); + return ServiceUtil.returnError(failMessage); } - return (Boolean) resp.get("hasPermission"); + return resp; } } Modified: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java Fri Aug 30 08:47:48 2019 @@ -23,6 +23,7 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.AbstractMap; import java.util.AbstractSet; +import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; @@ -163,14 +164,8 @@ public class ModelService extends Abstra /** Sets the max number of times this service will retry when failed (persisted async only) */ public int maxRetry = 0; - /** Permission service name */ - public String permissionServiceName; - - /** Permission service main-action */ - public String permissionMainAction; - - /** Permission service resource-description */ - public String permissionResourceDesc; + /** Permission service*/ + ModelPermission modelPermission = null; /** Semaphore setting (wait, fail, none) */ public String semaphore; @@ -246,9 +241,9 @@ public class ModelService extends Abstra } this.transactionTimeout = model.transactionTimeout; this.maxRetry = model.maxRetry; - this.permissionServiceName = model.permissionServiceName; - this.permissionMainAction = model.permissionMainAction; - this.permissionResourceDesc = model.permissionResourceDesc; + if (model.modelPermission != null) { + modelPermission = model.modelPermission; + } this.implServices = model.implServices; this.overrideParameters = model.overrideParameters; this.inheritedParameters = model.inheritedParameters(); @@ -1002,55 +997,14 @@ public class ModelService extends Abstra * @return result of permission service invocation */ public Map<String, Object> evalPermission(DispatchContext dctx, Map<String, ? extends Object> context) { - if (UtilValidate.isNotEmpty(this.permissionServiceName)) { - ModelService thisService; - ModelService permission; - try { - thisService = dctx.getModelService(this.name); - permission = dctx.getModelService(this.permissionServiceName); - } catch (GenericServiceException e) { - Debug.logError(e, "Failed to get ModelService: " + e.toString(), module); - Map<String, Object> result = ServiceUtil.returnSuccess(); - result.put("hasPermission", Boolean.FALSE); - result.put("failMessage", e.getMessage()); - return result; - } - Map<String, Object> ctx = permission.makeValid(context, IN_PARAM); - if (UtilValidate.isNotEmpty(this.permissionMainAction)) { - ctx.put("mainAction", this.permissionMainAction); - } - if (UtilValidate.isNotEmpty(this.permissionResourceDesc)) { - ctx.put("resourceDescription", this.permissionResourceDesc); - } - ctx.put("resourceDescription", thisService.name); - - LocalDispatcher dispatcher = dctx.getDispatcher(); - Map<String, Object> resp; - try { - resp = dispatcher.runSync(permission.name, ctx, 300, true); - } catch (GenericServiceException e) { - Debug.logError(e, module); - Map<String, Object> result = ServiceUtil.returnSuccess(); - result.put("hasPermission", Boolean.FALSE); - result.put("failMessage", e.getMessage()); - return result; - } - if (ServiceUtil.isError(resp) || ServiceUtil.isFailure(resp)) { - Map<String, Object> result = ServiceUtil.returnSuccess(); - result.put("hasPermission", Boolean.FALSE); - String failMessage = (String) resp.get("failMessage"); - if (UtilValidate.isEmpty(failMessage)) { - failMessage = ServiceUtil.getErrorMessage(resp); - } - result.put("failMessage", failMessage); - return result; - } - return resp; + if (this.modelPermission != null) { + return modelPermission.evalPermission(dctx, context); + } else { + Map<String, Object> result = ServiceUtil.returnSuccess(); + result.put("hasPermission", Boolean.FALSE); + result.put("failMessage", UtilProperties.getMessage(resource, "ServicePermissionErrorDefinitionProblem", (Locale) context.get("locale"))); + return result; } - Map<String, Object> result = ServiceUtil.returnSuccess(); - result.put("hasPermission", Boolean.FALSE); - result.put("failMessage", "No ModelService found; no service name specified!"); - return result; } /** @@ -1066,18 +1020,25 @@ public class ModelService extends Abstra * Evaluates permissions for a service. * @param dctx DispatchContext from the invoked service * @param context Map containing userLogin information - * @return true if all permissions evaluate true. + * @return Map if all permissions evaluate return success else return the error message list. */ - public boolean evalPermissions(DispatchContext dctx, Map<String, ? extends Object> context) { + public Map<String, Object> evalPermissions(DispatchContext dctx, Map<String, ? extends Object> context) { + List<String> permGroupErrors = new ArrayList<String>(); + // old permission checking if (this.containsPermissions()) { for (ModelPermGroup group: this.permissionGroups) { - if (!group.evalPermissions(dctx, context)) { - return false; + if (Debug.verboseOn()) Debug.logVerbose(" Permission : Analyse " + group.toString(), module); + Map<String, Object> permResult = group.evalPermissions(dctx, context); + if (! ServiceUtil.isSuccess(permResult)) { + ServiceUtil.addErrors(permGroupErrors, null, permResult); } } } - return true; + if (UtilValidate.isEmpty(permGroupErrors)) { + return ServiceUtil.returnSuccess(); + } + return ServiceUtil.returnError(permGroupErrors); } /** Modified: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelServiceReader.java Fri Aug 30 08:47:48 2019 @@ -308,9 +308,11 @@ public class ModelServiceReader implemen private static void createPermission(Element baseElement, ModelService model) { Element e = UtilXml.firstChildElement(baseElement, "permission-service"); if (e != null) { - model.permissionServiceName = e.getAttribute("service-name"); - model.permissionMainAction = e.getAttribute("main-action"); - model.permissionResourceDesc = e.getAttribute("resource-description"); + 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 } } @@ -347,6 +349,7 @@ public class ModelServiceReader implemen 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); Modified: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ServiceDispatcher.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ServiceDispatcher.java?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ServiceDispatcher.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ServiceDispatcher.java Fri Aug 30 08:47:48 2019 @@ -392,6 +392,8 @@ public final class ServiceDispatcher { // validate the context if (modelService.validate && !isError && !isFailure) { try { + // FIXME without this line all simple test failed + context = ctx.makeValidContext(modelService.name, ModelService.IN_PARAM, context); modelService.validate(context, ModelService.IN_PARAM, locale); } catch (ServiceValidationException e) { Debug.logError(e, "Incoming context (in runSync : " + modelService.name + ") does not match expected requirements", module); @@ -502,6 +504,7 @@ public final class ServiceDispatcher { ServiceEcaUtil.evalRules(modelService.name, eventMap, "out-validate", ctx, ecaContext, result, isError, isFailure); } try { + result = ctx.makeValidContext(modelService.name, ModelService.OUT_PARAM, result); modelService.validate(result, ModelService.OUT_PARAM, locale); } catch (ServiceValidationException e) { rs.setEndStamp(); @@ -891,6 +894,7 @@ public final class ServiceDispatcher { // checks if parameters were passed for authentication private Map<String, Object> checkAuth(String localName, Map<String, Object> context, ModelService origService) throws ServiceAuthException, GenericServiceException { String service = null; + Locale locale = (Locale) context.get("locale"); try { service = ServiceConfigUtil.getServiceEngine().getAuthorization().getServiceName(); } catch (GenericConfigException e) { @@ -911,11 +915,10 @@ public final class ServiceDispatcher { if (UtilValidate.isNotEmpty(context.get("login.password"))) { String password = (String) context.get("login.password"); - - context.put("userLogin", getLoginObject(service, localName, username, password, (Locale) context.get("locale"))); + context.put("userLogin", getLoginObject(service, localName, username, password, locale)); context.remove("login.password"); } else { - context.put("userLogin", getLoginObject(service, localName, username, null, (Locale) context.get("locale"))); + context.put("userLogin", getLoginObject(service, localName, username, null, locale)); } context.remove("login.username"); } else { @@ -949,30 +952,20 @@ public final class ServiceDispatcher { // evaluate permissions for the service or throw exception if fail. DispatchContext dctx = this.getLocalContext(localName); - if (UtilValidate.isNotEmpty(origService.permissionServiceName)) { - Map<String, Object> permResp = origService.evalPermission(dctx, context); - Boolean hasPermission = (Boolean) permResp.get("hasPermission"); - if (hasPermission == null) { - throw new ServiceAuthException("ERROR: the permission-service [" + origService.permissionServiceName + "] did not return a result. Not running the service [" + origService.name + "]"); - } - if (hasPermission) { - context.putAll(permResp); - } else { - String message = (String) permResp.get("failMessage"); - if (UtilValidate.isEmpty(message)) { - message = ServiceUtil.getErrorMessage(permResp); - } - if (UtilValidate.isEmpty(message)) { - message = "You do not have permission to invoke the service [" + origService.name + "]"; - } - throw new ServiceAuthException(message); + Map<String, Object> permResp = null; + if (origService.modelPermission != null) { + permResp = origService.evalPermission(dctx, context); + if (ServiceUtil.isSuccess(permResp)) { + //Ok the service have authorization to run, complete the context with the permission response map + context.putAll(origService.makeValid(permResp, ModelService.IN_PARAM)); } } else { - if (!origService.evalPermissions(dctx, context)) { - throw new ServiceAuthException("You do not have permission to invoke the service [" + origService.name + "]"); - } + permResp = origService.evalPermissions(dctx, context); + } + if (ServiceUtil.isFailure(permResp) || ServiceUtil.isError(permResp)) { + throw new ServiceAuthException(UtilProperties.getMessage("ServiceErrorUiLabels", "ServicePermissionError", + UtilMisc.toMap("serviceName", origService.name, "failMessage", ServiceUtil.getErrorMessage(permResp)), locale)); } - return origService.makeValid(context, ModelService.IN_PARAM); } Added: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java?rev=1866137&view=auto ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java (added) +++ ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java Fri Aug 30 08:47:48 2019 @@ -0,0 +1,40 @@ +/* + * 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. + */ +package org.apache.ofbiz.service.test; + +import java.util.Map; +import org.apache.ofbiz.service.DispatchContext; +import org.apache.ofbiz.service.ServiceUtil; + +public class ServiceEngineTestPermissionServices { + + public static final String module = ServiceEngineTestPermissionServices.class.getName(); + public static final String resource = "ServiceErrorUiLabels"; + + public static Map<String, Object> genericTestService(DispatchContext dctx, Map<String, ? extends Object> context) { + return ServiceUtil.returnSuccess(); + } + + public static Map<String, Object> testPermissionPing(DispatchContext dctx, Map<String, ? extends Object> context) { + Map<String, Object> result = ServiceUtil.returnSuccess(); + result.put("hasPermission", "Y".equalsIgnoreCase((String)context.get("givePermission"))); + return result; + } + +} \ No newline at end of file Propchange: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java ------------------------------------------------------------------------------ svn:keywords = Date Rev Author URL Id Propchange: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServiceEngineTestPermissionServices.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Added: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java?rev=1866137&view=auto ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java (added) +++ ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java Fri Aug 30 08:47:48 2019 @@ -0,0 +1,101 @@ +/******************************************************************************* + * 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. + *******************************************************************************/ +package org.apache.ofbiz.service.test; + +import org.apache.ofbiz.base.util.UtilMisc; +import org.apache.ofbiz.entity.GenericValue; +import org.apache.ofbiz.entity.util.EntityQuery; +import org.apache.ofbiz.service.ServiceAuthException; +import org.apache.ofbiz.service.ServiceUtil; +import org.apache.ofbiz.service.testtools.OFBizTestCase; + +import java.util.Map; + +public class ServicePermissionTests extends OFBizTestCase { + + public ServicePermissionTests(String name) { + super(name); + } + + + private GenericValue getUserLogin(String userLoginId) throws Exception { + return EntityQuery.use(delegator).from("UserLogin").where("userLoginId", userLoginId).queryOne(); + } + + public void testPermissionSuccess() throws Exception { + Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser1")); + Map<String, Object> results = dispatcher.runSync("testSimplePermission", testingPermMap); + assertTrue(ServiceUtil.isSuccess(results)); + } + + public void testServicePermissionSuccess() throws Exception { + Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser1"), "givePermission", "Y"); + Map<String, Object> results = dispatcher.runSync("testSimpleServicePermission", testingPermMap); + assertTrue(ServiceUtil.isSuccess(results)); + } + + public void testGroupPermissionSuccess() throws Exception { + Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser1"), "givePermission", "Y"); + Map<String, Object> results = dispatcher.runSync("testSimpleGroupAndPermission", testingPermMap); + assertTrue(ServiceUtil.isSuccess(results)); + + testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser1"), "givePermission", "N"); + results = dispatcher.runSync("testSimpleGroupOrPermission", testingPermMap); + assertTrue(ServiceUtil.isSuccess(results)); + } + + public void testPermissionFailed() throws Exception { + Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser2")); + try { + Map<String, Object> results = dispatcher.runSync("testSimplePermission", testingPermMap); + assertFalse("The service testSimplePermission don't raise service exception", ServiceUtil.isError(results)); + } catch (ServiceAuthException e) { + assertNotNull(e); + } + } + + public void testServicePermissionFailed() throws Exception { + Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser2"), "givePermission", "N"); + try{ + Map<String, Object> results = dispatcher.runSync("testSimpleServicePermission", testingPermMap); + assertFalse("The service testServicePermission don't raise service exception", ServiceUtil.isError(results)); + } catch (ServiceAuthException e) { + assertNotNull(e); + } + } + + public void testGroupPermissionFailed() throws Exception { + Map<String, Object> testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser2"), "givePermission", "Y"); + try{ + Map<String, Object> results = dispatcher.runSync("testSimpleGroupAndPermission", testingPermMap); + assertFalse("The testGroupPermission don't raise service exception", ServiceUtil.isError(results)); + } catch (ServiceAuthException e) { + assertNotNull(e); + } + + testingPermMap = UtilMisc.toMap("userLogin", getUserLogin("permUser1"), "givePermission", "N"); + try{ + Map<String, Object> results = dispatcher.runSync("testSimpleGroupOrPermission", testingPermMap); + assertFalse("The testGroupPermission don't raise service exception", ServiceUtil.isError(results)); + } catch (ServiceAuthException e) { + assertNotNull(e); + } + } + +} \ No newline at end of file Propchange: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java ------------------------------------------------------------------------------ svn:keywords = Date Rev Author URL Id Propchange: ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/test/ServicePermissionTests.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Added: ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml?rev=1866137&view=auto ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml (added) +++ ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml Fri Aug 30 08:47:48 2019 @@ -0,0 +1,40 @@ +<?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. +--> +<entity-engine-xml> + <!--load security group for test--> + <SecurityPermission description="Permission to View" permissionId="TESTPERM_VIEW"/> + <SecurityPermission description="Permission to Update" permissionId="TESTPERM_UPDATE"/> + <SecurityGroup groupId="TESTPERMADMIN" description="permissions admin."/> + <SecurityGroupPermission groupId="TESTPERMADMIN" permissionId="TESTPERM_VIEW" fromDate="2001-05-13 12:00:00.0"/> + <SecurityGroupPermission groupId="TESTPERMADMIN" permissionId="TESTPERM_UPDATE" fromDate="2001-05-13 12:00:00.0"/> + <SecurityGroup groupId="TESTPERMVIEW" description="permissions reader."/> + <SecurityGroupPermission groupId="TESTPERMVIEW" permissionId="TESTPERM_VIEW" fromDate="2001-05-13 12:00:00.0"/> + + <Party partyId="permUser1" partyTypeId="PERSON"/> + <Person partyId="permUser1" firstName="Permission" lastName="User 1"/> + <UserLogin userLoginId="permUser1" partyId="permUser1"/> + <Party partyId="permUser2" partyTypeId="PERSON"/> + <Person partyId="permUser2" firstName="Permission" lastName="User 2"/> + <UserLogin userLoginId="permUser2" partyId="permUser2"/> + + <UserLoginSecurityGroup groupId="TESTPERMADMIN" userLoginId="permUser1" fromDate="2001-01-01 12:00:00.0"/> + <UserLoginSecurityGroup groupId="TESTPERMVIEW" userLoginId="permUser2" fromDate="2001-01-01 12:00:00.0"/> + +</entity-engine-xml> \ No newline at end of file Propchange: ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml ------------------------------------------------------------------------------ svn:eol-style = native Propchange: ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml ------------------------------------------------------------------------------ svn:keywords = Date Rev Author URL Id Propchange: ofbiz/ofbiz-framework/trunk/framework/service/testdef/data/PermissionServiceTestData.xml ------------------------------------------------------------------------------ svn:mime-type = text/xml Modified: ofbiz/ofbiz-framework/trunk/framework/service/testdef/servicetests.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/service/testdef/servicetests.xml?rev=1866137&r1=1866136&r2=1866137&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/service/testdef/servicetests.xml (original) +++ ofbiz/ofbiz-framework/trunk/framework/service/testdef/servicetests.xml Fri Aug 30 08:47:48 2019 @@ -73,4 +73,11 @@ under the License. <test-case case-name="service-xml-rpc-local-engine"> <service-test service-name="testXmlRpcClientAdd"/> </test-case> + <test-case case-name="load-data-service-permission-tests"> + <entity-xml entity-xml-url="component://service/testdef/data/PermissionServiceTestData.xml"/> + </test-case> + <test-case case-name="service-permission-tests"> + <junit-test-suite class-name="org.apache.ofbiz.service.test.ServicePermissionTests"/> + </test-case> + </test-suite> |
Free forum by Nabble | Edit this page |