From 0591403dea8bbd75b3856684c8aec1bfb7e648e1 Mon Sep 17 00:00:00 2001 From: Ahmed Sayed Date: Wed, 31 Jul 2019 12:17:55 +0200 Subject: [PATCH 1/3] ignore Multipart requests in HttpSessionRequestCache.requestMatcher --- .../configurers/RequestCacheConfigurer.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java index 00045baba6..ffcedad60b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java @@ -142,22 +142,12 @@ public final class RequestCacheConfigurer> exte return null; } } + @SuppressWarnings("unchecked") private RequestMatcher createDefaultSavedRequestMatcher(H http) { - ContentNegotiationStrategy contentNegotiationStrategy = http - .getSharedObject(ContentNegotiationStrategy.class); - if (contentNegotiationStrategy == null) { - contentNegotiationStrategy = new HeaderContentNegotiationStrategy(); - } - RequestMatcher notFavIcon = new NegatedRequestMatcher(new AntPathRequestMatcher( "/**/favicon.*")); - MediaTypeRequestMatcher jsonRequest = new MediaTypeRequestMatcher( - contentNegotiationStrategy, MediaType.APPLICATION_JSON); - jsonRequest.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); - RequestMatcher notJson = new NegatedRequestMatcher(jsonRequest); - RequestMatcher notXRequestedWith = new NegatedRequestMatcher( new RequestHeaderRequestMatcher("X-Requested-With", "XMLHttpRequest")); @@ -169,9 +159,21 @@ public final class RequestCacheConfigurer> exte matchers.add(0, getRequests); } matchers.add(notFavIcon); - matchers.add(notJson); + matchers.add(notMatchingMediaType(http, MediaType.APPLICATION_JSON)); matchers.add(notXRequestedWith); + matchers.add(notMatchingMediaType(http, MediaType.MULTIPART_FORM_DATA)); return new AndRequestMatcher(matchers); } + + private RequestMatcher notMatchingMediaType(H http, MediaType mediaType) { + ContentNegotiationStrategy contentNegotiationStrategy = http.getSharedObject(ContentNegotiationStrategy.class); + if (contentNegotiationStrategy == null) { + contentNegotiationStrategy = new HeaderContentNegotiationStrategy(); + } + + MediaTypeRequestMatcher jsonRequest = new MediaTypeRequestMatcher(contentNegotiationStrategy, mediaType); + jsonRequest.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); + return new NegatedRequestMatcher(jsonRequest); + } } From 1ab05dae025563686dd62d9e29774807bbfce41a Mon Sep 17 00:00:00 2001 From: Ahmed Sayed Date: Wed, 14 Aug 2019 21:35:34 +0200 Subject: [PATCH 2/3] added test --- .../RequestCacheConfigurerTests.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java index 683a4951c2..a193f1c887 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java @@ -26,6 +26,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpSession; +import org.springframework.mock.web.MockMultipartFile; import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -45,6 +46,7 @@ import static org.mockito.Mockito.verify; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; /** @@ -262,6 +264,21 @@ public class RequestCacheConfigurerTests { .andExpect(redirectedUrl("/")); } + // SEC-7060 + @Test + public void postWhenRequestIsMultipartThenPostAuthenticationRedirectsToRoot() throws Exception { + this.spring.register(RequestCacheDefaultsConfig.class, DefaultSecurityConfig.class).autowire(); + + MockMultipartFile aFile = new MockMultipartFile("aFile", "A_FILE".getBytes()); + + MockHttpSession session = (MockHttpSession) + this.mvc.perform(multipart("/upload") + .file(aFile)) + .andReturn().getRequest().getSession(); + + this.mvc.perform(formLogin(session)).andExpect(redirectedUrl("/")); + } + @EnableWebSecurity static class RequestCacheDisabledConfig extends WebSecurityConfigurerAdapter { @Override From 08ea2348d6744534d86c8a0493bdae50916c4e72 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 15 Aug 2019 09:20:45 -0500 Subject: [PATCH 3/3] Polish RequestCache ignores multipart requests --- .../annotation/web/configurers/RequestCacheConfigurer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java index ffcedad60b..3a21c2d927 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java @@ -172,8 +172,8 @@ public final class RequestCacheConfigurer> exte contentNegotiationStrategy = new HeaderContentNegotiationStrategy(); } - MediaTypeRequestMatcher jsonRequest = new MediaTypeRequestMatcher(contentNegotiationStrategy, mediaType); - jsonRequest.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); - return new NegatedRequestMatcher(jsonRequest); + MediaTypeRequestMatcher mediaRequest = new MediaTypeRequestMatcher(contentNegotiationStrategy, mediaType); + mediaRequest.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); + return new NegatedRequestMatcher(mediaRequest); } }