Skip to content

Add createdTime field to SessionInformation #17513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add createdTime field to SessionInformation #17513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/src/test/java/org/springframework/security/SerializationSamples.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ final class SerializationSamples {
generatorByClassName.put(OAuth2AuthorizationExchange.class, (r) -> TestOAuth2AuthorizationExchanges.success());
generatorByClassName.put(OidcUserInfo.class, (r) -> OidcUserInfo.builder().email("email@example.com").build());
generatorByClassName.put(SessionInformation.class,
(r) -> new SessionInformation(user, r.alphanumeric(4), new Date(1704378933936L)));
(r) -> new SessionInformation(user, r.alphanumeric(4), new Date(1704378933936L), new Date()));
generatorByClassName.put(ReactiveSessionInformation.class,
(r) -> new ReactiveSessionInformation(user, r.alphanumeric(4), Instant.ofEpochMilli(1704378933936L)));
generatorByClassName.put(OAuth2AccessToken.class, (r) -> TestOAuth2AccessTokens.scopes("scope"));
Expand Down
3 changes: 2 additions & 1 deletion ...framework/security/config/annotation/web/configurers/NamespaceSessionManagementTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ public void authenticateWhenUsingInvalidSessionUrlThenMatchesNamespace() throws
public void authenticateWhenUsingExpiredUrlThenMatchesNamespace() throws Exception {
this.spring.register(CustomSessionManagementConfig.class).autowire();
MockHttpSession session = new MockHttpSession();
SessionInformation sessionInformation = new SessionInformation(new Object(), session.getId(), new Date(0));
SessionInformation sessionInformation = new SessionInformation(new Object(), session.getId(), new Date(0),
new Date());
sessionInformation.expireNow();
SessionRegistry sessionRegistry = this.spring.getContext().getBean(SessionRegistry.class);
given(sessionRegistry.getSessionInformation(session.getId())).willReturn(sessionInformation);
Expand Down
4 changes: 2 additions & 2 deletions .../org/springframework/security/config/annotation/web/session/SessionConcurrencyDslTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class SessionConcurrencyDslTests {
mockkObject(ExpiredUrlConfig.SESSION_REGISTRY)

val session = MockHttpSession()
val sessionInformation = SessionInformation("", session.id, Date(0))
val sessionInformation = SessionInformation("", session.id, Date(0), Date())
sessionInformation.expireNow()
every { ExpiredUrlConfig.SESSION_REGISTRY.getSessionInformation(any()) } returns sessionInformation

Expand Down Expand Up @@ -141,7 +141,7 @@ class SessionConcurrencyDslTests {
mockkObject(ExpiredSessionStrategyConfig.SESSION_REGISTRY)

val session = MockHttpSession()
val sessionInformation = SessionInformation("", session.id, Date(0))
val sessionInformation = SessionInformation("", session.id, Date(0), Date())
sessionInformation.expireNow()
every { ExpiredSessionStrategyConfig.SESSION_REGISTRY.getSessionInformation(any()) } returns sessionInformation

Expand Down
11 changes: 10 additions & 1 deletion core/src/main/java/org/springframework/security/core/session/SessionInformation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
* <code>Filter</code>.
*
* @author Ben Alex
* @author Andrey Litvitski
*/
public class SessionInformation implements Serializable {

Expand All @@ -49,13 +50,17 @@ public class SessionInformation implements Serializable {

private boolean expired = false;

public SessionInformation(Object principal, String sessionId, Date lastRequest) {
private final Date createdTime;

public SessionInformation(Object principal, String sessionId, Date lastRequest, Date createdTime) {
Assert.notNull(principal, "Principal required");
Assert.hasText(sessionId, "SessionId required");
Assert.notNull(lastRequest, "LastRequest required");
Assert.notNull(lastRequest, "CreatedTime required");
this.principal = principal;
this.sessionId = sessionId;
this.lastRequest = lastRequest;
this.createdTime = createdTime;
}
Comment on lines +55 to 64
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that SessionInformation is created in SessionRegistry, so we don't need to maintain backward compatibility, right?


public void expireNow() {
Expand All @@ -74,6 +79,10 @@ public String getSessionId() {
return this.sessionId;
}

public Date getCreatedTime() {
return this.createdTime;
}

public boolean isExpired() {
return this.expired;
}
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ public void registerNewSession(String sessionId, Object principal) {
if (this.logger.isDebugEnabled()) {
this.logger.debug(LogMessage.format("Registering session %s, for principal %s", sessionId, principal));
}
this.sessionIds.put(sessionId, new SessionInformation(principal, sessionId, new Date()));
Date currentDate = new Date();
this.sessionIds.put(sessionId, new SessionInformation(principal, sessionId, new Date(currentDate.getTime()),
new Date(currentDate.getTime())));
Comment on lines +136 to +138
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's okay, since we wouldn't want to have a link to the same Date?

this.principals.compute(principal, (key, sessionsUsedByPrincipal) -> {
if (sessionsUsedByPrincipal == null) {
sessionsUsedByPrincipal = new CopyOnWriteArraySet<>();
Expand Down
5 changes: 4 additions & 1 deletion core/src/test/java/org/springframework/security/core/session/SessionInformationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* Tests {@link SessionInformation}.
*
* @author Ben Alex
* @author Andrey Litvitski
*/
public class SessionInformationTests {

Expand All @@ -34,10 +35,12 @@ public void testObject() throws Exception {
Object principal = "Some principal object";
String sessionId = "1234567890";
Date currentDate = new Date();
SessionInformation info = new SessionInformation(principal, sessionId, currentDate);
SessionInformation info = new SessionInformation(principal, sessionId, new Date(currentDate.getTime()),
new Date(currentDate.getTime()));
assertThat(info.getPrincipal()).isEqualTo(principal);
assertThat(info.getSessionId()).isEqualTo(sessionId);
assertThat(info.getLastRequest()).isEqualTo(currentDate);
assertThat(info.getCreatedTime()).isEqualTo(currentDate);
Thread.sleep(10);
info.refreshLastRequest();
assertThat(info.getLastRequest().after(currentDate)).isTrue();
Expand Down
6 changes: 5 additions & 1 deletion .../java/org/springframework/security/oauth2/client/oidc/session/OidcSessionInformation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ public class OidcSessionInformation extends SessionInformation {
* @param user the OIDC Provider's session and end user
*/
public OidcSessionInformation(String sessionId, Map<String, String> authorities, OidcUser user) {
super(user, sessionId, new Date());
this(sessionId, authorities, user, new Date());
}

private OidcSessionInformation(String sessionId, Map<String, String> authorities, OidcUser user, Date now) {
super(user, sessionId, new Date(now.getTime()), new Date(now.getTime()));
this.authorities = (authorities != null) ? new LinkedHashMap<>(authorities) : Collections.emptyMap();
}
Comment on lines 48 to 55
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only way to solve the problem I wrote about here #17513 (comment)


Expand Down
14 changes: 7 additions & 7 deletions ...urity/web/authentication/session/ConcurrentSessionControlAuthenticationStrategyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void setup() {
this.request = new MockHttpServletRequest();
this.response = new MockHttpServletResponse();
this.sessionInformation = new SessionInformation(this.authentication.getPrincipal(), "unique",
new Date(1374766134216L));
new Date(1374766134216L), new Date());
this.strategy = new ConcurrentSessionControlAuthenticationStrategy(this.sessionRegistry);
}

Expand Down Expand Up @@ -123,7 +123,7 @@ public void maxSessionsExpireExistingUser() {
@Test
public void maxSessionsExpireLeastRecentExistingUser() {
SessionInformation moreRecentSessionInfo = new SessionInformation(this.authentication.getPrincipal(), "unique",
new Date(1374766999999L));
new Date(1374766999999L), new Date());
given(this.sessionRegistry.getAllSessions(any(), anyBoolean()))
.willReturn(Arrays.<SessionInformation>asList(moreRecentSessionInfo, this.sessionInformation));
this.strategy.setMaximumSessions(2);
Expand All @@ -134,9 +134,9 @@ public void maxSessionsExpireLeastRecentExistingUser() {
@Test
public void onAuthenticationWhenMaxSessionsExceededByTwoThenTwoSessionsExpired() {
SessionInformation oldestSessionInfo = new SessionInformation(this.authentication.getPrincipal(), "unique1",
new Date(1374766134214L));
new Date(1374766134214L), new Date());
SessionInformation secondOldestSessionInfo = new SessionInformation(this.authentication.getPrincipal(),
"unique2", new Date(1374766134215L));
"unique2", new Date(1374766134215L), new Date());
given(this.sessionRegistry.getAllSessions(any(), anyBoolean())).willReturn(
Arrays.<SessionInformation>asList(oldestSessionInfo, secondOldestSessionInfo, this.sessionInformation));
this.strategy.setMaximumSessions(2);
Expand Down Expand Up @@ -196,7 +196,7 @@ public void maxSessionsExpireExistingUserUsingSessionLimit() {
@Test
public void maxSessionsExpireLeastRecentExistingUserUsingSessionLimit() {
SessionInformation moreRecentSessionInfo = new SessionInformation(this.authentication.getPrincipal(), "unique",
new Date(1374766999999L));
new Date(1374766999999L), new Date());
given(this.sessionRegistry.getAllSessions(any(), anyBoolean()))
.willReturn(Arrays.asList(moreRecentSessionInfo, this.sessionInformation));
this.strategy.setMaximumSessions(SessionLimit.of(2));
Expand All @@ -207,9 +207,9 @@ public void maxSessionsExpireLeastRecentExistingUserUsingSessionLimit() {
@Test
public void onAuthenticationWhenMaxSessionsExceededByTwoThenTwoSessionsExpiredUsingSessionLimit() {
SessionInformation oldestSessionInfo = new SessionInformation(this.authentication.getPrincipal(), "unique1",
new Date(1374766134214L));
new Date(1374766134214L), new Date());
SessionInformation secondOldestSessionInfo = new SessionInformation(this.authentication.getPrincipal(),
"unique2", new Date(1374766134215L));
"unique2", new Date(1374766134215L), new Date());
given(this.sessionRegistry.getAllSessions(any(), anyBoolean()))
.willReturn(Arrays.asList(oldestSessionInfo, secondOldestSessionInfo, this.sessionInformation));
this.strategy.setMaximumSessions(SessionLimit.of(2));
Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/org/springframework/security/web/concurrent/ConcurrentSessionFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ public void setLogoutHandlersWhenEmptyThenThrowsException() {
private SessionRegistry mockSessionRegistry() {
SessionRegistry registry = mock(SessionRegistry.class);
SessionInformation information = new SessionInformation("user", "sessionId",
new Date(System.currentTimeMillis() - 1000));
new Date(System.currentTimeMillis() - 1000), new Date());
information.expireNow();
given(registry.getSessionInformation(anyString())).willReturn(information);
return registry;
Expand Down
8 changes: 5 additions & 3 deletions ...st/java/org/springframework/security/web/session/SessionInformationExpiredEventTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,22 @@ public void constructorWhenSessionInformationNullThenThrowsException() {
@Test
public void constructorWhenRequestNullThenThrowsException() {
assertThatIllegalArgumentException().isThrownBy(() -> new SessionInformationExpiredEvent(
new SessionInformation("fake", "sessionId", new Date()), null, new MockHttpServletResponse()));
new SessionInformation("fake", "sessionId", new Date(), new Date()), null,
new MockHttpServletResponse()));
}

@Test
public void constructorWhenResponseNullThenThrowsException() {
assertThatIllegalArgumentException().isThrownBy(() -> new SessionInformationExpiredEvent(
new SessionInformation("fake", "sessionId", new Date()), new MockHttpServletRequest(), null));
new SessionInformation("fake", "sessionId", new Date(), new Date()), new MockHttpServletRequest(),
null));
}

@Test
void constructorWhenFilterChainThenGetFilterChainReturnsNotNull() {
MockFilterChain filterChain = new MockFilterChain();
SessionInformationExpiredEvent event = new SessionInformationExpiredEvent(
new SessionInformation("fake", "sessionId", new Date()), new MockHttpServletRequest(),
new SessionInformation("fake", "sessionId", new Date(), new Date()), new MockHttpServletRequest(),
new MockHttpServletResponse(), filterChain);
assertThat(event.getFilterChain()).isSameAs(filterChain);
}
Expand Down