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 e280efc85c..86c8f21dd3 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 @@ -32,6 +32,7 @@ import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.MediaTypeRequestMatcher; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; +import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.accept.ContentNegotiationStrategy; @@ -140,7 +141,7 @@ public final class RequestCacheConfigurer> @SuppressWarnings("unchecked") private RequestMatcher createDefaultSavedRequestMatcher(H http) { - RequestMatcher notFavIcon = new NegatedRequestMatcher(getFaviconRequestMatcher()); + RequestMatcher notIgnoredBackgroundRequest = new NegatedRequestMatcher(getIgnoredBackgroundRequestMatcher()); RequestMatcher notXRequestedWith = new NegatedRequestMatcher( new RequestHeaderRequestMatcher("X-Requested-With", "XMLHttpRequest")); RequestMatcher notWebSocket = new NegatedRequestMatcher( @@ -152,7 +153,7 @@ public final class RequestCacheConfigurer> RequestMatcher getRequests = getRequestMatcherBuilder().matcher(HttpMethod.GET, "/**"); matchers.add(0, getRequests); } - matchers.add(notFavIcon); + matchers.add(notIgnoredBackgroundRequest); matchers.add(notMatchingMediaType(http, MediaType.APPLICATION_JSON)); matchers.add(notXRequestedWith); matchers.add(notMatchingMediaType(http, MediaType.MULTIPART_FORM_DATA)); @@ -171,8 +172,24 @@ public final class RequestCacheConfigurer> return new NegatedRequestMatcher(mediaRequest); } - private RequestMatcher getFaviconRequestMatcher() { - return getRequestMatcherBuilder().matcher("/favicon.*"); + /** + * Path patterns for requests that browsers request automatically in the background + * (e.g. to display an icon), and that should therefore never be saved as the URL to + * redirect to after authentication succeeds. + */ + private static final String[] IGNORED_BACKGROUND_REQUEST_PATTERNS = { "/favicon.*", + // Safari/WebKit request apple-touch-icon.png, + // apple-touch-icon-precomposed.png, + // and sized variants (e.g. apple-touch-icon-152x152.png) even without a + // matching tag in the page + "/apple-touch-icon*.png" }; + + private RequestMatcher getIgnoredBackgroundRequestMatcher() { + List matchers = new ArrayList<>(); + for (String pattern : IGNORED_BACKGROUND_REQUEST_PATTERNS) { + matchers.add(getRequestMatcherBuilder().matcher(pattern)); + } + return new OrRequestMatcher(matchers); } } 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 cce83b88b5..2aa5b10be8 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 @@ -114,6 +114,50 @@ public class RequestCacheConfigurerTests { this.mvc.perform(formLogin(session)).andExpect(redirectedUrl("/")); } + @Test + public void getWhenBookmarkedUrlIsAppleTouchIconThenPostAuthenticationRedirectsToRoot() throws Exception { + this.spring.register(RequestCacheDefaultsConfig.class, DefaultSecurityConfig.class).autowire(); + // @formatter:off + MockHttpSession session = (MockHttpSession) this.mvc.perform(get("/apple-touch-icon.png")) + .andExpect(redirectedUrl("/login")) + .andReturn() + .getRequest() + .getSession(); + // @formatter:on + // ignores apple-touch-icon.png + this.mvc.perform(formLogin(session)).andExpect(redirectedUrl("/")); + } + + @Test + public void getWhenBookmarkedUrlIsAppleTouchIconPrecomposedThenPostAuthenticationRedirectsToRoot() + throws Exception { + this.spring.register(RequestCacheDefaultsConfig.class, DefaultSecurityConfig.class).autowire(); + // @formatter:off + MockHttpSession session = (MockHttpSession) this.mvc.perform(get("/apple-touch-icon-precomposed.png")) + .andExpect(redirectedUrl("/login")) + .andReturn() + .getRequest() + .getSession(); + // @formatter:on + // ignores apple-touch-icon-precomposed.png + this.mvc.perform(formLogin(session)).andExpect(redirectedUrl("/")); + } + + @Test + public void getWhenBookmarkedUrlIsAppleTouchIconSizedPrecomposedThenPostAuthenticationRedirectsToRoot() + throws Exception { + this.spring.register(RequestCacheDefaultsConfig.class, DefaultSecurityConfig.class).autowire(); + // @formatter:off + MockHttpSession session = (MockHttpSession) this.mvc.perform(get("/apple-touch-icon-152x152-precomposed.png")) + .andExpect(redirectedUrl("/login")) + .andReturn() + .getRequest() + .getSession(); + // @formatter:on + // ignores apple-touch-icon-152x152-precomposed.png + this.mvc.perform(formLogin(session)).andExpect(redirectedUrl("/")); + } + // SEC-2321 @Test public void getWhenBookmarkedRequestIsApplicationJsonThenPostAuthenticationRedirectsToRoot() throws Exception { diff --git a/web/src/main/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCache.java b/web/src/main/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCache.java index 1f498ba30c..9005930970 100644 --- a/web/src/main/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCache.java +++ b/web/src/main/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCache.java @@ -158,13 +158,25 @@ public class CookieServerRequestCache implements ServerRequestCache { StandardCharsets.UTF_8); } + /** + * Path patterns for requests that browsers request automatically in the background + * (e.g. to display an icon), and that should therefore never be saved as the URL to + * redirect to after authentication succeeds. + */ + private static final String[] IGNORED_BACKGROUND_REQUEST_PATTERNS = { "/favicon.*", + // Safari/WebKit request apple-touch-icon.png, + // apple-touch-icon-precomposed.png, + // and sized variants (e.g. apple-touch-icon-152x152.png) even without a + // matching tag in the page + "/apple-touch-icon*.png" }; + private static ServerWebExchangeMatcher createDefaultRequestMatcher() { ServerWebExchangeMatcher get = ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, "/**"); - ServerWebExchangeMatcher notFavicon = new NegatedServerWebExchangeMatcher( - ServerWebExchangeMatchers.pathMatchers("/favicon.*")); + ServerWebExchangeMatcher notIgnoredBackgroundRequest = new NegatedServerWebExchangeMatcher( + ServerWebExchangeMatchers.pathMatchers(IGNORED_BACKGROUND_REQUEST_PATTERNS)); MediaTypeServerWebExchangeMatcher html = new MediaTypeServerWebExchangeMatcher(MediaType.TEXT_HTML); html.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); - return new AndServerWebExchangeMatcher(get, notFavicon, html); + return new AndServerWebExchangeMatcher(get, notIgnoredBackgroundRequest, html); } } diff --git a/web/src/main/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCache.java b/web/src/main/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCache.java index 723e20dcdf..1b13bc1d6b 100644 --- a/web/src/main/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCache.java +++ b/web/src/main/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCache.java @@ -158,13 +158,25 @@ public class WebSessionServerRequestCache implements ServerRequestCache { // @formatter:on } + /** + * Path patterns for requests that browsers request automatically in the background + * (e.g. to display an icon), and that should therefore never be saved as the URL to + * redirect to after authentication succeeds. + */ + private static final String[] IGNORED_BACKGROUND_REQUEST_PATTERNS = { "/favicon.*", + // Safari/WebKit request apple-touch-icon.png, + // apple-touch-icon-precomposed.png, + // and sized variants (e.g. apple-touch-icon-152x152.png) even without a + // matching tag in the page + "/apple-touch-icon*.png" }; + private static ServerWebExchangeMatcher createDefaultRequestMatcher() { ServerWebExchangeMatcher get = ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, "/**"); - ServerWebExchangeMatcher notFavicon = new NegatedServerWebExchangeMatcher( - ServerWebExchangeMatchers.pathMatchers("/favicon.*")); + ServerWebExchangeMatcher notIgnoredBackgroundRequest = new NegatedServerWebExchangeMatcher( + ServerWebExchangeMatchers.pathMatchers(IGNORED_BACKGROUND_REQUEST_PATTERNS)); MediaTypeServerWebExchangeMatcher html = new MediaTypeServerWebExchangeMatcher(MediaType.TEXT_HTML); html.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL)); - return new AndServerWebExchangeMatcher(get, notFavicon, html); + return new AndServerWebExchangeMatcher(get, notIgnoredBackgroundRequest, html); } } diff --git a/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java b/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java index 1c3808d176..4c17c669c5 100644 --- a/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java +++ b/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java @@ -77,6 +77,33 @@ public class CookieServerRequestCacheTests { assertThat(cookies).isEmpty(); } + @Test + public void saveRequestWhenGetRequestAppleTouchIconThenNoCookie() { + MockServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/apple-touch-icon.png").accept(MediaType.TEXT_HTML)); + this.cache.saveRequest(exchange).block(); + MultiValueMap cookies = exchange.getResponse().getCookies(); + assertThat(cookies).isEmpty(); + } + + @Test + public void saveRequestWhenGetRequestAppleTouchIconPrecomposedThenNoCookie() { + MockServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/apple-touch-icon-precomposed.png").accept(MediaType.TEXT_HTML)); + this.cache.saveRequest(exchange).block(); + MultiValueMap cookies = exchange.getResponse().getCookies(); + assertThat(cookies).isEmpty(); + } + + @Test + public void saveRequestWhenGetRequestAppleTouchIconSizedPrecomposedThenNoCookie() { + MockServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/apple-touch-icon-152x152-precomposed.png").accept(MediaType.TEXT_HTML)); + this.cache.saveRequest(exchange).block(); + MultiValueMap cookies = exchange.getResponse().getCookies(); + assertThat(cookies).isEmpty(); + } + @Test public void saveRequestWhenPostRequestThenNoCookie() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.post("/secured/")); diff --git a/web/src/test/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCacheTests.java b/web/src/test/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCacheTests.java index d383cde5fe..d1f6e647ca 100644 --- a/web/src/test/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCacheTests.java +++ b/web/src/test/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCacheTests.java @@ -68,6 +68,33 @@ public class WebSessionServerRequestCacheTests { assertThat(saved).isNull(); } + @Test + public void saveRequestGetRequestWhenAppleTouchIconThenNotFound() { + MockServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/apple-touch-icon.png").accept(MediaType.TEXT_HTML)); + this.cache.saveRequest(exchange).block(); + URI saved = this.cache.getRedirectUri(exchange).block(); + assertThat(saved).isNull(); + } + + @Test + public void saveRequestGetRequestWhenAppleTouchIconPrecomposedThenNotFound() { + MockServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/apple-touch-icon-precomposed.png").accept(MediaType.TEXT_HTML)); + this.cache.saveRequest(exchange).block(); + URI saved = this.cache.getRedirectUri(exchange).block(); + assertThat(saved).isNull(); + } + + @Test + public void saveRequestGetRequestWhenAppleTouchIconSizedPrecomposedThenNotFound() { + MockServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/apple-touch-icon-152x152-precomposed.png").accept(MediaType.TEXT_HTML)); + this.cache.saveRequest(exchange).block(); + URI saved = this.cache.getRedirectUri(exchange).block(); + assertThat(saved).isNull(); + } + @Test public void saveRequestGetRequestWhenPostThenNotFound() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.post("/secured/"));