[
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)
Locked
