Re: svn commit: r585432 - /ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl

[Adrian Crum]

Jacopo,

How was the original logic incorrect? The original logic was this:

For each application:
   Permission to use the application defaults to false
   If the user has one of the permissions in the application's base-permission list,
     OR if the base-permission list contains "NONE", then permission to use
     the application is true

The reason all of the applications became visible to a user with the OFBTOOLS permission is because
all of the applications have the OFBTOOLS permission in their base-permission list.

My understanding is that the OFBTOOLS permission was intended to grant access to the Webtools
application. I don't know why it has been included in every other application.

-Adrian


[hidden email] wrote:

> Author: jacopoc
> Date: Wed Oct 17 03:00:52 2007
> New Revision: 585432
>
> URL: http://svn.apache.org/viewvc?rev=585432&view=rev
> Log:
> Fixed incorrect logic, introduced in rev. 584400, that was causing a problem in the main application bar: all the applications were visible to a user with the OFBTOOLS permission.
>
> Modified:
>     ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl
>
> Modified: ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl?rev=585432&r1=585431&r2=585432&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl (original)
> +++ ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl Wed Oct 17 03:00:52 2007
> @@ -28,12 +28,12 @@
>      <ul>
>        <#list displayApps as display>
>          <#assign thisApp = display.getContextRoot()>
> -        <#assign permission = false>
> +        <#assign permission = true>
>          <#assign selected = false>
>          <#assign permissions = display.getBasePermission()>
>          <#list permissions as perm>
> -          <#if (perm == "NONE" || security.hasEntityPermission(perm, "_VIEW", session) || security.hasEntityPermission(perm, "_ADMIN", session))>
> -            <#assign permission = true>
> +          <#if (perm != "NONE" && (!security.hasEntityPermission(perm, "_VIEW", session) && !security.hasEntityPermission(perm, "_ADMIN", session)))>
> +            <#assign permission = false>
>            </#if>
>          </#list>
>          <#if permission == true>
>
>
>

[Jacopo Cappellato]

Adrian,

I think that you could be right.
I'm not sure I understand the meaning of the OFBTOOLS permission, but I
don't think it was intended as the base permission for the Webtools
application... but I could be wrong.

Any hints from others?

Jacopo

Adrian Crum wrote:

> Jacopo,
>
> How was the original logic incorrect? The original logic was this:
>
> For each application:
>   Permission to use the application defaults to false
>   If the user has one of the permissions in the application's
> base-permission list,
>     OR if the base-permission list contains "NONE", then permission to use
>     the application is true
>
> The reason all of the applications became visible to a user with the
> OFBTOOLS permission is because all of the applications have the OFBTOOLS
> permission in their base-permission list.
>
> My understanding is that the OFBTOOLS permission was intended to grant
> access to the Webtools application. I don't know why it has been
> included in every other application.
>
> -Adrian
>

[Adrian Crum]

Jacopo,

Doing a Google search, I found these notes from Si:

http://www.opensourcestrategies.com/ofbiz/security.php

According to Si, the list of base permissions should be ANDed, not ORed. I don't know the reasoning
for that, however.

-Adrian

Jacopo Cappellato wrote:

> Adrian,
>
> I think that you could be right.
> I'm not sure I understand the meaning of the OFBTOOLS permission, but I
> don't think it was intended as the base permission for the Webtools
> application... but I could be wrong.
>
> Any hints from others?
>
> Jacopo
>
> Adrian Crum wrote:
>
>> Jacopo,
>>
>> How was the original logic incorrect? The original logic was this:
>>
>> For each application:
>>   Permission to use the application defaults to false
>>   If the user has one of the permissions in the application's
>> base-permission list,
>>     OR if the base-permission list contains "NONE", then permission to
>> use
>>     the application is true
>>
>> The reason all of the applications became visible to a user with the
>> OFBTOOLS permission is because all of the applications have the
>> OFBTOOLS permission in their base-permission list.
>>
>> My understanding is that the OFBTOOLS permission was intended to grant
>> access to the Webtools application. I don't know why it has been
>> included in every other application.
>>
>> -Adrian
>>
>

[David E Jones]

Not sure where Si got his notes, but I think what you are writing is  
correct Adrian.

If I remember right from the initial discussions around this the  
point was to be able to add on other applications and have more  
control over which a user can see, the common scenario being that you  
would want to be able to setup a user that could see the add-on  
application (even though it does have a security permission), but not  
the base ofbiz applications. With that you could just not include the  
OFBTOOLS permission in your add-on application and off you go...

-David


On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:

> Jacopo,
>
> Doing a Google search, I found these notes from Si:
>
> http://www.opensourcestrategies.com/ofbiz/security.php
>
> According to Si, the list of base permissions should be ANDed, not  
> ORed. I don't know the reasoning for that, however.
>
> -Adrian
>
> Jacopo Cappellato wrote:
>
>> Adrian,
>> I think that you could be right.
>> I'm not sure I understand the meaning of the OFBTOOLS permission,  
>> but I don't think it was intended as the base permission for the  
>> Webtools application... but I could be wrong.
>> Any hints from others?
>> Jacopo
>> Adrian Crum wrote:
>>> Jacopo,
>>>
>>> How was the original logic incorrect? The original logic was this:
>>>
>>> For each application:
>>>   Permission to use the application defaults to false
>>>   If the user has one of the permissions in the application's  
>>> base-permission list,
>>>     OR if the base-permission list contains "NONE", then  
>>> permission to use
>>>     the application is true
>>>
>>> The reason all of the applications became visible to a user with  
>>> the OFBTOOLS permission is because all of the applications have  
>>> the OFBTOOLS permission in their base-permission list.
>>>
>>> My understanding is that the OFBTOOLS permission was intended to  
>>> grant access to the Webtools application. I don't know why it has  
>>> been included in every other application.
>>>
>>> -Adrian
>>>
>

[Adrian Crum]

David,

Thanks for your input!

Si makes a good point about permissions that have unexpected side-effects. Let's say you're using
some of the Party Manager screens in a custom app. Inside those screens are permission checks for
the PARTYMGR_VIEW permission, so you give that permission to your custom app users. Oops, now they
have access to the Party Manager application. By ANDing the base-permission list, if they don't have
the OFBTOOLS permission, then the Party Manager application doesn't appear.

The OFBTOOLS permission idea solves the problem. It just seems counter-intuitive in the
base-permission list.

-Adrian

David E Jones wrote:

>
> Not sure where Si got his notes, but I think what you are writing is  
> correct Adrian.
>
> If I remember right from the initial discussions around this the  point
> was to be able to add on other applications and have more  control over
> which a user can see, the common scenario being that you  would want to
> be able to setup a user that could see the add-on  application (even
> though it does have a security permission), but not  the base ofbiz
> applications. With that you could just not include the  OFBTOOLS
> permission in your add-on application and off you go...
>
> -David
>
>
> On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
>
>> Jacopo,
>>
>> Doing a Google search, I found these notes from Si:
>>
>> http://www.opensourcestrategies.com/ofbiz/security.php
>>
>> According to Si, the list of base permissions should be ANDed, not  
>> ORed. I don't know the reasoning for that, however.
>>
>> -Adrian
>>
>> Jacopo Cappellato wrote:
>>
>>> Adrian,
>>> I think that you could be right.
>>> I'm not sure I understand the meaning of the OFBTOOLS permission,  
>>> but I don't think it was intended as the base permission for the  
>>> Webtools application... but I could be wrong.
>>> Any hints from others?
>>> Jacopo
>>> Adrian Crum wrote:
>>>
>>>> Jacopo,
>>>>
>>>> How was the original logic incorrect? The original logic was this:
>>>>
>>>> For each application:
>>>>   Permission to use the application defaults to false
>>>>   If the user has one of the permissions in the application's  
>>>> base-permission list,
>>>>     OR if the base-permission list contains "NONE", then  permission
>>>> to use
>>>>     the application is true
>>>>
>>>> The reason all of the applications became visible to a user with  
>>>> the OFBTOOLS permission is because all of the applications have  the
>>>> OFBTOOLS permission in their base-permission list.
>>>>
>>>> My understanding is that the OFBTOOLS permission was intended to  
>>>> grant access to the Webtools application. I don't know why it has  
>>>> been included in every other application.
>>>>
>>>> -Adrian
>>>>
>>
>

[Jacopo Cappellato]

So, at the end of the story... is my last patch correct, right?
Sorry, but I'm a bit tired today and I'm getting a bit dumb.

Jacopo


Adrian Crum wrote:

> David,
>
> Thanks for your input!
>
> Si makes a good point about permissions that have unexpected
> side-effects. Let's say you're using some of the Party Manager screens
> in a custom app. Inside those screens are permission checks for the
> PARTYMGR_VIEW permission, so you give that permission to your custom app
> users. Oops, now they have access to the Party Manager application. By
> ANDing the base-permission list, if they don't have the OFBTOOLS
> permission, then the Party Manager application doesn't appear.
>
> The OFBTOOLS permission idea solves the problem. It just seems
> counter-intuitive in the base-permission list.
>
> -Adrian
>
> David E Jones wrote:
>
>>
>> Not sure where Si got his notes, but I think what you are writing is  
>> correct Adrian.
>>
>> If I remember right from the initial discussions around this the  
>> point was to be able to add on other applications and have more  
>> control over which a user can see, the common scenario being that you  
>> would want to be able to setup a user that could see the add-on  
>> application (even though it does have a security permission), but not  
>> the base ofbiz applications. With that you could just not include the  
>> OFBTOOLS permission in your add-on application and off you go...
>>
>> -David
>>
>>
>> On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
>>
>>> Jacopo,
>>>
>>> Doing a Google search, I found these notes from Si:
>>>
>>> http://www.opensourcestrategies.com/ofbiz/security.php
>>>
>>> According to Si, the list of base permissions should be ANDed, not  
>>> ORed. I don't know the reasoning for that, however.
>>>
>>> -Adrian
>>>
>>> Jacopo Cappellato wrote:
>>>
>>>> Adrian,
>>>> I think that you could be right.
>>>> I'm not sure I understand the meaning of the OFBTOOLS permission,  
>>>> but I don't think it was intended as the base permission for the  
>>>> Webtools application... but I could be wrong.
>>>> Any hints from others?
>>>> Jacopo
>>>> Adrian Crum wrote:
>>>>
>>>>> Jacopo,
>>>>>
>>>>> How was the original logic incorrect? The original logic was this:
>>>>>
>>>>> For each application:
>>>>>   Permission to use the application defaults to false
>>>>>   If the user has one of the permissions in the application's  
>>>>> base-permission list,
>>>>>     OR if the base-permission list contains "NONE", then  
>>>>> permission to use
>>>>>     the application is true
>>>>>
>>>>> The reason all of the applications became visible to a user with  
>>>>> the OFBTOOLS permission is because all of the applications have  
>>>>> the OFBTOOLS permission in their base-permission list.
>>>>>
>>>>> My understanding is that the OFBTOOLS permission was intended to  
>>>>> grant access to the Webtools application. I don't know why it has  
>>>>> been included in every other application.
>>>>>
>>>>> -Adrian
>>>>>
>>>
>>

[Adrian Crum]

Yes, your patch is correct. Thanks for catching that!

Jacopo Cappellato wrote:

> So, at the end of the story... is my last patch correct, right?
> Sorry, but I'm a bit tired today and I'm getting a bit dumb.
>
> Jacopo
>
>
> Adrian Crum wrote:
>
>> David,
>>
>> Thanks for your input!
>>
>> Si makes a good point about permissions that have unexpected
>> side-effects. Let's say you're using some of the Party Manager screens
>> in a custom app. Inside those screens are permission checks for the
>> PARTYMGR_VIEW permission, so you give that permission to your custom
>> app users. Oops, now they have access to the Party Manager
>> application. By ANDing the base-permission list, if they don't have
>> the OFBTOOLS permission, then the Party Manager application doesn't
>> appear.
>>
>> The OFBTOOLS permission idea solves the problem. It just seems
>> counter-intuitive in the base-permission list.
>>
>> -Adrian
>>
>> David E Jones wrote:
>>
>>>
>>> Not sure where Si got his notes, but I think what you are writing is  
>>> correct Adrian.
>>>
>>> If I remember right from the initial discussions around this the  
>>> point was to be able to add on other applications and have more  
>>> control over which a user can see, the common scenario being that
>>> you  would want to be able to setup a user that could see the add-on  
>>> application (even though it does have a security permission), but
>>> not  the base ofbiz applications. With that you could just not
>>> include the  OFBTOOLS permission in your add-on application and off
>>> you go...
>>>
>>> -David
>>>
>>>
>>> On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
>>>
>>>> Jacopo,
>>>>
>>>> Doing a Google search, I found these notes from Si:
>>>>
>>>> http://www.opensourcestrategies.com/ofbiz/security.php
>>>>
>>>> According to Si, the list of base permissions should be ANDed, not  
>>>> ORed. I don't know the reasoning for that, however.
>>>>
>>>> -Adrian
>>>>
>>>> Jacopo Cappellato wrote:
>>>>
>>>>> Adrian,
>>>>> I think that you could be right.
>>>>> I'm not sure I understand the meaning of the OFBTOOLS permission,  
>>>>> but I don't think it was intended as the base permission for the  
>>>>> Webtools application... but I could be wrong.
>>>>> Any hints from others?
>>>>> Jacopo
>>>>> Adrian Crum wrote:
>>>>>
>>>>>> Jacopo,
>>>>>>
>>>>>> How was the original logic incorrect? The original logic was this:
>>>>>>
>>>>>> For each application:
>>>>>>   Permission to use the application defaults to false
>>>>>>   If the user has one of the permissions in the application's  
>>>>>> base-permission list,
>>>>>>     OR if the base-permission list contains "NONE", then  
>>>>>> permission to use
>>>>>>     the application is true
>>>>>>
>>>>>> The reason all of the applications became visible to a user with  
>>>>>> the OFBTOOLS permission is because all of the applications have  
>>>>>> the OFBTOOLS permission in their base-permission list.
>>>>>>
>>>>>> My understanding is that the OFBTOOLS permission was intended to  
>>>>>> grant access to the Webtools application. I don't know why it has  
>>>>>> been included in every other application.
>>>>>>
>>>>>> -Adrian
>>>>>>
>>>>
>>>
>
>

[David E Jones]

I haven't reviewed the patch, but just to make sure we're all on the  
same page: it is doing an AND, right?

In other words it requires all permissions in the list, not allowing  
just any?

Personally I don't think either AND or OR is implied by the attribute  
or it's usage, just a matter of what is most useful and then perhaps  
documenting it with an annotation in the XSD file or something, and I  
think based on the discussions I remember (I don't remember who  
implemented that though, perhaps Andrew Z.) we decided AND would be  
more useful.

-David


On Oct 17, 2007, at 11:27 AM, Adrian Crum wrote:

> Yes, your patch is correct. Thanks for catching that!
>
> Jacopo Cappellato wrote:
>
>> So, at the end of the story... is my last patch correct, right?
>> Sorry, but I'm a bit tired today and I'm getting a bit dumb.
>> Jacopo
>> Adrian Crum wrote:
>>> David,
>>>
>>> Thanks for your input!
>>>
>>> Si makes a good point about permissions that have unexpected side-
>>> effects. Let's say you're using some of the Party Manager screens  
>>> in a custom app. Inside those screens are permission checks for  
>>> the PARTYMGR_VIEW permission, so you give that permission to your  
>>> custom app users. Oops, now they have access to the Party Manager  
>>> application. By ANDing the base-permission list, if they don't  
>>> have the OFBTOOLS permission, then the Party Manager application  
>>> doesn't appear.
>>>
>>> The OFBTOOLS permission idea solves the problem. It just seems  
>>> counter-intuitive in the base-permission list.
>>>
>>> -Adrian
>>>
>>> David E Jones wrote:
>>>
>>>>
>>>> Not sure where Si got his notes, but I think what you are  
>>>> writing is  correct Adrian.
>>>>
>>>> If I remember right from the initial discussions around this  
>>>> the  point was to be able to add on other applications and have  
>>>> more  control over which a user can see, the common scenario  
>>>> being that you  would want to be able to setup a user that could  
>>>> see the add-on  application (even though it does have a security  
>>>> permission), but not  the base ofbiz applications. With that you  
>>>> could just not include the  OFBTOOLS permission in your add-on  
>>>> application and off you go...
>>>>
>>>> -David
>>>>
>>>>
>>>> On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
>>>>
>>>>> Jacopo,
>>>>>
>>>>> Doing a Google search, I found these notes from Si:
>>>>>
>>>>> http://www.opensourcestrategies.com/ofbiz/security.php
>>>>>
>>>>> According to Si, the list of base permissions should be ANDed,  
>>>>> not  ORed. I don't know the reasoning for that, however.
>>>>>
>>>>> -Adrian
>>>>>
>>>>> Jacopo Cappellato wrote:
>>>>>
>>>>>> Adrian,
>>>>>> I think that you could be right.
>>>>>> I'm not sure I understand the meaning of the OFBTOOLS  
>>>>>> permission,  but I don't think it was intended as the base  
>>>>>> permission for the  Webtools application... but I could be wrong.
>>>>>> Any hints from others?
>>>>>> Jacopo
>>>>>> Adrian Crum wrote:
>>>>>>
>>>>>>> Jacopo,
>>>>>>>
>>>>>>> How was the original logic incorrect? The original logic was  
>>>>>>> this:
>>>>>>>
>>>>>>> For each application:
>>>>>>>   Permission to use the application defaults to false
>>>>>>>   If the user has one of the permissions in the  
>>>>>>> application's  base-permission list,
>>>>>>>     OR if the base-permission list contains "NONE", then  
>>>>>>> permission to use
>>>>>>>     the application is true
>>>>>>>
>>>>>>> The reason all of the applications became visible to a user  
>>>>>>> with  the OFBTOOLS permission is because all of the  
>>>>>>> applications have  the OFBTOOLS permission in their base-
>>>>>>> permission list.
>>>>>>>
>>>>>>> My understanding is that the OFBTOOLS permission was intended  
>>>>>>> to  grant access to the Webtools application. I don't know  
>>>>>>> why it has  been included in every other application.
>>>>>>>
>>>>>>> -Adrian
>>>>>>>
>>>>>
>>>>
>

[Adrian Crum]

The current SVN is doing an AND on the base-permission list.

To recap: I added the ADMIN permission check to the main application bar logic. When I did that I
also changed the logic to OR the base-permission list - based upon my misunderstanding of the usage
of the base-permission list. Jacopo caught that and changed it back to ANDing the base-permission list.

At any rate, the discussion has been informative. David - thanks again for your input!


David E Jones wrote:

>
> I haven't reviewed the patch, but just to make sure we're all on the  
> same page: it is doing an AND, right?
>
> In other words it requires all permissions in the list, not allowing  
> just any?
>
> Personally I don't think either AND or OR is implied by the attribute  
> or it's usage, just a matter of what is most useful and then perhaps  
> documenting it with an annotation in the XSD file or something, and I  
> think based on the discussions I remember (I don't remember who  
> implemented that though, perhaps Andrew Z.) we decided AND would be  
> more useful.
>
> -David
>
>
> On Oct 17, 2007, at 11:27 AM, Adrian Crum wrote:
>
>> Yes, your patch is correct. Thanks for catching that!
>>
>> Jacopo Cappellato wrote:
>>
>>> So, at the end of the story... is my last patch correct, right?
>>> Sorry, but I'm a bit tired today and I'm getting a bit dumb.
>>> Jacopo
>>> Adrian Crum wrote:
>>>
>>>> David,
>>>>
>>>> Thanks for your input!
>>>>
>>>> Si makes a good point about permissions that have unexpected side-
>>>> effects. Let's say you're using some of the Party Manager screens  
>>>> in a custom app. Inside those screens are permission checks for  the
>>>> PARTYMGR_VIEW permission, so you give that permission to your  
>>>> custom app users. Oops, now they have access to the Party Manager  
>>>> application. By ANDing the base-permission list, if they don't  have
>>>> the OFBTOOLS permission, then the Party Manager application  doesn't
>>>> appear.
>>>>
>>>> The OFBTOOLS permission idea solves the problem. It just seems  
>>>> counter-intuitive in the base-permission list.
>>>>
>>>> -Adrian
>>>>
>>>> David E Jones wrote:
>>>>
>>>>>
>>>>> Not sure where Si got his notes, but I think what you are  writing
>>>>> is  correct Adrian.
>>>>>
>>>>> If I remember right from the initial discussions around this  the  
>>>>> point was to be able to add on other applications and have  more  
>>>>> control over which a user can see, the common scenario  being that
>>>>> you  would want to be able to setup a user that could  see the
>>>>> add-on  application (even though it does have a security  
>>>>> permission), but not  the base ofbiz applications. With that you  
>>>>> could just not include the  OFBTOOLS permission in your add-on  
>>>>> application and off you go...
>>>>>
>>>>> -David
>>>>>
>>>>>
>>>>> On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
>>>>>
>>>>>> Jacopo,
>>>>>>
>>>>>> Doing a Google search, I found these notes from Si:
>>>>>>
>>>>>> http://www.opensourcestrategies.com/ofbiz/security.php
>>>>>>
>>>>>> According to Si, the list of base permissions should be ANDed,  
>>>>>> not  ORed. I don't know the reasoning for that, however.
>>>>>>
>>>>>> -Adrian
>>>>>>
>>>>>> Jacopo Cappellato wrote:
>>>>>>
>>>>>>> Adrian,
>>>>>>> I think that you could be right.
>>>>>>> I'm not sure I understand the meaning of the OFBTOOLS  
>>>>>>> permission,  but I don't think it was intended as the base  
>>>>>>> permission for the  Webtools application... but I could be wrong.
>>>>>>> Any hints from others?
>>>>>>> Jacopo
>>>>>>> Adrian Crum wrote:
>>>>>>>
>>>>>>>> Jacopo,
>>>>>>>>
>>>>>>>> How was the original logic incorrect? The original logic was  this:
>>>>>>>>
>>>>>>>> For each application:
>>>>>>>>   Permission to use the application defaults to false
>>>>>>>>   If the user has one of the permissions in the  application's  
>>>>>>>> base-permission list,
>>>>>>>>     OR if the base-permission list contains "NONE", then  
>>>>>>>> permission to use
>>>>>>>>     the application is true
>>>>>>>>
>>>>>>>> The reason all of the applications became visible to a user  
>>>>>>>> with  the OFBTOOLS permission is because all of the  
>>>>>>>> applications have  the OFBTOOLS permission in their base-
>>>>>>>> permission list.
>>>>>>>>
>>>>>>>> My understanding is that the OFBTOOLS permission was intended  
>>>>>>>> to  grant access to the Webtools application. I don't know  why
>>>>>>>> it has  been included in every other application.
>>>>>>>>
>>>>>>>> -Adrian
>>>>>>>>
>>>>>>
>>>>>
>>
>

[Jacques Le Roux]

I commited the patch and I should have read http://www.opensourcestrategies.com/ofbiz/security.php before (though I have surely read
and forgot). Sorry for that

Thanks to Jacopo for having catched it and, to all participants for the explanations I got from this thread !

Jacques

De : "Adrian Crum" <[hidden email]>

> The current SVN is doing an AND on the base-permission list.
>
> To recap: I added the ADMIN permission check to the main application bar logic. When I did that I
> also changed the logic to OR the base-permission list - based upon my misunderstanding of the usage
> of the base-permission list. Jacopo caught that and changed it back to ANDing the base-permission list.
>
> At any rate, the discussion has been informative. David - thanks again for your input!
>
>
> David E Jones wrote:
>
> >
> > I haven't reviewed the patch, but just to make sure we're all on the
> > same page: it is doing an AND, right?
> >
> > In other words it requires all permissions in the list, not allowing
> > just any?
> >
> > Personally I don't think either AND or OR is implied by the attribute
> > or it's usage, just a matter of what is most useful and then perhaps
> > documenting it with an annotation in the XSD file or something, and I
> > think based on the discussions I remember (I don't remember who
> > implemented that though, perhaps Andrew Z.) we decided AND would be
> > more useful.
> >
> > -David
> >
> >
> > On Oct 17, 2007, at 11:27 AM, Adrian Crum wrote:
> >
> >> Yes, your patch is correct. Thanks for catching that!
> >>
> >> Jacopo Cappellato wrote:
> >>
> >>> So, at the end of the story... is my last patch correct, right?
> >>> Sorry, but I'm a bit tired today and I'm getting a bit dumb.
> >>> Jacopo
> >>> Adrian Crum wrote:
> >>>
> >>>> David,
> >>>>
> >>>> Thanks for your input!
> >>>>
> >>>> Si makes a good point about permissions that have unexpected side-
> >>>> effects. Let's say you're using some of the Party Manager screens
> >>>> in a custom app. Inside those screens are permission checks for  the
> >>>> PARTYMGR_VIEW permission, so you give that permission to your
> >>>> custom app users. Oops, now they have access to the Party Manager
> >>>> application. By ANDing the base-permission list, if they don't  have
> >>>> the OFBTOOLS permission, then the Party Manager application  doesn't
> >>>> appear.
> >>>>
> >>>> The OFBTOOLS permission idea solves the problem. It just seems
> >>>> counter-intuitive in the base-permission list.
> >>>>
> >>>> -Adrian
> >>>>
> >>>> David E Jones wrote:
> >>>>
> >>>>>
> >>>>> Not sure where Si got his notes, but I think what you are  writing
> >>>>> is  correct Adrian.
> >>>>>
> >>>>> If I remember right from the initial discussions around this  the
> >>>>> point was to be able to add on other applications and have  more
> >>>>> control over which a user can see, the common scenario  being that
> >>>>> you  would want to be able to setup a user that could  see the
> >>>>> add-on  application (even though it does have a security
> >>>>> permission), but not  the base ofbiz applications. With that you
> >>>>> could just not include the  OFBTOOLS permission in your add-on
> >>>>> application and off you go...
> >>>>>
> >>>>> -David
> >>>>>
> >>>>>
> >>>>> On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
> >>>>>
> >>>>>> Jacopo,
> >>>>>>
> >>>>>> Doing a Google search, I found these notes from Si:
> >>>>>>
> >>>>>> http://www.opensourcestrategies.com/ofbiz/security.php
> >>>>>>
> >>>>>> According to Si, the list of base permissions should be ANDed,
> >>>>>> not  ORed. I don't know the reasoning for that, however.
> >>>>>>
> >>>>>> -Adrian
> >>>>>>
> >>>>>> Jacopo Cappellato wrote:
> >>>>>>
> >>>>>>> Adrian,
> >>>>>>> I think that you could be right.
> >>>>>>> I'm not sure I understand the meaning of the OFBTOOLS
> >>>>>>> permission,  but I don't think it was intended as the base
> >>>>>>> permission for the  Webtools application... but I could be wrong.
> >>>>>>> Any hints from others?
> >>>>>>> Jacopo
> >>>>>>> Adrian Crum wrote:
> >>>>>>>
> >>>>>>>> Jacopo,
> >>>>>>>>
> >>>>>>>> How was the original logic incorrect? The original logic was  this:
> >>>>>>>>
> >>>>>>>> For each application:
> >>>>>>>>   Permission to use the application defaults to false
> >>>>>>>>   If the user has one of the permissions in the  application's
> >>>>>>>> base-permission list,
> >>>>>>>>     OR if the base-permission list contains "NONE", then
> >>>>>>>> permission to use
> >>>>>>>>     the application is true
> >>>>>>>>
> >>>>>>>> The reason all of the applications became visible to a user
> >>>>>>>> with  the OFBTOOLS permission is because all of the
> >>>>>>>> applications have  the OFBTOOLS permission in their base-
> >>>>>>>> permission list.
> >>>>>>>>
> >>>>>>>> My understanding is that the OFBTOOLS permission was intended
> >>>>>>>> to  grant access to the Webtools application. I don't know  why
> >>>>>>>> it has  been included in every other application.
> >>>>>>>>
> >>>>>>>> -Adrian
> >>>>>>>>
> >>>>>>
> >>>>>
> >>
> >
>

[Adrian Crum]

Jacques,

When you have a few minutes, please take a look at https://issues.apache.org/jira/browse/OFBIZ-1349 
- that should eliminate any future confusion.

-Adrian

Jacques Le Roux wrote:

> I commited the patch and I should have read http://www.opensourcestrategies.com/ofbiz/security.php before (though I have surely read
> and forgot). Sorry for that
>
> Thanks to Jacopo for having catched it and, to all participants for the explanations I got from this thread !
>
> Jacques
>
> De : "Adrian Crum" <[hidden email]>
>
>
>>The current SVN is doing an AND on the base-permission list.
>>
>>To recap: I added the ADMIN permission check to the main application bar logic. When I did that I
>>also changed the logic to OR the base-permission list - based upon my misunderstanding of the usage
>>of the base-permission list. Jacopo caught that and changed it back to ANDing the base-permission list.
>>
>>At any rate, the discussion has been informative. David - thanks again for your input!
>>
>>
>>David E Jones wrote:
>>
>>
>>>I haven't reviewed the patch, but just to make sure we're all on the
>>>same page: it is doing an AND, right?
>>>
>>>In other words it requires all permissions in the list, not allowing
>>>just any?
>>>
>>>Personally I don't think either AND or OR is implied by the attribute
>>>or it's usage, just a matter of what is most useful and then perhaps
>>>documenting it with an annotation in the XSD file or something, and I
>>>think based on the discussions I remember (I don't remember who
>>>implemented that though, perhaps Andrew Z.) we decided AND would be
>>>more useful.
>>>
>>>-David
>>>
>>>
>>>On Oct 17, 2007, at 11:27 AM, Adrian Crum wrote:
>>>
>>>
>>>>Yes, your patch is correct. Thanks for catching that!
>>>>
>>>>Jacopo Cappellato wrote:
>>>>
>>>>
>>>>>So, at the end of the story... is my last patch correct, right?
>>>>>Sorry, but I'm a bit tired today and I'm getting a bit dumb.
>>>>>Jacopo
>>>>>Adrian Crum wrote:
>>>>>
>>>>>
>>>>>>David,
>>>>>>
>>>>>>Thanks for your input!
>>>>>>
>>>>>>Si makes a good point about permissions that have unexpected side-
>>>>>>effects. Let's say you're using some of the Party Manager screens
>>>>>>in a custom app. Inside those screens are permission checks for  the
>>>>>>PARTYMGR_VIEW permission, so you give that permission to your
>>>>>>custom app users. Oops, now they have access to the Party Manager
>>>>>>application. By ANDing the base-permission list, if they don't  have
>>>>>>the OFBTOOLS permission, then the Party Manager application  doesn't
>>>>>>appear.
>>>>>>
>>>>>>The OFBTOOLS permission idea solves the problem. It just seems
>>>>>>counter-intuitive in the base-permission list.
>>>>>>
>>>>>>-Adrian
>>>>>>
>>>>>>David E Jones wrote:
>>>>>>
>>>>>>
>>>>>>>Not sure where Si got his notes, but I think what you are  writing
>>>>>>>is  correct Adrian.
>>>>>>>
>>>>>>>If I remember right from the initial discussions around this  the
>>>>>>>point was to be able to add on other applications and have  more
>>>>>>>control over which a user can see, the common scenario  being that
>>>>>>>you  would want to be able to setup a user that could  see the
>>>>>>>add-on  application (even though it does have a security
>>>>>>>permission), but not  the base ofbiz applications. With that you
>>>>>>>could just not include the  OFBTOOLS permission in your add-on
>>>>>>>application and off you go...
>>>>>>>
>>>>>>>-David
>>>>>>>
>>>>>>>
>>>>>>>On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
>>>>>>>
>>>>>>>
>>>>>>>>Jacopo,
>>>>>>>>
>>>>>>>>Doing a Google search, I found these notes from Si:
>>>>>>>>
>>>>>>>>http://www.opensourcestrategies.com/ofbiz/security.php
>>>>>>>>
>>>>>>>>According to Si, the list of base permissions should be ANDed,
>>>>>>>>not  ORed. I don't know the reasoning for that, however.
>>>>>>>>
>>>>>>>>-Adrian
>>>>>>>>
>>>>>>>>Jacopo Cappellato wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>>Adrian,
>>>>>>>>>I think that you could be right.
>>>>>>>>>I'm not sure I understand the meaning of the OFBTOOLS
>>>>>>>>>permission,  but I don't think it was intended as the base
>>>>>>>>>permission for the  Webtools application... but I could be wrong.
>>>>>>>>>Any hints from others?
>>>>>>>>>Jacopo
>>>>>>>>>Adrian Crum wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>Jacopo,
>>>>>>>>>>
>>>>>>>>>>How was the original logic incorrect? The original logic was  this:
>>>>>>>>>>
>>>>>>>>>>For each application:
>>>>>>>>>>  Permission to use the application defaults to false
>>>>>>>>>>  If the user has one of the permissions in the  application's
>>>>>>>>>>base-permission list,
>>>>>>>>>>    OR if the base-permission list contains "NONE", then
>>>>>>>>>>permission to use
>>>>>>>>>>    the application is true
>>>>>>>>>>
>>>>>>>>>>The reason all of the applications became visible to a user
>>>>>>>>>>with  the OFBTOOLS permission is because all of the
>>>>>>>>>>applications have  the OFBTOOLS permission in their base-
>>>>>>>>>>permission list.
>>>>>>>>>>
>>>>>>>>>>My understanding is that the OFBTOOLS permission was intended
>>>>>>>>>>to  grant access to the Webtools application. I don't know  why
>>>>>>>>>>it has  been included in every other application.
>>>>>>>>>>
>>>>>>>>>>-Adrian
>>>>>>>>>>
>>>>>>>>
>
>

[jonwimp]

Adrian,

Last I saw, if the base permission lists more than 1 item, then a userlogin will require ALL the
items on that list.

Pretty strange, yeah. But that's how I saw it work.

Jonathon

Adrian Crum wrote:

> Jacopo,
>
> How was the original logic incorrect? The original logic was this:
>
> For each application:
>   Permission to use the application defaults to false
>   If the user has one of the permissions in the application's
> base-permission list,
>     OR if the base-permission list contains "NONE", then permission to use
>     the application is true
>
> The reason all of the applications became visible to a user with the
> OFBTOOLS permission is because all of the applications have the OFBTOOLS
> permission in their base-permission list.
>
> My understanding is that the OFBTOOLS permission was intended to grant
> access to the Webtools application. I don't know why it has been
> included in every other application.
>
> -Adrian
>
>
> [hidden email] wrote:
>> Author: jacopoc
>> Date: Wed Oct 17 03:00:52 2007
>> New Revision: 585432
>>
>> URL: http://svn.apache.org/viewvc?rev=585432&view=rev
>> Log:
>> Fixed incorrect logic, introduced in rev. 584400, that was causing a
>> problem in the main application bar: all the applications were visible
>> to a user with the OFBTOOLS permission.
>>
>> Modified:
>>     ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl
>>
>> Modified: ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl?rev=585432&r1=585431&r2=585432&view=diff 
>>
>> ==============================================================================
>>
>> --- ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl (original)
>> +++ ofbiz/trunk/framework/common/webcommon/includes/appbar.ftl Wed Oct
>> 17 03:00:52 2007
>> @@ -28,12 +28,12 @@
>>      <ul>
>>        <#list displayApps as display>
>>          <#assign thisApp = display.getContextRoot()>
>> -        <#assign permission = false>
>> +        <#assign permission = true>
>>          <#assign selected = false>
>>          <#assign permissions = display.getBasePermission()>
>>          <#list permissions as perm>
>> -          <#if (perm == "NONE" || security.hasEntityPermission(perm,
>> "_VIEW", session) || security.hasEntityPermission(perm, "_ADMIN",
>> session))>
>> -            <#assign permission = true>
>> +          <#if (perm != "NONE" &&
>> (!security.hasEntityPermission(perm, "_VIEW", session) &&
>> !security.hasEntityPermission(perm, "_ADMIN", session)))>
>> +            <#assign permission = false>
>>            </#if>
>>          </#list>
>>          <#if permission == true>
>>
>>
>>
>
>

[jonwimp]

"AND" would be more useful, I think.

Adding permission items to the list should further restrict or constrict access. And adding more
permissions to the userlogins will further open up access. Seems like a rather complete feature
set to me.

Jonathon

David E Jones wrote:

>
> I haven't reviewed the patch, but just to make sure we're all on the
> same page: it is doing an AND, right?
>
> In other words it requires all permissions in the list, not allowing
> just any?
>
> Personally I don't think either AND or OR is implied by the attribute or
> it's usage, just a matter of what is most useful and then perhaps
> documenting it with an annotation in the XSD file or something, and I
> think based on the discussions I remember (I don't remember who
> implemented that though, perhaps Andrew Z.) we decided AND would be more
> useful.
>
> -David
>
>
> On Oct 17, 2007, at 11:27 AM, Adrian Crum wrote:
>
>> Yes, your patch is correct. Thanks for catching that!
>>
>> Jacopo Cappellato wrote:
>>
>>> So, at the end of the story... is my last patch correct, right?
>>> Sorry, but I'm a bit tired today and I'm getting a bit dumb.
>>> Jacopo
>>> Adrian Crum wrote:
>>>> David,
>>>>
>>>> Thanks for your input!
>>>>
>>>> Si makes a good point about permissions that have unexpected
>>>> side-effects. Let's say you're using some of the Party Manager
>>>> screens in a custom app. Inside those screens are permission checks
>>>> for the PARTYMGR_VIEW permission, so you give that permission to
>>>> your custom app users. Oops, now they have access to the Party
>>>> Manager application. By ANDing the base-permission list, if they
>>>> don't have the OFBTOOLS permission, then the Party Manager
>>>> application doesn't appear.
>>>>
>>>> The OFBTOOLS permission idea solves the problem. It just seems
>>>> counter-intuitive in the base-permission list.
>>>>
>>>> -Adrian
>>>>
>>>> David E Jones wrote:
>>>>
>>>>>
>>>>> Not sure where Si got his notes, but I think what you are writing
>>>>> is  correct Adrian.
>>>>>
>>>>> If I remember right from the initial discussions around this the  
>>>>> point was to be able to add on other applications and have more  
>>>>> control over which a user can see, the common scenario being that
>>>>> you  would want to be able to setup a user that could see the
>>>>> add-on  application (even though it does have a security
>>>>> permission), but not  the base ofbiz applications. With that you
>>>>> could just not include the  OFBTOOLS permission in your add-on
>>>>> application and off you go...
>>>>>
>>>>> -David
>>>>>
>>>>>
>>>>> On Oct 17, 2007, at 10:44 AM, Adrian Crum wrote:
>>>>>
>>>>>> Jacopo,
>>>>>>
>>>>>> Doing a Google search, I found these notes from Si:
>>>>>>
>>>>>> http://www.opensourcestrategies.com/ofbiz/security.php
>>>>>>
>>>>>> According to Si, the list of base permissions should be ANDed,
>>>>>> not  ORed. I don't know the reasoning for that, however.
>>>>>>
>>>>>> -Adrian
>>>>>>
>>>>>> Jacopo Cappellato wrote:
>>>>>>
>>>>>>> Adrian,
>>>>>>> I think that you could be right.
>>>>>>> I'm not sure I understand the meaning of the OFBTOOLS
>>>>>>> permission,  but I don't think it was intended as the base
>>>>>>> permission for the  Webtools application... but I could be wrong.
>>>>>>> Any hints from others?
>>>>>>> Jacopo
>>>>>>> Adrian Crum wrote:
>>>>>>>
>>>>>>>> Jacopo,
>>>>>>>>
>>>>>>>> How was the original logic incorrect? The original logic was this:
>>>>>>>>
>>>>>>>> For each application:
>>>>>>>>   Permission to use the application defaults to false
>>>>>>>>   If the user has one of the permissions in the application's  
>>>>>>>> base-permission list,
>>>>>>>>     OR if the base-permission list contains "NONE", then  
>>>>>>>> permission to use
>>>>>>>>     the application is true
>>>>>>>>
>>>>>>>> The reason all of the applications became visible to a user
>>>>>>>> with  the OFBTOOLS permission is because all of the applications
>>>>>>>> have  the OFBTOOLS permission in their base-permission list.
>>>>>>>>
>>>>>>>> My understanding is that the OFBTOOLS permission was intended
>>>>>>>> to  grant access to the Webtools application. I don't know why
>>>>>>>> it has  been included in every other application.
>>>>>>>>
>>>>>>>> -Adrian
>>>>>>>>
>>>>>>
>>>>>
>>
>