Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error response body does not match Content-Type #33716

Closed
osiegmar opened this issue Jan 7, 2023 · 6 comments
Closed

Error response body does not match Content-Type #33716

osiegmar opened this issue Jan 7, 2023 · 6 comments
Labels
status: duplicate A duplicate of another issue

Comments

@osiegmar
Copy link

osiegmar commented Jan 7, 2023

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 spring-projects/spring-framework#29588

To Reproduce

  • Setup an application with
    • basic auth configuration and
    • an endpoint that needs specific privileges (e.g. @Secured("ROLE_ADMIN"))
  • Send a request to that endpoint
    • with a valid user/auth but insufficient privileges and
    • specify a request header Accept: application/problem+json, application/json

Expected behavior

  • The Content-Type response header must reflect the actual type of the content
  • When Problem Details are enabled, I'd expect that all errors (including 403 Forbidden) are returned as a Problem Detail response (RFC 7807). Also note that 401 Unauthorized does not contain a response body at all – I don't know if this is intended or another bug.

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 spring-projects/spring-security#12450 but learned that is related to Spring Boot, not Spring Security.

@MaksimMyshkin
Copy link

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
Copy link
Contributor

mbhave commented Apr 6, 2023

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
Copy link

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 .

@philwebb philwebb modified the milestones: 3.0.x, 3.1.x Nov 8, 2023
@mhalbritter
Copy link
Contributor

mhalbritter commented Nov 16, 2023

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
Copy link
Contributor

mhalbritter commented Nov 16, 2023

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}

@mhalbritter mhalbritter added the for: team-meeting An issue we'd like to discuss as a team to make progress label Nov 16, 2023
@bclozel
Copy link
Member

bclozel commented Jan 16, 2024

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 #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.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2024
@bclozel bclozel added status: duplicate A duplicate of another issue and removed type: bug A general bug for: team-meeting An issue we'd like to discuss as a team to make progress labels Jan 16, 2024
@bclozel bclozel removed this from the 3.1.x milestone Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

8 participants