[ 
https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17032541#comment-17032541
 ] 

James Yong commented on OFBIZ-11306:
------------------------------------

Hi Michael,
{quote}Yes, my main point is that we should avoid committing (portions) of a 
new feature into trunk directly until the feature is finished, reviewed, testet 
and accepted.
{quote}
Agreed. 

Some bugs (OFBIZ-11319, OFBIZ-11329) are discovered while working on this 
feature. Without fixing the bugs, the feature cannot be adequately tested. 

The improvement (OFBIZ-11317) introduced a 'controlPath' attribute to ofbizUrl 
macro to allow ofbizUrl macro to accept "inter-app" URL. As the new feature 
make uses of ofbizUrl macro to dynamically add the csrf token to links, this 
improvement allow the new feature to add csrf token to both "intra-app" and 
"inter-app" links with equal ease.
{quote}why do you want to store the tokens in the cache instead of the session?
{quote}
To add on to Jacques reply.

OFBiz contains a group of web applications. Pages in a web application (e.g. A) 
may contain links to another web application (e.g. B). These links which can 
contain CSRF tokens, are generated by A. However B needs to know whether the 
CSRF tokens received are correct or not. Storing the tokens in a cache map 
allows B to read the CSRF tokens generated by A.

Do note that the following tokens are stored in cache map:
 # Tokens for non-ajax links after user has logged in.

The following tokens are stored in http sessions as there is no need for 
inter-app link:
 # Tokens for Ajax links
 # Tokens for links where user has not login.    

> 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.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)

Reply via email to