From 2bdb0231c263625e97f86a963439a9358647f8bd Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 2 May 2016 16:49:37 -0400 Subject: [PATCH] CookieCsrfTokenRepository supports HttpOnly CookieCsrfTokenRepository supports HttpOnly Fixes gh-3835 * Add Servlet 3 tests and javadocs Issue gh-3835 * Add copyright header Issue gh-3835 --- .../web/csrf/CookieCsrfTokenRepository.java | 37 +++++++- ...ookieCsrfTokenRepositoryServlet3Tests.java | 92 +++++++++++++++++++ .../csrf/CookieCsrfTokenRepositoryTests.java | 30 +++++- 3 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryServlet3Tests.java diff --git a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java index 1a85dd9761..9b6f90b599 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -16,6 +16,7 @@ package org.springframework.security.web.csrf; +import java.lang.reflect.Method; import java.util.UUID; import javax.servlet.http.Cookie; @@ -23,6 +24,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.springframework.util.Assert; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.util.WebUtils; @@ -47,6 +49,17 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { private String cookieName = DEFAULT_CSRF_COOKIE_NAME; + private final Method setHttpOnlyMethod; + + private boolean cookieHttpOnly; + + public CookieCsrfTokenRepository() { + this.setHttpOnlyMethod = ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", boolean.class); + if (this.setHttpOnlyMethod != null) { + this.cookieHttpOnly = true; + } + } + @Override public CsrfToken generateToken(HttpServletRequest request) { return new DefaultCsrfToken(this.headerName, this.parameterName, @@ -66,6 +79,10 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { else { cookie.setMaxAge(-1); } + if (cookieHttpOnly && setHttpOnlyMethod != null) { + ReflectionUtils.invokeMethod(setHttpOnlyMethod, cookie, Boolean.TRUE); + } + response.addCookie(cookie); } @@ -94,7 +111,7 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { } /** - * Sets the name of the HTTP header that should be used to provide the token + * Sets the name of the HTTP header that should be used to provide the token. * * @param headerName the name of the HTTP header that should be used to provide the * token @@ -105,7 +122,7 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { } /** - * Sets the name of the cookie that the expected CSRF token is saved to and read from + * Sets the name of the cookie that the expected CSRF token is saved to and read from. * * @param cookieName the name of the cookie that the expected CSRF token is saved to * and read from @@ -115,6 +132,22 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { this.cookieName = cookieName; } + /** + * Sets the HttpOnly attribute on the cookie containing the CSRF token. + * The cookie will only be marked as HttpOnly if both cookieHttpOnly is true and the underlying version of Servlet is 3.0 or greater. + * Defaults to true if the underlying version of Servlet is 3.0 or greater. + * NOTE: The {@link Cookie#setHttpOnly(boolean)} was introduced in Servlet 3.0. + * + * @param cookieHttpOnly true sets the HttpOnly attribute, false does not set it (depending on Servlet version) + * @throws IllegalArgumentException if cookieHttpOnly is true and the underlying version of Servlet is less than 3.0 + */ + public void setCookieHttpOnly(boolean cookieHttpOnly) { + if (cookieHttpOnly && setHttpOnlyMethod == null) { + throw new IllegalArgumentException("Cookie will not be marked as HttpOnly because you are using a version of Servlet less than 3.0. NOTE: The Cookie#setHttpOnly(boolean) was introduced in Servlet 3.0."); + } + this.cookieHttpOnly = cookieHttpOnly; + } + private String getCookiePath(HttpServletRequest request) { String contextPath = request.getContextPath(); return contextPath.length() > 0 ? contextPath : "/"; diff --git a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryServlet3Tests.java b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryServlet3Tests.java new file mode 100644 index 0000000000..ea3c58cc29 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryServlet3Tests.java @@ -0,0 +1,92 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.web.csrf; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.springframework.util.ReflectionUtils; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.lang.reflect.Method; + +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.same; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.*; +import static org.powermock.api.mockito.PowerMockito.spy; +import static org.powermock.api.mockito.PowerMockito.*; +import static org.powermock.api.mockito.PowerMockito.when; + +/** + * @author Joe Grandja + * @since 4.1 + */ +@RunWith(PowerMockRunner.class) +@PrepareForTest({ReflectionUtils.class, Method.class}) +public class CookieCsrfTokenRepositoryServlet3Tests { + + @Mock + private Method method; + + @Test + public void httpOnlyServlet30() throws Exception { + spy(ReflectionUtils.class); + when(ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", + boolean.class)).thenReturn(method); + + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getContextPath()).thenReturn("/contextpath"); + HttpServletResponse response = mock(HttpServletResponse.class); + ArgumentCaptor cookie = ArgumentCaptor.forClass(Cookie.class); + + CookieCsrfTokenRepository repository = new CookieCsrfTokenRepository(); + + CsrfToken token = repository.generateToken(request); + repository.saveToken(token, request, response); + + verify(response).addCookie(cookie.capture()); + verifyStatic(); + ReflectionUtils.invokeMethod(same(method), eq(cookie.getValue()), eq(true)); + } + + @Test + public void httpOnlyPreServlet30() throws Exception { + spy(ReflectionUtils.class); + when(ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", + boolean.class)).thenReturn(null); + + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getContextPath()).thenReturn("/contextpath"); + HttpServletResponse response = mock(HttpServletResponse.class); + ArgumentCaptor cookie = ArgumentCaptor.forClass(Cookie.class); + + CookieCsrfTokenRepository repository = new CookieCsrfTokenRepository(); + + CsrfToken token = repository.generateToken(request); + repository.saveToken(token, request, response); + + verify(response).addCookie(cookie.capture()); + verifyStatic(never()); + ReflectionUtils.invokeMethod(same(method), eq(cookie.getValue()), eq(true)); + } + +} \ No newline at end of file diff --git a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java index c0d3ec8e98..9ffad55485 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java @@ -16,14 +16,13 @@ package org.springframework.security.web.csrf; -import javax.servlet.http.Cookie; - import org.junit.Before; import org.junit.Test; - import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import javax.servlet.http.Cookie; + import static org.assertj.core.api.Assertions.assertThat; /** @@ -84,6 +83,7 @@ public class CookieCsrfTokenRepositoryTests { assertThat(tokenCookie.getPath()).isEqualTo(this.request.getContextPath()); assertThat(tokenCookie.getSecure()).isEqualTo(this.request.isSecure()); assertThat(tokenCookie.getValue()).isEqualTo(token.getToken()); + assertThat(tokenCookie.isHttpOnly()).isEqualTo(true); } @Test @@ -114,6 +114,30 @@ public class CookieCsrfTokenRepositoryTests { assertThat(tokenCookie.getValue()).isEmpty(); } + @Test + public void saveTokenHttpOnlyTrue() { + this.repository.setCookieHttpOnly(true); + CsrfToken token = this.repository.generateToken(this.request); + this.repository.saveToken(token, this.request, this.response); + + Cookie tokenCookie = this.response + .getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); + + assertThat(tokenCookie.isHttpOnly()).isTrue(); + } + + @Test + public void saveTokenHttpOnlyFalse() { + this.repository.setCookieHttpOnly(false); + CsrfToken token = this.repository.generateToken(this.request); + this.repository.saveToken(token, this.request, this.response); + + Cookie tokenCookie = this.response + .getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); + + assertThat(tokenCookie.isHttpOnly()).isFalse(); + } + @Test public void loadTokenNoCookiesNull() { assertThat(this.repository.loadToken(this.request)).isNull();