From 915d68e21614b662e8bef16d3eff3f7f31d79618 Mon Sep 17 00:00:00 2001 From: Marcus Hert Da Coregio Date: Tue, 6 Feb 2024 10:34:37 -0300 Subject: [PATCH] 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 --- .../web/server/SessionManagementSpecTests.java | 4 ++-- .../session/InMemoryReactiveSessionRegistry.java | 7 +++---- .../core/session/ReactiveSessionRegistry.java | 4 ++-- ...nControlServerAuthenticationSuccessHandler.java | 4 ++-- .../WebSessionStoreReactiveSessionRegistry.java | 6 +++--- ...rolServerAuthenticationSuccessHandlerTests.java | 12 +++++------- .../InMemoryReactiveSessionRegistryTests.java | 9 ++++----- ...ebSessionStoreReactiveSessionRegistryTests.java | 14 ++++++-------- 8 files changed, 27 insertions(+), 33 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/web/server/SessionManagementSpecTests.java b/config/src/test/java/org/springframework/security/config/web/server/SessionManagementSpecTests.java index ca2fcf9f80..089a3916d4 100644 --- a/config/src/test/java/org/springframework/security/config/web/server/SessionManagementSpecTests.java +++ b/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() diff --git a/core/src/main/java/org/springframework/security/core/session/InMemoryReactiveSessionRegistry.java b/core/src/main/java/org/springframework/security/core/session/InMemoryReactiveSessionRegistry.java index 046a039505..8e0546568c 100644 --- a/core/src/main/java/org/springframework/security/core/session/InMemoryReactiveSessionRegistry.java +++ b/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 getAllSessions(Object principal, boolean includeExpiredSessions) { + public Flux 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 diff --git a/core/src/main/java/org/springframework/security/core/session/ReactiveSessionRegistry.java b/core/src/main/java/org/springframework/security/core/session/ReactiveSessionRegistry.java index b5da453164..60e4a02cc2 100644 --- a/core/src/main/java/org/springframework/security/core/session/ReactiveSessionRegistry.java +++ b/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 getAllSessions(Object principal, boolean includeExpiredSessions); + Flux getAllSessions(Object principal); /** * Saves the {@link ReactiveSessionInformation} diff --git a/web/src/main/java/org/springframework/security/web/server/authentication/ConcurrentSessionControlServerAuthenticationSuccessHandler.java b/web/src/main/java/org/springframework/security/web/server/authentication/ConcurrentSessionControlServerAuthenticationSuccessHandler.java index d65951719f..ca777da888 100644 --- a/web/src/main/java/org/springframework/security/web/server/authentication/ConcurrentSessionControlServerAuthenticationSuccessHandler.java +++ b/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 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() diff --git a/web/src/main/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistry.java b/web/src/main/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistry.java index 5d8cd5cc4d..781c438a6e 100644 --- a/web/src/main/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistry.java +++ b/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 getAllSessions(Object principal, boolean includeExpiredSessions) { - return this.sessionRegistry.getAllSessions(principal, includeExpiredSessions).map(WebSessionInformation::new); + public Flux getAllSessions(Object principal) { + return this.sessionRegistry.getAllSessions(principal).map(WebSessionInformation::new); } @Override diff --git a/web/src/test/java/org/springframework/security/web/server/authentication/session/ConcurrentSessionControlServerAuthenticationSuccessHandlerTests.java b/web/src/test/java/org/springframework/security/web/server/authentication/session/ConcurrentSessionControlServerAuthenticationSuccessHandlerTests.java index 88c97e2e22..643b3015fc 100644 --- a/web/src/test/java/org/springframework/security/web/server/authentication/session/ConcurrentSessionControlServerAuthenticationSuccessHandlerTests.java +++ b/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 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 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 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(); diff --git a/web/src/test/java/org/springframework/security/web/server/authentication/session/InMemoryReactiveSessionRegistryTests.java b/web/src/test/java/org/springframework/security/web/server/authentication/session/InMemoryReactiveSessionRegistryTests.java index ceca216a21..630d272b83 100644 --- a/web/src/test/java/org/springframework/security/web/server/authentication/session/InMemoryReactiveSessionRegistryTests.java +++ b/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 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 sessions = this.sessionRegistry - .getAllSessions(authentication.getPrincipal(), false) + List 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 sessions = this.sessionRegistry.getAllSessions(authentication.getName(), false) + List sessions = this.sessionRegistry.getAllSessions(authentication.getName()) .collectList() .block(); assertThat(this.sessionRegistry.getSessionInformation("1234").block()).isNull(); diff --git a/web/src/test/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistryTests.java b/web/src/test/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistryTests.java index e0430bdfcf..83dc8d5a50 100644 --- a/web/src/test/java/org/springframework/security/web/session/WebSessionStoreReactiveSessionRegistryTests.java +++ b/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 saved = this.registry.getAllSessions(session.getPrincipal(), false) + List 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() {