SEC-1621: SessionManagementFilter should not create (always) new session before redirecting to invalidSessionUrl #1862

Closed
spring-issuemaster opened this Issue Nov 10, 2010 · 3 comments

1 participant

@spring-issuemaster

Johannes Scharf (Migrated from SEC-1621) said:

The SessionManagementFilter always creates a new session before redirecting to invalidSessionUrl.

I think that is a waste of resources and absolutely not necessary. Isn't it a little bit strange if a new session is created before the user gets redirected to a page which says "session timeout"?

BTW the message
"Starting new session (if required) and redirecting to '/main/error/invalidsession.htm"
which is logged before request.getSession() is a little bit confusing ("if required") as the session gets always created afterwards.

Maybe I get something wrong but if not, please change the code to not create a new session (in any case).

@spring-issuemaster

Luke Taylor said:

This behaviour was added deliberately. If a new session is not created then the subsequent request for the timeout page also has an invalid session, resulting in an infinite loop unless the user explicitly exempts that page from the filter chain, which most forget to do. Since a timeout warning occurs when a user attempting to re-access the system, I don't think creating a new session at this point is such a big deal. The normal pattern would be to show a page saying something like "your session has expired, please click here to log back in".

@spring-issuemaster

Johannes Scharf said:

Ok, that's a different point of view. The creation of the session is indeed not a big deal, but I just don't like the unnecessary creation.

I understand your argument regarding the endless loop - but IMHO the same "problem" occurs with a login page in Spring Security. Such a page must also be excluded from the filter chain or must be "protected" anonymously to avoid a endless loop.
Shouldn't it be up to the user to avoid this with the "invalid session" page? Maybe a smart hint in the docs would be enough...

However the log message "Starting new session (if required)..." is confusing. Please remove the part "if required" as it suggests that a session will NOT be created in any case.

@spring-issuemaster

Johannes Scharf said:

Forget my comment about the log message. "if required" means in that context that a new session will be created if there isn't already one. The message is ok.

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M2 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment