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 <jacques.le.r...@les7arts.com> 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 my > https://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 at > >> https://issues.apache.org/jira/browse/OFBIZ-11472 with Swapnil > >> > >> If we do so, the link > >> https://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 update https://ofbiz.apache.org/ofbiz-demos.html for that. > >> > >> Jacques > >> > >> >
