Expire as many sessions as exceed maximum allowed

Fixes: gh-7166

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -15,6 +15,7 @@
  */
 package org.springframework.security.web.authentication.session;
 
+import java.util.Comparator;
 import java.util.List;
 
 import javax.servlet.http.HttpServletRequest;
@@ -45,8 +46,9 @@ import org.springframework.util.Assert;
  * </p>
  * <p>
  * If a user has reached the maximum number of permitted sessions, the behaviour depends
- * on the <tt>exceptionIfMaxExceeded</tt> property. The default behaviour is to expired
- * the least recently used session, which will be invalidated by the
+ * on the <tt>exceptionIfMaxExceeded</tt> property. The default behaviour is to expire
+ * any sessions that exceed the maximum number of permitted sessions, starting with the
+ * least recently used sessions. The expired sessions will be invalidated by the
  * {@link ConcurrentSessionFilter} if accessed again. If <tt>exceptionIfMaxExceeded</tt>
  * is set to <tt>true</tt>, however, the user will be prevented from starting a new
  * authenticated session.
@@ -156,18 +158,13 @@ public class ConcurrentSessionControlAuthenticationStrategy implements
 					"Maximum sessions of {0} for this principal exceeded"));
 		}
 
-		// Determine least recently used session, and mark it for invalidation
-		SessionInformation leastRecentlyUsed = null;
-
-		for (SessionInformation session : sessions) {
-			if ((leastRecentlyUsed == null)
-					|| session.getLastRequest()
-							.before(leastRecentlyUsed.getLastRequest())) {
-				leastRecentlyUsed = session;
-			}
+		// Determine least recently used sessions, and mark them for invalidation
+		sessions.sort(Comparator.comparing(SessionInformation::getLastRequest));
+		int maximumSessionsExceededBy = sessions.size() - allowableSessions + 1;
+		List<SessionInformation> sessionsToBeExpired = sessions.subList(0, maximumSessionsExceededBy);
+		for (SessionInformation session: sessionsToBeExpired) {
+			session.expireNow();
 		}
-
-		leastRecentlyUsed.expireNow();
 	}
 
 	/**

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -134,6 +134,25 @@ public class ConcurrentSessionControlAuthenticationStrategyTests {
 		assertThat(sessionInformation.isExpired()).isTrue();
 	}
 
+	@Test
+	public void onAuthenticationWhenMaxSessionsExceededByTwoThenTwoSessionsExpired() {
+		SessionInformation oldestSessionInfo = new SessionInformation(
+				authentication.getPrincipal(), "unique1", new Date(1374766134214L));
+		SessionInformation secondOldestSessionInfo = new SessionInformation(
+				authentication.getPrincipal(), "unique2", new Date(1374766134215L));
+		when(sessionRegistry.getAllSessions(any(), anyBoolean())).thenReturn(
+				Arrays.<SessionInformation> asList(oldestSessionInfo,
+						secondOldestSessionInfo,
+						sessionInformation));
+		strategy.setMaximumSessions(2);
+
+		strategy.onAuthentication(authentication, request, response);
+
+		assertThat(oldestSessionInfo.isExpired()).isTrue();
+		assertThat(secondOldestSessionInfo.isExpired()).isTrue();
+		assertThat(sessionInformation.isExpired()).isFalse();
+	}
+
 	@Test(expected = IllegalArgumentException.class)
 	public void setMessageSourceNull() {
 		strategy.setMessageSource(null);