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

ConcurrentSessionFilter should be able to pass the request downstream (after doLogout), such that all authentication redirects can be handled by ExceptionTranslationFilter. #8363

Open
jvanheesch opened this issue Apr 9, 2020 · 3 comments
Labels
in: web An issue in web modules (web, webmvc)

Comments

@jvanheesch
Copy link

This issue was initially discussed here.

We use Keycloak for SSO.
When Keycloak sends a logout request to our client application, it includes the sessionId of the session to invalidate.
However, invalidating a session by id is not supported by the servlet spec (I think).
As such, the way we accomplish this session invalidation is by expiring the corresponding org.springframework.security.core.session.SessionInformation object.
A custom javax.servlet.http.HttpFilter invalidates the corresponding HttpSession on the next incoming request for that session, and passes the request downstream:

public class InvalidateExpiredSessionsFilter extends HttpFilter {
    private final SessionRegistry sessionRegistry;

    public InvalidateExpiredSessionsFilter(SessionRegistry sessionRegistry) {
        this.sessionRegistry = sessionRegistry;
    }

    @Override
    protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
        HttpSession session = request.getSession(false);
        if (session != null) {
            SessionInformation sessionInformation = this.sessionRegistry.getSessionInformation(session.getId());
            if (sessionInformation != null && sessionInformation.isExpired()) {
                session.invalidate();
            }
        }
        super.doFilter(request, response, chain);
    }
}

This way, invalidated sessions & non-existing/timed-out sessions are both handled by ExceptionTranslationFilter using authenticationEntryPoint (after an AccessDeniedException is thrown).

Rather than defining a custom javax.servlet.http.HttpFilter , we'd love to reuse org.springframework.security.web.session.ConcurrentSessionFilter.
This filter comes out-of-the-box and serves a similar purpose.
However, we currently cannot use this filter as there's no option to pass the request downstream (after doLogout(request, response)), because SessionInformationExpiredStrategy has no access to the FilterChain.

@jvanheesch jvanheesch changed the title ConcurrentSessionFilter should be able to pass the request downstream (after logging out), such that all authentication redirects can be handled by ExceptionTranslationFilter. ConcurrentSessionFilter should be able to pass the request downstream (after doLogout), such that all authentication redirects can be handled by ExceptionTranslationFilter. Apr 9, 2020
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 9, 2020
@jzheaux jzheaux self-assigned this Apr 9, 2020
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 9, 2020
@rwinch
Copy link
Member

rwinch commented Apr 9, 2020

I think a better approach is to simulate a browser request by using something like RestTemplate with the JSESSIONID cookie set and then calling back to the container. This ensures that all of the proper log out logic happens immediately.

@jzheaux jzheaux assigned rwinch and unassigned jzheaux Apr 9, 2020
@mengelbrecht
Copy link
Contributor

mengelbrecht commented Apr 11, 2020

If I understood the scenario correctly, you could also implement a custom SessionInformationExpiredStrategy which redirects to the SessionInformationExpiredEvent request url. This way the same request is performed but the logout has already taken place. If the request url requires authentication the ExceptionTranslationFilter will trigger the authentication process.

@jvanheesch
Copy link
Author

@rwinch I really like that your approach does not delay the logout until the next request for that user.
However, I don't like the idea that requests for a user are sent from somewhere other than the user's browser.
This could break some developer assumptions (e.g. requests sent by RestTemplate will not include the user's cookies, other than JSESSIONID).
Is it important from a security standpoint that the logout happens immediately, or is it secure to delay this as in the current approach?

@mengelbrecht That seems like a workaround for this issue.
As SessionInformationExpiredStrategy doesn't allow me to pass the request downstream, we use a redirect to travel the whole filter chain again from the start.
I do like this approach though, because it indeed allows me to use Spring's ConcurrentSessionFilter and ExceptionTranslationFilter.

@rwinch rwinch removed their assignment Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc)
Projects
None yet
Development

No branches or pull requests

5 participants