Improper permission check on screens

[Suraj Khurana]

Hello all,

We use *<if-has-permission* element for checking the specified permission
of logged in party.
There are two supported attributes as well in which *permission *is
mandatory and *action *is optional.
If action is not passed then it looks for specific permission.

*For Example: *
<if-has-permission permission="LABEL_MANAGER_VIEW"/>
It should be like <if-has-permission permission="LABEL_MANAGER"
action="_VIEW"/>

   - Now if someone has LABEL_MANAGER_ADMIN permission, then that
   user won't be granted permission. It should check for _ADMIN permission as
   well.


This is properly handled when you pass action attribute, it checks for
specific permission passed and _ADMIN permission as well.

Proposed solution:

We must use permission and action attributes at every such code occurrences
to avoid this situation.

--
Best Regards,
*Suraj Khurana* | Sr. Enterprise Software Engineer
HotWax Commerce  by  HotWax Systems
Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010

[Rishi Solanki]

Suraj,

Thanks for the detailed description, and it would be nice to have this
change.
+1 for the proposal, with caution below;

We have actions as VIEW, CREATE, UPDATE, DELETE and ADMIN. And all actions
from left to right override others, so while doing so we should try to
manage the same.

I mean to say that, if we go for ADMIN then other permission checks will be
pushed aside by the permission services. Same behavior should be maintain
when we do this change.




Rishi Solanki
Sr Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: [phone]
http://www.hotwaxsystems.com
www.hotwax.co

On Thu, Aug 31, 2017 at 4:17 PM, Suraj Khurana <
[hidden email]> wrote:

> Hello all,
>
> We use *<if-has-permission* element for checking the specified permission
> of logged in party.
> There are two supported attributes as well in which *permission *is
> mandatory and *action *is optional.
> If action is not passed then it looks for specific permission.
>
> *For Example: *
> <if-has-permission permission="LABEL_MANAGER_VIEW"/>
> It should be like <if-has-permission permission="LABEL_MANAGER"
> action="_VIEW"/>
>
>    - Now if someone has LABEL_MANAGER_ADMIN permission, then that
>    user won't be granted permission. It should check for _ADMIN permission
> as
>    well.
>
>
> This is properly handled when you pass action attribute, it checks for
> specific permission passed and _ADMIN permission as well.
>
> Proposed solution:
>
> We must use permission and action attributes at every such code occurrences
> to avoid this situation.
>
> --
> Best Regards,
> *Suraj Khurana* | Sr. Enterprise Software Engineer
> HotWax Commerce  by  HotWax Systems
> Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
>

[Devanshu Vyas-2]

Big +1 for the proposal.

Thanks & Regards,
Devanshu Vyas.

On Thu, Aug 31, 2017 at 7:10 PM, Rishi Solanki <[hidden email]>
wrote:

> Suraj,
>
> Thanks for the detailed description, and it would be nice to have this
> change.
> +1 for the proposal, with caution below;
>
> We have actions as VIEW, CREATE, UPDATE, DELETE and ADMIN. And all actions
> from left to right override others, so while doing so we should try to
> manage the same.
>
> I mean to say that, if we go for ADMIN then other permission checks will be
> pushed aside by the permission services. Same behavior should be maintain
> when we do this change.
>
>
>
>
> Rishi Solanki
> Sr Manager, Enterprise Software Development
> HotWax Systems Pvt. Ltd.
> Direct: [phone]
> http://www.hotwaxsystems.com
> www.hotwax.co
>
> On Thu, Aug 31, 2017 at 4:17 PM, Suraj Khurana <
> [hidden email]> wrote:
>
> > Hello all,
> >
> > We use *<if-has-permission* element for checking the specified permission
> > of logged in party.
> > There are two supported attributes as well in which *permission *is
> > mandatory and *action *is optional.
> > If action is not passed then it looks for specific permission.
> >
> > *For Example: *
> > <if-has-permission permission="LABEL_MANAGER_VIEW"/>
> > It should be like <if-has-permission permission="LABEL_MANAGER"
> > action="_VIEW"/>
> >
> >    - Now if someone has LABEL_MANAGER_ADMIN permission, then that
> >    user won't be granted permission. It should check for _ADMIN
> permission
> > as
> >    well.
> >
> >
> > This is properly handled when you pass action attribute, it checks for
> > specific permission passed and _ADMIN permission as well.
> >
> > Proposed solution:
> >
> > We must use permission and action attributes at every such code
> occurrences
> > to avoid this situation.
> >
> > --
> > Best Regards,
> > *Suraj Khurana* | Sr. Enterprise Software Engineer
> > HotWax Commerce  by  HotWax Systems
> > Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
> >
>

[Suraj Khurana]

Thanks everyone for your inputs.

Here <https://issues.apache.org/jira/browse/OFBIZ-9740> is the Jira ticket
created for the same.

--
Best Regards,
*Suraj Khurana* | Sr. Enterprise Software Engineer
*HotWax Commerce*  by  *HotWax Systems*
Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010


On Sun, Sep 10, 2017 at 1:50 PM, Devanshu Vyas <[hidden email]>
wrote:

> Big +1 for the proposal.
>
> Thanks & Regards,
> Devanshu Vyas.
>
> On Thu, Aug 31, 2017 at 7:10 PM, Rishi Solanki <[hidden email]>
> wrote:
>
> > Suraj,
> >
> > Thanks for the detailed description, and it would be nice to have this
> > change.
> > +1 for the proposal, with caution below;
> >
> > We have actions as VIEW, CREATE, UPDATE, DELETE and ADMIN. And all
> actions
> > from left to right override others, so while doing so we should try to
> > manage the same.
> >
> > I mean to say that, if we go for ADMIN then other permission checks will
> be
> > pushed aside by the permission services. Same behavior should be maintain
> > when we do this change.
> >
> >
> >
> >
> > Rishi Solanki
> > Sr Manager, Enterprise Software Development
> > HotWax Systems Pvt. Ltd.
> > Direct: [phone]
> > http://www.hotwaxsystems.com
> > www.hotwax.co
> >
> > On Thu, Aug 31, 2017 at 4:17 PM, Suraj Khurana <
> > [hidden email]> wrote:
> >
> > > Hello all,
> > >
> > > We use *<if-has-permission* element for checking the specified
> permission
> > > of logged in party.
> > > There are two supported attributes as well in which *permission *is
> > > mandatory and *action *is optional.
> > > If action is not passed then it looks for specific permission.
> > >
> > > *For Example: *
> > > <if-has-permission permission="LABEL_MANAGER_VIEW"/>
> > > It should be like <if-has-permission permission="LABEL_MANAGER"
> > > action="_VIEW"/>
> > >
> > >    - Now if someone has LABEL_MANAGER_ADMIN permission, then that
> > >    user won't be granted permission. It should check for _ADMIN
> > permission
> > > as
> > >    well.
> > >
> > >
> > > This is properly handled when you pass action attribute, it checks for
> > > specific permission passed and _ADMIN permission as well.
> > >
> > > Proposed solution:
> > >
> > > We must use permission and action attributes at every such code
> > occurrences
> > > to avoid this situation.
> > >
> > > --
> > > Best Regards,
> > > *Suraj Khurana* | Sr. Enterprise Software Engineer
> > > HotWax Commerce  by  HotWax Systems
> > > Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
> > >
> >
>