Remove Servlet 2.5 Support for Session Fixation

This commit removes existence validation of a method only available in Servlet 3.1.
Spring Framework baseline is Servlet 3.1 so is not longer required.

Fixes: gh-6259

Author: raphaelDL

config/src/main/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java

@@ -212,8 +212,7 @@ public SessionManagementConfigurer<H> sessionCreationPolicy(
 
 	/**
 	 * Allows explicitly specifying the {@link SessionAuthenticationStrategy}.
-	 * The default is to use {@link SessionFixationProtectionStrategy} for Servlet 3.1 or
-	 * {@link ChangeSessionIdAuthenticationStrategy} for Servlet 3.1+.
+	 * The default is to use {@link ChangeSessionIdAuthenticationStrategy}.
 	 * If restricting the maximum number of sessions is configured, then
 	 * {@link CompositeSessionAuthenticationStrategy} delegating to
 	 * {@link ConcurrentSessionControlAuthenticationStrategy},
@@ -305,13 +304,11 @@ public SessionManagementConfigurer<H> migrateSession() {
 
 		/**
 		 * Specifies that the Servlet container-provided session fixation protection
-		 * should be used. When a session authenticates, the Servlet 3.1 method
+		 * should be used. When a session authenticates, the Servlet method
 		 * {@code HttpServletRequest#changeSessionId()} is called to change the session ID
-		 * and retain all session attributes. Using this option in a Servlet 3.0 or older
-		 * container results in an {@link IllegalStateException}.
+		 * and retain all session attributes.
 		 *
 		 * @return the {@link SessionManagementConfigurer} for further customizations
-		 * @throws IllegalStateException if the container is not Servlet 3.1 or newer.
 		 */
 		public SessionManagementConfigurer<H> changeSessionId() {
 			setSessionFixationAuthenticationStrategy(
@@ -664,11 +661,6 @@ private boolean isConcurrentSessionControlEnabled() {
 	 * @return the default {@link SessionAuthenticationStrategy} for session fixation
 	 */
 	private static SessionAuthenticationStrategy createDefaultSessionFixationProtectionStrategy() {
-		try {
 			return new ChangeSessionIdAuthenticationStrategy();
-		}
-		catch (IllegalStateException e) {
-			return new SessionFixationProtectionStrategy();
-		}
 	}
 }

config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java

@@ -15,12 +15,10 @@
  */
 package org.springframework.security.config.http;
 
-import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
 
 import javax.servlet.ServletRequest;
-import javax.servlet.http.HttpServletRequest;
 
 import org.w3c.dom.Element;
 
@@ -73,7 +71,6 @@
 import org.springframework.security.web.session.SimpleRedirectSessionInformationExpiredStrategy;
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.util.ClassUtils;
-import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 
@@ -350,10 +347,7 @@ private void createSessionManagementFilters() {
 		}
 
 		if (!StringUtils.hasText(sessionFixationAttribute)) {
-			Method changeSessionIdMethod = ReflectionUtils.findMethod(
-					HttpServletRequest.class, "changeSessionId");
-			sessionFixationAttribute = changeSessionIdMethod == null ? OPT_SESSION_FIXATION_MIGRATE_SESSION
-					: OPT_CHANGE_SESSION_ID;
+			sessionFixationAttribute =  OPT_CHANGE_SESSION_ID;
 		}
 		else if (StringUtils.hasText(sessionAuthStratRef)) {
 			pc.getReaderContext().error(

config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerServlet31Tests.java

@@ -17,15 +17,13 @@
 
 import java.lang.reflect.Method;
 import javax.servlet.Filter;
-import javax.servlet.http.HttpServletRequest;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.powermock.core.classloader.annotations.PowerMockIgnore;
-import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
 import org.springframework.context.ConfigurableApplicationContext;
@@ -44,21 +42,14 @@
 import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
 import org.springframework.security.web.csrf.CsrfToken;
 import org.springframework.security.web.csrf.HttpSessionCsrfTokenRepository;
-import org.springframework.util.ReflectionUtils;
 
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.same;
-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 static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 
 /**
  *
  * @author Rob Winch
  */
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({ ReflectionUtils.class, Method.class })
 @PowerMockIgnore({ "org.w3c.dom.*", "org.xml.sax.*", "org.apache.xerces.*", "javax.xml.parsers.*" })
 public class SessionManagementConfigurerServlet31Tests {
 	@Mock
@@ -87,10 +78,9 @@ public void teardown() {
 	}
 
 	@Test
-	public void changeSessionIdDefaultsInServlet31Plus() throws Exception {
-		spy(ReflectionUtils.class);
-		Method method = mock(Method.class);
+	public void changeSessionIdThenPreserveParameters() throws Exception {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		String id =  request.getSession().getId();
 		request.getSession();
 		request.setServletPath("/login");
 		request.setMethod("POST");
@@ -100,15 +90,14 @@ public void changeSessionIdDefaultsInServlet31Plus() throws Exception {
 		CsrfToken token = repository.generateToken(request);
 		repository.saveToken(token, request, response);
 		request.setParameter(token.getParameterName(), token.getToken());
-		when(ReflectionUtils.findMethod(HttpServletRequest.class, "changeSessionId"))
-				.thenReturn(method);
+		request.getSession().setAttribute("attribute1", "value1");
 
 		loadConfig(SessionManagementDefaultSessionFixationServlet31Config.class);
 
 		springSecurityFilterChain.doFilter(request, response, chain);
 
-		verifyStatic(ReflectionUtils.class);
-		ReflectionUtils.invokeMethod(same(method), any(HttpServletRequest.class));
+		assertThat(!request.getSession().getId().equals(id));
+		assertThat(request.getSession().getAttribute("attribute1").equals("value1"));
 	}
 
 	@EnableWebSecurity

config/src/test/java/org/springframework/security/config/http/SessionManagementConfigServlet31Tests.java

@@ -17,7 +17,6 @@
 
 import java.lang.reflect.Method;
 import javax.servlet.Filter;
-import javax.servlet.http.HttpServletRequest;
 
 import org.junit.After;
 import org.junit.Before;
@@ -39,12 +38,7 @@
 import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
 import org.springframework.util.ReflectionUtils;
 
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.same;
-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 static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 
 /**
  * @author Rob Winch
@@ -86,40 +80,40 @@ public void teardown() {
 	}
 
 	@Test
-	public void changeSessionIdDefaultsInServlet31Plus() throws Exception {
-		spy(ReflectionUtils.class);
-		Method method = mock(Method.class);
+	public void changeSessionIdThenPreserveParameters() throws Exception {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
 		request.getSession();
 		request.setServletPath("/login");
 		request.setMethod("POST");
 		request.setParameter("username", "user");
 		request.setParameter("password", "password");
-		when(ReflectionUtils.findMethod(HttpServletRequest.class, "changeSessionId"))
-				.thenReturn(method);
+
+		request.getSession().setAttribute("attribute1", "value1");
+
+		String id = request.getSession().getId();
 
 		loadContext("<http>\n" + "        <form-login/>\n"
 				+ "        <session-management/>\n" + "        <csrf disabled='true'/>\n"
 				+ "    </http>" + XML_AUTHENTICATION_MANAGER);
 
 		springSecurityFilterChain.doFilter(request, response, chain);
 
-		verifyStatic(ReflectionUtils.class);
-		ReflectionUtils.invokeMethod(same(method), any(HttpServletRequest.class));
+
+		assertThat(!request.getSession().getId().equals(id));
+		assertThat(request.getSession().getAttribute("attribute1").equals("value1"));
 	}
 
 	@Test
 	public void changeSessionId() throws Exception {
-		spy(ReflectionUtils.class);
-		Method method = mock(Method.class);
+
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
 		request.getSession();
 		request.setServletPath("/login");
 		request.setMethod("POST");
 		request.setParameter("username", "user");
 		request.setParameter("password", "password");
-		when(ReflectionUtils.findMethod(HttpServletRequest.class, "changeSessionId"))
-				.thenReturn(method);
+
+		String id = request.getSession().getId();
 
 		loadContext("<http>\n"
 				+ "        <form-login/>\n"
@@ -129,8 +123,8 @@ public void changeSessionId() throws Exception {
 
 		springSecurityFilterChain.doFilter(request, response, chain);
 
-		verifyStatic(ReflectionUtils.class);
-		ReflectionUtils.invokeMethod(same(method), any(HttpServletRequest.class));
+		assertThat(!request.getSession().getId().equals(id));
+
 	}
 
 	private void loadContext(String context) {

web/src/main/java/org/springframework/security/web/authentication/session/ChangeSessionIdAuthenticationStrategy.java

@@ -15,33 +15,18 @@
  */
 package org.springframework.security.web.authentication.session;
 
-import java.lang.reflect.Method;
-
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
 
-import org.springframework.util.ReflectionUtils;
-
 /**
  * Uses {@code HttpServletRequest.changeSessionId()} to protect against session fixation
- * attacks. This is the default implementation for Servlet 3.1+.
+ * attacks. This is the default implementation.
  *
  * @author Rob Winch
  * @since 3.2
  */
 public final class ChangeSessionIdAuthenticationStrategy
 		extends AbstractSessionFixationProtectionStrategy {
-	private final Method changeSessionIdMethod;
-
-	public ChangeSessionIdAuthenticationStrategy() {
-		Method changeSessionIdMethod = ReflectionUtils
-				.findMethod(HttpServletRequest.class, "changeSessionId");
-		if (changeSessionIdMethod == null) {
-			throw new IllegalStateException(
-					"HttpServletRequest.changeSessionId is undefined. Are you using a Servlet 3.1+ environment?");
-		}
-		this.changeSessionIdMethod = changeSessionIdMethod;
-	}
 
 	/*
 	 * (non-Javadoc)
@@ -52,7 +37,7 @@ public ChangeSessionIdAuthenticationStrategy() {
 	 */
 	@Override
 	HttpSession applySessionFixation(HttpServletRequest request) {
-		ReflectionUtils.invokeMethod(this.changeSessionIdMethod, request);
+		request.changeSessionId();
 		return request.getSession();
 	}
 }

web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java

@@ -24,7 +24,7 @@
 import javax.servlet.http.HttpSession;
 
 /**
- * The default implementation of {@link SessionAuthenticationStrategy} when using <
+ * The implementation of {@link SessionAuthenticationStrategy} when using <
  * Servlet 3.1.
  * <p>
  * Creates a new session for the newly authenticated user if they already have a session

web/src/test/java/org/springframework/security/web/authentication/session/ChangeSessionIdAuthenticationStrategyTests.java

@@ -15,55 +15,26 @@
  */
 package org.springframework.security.web.authentication.session;
 
-import static org.mockito.Matchers.*;
-import static org.powermock.api.mockito.PowerMockito.*;
-
-import java.lang.reflect.Method;
-
-import javax.servlet.http.HttpServletRequest;
-
+import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 import org.springframework.mock.web.MockHttpServletRequest;
-import org.springframework.util.ReflectionUtils;
 
 /**
  * @author Rob Winch
  *
  */
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({ ReflectionUtils.class, Method.class })
 public class ChangeSessionIdAuthenticationStrategyTests {
-	@Mock
-	private Method method;
-
-	@Test(expected = IllegalStateException.class)
-	public void constructChangeIdMethodNotFound() {
-		spy(ReflectionUtils.class);
-		MockHttpServletRequest request = new MockHttpServletRequest();
-		request.getSession();
-		when(ReflectionUtils.findMethod(HttpServletRequest.class, "changeSessionId"))
-				.thenReturn(null);
-
-		new ChangeSessionIdAuthenticationStrategy();
-	}
 
 	@Test
-	public void applySessionFixation() throws Exception {
-		spy(ReflectionUtils.class);
-		Method method = mock(Method.class);
+	public void applySessionFixation() {
 		MockHttpServletRequest request = new MockHttpServletRequest();
-		request.getSession();
-		when(ReflectionUtils.findMethod(HttpServletRequest.class, "changeSessionId"))
-				.thenReturn(method);
+		String id = request.getSession().getId();
 
-		new ChangeSessionIdAuthenticationStrategy().applySessionFixation(request);
+			new ChangeSessionIdAuthenticationStrategy().applySessionFixation(request);
 
-		verifyStatic(ReflectionUtils.class);
-		ReflectionUtils.invokeMethod(same(method), eq(request));
+		Assert.assertNotEquals(id, request.getSession().getId());
 	}
-
 }