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-1567: The contract for AbstractAuthenticationProcessingFilter subclasses is ambiguous. #1808

spring-issuemaster opened this Issue Sep 8, 2010 · 2 comments


None yet
1 participant

Jasper Blues (Migrated from SEC-1567) said:

AbstractAuthenticationProcessingFilter treats a returned Authentication implementation as successful, even if Authentication.isAuthenticated == false.

Only 'null' or AuthenticationFailureException are honored as failed authentication attempts.

This seems ambiguous. Wouldn't it be better if only Authentication.isAuthenticated == true is honored as successful authentication?

Luke Taylor said:

Hi Jasper. If you're talking about the attemptAuthentication method, the Javadoc says:

The implementation should do one of the following:

  • Return a populated authentication token for the authenticated user, indicating successful authentication
  • Return null, indicating that the authentication process is still in progress. Before returning, the implementation should perform any additional work required to complete the process.
  • Throw an AuthenticationException if the authentication process fails

Which doesn't really seem very ambiguous to me... The "isAuthenticated" flag was originally intended as an indicator that a token had not been processed (by the AuthenticationManager). In this case, the method is supposed to perform the authentication which should by definition mean that a user has been successfully authenticated if the method returns a non-null value. There's no practical reason I can see why it would return a token which had isAuthenticated==false. Perhaps you could explain the use case where you envisage doing this.

Luke Taylor said:

No further input, so closing.

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M2 milestone Feb 5, 2016

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