From 5367524030c7964b113bd648a079b430b05dd72b Mon Sep 17 00:00:00 2001 From: Marcus Da Coregio Date: Thu, 14 Apr 2022 16:30:42 -0300 Subject: [PATCH] Change the default of shouldFilterAllDispatchTypes to true Closes gh-11107 --- .../AuthorizeHttpRequestsConfigurer.java | 5 ++- .../authorize-http-requests.adoc | 8 ++--- .../access/intercept/AuthorizationFilter.java | 4 +-- .../intercept/AuthorizationFilterTests.java | 32 +++++++++---------- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java index 59ae9d669d..289a936847 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java @@ -118,7 +118,7 @@ public final class AuthorizeHttpRequestsConfigurer authorize - .shouldFilterAllDispatcherTypes(true) + .shouldFilterAllDispatcherTypes(false) .anyRequest.authenticated() ) // ... diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java b/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java index 8aadee3029..5d00d57292 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java @@ -50,7 +50,7 @@ public class AuthorizationFilter extends OncePerRequestFilter { private AuthorizationEventPublisher eventPublisher = AuthorizationFilter::noPublish; - private boolean shouldFilterAllDispatcherTypes = false; + private boolean shouldFilterAllDispatcherTypes = true; /** * Creates an instance. @@ -120,7 +120,7 @@ public class AuthorizationFilter extends OncePerRequestFilter { /** * Sets whether to filter all dispatcher types. * @param shouldFilterAllDispatcherTypes should filter all dispatcher types. Default - * is {@code false} + * is {@code true} * @since 5.7 */ public void setShouldFilterAllDispatcherTypes(boolean shouldFilterAllDispatcherTypes) { diff --git a/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java b/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java index c80528bbaf..c894432198 100644 --- a/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java @@ -167,7 +167,7 @@ public class AuthorizationFilterTests { } @Test - public void doFilterWhenErrorThenDoNotFilter() throws Exception { + public void doFilterWhenErrorThenDoFilter() throws Exception { AuthorizationManager authorizationManager = mock(AuthorizationManager.class); AuthorizationFilter authorizationFilter = new AuthorizationFilter(authorizationManager); MockHttpServletRequest mockRequest = new MockHttpServletRequest(null, "/path"); @@ -176,25 +176,25 @@ public class AuthorizationFilterTests { MockHttpServletResponse mockResponse = new MockHttpServletResponse(); FilterChain mockFilterChain = mock(FilterChain.class); + authorizationFilter.doFilter(mockRequest, mockResponse, mockFilterChain); + verify(authorizationManager).check(any(Supplier.class), eq(mockRequest)); + } + + @Test + public void doFilterWhenErrorAndShouldFilterAllDispatcherTypesFalseThenDoNotFilter() throws Exception { + AuthorizationManager authorizationManager = mock(AuthorizationManager.class); + AuthorizationFilter authorizationFilter = new AuthorizationFilter(authorizationManager); + authorizationFilter.setShouldFilterAllDispatcherTypes(false); + MockHttpServletRequest mockRequest = new MockHttpServletRequest(null, "/path"); + mockRequest.setDispatcherType(DispatcherType.ERROR); + mockRequest.setAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE, "/error"); + MockHttpServletResponse mockResponse = new MockHttpServletResponse(); + FilterChain mockFilterChain = mock(FilterChain.class); + authorizationFilter.doFilter(mockRequest, mockResponse, mockFilterChain); verifyNoInteractions(authorizationManager); } - @Test - public void doFilterWhenErrorAndShouldFilterAllDispatcherTypesThenFilter() throws Exception { - AuthorizationManager authorizationManager = mock(AuthorizationManager.class); - AuthorizationFilter authorizationFilter = new AuthorizationFilter(authorizationManager); - authorizationFilter.setShouldFilterAllDispatcherTypes(true); - MockHttpServletRequest mockRequest = new MockHttpServletRequest(null, "/path"); - mockRequest.setDispatcherType(DispatcherType.ERROR); - mockRequest.setAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE, "/error"); - MockHttpServletResponse mockResponse = new MockHttpServletResponse(); - FilterChain mockFilterChain = mock(FilterChain.class); - - authorizationFilter.doFilter(mockRequest, mockResponse, mockFilterChain); - verify(authorizationManager).check(any(Supplier.class), any(HttpServletRequest.class)); - } - @Test public void doFilterNestedErrorDispatchWhenAuthorizationManagerThenUses() throws Exception { AuthorizationManager authorizationManager = mock(AuthorizationManager.class);