Hi Jacques,

Understand the intent of checkSecureParameter function is to avoid sensitive 
information 
in the URL during POST method. A proposal is made to provide an attribute (i.e. 
allow-query-string-for-service-event) to allow url parameters / query string 
for certain request. Shouldn't the value for this attribute be false, instead 
of true, when no value is specified for the attribute?

The property service.http.parameters.require.encrypted is also not required for 
the proposed change.

As a side note, checkSecureParameter function restricts CSRF attack to a POST 
method but doesn't prevent CSRF entirely. Maybe can explore the feasibility of 
supporting CSRF token for OFBiz form in future..

Regards,
James



On 2019/11/05 07:47:19, Jacques Le Roux <jacques.le.r...@les7arts.com> wrote: 
> Hi James,
> 
> We had service.http.parameters.require.encrypted in url.properties. It's was 
> the same but an all or nothing. It's now removed.
> 
> I'm not against your late proposition. It now means to revert changes from 
> OFBIZ-11260!
> 
> But then it should be reversed. By default 
> allow-query-string-for-service-event would be true and if someone wants to 
> prevent a query string for a 
> particular event then false can be used.
> 
> I'm not sure much people will care of that, not sure what others think...
> 
> Jacques
> 
> Le 05/11/2019 à 01:28, James Yong a écrit :
> > Hi Jacques, Samuel, all,
> >
> > I think the security concerns raised are valid.
> >
> > However we can look into adding an attribute when the url parameter check 
> > isn’t required.
> > For example,
> > <request-map … >
> 
> >    <security https="true" auth=“true” 
> > allow-query-string-for-service-event=“true”/>
> 
> >    …
> >   
> > Regards,
> > James
> >
> > On 2019/10/31 14:20:11, Jacques Le Roux <jacques.le.r...@les7arts.com> 
> > wrote:
> >> Hi Samuel,
> >>
> >> You can go ahead. I became entangled with non ending issues while working 
> >> on this and this change will not change anything about those unrelated 
> >> issues.
> >>
> >> Jacques
> >>
> >> Le 30/10/2019 à 17:01, Jacques Le Roux a écrit :
> >>> Le 30/10/2019 à 15:34, Samuel a écrit :
> >>>> Hi Jacques,
> >>>>
> >>>> On 27/10/2019 17:42, Jacques Le Roux wrote:
> >>>>
> >>>>> … So I have no problem removing this method... and closing OFBIZ-2330, 
> >>>>> maybe after "fixing" OFBIZ-9804...
> >>>> I'm not sure to get your point with OFBIZ-9804, if we simply remove 
> >>>> `checkSecureParameter` we fix this issue, don't we ?
> >>>>
> >>>> Samuel
> >>>>
> >>> Yes, kinda. I prefer to have all calls to 
> >>> updateContactListPartyNoUserLogin similar. Please wait a bit before I 
> >>> close OFBIZ-9804...
> >>>
> >>> Jacques
> >>>
> >>>
> 

Reply via email to