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

Greg Huber edited comment on ROL-2058 at 3/20/15 1:58 PM:
----------------------------------------------------------

I would say you need the no-opp setSalt(..) for completeness (messages are 
there for a reason).   The timeout is personal I guess. 

I never really like the way the salt csrf works here for struts UI as its leads 
to problems of saving (restart the server and the salt cache is gone).  I 
prefer to use an interceptor instead of the filter, and then return back to the 
action rather than the general exception page on any salt issue.  It is then 
possible to resubmit the page with a new salt value.

<interceptor-ref name="UIActionSaltInterceptor" >
                    <param name="excludeMethods">*</param>
                    <param name="includeMethods">save</param>
</interceptor-ref>
{code}
public class UIActionSaltInterceptor extends MethodFilterInterceptor {

        private static final Logger log = LoggerFactory
                        .getLogger(UIActionSaltInterceptor.class);

        private String inputResultName = Action.INPUT;

        /**
         * Set the <code>inputResultName</code> (result name to be returned when
         * action fails the salt check). Default to {@link Action#INPUT}
         * 
         * struts.xml interceptor parameter:
         * 
         * <param name="inputResultName">input</param>
         * 
         * @param inputResultName
         *            what result name to use when there is a salt error.
         */
        public void setInputResultName(String inputResultName) {
                this.inputResultName = inputResultName;
        }

        /**
         * Intercept {@link ActionInvocation} and returns a
         * <code>inputResultName</code> when action fails the salt check.
         * 
         * @return String result name
         */
        @Override
        protected String doIntercept(ActionInvocation invocation) throws 
Exception {
                Object action = invocation.getAction();

                if (action instanceof UIAction) {

                        UIAction theAction = (UIAction) action;

                        final ActionContext context = 
invocation.getInvocationContext();
                        HttpServletRequest request = (HttpServletRequest) 
context
                                        .get(ServletActionContext.HTTP_REQUEST);

                        // Check post
                        if (("POST").equals(request.getMethod())) {

                                SaltCache saltCache = SaltCache.getInstance();
                                if (saltCache.isCacheEnabled()) {

                                        String salt = (String) 
request.getParameter("salt");

                                        if (salt == null || saltCache.get(salt) 
== null
                                                        || 
saltCache.get(salt).equals(false)) {

                                                if (log.isDebugEnabled())
                                                        log.debug("Failed salt 
check on action "
                                                                        + 
theAction
                                                                        + ", 
returning result name 'input'");

                                                // Indicate the error to the 
user
                                                
theAction.addError("error.permissions.deniedSalt");

                                                return inputResultName;

                                        }

                                        // Cleanup
                                        saltCache.remove(salt);
                                }
                        }
                }

                return invocation.invoke();
        }
} 
{code}


was (Author: gregh99):
I would say you need the no-opp setSalt(..) for completeness (messages are 
there for a reason).   The timeout is personal I guess. 

I never really like the way the salt csrf works here for struts UI as its leads 
to problems of saving (restart the server and the salt cache is gone).  I 
prefer to use an interceptor instead of the filter, and then return back to the 
action rather than the general exception page on any salt issue.  It is then 
possible to resubmit the page with a new salt value.

<interceptor-ref name="UIActionSaltInterceptor" >
                    <param name="excludeMethods">*</param>
                    <param name="includeMethods">save</param>
</interceptor-ref>
{code:java}
public class UIActionSaltInterceptor extends MethodFilterInterceptor {

        private static final Logger log = LoggerFactory
                        .getLogger(UIActionSaltInterceptor.class);

        private String inputResultName = Action.INPUT;

        /**
         * Set the <code>inputResultName</code> (result name to be returned when
         * action fails the salt check). Default to {@link Action#INPUT}
         * 
         * struts.xml interceptor parameter:
         * 
         * <param name="inputResultName">input</param>
         * 
         * @param inputResultName
         *            what result name to use when there is a salt error.
         */
        public void setInputResultName(String inputResultName) {
                this.inputResultName = inputResultName;
        }

        /**
         * Intercept {@link ActionInvocation} and returns a
         * <code>inputResultName</code> when action fails the salt check.
         * 
         * @return String result name
         */
        @Override
        protected String doIntercept(ActionInvocation invocation) throws 
Exception {
                Object action = invocation.getAction();

                if (action instanceof UIAction) {

                        UIAction theAction = (UIAction) action;

                        final ActionContext context = 
invocation.getInvocationContext();
                        HttpServletRequest request = (HttpServletRequest) 
context
                                        .get(ServletActionContext.HTTP_REQUEST);

                        // Check post
                        if (("POST").equals(request.getMethod())) {

                                SaltCache saltCache = SaltCache.getInstance();
                                if (saltCache.isCacheEnabled()) {

                                        String salt = (String) 
request.getParameter("salt");

                                        if (salt == null || saltCache.get(salt) 
== null
                                                        || 
saltCache.get(salt).equals(false)) {

                                                if (log.isDebugEnabled())
                                                        log.debug("Failed salt 
check on action "
                                                                        + 
theAction
                                                                        + ", 
returning result name 'input'");

                                                // Indicate the error to the 
user
                                                
theAction.addError("error.permissions.deniedSalt");

                                                return inputResultName;

                                        }

                                        // Cleanup
                                        saltCache.remove(salt);
                                }
                        }
                }

                return invocation.invoke();
        }
} 
{code:java}

> No salt renewal on POST request
> -------------------------------
>
>                 Key: ROL-2058
>                 URL: https://issues.apache.org/jira/browse/ROL-2058
>             Project: Apache Roller
>          Issue Type: Bug
>          Components: User Interface - General
>    Affects Versions: 5.1.1
>         Environment: WildFly 8.2.0.Final
>            Reporter: Kohei Nozaki
>            Assignee: David Johnson
>             Fix For: 5.1.2
>
>         Attachments: ROL-2058.patch
>
>
> Roller continues using previous salt value which sent from client as POST 
> parameter. this leads fixing of salt value in the form element of html, and 
> brings ServletException("Security Violation") by ValidateSaltFilter at some 
> use cases (e.g. long-term editing over 60 minutes) unexpectedly.
> Seems to that the cause is existence of 
> org.apache.roller.weblogger.ui.struts2.util.UIAction#setSalt(String) method. 
> this overwrites salt with previous value which sent by client as POST 
> parameter. it's unnecessary behavior because new salt value comes through 
> preceding invocation of UIAction#setRequest(Map).
> Original discussion in the mailing list:
> http://markmail.org/search/?q=list%3Aorg.apache.roller.user#query:list%3Aorg.apache.roller.user+page:1+mid:tnqn4qjuwmwun4oh+state:results



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to