diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java index b243a5a37e1..6598236656f 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java @@ -16,7 +16,6 @@ package org.springframework.security.web.authentication.rememberme; import java.io.UnsupportedEncodingException; -import java.lang.reflect.Method; import java.util.Base64; import java.net.URLDecoder; import java.net.URLEncoder; @@ -46,7 +45,6 @@ import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; import org.springframework.security.web.authentication.logout.LogoutHandler; import org.springframework.util.Assert; -import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -86,7 +84,6 @@ public abstract class AbstractRememberMeServices implements RememberMeServices, private String key; private int tokenValiditySeconds = TWO_WEEKS_S; private Boolean useSecureCookie = null; - private Method setHttpOnlyMethod; private GrantedAuthoritiesMapper authoritiesMapper = new NullAuthoritiesMapper(); protected AbstractRememberMeServices(String key, UserDetailsService userDetailsService) { @@ -94,8 +91,6 @@ protected AbstractRememberMeServices(String key, UserDetailsService userDetailsS Assert.notNull(userDetailsService, "UserDetailsService cannot be null"); this.key = key; this.userDetailsService = userDetailsService; - this.setHttpOnlyMethod = ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", - boolean.class); } @Override @@ -396,8 +391,8 @@ protected void cancelCookie(HttpServletRequest request, HttpServletResponse resp * * By default a secure cookie will be used if the connection is secure. You can set * the {@code useSecureCookie} property to {@code false} to override this. If you set - * it to {@code true}, the cookie will always be flagged as secure. If Servlet 3.0 is - * used, the cookie will be marked as HttpOnly. + * it to {@code true}, the cookie will always be flagged as secure. By default the cookie + * will be marked as HttpOnly. * * @param tokens the tokens which will be encoded to make the cookie value. * @param maxAge the value passed to {@link Cookie#setMaxAge(int)} @@ -424,12 +419,7 @@ protected void setCookie(String[] tokens, int maxAge, HttpServletRequest request cookie.setSecure(useSecureCookie); } - if (setHttpOnlyMethod != null) { - ReflectionUtils.invokeMethod(setHttpOnlyMethod, cookie, Boolean.TRUE); - } - else if (logger.isDebugEnabled()) { - logger.debug("Note: Cookie will not be marked as HttpOnly because you are not using Servlet 3.0 (Cookie#setHttpOnly(boolean) was not found)."); - } + cookie.setHttpOnly(true); response.addCookie(cookie); } 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 4e692a3f207..73ca49017ff 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,7 +16,6 @@ package org.springframework.security.web.csrf; -import java.lang.reflect.Method; import java.util.UUID; import javax.servlet.http.Cookie; @@ -24,7 +23,6 @@ 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; @@ -49,19 +47,13 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { private String cookieName = DEFAULT_CSRF_COOKIE_NAME; - private final Method setHttpOnlyMethod; - - private boolean cookieHttpOnly; + private boolean cookieHttpOnly = true; private String cookiePath; private String cookieDomain; public CookieCsrfTokenRepository() { - this.setHttpOnlyMethod = ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", boolean.class); - if (this.setHttpOnlyMethod != null) { - this.cookieHttpOnly = true; - } } @Override @@ -87,9 +79,7 @@ public void saveToken(CsrfToken token, HttpServletRequest request, else { cookie.setMaxAge(-1); } - if (cookieHttpOnly && setHttpOnlyMethod != null) { - ReflectionUtils.invokeMethod(setHttpOnlyMethod, cookie, Boolean.TRUE); - } + cookie.setHttpOnly(cookieHttpOnly); if (this.cookieDomain != null && !this.cookieDomain.isEmpty()) { cookie.setDomain(this.cookieDomain); } @@ -145,17 +135,11 @@ public void setCookieName(String 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. + * Defaults to true. * - * @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 + * @param cookieHttpOnly true sets the HttpOnly attribute, false does not set it */ 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; } diff --git a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesServlet3Tests.java b/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesServlet3Tests.java deleted file mode 100644 index b1186fd4a99..00000000000 --- a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesServlet3Tests.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Copyright 2002-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.authentication.rememberme; - -import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.same; -import static org.mockito.Mockito.verify; -import static org.powermock.api.mockito.PowerMockito.mock; -import static org.powermock.api.mockito.PowerMockito.spy; -import static org.powermock.api.mockito.PowerMockito.verifyStatic; -import static org.powermock.api.mockito.PowerMockito.when; - -import java.lang.reflect.Method; - -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.junit.Before; -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.security.core.userdetails.UserDetailsService; -import org.springframework.security.web.authentication.rememberme.AbstractRememberMeServicesTests.MockRememberMeServices; -import org.springframework.util.ReflectionUtils; - -/** - * - * @author Rob Winch - */ -@RunWith(PowerMockRunner.class) -@PrepareForTest({ Method.class, ReflectionUtils.class }) -public class AbstractRememberMeServicesServlet3Tests { - @Mock - private Method method; - - @Before - public void setUp() throws Exception { - spy(ReflectionUtils.class); - - when(ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", boolean.class)) - .thenReturn(method); - } - - @Test - public void httpOnlySetInServlet30DefaultConstructor() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); - when(request.getContextPath()).thenReturn("/contextpath"); - HttpServletResponse response = mock(HttpServletResponse.class); - ArgumentCaptor cookie = ArgumentCaptor.forClass(Cookie.class); - MockRememberMeServices services = new MockRememberMeServices(); - services.setCookie(new String[] { "mycookie" }, 1000, request, response); - verify(response).addCookie(cookie.capture()); - verifyStatic(ReflectionUtils.class); - ReflectionUtils.invokeMethod(same(method), eq(cookie.getValue()), eq(true)); - } - - @Test - public void httpOnlySetInServlet30() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); - when(request.getContextPath()).thenReturn("/contextpath"); - HttpServletResponse response = mock(HttpServletResponse.class); - ArgumentCaptor cookie = ArgumentCaptor.forClass(Cookie.class); - MockRememberMeServices services = new MockRememberMeServices("key", - mock(UserDetailsService.class)); - services.setCookie(new String[] { "mycookie" }, 1000, request, response); - verify(response).addCookie(cookie.capture()); - verifyStatic(ReflectionUtils.class); - ReflectionUtils.invokeMethod(same(method), eq(cookie.getValue()), eq(true)); - } -} diff --git a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java b/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java index 62aa3796089..ae569f2e494 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java @@ -16,8 +16,6 @@ package org.springframework.security.web.authentication.rememberme; import static org.assertj.core.api.Assertions.assertThat; -import static org.powermock.api.mockito.PowerMockito.spy; -import static org.powermock.api.mockito.PowerMockito.when; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; @@ -41,7 +39,6 @@ import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.core.userdetails.UsernameNotFoundException; -import org.springframework.test.util.ReflectionTestUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; @@ -369,17 +366,16 @@ protected String encodeCookie(String[] cookieTokens) { } @Test - public void setHttpOnlyIgnoredForServlet25() throws Exception { - spy(ReflectionUtils.class); - when(ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", - boolean.class)).thenReturn(null); + public void setCookieSetsIsHttpOnlyFlagByDefault() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletResponse response = new MockHttpServletResponse(); + request.setContextPath("contextpath"); MockRememberMeServices services = new MockRememberMeServices(uds); - assertThat(ReflectionTestUtils.getField(services, "setHttpOnlyMethod")).isNull(); - - services = new MockRememberMeServices("key", - new MockUserDetailsService(joe, false)); - assertThat(ReflectionTestUtils.getField(services, "setHttpOnlyMethod")).isNull(); + services.setCookie(new String[] { "mycookie" }, 1000, request, response); + Cookie cookie = response.getCookie( + AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + assertThat(cookie.isHttpOnly()).isTrue(); } // SEC-2791 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 deleted file mode 100644 index 6503a63c06b..00000000000 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryServlet3Tests.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * 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 java.lang.reflect.Method; - -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -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 static org.mockito.Matchers.eq; -import static org.mockito.Matchers.same; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.powermock.api.mockito.PowerMockito.spy; -import static org.powermock.api.mockito.PowerMockito.verifyStatic; -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(this.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.class); - ReflectionUtils.invokeMethod(same(this.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(ReflectionUtils.class, never()); - ReflectionUtils.invokeMethod(same(this.method), eq(cookie.getValue()), eq(true)); - } - -}