فهرست منبع

Remove Servlet 2.5 and 3.0 Support for Remember Me and CSRF

Fixes: gh-6263, Fixes: gh-6262
Dongmin Shin 6 سال پیش
والد
کامیت
fc802e1a7c

+ 3 - 13
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.RememberMeServices;
 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 @@ public abstract class AbstractRememberMeServices implements RememberMeServices,
 		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 @@ public abstract class AbstractRememberMeServices implements RememberMeServices,
 	 *
 	 * 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 @@ public abstract class AbstractRememberMeServices implements RememberMeServices,
 			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);
 	}

+ 4 - 20
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.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;
 
@@ -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 final class CookieCsrfTokenRepository implements CsrfTokenRepository {
 		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 final class CookieCsrfTokenRepository implements CsrfTokenRepository {
 
 	/**
 	 * Sets the HttpOnly attribute on the cookie containing the CSRF token.
-	 * The cookie will only be marked as HttpOnly if both <code>cookieHttpOnly</code> is <code>true</code> and the underlying version of Servlet is 3.0 or greater.
-	 * Defaults to <code>true</code> 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 <code>true</code>.
 	 *
-	 * @param cookieHttpOnly <code>true</code> sets the HttpOnly attribute, <code>false</code> does not set it (depending on Servlet version)
-	 * @throws IllegalArgumentException if <code>cookieHttpOnly</code> is <code>true</code> and the underlying version of Servlet is less than 3.0
+	 * @param cookieHttpOnly <code>true</code> sets the HttpOnly attribute, <code>false</code> 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;
 	}
 

+ 0 - 87
web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesServlet3Tests.java

@@ -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> 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> 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));
-	}
-}

+ 8 - 12
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.User;
 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 @@ public class AbstractRememberMeServicesTests {
 	}
 
 	@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

+ 0 - 95
web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryServlet3Tests.java

@@ -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> 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> 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));
-	}
-
-}