Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

SEC-1949: ProviderManager should not query additional authentication providers when a BadCredentialsException is thrown #2175

spring-issuemaster opened this Issue Apr 6, 2012 · 3 comments


None yet
2 participants

Phil Krasko (Migrated from SEC-1949) said:

If a BadCredentialsException is thrown, the provider manager should break its loop and prevent calling further authentication providers.

Per the javadocs:
BadCredentialsException - Thrown if an authentication request is rejected because the credentials are invalid.

There is no point to call additional providers if the credentials are invalid.

This is somewhat related to SEC-546.

Rob Winch said:

Can I ask how this is causing you a problem. This is intentional behavior so that multiple AuthenticationProviders can be used. For example, the user may not have valid credentials in LDAP (tried first) and do have valid credentials in a database (tried second).

Phil Krasko said:

If a bad credentials exception is thrown it means the user was found, is neither locked or disabled, but the provided password is incorrect. Why would you go to another authentication provider? I would expect to try another provider if a UsernameNotFoundException or AuthenticationServiceException was thrown.

In my scenario I know which provider the user should be authenticating with.
In each provider I immediately check to see the the user type is supported by the provider. If not, I throw a AuthenticationServiceException to short circuit and skip to the next provider.

I don't want to continue down the chain of providers when a bad credentials exception is thrown because the provider I selected is the only one the user can authenticate with.

Rob Winch said:

Thank you for explaining this in more detail. Unfortunately, I do not think we can make the proposed changes. I am closing this ticket as Won't Fix for the following reasons:

  • BadCredentialException means that the "credentials are invalid" (see javadoc). It does not necessarily mean that the user was found. There are often times the difference cannot be determined. For example, LDAP bind authentication cannot distinguish between the two scenarios. This means we cannot use BadCredentials vs UsernameNotFoundException to determine if the ProviderManager should continue.
  • UserNotFoundException is intended for UserDetailsService implementations and for AuthenticationProviders that use a UserDetailsService (see javadoc). It is not intended for general purpose AuthenticationProvider implementations and we do not want to leak this abstraction. For example, we do not want AuthenticationProvider implementations to throw it when they want the ProviderManager to continue to the next AuthenticationProvider.
  • Many, if not all, AuthenticationProviders convert the UsernameNotFoundException to a BadCredentialsException to ensure phishing is not done. You can see an example of this on AbstractUserDetailsAuthenticationProvider which has the hideUserNotFoundException property set to true by default. When the property is true it transforms the UserNotFoundException into BadCredentialsException. This means the ProviderManager will not be able to distinguish between an invalid username (UserNameNotFoundException) and an invalid username/password combination (BadCredentialsException). Users could set this property to false, but we do not want to encourage them to since this will allow for phishing.
  • There are others relying on ProviderManager to continue looping through the AuthenticationProviders

This means you will need to extend Spring Security to meet this requirement. If you need the ability to select a specific provider and it cannot be determined by the supports method, you have at least a few options:

  • Since AuthenticationManager is an interface, you can implement the logic yourself. The logic is essentially just looping over the AuthenticationProvider implementations so it should not be difficult to implement.
  • Allow ProviderManager to iterate over all the AuthenticationProviders. If the AuthenticationProvider cannot authenticate the user, it can return null. This should be rather low cost in terms of performance.

If you need further guidance, I'd be happy to assist on the forums.

@spring-issuemaster spring-issuemaster added this to the 3.1.2 milestone Feb 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment