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

James Yong commented on OFBIZ-11329:
------------------------------------

Thanks for the patch.  

*Review for OFBIZ-11329:
*
(1) Improvement to SetTimeZoneFromBrowser javascript function is good. i.e. it 
will only set value in sessionStorage if response is success.

(2) Instead of hardcoding in java code the exemption of csrfToken check for 
SetTimeZoneFromBrowser, should set security csrf-token to false in the 
corresponding request map with some comments. But as SetTimeZoneFromBrowser can 
change the data in the database, I think it should not be exempted from CSRF 
token check.

(3) Note that the existing implementation of SetTimeZoneFromBrowser doesn't 
check whether the submitted timezone is valid or different from the UserLogin's 
lastTimeZone. Not sure if this should be in another JIRA issue.

*Review for OFBIZ-11306:*

(1) In line 304 of CsrfUtil.java:
{code:java}
 request.setAttribute("_ERROR_MESSAGE_",
                        "Invalid or missing CSRF token to path '" + 
request.getPathInfo() + "'. Click <a href='"
                                + request.getContextPath() + "'>here</a> to 
continue.");
                if (throwRequestHandlerExceptionOnMissingLocalRequest) {
                    throw new RequestHandlerExceptionAllowExternalRequests();
                }
{code}
there should be no need to check for 
throwRequestHandlerExceptionOnMissingLocalRequest. The property is for missing 
request map but we are handling missing or invalid CSRF token.

(2) Found that additional info which should be returned from ajax request of 
SetTimeZoneFromBrowser, due to the jsonResponseFromRequestAttribute service and 
my implementation of OFBIZ-11306. 

> setUserTimeZone should ran only once based on error
> ---------------------------------------------------
>
>                 Key: OFBIZ-11329
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-11329
>             Project: OFBiz
>          Issue Type: Sub-task
>          Components: framework, webpos
>    Affects Versions: Trunk
>            Reporter: Jacques Le Roux
>            Assignee: James Yong
>            Priority: Minor
>         Attachments: OFBIZ-11329-plugins.patch, OFBIZ-11329.patch, 
> OFBIZ-11329.patch
>
>
> This will be useful when committing CSRF solution as explained in OFBIZ-11306



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to