Hi James,
Inline...
Le 06/11/2019 à 17:53, James Yong a écrit :
Hi Jacques,
Understand the intent of checkSecureParameter function is to avoid sensitive
information
in the URL during POST method.
Was ;) It does not exist anymore, in trunk at least, after OFBIZ-11260.
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?
Depends, if we want to have the same behaviour than in trunk after OFBIZ-11260 then it should be false by default. Note that we "never" get back from
changes, except in case of regression.
The property service.http.parameters.require.encrypted is also not required for
the proposed change.
Yes, re-introducing will need to revert work done with OFBIZ-11260
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..
Yes, we already do: https://issues.apache.org/jira/browse/OFBIZ-10427
Jacques
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
