Spring Security
  1. Spring Security
  2. SEC-946

SS redirects to saved request URL instead of default target URL

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 3.0.0 M1
    • Component/s: Core
    • Labels:
      None

      Description

      When an unauthenticated user attempts to access a protected resource, AuthenticationProcessingFilter (actually AbstractProcessingFilter) saves the target URL to the session and then redirects to the login page. If the user decides not to complete the login at that point in time, the target URL remains on the session. If the user later decides to directly access a Login link (i.e. they hit the login page directly rather than indirectly via interception), the user will be redirected to the saved request URL rather than the default target URL. It would seem that the default target URL is the intended target in this scenario.

      Besides causing some weirdness when users login, it also makes it unreliable to use the presence of a target URL to differentiate direct logins from interception-based logins. It is useful to distinguish the two when rendering the login form. In the case of an interception, I'd like to be able to include some explanatory text (e.g. "You must sign in to access the requested page.") just so the user isn't wondering why a login page just appeared.

      I've devised an app-level workaround though ultimately this probably should be a framework level fix. In the workaround, suppose I set the login URL to /login.jsp. In the JSP I will use a URL like this: login.jsp?direct=true. Then inside login.jsp I'll have this:

      <c:if test="$

      {not empty param.direct}

      ">
      <c:remove scope="session" var="SPRING_SECURITY_SAVED_REQUEST_KEY"/>
      </c:if>

      I posted a message about this here as well: [url]http://forum.springframework.org/showthread.php?t=58243[/url]

        Issue Links

          Activity

          Hide
          Willie Wheeler added a comment -

          Slight correction--it appears that it's actually ExceptionTranslationFilter that saves the SavedRequest instance to the session, not the AbstractProcessingFilter.

          Show
          Willie Wheeler added a comment - Slight correction--it appears that it's actually ExceptionTranslationFilter that saves the SavedRequest instance to the session, not the AbstractProcessingFilter.
          Hide
          Luke Taylor added a comment -

          I think the best option here is if the decision to remove the SavedRequest is placed in the AbstractProcessingFilter based class - this will know whether it has redirected to the SavedRequest URL or not. If not, then it should be removed. This may cause a problem for people who have overridden the filter behaviour to start something like a password change workflow, for example, with the intention of making use of the SavedRequest at a later stage. They would have to make sure they also overrode the code which was responsible for removing the SavedRequest from the session.

          Show
          Luke Taylor added a comment - I think the best option here is if the decision to remove the SavedRequest is placed in the AbstractProcessingFilter based class - this will know whether it has redirected to the SavedRequest URL or not. If not, then it should be removed. This may cause a problem for people who have overridden the filter behaviour to start something like a password change workflow, for example, with the intention of making use of the SavedRequest at a later stage. They would have to make sure they also overrode the code which was responsible for removing the SavedRequest from the session.
          Hide
          Luke Taylor added a comment -

          This is related to the URL navigation strategy issue because the logic for removing the saved request or not should be encapsulated in the strategy implementation.

          Show
          Luke Taylor added a comment - This is related to the URL navigation strategy issue because the logic for removing the saved request or not should be encapsulated in the strategy implementation.
          Hide
          Luke Taylor added a comment -

          I think the recent changes I've made for SEC-745 should go some way to alleviating the problem of stale SavedRequests being left in the session. This could happen when some other redirection strategy was used other than redirecting to the SavedRequest URL. It will now be removed at the point where such a redirect occurs.

          The situation where you provide both a direct login and a login "on-demand", and the user triggers the latter and then decides afterwards to use the direct login approach is more complicated. You could go with the approach you suggested - differentiating between the two at the login form (or controller) and removing the SavedRequest there.

          The class that implements most of the logic for this now is SavedRequestAwareSuccessfulAuthenticationHandler (which is used as the default login success strategy for AbstractProcessingFilter). We could potentially encapsulate the logic in there for removing the SavedRequest - possibly by allowing an additional parameter to be supplied with the form. You can already set a parameter to define the target Url and override the SavedRequest. That would be another possibility (and the new code removes the SavedRequest if it finds the parameter).

          Open to further suggestions .

          Show
          Luke Taylor added a comment - I think the recent changes I've made for SEC-745 should go some way to alleviating the problem of stale SavedRequests being left in the session. This could happen when some other redirection strategy was used other than redirecting to the SavedRequest URL. It will now be removed at the point where such a redirect occurs. The situation where you provide both a direct login and a login "on-demand", and the user triggers the latter and then decides afterwards to use the direct login approach is more complicated. You could go with the approach you suggested - differentiating between the two at the login form (or controller) and removing the SavedRequest there. The class that implements most of the logic for this now is SavedRequestAwareSuccessfulAuthenticationHandler (which is used as the default login success strategy for AbstractProcessingFilter). We could potentially encapsulate the logic in there for removing the SavedRequest - possibly by allowing an additional parameter to be supplied with the form. You can already set a parameter to define the target Url and override the SavedRequest. That would be another possibility (and the new code removes the SavedRequest if it finds the parameter). Open to further suggestions .
          Hide
          Luke Taylor added a comment -

          Closing as "fixed" with the caveats mentioned above. The removal of the SavedRequest is a "best effort" undertaking. It will almost certainly be possible to think up situations where an application flow could result in the request remaining in the session. In those cases, the application should be aware of the possibility and remove the save request itself (an example would be the "direct" login case above).

          Show
          Luke Taylor added a comment - Closing as "fixed" with the caveats mentioned above. The removal of the SavedRequest is a "best effort" undertaking. It will almost certainly be possible to think up situations where an application flow could result in the request remaining in the session. In those cases, the application should be aware of the possibility and remove the save request itself (an example would be the "direct" login case above).

            People

            • Assignee:
              Luke Taylor
              Reporter:
              Willie Wheeler
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: