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 page is accessible when no credentials are provided #26356

Closed
rubensa opened this issue May 4, 2021 · 2 comments
Closed

Error page is accessible when no credentials are provided #26356

rubensa opened this issue May 4, 2021 · 2 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@rubensa
Copy link

rubensa commented May 4, 2021

There is an inconsistency in the default 401 Unauthorized response.
If client credentials are not provided:

  • 401 Unauthorized response is returned
  • The response includes JSON with the error (like with any exception exposed by /error endpoint)

If wrong client credentials are provided:

  • 401 Unauthorized response is returned
  • The response does not include JSON

Sample pom file:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <parent>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter-parent</artifactId>
    <version>2.4.5</version>
    <relativePath /> <!-- lookup parent from repository -->
  </parent>
  <groupId>org.eu.rubensa.springboot.error</groupId>
  <artifactId>springboot-error-test</artifactId>
  <version>0.0.1-SNAPSHOT</version>
  <name>springboot-error-test</name>
  <description>Project for testing Spring Boot error handling</description>
  <properties>
    <java.version>8</java.version>
  </properties>
  <dependencies>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-web</artifactId>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-security</artifactId>
    </dependency>

    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-test</artifactId>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>org.springframework.security</groupId>
      <artifactId>spring-security-test</artifactId>
      <scope>test</scope>
    </dependency>
  </dependencies>
</project>

Sample test class:

package org.eu.rubensa.springboot.error;

import com.fasterxml.jackson.databind.JsonNode;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

/**
 * The @SpringBootTest annotation will load the fully ApplicationContext. This
 * will not use slicing and scan for all the stereotype annotations
 * (@Component, @Service, @Respository and @Controller / @RestController) and
 * loads the full application context.
 */
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT,
    // From Spring 2.3.0 "server.error.include-message" and
    // "server.error.include-binding-errors" is set to "never"
    properties = { "server.error.include-message=always",
        /**
         * When you add the Security starter without custom security configurations,
         * Spring Boot endpoints will be secured using HTTP basic authentication with a
         * default user and generated password. To override that, you can configure
         * credentials in application.properties as follows
         */
        "spring.security.user.name=username", "spring.security.user.password=password" })
public class BadCredentialsInconsistencyTest {
  @Autowired
  private TestRestTemplate testRestTemplate;

  @Test
  public void testBadCredentials() throws Exception {
    final ResponseEntity<JsonNode> response = testRestTemplate.withBasicAuth("username", "wrongpassword")
        .exchange("/test", HttpMethod.GET, null, JsonNode.class);
    Assertions.assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
    JsonNode jsonResponse = response.getBody();
    /**
     * Since Spring Boot 2.0 the /error end-point is protected so the exception is
     * not "exposed" as JSON.
     * <p>
     * see: https://github.com/spring-projects/spring-security/issues/4467
     */
    Assertions.assertThat(jsonResponse).isNull();
  }

  @Test
  public void testNoCredentials() throws Exception {
    final ResponseEntity<JsonNode> response = testRestTemplate.exchange("/test", HttpMethod.GET, null, JsonNode.class);
    Assertions.assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
    JsonNode jsonResponse = response.getBody();
    /**
     * Since Spring Boot 2.0 the /error end-point is protected so the exception is
     * not "exposed" as JSON.
     * <p>
     * Curiously if no credentials provided the error page is reached to "expose"
     * the exception as JSON.
     * <p>
     * see: https://github.com/spring-projects/spring-security/issues/4467
     */
    Assertions.assertThat(jsonResponse.findValue("status").asInt()).isEqualTo(401);
    Assertions.assertThat(jsonResponse.findValue("error").asText()).isEqualTo("Unauthorized");
    Assertions.assertThat(jsonResponse.findValue("message").asText()).contains("Unauthorized");
    Assertions.assertThat(jsonResponse.findValue("path").asText()).isEqualTo("/test");
  }

  /**
   * A nested @Configuration class wild be used instead of the application’s
   * primary configuration.
   * <p>
   * Unlike a nested @Configuration class, which would be used instead of your
   * application’s primary configuration, a nested @TestConfiguration class is
   * used in addition to your application’s primary configuration.
   */
  @Configuration
  /**
   * Tells Spring Boot to start adding beans based on classpath settings, other
   * beans, and various property settings.
   */

  @EnableAutoConfiguration
  /**
   * The @ComponentScan tells Spring to look for other components, configurations,
   * and services in the the TestWebConfig package, letting it find the
   * TestController class.
   * <p>
   * We only want to test the classes defined inside this test configuration
   */
  static class TestConfig {
    @RestController
    public static class TestController {
      @GetMapping("/test")
      public void doNothing() {
      }
    }
  }
}

Spring Boot Version: 2.4.5

Related issues:

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 4, 2021
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 17, 2021
@philwebb philwebb added this to the 2.5.x milestone Nov 17, 2021
@mbhave mbhave changed the title 401 Unauthorized response inconsistency Error page is accessible when no credentials are provided Nov 18, 2021
@mbhave mbhave modified the milestones: 2.5.x, 2.6.0 Nov 18, 2021
@mbhave mbhave closed this as completed in dd1d148 Nov 18, 2021
@Selindek
Copy link

Selindek commented Dec 8, 2021

Is it possible that this patch broke my MultipartException handler? It worked perfectly in 2.5.7, but now the response body is null

@wilkinsona
Copy link
Member

@Selindek It's impossible to say without some more information. Are you using Spring Security in your application and, if so, how is it configured?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants