From 927840fe88052a40c72fe80a16372f594880e02b Mon Sep 17 00:00:00 2001 From: Joaquin Santana Date: Mon, 13 May 2024 17:09:34 +0200 Subject: [PATCH] Do Not Invalidate Current Session When It Is Registered Closes gh-15066 --- ...dServerMaximumSessionsExceededHandler.java | 3 ++ ...erMaximumSessionsExceededHandlerTests.java | 46 +++++++++++++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/server/authentication/InvalidateLeastUsedServerMaximumSessionsExceededHandler.java b/web/src/main/java/org/springframework/security/web/server/authentication/InvalidateLeastUsedServerMaximumSessionsExceededHandler.java index 28446df993..5415686f69 100644 --- a/web/src/main/java/org/springframework/security/web/server/authentication/InvalidateLeastUsedServerMaximumSessionsExceededHandler.java +++ b/web/src/main/java/org/springframework/security/web/server/authentication/InvalidateLeastUsedServerMaximumSessionsExceededHandler.java @@ -24,6 +24,7 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.security.core.session.ReactiveSessionInformation; +import org.springframework.util.Assert; import org.springframework.web.server.session.WebSessionStore; /** @@ -42,12 +43,14 @@ public final class InvalidateLeastUsedServerMaximumSessionsExceededHandler private final WebSessionStore webSessionStore; public InvalidateLeastUsedServerMaximumSessionsExceededHandler(WebSessionStore webSessionStore) { + Assert.notNull(webSessionStore, "webSessionStore cannot be null"); this.webSessionStore = webSessionStore; } @Override public Mono handle(MaximumSessionsContext context) { List sessions = new ArrayList<>(context.getSessions()); + sessions.removeIf((session) -> session.getSessionId().equals(context.getCurrentSession().getId())); sessions.sort(Comparator.comparing(ReactiveSessionInformation::getLastAccessTime)); int maximumSessionsExceededBy = sessions.size() - context.getMaximumSessionsAllowed() + 1; List leastRecentlyUsedSessionsToInvalidate = sessions.subList(0, diff --git a/web/src/test/java/org/springframework/security/web/server/authentication/session/InvalidateLeastUsedServerMaximumSessionsExceededHandlerTests.java b/web/src/test/java/org/springframework/security/web/server/authentication/session/InvalidateLeastUsedServerMaximumSessionsExceededHandlerTests.java index eb1db969c3..81a55b141b 100644 --- a/web/src/test/java/org/springframework/security/web/server/authentication/session/InvalidateLeastUsedServerMaximumSessionsExceededHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/server/authentication/session/InvalidateLeastUsedServerMaximumSessionsExceededHandlerTests.java @@ -23,6 +23,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; +import org.springframework.mock.web.server.MockWebSession; import org.springframework.security.core.Authentication; import org.springframework.security.core.session.ReactiveSessionInformation; import org.springframework.security.web.server.authentication.InvalidateLeastUsedServerMaximumSessionsExceededHandler; @@ -34,6 +35,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -59,18 +61,21 @@ class InvalidateLeastUsedServerMaximumSessionsExceededHandlerTests { ReactiveSessionInformation session2 = mock(ReactiveSessionInformation.class); given(session1.getLastAccessTime()).willReturn(Instant.ofEpochMilli(1700827760010L)); given(session2.getLastAccessTime()).willReturn(Instant.ofEpochMilli(1700827760000L)); + given(session1.getSessionId()).willReturn("session1"); given(session2.getSessionId()).willReturn("session2"); given(session2.invalidate()).willReturn(Mono.empty()); MaximumSessionsContext context = new MaximumSessionsContext(mock(Authentication.class), - List.of(session1, session2), 2, null); + List.of(session1, session2), 2, createWebSession()); this.handler.handle(context).block(); verify(session2).invalidate(); verify(session1).getLastAccessTime(); // used by comparator to sort the sessions verify(session2).getLastAccessTime(); // used by comparator to sort the sessions - verify(session2).getSessionId(); // used to invalidate session against the + verify(session1).getSessionId(); + verify(session2, times(2)).getSessionId(); // used to invalidate session against + // the // WebSessionStore verify(this.webSessionStore).removeSession("session2"); verifyNoMoreInteractions(this.webSessionStore); @@ -90,16 +95,18 @@ class InvalidateLeastUsedServerMaximumSessionsExceededHandlerTests { given(session2.invalidate()).willReturn(Mono.empty()); given(session1.getSessionId()).willReturn("session1"); given(session2.getSessionId()).willReturn("session2"); + given(session3.getSessionId()).willReturn("session3"); MaximumSessionsContext context = new MaximumSessionsContext(mock(Authentication.class), - List.of(session1, session2, session3), 2, null); + List.of(session1, session2, session3), 2, createWebSession()); this.handler.handle(context).block(); // @formatter:off verify(session1).invalidate(); verify(session2).invalidate(); - verify(session1).getSessionId(); - verify(session2).getSessionId(); + verify(session1, times(2)).getSessionId(); + verify(session2, times(2)).getSessionId(); + verify(session3).getSessionId(); verify(session1, atLeastOnce()).getLastAccessTime(); // used by comparator to sort the sessions verify(session2, atLeastOnce()).getLastAccessTime(); // used by comparator to sort the sessions verify(session3, atLeastOnce()).getLastAccessTime(); // used by comparator to sort the sessions @@ -112,4 +119,33 @@ class InvalidateLeastUsedServerMaximumSessionsExceededHandlerTests { // @formatter:on } + @Test + void handleWhenCurrentSessionIsRegisteredThenDoNotInvalidateCurrentSession() { + ReactiveSessionInformation session1 = mock(ReactiveSessionInformation.class); + ReactiveSessionInformation session2 = mock(ReactiveSessionInformation.class); + MockWebSession currentSession = createWebSession(); + given(session1.getLastAccessTime()).willReturn(Instant.ofEpochMilli(1700827760010L)); + given(session2.getLastAccessTime()).willReturn(Instant.ofEpochMilli(1700827760000L)); + given(session1.getSessionId()).willReturn("session1"); + given(session2.getSessionId()).willReturn(currentSession.getId()); + given(session1.invalidate()).willReturn(Mono.empty()); + + MaximumSessionsContext context = new MaximumSessionsContext(mock(Authentication.class), + List.of(session1, session2), 1, currentSession); + + this.handler.handle(context).block(); + + verify(session1).invalidate(); + verify(session2).getSessionId(); + verify(session1, times(2)).getSessionId(); + verify(this.webSessionStore).removeSession("session1"); + verifyNoMoreInteractions(this.webSessionStore); + verifyNoMoreInteractions(session2); + verifyNoMoreInteractions(session1); + } + + private MockWebSession createWebSession() { + return new MockWebSession(); + } + }