[
https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17032224#comment-17032224
]
Jacques Le Roux edited comment on OFBIZ-11306 at 2/7/20 9:10 AM:
-----------------------------------------------------------------
Hi Michael,
Nothing directly related to this issue is commited. There is a reference to
this issue when I committed OFBIZ-11329 which was a blocking subtask but not
directly related to this work, hence it's committed. It was discovered wrong
while working on this issue and had been fixed. Then the Jira-Git relation
makes these commits appears here.
[As I already explained|https://s.apache.org/72j22] I'm not a fan of feature
branches. If we want to adopt a specific Git workflow we need a consensus and I
guess a vote. If we do a such thing I urge all participants to seriouyly read
the articles I cited. Anyway there is not need here, the last patch is the
reference.
bq. Regarding the store of the tokens inside the OFBiz cache: this seems to be
an invalid approach to me. The tokens should be hold in the session or a cookie.
As I (actually James) already explained in my last answer to you, the session
can't be used because of inter webapps requests. Could you please explain how
you envision to use cookies for that?
You might be interested to read
https://stackoverflow.com/questions/20504846/why-is-it-common-to-put-csrf-prevention-tokens-in-cookies.
I also recommend you to read the exchanges we already had with James. For
instance we don't want a sole token value (in a cookie for instance) but have a
strategy where the tokens are generated for each request. Though allowing the
back and forth buttons which is normally an issue with tokens by requests.
Still, this is a WIP. At the moment our main remaining issue is related with
the introduction of REST requests with OFBIZ-11007 (James began before this was
committed).
Of course all ideas are welcome, but please backed up with reviews and strong
support.
was (Author: jacques.le.roux):
Hi Michael,
Nothing directly related to this issue is commited. There is a reference to
this issue when I committed OFBIZ-11329 which was a blocking subtask but not
directly related to this work, hence it's committed. It was discovered wrong
while working on this issue and had been fixed. Then the Jira-Git relation
makes these commits appears here.
[As I already explainedhttps://s.apache.org/72j22] I'm not a fan of feature
branches. If we want to adopt a specific Git workflow we need a consensus and I
guess a vote. If we do a such thing I urge all participants to seriouyly read
the articles I cited. Anyway there is not need here, the last patch is the
reference.
bq. Regarding the store of the tokens inside the OFBiz cache: this seems to be
an invalid approach to me. The tokens should be hold in the session or a cookie.
As I (actually James) already explained in my last answer to you, the session
can't be used because of inter webapps requests. Could you please explain how
you envision to use cookies for that?
You might be interested to read
https://stackoverflow.com/questions/20504846/why-is-it-common-to-put-csrf-prevention-tokens-in-cookies.
I also recommend you to read the exchanges we already had with James. For
instance we don't want a sole token value (in a cookie for instance) but have a
strategy where the tokens are generated for each request. Though allowing the
back and forth buttons which is normally an issue with tokens by requests.
Still, this is a WIP. At the moment our main remaining issue is related with
the introduction of REST requests with OFBIZ-11007 (James began before this was
committed).
Of course all ideas are welcome, but please backed up with reviews and strong
support.
> POC for CSRF Token
> ------------------
>
> Key: OFBIZ-11306
> URL: https://issues.apache.org/jira/browse/OFBIZ-11306
> Project: OFBiz
> Issue Type: Improvement
> Components: ALL APPLICATIONS
> Affects Versions: Upcoming Branch
> Reporter: James Yong
> Assignee: Jacques Le Roux
> Priority: Minor
> Labels: CSRF
> Fix For: Upcoming Branch
>
> Attachments: CsrfTokenAjaxTransform.java, CsrfTokenTransform.java,
> CsrfUtil.java, OFBIZ-11306-v2.patch, OFBIZ-11306.patch, OFBIZ-11306.patch,
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch,
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch,
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch,
> OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch,
> OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch,
> OFBIZ-11306_Plugins.patch
>
>
> CRSF tokens are generated using SecureRandom class (maybe later a JWT with a
> "time out").
> They are stored in the user sessions (for AJAX calls and unauthenticated HTTP
> calls) or OFBiz UtilCache (for authenticated HTTP calls), and verified during
> POST request.
> # In *controllers* a new csrf-token attribute is added to the security tag to
> exempt or force CSRF token check.
> # In *Widget Forms* a hidden token field is auto-generated.
> # In *FTL form* a CSRF token is passed through <@ofbizUrl> to automatise the
> change. Using <@ofbizUrl> macro to generate the CSRF token means there is no
> need to manually add the CSRF token field to each form in the ftl files. It
> will save time for users doing custom implementation and maintenance. While
> there is CSRF token in the form URL, the token is invalidated during form
> submission. So it's uniqueand harmless even though the CSRF token of the form
> submission is shown in the browser address bar.
> # For *Ajax calls* an ajaxPrefilter function (observer on DOM ready) is added
> through OfbizUtil.js (itself called at start in decorators and such)
> # The html metadata is storing the csrf token used by JQuery AJAX. This token
> will not change to another value after it is consumed
> # Csrf tokens for the user are removed from the UtilCache when the user logs
> out or session invalidated.
> The general rule are as follows:
> * RequestMap configured with 'get' method will be exempted from CSRF token
> check.
> * RequestMap configured with 'post' or 'all' method will be subjected to CSRF
> token check. (Note there are discussions that RequestMap with ‘all’ method
> should also not be subjected to CSRF token check. This will be done after
> ensuring a separate uri is used when posting changes.)
> * "main" request URIs are exempted from CSRF token check.
> * Setting csrf-token to false or true on the Request Map will override the
> general rules above.
> To implement:
> * -Allow token map size to be configurable in properties.- OK that's done
> locally
> To Discuss:
> * Invalidate authenticated user session when CSRF token check fails.
> * Configure the general rules in a Service method (which will be run inside
> the constructor of RequestMap class) when determining the final
> securityCsrfToken value.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
