You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2007/10/09 09:39:26 UTC
Re: svn commit: r579242 -
/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
Hi Jacopo,
Just did a cursory review, looks good to me. I did not look at the original code (just at your changes).
Isn't the comment :
// final check - will pass if userLogin's partyId = partyId for order or if userLogin has ORDERMGR_CREATE permission
related to the code block where you put a FIXME ?
Jacques
> Author: jacopoc
> Date: Tue Sep 25 06:33:28 2007
> New Revision: 579242
>
> URL: http://svn.apache.org/viewvc?rev=579242&view=rev
> Log:
> First step in the process of refactoring the permission checks on order's creation/update.
> I've tried to isolate in a centralized method all the permission logic for a subset of the order related services; next step will
be that of further cleaning up the logic (and code) and move the logic in a modern permission service.
> To all committers and developers: I'd really appreciate if you could review this commit; by the way I'll continue my tests on
this.
>
> Modified:
> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=579242&r1=579241&r2=579242&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Tue Sep 25 06:33:28 2007
> @@ -92,6 +92,69 @@
> public static final int orderRounding = UtilNumber.getBigDecimalRoundingMode("order.rounding");
> public static final BigDecimal ZERO = BigDecimal.ZERO.setScale(taxDecimals, taxRounding);
>
> +
> + private static boolean hasPermission(String orderId, GenericValue userLogin, String action, Security security,
GenericDelegator delegator) {
> + OrderReadHelper orh = new OrderReadHelper(delegator, orderId);
> + String orderTypeId = orh.getOrderTypeId();
> + String partyId = null;
> + GenericValue orderParty = orh.getEndUserParty();
> + if (UtilValidate.isEmpty(orderParty)) {
> + orderParty = orh.getPlacingParty();
> + }
> + if (UtilValidate.isNotEmpty(orderParty)) {
> + partyId = orderParty.getString("partyId");
> + }
> + boolean hasPermission = hasPermission(orderTypeId, partyId, userLogin, action, security);
> + if (!hasPermission) {
> + GenericValue placingCustomer = null;
> + try {
> + Map placingCustomerFields = UtilMisc.toMap("orderId", orderId, "partyId", userLogin.getString("partyId"),
"roleTypeId", "PLACING_CUSTOMER");
> + placingCustomer = delegator.findByPrimaryKey("OrderRole", placingCustomerFields);
> + } catch (GenericEntityException e) {
> + Debug.logError("Could not select OrderRoles for order " + orderId + " due to " + e.getMessage(), module);
> + }
> + hasPermission = (placingCustomer != null);
> + }
> + return hasPermission;
> + }
> +
> + private static boolean hasPermission(String orderTypeId, String partyId, GenericValue userLogin, String action, Security
security) {
> + boolean hasPermission = security.hasEntityPermission("ORDERMGR", "_" + action, userLogin);
> + if (!hasPermission) {
> + if (orderTypeId.equals("SALES_ORDER")) {
> + if (security.hasEntityPermission("ORDERMGR", "_SALES_" + action, userLogin)) {
> + hasPermission = true;
> + } else {
> + // check sales agent/customer relationship
> + List repsCustomers = new LinkedList();
> + try {
> + repsCustomers =
EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship",
> + UtilMisc.toMap("roleTypeIdFrom", "AGENT", "roleTypeIdTo", "CUSTOMER", "partyIdTo", partyId)));
> + } catch (GenericEntityException ex) {
> + Debug.logError("Could not determine if " + partyId + " is a customer of user " +
userLogin.getString("userLoginId") + " due to " + ex.getMessage(), module);
> + }
> + if ((repsCustomers != null) && (repsCustomers.size() > 0) && (security.hasEntityPermission("SALESREP",
"_ORDER_" + action, userLogin))) {
> + hasPermission = true;
> + }
> + if (!hasPermission) {
> + // check sales sales rep/customer relationship
> + try {
> + repsCustomers =
EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship",
> + UtilMisc.toMap("roleTypeIdFrom", "SALES_REP", "roleTypeIdTo", "CUSTOMER", "partyIdTo",
partyId)));
> + } catch (GenericEntityException ex) {
> + Debug.logError("Could not determine if " + partyId + " is a customer of user " +
userLogin.getString("userLoginId") + " due to " + ex.getMessage(), module);
> + }
> + if ((repsCustomers != null) && (repsCustomers.size() > 0) && (security.hasEntityPermission("SALESREP",
"_ORDER_" + action, userLogin))) {
> + hasPermission = true;
> + }
> + }
> + }
> + } else if ((orderTypeId.equals("PURCHASE_ORDER") && (security.hasEntityPermission("ORDERMGR", "_PURCHASE_" + action,
userLogin)))) {
> + hasPermission = true;
> + }
> + }
> + return hasPermission;
> + }
> /** Service for creating a new order */
> public static Map createOrder(DispatchContext ctx, Map context) {
> GenericDelegator delegator = ctx.getDelegator();
> @@ -112,39 +175,9 @@
> // if it is an AGENT (sales rep) creating an order for his customer
> // PURCHASE ORDERS - if there is a PURCHASE_ORDER permission
> Map resultSecurity = new HashMap();
> - boolean hasPermission = false;
> - if (orderTypeId.equals("SALES_ORDER")) {
> - if (security.hasEntityPermission("ORDERMGR", "_SALES_CREATE", userLogin)) {
> - hasPermission = true;
> - } else {
> - // check sales agent/customer relationship
> - List repsCustomers = new LinkedList();
> - try {
> - repsCustomers =
EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship",
> - UtilMisc.toMap("roleTypeIdFrom", "AGENT", "roleTypeIdTo", "CUSTOMER", "partyIdTo", partyId)));
> - } catch (GenericEntityException ex) {
> - Debug.logError("Could not determine if " + partyId + " is a customer of user " +
userLogin.getString("userLoginId") + " due to " + ex.getMessage(), module);
> - }
> - if ((repsCustomers != null) && (repsCustomers.size() > 0) && (security.hasEntityPermission("SALESREP",
"_ORDER_CREATE", userLogin))) {
> - hasPermission = true;
> - }
> - if (!hasPermission) {
> - // check sales sales rep/customer relationship
> - try {
> - repsCustomers =
EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship",
> - UtilMisc.toMap("roleTypeIdFrom", "SALES_REP", "roleTypeIdTo", "CUSTOMER", "partyIdTo",
partyId)));
> - } catch (GenericEntityException ex) {
> - Debug.logError("Could not determine if " + partyId + " is a customer of user " +
userLogin.getString("userLoginId") + " due to " + ex.getMessage(), module);
> - }
> - if ((repsCustomers != null) && (repsCustomers.size() > 0) && (security.hasEntityPermission("SALESREP",
"_ORDER_CREATE", userLogin))) {
> - hasPermission = true;
> - }
> - }
> - }
> - } else if ((orderTypeId.equals("PURCHASE_ORDER") && (security.hasEntityPermission("ORDERMGR", "_PURCHASE_CREATE",
userLogin)))) {
> - hasPermission = true;
> - }
> + boolean hasPermission = OrderServices.hasPermission(orderTypeId, partyId, userLogin, "CREATE", security);
> // final check - will pass if userLogin's partyId = partyId for order or if userLogin has ORDERMGR_CREATE permission
> + // jacopoc: what is the meaning of this code block? FIXME
> if (!hasPermission) {
> partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, resultSecurity, "ORDERMGR", "_CREATE");
> if (resultSecurity.size() > 0) {
> @@ -1266,16 +1299,9 @@
>
> // check and make sure we have permission to change the order
> Security security = ctx.getSecurity();
> - if (!security.hasEntityPermission("ORDERMGR", "_UPDATE", userLogin)) {
> - GenericValue placingCustomer = null;
> - try {
> - Map placingCustomerFields = UtilMisc.toMap("orderId", orderId, "partyId", userLogin.getString("partyId"),
"roleTypeId", "PLACING_CUSTOMER");
> - placingCustomer = delegator.findByPrimaryKey("OrderRole", placingCustomerFields);
> - } catch (GenericEntityException e) {
> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderErrorCannotGetOrderRoleEntity
",locale) + e.getMessage());
> - }
> - if (placingCustomer == null)
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> + boolean hasPermission = OrderServices.hasPermission(orderId, userLogin, "UPDATE", security, delegator);
> + if (!hasPermission) {
> + return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> }
>
> // get the order header
> @@ -1489,16 +1515,9 @@
>
> // check and make sure we have permission to change the order
> Security security = ctx.getSecurity();
> - if (!security.hasEntityPermission("ORDERMGR", "_UPDATE", userLogin)) {
> - GenericValue placingCustomer = null;
> - try {
> - Map placingCustomerFields = UtilMisc.toMap("orderId", orderId, "partyId", userLogin.getString("partyId"),
"roleTypeId", "PLACING_CUSTOMER");
> - placingCustomer = delegator.findByPrimaryKey("OrderRole", placingCustomerFields);
> - } catch (GenericEntityException e) {
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderErrorCannotGetOrderRoleEntity",locale) + e.getMessage());
> - }
> - if (placingCustomer == null)
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> + boolean hasPermission = OrderServices.hasPermission(orderId, userLogin, "UPDATE", security, delegator);
> + if (!hasPermission) {
> + return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> }
>
> // get the order header
> @@ -1593,17 +1612,9 @@
>
> // check and make sure we have permission to change the order
> Security security = ctx.getSecurity();
> - if (!security.hasEntityPermission("ORDERMGR", "_UPDATE", userLogin)) {
> - GenericValue placingCustomer = null;
> - try {
> - Map placingCustomerFields = UtilMisc.toMap("orderId", orderId, "partyId", userLogin.getString("partyId"),
"roleTypeId", "PLACING_CUSTOMER");
> - placingCustomer = delegator.findByPrimaryKey("OrderRole", placingCustomerFields);
> - } catch (GenericEntityException e) {
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderErrorCannotGetOrderRoleEntity",locale) + e.getMessage());
> - }
> - if (placingCustomer == null) {
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> - }
> + boolean hasPermission = OrderServices.hasPermission(orderId, userLogin, "UPDATE", security, delegator);
> + if (!hasPermission) {
> + return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> }
>
> // get the order header
> @@ -1734,17 +1745,10 @@
>
> // check and make sure we have permission to change the order
> Security security = ctx.getSecurity();
> - if (!security.hasEntityPermission("ORDERMGR", "_UPDATE", userLogin)) {
> - GenericValue placingCustomer = null;
> - try {
> - Map placingCustomerFields = UtilMisc.toMap("orderId", orderId, "partyId", userLogin.getString("partyId"),
"roleTypeId", "PLACING_CUSTOMER");
> - placingCustomer = delegator.findByPrimaryKey("OrderRole", placingCustomerFields);
> - } catch (GenericEntityException e) {
> - Debug.logError(e, module);
> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderErrorCannotGetOrderRoleEntity",
UtilMisc.toMap("itemMsgInfo",itemMsgInfo),locale));
> - }
> - if (placingCustomer == null)
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> +
> + boolean hasPermission = OrderServices.hasPermission(orderId, userLogin, "UPDATE", security, delegator);
> + if (!hasPermission) {
> + return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> }
>
> Map fields = UtilMisc.toMap("orderId", orderId);
> @@ -1847,16 +1851,9 @@
>
> // check and make sure we have permission to change the order
> Security security = ctx.getSecurity();
> - if (!security.hasEntityPermission("ORDERMGR", "_UPDATE", userLogin)) {
> - GenericValue placingCustomer = null;
> - try {
> - Map placingCustomerFields = UtilMisc.toMap("orderId", orderId, "partyId", userLogin.getString("partyId"),
"roleTypeId", "PLACING_CUSTOMER");
> - placingCustomer = delegator.findByPrimaryKey("OrderRole", placingCustomerFields);
> - } catch (GenericEntityException e) {
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderErrorCannotGetOrderRoleEntity",locale) + e.getMessage());
> - }
> - if (placingCustomer == null)
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> + boolean hasPermission = OrderServices.hasPermission(orderId, userLogin, "UPDATE", security, delegator);
> + if (!hasPermission) {
> + return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> }
>
> Map fields = UtilMisc.toMap("orderId", orderId);
> @@ -1943,17 +1940,9 @@
>
> // check and make sure we have permission to change the order
> Security security = ctx.getSecurity();
> - if (!security.hasEntityPermission("ORDERMGR", "_UPDATE", userLogin)) {
> - GenericValue placingCustomer = null;
> - try {
> - Map placingCustomerFields = UtilMisc.toMap("orderId", orderId, "partyId", userLogin.getString("partyId"),
"roleTypeId", "PLACING_CUSTOMER");
> - placingCustomer = delegator.findByPrimaryKey("OrderRole", placingCustomerFields);
> - } catch (GenericEntityException e) {
> - return ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderErrorCannotGetOrderRoleEntity",
locale) + e.getMessage());
> - }
> - if (placingCustomer == null) {
> - return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> - }
> + boolean hasPermission = OrderServices.hasPermission(orderId, userLogin, "UPDATE", security, delegator);
> + if (!hasPermission) {
> + return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,"OrderYouDoNotHavePermissionToChangeThisOrdersStatus",locale));
> }
>
> if ("Y".equals(context.get("setItemStatus"))) {
>
>