Skip to content

Error code returned by controller is not preserved #36948

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

Closed
filiphr opened this issue Aug 11, 2023 · 2 comments
Closed

Error code returned by controller is not preserved #36948

filiphr opened this issue Aug 11, 2023 · 2 comments
Labels
for: external-project For an external project and not something we can fix

Comments

@filiphr
Copy link
Contributor

filiphr commented Aug 11, 2023

Response status code is incorrect when using multiple SecurityFilterChain(s).

It seems like the changes done in cedd553 for removing the ErrorPageSecurityFilter have lead to the use of multiple security filter chains not working correctly.

This might be linked to spring-projects/spring-security#12771, and perhaps that is how Spring Security worked before. However, I do not agree that we need to add something additional to get the correct error code if we have configured it like that.

I have created a example repo with some tests and configuration.

The security configuration looks like:

@EnableWebSecurity
@Configuration
public class SecurityConfiguration {

    @Bean
    @Order(1)
    public SecurityFilterChain firstSecurityFilterChain(HttpSecurity http) throws Exception {
        return http.sessionManagement(sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
                .csrf(AbstractHttpConfigurer::disable)
                // Comment out line below for Spring Boot 2.7
                //.antMatcher("/ignored-api/*")
                // Comment line below for Spring Boot 2.7
                .securityMatcher(AntPathRequestMatcher.antMatcher("/ignored-api/*"))
                .authorizeHttpRequests(configurer -> configurer.anyRequest().denyAll())
                .httpBasic(Customizer.withDefaults())
                .build();
    }

    @Bean
    @Order(2)
    public SecurityFilterChain secondSecurityFilterChain(HttpSecurity http) throws Exception {
        return http.sessionManagement(sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
                .csrf(AbstractHttpConfigurer::disable)
                .exceptionHandling(exceptionHandling -> exceptionHandling.defaultAuthenticationEntryPointFor(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED),
                        AnyRequestMatcher.INSTANCE))
                .securityContext(securityContext -> securityContext.securityContextRepository(new NullSecurityContextRepository()))
                .authorizeHttpRequests(configurer -> configurer.anyRequest().authenticated())
                .httpBasic(Customizer.withDefaults())
                .build();
    }

}

My rest controller looks like:

@RestController
public class TestRestController {

    @GetMapping("/ignored-api/forbidden")
    public ResponseEntity<?> ignoredForbidden() {
        return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null);
    }

    @GetMapping("/ignored-api/ok")
    public ResponseEntity<?> ignoredOk() {
        return ResponseEntity.status(HttpStatus.OK).body(null);
    }

    @GetMapping("/allowed-api/forbidden")
    public ResponseEntity<?> allowedForbidden() {
        return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null);
    }

    @GetMapping("/allowed-api/ok")
    public ResponseEntity<?> allowedOk() {
        return ResponseEntity.status(HttpStatus.OK).body(null);
    }
}

and the tests look like:

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
public class TestRestControllerTest {

    @Autowired
    protected TestRestTemplate restTemplate;

    @Test
    void ignoredForbiddenNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/ignored-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void ignoredForbiddenAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/ignored-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void ignoredOkNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/ignored-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void ignoredOkAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/ignored-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void ignoredUnknownNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/ignored-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void ignoredUnknownAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/ignored-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void allowedForbiddenNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/allowed-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void allowedForbiddenAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/allowed-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void allowedOkNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/allowed-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void allowedOkAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/allowed-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.OK);
    }

    @Test
    void allowedUnknownNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/allowed-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void allowedUnknownAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/allowed-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.NOT_FOUND);
    }
}

In Spring Boot 2.7 all the tests are green. With Spring Boot 3.1 the following ones are failing:

  • ignoredUnknownAuthenticated - in 2.7 the status code is HTTP 403, with 3.1 it is HTTP 401
  • ignoredForbiddenAuthenticated - in 2.7 the status code is HTTP 403, with 3.1 it is HTTP 401
  • ignoredOkAuthenticated - in 2.7 the status code is HTTP 403, with 3.1 it is HTTP 401
  • allowedUnknownAuthenticated - in 2.7 the status code is HTTP 404, with 3.1 it is HTTP 401
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 11, 2023
@philwebb
Copy link
Member

I'm afraid the comment I added to #33341 also applies here. We're really not keen to start deviating from Spring Security defaults. Please add a comment to spring-projects/spring-security#12771 so that they have another data point for the problem.

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Aug 13, 2023
@philwebb philwebb added for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 13, 2023
@filiphr
Copy link
Contributor Author

filiphr commented Aug 14, 2023

Thanks for checking @philwebb. I'll add a comment to the Spring Security issue. It is slightly different than error pages, but it isn't much logical to me what Spring Security is doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix
Projects
None yet
Development

No branches or pull requests

3 participants