Spring Security
  1. Spring Security
  2. SEC-1047

DigestProcessingFilter does not leave the UsernamePasswordAuthenticationToken.authenticated=true

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.4
    • Fix Version/s: 3.0.0 RC1
    • Component/s: Core
    • Labels:
      None

      Description

      When using DigestProcessingFilter, if the Digest dialog completes successfully and the response is correct having matched it against the userDetailsService registered with the filter, it creates a UsernamePasswordAuthenticationToken, but it does not set the authenticated flag to true. This results in the AbstractSecurityInterceptor.authenticateIfRequired() method being called, which reauthenticates the user against all the configured userDetailsService.

      This causes additional unnecessary authentication load against the userDetailsServices if there is more than one service.

      Constrast this with the BasicProcessingFilter, which does create a UsernamePasswordAuthenticationToken with authenticated=true, therefore when reaching AbstractSecurityInterceptor.authenticateIfRequired(), it satisfies the authentication.isAuthenticated() test so returns immediately.

      The difference I can see is that the BasicProcessingFilter goes through the AuthenticationManager.authenticate(), which therefore populates the Authentication object with the relevant GrantedAuthorities, whereas the Digest version only uses the supplied userDetailsService for the digest validation and not for the authentication itself.

      I have put this as a bug as it seems like it should not be doing this the way it does and should really be limiting the authentication to the specified service, but I don't have a great deal of knowledge about the intentions of security, so this might be an enhancement request

        Activity

        Hide
        Luke Taylor added a comment -

        The "authenticated" flag is intended to indicate that the token has been processed by the AuthenticationManager, allowing it to reject the authentication request for other reasons - perhaps due to information in the authentication details object, or because the user has too many sessions open. However, digest authentication does require that the password is loaded in order to construct the authentication token, which means there is the potential for extra load on the storage system.

        I don't really see any harm in adding an option to use the authorities that are loaded in the filter directly, thus bypassing the call to the AuthenticationManager.

        Show
        Luke Taylor added a comment - The "authenticated" flag is intended to indicate that the token has been processed by the AuthenticationManager, allowing it to reject the authentication request for other reasons - perhaps due to information in the authentication details object, or because the user has too many sessions open. However, digest authentication does require that the password is loaded in order to construct the authentication token, which means there is the potential for extra load on the storage system. I don't really see any harm in adding an option to use the authorities that are loaded in the filter directly, thus bypassing the call to the AuthenticationManager.
        Hide
        Mike Wiesner added a comment -

        I've added setCreateAuthenticatedToken() where you can get rid of the re-authentication, but the default is false, as it has some security implications (see the Javadoc).

        Show
        Mike Wiesner added a comment - I've added setCreateAuthenticatedToken() where you can get rid of the re-authentication, but the default is false, as it has some security implications (see the Javadoc).

          People

          • Assignee:
            Mike Wiesner
            Reporter:
            Antony Bowesman
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: