Hi James,
1. I like the idea. Maybe we could create the class but let the implementation
(with explanations) for those who really need it?
2. I did not mean there was a correlation between csrf-token check and auth
check. My main idea is to avoid hardcoded things like
if (requestUri.equals("main")) { return false; }
else {
We can set that in controllers using csrf-token="false", like it is for "logout". It's not difficult, just a global S/R. I thought there were other
cases but it seems not. so I now wonder if it's a good idea.
I don't think there is a need to systematise a default to csrf-token="false" when auth="false". I just want to work on OFBIZ-4956 and while doing so
check that if we change auth="false" to true, as it implies csrf-token="true", there will not be undesired side effects. And in other cases
(auth="false" must remain) we need to decide if should set the CSRF token check to false.
Anyway it does not hinder our work on CSRF, but I feel I now miss something that I wanted to say then, just an intuitive feeling, certainly nothing
serious
I think we are ready and I'll soon commit...
Jacques
Le 29/03/2020 à 03:28, James Yong a écrit :
Hi Jacques,
For 1, seems like a ICsrfDefenseStrategy class implementation issue. We can use
another Jira for the enhancement / discussion when this JIRA (OFBIZ-11306) is
completed.
For 2, csrf-token check is independent of auth check, and the current implementation should work as it is. So reviewing
whether auth="false" be "true", should be in another JIRA (i.e. OFBIZ-4956). If there is a need for
all auth="false" to default to csrf-token="false", we can implement another ICsrfDefenseStrategy
class or modify the existing CsrfDefenseStrategy class.
Regards,
James
On 2020/03/27 18:16:58, Jacques Le Roux<[email protected]> wrote:
Hi All,
Before I create a PR as a last opportunity to allow reviews and tests, I'd like
to ask 2 last questions:
1. should we not use a JWT rather than a (pseudo) random value for the CSRF
token, this for timeout reason? Don't get me wrong I'm sure that the
random values generated by java.security.SecureRandom, as currently used,
are safe enough. It's just that I wonder about the timeout. Should we care?
2. In relation with OFBIZ-4956, we need to check the remaining 195 cases where
auth="false" and decide if we should change to "true", with the CSRF
defense then used by default. In other cases (auth="false" must remain) we
need to decide if should set the CSRF token check to false.
Apart that
myhttps://github.com/JacquesLeRoux/ofbiz-framework/tree/POC-for-CSRF-Token-OFBIZ-11306
branch is ready to create a PR. We can't wait too
long about those 2 points, even if the 2nd needs a "bit" of work. Anyway, for
now I'll wait answers, and hopefully help for OFBIZ-4956.
Thanks
Jacques
Le 26/03/2020 à 07:39, James Yong a écrit :
+1 with CSRF defense enabled in Demo
Hi,
I thought about that a bit more. I suggest to let the stable version (soon,
R17) as is, ie with CSRF defense enabled. This way users, mostly
interested in stable, would see the real situation.
And to use the NoCsrfDefenseStrategy in trunk. So developers, often brought to
use the trunk for development reasons, would have more latitude; as
they certainly will do locally.
If nobody disagree we will do so
athttps://issues.apache.org/jira/browse/OFBIZ-11472 with Swapnil
If we do so, the
linkhttps://demo-stable.ofbiz.apache.org/ordermgr/control/main?USERNAME=admin&PASSWORD=ofbiz&JavaScriptEnabled=Y
will no longer work.
https://demo-stable.ofbiz.apache.org/ordermgr should be used and we need to
updatehttps://ofbiz.apache.org/ofbiz-demos.html for that.
Jacques
