Not all Spring AuthenticationException(s) map to a 401 status code
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)
Good point. I wasn't aware of that.
@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
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.
I have the change locally but I have a hard time raising this exception for the webflux module. @bertramn @cbornet Any ideas?
See https://github.com/zalando/problem-spring-web/compare/feature/auth-service-exception
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.
Has anyone an idea to solve that? Otherwise I'm considering closing this without fixing.
@whiskeysierra I did some comments in https://github.com/zalando/problem-spring-web/commit/78f6c59de233860057e0fa684b5d312d78d27c2c . Hope that helps
Expected behaviour comment: Reding the javadocs, I believe InternalAuthenticationServiceException should result in 500, and AuthenticationServiceException is more 503.
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.