diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java b/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java index 9e8bbad3c5..6ae2ce9a36 100644 --- a/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java +++ b/web/src/main/java/org/springframework/security/web/savedrequest/CookieRequestCache.java @@ -16,6 +16,7 @@ package org.springframework.security.web.savedrequest; +import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Collections; import java.util.Objects; @@ -63,7 +64,7 @@ public class CookieRequestCache implements RequestCache { this.logger.debug("Request not saved as configured RequestMatcher did not match"); return; } - String redirectUrl = UrlUtils.buildFullRequestUrl(request); + String redirectUrl = buildRelativeRequestUrl(request); Cookie savedCookie = new Cookie(COOKIE_NAME, encodeCookie(redirectUrl)); savedCookie.setMaxAge(COOKIE_MAX_AGE); savedCookie.setSecure(request.isSecure()); @@ -84,10 +85,14 @@ public class CookieRequestCache implements RequestCache { return null; } UriComponents uriComponents = UriComponentsBuilder.fromUriString(originalURI).build(); + if (!isRelativePath(originalURI)) { + this.logger.debug("Did not use saved request since cookie did not contain a relative path"); + return null; + } DefaultSavedRequest.Builder builder = new DefaultSavedRequest.Builder(); - int port = getPort(uriComponents); - return builder.setScheme(uriComponents.getScheme()) - .setServerName(uriComponents.getHost()) + int port = request.getServerPort(); + return builder.setScheme(request.getScheme()) + .setServerName(request.getServerName()) .setRequestURI(uriComponents.getPath()) .setQueryString(uriComponents.getQuery()) .setServerPort(port) @@ -97,15 +102,8 @@ public class CookieRequestCache implements RequestCache { .build(); } - private int getPort(UriComponents uriComponents) { - int port = uriComponents.getPort(); - if (port != -1) { - return port; - } - if ("https".equalsIgnoreCase(uriComponents.getScheme())) { - return 443; - } - return 80; + private boolean isRelativePath(String uri) { + return uri.startsWith("/") && !uri.startsWith("//"); } @Override @@ -130,13 +128,19 @@ public class CookieRequestCache implements RequestCache { response.addCookie(removeSavedRequestCookie); } + private static String buildRelativeRequestUrl(HttpServletRequest request) { + String uri = request.getRequestURI(); + String query = request.getQueryString(); + return (query != null) ? uri + "?" + query : uri; + } + private static String encodeCookie(String cookieValue) { - return Base64.getEncoder().encodeToString(cookieValue.getBytes()); + return Base64.getEncoder().encodeToString(cookieValue.getBytes(StandardCharsets.UTF_8)); } private @Nullable String decodeCookie(String encodedCookieValue) { try { - return new String(Base64.getDecoder().decode(encodedCookieValue.getBytes())); + return new String(Base64.getDecoder().decode(encodedCookieValue.getBytes(StandardCharsets.UTF_8))); } catch (IllegalArgumentException ex) { this.logger.debug("Failed decode cookie value " + encodedCookieValue); 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 f3d2ea915d..1f498ba30c 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 @@ -17,6 +17,7 @@ package org.springframework.security.web.server.savedrequest; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Base64; import java.util.Collections; @@ -97,8 +98,13 @@ public class CookieServerRequestCache implements ServerRequestCache { return Mono.justOrEmpty(cookieMap.getFirst(REDIRECT_URI_COOKIE_NAME)) .map(HttpCookie::getValue) .map(CookieServerRequestCache::decodeCookie) - .onErrorResume(IllegalArgumentException.class, (ex) -> Mono.empty()) - .map(URI::create); + .flatMap((decoded) -> isRelativePath(decoded) ? Mono.just(decoded) : Mono.empty()) + .map(URI::create) + .onErrorResume(IllegalArgumentException.class, (ex) -> Mono.empty()); + } + + private boolean isRelativePath(String uri) { + return uri.startsWith("/") && !uri.startsWith("//"); } @Override @@ -143,11 +149,13 @@ public class CookieServerRequestCache implements ServerRequestCache { } private static String encodeCookie(String cookieValue) { - return new String(Base64.getEncoder().encode(cookieValue.getBytes())); + return new String(Base64.getEncoder().encode(cookieValue.getBytes(StandardCharsets.UTF_8)), + StandardCharsets.UTF_8); } private static String decodeCookie(String encodedCookieValue) { - return new String(Base64.getDecoder().decode(encodedCookieValue.getBytes())); + return new String(Base64.getDecoder().decode(encodedCookieValue.getBytes(StandardCharsets.UTF_8)), + StandardCharsets.UTF_8); } private static ServerWebExchangeMatcher createDefaultRequestMatcher() { diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java index 8862e5ca94..2ea66283af 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java +++ b/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java @@ -54,7 +54,7 @@ public class CookieRequestCacheTests { Cookie savedCookie = response.getCookie(DEFAULT_COOKIE_NAME); assertThat(savedCookie).isNotNull(); String redirectUrl = decodeCookie(savedCookie.getValue()); - assertThat(redirectUrl).isEqualTo("https://abc.com/destination?param1=a¶m2=b¶m3=1122"); + assertThat(redirectUrl).isEqualTo("/destination?param1=a¶m2=b¶m3=1122"); assertThat(savedCookie.getMaxAge()).isEqualTo(-1); assertThat(savedCookie.getPath()).isEqualTo("/"); assertThat(savedCookie.isHttpOnly()).isTrue(); @@ -101,18 +101,53 @@ public class CookieRequestCacheTests { public void getRequestWhenRequestContainsSavedRequestCookieThenReturnsSaveRequest() { CookieRequestCache cookieRequestCache = new CookieRequestCache(); MockHttpServletRequest request = new MockHttpServletRequest(); - String redirectUrl = "https://abc.com/destination?param1=a¶m2=b¶m3=1122"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setScheme("https"); + request.setServerName("abc.com"); + request.setServerPort(443); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination?param1=a¶m2=b¶m3=1122"))); SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); assertThat(savedRequest).isNotNull(); - assertThat(savedRequest.getRedirectUrl()).isEqualTo(redirectUrl); + assertThat(savedRequest.getRedirectUrl()) + .isEqualTo("https://abc.com/destination?param1=a¶m2=b¶m3=1122"); + } + + @Test + public void getRequestWhenRelativePathInCookieThenRedirectUrlUsesCurrentRequestOrigin() { + CookieRequestCache cookieRequestCache = new CookieRequestCache(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setScheme("https"); + request.setServerName("myapp.com"); + request.setServerPort(443); + request.setSecure(true); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/secret?token=abc"))); + SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); + assertThat(savedRequest).isNotNull(); + assertThat(savedRequest.getRedirectUrl()).isEqualTo("https://myapp.com/secret?token=abc"); + } + + @Test + public void getRequestWhenAbsoluteUrlInCookieThenReturnsNull() { + CookieRequestCache cookieRequestCache = new CookieRequestCache(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("https://evil.com/phishing"))); + SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); + assertThat(savedRequest).isNull(); + } + + @Test + public void getRequestWhenProtocolRelativeUrlInCookieThenReturnsNull() { + CookieRequestCache cookieRequestCache = new CookieRequestCache(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("//evil.com/phishing"))); + SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); + assertThat(savedRequest).isNull(); } @Test public void getRequestWhenRequestContainsSavedRequestCookieThenSavedRequestContainsRequestParameters() { CookieRequestCache cookieRequestCache = new CookieRequestCache(); MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("https://abc.com/destination"))); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination"))); request.setParameter("single", "first"); request.addParameter("multi", "second"); request.addParameter("multi", "third"); @@ -143,8 +178,7 @@ public class CookieRequestCacheTests { request.setServerName("abc.com"); request.setRequestURI("/destination"); request.setQueryString("param1=a¶m2=b¶m3=1122"); - String redirectUrl = "https://abc.com/destination?param1=a¶m2=b¶m3=1122"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination?param1=a¶m2=b¶m3=1122"))); MockHttpServletResponse response = new MockHttpServletResponse(); cookieRequestCache.getMatchingRequest(request, response); Cookie expiredCookie = response.getCookie(DEFAULT_COOKIE_NAME); @@ -162,8 +196,7 @@ public class CookieRequestCacheTests { request.setScheme("https"); request.setServerName("abc.com"); request.setRequestURI("/destination"); - String redirectUrl = "https://abc.com/api"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/api"))); MockHttpServletResponse response = new MockHttpServletResponse(); final HttpServletRequest matchingRequest = cookieRequestCache.getMatchingRequest(request, response); assertThat(matchingRequest).isNull(); @@ -182,8 +215,8 @@ public class CookieRequestCacheTests { request.setRequestURI("/destination"); request.setQueryString("goto=https%3A%2F%2Fstart.spring.io"); request.setParameter("goto", "https://start.spring.io"); - String redirectUrl = "https://abc.com/destination?goto=https%3A%2F%2Fstart.spring.io"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setCookies( + new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination?goto=https%3A%2F%2Fstart.spring.io"))); MockHttpServletResponse response = new MockHttpServletResponse(); final HttpServletRequest matchingRequest = cookieRequestCache.getMatchingRequest(request, response); assertThat(matchingRequest).isNotNull(); @@ -212,7 +245,7 @@ public class CookieRequestCacheTests { request.setServerName("example.com"); request.setRequestURI("/destination"); request.setPreferredLocales(Arrays.asList(Locale.FRENCH, Locale.GERMANY)); - String redirectUrl = "https://example.com/destination"; + String redirectUrl = "/destination"; request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); MockHttpServletResponse response = new MockHttpServletResponse(); HttpServletRequest matchingRequest = cookieRequestCache.getMatchingRequest(request, response); diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java index a5416bb847..8f88af7a29 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java @@ -16,8 +16,6 @@ package org.springframework.security.web.savedrequest; -import java.util.Base64; - import jakarta.servlet.http.Cookie; import org.junit.jupiter.api.Test; @@ -53,14 +51,15 @@ public class RequestCacheAwareFilterTests { request.setScheme("https"); request.setServerPort(443); request.setSecure(true); - String encodedRedirectUrl = Base64.getEncoder().encodeToString("https://abc.com/destination".getBytes()); - Cookie savedRequest = new Cookie("REDIRECT_URI", encodedRedirectUrl); + MockHttpServletResponse response = new MockHttpServletResponse(); + cache.saveRequest(request, response); + Cookie savedRequest = response.getCookie("REDIRECT_URI"); savedRequest.setMaxAge(-1); savedRequest.setSecure(request.isSecure()); savedRequest.setPath("/"); savedRequest.setHttpOnly(true); request.setCookies(savedRequest); - MockHttpServletResponse response = new MockHttpServletResponse(); + response = new MockHttpServletResponse(); filter.doFilter(request, response, new MockFilterChain()); Cookie expiredCookie = response.getCookie("REDIRECT_URI"); assertThat(expiredCookie).isNotNull(); 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 9dee46fe0b..1c3808d176 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 @@ -117,6 +117,26 @@ public class CookieServerRequestCacheTests { assertThat(redirectUri).isNull(); } + @Test + public void getRedirectUriWhenCookieContainsAbsoluteUrlThenRedirectUriIsNull() { + String encodedAbsoluteUrl = Base64.getEncoder().encodeToString("https://evil.com/phishing".getBytes()); + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/login") + .accept(MediaType.TEXT_HTML) + .cookie(new HttpCookie("REDIRECT_URI", encodedAbsoluteUrl))); + URI redirectUri = this.cache.getRedirectUri(exchange).block(); + assertThat(redirectUri).isNull(); + } + + @Test + public void getRedirectUriWhenCookieContainsProtocolRelativeUrlThenRedirectUriIsNull() { + String encodedProtocolRelativeUrl = Base64.getEncoder().encodeToString("//evil.com/phishing".getBytes()); + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/login") + .accept(MediaType.TEXT_HTML) + .cookie(new HttpCookie("REDIRECT_URI", encodedProtocolRelativeUrl))); + URI redirectUri = this.cache.getRedirectUri(exchange).block(); + assertThat(redirectUri).isNull(); + } + @Test public void getRedirectUriWhenNoCookieThenRedirectUriIsNull() { MockServerWebExchange exchange = MockServerWebExchange