diff --git a/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenValidator.java b/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenValidator.java index 437174f57e..241b0ac001 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenValidator.java +++ b/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenValidator.java @@ -30,12 +30,15 @@ */ package sonia.scm.security; +import com.google.common.collect.ImmutableSet; import sonia.scm.plugin.Extension; import javax.inject.Inject; import javax.inject.Provider; import javax.servlet.http.HttpServletRequest; +import java.util.Locale; import java.util.Optional; +import java.util.Set; /** * Validates xsrf protected access tokens. The validator check if the current request contains an xsrf key which is @@ -47,7 +50,11 @@ import java.util.Optional; */ @Extension public class XsrfAccessTokenValidator implements AccessTokenValidator { - + + private static final Set ALLOWED_METHOD = ImmutableSet.of( + "GET", "HEAD", "OPTIONS" + ); + private final Provider requestProvider; @@ -65,8 +72,10 @@ public class XsrfAccessTokenValidator implements AccessTokenValidator { public boolean validate(AccessToken accessToken) { Optional xsrfClaim = accessToken.getCustom(Xsrf.TOKEN_KEY); if (xsrfClaim.isPresent()) { - String xsrfHeaderValue = requestProvider.get().getHeader(Xsrf.HEADER_KEY); - return xsrfClaim.get().equals(xsrfHeaderValue); + HttpServletRequest request = requestProvider.get(); + String xsrfHeaderValue = request.getHeader(Xsrf.HEADER_KEY); + return ALLOWED_METHOD.contains(request.getMethod().toUpperCase(Locale.ENGLISH)) + || xsrfClaim.get().equals(xsrfHeaderValue); } return true; } diff --git a/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenValidatorTest.java b/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenValidatorTest.java index a89744cd6e..0692a76bd2 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenValidatorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenValidatorTest.java @@ -1,19 +1,19 @@ /** * Copyright (c) 2014, Sebastian Sdorra * All rights reserved. - * + *

* Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: - * + *

* 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. + * this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + *

* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE @@ -24,33 +24,39 @@ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * + *

* http://bitbucket.org/sdorra/scm-manager - * */ package sonia.scm.security; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.EnumSource; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.GET; import java.util.Optional; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.when; /** * Tests {@link XsrfAccessTokenValidator}. - * + * * @author Sebastian Sdorra */ -@RunWith(MockitoJUnitRunner.class) -public class XsrfAccessTokenValidatorTest { +@ExtendWith(MockitoExtension.class) +class XsrfAccessTokenValidatorTest { @Mock private HttpServletRequest request; @@ -59,62 +65,94 @@ public class XsrfAccessTokenValidatorTest { private AccessToken accessToken; private XsrfAccessTokenValidator validator; - + /** * Prepare object under test. */ - @Before - public void prepareObjectUnderTest() { + @BeforeEach + void prepareObjectUnderTest() { validator = new XsrfAccessTokenValidator(() -> request); } - - /** - * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)}. - */ - @Test - public void testValidate() { + + @Nested + class RequestMethodPost { + + @BeforeEach + void setRequestMethod() { + lenient().when(request.getMethod()).thenReturn("POST"); + } + + /** + * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)}. + */ + @Test + void testValidate() { + // prepare + when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.of("abc")); + when(request.getHeader(Xsrf.HEADER_KEY)).thenReturn("abc"); + + // execute and assert + assertTrue(validator.validate(accessToken)); + } + + /** + * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)} with wrong header. + */ + @Test + void testValidateWithWrongHeader() { + // prepare + when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.of("abc")); + when(request.getHeader(Xsrf.HEADER_KEY)).thenReturn("123"); + + // execute and assert + assertFalse(validator.validate(accessToken)); + } + + /** + * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)} without header. + */ + @Test + void testValidateWithoutHeader() { + // prepare + when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.of("abc")); + + // execute and assert + assertFalse(validator.validate(accessToken)); + } + + /** + * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)} without claims key. + */ + @Test + void testValidateWithoutClaimsKey() { + // prepare + when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.empty()); + + // execute and assert + assertTrue(validator.validate(accessToken)); + } + + } + + @ParameterizedTest + @CsvSource({"GET", "HEAD", "OPTIONS"}) + void shouldNotValidateReadRequests(String method) { // prepare + when(request.getMethod()).thenReturn(method); when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.of("abc")); - when(request.getHeader(Xsrf.HEADER_KEY)).thenReturn("abc"); - + // execute and assert assertTrue(validator.validate(accessToken)); } - - /** - * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)} with wrong header. - */ - @Test - public void testValidateWithWrongHeader() { + + @ParameterizedTest + @CsvSource({"POST", "PUT", "DELETE", "PATCH"}) + void shouldFailValidationOfWriteRequests(String method) { // prepare + when(request.getMethod()).thenReturn(method); when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.of("abc")); - when(request.getHeader(Xsrf.HEADER_KEY)).thenReturn("123"); - + // execute and assert assertFalse(validator.validate(accessToken)); } - - /** - * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)} without header. - */ - @Test - public void testValidateWithoutHeader() { - // prepare - when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.of("abc")); - - // execute and assert - assertFalse(validator.validate(accessToken)); - } - - /** - * Tests {@link XsrfAccessTokenValidator#validate(AccessToken)} without claims key. - */ - @Test - public void testValidateWithoutClaimsKey() { - // prepare - when(accessToken.getCustom(Xsrf.TOKEN_KEY)).thenReturn(Optional.empty()); - - // execute and assert - assertTrue(validator.validate(accessToken)); - } }