SEC-1047: DigestProcessingFilter does not leave the UsernamePasswordAuthenticationToken.authenticated=true #1299

Closed
spring-issuemaster opened this Issue Dec 2, 2008 · 2 comments

1 participant

@spring-issuemaster

Antony Bowesman (Migrated from SEC-1047) said:

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

@spring-issuemaster

Luke Taylor said:

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.

@spring-issuemaster

Mike Wiesner said:

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

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