Browse Source

Remove includeExpiredSessions parameter

The reactive implementation of max sessions does not keep track of expired sessions, therefore we do not need such parameter

Issue gh-6192
Marcus Hert Da Coregio 1 year ago
parent
commit
915d68e216

+ 2 - 2
config/src/test/java/org/springframework/security/config/web/server/SessionManagementSpecTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -334,7 +334,7 @@ public class SessionManagementSpecTests {
 			.expectStatus()
 			.isOk();
 		ReactiveSessionRegistry sessionRegistry = this.spring.getContext().getBean(ReactiveSessionRegistry.class);
-		sessionRegistry.getAllSessions(PasswordEncodedUser.user(), false)
+		sessionRegistry.getAllSessions(PasswordEncodedUser.user())
 			.flatMap(ReactiveSessionInformation::invalidate)
 			.blockLast();
 		this.client.get()

+ 3 - 4
core/src/main/java/org/springframework/security/core/session/InMemoryReactiveSessionRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -50,10 +50,9 @@ public class InMemoryReactiveSessionRegistry implements ReactiveSessionRegistry
 	}
 
 	@Override
-	public Flux<ReactiveSessionInformation> getAllSessions(Object principal, boolean includeExpiredSessions) {
+	public Flux<ReactiveSessionInformation> getAllSessions(Object principal) {
 		return Flux.fromIterable(this.sessionIdsByPrincipal.getOrDefault(principal, Collections.emptySet()))
-			.map(this.sessionById::get)
-			.filter((sessionInformation) -> includeExpiredSessions || !sessionInformation.isExpired());
+			.map(this.sessionById::get);
 	}
 
 	@Override

+ 2 - 2
core/src/main/java/org/springframework/security/core/session/ReactiveSessionRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -34,7 +34,7 @@ public interface ReactiveSessionRegistry {
 	 * @return the {@link ReactiveSessionInformation} instances associated with the
 	 * principal
 	 */
-	Flux<ReactiveSessionInformation> getAllSessions(Object principal, boolean includeExpiredSessions);
+	Flux<ReactiveSessionInformation> getAllSessions(Object principal);
 
 	/**
 	 * Saves the {@link ReactiveSessionInformation}

+ 2 - 2
web/src/main/java/org/springframework/security/web/server/authentication/ConcurrentSessionControlServerAuthenticationSuccessHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -62,7 +62,7 @@ public final class ConcurrentSessionControlServerAuthenticationSuccessHandler
 
 	private Mono<Void> handleConcurrency(WebFilterExchange exchange, Authentication authentication,
 			Integer maximumSessions) {
-		return this.sessionRegistry.getAllSessions(authentication.getPrincipal(), false)
+		return this.sessionRegistry.getAllSessions(authentication.getPrincipal())
 			.collectList()
 			.flatMap((registeredSessions) -> exchange.getExchange()
 				.getSession()

+ 3 - 3
web/src/main/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -46,8 +46,8 @@ public final class WebSessionStoreReactiveSessionRegistry implements ReactiveSes
 	}
 
 	@Override
-	public Flux<ReactiveSessionInformation> getAllSessions(Object principal, boolean includeExpiredSessions) {
-		return this.sessionRegistry.getAllSessions(principal, includeExpiredSessions).map(WebSessionInformation::new);
+	public Flux<ReactiveSessionInformation> getAllSessions(Object principal) {
+		return this.sessionRegistry.getAllSessions(principal).map(WebSessionInformation::new);
 	}
 
 	@Override

+ 5 - 7
web/src/test/java/org/springframework/security/web/server/authentication/session/ConcurrentSessionControlServerAuthenticationSuccessHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -111,7 +111,7 @@ class ConcurrentSessionControlServerAuthenticationSuccessHandlerTests {
 		Authentication authentication = TestAuthentication.authenticatedUser();
 		List<ReactiveSessionInformation> sessions = Arrays.asList(createSessionInformation("100"),
 				createSessionInformation("101"));
-		given(this.sessionRegistry.getAllSessions(authentication.getPrincipal(), false))
+		given(this.sessionRegistry.getAllSessions(authentication.getPrincipal()))
 			.willReturn(Flux.fromIterable(sessions));
 		this.strategy.onAuthenticationSuccess(new WebFilterExchange(this.exchange, this.chain), authentication).block();
 		verify(this.handler).handle(this.contextCaptor.capture());
@@ -127,7 +127,7 @@ class ConcurrentSessionControlServerAuthenticationSuccessHandlerTests {
 		List<ReactiveSessionInformation> sessions = Arrays.asList(createSessionInformation("100"),
 				createSessionInformation("101"), createSessionInformation("102"), createSessionInformation("103"),
 				createSessionInformation("104"));
-		given(this.sessionRegistry.getAllSessions(authentication.getPrincipal(), false))
+		given(this.sessionRegistry.getAllSessions(authentication.getPrincipal()))
 			.willReturn(Flux.fromIterable(sessions));
 		this.strategy.onAuthenticationSuccess(new WebFilterExchange(this.exchange, this.chain), authentication).block();
 		verify(this.handler).handle(this.contextCaptor.capture());
@@ -151,10 +151,8 @@ class ConcurrentSessionControlServerAuthenticationSuccessHandlerTests {
 		List<ReactiveSessionInformation> adminSessions = Arrays.asList(createSessionInformation("200"),
 				createSessionInformation("201"));
 
-		given(this.sessionRegistry.getAllSessions(user.getPrincipal(), false))
-			.willReturn(Flux.fromIterable(userSessions));
-		given(this.sessionRegistry.getAllSessions(admin.getPrincipal(), false))
-			.willReturn(Flux.fromIterable(adminSessions));
+		given(this.sessionRegistry.getAllSessions(user.getPrincipal())).willReturn(Flux.fromIterable(userSessions));
+		given(this.sessionRegistry.getAllSessions(admin.getPrincipal())).willReturn(Flux.fromIterable(adminSessions));
 
 		this.strategy.onAuthenticationSuccess(new WebFilterExchange(this.exchange, this.chain), user).block();
 		this.strategy.onAuthenticationSuccess(new WebFilterExchange(this.exchange, this.chain), admin).block();

+ 4 - 5
web/src/test/java/org/springframework/security/web/server/authentication/session/InMemoryReactiveSessionRegistryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -46,7 +46,7 @@ class InMemoryReactiveSessionRegistryTests {
 				"1234", this.now);
 		this.sessionRegistry.saveSessionInformation(sessionInformation).block();
 		List<ReactiveSessionInformation> principalSessions = this.sessionRegistry
-			.getAllSessions(authentication.getPrincipal(), false)
+			.getAllSessions(authentication.getPrincipal())
 			.collectList()
 			.block();
 		assertThat(principalSessions).hasSize(1);
@@ -65,8 +65,7 @@ class InMemoryReactiveSessionRegistryTests {
 		this.sessionRegistry.saveSessionInformation(sessionInformation1).block();
 		this.sessionRegistry.saveSessionInformation(sessionInformation2).block();
 		this.sessionRegistry.saveSessionInformation(sessionInformation3).block();
-		List<ReactiveSessionInformation> sessions = this.sessionRegistry
-			.getAllSessions(authentication.getPrincipal(), false)
+		List<ReactiveSessionInformation> sessions = this.sessionRegistry.getAllSessions(authentication.getPrincipal())
 			.collectList()
 			.block();
 		assertThat(sessions).hasSize(3);
@@ -82,7 +81,7 @@ class InMemoryReactiveSessionRegistryTests {
 				"1234", this.now);
 		this.sessionRegistry.saveSessionInformation(sessionInformation).block();
 		this.sessionRegistry.removeSessionInformation("1234").block();
-		List<ReactiveSessionInformation> sessions = this.sessionRegistry.getAllSessions(authentication.getName(), false)
+		List<ReactiveSessionInformation> sessions = this.sessionRegistry.getAllSessions(authentication.getName())
 			.collectList()
 			.block();
 		assertThat(this.sessionRegistry.getSessionInformation("1234").block()).isNull();

+ 6 - 8
web/src/test/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -31,8 +31,6 @@ import org.springframework.web.server.session.WebSessionStore;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -101,12 +99,12 @@ class WebSessionStoreReactiveSessionRegistryTests {
 		given(this.webSessionStore.retrieveSession(session.getSessionId())).willReturn(Mono.just(webSession));
 
 		this.registry.saveSessionInformation(session).block();
-		List<ReactiveSessionInformation> saved = this.registry.getAllSessions(session.getPrincipal(), false)
+		List<ReactiveSessionInformation> saved = this.registry.getAllSessions(session.getPrincipal())
 			.collectList()
 			.block();
 		saved.forEach((info) -> info.invalidate().block());
 		verify(webSession).invalidate();
-		assertThat(this.registry.getAllSessions(session.getPrincipal(), false).collectList().block()).isEmpty();
+		assertThat(this.registry.getAllSessions(session.getPrincipal()).collectList().block()).isEmpty();
 	}
 
 	@Test
@@ -116,7 +114,7 @@ class WebSessionStoreReactiveSessionRegistryTests {
 		given(sessionRegistry.removeSessionInformation(any())).willReturn(Mono.empty());
 		given(sessionRegistry.updateLastAccessTime(any())).willReturn(Mono.empty());
 		given(sessionRegistry.getSessionInformation(any())).willReturn(Mono.empty());
-		given(sessionRegistry.getAllSessions(any(), anyBoolean())).willReturn(Flux.empty());
+		given(sessionRegistry.getAllSessions(any())).willReturn(Flux.empty());
 		this.registry.setSessionRegistry(sessionRegistry);
 		ReactiveSessionInformation session = createSession();
 		this.registry.saveSessionInformation(session).block();
@@ -127,8 +125,8 @@ class WebSessionStoreReactiveSessionRegistryTests {
 		verify(sessionRegistry).updateLastAccessTime(any());
 		this.registry.getSessionInformation(session.getSessionId()).block();
 		verify(sessionRegistry).getSessionInformation(any());
-		this.registry.getAllSessions(session.getPrincipal(), false).blockFirst();
-		verify(sessionRegistry).getAllSessions(any(), eq(false));
+		this.registry.getAllSessions(session.getPrincipal()).blockFirst();
+		verify(sessionRegistry).getAllSessions(any());
 	}
 
 	private static ReactiveSessionInformation createSession() {