Hi,

Actually, there are 3. With the current configuration, 2 of those methods will 
never be called. You need to explicitly allow or suppress certain actions. 
Given the nature of the three methods, I think they all need a different 
style:

For 'allow', I suggest to change the message of 'allow' to "Allowing Cross-
site request, request URL: {}, Origin: {}" and keep the level at INFO.

For 'suppress', change the message to "Suppressing a Cross-site action, 
request URL: {}, Origin: {}" and change the level to WARN.

For 'abort' change the message to "Aborting a Cross-site request, request URL: 
{}, Origin: {}" and change the level to WARN or ERROR.

What do you think?

Best regards,
Emond

On dinsdag 20 september 2016 22:11:54 CEST you wrote:
> Hi,
> 
> There are two log.info() calls starting with "Possible CSRF attack..."
> which IMO should be with level WARN.
> Or the chance of false positives is bigger ?
> 
> 
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
> 
> On Tue, Sep 20, 2016 at 10:08 PM, <mgrigo...@apache.org> wrote:
> > Repository: wicket
> > 
> > Updated Branches:
> >   refs/heads/master c819c6c4c -> 247619ab1
> > 
> > WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension
> > 
> > Wrap a debug logiing in LOG.isDebugEnabled()
> > 
> > 
> > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/247619ab
> > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/247619ab
> > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/247619ab
> > 
> > Branch: refs/heads/master
> > Commit: 247619ab176c64acc3d07adcc45725e019e11a62
> > Parents: c819c6c
> > Author: Martin Tzvetanov Grigorov <mgrigo...@apache.org>
> > Authored: Tue Sep 20 22:07:37 2016 +0200
> > Committer: Martin Tzvetanov Grigorov <mgrigo...@apache.org>
> > Committed: Tue Sep 20 22:07:37 2016 +0200
> > 
> > ----------------------------------------------------------------------
> > 
> >  .../protocol/http/CsrfPreventionRequestCycleListener.java    | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > ----------------------------------------------------------------------
> > 
> > 
> > http://git-wip-us.apache.org/repos/asf/wicket/blob/
> > 247619ab/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java
> > ----------------------------------------------------------------------
> > diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java b/wicket-core/src/main/java/
> > org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
> > index ce03862..e6b61dc 100644
> > --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java
> > +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java
> > @@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest;
> > 
> >  import org.apache.wicket.RestartResponseException;
> >  import org.apache.wicket.core.request.handler.IPageRequestHandler;
> >  import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
> > 
> > -import org.apache.wicket.protocol.http.WebApplication;
> > 
> >  import org.apache.wicket.request.IRequestHandler;
> >  import org.apache.wicket.request.IRequestHandlerDelegate;
> >  import org.apache.wicket.request.component.IRequestablePage;
> > 
> > @@ -358,8 +357,11 @@ public class CsrfPreventionRequestCycleListener
> > extends AbstractRequestCycleList
> > 
> >                         }
> >                         else
> >                         {
> > 
> > -                               log.debug("Targeted page {} was opted out
> > of the CSRF origin checks, allowed",
> > -                                       targetedPage.getClass().
> > getName());
> > +                               if (log.isDebugEnabled())
> > +                               {
> > +                                       log.debug("Targeted page {} was
> > opted out of the CSRF origin checks, allowed",
> > +
> > 
> >  targetedPage.getClass().getName());
> > 
> > +                               }
> > 
> >                                 allowHandler(containerRequest, sourceUri,
> > 
> > targetedPage);
> > 
> >                         }
> >                 
> >                 }


Reply via email to