From f34ea188e29cafd6805236de21314905f1c03e0e Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 12 May 2022 10:32:13 -0500 Subject: [PATCH] RequestRejectedException is 400 by Default Closes gh-7568 --- .../configurers/NamespaceHttpFirewallTests.java | 17 +++++++---------- .../HttpPathParameterStrippingTests.java | 16 ++++++++-------- .../security/web/FilterChainProxy.java | 4 ++-- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/NamespaceHttpFirewallTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/NamespaceHttpFirewallTests.java index 8d328b33fbe..cba9b848b05 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/NamespaceHttpFirewallTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/NamespaceHttpFirewallTests.java @@ -33,8 +33,8 @@ import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.web.servlet.MockMvc; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; /** * Tests to verify that all the functionality of <http-firewall> attributes is @@ -52,24 +52,21 @@ public class NamespaceHttpFirewallTests { MockMvc mvc; @Test - public void requestWhenPathContainsDoubleDotsThenBehaviorMatchesNamespace() { + public void requestWhenPathContainsDoubleDotsThenBehaviorMatchesNamespace() throws Exception { this.rule.register(HttpFirewallConfig.class).autowire(); - assertThatExceptionOfType(RequestRejectedException.class) - .isThrownBy(() -> this.mvc.perform(get("/public/../private/"))); + this.mvc.perform(get("/public/../private/")).andExpect(status().isBadRequest()); } @Test - public void requestWithCustomFirewallThenBehaviorMatchesNamespace() { + public void requestWithCustomFirewallThenBehaviorMatchesNamespace() throws Exception { this.rule.register(CustomHttpFirewallConfig.class).autowire(); - assertThatExceptionOfType(RequestRejectedException.class) - .isThrownBy(() -> this.mvc.perform(get("/").param("deny", "true"))); + this.mvc.perform(get("/").param("deny", "true")).andExpect(status().isBadRequest()); } @Test - public void requestWithCustomFirewallBeanThenBehaviorMatchesNamespace() { + public void requestWithCustomFirewallBeanThenBehaviorMatchesNamespace() throws Exception { this.rule.register(CustomHttpFirewallBeanConfig.class).autowire(); - assertThatExceptionOfType(RequestRejectedException.class) - .isThrownBy(() -> this.mvc.perform(get("/").param("deny", "true"))); + this.mvc.perform(get("/").param("deny", "true")).andExpect(status().isBadRequest()); } @EnableWebSecurity diff --git a/itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java b/itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java index cd7c64556aa..73e672df3dc 100644 --- a/itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java +++ b/itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java @@ -21,6 +21,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -29,11 +30,10 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; -import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThat; @ContextConfiguration(locations = { "/http-path-param-stripping-app-context.xml" }) @ExtendWith(SpringExtension.class) @@ -48,8 +48,8 @@ public void securedFilterChainCannotBeBypassedByAddingPathParameters() throws Ex request.setPathInfo("/secured;x=y/admin.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); - assertThatExceptionOfType(RequestRejectedException.class) - .isThrownBy(() -> this.fcp.doFilter(request, response, new MockFilterChain())); + this.fcp.doFilter(request, response, new MockFilterChain()); + assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); } @Test @@ -58,8 +58,8 @@ public void adminFilePatternCannotBeBypassedByAddingPathParameters() throws Exce request.setServletPath("/secured/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); - assertThatExceptionOfType(RequestRejectedException.class) - .isThrownBy(() -> this.fcp.doFilter(request, response, new MockFilterChain())); + this.fcp.doFilter(request, response, new MockFilterChain()); + assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); } @Test @@ -69,8 +69,8 @@ public void adminFilePatternCannotBeBypassedByAddingPathParametersWithPathInfo() request.setPathInfo("/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); - assertThatExceptionOfType(RequestRejectedException.class) - .isThrownBy(() -> this.fcp.doFilter(request, response, new MockFilterChain())); + this.fcp.doFilter(request, response, new MockFilterChain()); + assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); } public HttpSession createAuthenticatedSession(String... roles) { diff --git a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java index b4f249f129a..fe80b492cbf 100644 --- a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java +++ b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java @@ -33,9 +33,9 @@ import org.springframework.core.log.LogMessage; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.web.firewall.DefaultRequestRejectedHandler; import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.HttpStatusRequestRejectedHandler; import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.security.web.firewall.RequestRejectedHandler; import org.springframework.security.web.firewall.StrictHttpFirewall; @@ -151,7 +151,7 @@ public class FilterChainProxy extends GenericFilterBean { private HttpFirewall firewall = new StrictHttpFirewall(); - private RequestRejectedHandler requestRejectedHandler = new DefaultRequestRejectedHandler(); + private RequestRejectedHandler requestRejectedHandler = new HttpStatusRequestRejectedHandler(); public FilterChainProxy() { }