spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.21k stars 40.69k forks source link

Error response body does not match Content-Type #33716

Closed osiegmar closed 10 months ago

osiegmar commented 1 year ago

Describe the bug In an application (using Spring Boot 3.0.1) the response body does not match the Content-Type header for a 403 Forbidden response if the request contains the header Accept: application/problem+json, application/json:

Content-Type: application/problem+json

{"timestamp":"2022-12-23T07:44:25.247+00:00","status":403,"error":"Forbidden","path":"/secret"}

Note: I'm using the shown mime type order because of https://github.com/spring-projects/spring-framework/issues/29588

To Reproduce

Expected behavior

Sample

@SpringBootApplication
@RestController
@EnableWebSecurity
@EnableMethodSecurity
public class Application {

    @Bean
    public UserDetailsService userDetailsService() {
        return new InMemoryUserDetailsManager(User.withDefaultPasswordEncoder()
            .username("user").password("password").roles("USER").build());
    }

    @PreAuthorize("hasRole('ROLE_ADMIN')")
    @GetMapping("/secret")
    public String secret() {
        return "Secret";
    }

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

}

Request:

curl -i http://localhost:8080/secret \
     -u "user:password" \
     -H "Accept: application/problem+json, application/json"

I already opened this issue as https://github.com/spring-projects/spring-security/issues/12450 but learned that is related to Spring Boot, not Spring Security.

MaksimMyshkin commented 1 year ago

Also noticed that any exception not inherited from ResponseStatusException, annotated with @ResponseStatus or handled with @ExceptionHandler explicitly returning in old (not problem detail) format.

mbhave commented 1 year ago

Regarding 401 not containing any body at all, this is expected because the error dispatch does not get to the Spring Boot error controller at all unless the error page is explicitly opened. This is related to the change in defaults in Spring Security 6 which applies the filter to all dispatch types.

kumarisback commented 1 year ago

Hi, I have just started spring boot I don't have that much knowledge But what I have understand is that you want the response in application/problem+json but you are not getting in that format by default if the authorization fails then it will give response in text/html until you have configured your exception like this

  @ExceptionHandler(AccessDeniedException.class)
  @ResponseStatus(HttpStatus.UNAUTHORIZED)
  ResponseEntity<?> handleAccessDeniedException(AccessDeniedException e) {
      return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
              .contentType(MediaType.APPLICATION_PROBLEM_JSON)
              .body(Problem.builder()
                      .withStatus(Status.UNAUTHORIZED)
                      .withTitle("Unauthorized")
                      .withDetail(e.getMessage())
                      .build());
  }

this will give the proper response as you expected .

And for the 401 by default, this will give the same type as text/html (both the error and HTML error page are together)so get a proper response like this

@Component

public class MyAuthenticationEntryPoint implements AuthenticationEntryPoint {

@Override
public void commence(HttpServletRequest request, HttpServletResponse response,
                     AuthenticationException authException) throws IOException, ServletException {
    response.setContentType("application/json");
    response.setStatus(HttpStatus.UNAUTHORIZED.value());
    PrintWriter out = response.getWriter();
    out.println("{ \"error\": \"Unauthorized\", \"message\": \"" + authException.getMessage() + "\" }");
}

}

let me know if i get it right or wrong .

mhalbritter commented 12 months ago

Here's a reproducer: sb-33716.zip

Actually, this has nothing to do with problem details, it can also be reproduced with

curl -i http://localhost:8080/secret -u "user:password" -H "Accept: application/dummy+json"

And that's because org.springframework.http.converter.json.MappingJackson2HttpMessageConverter#MappingJackson2HttpMessageConverter(com.fasterxml.jackson.databind.ObjectMapper) is using application/*+json as the mime type it can write.

mhalbritter commented 12 months ago

After some digging, I don't think this is a bug in Spring Boot. This not only happens to the ErrorController, but to any controller.

This controller

@SpringBootApplication
@RestController
public class Application {

    @GetMapping(value = "/")
    public Map<String, Object> index() {
        return Map.of("a", 1, "b", 2);
    }

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

}

responds to

curl -i http://localhost:8080/ -H "Accept: application/problemdetails+json"

(really to any application/*+json request) with:

HTTP/1.1 200 
Content-Type: application/problemdetails+json
Transfer-Encoding: chunked
Date: Thu, 16 Nov 2023 10:51:48 GMT

{"a":1,"b":2}
bclozel commented 10 months ago

I think this issue is conflating several existing issues in Spring Framework and Spring Boot.

First, the fact that a regular JSON serialization can use the application/problemdetails+json media type, without serializing an actual ProblemDetails instance. This has been covered by spring-projects/spring-framework#29588 and is now solved.

When it comes to the broad mapping of the main JSON encoder with the application/*+json (as demonstrated by Moritz), this has been discussed and explained in spring-projects/spring-framework#26212. In summary, apps should register specific support for media types with the JSON encoder.

Now the remaining part of this issue is about the fact that common exceptions thrown by Spring libraries are not all mapped by Spring Framework and additional support is required in Spring Boot. To get the full support for the problem details RFC in the Spring Boot error handling, we also need to render such responses as HTML views - this will be done in spring-projects/spring-boot#19525. For that, we require content negotiation support for error rendering in Spring Framework and this is tracked by spring-projects/spring-framework#31936.

In summary, we acknowledge that the problem still exists and we're tracking a detailed plan to support that in several existing issues. I'm closing this issue as a result.