Spring Security
  1. Spring Security
  2. SEC-1567

The contract for AbstractAuthenticationProcessingFilter subclasses is ambiguous.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Invalid
    • Affects Version/s: 3.0.2
    • Fix Version/s: 3.1.0.M2
    • Component/s: Core
    • Labels:
      None
    • Environment:
      n/a

      Description

      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?

        Activity

        Hide
        Luke Taylor added a comment -

        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.

        Show
        Luke Taylor added a comment - 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.
        Hide
        Luke Taylor added a comment -

        No further input, so closing.

        Show
        Luke Taylor added a comment - No further input, so closing.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: