Re: svn commit: r1353381 - in /ofbiz/trunk: applications/accounting/data/ applications/accounting/script/org/ofbiz/accounting/payment/ applications/accounting/src/org/ofbiz/accounting/payment/ applications/accounting/widget/ applications/order/webapp/order...

[Jacopo Cappellato-4]

Hi Hans,

I understand the issue you are fixing in this commit (and at least another one of last week) but I disagree with the approach.
We should never add permission checks on the *_ADMIN permissions: we should instead always use one of the fine grained _CREATE, _UPDATE, _DELETE; the _ADMIN permission will be automatically checked by the framework if the fine grained ones fail.
So instead of (for example):

> @@ -24,6 +24,7 @@ under the License.
>         <if>
>             <condition>
>                 <and>
> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>

you should have:

> @@ -24,6 +24,7 @@ under the License.
>         <if>
>             <condition>
>                 <and>
> +                    <not><if-has-permission permission="ACCOUNTING" action="_CREATE"/></not>
>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>



The code above will grant access to users having at least one of the following:
PAYINFO_CREATE
PAYINFO_ADMIN
ACCOUNTING_CREATE
ACCOUNTING_ADMIN

Kind regards,

Jacopo


On Jun 25, 2012, at 4:23 AM, [hidden email] wrote:

> Author: hansbak
> Date: Mon Jun 25 02:22:58 2012
> New Revision: 1353381
>
> URL: http://svn.apache.org/viewvc?rev=1353381&view=rev
> Log:
> Give ACCOUNTING_ADMIN the same access as PAY_INFO_ADMIN because part of accounting component
>
> Modified:
>    ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>    ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>    ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>    ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>    ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>    ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>    ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>    ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>
> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml (original)
> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml Mon Jun 25 02:22:58 2012
> @@ -26,7 +26,6 @@ under the License.
>     <SecurityGroupPermission groupId="BIZADMIN" permissionId="PAYPROC_ADMIN"/>
>
>     <!-- Payment Information security -->
> -    <SecurityGroupPermission groupId="FULLADMIN" permissionId="PAY_INFO_ADMIN"/>
>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_CREATE"/>
>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_DELETE"/>
>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_UPDATE"/>
>
> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml (original)
> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml Mon Jun 25 02:22:58 2012
> @@ -68,7 +68,6 @@ under the License.
>
>     <!-- add admin to SUPER permission group -->
>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_ADMIN"/>
> -    <SecurityGroupPermission groupId="SUPER" permissionId="PAY_INFO_ADMIN"/>
>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_COMM_VIEW"/>
>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_PRINT_CHECKS"/>
>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCTG_PREF_ADMIN"/>
>
> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml (original)
> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml Mon Jun 25 02:22:58 2012
> @@ -24,6 +24,7 @@ under the License.
>         <if>
>             <condition>
>                 <and>
> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
> @@ -86,6 +87,7 @@ under the License.
>         <if>
>             <condition>
>                 <and>
> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>                     <not><if-has-permission permission="PAY_INFO" action="_UPDATE"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdFrom" operator="equals"/></not>
>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdTo" operator="equals"/></not>
>
> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (original)
> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java Mon Jun 25 02:22:58 2012
> @@ -89,7 +89,7 @@ public class PaymentMethodServices {
>
>         // <b>security check</b>: userLogin partyId must equal paymentMethod partyId, or must have PAY_INFO_DELETE permission
>         if (paymentMethod.get("partyId") == null || !paymentMethod.getString("partyId").equals(userLogin.getString("partyId"))) {
> -            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin)) {
> +            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>                 return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>                         "AccountingPaymentMethodNoPermissionToDelete", locale));
>             }
> @@ -139,7 +139,7 @@ public class PaymentMethodServices {
>
>         Timestamp now = UtilDateTime.nowTimestamp();
>
> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>
>         if (result.size() > 0) return result;
>
> @@ -260,7 +260,7 @@ public class PaymentMethodServices {
>
>         Timestamp now = UtilDateTime.nowTimestamp();
>
> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>
>         if (result.size() > 0) return result;
>
> @@ -286,7 +286,7 @@ public class PaymentMethodServices {
>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>                     "AccountingCreditCardUpdateWithPaymentMethodId", locale) + paymentMethodId);
>         }
> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>                     "AccountingCreditCardUpdateWithoutPermission", UtilMisc.toMap("partyId", partyId,
>                             "paymentMethodId", paymentMethodId), locale));
> @@ -488,7 +488,7 @@ public class PaymentMethodServices {
>
>         Timestamp now = UtilDateTime.nowTimestamp();
>
> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>
>         if (result.size() > 0)
>             return result;
> @@ -545,7 +545,7 @@ public class PaymentMethodServices {
>
>         Timestamp now = UtilDateTime.nowTimestamp();
>
> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>
>         if (result.size() > 0)
>             return result;
> @@ -574,7 +574,7 @@ public class PaymentMethodServices {
>                     "AccountingGiftCardCannotBeUpdated",
>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>         }
> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>                     "AccountingGiftCardPartyNotAuthorized",
>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
> @@ -679,7 +679,7 @@ public class PaymentMethodServices {
>
>         Timestamp now = UtilDateTime.nowTimestamp();
>
> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>
>         if (result.size() > 0) return result;
>
> @@ -777,7 +777,7 @@ public class PaymentMethodServices {
>
>         Timestamp now = UtilDateTime.nowTimestamp();
>
> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>
>         if (result.size() > 0) return result;
>
> @@ -806,7 +806,7 @@ public class PaymentMethodServices {
>                     "AccountingEftAccountCannotBeUpdated",
>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>         }
> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>                     "AccountingEftAccountCannotBeUpdated",
>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>
> Modified: ofbiz/trunk/applications/accounting/widget/GlScreens.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/widget/GlScreens.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/widget/GlScreens.xml (original)
> +++ ofbiz/trunk/applications/accounting/widget/GlScreens.xml Mon Jun 25 02:22:58 2012
> @@ -445,7 +445,12 @@ under the License.
>                 <decorator-screen name="CommonAdminChecksDecorator" location="${parameters.mainDecoratorLocation}">
>                     <decorator-section name="checks-body">
>                         <section>
> -                        <condition><if-has-permission permission="PAY_INFO" action="_UPDATE"/></condition>
> +                        <condition>
> +                            <or>
> +                                <if-has-permission permission="ACCOUNTING" action="_ADMIN"/>
> +                                <if-has-permission permission="PAY_INFO" action="_UPDATE"/>
> +                            </or>
> +                        </condition>
>                         <widgets>
>                             <screenlet title="${uiLabelMap.AccountingSendChecks}">
>                                 <include-form name="ListChecksToSend" location="component://accounting/widget/PaymentForms.xml"/>
>
> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl (original)
> +++ ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl Mon Jun 25 02:22:58 2012
> @@ -54,7 +54,7 @@ under the License.
>            <#assign statusItem = payment.getRelatedOne("StatusItem", false)>
>            <#assign partyName = delegator.findOne("PartyNameView", {"partyId" : payment.partyIdTo}, true)>
>            <tr>
> -             <#if security.hasPermission("PAY_INFO_VIEW", session) || security.hasPermission("PAY_INFO_ADMIN", session)>
> +             <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                <td><a href="/accounting/control/paymentOverview?paymentId=${payment.paymentId}">${payment.paymentId}</a></td>
>              <#else>
>                <td>${payment.paymentId}</td>
> @@ -342,7 +342,7 @@ under the License.
>                       <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>                       <br />
>
> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                         ${creditCard.cardType}
>                         <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>                         ${creditCard.expireDate}
> @@ -469,7 +469,7 @@ under the License.
>                 <td valign="top" width="60%">
>                   <div>
>                     <#if giftCard?has_content>
> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                         ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>                         &nbsp;[<#if oppStatusItem?exists>${oppStatusItem.get("description",locale)}<#else>${orderPaymentPreference.statusId}</#if>]
>                       <#else>
> @@ -596,7 +596,7 @@ under the License.
>                <#if "CREDIT_CARD" == paymentMethod.paymentMethodTypeId>
>                  <#assign creditCard = paymentMethodValueMap.creditCard/>
>                  <#if (creditCard?has_content)>
> -                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
> +                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                      ${creditCard.cardType?if_exists} <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/> ${creditCard.expireDate?if_exists}
>                    <#else>
>                      ${Static["org.ofbiz.party.contact.ContactHelper"].formatCreditCard(creditCard)}
>
> Modified: ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy (original)
> +++ ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy Mon Jun 25 02:22:58 2012
> @@ -23,7 +23,7 @@ context.hasCreatePermission = security.h
> context.hasUpdatePermission = security.hasEntityPermission("PARTYMGR", "_UPDATE", session);
> context.hasDeletePermission = security.hasEntityPermission("PARTYMGR", "_DELETE", session);
> // extended pay_info permissions
> -context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session);
> +context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session);
> // extended pcm (party contact mechanism) permissions
> context.hasPcmCreatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_CREATE", session);
> context.hasPcmUpdatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_UPDATE", session);
>
> Modified: ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl (original)
> +++ ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl Mon Jun 25 02:22:58 2012
> @@ -38,7 +38,7 @@ under the License.
>     <div class="screenlet-title-bar">
>       <ul>
>         <li class="h3">${uiLabelMap.PartyPaymentMethodInformation}</li>
> -        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session)>
> +        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>           <li><a href="<@ofbizUrl>editeftaccount?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewEftAccount}</a></li>
>           <li><a href="<@ofbizUrl>editgiftcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewGiftCard}</a></li>
>           <li><a href="<@ofbizUrl>editcreditcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewCreditCard}</a></li>
> @@ -67,7 +67,7 @@ under the License.
>                   ${creditCard.lastNameOnCard}
>                   <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>                   &nbsp;-&nbsp;
> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                     ${creditCard.cardType}
>                     <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>                     ${creditCard.expireDate}
> @@ -83,7 +83,7 @@ under the License.
>                   <#if security.hasEntityPermission("MANUAL", "_PAYMENT", session)>
>                     <a href="/accounting/control/manualETx?paymentMethodId=${paymentMethod.paymentMethodId}${externalKeyParam}">${uiLabelMap.PartyManualTx}</a>
>                   </#if>
> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                     <a href="<@ofbizUrl>editcreditcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>                   </#if>
>                 <#-- </td> -->
> @@ -93,7 +93,7 @@ under the License.
>                   ${uiLabelMap.AccountingGiftCard}
>                 </td>
>                 <td>
> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                     ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>                   <#else>
>                     <@maskSensitiveNumber cardNumber=giftCard.cardNumber?if_exists/>
> @@ -105,7 +105,7 @@ under the License.
>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</b></#if>
>                 </td>
>                 <td class="button-col">
> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                     <a href="<@ofbizUrl>editgiftcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>                   </#if>
>                 <#-- </td> -->
> @@ -121,7 +121,7 @@ under the License.
>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</#if>
>                 </td>
>                 <td class="button-col">
> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                     <a href="<@ofbizUrl>editeftaccount?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>                   </#if>
>                 <#-- </td> -->
> @@ -143,7 +143,7 @@ under the License.
>                 <td class="button-col">
>                   &nbsp;
>               </#if>
> -              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session)>
> +              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>                 <a href="<@ofbizUrl>deletePaymentMethod/viewprofile?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonExpire}</a>
>               <#else>
>                 &nbsp;
>
> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java?rev=1353381&r1=1353380&r2=1353381&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java (original)
> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java Mon Jun 25 02:22:58 2012
> @@ -184,6 +184,9 @@ public class ServiceUtil {
>      *<b>security check</b>: userLogin partyId must equal partyId, or must have [secEntity][secOperation] permission
>      */
>     public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation) {
> +        return getPartyIdCheckSecurity(userLogin, security, context, result, secEntity, secOperation, null, null);
> +    }
> +    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation, String adminSecEntity, String adminSecOperation) {
>         String partyId = (String) context.get("partyId");
>         Locale locale = getLocale(context);
>         if (UtilValidate.isEmpty(partyId)) {
> @@ -198,9 +201,9 @@ public class ServiceUtil {
>             return partyId;
>         }
>
> -        // <b>security check</b>: userLogin partyId must equal partyId, or must have PARTYMGR_CREATE permission
> +        // <b>security check</b>: userLogin partyId must equal partyId, or must have either of the two permissions
>         if (!partyId.equals(userLogin.getString("partyId"))) {
> -            if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
> +            if (!security.hasEntityPermission(secEntity, secOperation, userLogin) && !(adminSecEntity != null && adminSecOperation != null && security.hasEntityPermission(adminSecEntity, adminSecOperation, userLogin))) {
>                 result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_ERROR);
>                 String errMsg = UtilProperties.getMessage(ServiceUtil.resource, "serviceUtil.no_permission_to_operation", locale) + ".";
>                 result.put(ModelService.ERROR_MESSAGE, errMsg);
>
>

[hans_bakker]

Hi Jacopo,

thanks for reviewing this commit, my compliments for your work in this
area.

The question here is do you really want to allow access for
ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps
the intention here was that, by creating a separate action PAY_INFO the
ACCOUNTING_CREATE should not have access.

The reason of my commit is that ACCOUNTING_ADMIN should really, as the
description states, have full access to the complete accounting
component and I did not want to change the other permissions.

Personally i am fine with your suggestion and sure yes, we can do it
that way.....

Regards,
Hans


On 06/25/2012 09:27 PM, Jacopo Cappellato wrote:

> Hi Hans,
>
> I understand the issue you are fixing in this commit (and at least another one of last week) but I disagree with the approach.
> We should never add permission checks on the *_ADMIN permissions: we should instead always use one of the fine grained _CREATE, _UPDATE, _DELETE; the _ADMIN permission will be automatically checked by the framework if the fine grained ones fail.
> So instead of (for example):
>
>> @@ -24,6 +24,7 @@ under the License.
>>          <if>
>>              <condition>
>>                  <and>
>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>                      <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
> you should have:
>
>> @@ -24,6 +24,7 @@ under the License.
>>          <if>
>>              <condition>
>>                  <and>
>> +                    <not><if-has-permission permission="ACCOUNTING" action="_CREATE"/></not>
>>                      <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>
>
> The code above will grant access to users having at least one of the following:
> PAYINFO_CREATE
> PAYINFO_ADMIN
> ACCOUNTING_CREATE
> ACCOUNTING_ADMIN
>
> Kind regards,
>
> Jacopo
>
>
> On Jun 25, 2012, at 4:23 AM, [hidden email] wrote:
>
>> Author: hansbak
>> Date: Mon Jun 25 02:22:58 2012
>> New Revision: 1353381
>>
>> URL: http://svn.apache.org/viewvc?rev=1353381&view=rev
>> Log:
>> Give ACCOUNTING_ADMIN the same access as PAY_INFO_ADMIN because part of accounting component
>>
>> Modified:
>>     ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>     ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>     ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>     ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>     ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>     ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>     ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>     ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>     ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>
>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml (original)
>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml Mon Jun 25 02:22:58 2012
>> @@ -26,7 +26,6 @@ under the License.
>>      <SecurityGroupPermission groupId="BIZADMIN" permissionId="PAYPROC_ADMIN"/>
>>
>>      <!-- Payment Information security -->
>> -    <SecurityGroupPermission groupId="FULLADMIN" permissionId="PAY_INFO_ADMIN"/>
>>      <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_CREATE"/>
>>      <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_DELETE"/>
>>      <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_UPDATE"/>
>>
>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml (original)
>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml Mon Jun 25 02:22:58 2012
>> @@ -68,7 +68,6 @@ under the License.
>>
>>      <!-- add admin to SUPER permission group -->
>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_ADMIN"/>
>> -    <SecurityGroupPermission groupId="SUPER" permissionId="PAY_INFO_ADMIN"/>
>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_COMM_VIEW"/>
>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_PRINT_CHECKS"/>
>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCTG_PREF_ADMIN"/>
>>
>> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml (original)
>> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml Mon Jun 25 02:22:58 2012
>> @@ -24,6 +24,7 @@ under the License.
>>          <if>
>>              <condition>
>>                  <and>
>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>                      <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>> @@ -86,6 +87,7 @@ under the License.
>>          <if>
>>              <condition>
>>                  <and>
>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>                      <not><if-has-permission permission="PAY_INFO" action="_UPDATE"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdFrom" operator="equals"/></not>
>>                      <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdTo" operator="equals"/></not>
>>
>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (original)
>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java Mon Jun 25 02:22:58 2012
>> @@ -89,7 +89,7 @@ public class PaymentMethodServices {
>>
>>          // <b>security check</b>: userLogin partyId must equal paymentMethod partyId, or must have PAY_INFO_DELETE permission
>>          if (paymentMethod.get("partyId") == null || !paymentMethod.getString("partyId").equals(userLogin.getString("partyId"))) {
>> -            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin)) {
>> +            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>                  return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>                          "AccountingPaymentMethodNoPermissionToDelete", locale));
>>              }
>> @@ -139,7 +139,7 @@ public class PaymentMethodServices {
>>
>>          Timestamp now = UtilDateTime.nowTimestamp();
>>
>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>
>>          if (result.size() > 0) return result;
>>
>> @@ -260,7 +260,7 @@ public class PaymentMethodServices {
>>
>>          Timestamp now = UtilDateTime.nowTimestamp();
>>
>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>
>>          if (result.size() > 0) return result;
>>
>> @@ -286,7 +286,7 @@ public class PaymentMethodServices {
>>              return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>                      "AccountingCreditCardUpdateWithPaymentMethodId", locale) + paymentMethodId);
>>          }
>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>              return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>                      "AccountingCreditCardUpdateWithoutPermission", UtilMisc.toMap("partyId", partyId,
>>                              "paymentMethodId", paymentMethodId), locale));
>> @@ -488,7 +488,7 @@ public class PaymentMethodServices {
>>
>>          Timestamp now = UtilDateTime.nowTimestamp();
>>
>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>
>>          if (result.size() > 0)
>>              return result;
>> @@ -545,7 +545,7 @@ public class PaymentMethodServices {
>>
>>          Timestamp now = UtilDateTime.nowTimestamp();
>>
>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>
>>          if (result.size() > 0)
>>              return result;
>> @@ -574,7 +574,7 @@ public class PaymentMethodServices {
>>                      "AccountingGiftCardCannotBeUpdated",
>>                      UtilMisc.toMap("errorString", paymentMethodId), locale));
>>          }
>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>              return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>                      "AccountingGiftCardPartyNotAuthorized",
>>                      UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>> @@ -679,7 +679,7 @@ public class PaymentMethodServices {
>>
>>          Timestamp now = UtilDateTime.nowTimestamp();
>>
>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>
>>          if (result.size() > 0) return result;
>>
>> @@ -777,7 +777,7 @@ public class PaymentMethodServices {
>>
>>          Timestamp now = UtilDateTime.nowTimestamp();
>>
>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>
>>          if (result.size() > 0) return result;
>>
>> @@ -806,7 +806,7 @@ public class PaymentMethodServices {
>>                      "AccountingEftAccountCannotBeUpdated",
>>                      UtilMisc.toMap("errorString", paymentMethodId), locale));
>>          }
>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>              return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>                      "AccountingEftAccountCannotBeUpdated",
>>                      UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>
>> Modified: ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/widget/GlScreens.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/accounting/widget/GlScreens.xml (original)
>> +++ ofbiz/trunk/applications/accounting/widget/GlScreens.xml Mon Jun 25 02:22:58 2012
>> @@ -445,7 +445,12 @@ under the License.
>>                  <decorator-screen name="CommonAdminChecksDecorator" location="${parameters.mainDecoratorLocation}">
>>                      <decorator-section name="checks-body">
>>                          <section>
>> -                        <condition><if-has-permission permission="PAY_INFO" action="_UPDATE"/></condition>
>> +                        <condition>
>> +                            <or>
>> +                                <if-has-permission permission="ACCOUNTING" action="_ADMIN"/>
>> +                                <if-has-permission permission="PAY_INFO" action="_UPDATE"/>
>> +                            </or>
>> +                        </condition>
>>                          <widgets>
>>                              <screenlet title="${uiLabelMap.AccountingSendChecks}">
>>                                  <include-form name="ListChecksToSend" location="component://accounting/widget/PaymentForms.xml"/>
>>
>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl (original)
>> +++ ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl Mon Jun 25 02:22:58 2012
>> @@ -54,7 +54,7 @@ under the License.
>>             <#assign statusItem = payment.getRelatedOne("StatusItem", false)>
>>             <#assign partyName = delegator.findOne("PartyNameView", {"partyId" : payment.partyIdTo}, true)>
>>             <tr>
>> -             <#if security.hasPermission("PAY_INFO_VIEW", session) || security.hasPermission("PAY_INFO_ADMIN", session)>
>> +             <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                 <td><a href="/accounting/control/paymentOverview?paymentId=${payment.paymentId}">${payment.paymentId}</a></td>
>>               <#else>
>>                 <td>${payment.paymentId}</td>
>> @@ -342,7 +342,7 @@ under the License.
>>                        <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>                        <br />
>>
>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                          ${creditCard.cardType}
>>                          <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>                          ${creditCard.expireDate}
>> @@ -469,7 +469,7 @@ under the License.
>>                  <td valign="top" width="60%">
>>                    <div>
>>                      <#if giftCard?has_content>
>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                          ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>                          &nbsp;[<#if oppStatusItem?exists>${oppStatusItem.get("description",locale)}<#else>${orderPaymentPreference.statusId}</#if>]
>>                        <#else>
>> @@ -596,7 +596,7 @@ under the License.
>>                 <#if "CREDIT_CARD" == paymentMethod.paymentMethodTypeId>
>>                   <#assign creditCard = paymentMethodValueMap.creditCard/>
>>                   <#if (creditCard?has_content)>
>> -                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>> +                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                       ${creditCard.cardType?if_exists} <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/> ${creditCard.expireDate?if_exists}
>>                     <#else>
>>                       ${Static["org.ofbiz.party.contact.ContactHelper"].formatCreditCard(creditCard)}
>>
>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy (original)
>> +++ ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy Mon Jun 25 02:22:58 2012
>> @@ -23,7 +23,7 @@ context.hasCreatePermission = security.h
>> context.hasUpdatePermission = security.hasEntityPermission("PARTYMGR", "_UPDATE", session);
>> context.hasDeletePermission = security.hasEntityPermission("PARTYMGR", "_DELETE", session);
>> // extended pay_info permissions
>> -context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session);
>> +context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session);
>> // extended pcm (party contact mechanism) permissions
>> context.hasPcmCreatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_CREATE", session);
>> context.hasPcmUpdatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_UPDATE", session);
>>
>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl (original)
>> +++ ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl Mon Jun 25 02:22:58 2012
>> @@ -38,7 +38,7 @@ under the License.
>>      <div class="screenlet-title-bar">
>>        <ul>
>>          <li class="h3">${uiLabelMap.PartyPaymentMethodInformation}</li>
>> -        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session)>
>> +        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>            <li><a href="<@ofbizUrl>editeftaccount?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewEftAccount}</a></li>
>>            <li><a href="<@ofbizUrl>editgiftcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewGiftCard}</a></li>
>>            <li><a href="<@ofbizUrl>editcreditcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewCreditCard}</a></li>
>> @@ -67,7 +67,7 @@ under the License.
>>                    ${creditCard.lastNameOnCard}
>>                    <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>                    &nbsp;-&nbsp;
>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                      ${creditCard.cardType}
>>                      <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>                      ${creditCard.expireDate}
>> @@ -83,7 +83,7 @@ under the License.
>>                    <#if security.hasEntityPermission("MANUAL", "_PAYMENT", session)>
>>                      <a href="/accounting/control/manualETx?paymentMethodId=${paymentMethod.paymentMethodId}${externalKeyParam}">${uiLabelMap.PartyManualTx}</a>
>>                    </#if>
>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                      <a href="<@ofbizUrl>editcreditcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>                    </#if>
>>                  <#-- </td> -->
>> @@ -93,7 +93,7 @@ under the License.
>>                    ${uiLabelMap.AccountingGiftCard}
>>                  </td>
>>                  <td>
>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                      ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>                    <#else>
>>                      <@maskSensitiveNumber cardNumber=giftCard.cardNumber?if_exists/>
>> @@ -105,7 +105,7 @@ under the License.
>>                    <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</b></#if>
>>                  </td>
>>                  <td class="button-col">
>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                      <a href="<@ofbizUrl>editgiftcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>                    </#if>
>>                  <#-- </td> -->
>> @@ -121,7 +121,7 @@ under the License.
>>                    <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</#if>
>>                  </td>
>>                  <td class="button-col">
>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                      <a href="<@ofbizUrl>editeftaccount?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>                    </#if>
>>                  <#-- </td> -->
>> @@ -143,7 +143,7 @@ under the License.
>>                  <td class="button-col">
>>                    &nbsp;
>>                </#if>
>> -              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session)>
>> +              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>                  <a href="<@ofbizUrl>deletePaymentMethod/viewprofile?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonExpire}</a>
>>                <#else>
>>                  &nbsp;
>>
>> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java (original)
>> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java Mon Jun 25 02:22:58 2012
>> @@ -184,6 +184,9 @@ public class ServiceUtil {
>>       *<b>security check</b>: userLogin partyId must equal partyId, or must have [secEntity][secOperation] permission
>>       */
>>      public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation) {
>> +        return getPartyIdCheckSecurity(userLogin, security, context, result, secEntity, secOperation, null, null);
>> +    }
>> +    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation, String adminSecEntity, String adminSecOperation) {
>>          String partyId = (String) context.get("partyId");
>>          Locale locale = getLocale(context);
>>          if (UtilValidate.isEmpty(partyId)) {
>> @@ -198,9 +201,9 @@ public class ServiceUtil {
>>              return partyId;
>>          }
>>
>> -        // <b>security check</b>: userLogin partyId must equal partyId, or must have PARTYMGR_CREATE permission
>> +        // <b>security check</b>: userLogin partyId must equal partyId, or must have either of the two permissions
>>          if (!partyId.equals(userLogin.getString("partyId"))) {
>> -            if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
>> +            if (!security.hasEntityPermission(secEntity, secOperation, userLogin) && !(adminSecEntity != null && adminSecOperation != null && security.hasEntityPermission(adminSecEntity, adminSecOperation, userLogin))) {
>>                  result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_ERROR);
>>                  String errMsg = UtilProperties.getMessage(ServiceUtil.resource, "serviceUtil.no_permission_to_operation", locale) + ".";
>>                  result.put(ModelService.ERROR_MESSAGE, errMsg);
>>
>>

[Jacopo Cappellato-4]

Hi Hans,

I think that the intended meaning is the following:

ACCOUNTING_<ACTION>: global permission to all parts of the Accounting application when performing the <ACTION>
ACCOUNTING_ADMIN: global permission to all parts of the Accounting application when performing all the actions

So for example:

ACCOUNTING_CREATE: global permission to CREATE all records in the Accounting application

In other words:

ACCOUNTING_ADMIN = ACCOUNTING_CREATE + ACCOUNTING_UPDATE + ACCOUNTING_DELETE + ACCOUNTING_VIEW

(and it is not more than this).

Jacopo


On Jun 25, 2012, at 9:46 PM, Hans Bakker wrote:

> Hi Jacopo,
>
> thanks for reviewing this commit, my compliments for your work in this area.
>
> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>
> The reason of my commit is that ACCOUNTING_ADMIN should really, as the description states, have full access to the complete accounting component and I did not want to change the other permissions.
>
> Personally i am fine with your suggestion and sure yes, we can do it that way.....
>
> Regards,
> Hans
>
>
> On 06/25/2012 09:27 PM, Jacopo Cappellato wrote:
>> Hi Hans,
>>
>> I understand the issue you are fixing in this commit (and at least another one of last week) but I disagree with the approach.
>> We should never add permission checks on the *_ADMIN permissions: we should instead always use one of the fine grained _CREATE, _UPDATE, _DELETE; the _ADMIN permission will be automatically checked by the framework if the fine grained ones fail.
>> So instead of (for example):
>>
>>> @@ -24,6 +24,7 @@ under the License.
>>>         <if>
>>>             <condition>
>>>                 <and>
>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>> you should have:
>>
>>> @@ -24,6 +24,7 @@ under the License.
>>>         <if>
>>>             <condition>
>>>                 <and>
>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_CREATE"/></not>
>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>
>>
>> The code above will grant access to users having at least one of the following:
>> PAYINFO_CREATE
>> PAYINFO_ADMIN
>> ACCOUNTING_CREATE
>> ACCOUNTING_ADMIN
>>
>> Kind regards,
>>
>> Jacopo
>>
>>
>> On Jun 25, 2012, at 4:23 AM, [hidden email] wrote:
>>
>>> Author: hansbak
>>> Date: Mon Jun 25 02:22:58 2012
>>> New Revision: 1353381
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1353381&view=rev
>>> Log:
>>> Give ACCOUNTING_ADMIN the same access as PAY_INFO_ADMIN because part of accounting component
>>>
>>> Modified:
>>>    ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>    ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>    ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>    ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>    ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>    ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>    ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>    ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>
>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml (original)
>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml Mon Jun 25 02:22:58 2012
>>> @@ -26,7 +26,6 @@ under the License.
>>>     <SecurityGroupPermission groupId="BIZADMIN" permissionId="PAYPROC_ADMIN"/>
>>>
>>>     <!-- Payment Information security -->
>>> -    <SecurityGroupPermission groupId="FULLADMIN" permissionId="PAY_INFO_ADMIN"/>
>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_CREATE"/>
>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_DELETE"/>
>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_UPDATE"/>
>>>
>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml (original)
>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml Mon Jun 25 02:22:58 2012
>>> @@ -68,7 +68,6 @@ under the License.
>>>
>>>     <!-- add admin to SUPER permission group -->
>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_ADMIN"/>
>>> -    <SecurityGroupPermission groupId="SUPER" permissionId="PAY_INFO_ADMIN"/>
>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_COMM_VIEW"/>
>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_PRINT_CHECKS"/>
>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCTG_PREF_ADMIN"/>
>>>
>>> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml (original)
>>> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml Mon Jun 25 02:22:58 2012
>>> @@ -24,6 +24,7 @@ under the License.
>>>         <if>
>>>             <condition>
>>>                 <and>
>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>> @@ -86,6 +87,7 @@ under the License.
>>>         <if>
>>>             <condition>
>>>                 <and>
>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>                     <not><if-has-permission permission="PAY_INFO" action="_UPDATE"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdFrom" operator="equals"/></not>
>>>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdTo" operator="equals"/></not>
>>>
>>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (original)
>>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java Mon Jun 25 02:22:58 2012
>>> @@ -89,7 +89,7 @@ public class PaymentMethodServices {
>>>
>>>         // <b>security check</b>: userLogin partyId must equal paymentMethod partyId, or must have PAY_INFO_DELETE permission
>>>         if (paymentMethod.get("partyId") == null || !paymentMethod.getString("partyId").equals(userLogin.getString("partyId"))) {
>>> -            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin)) {
>>> +            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>                 return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>                         "AccountingPaymentMethodNoPermissionToDelete", locale));
>>>             }
>>> @@ -139,7 +139,7 @@ public class PaymentMethodServices {
>>>
>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>
>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>
>>>         if (result.size() > 0) return result;
>>>
>>> @@ -260,7 +260,7 @@ public class PaymentMethodServices {
>>>
>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>
>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>
>>>         if (result.size() > 0) return result;
>>>
>>> @@ -286,7 +286,7 @@ public class PaymentMethodServices {
>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>                     "AccountingCreditCardUpdateWithPaymentMethodId", locale) + paymentMethodId);
>>>         }
>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>                     "AccountingCreditCardUpdateWithoutPermission", UtilMisc.toMap("partyId", partyId,
>>>                             "paymentMethodId", paymentMethodId), locale));
>>> @@ -488,7 +488,7 @@ public class PaymentMethodServices {
>>>
>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>
>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>
>>>         if (result.size() > 0)
>>>             return result;
>>> @@ -545,7 +545,7 @@ public class PaymentMethodServices {
>>>
>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>
>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>
>>>         if (result.size() > 0)
>>>             return result;
>>> @@ -574,7 +574,7 @@ public class PaymentMethodServices {
>>>                     "AccountingGiftCardCannotBeUpdated",
>>>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>         }
>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>                     "AccountingGiftCardPartyNotAuthorized",
>>>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>> @@ -679,7 +679,7 @@ public class PaymentMethodServices {
>>>
>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>
>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>
>>>         if (result.size() > 0) return result;
>>>
>>> @@ -777,7 +777,7 @@ public class PaymentMethodServices {
>>>
>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>
>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>
>>>         if (result.size() > 0) return result;
>>>
>>> @@ -806,7 +806,7 @@ public class PaymentMethodServices {
>>>                     "AccountingEftAccountCannotBeUpdated",
>>>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>         }
>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>                     "AccountingEftAccountCannotBeUpdated",
>>>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>
>>> Modified: ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/widget/GlScreens.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/accounting/widget/GlScreens.xml (original)
>>> +++ ofbiz/trunk/applications/accounting/widget/GlScreens.xml Mon Jun 25 02:22:58 2012
>>> @@ -445,7 +445,12 @@ under the License.
>>>                 <decorator-screen name="CommonAdminChecksDecorator" location="${parameters.mainDecoratorLocation}">
>>>                     <decorator-section name="checks-body">
>>>                         <section>
>>> -                        <condition><if-has-permission permission="PAY_INFO" action="_UPDATE"/></condition>
>>> +                        <condition>
>>> +                            <or>
>>> +                                <if-has-permission permission="ACCOUNTING" action="_ADMIN"/>
>>> +                                <if-has-permission permission="PAY_INFO" action="_UPDATE"/>
>>> +                            </or>
>>> +                        </condition>
>>>                         <widgets>
>>>                             <screenlet title="${uiLabelMap.AccountingSendChecks}">
>>>                                 <include-form name="ListChecksToSend" location="component://accounting/widget/PaymentForms.xml"/>
>>>
>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl (original)
>>> +++ ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl Mon Jun 25 02:22:58 2012
>>> @@ -54,7 +54,7 @@ under the License.
>>>            <#assign statusItem = payment.getRelatedOne("StatusItem", false)>
>>>            <#assign partyName = delegator.findOne("PartyNameView", {"partyId" : payment.partyIdTo}, true)>
>>>            <tr>
>>> -             <#if security.hasPermission("PAY_INFO_VIEW", session) || security.hasPermission("PAY_INFO_ADMIN", session)>
>>> +             <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                <td><a href="/accounting/control/paymentOverview?paymentId=${payment.paymentId}">${payment.paymentId}</a></td>
>>>              <#else>
>>>                <td>${payment.paymentId}</td>
>>> @@ -342,7 +342,7 @@ under the License.
>>>                       <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>                       <br />
>>>
>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                         ${creditCard.cardType}
>>>                         <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>                         ${creditCard.expireDate}
>>> @@ -469,7 +469,7 @@ under the License.
>>>                 <td valign="top" width="60%">
>>>                   <div>
>>>                     <#if giftCard?has_content>
>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                         ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>                         &nbsp;[<#if oppStatusItem?exists>${oppStatusItem.get("description",locale)}<#else>${orderPaymentPreference.statusId}</#if>]
>>>                       <#else>
>>> @@ -596,7 +596,7 @@ under the License.
>>>                <#if "CREDIT_CARD" == paymentMethod.paymentMethodTypeId>
>>>                  <#assign creditCard = paymentMethodValueMap.creditCard/>
>>>                  <#if (creditCard?has_content)>
>>> -                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>> +                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                      ${creditCard.cardType?if_exists} <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/> ${creditCard.expireDate?if_exists}
>>>                    <#else>
>>>                      ${Static["org.ofbiz.party.contact.ContactHelper"].formatCreditCard(creditCard)}
>>>
>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy (original)
>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy Mon Jun 25 02:22:58 2012
>>> @@ -23,7 +23,7 @@ context.hasCreatePermission = security.h
>>> context.hasUpdatePermission = security.hasEntityPermission("PARTYMGR", "_UPDATE", session);
>>> context.hasDeletePermission = security.hasEntityPermission("PARTYMGR", "_DELETE", session);
>>> // extended pay_info permissions
>>> -context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session);
>>> +context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session);
>>> // extended pcm (party contact mechanism) permissions
>>> context.hasPcmCreatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_CREATE", session);
>>> context.hasPcmUpdatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_UPDATE", session);
>>>
>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl (original)
>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl Mon Jun 25 02:22:58 2012
>>> @@ -38,7 +38,7 @@ under the License.
>>>     <div class="screenlet-title-bar">
>>>       <ul>
>>>         <li class="h3">${uiLabelMap.PartyPaymentMethodInformation}</li>
>>> -        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session)>
>>> +        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>           <li><a href="<@ofbizUrl>editeftaccount?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewEftAccount}</a></li>
>>>           <li><a href="<@ofbizUrl>editgiftcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewGiftCard}</a></li>
>>>           <li><a href="<@ofbizUrl>editcreditcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewCreditCard}</a></li>
>>> @@ -67,7 +67,7 @@ under the License.
>>>                   ${creditCard.lastNameOnCard}
>>>                   <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>                   &nbsp;-&nbsp;
>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                     ${creditCard.cardType}
>>>                     <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>                     ${creditCard.expireDate}
>>> @@ -83,7 +83,7 @@ under the License.
>>>                   <#if security.hasEntityPermission("MANUAL", "_PAYMENT", session)>
>>>                     <a href="/accounting/control/manualETx?paymentMethodId=${paymentMethod.paymentMethodId}${externalKeyParam}">${uiLabelMap.PartyManualTx}</a>
>>>                   </#if>
>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                     <a href="<@ofbizUrl>editcreditcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>                   </#if>
>>>                 <#-- </td> -->
>>> @@ -93,7 +93,7 @@ under the License.
>>>                   ${uiLabelMap.AccountingGiftCard}
>>>                 </td>
>>>                 <td>
>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                     ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>                   <#else>
>>>                     <@maskSensitiveNumber cardNumber=giftCard.cardNumber?if_exists/>
>>> @@ -105,7 +105,7 @@ under the License.
>>>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</b></#if>
>>>                 </td>
>>>                 <td class="button-col">
>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                     <a href="<@ofbizUrl>editgiftcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>                   </#if>
>>>                 <#-- </td> -->
>>> @@ -121,7 +121,7 @@ under the License.
>>>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</#if>
>>>                 </td>
>>>                 <td class="button-col">
>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                     <a href="<@ofbizUrl>editeftaccount?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>                   </#if>
>>>                 <#-- </td> -->
>>> @@ -143,7 +143,7 @@ under the License.
>>>                 <td class="button-col">
>>>                   &nbsp;
>>>               </#if>
>>> -              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session)>
>>> +              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>                 <a href="<@ofbizUrl>deletePaymentMethod/viewprofile?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonExpire}</a>
>>>               <#else>
>>>                 &nbsp;
>>>
>>> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java (original)
>>> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java Mon Jun 25 02:22:58 2012
>>> @@ -184,6 +184,9 @@ public class ServiceUtil {
>>>      *<b>security check</b>: userLogin partyId must equal partyId, or must have [secEntity][secOperation] permission
>>>      */
>>>     public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation) {
>>> +        return getPartyIdCheckSecurity(userLogin, security, context, result, secEntity, secOperation, null, null);
>>> +    }
>>> +    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation, String adminSecEntity, String adminSecOperation) {
>>>         String partyId = (String) context.get("partyId");
>>>         Locale locale = getLocale(context);
>>>         if (UtilValidate.isEmpty(partyId)) {
>>> @@ -198,9 +201,9 @@ public class ServiceUtil {
>>>             return partyId;
>>>         }
>>>
>>> -        // <b>security check</b>: userLogin partyId must equal partyId, or must have PARTYMGR_CREATE permission
>>> +        // <b>security check</b>: userLogin partyId must equal partyId, or must have either of the two permissions
>>>         if (!partyId.equals(userLogin.getString("partyId"))) {
>>> -            if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
>>> +            if (!security.hasEntityPermission(secEntity, secOperation, userLogin) && !(adminSecEntity != null && adminSecOperation != null && security.hasEntityPermission(adminSecEntity, adminSecOperation, userLogin))) {
>>>                 result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_ERROR);
>>>                 String errMsg = UtilProperties.getMessage(ServiceUtil.resource, "serviceUtil.no_permission_to_operation", locale) + ".";
>>>                 result.put(ModelService.ERROR_MESSAGE, errMsg);
>>>
>>>
>
>

[hans_bakker]

Yes Jacopo, I know all that, but the question is as I wrote:

The question here is do you really want to allow access for
ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps
the intention here was that, by creating a separate action PAY_INFO the
ACCOUNTING_CREATE should not have access.

Regards,
Hans

On 06/26/2012 03:54 AM, Jacopo Cappellato wrote:

> Hi Hans,
>
> I think that the intended meaning is the following:
>
> ACCOUNTING_<ACTION>: global permission to all parts of the Accounting application when performing the <ACTION>
> ACCOUNTING_ADMIN: global permission to all parts of the Accounting application when performing all the actions
>
> So for example:
>
> ACCOUNTING_CREATE: global permission to CREATE all records in the Accounting application
>
> In other words:
>
> ACCOUNTING_ADMIN = ACCOUNTING_CREATE + ACCOUNTING_UPDATE + ACCOUNTING_DELETE + ACCOUNTING_VIEW
>
> (and it is not more than this).
>
> Jacopo
>
>
> On Jun 25, 2012, at 9:46 PM, Hans Bakker wrote:
>
>> Hi Jacopo,
>>
>> thanks for reviewing this commit, my compliments for your work in this area.
>>
>> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>>
>> The reason of my commit is that ACCOUNTING_ADMIN should really, as the description states, have full access to the complete accounting component and I did not want to change the other permissions.
>>
>> Personally i am fine with your suggestion and sure yes, we can do it that way.....
>>
>> Regards,
>> Hans
>>
>>
>> On 06/25/2012 09:27 PM, Jacopo Cappellato wrote:
>>> Hi Hans,
>>>
>>> I understand the issue you are fixing in this commit (and at least another one of last week) but I disagree with the approach.
>>> We should never add permission checks on the *_ADMIN permissions: we should instead always use one of the fine grained _CREATE, _UPDATE, _DELETE; the _ADMIN permission will be automatically checked by the framework if the fine grained ones fail.
>>> So instead of (for example):
>>>
>>>> @@ -24,6 +24,7 @@ under the License.
>>>>          <if>
>>>>              <condition>
>>>>                  <and>
>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>                      <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>> you should have:
>>>
>>>> @@ -24,6 +24,7 @@ under the License.
>>>>          <if>
>>>>              <condition>
>>>>                  <and>
>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_CREATE"/></not>
>>>>                      <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>
>>> The code above will grant access to users having at least one of the following:
>>> PAYINFO_CREATE
>>> PAYINFO_ADMIN
>>> ACCOUNTING_CREATE
>>> ACCOUNTING_ADMIN
>>>
>>> Kind regards,
>>>
>>> Jacopo
>>>
>>>
>>> On Jun 25, 2012, at 4:23 AM, [hidden email] wrote:
>>>
>>>> Author: hansbak
>>>> Date: Mon Jun 25 02:22:58 2012
>>>> New Revision: 1353381
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1353381&view=rev
>>>> Log:
>>>> Give ACCOUNTING_ADMIN the same access as PAY_INFO_ADMIN because part of accounting component
>>>>
>>>> Modified:
>>>>     ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>>     ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>>     ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>>     ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>>     ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>>     ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>>     ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>>     ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>>     ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>>
>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml (original)
>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml Mon Jun 25 02:22:58 2012
>>>> @@ -26,7 +26,6 @@ under the License.
>>>>      <SecurityGroupPermission groupId="BIZADMIN" permissionId="PAYPROC_ADMIN"/>
>>>>
>>>>      <!-- Payment Information security -->
>>>> -    <SecurityGroupPermission groupId="FULLADMIN" permissionId="PAY_INFO_ADMIN"/>
>>>>      <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_CREATE"/>
>>>>      <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_DELETE"/>
>>>>      <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_UPDATE"/>
>>>>
>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml (original)
>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml Mon Jun 25 02:22:58 2012
>>>> @@ -68,7 +68,6 @@ under the License.
>>>>
>>>>      <!-- add admin to SUPER permission group -->
>>>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_ADMIN"/>
>>>> -    <SecurityGroupPermission groupId="SUPER" permissionId="PAY_INFO_ADMIN"/>
>>>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_COMM_VIEW"/>
>>>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_PRINT_CHECKS"/>
>>>>      <SecurityGroupPermission groupId="SUPER" permissionId="ACCTG_PREF_ADMIN"/>
>>>>
>>>> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml (original)
>>>> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml Mon Jun 25 02:22:58 2012
>>>> @@ -24,6 +24,7 @@ under the License.
>>>>          <if>
>>>>              <condition>
>>>>                  <and>
>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>                      <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>> @@ -86,6 +87,7 @@ under the License.
>>>>          <if>
>>>>              <condition>
>>>>                  <and>
>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>                      <not><if-has-permission permission="PAY_INFO" action="_UPDATE"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdFrom" operator="equals"/></not>
>>>>                      <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdTo" operator="equals"/></not>
>>>>
>>>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (original)
>>>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java Mon Jun 25 02:22:58 2012
>>>> @@ -89,7 +89,7 @@ public class PaymentMethodServices {
>>>>
>>>>          // <b>security check</b>: userLogin partyId must equal paymentMethod partyId, or must have PAY_INFO_DELETE permission
>>>>          if (paymentMethod.get("partyId") == null || !paymentMethod.getString("partyId").equals(userLogin.getString("partyId"))) {
>>>> -            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin)) {
>>>> +            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>                  return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>                          "AccountingPaymentMethodNoPermissionToDelete", locale));
>>>>              }
>>>> @@ -139,7 +139,7 @@ public class PaymentMethodServices {
>>>>
>>>>          Timestamp now = UtilDateTime.nowTimestamp();
>>>>
>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>
>>>>          if (result.size() > 0) return result;
>>>>
>>>> @@ -260,7 +260,7 @@ public class PaymentMethodServices {
>>>>
>>>>          Timestamp now = UtilDateTime.nowTimestamp();
>>>>
>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>
>>>>          if (result.size() > 0) return result;
>>>>
>>>> @@ -286,7 +286,7 @@ public class PaymentMethodServices {
>>>>              return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>                      "AccountingCreditCardUpdateWithPaymentMethodId", locale) + paymentMethodId);
>>>>          }
>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>              return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>                      "AccountingCreditCardUpdateWithoutPermission", UtilMisc.toMap("partyId", partyId,
>>>>                              "paymentMethodId", paymentMethodId), locale));
>>>> @@ -488,7 +488,7 @@ public class PaymentMethodServices {
>>>>
>>>>          Timestamp now = UtilDateTime.nowTimestamp();
>>>>
>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>
>>>>          if (result.size() > 0)
>>>>              return result;
>>>> @@ -545,7 +545,7 @@ public class PaymentMethodServices {
>>>>
>>>>          Timestamp now = UtilDateTime.nowTimestamp();
>>>>
>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>
>>>>          if (result.size() > 0)
>>>>              return result;
>>>> @@ -574,7 +574,7 @@ public class PaymentMethodServices {
>>>>                      "AccountingGiftCardCannotBeUpdated",
>>>>                      UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>          }
>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>              return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>                      "AccountingGiftCardPartyNotAuthorized",
>>>>                      UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>> @@ -679,7 +679,7 @@ public class PaymentMethodServices {
>>>>
>>>>          Timestamp now = UtilDateTime.nowTimestamp();
>>>>
>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>
>>>>          if (result.size() > 0) return result;
>>>>
>>>> @@ -777,7 +777,7 @@ public class PaymentMethodServices {
>>>>
>>>>          Timestamp now = UtilDateTime.nowTimestamp();
>>>>
>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>
>>>>          if (result.size() > 0) return result;
>>>>
>>>> @@ -806,7 +806,7 @@ public class PaymentMethodServices {
>>>>                      "AccountingEftAccountCannotBeUpdated",
>>>>                      UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>          }
>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>              return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>                      "AccountingEftAccountCannotBeUpdated",
>>>>                      UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>>
>>>> Modified: ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/widget/GlScreens.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/accounting/widget/GlScreens.xml (original)
>>>> +++ ofbiz/trunk/applications/accounting/widget/GlScreens.xml Mon Jun 25 02:22:58 2012
>>>> @@ -445,7 +445,12 @@ under the License.
>>>>                  <decorator-screen name="CommonAdminChecksDecorator" location="${parameters.mainDecoratorLocation}">
>>>>                      <decorator-section name="checks-body">
>>>>                          <section>
>>>> -                        <condition><if-has-permission permission="PAY_INFO" action="_UPDATE"/></condition>
>>>> +                        <condition>
>>>> +                            <or>
>>>> +                                <if-has-permission permission="ACCOUNTING" action="_ADMIN"/>
>>>> +                                <if-has-permission permission="PAY_INFO" action="_UPDATE"/>
>>>> +                            </or>
>>>> +                        </condition>
>>>>                          <widgets>
>>>>                              <screenlet title="${uiLabelMap.AccountingSendChecks}">
>>>>                                  <include-form name="ListChecksToSend" location="component://accounting/widget/PaymentForms.xml"/>
>>>>
>>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl (original)
>>>> +++ ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl Mon Jun 25 02:22:58 2012
>>>> @@ -54,7 +54,7 @@ under the License.
>>>>             <#assign statusItem = payment.getRelatedOne("StatusItem", false)>
>>>>             <#assign partyName = delegator.findOne("PartyNameView", {"partyId" : payment.partyIdTo}, true)>
>>>>             <tr>
>>>> -             <#if security.hasPermission("PAY_INFO_VIEW", session) || security.hasPermission("PAY_INFO_ADMIN", session)>
>>>> +             <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                 <td><a href="/accounting/control/paymentOverview?paymentId=${payment.paymentId}">${payment.paymentId}</a></td>
>>>>               <#else>
>>>>                 <td>${payment.paymentId}</td>
>>>> @@ -342,7 +342,7 @@ under the License.
>>>>                        <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>                        <br />
>>>>
>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                          ${creditCard.cardType}
>>>>                          <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>                          ${creditCard.expireDate}
>>>> @@ -469,7 +469,7 @@ under the License.
>>>>                  <td valign="top" width="60%">
>>>>                    <div>
>>>>                      <#if giftCard?has_content>
>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                          ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>                          &nbsp;[<#if oppStatusItem?exists>${oppStatusItem.get("description",locale)}<#else>${orderPaymentPreference.statusId}</#if>]
>>>>                        <#else>
>>>> @@ -596,7 +596,7 @@ under the License.
>>>>                 <#if "CREDIT_CARD" == paymentMethod.paymentMethodTypeId>
>>>>                   <#assign creditCard = paymentMethodValueMap.creditCard/>
>>>>                   <#if (creditCard?has_content)>
>>>> -                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>> +                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                       ${creditCard.cardType?if_exists} <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/> ${creditCard.expireDate?if_exists}
>>>>                     <#else>
>>>>                       ${Static["org.ofbiz.party.contact.ContactHelper"].formatCreditCard(creditCard)}
>>>>
>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy (original)
>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy Mon Jun 25 02:22:58 2012
>>>> @@ -23,7 +23,7 @@ context.hasCreatePermission = security.h
>>>> context.hasUpdatePermission = security.hasEntityPermission("PARTYMGR", "_UPDATE", session);
>>>> context.hasDeletePermission = security.hasEntityPermission("PARTYMGR", "_DELETE", session);
>>>> // extended pay_info permissions
>>>> -context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session);
>>>> +context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session);
>>>> // extended pcm (party contact mechanism) permissions
>>>> context.hasPcmCreatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_CREATE", session);
>>>> context.hasPcmUpdatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_UPDATE", session);
>>>>
>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl (original)
>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl Mon Jun 25 02:22:58 2012
>>>> @@ -38,7 +38,7 @@ under the License.
>>>>      <div class="screenlet-title-bar">
>>>>        <ul>
>>>>          <li class="h3">${uiLabelMap.PartyPaymentMethodInformation}</li>
>>>> -        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session)>
>>>> +        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>            <li><a href="<@ofbizUrl>editeftaccount?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewEftAccount}</a></li>
>>>>            <li><a href="<@ofbizUrl>editgiftcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewGiftCard}</a></li>
>>>>            <li><a href="<@ofbizUrl>editcreditcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewCreditCard}</a></li>
>>>> @@ -67,7 +67,7 @@ under the License.
>>>>                    ${creditCard.lastNameOnCard}
>>>>                    <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>                    &nbsp;-&nbsp;
>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                      ${creditCard.cardType}
>>>>                      <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>                      ${creditCard.expireDate}
>>>> @@ -83,7 +83,7 @@ under the License.
>>>>                    <#if security.hasEntityPermission("MANUAL", "_PAYMENT", session)>
>>>>                      <a href="/accounting/control/manualETx?paymentMethodId=${paymentMethod.paymentMethodId}${externalKeyParam}">${uiLabelMap.PartyManualTx}</a>
>>>>                    </#if>
>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                      <a href="<@ofbizUrl>editcreditcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>                    </#if>
>>>>                  <#-- </td> -->
>>>> @@ -93,7 +93,7 @@ under the License.
>>>>                    ${uiLabelMap.AccountingGiftCard}
>>>>                  </td>
>>>>                  <td>
>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                      ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>                    <#else>
>>>>                      <@maskSensitiveNumber cardNumber=giftCard.cardNumber?if_exists/>
>>>> @@ -105,7 +105,7 @@ under the License.
>>>>                    <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</b></#if>
>>>>                  </td>
>>>>                  <td class="button-col">
>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                      <a href="<@ofbizUrl>editgiftcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>                    </#if>
>>>>                  <#-- </td> -->
>>>> @@ -121,7 +121,7 @@ under the License.
>>>>                    <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</#if>
>>>>                  </td>
>>>>                  <td class="button-col">
>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                      <a href="<@ofbizUrl>editeftaccount?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>                    </#if>
>>>>                  <#-- </td> -->
>>>> @@ -143,7 +143,7 @@ under the License.
>>>>                  <td class="button-col">
>>>>                    &nbsp;
>>>>                </#if>
>>>> -              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session)>
>>>> +              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>                  <a href="<@ofbizUrl>deletePaymentMethod/viewprofile?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonExpire}</a>
>>>>                <#else>
>>>>                  &nbsp;
>>>>
>>>> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java (original)
>>>> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java Mon Jun 25 02:22:58 2012
>>>> @@ -184,6 +184,9 @@ public class ServiceUtil {
>>>>       *<b>security check</b>: userLogin partyId must equal partyId, or must have [secEntity][secOperation] permission
>>>>       */
>>>>      public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation) {
>>>> +        return getPartyIdCheckSecurity(userLogin, security, context, result, secEntity, secOperation, null, null);
>>>> +    }
>>>> +    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation, String adminSecEntity, String adminSecOperation) {
>>>>          String partyId = (String) context.get("partyId");
>>>>          Locale locale = getLocale(context);
>>>>          if (UtilValidate.isEmpty(partyId)) {
>>>> @@ -198,9 +201,9 @@ public class ServiceUtil {
>>>>              return partyId;
>>>>          }
>>>>
>>>> -        // <b>security check</b>: userLogin partyId must equal partyId, or must have PARTYMGR_CREATE permission
>>>> +        // <b>security check</b>: userLogin partyId must equal partyId, or must have either of the two permissions
>>>>          if (!partyId.equals(userLogin.getString("partyId"))) {
>>>> -            if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
>>>> +            if (!security.hasEntityPermission(secEntity, secOperation, userLogin) && !(adminSecEntity != null && adminSecOperation != null && security.hasEntityPermission(adminSecEntity, adminSecOperation, userLogin))) {
>>>>                  result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_ERROR);
>>>>                  String errMsg = UtilProperties.getMessage(ServiceUtil.resource, "serviceUtil.no_permission_to_operation", locale) + ".";
>>>>                  result.put(ModelService.ERROR_MESSAGE, errMsg);
>>>>
>>>>
>>

[Scott Gray-2]

If I had to guess I would think your proposition would be reversed:
PAY_INFO should grant limited access to ACCOUNTING functionality.
On 26/06/2012, at 12:01 PM, Hans Bakker wrote:

> Yes Jacopo, I know all that, but the question is as I wrote:
>
> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>
> Regards,
> Hans
>
> On 06/26/2012 03:54 AM, Jacopo Cappellato wrote:
>> Hi Hans,
>>
>> I think that the intended meaning is the following:
>>
>> ACCOUNTING_<ACTION>: global permission to all parts of the Accounting application when performing the <ACTION>
>> ACCOUNTING_ADMIN: global permission to all parts of the Accounting application when performing all the actions
>>
>> So for example:
>>
>> ACCOUNTING_CREATE: global permission to CREATE all records in the Accounting application
>>
>> In other words:
>>
>> ACCOUNTING_ADMIN = ACCOUNTING_CREATE + ACCOUNTING_UPDATE + ACCOUNTING_DELETE + ACCOUNTING_VIEW
>>
>> (and it is not more than this).
>>
>> Jacopo
>>
>>
>> On Jun 25, 2012, at 9:46 PM, Hans Bakker wrote:
>>
>>> Hi Jacopo,
>>>
>>> thanks for reviewing this commit, my compliments for your work in this area.
>>>
>>> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>>>
>>> The reason of my commit is that ACCOUNTING_ADMIN should really, as the description states, have full access to the complete accounting component and I did not want to change the other permissions.
>>>
>>> Personally i am fine with your suggestion and sure yes, we can do it that way.....
>>>
>>> Regards,
>>> Hans
>>>
>>>
>>> On 06/25/2012 09:27 PM, Jacopo Cappellato wrote:
>>>> Hi Hans,
>>>>
>>>> I understand the issue you are fixing in this commit (and at least another one of last week) but I disagree with the approach.
>>>> We should never add permission checks on the *_ADMIN permissions: we should instead always use one of the fine grained _CREATE, _UPDATE, _DELETE; the _ADMIN permission will be automatically checked by the framework if the fine grained ones fail.
>>>> So instead of (for example):
>>>>
>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>         <if>
>>>>>             <condition>
>>>>>                 <and>
>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>> you should have:
>>>>
>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>         <if>
>>>>>             <condition>
>>>>>                 <and>
>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_CREATE"/></not>
>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>
>>>> The code above will grant access to users having at least one of the following:
>>>> PAYINFO_CREATE
>>>> PAYINFO_ADMIN
>>>> ACCOUNTING_CREATE
>>>> ACCOUNTING_ADMIN
>>>>
>>>> Kind regards,
>>>>
>>>> Jacopo
>>>>
>>>>
>>>> On Jun 25, 2012, at 4:23 AM, [hidden email] wrote:
>>>>
>>>>> Author: hansbak
>>>>> Date: Mon Jun 25 02:22:58 2012
>>>>> New Revision: 1353381
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1353381&view=rev
>>>>> Log:
>>>>> Give ACCOUNTING_ADMIN the same access as PAY_INFO_ADMIN because part of accounting component
>>>>>
>>>>> Modified:
>>>>>    ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>>>    ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>>>    ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>>>    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>>>    ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>>>    ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>>>    ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>>>    ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>>>    ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>>>
>>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml (original)
>>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml Mon Jun 25 02:22:58 2012
>>>>> @@ -26,7 +26,6 @@ under the License.
>>>>>     <SecurityGroupPermission groupId="BIZADMIN" permissionId="PAYPROC_ADMIN"/>
>>>>>
>>>>>     <!-- Payment Information security -->
>>>>> -    <SecurityGroupPermission groupId="FULLADMIN" permissionId="PAY_INFO_ADMIN"/>
>>>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_CREATE"/>
>>>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_DELETE"/>
>>>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_UPDATE"/>
>>>>>
>>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml (original)
>>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml Mon Jun 25 02:22:58 2012
>>>>> @@ -68,7 +68,6 @@ under the License.
>>>>>
>>>>>     <!-- add admin to SUPER permission group -->
>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_ADMIN"/>
>>>>> -    <SecurityGroupPermission groupId="SUPER" permissionId="PAY_INFO_ADMIN"/>
>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_COMM_VIEW"/>
>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_PRINT_CHECKS"/>
>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCTG_PREF_ADMIN"/>
>>>>>
>>>>> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml (original)
>>>>> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml Mon Jun 25 02:22:58 2012
>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>         <if>
>>>>>             <condition>
>>>>>                 <and>
>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>> @@ -86,6 +87,7 @@ under the License.
>>>>>         <if>
>>>>>             <condition>
>>>>>                 <and>
>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_UPDATE"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdFrom" operator="equals"/></not>
>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdTo" operator="equals"/></not>
>>>>>
>>>>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (original)
>>>>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java Mon Jun 25 02:22:58 2012
>>>>> @@ -89,7 +89,7 @@ public class PaymentMethodServices {
>>>>>
>>>>>         // <b>security check</b>: userLogin partyId must equal paymentMethod partyId, or must have PAY_INFO_DELETE permission
>>>>>         if (paymentMethod.get("partyId") == null || !paymentMethod.getString("partyId").equals(userLogin.getString("partyId"))) {
>>>>> -            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin)) {
>>>>> +            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>                 return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>                         "AccountingPaymentMethodNoPermissionToDelete", locale));
>>>>>             }
>>>>> @@ -139,7 +139,7 @@ public class PaymentMethodServices {
>>>>>
>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>
>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>
>>>>>         if (result.size() > 0) return result;
>>>>>
>>>>> @@ -260,7 +260,7 @@ public class PaymentMethodServices {
>>>>>
>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>
>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>
>>>>>         if (result.size() > 0) return result;
>>>>>
>>>>> @@ -286,7 +286,7 @@ public class PaymentMethodServices {
>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>>                     "AccountingCreditCardUpdateWithPaymentMethodId", locale) + paymentMethodId);
>>>>>         }
>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>>                     "AccountingCreditCardUpdateWithoutPermission", UtilMisc.toMap("partyId", partyId,
>>>>>                             "paymentMethodId", paymentMethodId), locale));
>>>>> @@ -488,7 +488,7 @@ public class PaymentMethodServices {
>>>>>
>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>
>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>
>>>>>         if (result.size() > 0)
>>>>>             return result;
>>>>> @@ -545,7 +545,7 @@ public class PaymentMethodServices {
>>>>>
>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>
>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>
>>>>>         if (result.size() > 0)
>>>>>             return result;
>>>>> @@ -574,7 +574,7 @@ public class PaymentMethodServices {
>>>>>                     "AccountingGiftCardCannotBeUpdated",
>>>>>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>>         }
>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>                     "AccountingGiftCardPartyNotAuthorized",
>>>>>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>>> @@ -679,7 +679,7 @@ public class PaymentMethodServices {
>>>>>
>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>
>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>
>>>>>         if (result.size() > 0) return result;
>>>>>
>>>>> @@ -777,7 +777,7 @@ public class PaymentMethodServices {
>>>>>
>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>
>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>
>>>>>         if (result.size() > 0) return result;
>>>>>
>>>>> @@ -806,7 +806,7 @@ public class PaymentMethodServices {
>>>>>                     "AccountingEftAccountCannotBeUpdated",
>>>>>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>>         }
>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>                     "AccountingEftAccountCannotBeUpdated",
>>>>>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>>>
>>>>> Modified: ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/widget/GlScreens.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/accounting/widget/GlScreens.xml (original)
>>>>> +++ ofbiz/trunk/applications/accounting/widget/GlScreens.xml Mon Jun 25 02:22:58 2012
>>>>> @@ -445,7 +445,12 @@ under the License.
>>>>>                 <decorator-screen name="CommonAdminChecksDecorator" location="${parameters.mainDecoratorLocation}">
>>>>>                     <decorator-section name="checks-body">
>>>>>                         <section>
>>>>> -                        <condition><if-has-permission permission="PAY_INFO" action="_UPDATE"/></condition>
>>>>> +                        <condition>
>>>>> +                            <or>
>>>>> +                                <if-has-permission permission="ACCOUNTING" action="_ADMIN"/>
>>>>> +                                <if-has-permission permission="PAY_INFO" action="_UPDATE"/>
>>>>> +                            </or>
>>>>> +                        </condition>
>>>>>                         <widgets>
>>>>>                             <screenlet title="${uiLabelMap.AccountingSendChecks}">
>>>>>                                 <include-form name="ListChecksToSend" location="component://accounting/widget/PaymentForms.xml"/>
>>>>>
>>>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl (original)
>>>>> +++ ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl Mon Jun 25 02:22:58 2012
>>>>> @@ -54,7 +54,7 @@ under the License.
>>>>>            <#assign statusItem = payment.getRelatedOne("StatusItem", false)>
>>>>>            <#assign partyName = delegator.findOne("PartyNameView", {"partyId" : payment.partyIdTo}, true)>
>>>>>            <tr>
>>>>> -             <#if security.hasPermission("PAY_INFO_VIEW", session) || security.hasPermission("PAY_INFO_ADMIN", session)>
>>>>> +             <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                <td><a href="/accounting/control/paymentOverview?paymentId=${payment.paymentId}">${payment.paymentId}</a></td>
>>>>>              <#else>
>>>>>                <td>${payment.paymentId}</td>
>>>>> @@ -342,7 +342,7 @@ under the License.
>>>>>                       <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>>                       <br />
>>>>>
>>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                         ${creditCard.cardType}
>>>>>                         <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>>                         ${creditCard.expireDate}
>>>>> @@ -469,7 +469,7 @@ under the License.
>>>>>                 <td valign="top" width="60%">
>>>>>                   <div>
>>>>>                     <#if giftCard?has_content>
>>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                         ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>>                         &nbsp;[<#if oppStatusItem?exists>${oppStatusItem.get("description",locale)}<#else>${orderPaymentPreference.statusId}</#if>]
>>>>>                       <#else>
>>>>> @@ -596,7 +596,7 @@ under the License.
>>>>>                <#if "CREDIT_CARD" == paymentMethod.paymentMethodTypeId>
>>>>>                  <#assign creditCard = paymentMethodValueMap.creditCard/>
>>>>>                  <#if (creditCard?has_content)>
>>>>> -                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>> +                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                      ${creditCard.cardType?if_exists} <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/> ${creditCard.expireDate?if_exists}
>>>>>                    <#else>
>>>>>                      ${Static["org.ofbiz.party.contact.ContactHelper"].formatCreditCard(creditCard)}
>>>>>
>>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy (original)
>>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy Mon Jun 25 02:22:58 2012
>>>>> @@ -23,7 +23,7 @@ context.hasCreatePermission = security.h
>>>>> context.hasUpdatePermission = security.hasEntityPermission("PARTYMGR", "_UPDATE", session);
>>>>> context.hasDeletePermission = security.hasEntityPermission("PARTYMGR", "_DELETE", session);
>>>>> // extended pay_info permissions
>>>>> -context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session);
>>>>> +context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session);
>>>>> // extended pcm (party contact mechanism) permissions
>>>>> context.hasPcmCreatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_CREATE", session);
>>>>> context.hasPcmUpdatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_UPDATE", session);
>>>>>
>>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl (original)
>>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl Mon Jun 25 02:22:58 2012
>>>>> @@ -38,7 +38,7 @@ under the License.
>>>>>     <div class="screenlet-title-bar">
>>>>>       <ul>
>>>>>         <li class="h3">${uiLabelMap.PartyPaymentMethodInformation}</li>
>>>>> -        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session)>
>>>>> +        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>           <li><a href="<@ofbizUrl>editeftaccount?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewEftAccount}</a></li>
>>>>>           <li><a href="<@ofbizUrl>editgiftcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewGiftCard}</a></li>
>>>>>           <li><a href="<@ofbizUrl>editcreditcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewCreditCard}</a></li>
>>>>> @@ -67,7 +67,7 @@ under the License.
>>>>>                   ${creditCard.lastNameOnCard}
>>>>>                   <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>>                   &nbsp;-&nbsp;
>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                     ${creditCard.cardType}
>>>>>                     <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>>                     ${creditCard.expireDate}
>>>>> @@ -83,7 +83,7 @@ under the License.
>>>>>                   <#if security.hasEntityPermission("MANUAL", "_PAYMENT", session)>
>>>>>                     <a href="/accounting/control/manualETx?paymentMethodId=${paymentMethod.paymentMethodId}${externalKeyParam}">${uiLabelMap.PartyManualTx}</a>
>>>>>                   </#if>
>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                     <a href="<@ofbizUrl>editcreditcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>                   </#if>
>>>>>                 <#-- </td> -->
>>>>> @@ -93,7 +93,7 @@ under the License.
>>>>>                   ${uiLabelMap.AccountingGiftCard}
>>>>>                 </td>
>>>>>                 <td>
>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                     ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>>                   <#else>
>>>>>                     <@maskSensitiveNumber cardNumber=giftCard.cardNumber?if_exists/>
>>>>> @@ -105,7 +105,7 @@ under the License.
>>>>>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</b></#if>
>>>>>                 </td>
>>>>>                 <td class="button-col">
>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                     <a href="<@ofbizUrl>editgiftcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>                   </#if>
>>>>>                 <#-- </td> -->
>>>>> @@ -121,7 +121,7 @@ under the License.
>>>>>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</#if>
>>>>>                 </td>
>>>>>                 <td class="button-col">
>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                     <a href="<@ofbizUrl>editeftaccount?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>                   </#if>
>>>>>                 <#-- </td> -->
>>>>> @@ -143,7 +143,7 @@ under the License.
>>>>>                 <td class="button-col">
>>>>>                   &nbsp;
>>>>>               </#if>
>>>>> -              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session)>
>>>>> +              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>                 <a href="<@ofbizUrl>deletePaymentMethod/viewprofile?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonExpire}</a>
>>>>>               <#else>
>>>>>                 &nbsp;
>>>>>
>>>>> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java (original)
>>>>> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java Mon Jun 25 02:22:58 2012
>>>>> @@ -184,6 +184,9 @@ public class ServiceUtil {
>>>>>      *<b>security check</b>: userLogin partyId must equal partyId, or must have [secEntity][secOperation] permission
>>>>>      */
>>>>>     public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation) {
>>>>> +        return getPartyIdCheckSecurity(userLogin, security, context, result, secEntity, secOperation, null, null);
>>>>> +    }
>>>>> +    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation, String adminSecEntity, String adminSecOperation) {
>>>>>         String partyId = (String) context.get("partyId");
>>>>>         Locale locale = getLocale(context);
>>>>>         if (UtilValidate.isEmpty(partyId)) {
>>>>> @@ -198,9 +201,9 @@ public class ServiceUtil {
>>>>>             return partyId;
>>>>>         }
>>>>>
>>>>> -        // <b>security check</b>: userLogin partyId must equal partyId, or must have PARTYMGR_CREATE permission
>>>>> +        // <b>security check</b>: userLogin partyId must equal partyId, or must have either of the two permissions
>>>>>         if (!partyId.equals(userLogin.getString("partyId"))) {
>>>>> -            if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
>>>>> +            if (!security.hasEntityPermission(secEntity, secOperation, userLogin) && !(adminSecEntity != null && adminSecOperation != null && security.hasEntityPermission(adminSecEntity, adminSecOperation, userLogin))) {
>>>>>                 result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_ERROR);
>>>>>                 String errMsg = UtilProperties.getMessage(ServiceUtil.resource, "serviceUtil.no_permission_to_operation", locale) + ".";
>>>>>                 result.put(ModelService.ERROR_MESSAGE, errMsg);
>>>>>
>>>>>
>>>
>
>

[Jacopo Cappellato-4]

Yes,

Scott explained very well the concept: ACCOUNTING_CREATE will give grants to create all records in Accounting; PAY_INFO_CREATE will give grants only to create payments.

Jacopo

On Jun 26, 2012, at 5:38 AM, Scott Gray wrote:

> If I had to guess I would think your proposition would be reversed:
> PAY_INFO should grant limited access to ACCOUNTING functionality.
> On 26/06/2012, at 12:01 PM, Hans Bakker wrote:
>
>> Yes Jacopo, I know all that, but the question is as I wrote:
>>
>> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>>
>> Regards,
>> Hans
>>
>> On 06/26/2012 03:54 AM, Jacopo Cappellato wrote:
>>> Hi Hans,
>>>
>>> I think that the intended meaning is the following:
>>>
>>> ACCOUNTING_<ACTION>: global permission to all parts of the Accounting application when performing the <ACTION>
>>> ACCOUNTING_ADMIN: global permission to all parts of the Accounting application when performing all the actions
>>>
>>> So for example:
>>>
>>> ACCOUNTING_CREATE: global permission to CREATE all records in the Accounting application
>>>
>>> In other words:
>>>
>>> ACCOUNTING_ADMIN = ACCOUNTING_CREATE + ACCOUNTING_UPDATE + ACCOUNTING_DELETE + ACCOUNTING_VIEW
>>>
>>> (and it is not more than this).
>>>
>>> Jacopo
>>>
>>>
>>> On Jun 25, 2012, at 9:46 PM, Hans Bakker wrote:
>>>
>>>> Hi Jacopo,
>>>>
>>>> thanks for reviewing this commit, my compliments for your work in this area.
>>>>
>>>> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>>>>
>>>> The reason of my commit is that ACCOUNTING_ADMIN should really, as the description states, have full access to the complete accounting component and I did not want to change the other permissions.
>>>>
>>>> Personally i am fine with your suggestion and sure yes, we can do it that way.....
>>>>
>>>> Regards,
>>>> Hans
>>>>
>>>>
>>>> On 06/25/2012 09:27 PM, Jacopo Cappellato wrote:
>>>>> Hi Hans,
>>>>>
>>>>> I understand the issue you are fixing in this commit (and at least another one of last week) but I disagree with the approach.
>>>>> We should never add permission checks on the *_ADMIN permissions: we should instead always use one of the fine grained _CREATE, _UPDATE, _DELETE; the _ADMIN permission will be automatically checked by the framework if the fine grained ones fail.
>>>>> So instead of (for example):
>>>>>
>>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>>        <if>
>>>>>>            <condition>
>>>>>>                <and>
>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>>                    <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>> you should have:
>>>>>
>>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>>        <if>
>>>>>>            <condition>
>>>>>>                <and>
>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_CREATE"/></not>
>>>>>>                    <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>>
>>>>> The code above will grant access to users having at least one of the following:
>>>>> PAYINFO_CREATE
>>>>> PAYINFO_ADMIN
>>>>> ACCOUNTING_CREATE
>>>>> ACCOUNTING_ADMIN
>>>>>
>>>>> Kind regards,
>>>>>
>>>>> Jacopo
>>>>>
>>>>>
>>>>> On Jun 25, 2012, at 4:23 AM, [hidden email] wrote:
>>>>>
>>>>>> Author: hansbak
>>>>>> Date: Mon Jun 25 02:22:58 2012
>>>>>> New Revision: 1353381
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1353381&view=rev
>>>>>> Log:
>>>>>> Give ACCOUNTING_ADMIN the same access as PAY_INFO_ADMIN because part of accounting component
>>>>>>
>>>>>> Modified:
>>>>>>   ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>>>>   ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>>>>   ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>>>>   ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>>>>   ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>>>>   ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>>>>   ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>>>>   ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>>>>   ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml (original)
>>>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml Mon Jun 25 02:22:58 2012
>>>>>> @@ -26,7 +26,6 @@ under the License.
>>>>>>    <SecurityGroupPermission groupId="BIZADMIN" permissionId="PAYPROC_ADMIN"/>
>>>>>>
>>>>>>    <!-- Payment Information security -->
>>>>>> -    <SecurityGroupPermission groupId="FULLADMIN" permissionId="PAY_INFO_ADMIN"/>
>>>>>>    <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_CREATE"/>
>>>>>>    <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_DELETE"/>
>>>>>>    <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_UPDATE"/>
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml (original)
>>>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml Mon Jun 25 02:22:58 2012
>>>>>> @@ -68,7 +68,6 @@ under the License.
>>>>>>
>>>>>>    <!-- add admin to SUPER permission group -->
>>>>>>    <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_ADMIN"/>
>>>>>> -    <SecurityGroupPermission groupId="SUPER" permissionId="PAY_INFO_ADMIN"/>
>>>>>>    <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_COMM_VIEW"/>
>>>>>>    <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_PRINT_CHECKS"/>
>>>>>>    <SecurityGroupPermission groupId="SUPER" permissionId="ACCTG_PREF_ADMIN"/>
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml (original)
>>>>>> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml Mon Jun 25 02:22:58 2012
>>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>>        <if>
>>>>>>            <condition>
>>>>>>                <and>
>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>>                    <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>>> @@ -86,6 +87,7 @@ under the License.
>>>>>>        <if>
>>>>>>            <condition>
>>>>>>                <and>
>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>>                    <not><if-has-permission permission="PAY_INFO" action="_UPDATE"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdFrom" operator="equals"/></not>
>>>>>>                    <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdTo" operator="equals"/></not>
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (original)
>>>>>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java Mon Jun 25 02:22:58 2012
>>>>>> @@ -89,7 +89,7 @@ public class PaymentMethodServices {
>>>>>>
>>>>>>        // <b>security check</b>: userLogin partyId must equal paymentMethod partyId, or must have PAY_INFO_DELETE permission
>>>>>>        if (paymentMethod.get("partyId") == null || !paymentMethod.getString("partyId").equals(userLogin.getString("partyId"))) {
>>>>>> -            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin)) {
>>>>>> +            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>                return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>>                        "AccountingPaymentMethodNoPermissionToDelete", locale));
>>>>>>            }
>>>>>> @@ -139,7 +139,7 @@ public class PaymentMethodServices {
>>>>>>
>>>>>>        Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>
>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>>
>>>>>>        if (result.size() > 0) return result;
>>>>>>
>>>>>> @@ -260,7 +260,7 @@ public class PaymentMethodServices {
>>>>>>
>>>>>>        Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>
>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>>
>>>>>>        if (result.size() > 0) return result;
>>>>>>
>>>>>> @@ -286,7 +286,7 @@ public class PaymentMethodServices {
>>>>>>            return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>>>                    "AccountingCreditCardUpdateWithPaymentMethodId", locale) + paymentMethodId);
>>>>>>        }
>>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>            return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>>>                    "AccountingCreditCardUpdateWithoutPermission", UtilMisc.toMap("partyId", partyId,
>>>>>>                            "paymentMethodId", paymentMethodId), locale));
>>>>>> @@ -488,7 +488,7 @@ public class PaymentMethodServices {
>>>>>>
>>>>>>        Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>
>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>>
>>>>>>        if (result.size() > 0)
>>>>>>            return result;
>>>>>> @@ -545,7 +545,7 @@ public class PaymentMethodServices {
>>>>>>
>>>>>>        Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>
>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>>
>>>>>>        if (result.size() > 0)
>>>>>>            return result;
>>>>>> @@ -574,7 +574,7 @@ public class PaymentMethodServices {
>>>>>>                    "AccountingGiftCardCannotBeUpdated",
>>>>>>                    UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>>>        }
>>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>            return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>>                    "AccountingGiftCardPartyNotAuthorized",
>>>>>>                    UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>>>> @@ -679,7 +679,7 @@ public class PaymentMethodServices {
>>>>>>
>>>>>>        Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>
>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>>
>>>>>>        if (result.size() > 0) return result;
>>>>>>
>>>>>> @@ -777,7 +777,7 @@ public class PaymentMethodServices {
>>>>>>
>>>>>>        Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>
>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>>
>>>>>>        if (result.size() > 0) return result;
>>>>>>
>>>>>> @@ -806,7 +806,7 @@ public class PaymentMethodServices {
>>>>>>                    "AccountingEftAccountCannotBeUpdated",
>>>>>>                    UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>>>        }
>>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>            return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>>                    "AccountingEftAccountCannotBeUpdated",
>>>>>>                    UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/widget/GlScreens.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/accounting/widget/GlScreens.xml (original)
>>>>>> +++ ofbiz/trunk/applications/accounting/widget/GlScreens.xml Mon Jun 25 02:22:58 2012
>>>>>> @@ -445,7 +445,12 @@ under the License.
>>>>>>                <decorator-screen name="CommonAdminChecksDecorator" location="${parameters.mainDecoratorLocation}">
>>>>>>                    <decorator-section name="checks-body">
>>>>>>                        <section>
>>>>>> -                        <condition><if-has-permission permission="PAY_INFO" action="_UPDATE"/></condition>
>>>>>> +                        <condition>
>>>>>> +                            <or>
>>>>>> +                                <if-has-permission permission="ACCOUNTING" action="_ADMIN"/>
>>>>>> +                                <if-has-permission permission="PAY_INFO" action="_UPDATE"/>
>>>>>> +                            </or>
>>>>>> +                        </condition>
>>>>>>                        <widgets>
>>>>>>                            <screenlet title="${uiLabelMap.AccountingSendChecks}">
>>>>>>                                <include-form name="ListChecksToSend" location="component://accounting/widget/PaymentForms.xml"/>
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl (original)
>>>>>> +++ ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl Mon Jun 25 02:22:58 2012
>>>>>> @@ -54,7 +54,7 @@ under the License.
>>>>>>           <#assign statusItem = payment.getRelatedOne("StatusItem", false)>
>>>>>>           <#assign partyName = delegator.findOne("PartyNameView", {"partyId" : payment.partyIdTo}, true)>
>>>>>>           <tr>
>>>>>> -             <#if security.hasPermission("PAY_INFO_VIEW", session) || security.hasPermission("PAY_INFO_ADMIN", session)>
>>>>>> +             <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>               <td><a href="/accounting/control/paymentOverview?paymentId=${payment.paymentId}">${payment.paymentId}</a></td>
>>>>>>             <#else>
>>>>>>               <td>${payment.paymentId}</td>
>>>>>> @@ -342,7 +342,7 @@ under the License.
>>>>>>                      <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>>>                      <br />
>>>>>>
>>>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                        ${creditCard.cardType}
>>>>>>                        <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>>>                        ${creditCard.expireDate}
>>>>>> @@ -469,7 +469,7 @@ under the License.
>>>>>>                <td valign="top" width="60%">
>>>>>>                  <div>
>>>>>>                    <#if giftCard?has_content>
>>>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                        ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>>>                        &nbsp;[<#if oppStatusItem?exists>${oppStatusItem.get("description",locale)}<#else>${orderPaymentPreference.statusId}</#if>]
>>>>>>                      <#else>
>>>>>> @@ -596,7 +596,7 @@ under the License.
>>>>>>               <#if "CREDIT_CARD" == paymentMethod.paymentMethodTypeId>
>>>>>>                 <#assign creditCard = paymentMethodValueMap.creditCard/>
>>>>>>                 <#if (creditCard?has_content)>
>>>>>> -                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>> +                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                     ${creditCard.cardType?if_exists} <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/> ${creditCard.expireDate?if_exists}
>>>>>>                   <#else>
>>>>>>                     ${Static["org.ofbiz.party.contact.ContactHelper"].formatCreditCard(creditCard)}
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy (original)
>>>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy Mon Jun 25 02:22:58 2012
>>>>>> @@ -23,7 +23,7 @@ context.hasCreatePermission = security.h
>>>>>> context.hasUpdatePermission = security.hasEntityPermission("PARTYMGR", "_UPDATE", session);
>>>>>> context.hasDeletePermission = security.hasEntityPermission("PARTYMGR", "_DELETE", session);
>>>>>> // extended pay_info permissions
>>>>>> -context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session);
>>>>>> +context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session);
>>>>>> // extended pcm (party contact mechanism) permissions
>>>>>> context.hasPcmCreatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_CREATE", session);
>>>>>> context.hasPcmUpdatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_UPDATE", session);
>>>>>>
>>>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl (original)
>>>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl Mon Jun 25 02:22:58 2012
>>>>>> @@ -38,7 +38,7 @@ under the License.
>>>>>>    <div class="screenlet-title-bar">
>>>>>>      <ul>
>>>>>>        <li class="h3">${uiLabelMap.PartyPaymentMethodInformation}</li>
>>>>>> -        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session)>
>>>>>> +        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>          <li><a href="<@ofbizUrl>editeftaccount?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewEftAccount}</a></li>
>>>>>>          <li><a href="<@ofbizUrl>editgiftcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewGiftCard}</a></li>
>>>>>>          <li><a href="<@ofbizUrl>editcreditcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewCreditCard}</a></li>
>>>>>> @@ -67,7 +67,7 @@ under the License.
>>>>>>                  ${creditCard.lastNameOnCard}
>>>>>>                  <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>>>                  &nbsp;-&nbsp;
>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                    ${creditCard.cardType}
>>>>>>                    <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>>>                    ${creditCard.expireDate}
>>>>>> @@ -83,7 +83,7 @@ under the License.
>>>>>>                  <#if security.hasEntityPermission("MANUAL", "_PAYMENT", session)>
>>>>>>                    <a href="/accounting/control/manualETx?paymentMethodId=${paymentMethod.paymentMethodId}${externalKeyParam}">${uiLabelMap.PartyManualTx}</a>
>>>>>>                  </#if>
>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                    <a href="<@ofbizUrl>editcreditcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>>                  </#if>
>>>>>>                <#-- </td> -->
>>>>>> @@ -93,7 +93,7 @@ under the License.
>>>>>>                  ${uiLabelMap.AccountingGiftCard}
>>>>>>                </td>
>>>>>>                <td>
>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                    ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>>>                  <#else>
>>>>>>                    <@maskSensitiveNumber cardNumber=giftCard.cardNumber?if_exists/>
>>>>>> @@ -105,7 +105,7 @@ under the License.
>>>>>>                  <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</b></#if>
>>>>>>                </td>
>>>>>>                <td class="button-col">
>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                    <a href="<@ofbizUrl>editgiftcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>>                  </#if>
>>>>>>                <#-- </td> -->
>>>>>> @@ -121,7 +121,7 @@ under the License.
>>>>>>                  <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</#if>
>>>>>>                </td>
>>>>>>                <td class="button-col">
>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                    <a href="<@ofbizUrl>editeftaccount?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>>                  </#if>
>>>>>>                <#-- </td> -->
>>>>>> @@ -143,7 +143,7 @@ under the License.
>>>>>>                <td class="button-col">
>>>>>>                  &nbsp;
>>>>>>              </#if>
>>>>>> -              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session)>
>>>>>> +              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>                <a href="<@ofbizUrl>deletePaymentMethod/viewprofile?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonExpire}</a>
>>>>>>              <#else>
>>>>>>                &nbsp;
>>>>>>
>>>>>> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>> ==============================================================================
>>>>>> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java (original)
>>>>>> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java Mon Jun 25 02:22:58 2012
>>>>>> @@ -184,6 +184,9 @@ public class ServiceUtil {
>>>>>>     *<b>security check</b>: userLogin partyId must equal partyId, or must have [secEntity][secOperation] permission
>>>>>>     */
>>>>>>    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation) {
>>>>>> +        return getPartyIdCheckSecurity(userLogin, security, context, result, secEntity, secOperation, null, null);
>>>>>> +    }
>>>>>> +    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation, String adminSecEntity, String adminSecOperation) {
>>>>>>        String partyId = (String) context.get("partyId");
>>>>>>        Locale locale = getLocale(context);
>>>>>>        if (UtilValidate.isEmpty(partyId)) {
>>>>>> @@ -198,9 +201,9 @@ public class ServiceUtil {
>>>>>>            return partyId;
>>>>>>        }
>>>>>>
>>>>>> -        // <b>security check</b>: userLogin partyId must equal partyId, or must have PARTYMGR_CREATE permission
>>>>>> +        // <b>security check</b>: userLogin partyId must equal partyId, or must have either of the two permissions
>>>>>>        if (!partyId.equals(userLogin.getString("partyId"))) {
>>>>>> -            if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
>>>>>> +            if (!security.hasEntityPermission(secEntity, secOperation, userLogin) && !(adminSecEntity != null && adminSecOperation != null && security.hasEntityPermission(adminSecEntity, adminSecOperation, userLogin))) {
>>>>>>                result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_ERROR);
>>>>>>                String errMsg = UtilProperties.getMessage(ServiceUtil.resource, "serviceUtil.no_permission_to_operation", locale) + ".";
>>>>>>                result.put(ModelService.ERROR_MESSAGE, errMsg);
>>>>>>
>>>>>>
>>>>
>>
>>
>

[hans_bakker]

Ok sounds pretty reasonable, let me see what i can do in my free time....

Regards,
Hans

On 06/26/2012 12:08 PM, Jacopo Cappellato wrote:

> Yes,
>
> Scott explained very well the concept: ACCOUNTING_CREATE will give grants to create all records in Accounting; PAY_INFO_CREATE will give grants only to create payments.
>
> Jacopo
>
> On Jun 26, 2012, at 5:38 AM, Scott Gray wrote:
>
>> If I had to guess I would think your proposition would be reversed:
>> PAY_INFO should grant limited access to ACCOUNTING functionality.
>> On 26/06/2012, at 12:01 PM, Hans Bakker wrote:
>>
>>> Yes Jacopo, I know all that, but the question is as I wrote:
>>>
>>> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>>>
>>> Regards,
>>> Hans
>>>
>>> On 06/26/2012 03:54 AM, Jacopo Cappellato wrote:
>>>> Hi Hans,
>>>>
>>>> I think that the intended meaning is the following:
>>>>
>>>> ACCOUNTING_<ACTION>: global permission to all parts of the Accounting application when performing the <ACTION>
>>>> ACCOUNTING_ADMIN: global permission to all parts of the Accounting application when performing all the actions
>>>>
>>>> So for example:
>>>>
>>>> ACCOUNTING_CREATE: global permission to CREATE all records in the Accounting application
>>>>
>>>> In other words:
>>>>
>>>> ACCOUNTING_ADMIN = ACCOUNTING_CREATE + ACCOUNTING_UPDATE + ACCOUNTING_DELETE + ACCOUNTING_VIEW
>>>>
>>>> (and it is not more than this).
>>>>
>>>> Jacopo
>>>>
>>>>
>>>> On Jun 25, 2012, at 9:46 PM, Hans Bakker wrote:
>>>>
>>>>> Hi Jacopo,
>>>>>
>>>>> thanks for reviewing this commit, my compliments for your work in this area.
>>>>>
>>>>> The question here is do you really want to allow access for ACCOUNTING_CREATE to have the same access as PAY_INFO_CREATE ? Perhaps the intention here was that, by creating a separate action PAY_INFO the ACCOUNTING_CREATE should not have access.
>>>>>
>>>>> The reason of my commit is that ACCOUNTING_ADMIN should really, as the description states, have full access to the complete accounting component and I did not want to change the other permissions.
>>>>>
>>>>> Personally i am fine with your suggestion and sure yes, we can do it that way.....
>>>>>
>>>>> Regards,
>>>>> Hans
>>>>>
>>>>>
>>>>> On 06/25/2012 09:27 PM, Jacopo Cappellato wrote:
>>>>>> Hi Hans,
>>>>>>
>>>>>> I understand the issue you are fixing in this commit (and at least another one of last week) but I disagree with the approach.
>>>>>> We should never add permission checks on the *_ADMIN permissions: we should instead always use one of the fine grained _CREATE, _UPDATE, _DELETE; the _ADMIN permission will be automatically checked by the framework if the fine grained ones fail.
>>>>>> So instead of (for example):
>>>>>>
>>>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>>>         <if>
>>>>>>>             <condition>
>>>>>>>                 <and>
>>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>>> you should have:
>>>>>>
>>>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>>>         <if>
>>>>>>>             <condition>
>>>>>>>                 <and>
>>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_CREATE"/></not>
>>>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>>> The code above will grant access to users having at least one of the following:
>>>>>> PAYINFO_CREATE
>>>>>> PAYINFO_ADMIN
>>>>>> ACCOUNTING_CREATE
>>>>>> ACCOUNTING_ADMIN
>>>>>>
>>>>>> Kind regards,
>>>>>>
>>>>>> Jacopo
>>>>>>
>>>>>>
>>>>>> On Jun 25, 2012, at 4:23 AM, [hidden email] wrote:
>>>>>>
>>>>>>> Author: hansbak
>>>>>>> Date: Mon Jun 25 02:22:58 2012
>>>>>>> New Revision: 1353381
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1353381&view=rev
>>>>>>> Log:
>>>>>>> Give ACCOUNTING_ADMIN the same access as PAY_INFO_ADMIN because part of accounting component
>>>>>>>
>>>>>>> Modified:
>>>>>>>    ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>>>>>    ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>>>>>    ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>>>>>    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>>>>>    ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>>>>>    ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>>>>>    ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>>>>>    ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>>>>>    ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml (original)
>>>>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityGroupDemoData.xml Mon Jun 25 02:22:58 2012
>>>>>>> @@ -26,7 +26,6 @@ under the License.
>>>>>>>     <SecurityGroupPermission groupId="BIZADMIN" permissionId="PAYPROC_ADMIN"/>
>>>>>>>
>>>>>>>     <!-- Payment Information security -->
>>>>>>> -    <SecurityGroupPermission groupId="FULLADMIN" permissionId="PAY_INFO_ADMIN"/>
>>>>>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_CREATE"/>
>>>>>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_DELETE"/>
>>>>>>>     <SecurityGroupPermission groupId="FLEXADMIN" permissionId="PAY_INFO_UPDATE"/>
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml (original)
>>>>>>> +++ ofbiz/trunk/applications/accounting/data/AccountingSecurityPermissionSeedData.xml Mon Jun 25 02:22:58 2012
>>>>>>> @@ -68,7 +68,6 @@ under the License.
>>>>>>>
>>>>>>>     <!-- add admin to SUPER permission group -->
>>>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_ADMIN"/>
>>>>>>> -    <SecurityGroupPermission groupId="SUPER" permissionId="PAY_INFO_ADMIN"/>
>>>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_COMM_VIEW"/>
>>>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCOUNTING_PRINT_CHECKS"/>
>>>>>>>     <SecurityGroupPermission groupId="SUPER" permissionId="ACCTG_PREF_ADMIN"/>
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml (original)
>>>>>>> +++ ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/payment/PaymentServices.xml Mon Jun 25 02:22:58 2012
>>>>>>> @@ -24,6 +24,7 @@ under the License.
>>>>>>>         <if>
>>>>>>>             <condition>
>>>>>>>                 <and>
>>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_CREATE"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdFrom" operator="equals"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="parameters.partyIdTo" operator="equals"/></not>
>>>>>>> @@ -86,6 +87,7 @@ under the License.
>>>>>>>         <if>
>>>>>>>             <condition>
>>>>>>>                 <and>
>>>>>>> +                    <not><if-has-permission permission="ACCOUNTING" action="_ADMIN"/></not>
>>>>>>>                     <not><if-has-permission permission="PAY_INFO" action="_UPDATE"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdFrom" operator="equals"/></not>
>>>>>>>                     <not><if-compare-field field="userLogin.partyId" to-field="payment.partyIdTo" operator="equals"/></not>
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (original)
>>>>>>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java Mon Jun 25 02:22:58 2012
>>>>>>> @@ -89,7 +89,7 @@ public class PaymentMethodServices {
>>>>>>>
>>>>>>>         // <b>security check</b>: userLogin partyId must equal paymentMethod partyId, or must have PAY_INFO_DELETE permission
>>>>>>>         if (paymentMethod.get("partyId") == null || !paymentMethod.getString("partyId").equals(userLogin.getString("partyId"))) {
>>>>>>> -            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin)) {
>>>>>>> +            if (!security.hasEntityPermission("PAY_INFO", "_DELETE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>>                 return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>>>                         "AccountingPaymentMethodNoPermissionToDelete", locale));
>>>>>>>             }
>>>>>>> @@ -139,7 +139,7 @@ public class PaymentMethodServices {
>>>>>>>
>>>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>>
>>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>>>
>>>>>>>         if (result.size() > 0) return result;
>>>>>>>
>>>>>>> @@ -260,7 +260,7 @@ public class PaymentMethodServices {
>>>>>>>
>>>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>>
>>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>>>
>>>>>>>         if (result.size() > 0) return result;
>>>>>>>
>>>>>>> @@ -286,7 +286,7 @@ public class PaymentMethodServices {
>>>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>>>>                     "AccountingCreditCardUpdateWithPaymentMethodId", locale) + paymentMethodId);
>>>>>>>         }
>>>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resource,
>>>>>>>                     "AccountingCreditCardUpdateWithoutPermission", UtilMisc.toMap("partyId", partyId,
>>>>>>>                             "paymentMethodId", paymentMethodId), locale));
>>>>>>> @@ -488,7 +488,7 @@ public class PaymentMethodServices {
>>>>>>>
>>>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>>
>>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>>>
>>>>>>>         if (result.size() > 0)
>>>>>>>             return result;
>>>>>>> @@ -545,7 +545,7 @@ public class PaymentMethodServices {
>>>>>>>
>>>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>>
>>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>>>
>>>>>>>         if (result.size() > 0)
>>>>>>>             return result;
>>>>>>> @@ -574,7 +574,7 @@ public class PaymentMethodServices {
>>>>>>>                     "AccountingGiftCardCannotBeUpdated",
>>>>>>>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>>>>         }
>>>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>>>                     "AccountingGiftCardPartyNotAuthorized",
>>>>>>>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>>>>> @@ -679,7 +679,7 @@ public class PaymentMethodServices {
>>>>>>>
>>>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>>
>>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE");
>>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_CREATE", "ACCOUNTING", "_ADMIN");
>>>>>>>
>>>>>>>         if (result.size() > 0) return result;
>>>>>>>
>>>>>>> @@ -777,7 +777,7 @@ public class PaymentMethodServices {
>>>>>>>
>>>>>>>         Timestamp now = UtilDateTime.nowTimestamp();
>>>>>>>
>>>>>>> -        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE");
>>>>>>> +        String partyId = ServiceUtil.getPartyIdCheckSecurity(userLogin, security, context, result, "PAY_INFO", "_UPDATE", "ACCOUNTING", "_ADMIN");
>>>>>>>
>>>>>>>         if (result.size() > 0) return result;
>>>>>>>
>>>>>>> @@ -806,7 +806,7 @@ public class PaymentMethodServices {
>>>>>>>                     "AccountingEftAccountCannotBeUpdated",
>>>>>>>                     UtilMisc.toMap("errorString", paymentMethodId), locale));
>>>>>>>         }
>>>>>>> -        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin)) {
>>>>>>> +        if (!paymentMethod.getString("partyId").equals(partyId) && !security.hasEntityPermission("PAY_INFO", "_UPDATE", userLogin) && !security.hasEntityPermission("ACCOUNTING", "_ADMIN", userLogin)) {
>>>>>>>             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>>>>>>>                     "AccountingEftAccountCannotBeUpdated",
>>>>>>>                     UtilMisc.toMap("partyId", partyId, "paymentMethodId", paymentMethodId), locale));
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/accounting/widget/GlScreens.xml
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/widget/GlScreens.xml?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/accounting/widget/GlScreens.xml (original)
>>>>>>> +++ ofbiz/trunk/applications/accounting/widget/GlScreens.xml Mon Jun 25 02:22:58 2012
>>>>>>> @@ -445,7 +445,12 @@ under the License.
>>>>>>>                 <decorator-screen name="CommonAdminChecksDecorator" location="${parameters.mainDecoratorLocation}">
>>>>>>>                     <decorator-section name="checks-body">
>>>>>>>                         <section>
>>>>>>> -                        <condition><if-has-permission permission="PAY_INFO" action="_UPDATE"/></condition>
>>>>>>> +                        <condition>
>>>>>>> +                            <or>
>>>>>>> +                                <if-has-permission permission="ACCOUNTING" action="_ADMIN"/>
>>>>>>> +                                <if-has-permission permission="PAY_INFO" action="_UPDATE"/>
>>>>>>> +                            </or>
>>>>>>> +                        </condition>
>>>>>>>                         <widgets>
>>>>>>>                             <screenlet title="${uiLabelMap.AccountingSendChecks}">
>>>>>>>                                 <include-form name="ListChecksToSend" location="component://accounting/widget/PaymentForms.xml"/>
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl (original)
>>>>>>> +++ ofbiz/trunk/applications/order/webapp/ordermgr/order/orderpaymentinfo.ftl Mon Jun 25 02:22:58 2012
>>>>>>> @@ -54,7 +54,7 @@ under the License.
>>>>>>>            <#assign statusItem = payment.getRelatedOne("StatusItem", false)>
>>>>>>>            <#assign partyName = delegator.findOne("PartyNameView", {"partyId" : payment.partyIdTo}, true)>
>>>>>>>            <tr>
>>>>>>> -             <#if security.hasPermission("PAY_INFO_VIEW", session) || security.hasPermission("PAY_INFO_ADMIN", session)>
>>>>>>> +             <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                <td><a href="/accounting/control/paymentOverview?paymentId=${payment.paymentId}">${payment.paymentId}</a></td>
>>>>>>>              <#else>
>>>>>>>                <td>${payment.paymentId}</td>
>>>>>>> @@ -342,7 +342,7 @@ under the License.
>>>>>>>                       <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>>>>                       <br />
>>>>>>>
>>>>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                         ${creditCard.cardType}
>>>>>>>                         <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>>>>                         ${creditCard.expireDate}
>>>>>>> @@ -469,7 +469,7 @@ under the License.
>>>>>>>                 <td valign="top" width="60%">
>>>>>>>                   <div>
>>>>>>>                     <#if giftCard?has_content>
>>>>>>> -                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>>> +                      <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                         ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>>>>                         &nbsp;[<#if oppStatusItem?exists>${oppStatusItem.get("description",locale)}<#else>${orderPaymentPreference.statusId}</#if>]
>>>>>>>                       <#else>
>>>>>>> @@ -596,7 +596,7 @@ under the License.
>>>>>>>                <#if "CREDIT_CARD" == paymentMethod.paymentMethodTypeId>
>>>>>>>                  <#assign creditCard = paymentMethodValueMap.creditCard/>
>>>>>>>                  <#if (creditCard?has_content)>
>>>>>>> -                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>>> +                   <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                      ${creditCard.cardType?if_exists} <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/> ${creditCard.expireDate?if_exists}
>>>>>>>                    <#else>
>>>>>>>                      ${Static["org.ofbiz.party.contact.ContactHelper"].formatCreditCard(creditCard)}
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy (original)
>>>>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/HasPartyPermissions.groovy Mon Jun 25 02:22:58 2012
>>>>>>> @@ -23,7 +23,7 @@ context.hasCreatePermission = security.h
>>>>>>> context.hasUpdatePermission = security.hasEntityPermission("PARTYMGR", "_UPDATE", session);
>>>>>>> context.hasDeletePermission = security.hasEntityPermission("PARTYMGR", "_DELETE", session);
>>>>>>> // extended pay_info permissions
>>>>>>> -context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session);
>>>>>>> +context.hasPayInfoPermission = security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session);
>>>>>>> // extended pcm (party contact mechanism) permissions
>>>>>>> context.hasPcmCreatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_CREATE", session);
>>>>>>> context.hasPcmUpdatePermission = security.hasEntityPermission("PARTYMGR_PCM", "_UPDATE", session);
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl (original)
>>>>>>> +++ ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/PaymentMethods.ftl Mon Jun 25 02:22:58 2012
>>>>>>> @@ -38,7 +38,7 @@ under the License.
>>>>>>>     <div class="screenlet-title-bar">
>>>>>>>       <ul>
>>>>>>>         <li class="h3">${uiLabelMap.PartyPaymentMethodInformation}</li>
>>>>>>> -        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session)>
>>>>>>> +        <#if security.hasEntityPermission("PAY_INFO", "_CREATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>           <li><a href="<@ofbizUrl>editeftaccount?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewEftAccount}</a></li>
>>>>>>>           <li><a href="<@ofbizUrl>editgiftcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewGiftCard}</a></li>
>>>>>>>           <li><a href="<@ofbizUrl>editcreditcard?partyId=${partyId}</@ofbizUrl>">${uiLabelMap.AccountingCreateNewCreditCard}</a></li>
>>>>>>> @@ -67,7 +67,7 @@ under the License.
>>>>>>>                   ${creditCard.lastNameOnCard}
>>>>>>>                   <#if creditCard.suffixOnCard?has_content>&nbsp;${creditCard.suffixOnCard}</#if>
>>>>>>>                   &nbsp;-&nbsp;
>>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                     ${creditCard.cardType}
>>>>>>>                     <@maskSensitiveNumber cardNumber=creditCard.cardNumber?if_exists/>
>>>>>>>                     ${creditCard.expireDate}
>>>>>>> @@ -83,7 +83,7 @@ under the License.
>>>>>>>                   <#if security.hasEntityPermission("MANUAL", "_PAYMENT", session)>
>>>>>>>                     <a href="/accounting/control/manualETx?paymentMethodId=${paymentMethod.paymentMethodId}${externalKeyParam}">${uiLabelMap.PartyManualTx}</a>
>>>>>>>                   </#if>
>>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                     <a href="<@ofbizUrl>editcreditcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>>>                   </#if>
>>>>>>>                 <#-- </td> -->
>>>>>>> @@ -93,7 +93,7 @@ under the License.
>>>>>>>                   ${uiLabelMap.AccountingGiftCard}
>>>>>>>                 </td>
>>>>>>>                 <td>
>>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session)>
>>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_VIEW", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                     ${giftCard.cardNumber?default("N/A")} [${giftCard.pinNumber?default("N/A")}]
>>>>>>>                   <#else>
>>>>>>>                     <@maskSensitiveNumber cardNumber=giftCard.cardNumber?if_exists/>
>>>>>>> @@ -105,7 +105,7 @@ under the License.
>>>>>>>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</b></#if>
>>>>>>>                 </td>
>>>>>>>                 <td class="button-col">
>>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                     <a href="<@ofbizUrl>editgiftcard?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>>>                   </#if>
>>>>>>>                 <#-- </td> -->
>>>>>>> @@ -121,7 +121,7 @@ under the License.
>>>>>>>                   <#if paymentMethod.thruDate?has_content><b>(${uiLabelMap.PartyContactEffectiveThru}:&nbsp;${paymentMethod.thruDate.toString()}</#if>
>>>>>>>                 </td>
>>>>>>>                 <td class="button-col">
>>>>>>> -                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session)>
>>>>>>> +                  <#if security.hasEntityPermission("PAY_INFO", "_UPDATE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                     <a href="<@ofbizUrl>editeftaccount?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonUpdate}</a>
>>>>>>>                   </#if>
>>>>>>>                 <#-- </td> -->
>>>>>>> @@ -143,7 +143,7 @@ under the License.
>>>>>>>                 <td class="button-col">
>>>>>>>                   &nbsp;
>>>>>>>               </#if>
>>>>>>> -              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session)>
>>>>>>> +              <#if security.hasEntityPermission("PAY_INFO", "_DELETE", session) || security.hasEntityPermission("ACCOUNTING", "_ADMIN", session)>
>>>>>>>                 <a href="<@ofbizUrl>deletePaymentMethod/viewprofile?partyId=${partyId}&amp;paymentMethodId=${paymentMethod.paymentMethodId}</@ofbizUrl>">${uiLabelMap.CommonExpire}</a>
>>>>>>>               <#else>
>>>>>>>                 &nbsp;
>>>>>>>
>>>>>>> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java?rev=1353381&r1=1353380&r2=1353381&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java (original)
>>>>>>> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceUtil.java Mon Jun 25 02:22:58 2012
>>>>>>> @@ -184,6 +184,9 @@ public class ServiceUtil {
>>>>>>>      *<b>security check</b>: userLogin partyId must equal partyId, or must have [secEntity][secOperation] permission
>>>>>>>      */
>>>>>>>     public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation) {
>>>>>>> +        return getPartyIdCheckSecurity(userLogin, security, context, result, secEntity, secOperation, null, null);
>>>>>>> +    }
>>>>>>> +    public static String getPartyIdCheckSecurity(GenericValue userLogin, Security security, Map<String, ? extends Object> context, Map<String, Object> result, String secEntity, String secOperation, String adminSecEntity, String adminSecOperation) {
>>>>>>>         String partyId = (String) context.get("partyId");
>>>>>>>         Locale locale = getLocale(context);
>>>>>>>         if (UtilValidate.isEmpty(partyId)) {
>>>>>>> @@ -198,9 +201,9 @@ public class ServiceUtil {
>>>>>>>             return partyId;
>>>>>>>         }
>>>>>>>
>>>>>>> -        // <b>security check</b>: userLogin partyId must equal partyId, or must have PARTYMGR_CREATE permission
>>>>>>> +        // <b>security check</b>: userLogin partyId must equal partyId, or must have either of the two permissions
>>>>>>>         if (!partyId.equals(userLogin.getString("partyId"))) {
>>>>>>> -            if (!security.hasEntityPermission(secEntity, secOperation, userLogin)) {
>>>>>>> +            if (!security.hasEntityPermission(secEntity, secOperation, userLogin) && !(adminSecEntity != null && adminSecOperation != null && security.hasEntityPermission(adminSecEntity, adminSecOperation, userLogin))) {
>>>>>>>                 result.put(ModelService.RESPONSE_MESSAGE, ModelService.RESPOND_ERROR);
>>>>>>>                 String errMsg = UtilProperties.getMessage(ServiceUtil.resource, "serviceUtil.no_permission_to_operation", locale) + ".";
>>>>>>>                 result.put(ModelService.ERROR_MESSAGE, errMsg);
>>>>>>>
>>>>>>>
>>>