Add component to handle Authentication filter exceptions

[bertramn]

Detailed Description

To complement the Problem mapping capability of SecurityProblemSupport, a custom AuthenticationFailureHandler should be added to ensure exceptions in the security filter chain are also returned as proper RFC7807 responses.

The AuthenticationFailureHandler strategy can be used to modify the behaviour of spring security. When using spring boot as a resource server for APIs (not web pages), the out-of-the-box AuthenticationFailureHandlers are no sufficiently configurable to ensure the failure is written to the response in RFC7807 format. There are a few documented hacks out there that sort of get you half way but since this is part of a security system, probably not a good idea to copy-and-paste a bunch of car insurance quote examples together to return auth errors in json format.

Context

We use this AuthenticationFailureHandler together with the SecurityProblemSupport to setup spring boot APIs in a way that all responses are RFC7807 compliant. Other users might find it convenient to use an API optimised failure handler instead trying to make the spring supplied ones do something they are not designed to do.

If you think it's worthwhile, I'll throw in the unit tests for free ;) .

Possible Implementation

We use something like this to close the gap:

package org.zalando.problem.spring.web.advice.security;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.http.server.ServletServerHttpResponse;
import org.springframework.security.authentication.AuthenticationServiceException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.web.WebAttributes;
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
import org.springframework.stereotype.Component;
import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.client.RestTemplate;
import org.zalando.problem.Problem;
import org.zalando.problem.Status;
import org.zalando.problem.spring.web.advice.AdviceTrait;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.Arrays;
import java.util.Enumeration;
import java.util.List;
import java.util.stream.Collectors;

import static java.util.Collections.*;
import static java.util.Optional.ofNullable;
import static org.springframework.http.MediaType.APPLICATION_JSON;
import static org.zalando.problem.Status.INTERNAL_SERVER_ERROR;
import static org.zalando.problem.Status.UNAUTHORIZED;

@Component
public class ProblemAuthenticationFailureHandler implements AuthenticationFailureHandler, AdviceTrait {

  private final Logger logger = LoggerFactory.getLogger(ProblemAuthenticationFailureHandler.class);

  private MediaType defaultMediaType = APPLICATION_JSON;

  private List<HttpMessageConverter<?>> messageConverters = new RestTemplate().getMessageConverters();

  /**
   * If the client did not indicate which media type to use in the {@link HttpHeaders#ACCEPT} header,
   * this value will be used as the default to determine the response content type.
   *
   * @param defaultMediaType the media type to be used as default
   */
  public void setDefaultMediaType(MediaType defaultMediaType) {
    this.defaultMediaType = defaultMediaType;
  }

  /**
   * The message converters are used to write the authentication exception error to the response.
   *
   * @param messageConverters message converters available in the application context
   */
  @Autowired
  public void setMessageConverters(List<HttpMessageConverter<?>> messageConverters) {
    this.messageConverters = messageConverters;
  }

  @Override
  public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, AuthenticationException exception) {

    request.setAttribute(WebAttributes.AUTHENTICATION_EXCEPTION, exception);

    if (response.isCommitted()) {
      logger.error("Response is already committed. Unable to write exception {}: {}", exception.getClass().getSimpleName(), exception.getLocalizedMessage());
      return;
    }

    // important: not all AuthenticationException(s) are client side failures
    Status status = (exception instanceof AuthenticationServiceException) ? INTERNAL_SERVER_ERROR : UNAUTHORIZED;

    Problem problem = toProblem(exception, status);

    try {
      response.setStatus(status.getStatusCode());
      // TODO add spring sec headers
      internalWrite(request, response, problem, defaultMediaType);
    } catch (IOException e) {
      if (logger.isWarnEnabled()) {
        logger.error("Failed to write error response", e);
      }
    } catch (HttpMediaTypeNotAcceptableException e) {
      response.setStatus(HttpStatus.UNSUPPORTED_MEDIA_TYPE.value());
    }

  }

  /**
   * This process will extract the acceptable response content types from the request and sorts these in order of the
   * optional quality designator.
   *
   * @param request          the servlet request for extracting the accept headers
   * @param response         the servlet response to write the entity to
   * @param problem          the entity to write to the response
   * @param defaultMediaType the default media type should no accept headers be provided
   * @throws IOException                         something went wrong writing the entity to the response
   * @throws HttpMediaTypeNotAcceptableException the requested media type to respond with cannot be honoured
   */
  private void internalWrite(HttpServletRequest request, HttpServletResponse response, Problem problem, MediaType defaultMediaType) throws IOException, HttpMediaTypeNotAcceptableException {

    Enumeration<String> acceptedHeaderValues = ofNullable(request.getHeaders(HttpHeaders.ACCEPT))
      .orElse(enumeration(singletonList(defaultMediaType.toString())));

    List<MediaType> acceptedMediaTypes = list(acceptedHeaderValues).stream()
      .flatMap(x -> Arrays.stream(x.split(",")))
      .map(MediaType::parseMediaType)
      .collect(Collectors.toList()).stream()
      .sorted(MediaType.QUALITY_VALUE_COMPARATOR)
      .collect(Collectors.toList());

    for (MediaType acceptedMediaType : acceptedMediaTypes) {
      if (write(problem, acceptedMediaType, response)) {
        return;
      }
    }

    // nothing written clearly we do not support requested content type
    throw new HttpMediaTypeNotAcceptableException(messageConverters.stream()
      .map(HttpMessageConverter::getSupportedMediaTypes)
      .flatMap(List::stream)
      .collect(Collectors.toList())
    );

  }

  /**
   * Attempt to write the problem entity to the response for the given media type.
   *
   * @param problem           the response entity to write
   * @param acceptedMediaType the media type accepted by the caller (in quality order)
   * @param response          the servlet response to write the message to
   * @return <code>true</code> when the entity was written to the response successfully, <code>false</code> otherwise
   * @throws IOException something went wrong writing the entity to the response
   */
  @SuppressWarnings("unchecked")
  private boolean write(Problem problem, MediaType acceptedMediaType, HttpServletResponse response) throws IOException {

    for (HttpMessageConverter messageConverter : messageConverters) {
      if (messageConverter.canWrite(problem.getClass(), acceptedMediaType)) {
        messageConverter.write(problem, acceptedMediaType, new ServletServerHttpResponse(response));
        return true;
      }
    }

    return false;

  }

}

[whiskeysierra]

to ensure exceptions in the security filter chain are also returned as proper RFC7807 responses.

Do you have examples? What kind of exceptions would that be?

[whiskeysierra]

If you think it's worthwhile, I'll throw in the unit tests for free ;) .

I'd love that! :heart:

[bertramn]

We implemented a multi-tenanted OpenID Connect authentication processing filter based on org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter which uses a AuthenticationSuccessHandler and AuthenticationFailureHandler for the authentication outcome. The default SimpleUrlAuthenticationFailureHandler is for form based auth and unless weirdly configured will redirect to login urls and create sessions - neither desirable for a pure stateless API. It also does not do any content negotiation or failure entity mapping. The exception that needs to be handled is any variant of the AuthenticationException and IOException from the filters attemptAuthentication() method. If you inspect the implementation above you find the old friend from #292 ;).

Just looking through the stuff I've written a while back and just realised, I actually have a whole autoconfigure setup for "ProblemManagement". Let me put this all in a PR and you guys can pick and choose which bits you like.

[bertramn]

The one question I have is supported versions. If we want Spring Boot 1.5.x support then autoconfig will be much more painful and complex. If 2.x is fine that would make things a lot easier to code.

[whiskeysierra]

284 already requires 2.x and is in the pipeline. I feel like we can go for 2.x exclusively now.

[sanyarnd]

I'm not sure if it's really required to make this so complex, perhaps an instruction with

@Component
@Slf4j
@RequiredArgsConstructor
class ProblemAuthenticationFailureHandler implements ServerAuthenticationFailureHandler {
    private final ProblemAdvice advice;

    @Override
    public Mono<Void> onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) {
        Mono<ResponseEntity<Problem>> response = advice.create(exception, webFilterExchange.getExchange());
        return response.and(Mono.empty());
    }
}

is sufficient?

[chicobento]

Hi, stumble upon this issue when integrating with keycloak-adapter. Keycloak configures by default its on KeycloakAuthenticationFailureHandler.

I got around the issue by doing something like:

@Bean
public AuthenticationFailureHandler problemAuthenticationFailureHandler(final SecurityProblemSupport securityProblem) {
        return securityProblem::commence;
}

I wonder if SecurityProblemSupport shouldn't implement AuthenticationFailureHandler exactly as it does for AuthenticationEntryPoint.

[whiskeysierra]

@chicobento Probably makes sense, yes.