Skip to content

Commit

Permalink
Remove includeExpiredSessions parameter
Browse files Browse the repository at this point in the history
The reactive implementation of max sessions does not keep track of expired sessions, therefore we do not need such parameter

Issue gh-6192
  • Loading branch information
marcusdacoregio committed Feb 6, 2024
1 parent 6068e6b commit 915d68e
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -334,7 +334,7 @@ void loginWhenUnlimitedSessionsButSessionsInvalidatedManuallyThenInvalidates() {
.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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -50,10 +50,9 @@ public InMemoryReactiveSessionRegistry(ConcurrentMap<Object, Set<String>> sessio
}

@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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -62,7 +62,7 @@ public Mono<Void> onAuthenticationSuccess(WebFilterExchange exchange, Authentica

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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -46,8 +46,8 @@ public WebSessionStoreReactiveSessionRegistry(WebSessionStore webSessionStore) {
}

@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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -111,7 +111,7 @@ void onAuthenticationWhenMaximumSessionsIsOneAndExceededThenHandlerIsCalled() {
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());
Expand All @@ -127,7 +127,7 @@ void onAuthenticationWhenMaximumSessionsIsGreaterThanOneAndExceededThenHandlerIs
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());
Expand All @@ -151,10 +151,8 @@ void onAuthenticationWhenMaximumSessionsForUsersAreDifferentThenHandlerIsCalledW
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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -46,7 +46,7 @@ void saveWhenPrincipalThenRegisterPrincipalSession() {
"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);
Expand All @@ -65,8 +65,7 @@ void getAllSessionsWhenMultipleSessionsThenReturnAll() {
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);
Expand All @@ -82,7 +81,7 @@ void removeSessionInformationThenSessionIsRemoved() {
"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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -31,8 +31,6 @@
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;
Expand Down Expand Up @@ -101,12 +99,12 @@ void invalidateWhenReturnedFromGetAllSessionsThenWebSessionInvalidatedAndRemoved
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
Expand All @@ -116,7 +114,7 @@ void setSessionRegistryThenUses() {
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();
Expand All @@ -127,8 +125,8 @@ void setSessionRegistryThenUses() {
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() {
Expand Down

0 comments on commit 915d68e

Please sign in to comment.