Spring Security
  1. Spring Security
  2. SEC-1621

SessionManagementFilter should not create (always) new session before redirecting to invalidSessionUrl

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 3.0.3
    • Fix Version/s: 3.1.0.M2
    • Component/s: Web
    • Labels:
      None

      Description

      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).

        Activity

        Hide
        Luke Taylor added a comment -

        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".

        Show
        Luke Taylor added a comment - 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".
        Hide
        Johannes Scharf added a comment -

        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.

        Show
        Johannes Scharf added a comment - 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.
        Hide
        Johannes Scharf added a comment -

        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.

        Show
        Johannes Scharf added a comment - 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.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Johannes Scharf
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: