#793 added configuration parameter to enable/disable xsrf protection. The protection is disabled by default until it is battle tested.

This commit is contained in:
Sebastian Sdorra
2016-05-24 21:12:09 +02:00
parent 488d4e3323
commit 652b98f53c
3 changed files with 93 additions and 2 deletions

View File

@@ -177,6 +177,7 @@ public class ScmConfiguration
this.skipFailedAuthenticators = other.skipFailedAuthenticators;
this.loginAttemptLimit = other.loginAttemptLimit;
this.loginAttemptLimitTimeout = other.loginAttemptLimitTimeout;
this.enabledXsrfProtection = other.enabledXsrfProtection;
// deprecated fields
this.servername = other.servername;
@@ -424,6 +425,19 @@ public class ScmConfiguration
return disableGroupingGrid;
}
/**
* Returns {@code true} if the cookie xsrf protection is enabled.
*
* @see <a href="https://goo.gl/s67xO3">Issue 793</a>
* @return {@code true} if the cookie xsrf protection is enabled
*
* @since 1.47
*/
public boolean isEnabledXsrfProtection()
{
return enabledXsrfProtection;
}
/**
* Returns true if port forwarding is enabled.
*
@@ -800,6 +814,21 @@ public class ScmConfiguration
this.sslPort = sslPort;
}
/**
* Set {@code true} to enable xsrf cookie protection.
*
* @param enabledXsrfProtection {@code true} to enable xsrf protection
* @see <a href="https://goo.gl/s67xO3">Issue 793</a>
*
* @since 1.47
*/
public void setEnabledXsrfProtection(boolean enabledXsrfProtection)
{
this.enabledXsrfProtection = enabledXsrfProtection;
}
//~--- fields ---------------------------------------------------------------
/** Field description */
@@ -913,4 +942,12 @@ public class ScmConfiguration
/** Field description */
private boolean anonymousAccessEnabled = false;
/**
* Enables xsrf cookie protection.
*
* @since 1.47
*/
@XmlElement(name = "xsrf-protection")
private boolean enabledXsrfProtection = false;
}

View File

@@ -32,6 +32,7 @@ package sonia.scm.security;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.UUID;
import javax.inject.Singleton;
@@ -43,6 +44,7 @@ import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.util.HttpUtil;
import sonia.scm.web.filter.HttpFilter;
@@ -52,7 +54,8 @@ import sonia.scm.web.filter.HttpFilter;
* session, the web interface has to send the token from the cookie as http header on every request. If the filter
* receives an request to a protected session, without proper xsrf header the filter will abort the request and send an
* http error code back to the client. If the filter receives an request to a non protected session, from a non web
* interface client the filter will call the chain.
* interface client the filter will call the chain. The {@link XsrfProtectionFilter} is disabled by default and can be
* enabled with {@link ScmConfiguration#setEnabledXsrfProtection(boolean)}.
*
* TODO for scm-manager 2 we have to store the csrf token as part of the jwt token instead of session.
*
@@ -74,9 +77,29 @@ public final class XsrfProtectionFilter extends HttpFilter
*/
static final String KEY = "X-XSRF-Token";
@Inject
public XsrfProtectionFilter(ScmConfiguration configuration)
{
this.configuration = configuration;
}
@Override
protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws
IOException, ServletException
{
if (configuration.isEnabledXsrfProtection())
{
doXsrfProtection(request, response, chain);
}
else
{
logger.trace("xsrf protection is disabled, skipping check");
chain.doFilter(request, response);
}
}
private void doXsrfProtection(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws
IOException, ServletException
{
HttpSession session = request.getSession(true);
String storedToken = (String) session.getAttribute(KEY);
@@ -109,4 +132,5 @@ public final class XsrfProtectionFilter extends HttpFilter
return UUID.randomUUID().toString();
}
private ScmConfiguration configuration;
}

View File

@@ -44,8 +44,10 @@ import org.junit.Before;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.Mockito;
import static org.mockito.Mockito.*;
import org.mockito.runners.MockitoJUnitRunner;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.util.HttpUtil;
/**
@@ -68,7 +70,9 @@ public class XsrfProtectionFilterTest {
@Mock
private FilterChain chain;
private final XsrfProtectionFilter filter = new XsrfProtectionFilter();
private final ScmConfiguration configuration = new ScmConfiguration();
private final XsrfProtectionFilter filter = new XsrfProtectionFilter(configuration);
/**
* Prepare mocks for testing.
@@ -77,6 +81,7 @@ public class XsrfProtectionFilterTest {
public void setUp(){
when(request.getSession(true)).thenReturn(session);
when(request.getContextPath()).thenReturn("/scm");
configuration.setEnabledXsrfProtection(true);
}
/**
@@ -92,6 +97,31 @@ public class XsrfProtectionFilterTest {
verify(chain).doFilter(request, response);
}
/**
* Test filter method with disabled xsrf protection.
*
* @throws IOException
* @throws ServletException
*/
@Test
public void testDoFilterWithDisabledXsrfProtection() throws IOException, ServletException
{
// disable xsrf protection
configuration.setEnabledXsrfProtection(false);
// set webui user-agent
when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI);
// call the filter
filter.doFilter(request, response, chain);
// verify that no xsrf other any other cookie was set
verify(response, never()).addCookie(Mockito.any(Cookie.class));
// ensure filter chain is called
verify(chain).doFilter(request, response);
}
/**
* Test filter method for first web interface request.
*