Not all Spring AuthenticationException(s) map to a 401 status code

[bertramn]

Description

The AuthenticationAdviceTrait.java#L26 creates a 401 for ALL AuthenticationException(s) but the type hierarchy suggests that there can also be 500 type errors, namely AuthenticationServiceException and its subtype InternalAuthenticationServiceException.

Expected Behavior

A thrown AuthenticationServiceException should result in a 500 to the caller and log an error message.

Actual Behavior

A 401 is returned with a warning log written.

Possible Fix

public interface AuthenticationAdviceTrait extends AdviceTrait {

    @API(status = INTERNAL)
    @ExceptionHandler
    default ResponseEntity<Problem> handleAuthentication(final AuthenticationException e,
            final NativeWebRequest request) {
       if (exception instanceof AuthenticationServiceException) {
         return create(INTERNAL_SERVER_ERROR, e, request);
       } else {
         return create(UNAUTHORIZED, e, request);
       }
    }

}

Steps to Reproduce

observation while reading the code - got caught out on this issue a while back myself

Your Environment

0.23.0 using web (not flux)

[whiskeysierra]

Good point. I wasn't aware of that.

[whiskeysierra]

@DBlaesing Is there any security concern with that? Are we running a risk of exposing too much information to an attacker?

I was thinking of this, a bit:

Why am I getting a 404 error on a repository that exists? Typically, we send a 404 error when your client isn't properly authenticated. You might expect to see a 403 Forbidden in these cases. However, since we don't want to provide any information about private repositories, the API returns a 404 error instead.

https://developer.github.com/v3/troubleshooting/#why-am-i-getting-a-404-error-on-a-repository-that-exists

[whiskeysierra]

Now that we internally agreed that this is the right thing to do, I'm struggling to properly cause an exception of this type in my test.

[whiskeysierra]

I have the change locally but I have a hard time raising this exception for the webflux module. @bertramn @cbornet Any ideas?

[whiskeysierra]

See https://github.com/zalando/problem-spring-web/compare/feature/auth-service-exception

[bertramn]

Sorry have been out of action for a long time. The changes look about right although for some reason I am not able to open the project in IntelliJ (Ultimate). Also the Webflux test for the SecurityTrait is failing, albeit should be working.

[whiskeysierra]

Has anyone an idea to solve that? Otherwise I'm considering closing this without fixing.

[cbornet]

@whiskeysierra I did some comments in https://github.com/zalando/problem-spring-web/commit/78f6c59de233860057e0fa684b5d312d78d27c2c . Hope that helps

[skjolber]

Expected behaviour comment: Reding the javadocs, I believe InternalAuthenticationServiceException should result in 500, and AuthenticationServiceException is more 503.

[mreilaender]

I managed to create a reproducible example for this error. In this case spring throws a InsufficientAuthenticationException a 500 status is returned as stated in #582, #498 and is apparently not resolved by #674. I created a example project reproducing the protential bug. It seems that using two @ControllerAdvices implementing ProblemHandling and SecurityExceptionHandler creates the problem. It can be solved by using only one controller advice class implementing both interfaces but I wanted to mention this anyway.

[matt-locker]

Thanks to @mreilaender! Got it working with your hint!

Please update in documentation that only one ControllerAdvice should be used.