Skip to content
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.

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

Add ContinueRequestSessionInformationExpiredStrategy #14765

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ private void doFilter(HttpServletRequest request, HttpServletResponse response,
.of(() -> "Requested session ID " + request.getRequestedSessionId() + " has expired."));
doLogout(request, response);
this.sessionInformationExpiredStrategy
.onExpiredSessionDetected(new SessionInformationExpiredEvent(info, request, response));
.onExpiredSessionDetected(new SessionInformationExpiredEvent(info, request, response, chain));
return;
}
// Non-expired - update last request date/time
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.security.web.session;

import jakarta.servlet.ServletException;
import java.io.IOException;

/**
* A {@link SessionInformationExpiredStrategy} continues to execute subsequent filters
* even after session expiration.
*
* @author Rosie Yang
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add @since 6.3 too

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I changed too. I realized I need to pay more attention to contribution rules, sorry about that.

* @since 6.3
*/
public final class ContinueRequestSessionInformationExpiredStrategy implements SessionInformationExpiredStrategy {

@Override
public void onExpiredSessionDetected(SessionInformationExpiredEvent event) throws ServletException, IOException {
event.getFilterChain().doFilter(event.getRequest(), event.getResponse());
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2016 the original author or authors.
* Copyright 2012-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 All @@ -16,6 +16,7 @@

package org.springframework.security.web.session;

import jakarta.servlet.FilterChain;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

Expand All @@ -27,6 +28,7 @@
* An event for when a {@link SessionInformation} is expired.
*
* @author Rob Winch
* @author Rosie Yang
* @since 4.2
*/
public final class SessionInformationExpiredEvent extends ApplicationEvent {
Expand All @@ -35,6 +37,8 @@ public final class SessionInformationExpiredEvent extends ApplicationEvent {

private final HttpServletResponse response;

private final FilterChain filterChain;

/**
* Creates a new instance
* @param sessionInformation the SessionInformation that is expired
Expand All @@ -48,6 +52,17 @@ public SessionInformationExpiredEvent(SessionInformation sessionInformation, Htt
Assert.notNull(response, "response cannot be null");
this.request = request;
this.response = response;
this.filterChain = null;
}

public SessionInformationExpiredEvent(SessionInformation sessionInformation, HttpServletRequest request,
HttpServletResponse response, FilterChain filterChain) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of changing this constructor, a new constructor should be created accepting all those parameter + the FilterChain. The reason is because if we change this constructor it will be a breaking change for anyone that is using SessionInformationExpiredEvent.

Copy link
Author

Choose a reason for hiding this comment

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

As you mentioned, I add new constructor below original constructor. But filterChain value need for new constructor i had to handle this in original constructor too.

super(sessionInformation);
Assert.notNull(request, "request cannot be null");
Assert.notNull(response, "response cannot be null");
this.request = request;
this.response = response;
this.filterChain = filterChain;
}

/**
Expand All @@ -68,4 +83,7 @@ public SessionInformation getSessionInformation() {
return (SessionInformation) getSource();
}

public FilterChain getFilterChain() {
return this.filterChain;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.springframework.security.web.authentication.logout.LogoutHandler;
import org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler;
import org.springframework.security.web.session.ConcurrentSessionFilter;
import org.springframework.security.web.session.ContinueRequestSessionInformationExpiredStrategy;
import org.springframework.security.web.session.SessionInformationExpiredEvent;
import org.springframework.security.web.session.SessionInformationExpiredStrategy;
import org.springframework.security.web.session.SimpleRedirectSessionInformationExpiredStrategy;

Expand All @@ -59,6 +61,7 @@
* @author Ben Alex
* @author Luke Taylor
* @author Onur Kagan Ozcan
* @author Rosie Yang
*/
public class ConcurrentSessionFilterTests {

Expand Down Expand Up @@ -301,4 +304,18 @@ public void setLogoutHandlersWhenEmptyThenThrowsException() {
assertThatIllegalArgumentException().isThrownBy(() -> filter.setLogoutHandlers(new LogoutHandler[0]));
}

@Test
public void doFilterWhenRequestSessionInformationExpiredThenChainIsContinued() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest();
request.setSession(new MockHttpSession());
MockHttpServletResponse response = new MockHttpServletResponse();
RedirectStrategy redirect = mock(RedirectStrategy.class);
SessionRegistry registry = mock(SessionRegistry.class);
ContinueRequestSessionInformationExpiredStrategy expiredStrategy = new ContinueRequestSessionInformationExpiredStrategy();
ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry, expiredStrategy);
MockFilterChain chain = new MockFilterChain();
filter.doFilter(request, response, chain);
assertThat(chain.getRequest()).isNotNull();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@ public class SessionInformationExpiredEventTests {
@Test
public void constructorWhenSessionInformationNullThenThrowsException() {
assertThatIllegalArgumentException().isThrownBy(() -> new SessionInformationExpiredEvent(null,
new MockHttpServletRequest(), new MockHttpServletResponse()));
new MockHttpServletRequest(), new MockHttpServletResponse(), null));
}

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

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

}