Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.4
    • Fix Version/s: 3.0.0 M1
    • Component/s: Core
    • Labels:
      None

      Activity

      Hide
      Luke Taylor added a comment -

      I've committed some substantial refactorings for this issue.

      AbstractProcessingFilter now has an AuthenticationSuccessHandler and AuthenticationFailureHandler injected, which perform all the functionality of the previous navigational behaviour on authentication success and failure. (See parent issue for more information on the implementations).
      If no implementations are injected, the defaults are a SavedRequestAwareASH (which has the defaultTargetUrl="/") and a SimpleUrlAFH with no URL set (which will send a 401 (unauthorized) error by default with the reported exception message).

      Thus the following properties are no longer present in this class:

      exceptionMappings
      serverSideRedirect
      authenticationFailureUrl
      defaultTargetUrl and alwaysUseDefaultTargetUrl
      useRelativeContext

      and the corresponding strategy properties should be used instead. "serverSideRedirect is" now called "useForward" on the SimpleUrlAuthenticationFailureHandler.

      The methods related to determining Urls for success and failure have also been removed, as has the TargetUrlResolver.

      I also removed the additional "hook" methods to simplify things - onPreAuthentication was redundant as attempAuthentication has to be implemented by subclasses anyway and it was called immediately before this method. The onSuccessfulAuthenticaition and onUnsuccessfulAuthentication methods are superseded by the use of the strategy objects. In any case, successfulAuthentication() and unsuccessfulAuthentication() are already protected and can thus also be overridden.

      During the refactoring, it became obvious that the OpenID filter was making use of these methods in a rather contrived manner to get round the fact that the base class doesn't cater for situations where there is more than one stage to the authentication process. It was throwing an exception from the attemptAuthentication method to indicate that a redirect to the service provider should be performed from the authenticationFailure method (since this had access to the response object). Therefore, I've modified attemptAuthentication() to now takes a response object in addition to the request (to allow subclasses to perform a redirect). It can return null to indicate that the authentication process is still continuing.

      The OpenIDAuthenticationProcessingFilter now makes use of this ability, performing the redirect to the OpenID service provider from the attemptAuthentication() method (returning null) and then handling the redirect back from the service in the same method.

      Show
      Luke Taylor added a comment - I've committed some substantial refactorings for this issue. AbstractProcessingFilter now has an AuthenticationSuccessHandler and AuthenticationFailureHandler injected, which perform all the functionality of the previous navigational behaviour on authentication success and failure. (See parent issue for more information on the implementations). If no implementations are injected, the defaults are a SavedRequestAwareASH (which has the defaultTargetUrl="/") and a SimpleUrlAFH with no URL set (which will send a 401 (unauthorized) error by default with the reported exception message). Thus the following properties are no longer present in this class: exceptionMappings serverSideRedirect authenticationFailureUrl defaultTargetUrl and alwaysUseDefaultTargetUrl useRelativeContext and the corresponding strategy properties should be used instead. "serverSideRedirect is" now called "useForward" on the SimpleUrlAuthenticationFailureHandler. The methods related to determining Urls for success and failure have also been removed, as has the TargetUrlResolver. I also removed the additional "hook" methods to simplify things - onPreAuthentication was redundant as attempAuthentication has to be implemented by subclasses anyway and it was called immediately before this method. The onSuccessfulAuthenticaition and onUnsuccessfulAuthentication methods are superseded by the use of the strategy objects. In any case, successfulAuthentication() and unsuccessfulAuthentication() are already protected and can thus also be overridden. During the refactoring, it became obvious that the OpenID filter was making use of these methods in a rather contrived manner to get round the fact that the base class doesn't cater for situations where there is more than one stage to the authentication process. It was throwing an exception from the attemptAuthentication method to indicate that a redirect to the service provider should be performed from the authenticationFailure method (since this had access to the response object). Therefore, I've modified attemptAuthentication() to now takes a response object in addition to the request (to allow subclasses to perform a redirect). It can return null to indicate that the authentication process is still continuing. The OpenIDAuthenticationProcessingFilter now makes use of this ability, performing the redirect to the OpenID service provider from the attemptAuthentication() method (returning null) and then handling the redirect back from the service in the same method.
      Hide
      Luke Taylor added a comment -

      I've also refactored AbstractProcessingFilter to remove the use of the getDefaultTargetUrl template method in favour of a constructor argument instead (since the method was effectively called from the constructor and nowhere else).

      Show
      Luke Taylor added a comment - I've also refactored AbstractProcessingFilter to remove the use of the getDefaultTargetUrl template method in favour of a constructor argument instead (since the method was effectively called from the constructor and nowhere else).

        People

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

          Dates

          • Created:
            Updated:
            Resolved: