From 3b5d19c8a420b622cddd4a9a3a3dadbd9aaea257 Mon Sep 17 00:00:00 2001 From: Marcus Da Coregio Date: Mon, 7 Nov 2022 13:43:26 -0300 Subject: [PATCH] Adapt to Servlet API 6 changes and support Jakarta WebSocket 2.1 Closes gh-12146 Closes gh-12148 --- config/spring-security-config.gradle | 2 + .../SessionManagementConfigurerTests.java | 6 -- .../security/config/http/HttpConfigTests.java | 12 +--- .../config/http/MiscHttpConfigTests.java | 14 ----- .../http/SessionManagementConfigTests.java | 12 +--- .../spring-security-dependencies.gradle | 8 ++- ...ContextOnUpdateOrErrorResponseWrapper.java | 16 ----- .../web/jackson2/CookieDeserializer.java | 5 +- .../web/session/DisableEncodeUrlFilter.java | 10 ---- .../AbstractRememberMeServicesTests.java | 22 ------- ...SessionSecurityContextRepositoryTests.java | 14 ----- .../web/firewall/FirewalledResponseTests.java | 10 +--- .../web/jackson2/CookieMixinTests.java | 60 ++++++++++++++----- .../session/DisableEncodeUrlFilterTests.java | 10 ---- 14 files changed, 59 insertions(+), 142 deletions(-) diff --git a/config/spring-security-config.gradle b/config/spring-security-config.gradle index 61b6e3a319..36109d574e 100644 --- a/config/spring-security-config.gradle +++ b/config/spring-security-config.gradle @@ -63,6 +63,8 @@ dependencies { testImplementation 'io.rsocket:rsocket-transport-netty' testImplementation 'jakarta.annotation:jakarta.annotation-api' testImplementation 'jakarta.xml.bind:jakarta.xml.bind-api' + testImplementation 'jakarta.websocket:jakarta.websocket-api' + testImplementation 'jakarta.websocket:jakarta.websocket-client-api' testImplementation 'ldapsdk:ldapsdk:4.1' testImplementation('net.sourceforge.htmlunit:htmlunit') { exclude group: 'commons-logging', module: 'commons-logging' diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerTests.java index 8c3cc95285..c4564441db 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerTests.java @@ -328,9 +328,7 @@ public class SessionManagementConfigurerTests { HttpServletResponse responseToSpy = spy((HttpServletResponse) response); chain.doFilter(request, responseToSpy); verify(responseToSpy, atLeastOnce()).encodeRedirectURL(any()); - verify(responseToSpy, atLeastOnce()).encodeRedirectUrl(any()); verify(responseToSpy, atLeastOnce()).encodeURL(any()); - verify(responseToSpy, atLeastOnce()).encodeUrl(any()); }) .apply(springSecurity()) .build(); @@ -348,9 +346,7 @@ public class SessionManagementConfigurerTests { HttpServletResponse responseToSpy = spy((HttpServletResponse) response); chain.doFilter(request, responseToSpy); verify(responseToSpy, never()).encodeRedirectURL(any()); - verify(responseToSpy, never()).encodeRedirectUrl(any()); verify(responseToSpy, never()).encodeURL(any()); - verify(responseToSpy, never()).encodeUrl(any()); }) .apply(springSecurity()) .build(); @@ -807,9 +803,7 @@ public class SessionManagementConfigurerTests { @RequestMapping("/") String encoded(HttpServletResponse response) { response.encodeURL("/foo"); - response.encodeUrl("/foo"); response.encodeRedirectURL("/foo"); - response.encodeRedirectUrl("/foo"); return "encoded"; } diff --git a/config/src/test/java/org/springframework/security/config/http/HttpConfigTests.java b/config/src/test/java/org/springframework/security/config/http/HttpConfigTests.java index 9056fb6a77..d398366d62 100644 --- a/config/src/test/java/org/springframework/security/config/http/HttpConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/HttpConfigTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -149,16 +149,6 @@ public class HttpConfigTests { throw new RuntimeException("Unexpected invocation of encodeURL"); } - @Override - public String encodeUrl(String url) { - throw new RuntimeException("Unexpected invocation of encodeURL"); - } - - @Override - public String encodeRedirectUrl(String url) { - throw new RuntimeException("Unexpected invocation of encodeURL"); - } - } public static final class MockObservationRegistry implements FactoryBean { diff --git a/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java b/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java index c8b36a0b01..abe13893c0 100644 --- a/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java @@ -578,16 +578,12 @@ public class MiscHttpConfigTests { FilterChainProxy proxy = this.spring.getContext().getBean(FilterChainProxy.class); proxy.doFilter(request, responseToSpy, (req, resp) -> { HttpServletResponse httpResponse = (HttpServletResponse) resp; - httpResponse.encodeUrl("/"); httpResponse.encodeURL("/"); - httpResponse.encodeRedirectUrl("/"); httpResponse.encodeRedirectURL("/"); httpResponse.getWriter().write("encodeRedirect"); }); verify(responseToSpy, never()).encodeRedirectURL(any()); - verify(responseToSpy, never()).encodeRedirectUrl(any()); verify(responseToSpy, never()).encodeURL(any()); - verify(responseToSpy, never()).encodeUrl(any()); assertThat(responseToSpy.getContentAsString()).isEqualTo("encodeRedirect"); } @@ -1028,16 +1024,6 @@ public class MiscHttpConfigTests { throw new RuntimeException("Unexpected invocation of encodeURL"); } - @Override - public String encodeUrl(String url) { - throw new RuntimeException("Unexpected invocation of encodeURL"); - } - - @Override - public String encodeRedirectUrl(String url) { - throw new RuntimeException("Unexpected invocation of encodeURL"); - } - } } diff --git a/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigTests.java b/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigTests.java index d884a0b64f..4d83042079 100644 --- a/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/SessionManagementConfigTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -620,16 +620,6 @@ public class SessionManagementConfigTests { throw new RuntimeException("Unexpected invocation of encodeURL"); } - @Override - public String encodeUrl(String url) { - throw new RuntimeException("Unexpected invocation of encodeURL"); - } - - @Override - public String encodeRedirectUrl(String url) { - throw new RuntimeException("Unexpected invocation of encodeURL"); - } - } } diff --git a/dependencies/spring-security-dependencies.gradle b/dependencies/spring-security-dependencies.gradle index 3157bf6776..a79873d50e 100644 --- a/dependencies/spring-security-dependencies.gradle +++ b/dependencies/spring-security-dependencies.gradle @@ -29,11 +29,13 @@ dependencies { api "io.micrometer:micrometer-observation:$micrometerVersion" api "jakarta.annotation:jakarta.annotation-api:2.1.1" api "jakarta.inject:jakarta.inject-api:2.0.1" - api "jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:2.0.0" + api "jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:3.0.0" api "jakarta.servlet.jsp:jakarta.servlet.jsp-api:3.1.0" - api "jakarta.servlet:jakarta.servlet-api:5.0.0" - api "jakarta.xml.bind:jakarta.xml.bind-api:3.0.1" + api "jakarta.servlet:jakarta.servlet-api:6.0.0" + api "jakarta.xml.bind:jakarta.xml.bind-api:4.0.0" api "jakarta.persistence:jakarta.persistence-api:3.1.0" + api "jakarta.websocket:jakarta.websocket-api:2.1.0" + api "jakarta.websocket:jakarta.websocket-client-api:2.1.0" api "ldapsdk:ldapsdk:4.1" api "net.sourceforge.htmlunit:htmlunit:2.65.1" api "net.sourceforge.nekohtml:nekohtml:1.9.22" diff --git a/web/src/main/java/org/springframework/security/web/context/SaveContextOnUpdateOrErrorResponseWrapper.java b/web/src/main/java/org/springframework/security/web/context/SaveContextOnUpdateOrErrorResponseWrapper.java index 58711f9893..66b3e2cea5 100644 --- a/web/src/main/java/org/springframework/security/web/context/SaveContextOnUpdateOrErrorResponseWrapper.java +++ b/web/src/main/java/org/springframework/security/web/context/SaveContextOnUpdateOrErrorResponseWrapper.java @@ -105,14 +105,6 @@ public abstract class SaveContextOnUpdateOrErrorResponseWrapper extends OnCommit this.contextSaved = true; } - @Override - public final String encodeRedirectUrl(String url) { - if (this.disableUrlRewriting) { - return url; - } - return super.encodeRedirectUrl(url); - } - @Override public final String encodeRedirectURL(String url) { if (this.disableUrlRewriting) { @@ -121,14 +113,6 @@ public abstract class SaveContextOnUpdateOrErrorResponseWrapper extends OnCommit return super.encodeRedirectURL(url); } - @Override - public final String encodeUrl(String url) { - if (this.disableUrlRewriting) { - return url; - } - return super.encodeUrl(url); - } - @Override public final String encodeURL(String url) { if (this.disableUrlRewriting) { diff --git a/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java b/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java index 7b82b517ba..29a4ca231b 100644 --- a/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java +++ b/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,7 +51,8 @@ class CookieDeserializer extends JsonDeserializer { cookie.setSecure(readJsonNode(jsonNode, "secure").asBoolean()); cookie.setVersion(readJsonNode(jsonNode, "version").asInt()); cookie.setPath(readJsonNode(jsonNode, "path").asText()); - cookie.setHttpOnly(readJsonNode(jsonNode, "httpOnly").asBoolean()); + JsonNode attributes = readJsonNode(jsonNode, "attributes"); + cookie.setHttpOnly(readJsonNode(attributes, "HttpOnly").asBoolean()); return cookie; } diff --git a/web/src/main/java/org/springframework/security/web/session/DisableEncodeUrlFilter.java b/web/src/main/java/org/springframework/security/web/session/DisableEncodeUrlFilter.java index 2b15fa0f07..15e19f795a 100644 --- a/web/src/main/java/org/springframework/security/web/session/DisableEncodeUrlFilter.java +++ b/web/src/main/java/org/springframework/security/web/session/DisableEncodeUrlFilter.java @@ -61,21 +61,11 @@ public class DisableEncodeUrlFilter extends OncePerRequestFilter { super(response); } - @Override - public String encodeRedirectUrl(String url) { - return url; - } - @Override public String encodeRedirectURL(String url) { return url; } - @Override - public String encodeUrl(String url) { - return url; - } - @Override public String encodeURL(String url) { return url; diff --git a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java b/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java index 85cb95bde9..0a175f9689 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java @@ -362,28 +362,6 @@ public class AbstractRememberMeServicesTests { assertThat(cookie.isHttpOnly()).isTrue(); } - // SEC-2791 - @Test - public void setCookieMaxAge0VersionSet() { - MockRememberMeServices services = new MockRememberMeServices(); - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpServletResponse response = new MockHttpServletResponse(); - services.setCookie(new String[] { "value" }, 0, request, response); - Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); - assertThat(cookie.getVersion()).isEqualTo(1); - } - - // SEC-2791 - @Test - public void setCookieMaxAgeNegativeVersionSet() { - MockRememberMeServices services = new MockRememberMeServices(); - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpServletResponse response = new MockHttpServletResponse(); - services.setCookie(new String[] { "value" }, -1, request, response); - Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); - assertThat(cookie.getVersion()).isEqualTo(1); - } - // SEC-2791 @Test public void setCookieMaxAge1VersionSet() { diff --git a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java index 59e7c2fe4a..bca83913fa 100644 --- a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java @@ -540,21 +540,11 @@ public class HttpSessionSecurityContextRepositoryTests { MockHttpServletRequest request = new MockHttpServletRequest(); final String sessionId = ";jsessionid=id"; MockHttpServletResponse response = new MockHttpServletResponse() { - @Override - public String encodeRedirectUrl(String url) { - return url + sessionId; - } - @Override public String encodeRedirectURL(String url) { return url + sessionId; } - @Override - public String encodeUrl(String url) { - return url + sessionId; - } - @Override public String encodeURL(String url) { return url + sessionId; @@ -563,16 +553,12 @@ public class HttpSessionSecurityContextRepositoryTests { HttpRequestResponseHolder holder = new HttpRequestResponseHolder(request, response); repo.loadContext(holder); String url = "/aUrl"; - assertThat(holder.getResponse().encodeRedirectUrl(url)).isEqualTo(url + sessionId); assertThat(holder.getResponse().encodeRedirectURL(url)).isEqualTo(url + sessionId); - assertThat(holder.getResponse().encodeUrl(url)).isEqualTo(url + sessionId); assertThat(holder.getResponse().encodeURL(url)).isEqualTo(url + sessionId); repo.setDisableUrlRewriting(true); holder = new HttpRequestResponseHolder(request, response); repo.loadContext(holder); - assertThat(holder.getResponse().encodeRedirectUrl(url)).isEqualTo(url); assertThat(holder.getResponse().encodeRedirectURL(url)).isEqualTo(url); - assertThat(holder.getResponse().encodeUrl(url)).isEqualTo(url); assertThat(holder.getResponse().encodeURL(url)).isEqualTo(url); } diff --git a/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java b/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java index 5a3b0705e4..2a0aa9a453 100644 --- a/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java +++ b/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -140,14 +140,6 @@ public class FirewalledResponseTests { .withMessageContaining(CRLF_MESSAGE); } - @Test - public void addCookieWhenCookieCommentContainsCrlfThenException() { - Cookie cookie = new Cookie("foo", "bar"); - cookie.setComment("foo\r\nbar"); - assertThatIllegalArgumentException().isThrownBy(() -> this.fwResponse.addCookie(cookie)) - .withMessageContaining(CRLF_MESSAGE); - } - @Test public void rejectAnyLineEndingInNameAndValue() { validateLineEnding("foo", "foo\rbar"); diff --git a/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java index 7a5b448fa3..0e8cdfd032 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,19 +33,35 @@ import static org.assertj.core.api.Assertions.assertThat; public class CookieMixinTests extends AbstractMixinTests { // @formatter:off - private static final String COOKIE_JSON = "{" - + "\"@class\": \"jakarta.servlet.http.Cookie\", " - + "\"name\": \"demo\", " - + "\"value\": \"cookie1\"," - + "\"comment\": null, " - + "\"maxAge\": -1, " - + "\"path\": null, " - + "\"secure\": false, " - + "\"version\": 0, " - + "\"isHttpOnly\": false, " - + "\"domain\": null" - + "}"; + private static final String COOKIE_JSON = "{" + + " \"@class\": \"jakarta.servlet.http.Cookie\"," + + " \"name\": \"demo\"," + + " \"value\": \"cookie1\"," + + " \"attributes\":{\"@class\":\"java.util.Collections$EmptyMap\"}," + + " \"comment\": null," + + " \"maxAge\": -1," + + " \"path\": null," + + " \"secure\": false," + + " \"version\": 0," + + " \"domain\": null" + + "}"; // @formatter:on + + // @formatter:off + private static final String COOKIE_HTTP_ONLY_JSON = "{" + + " \"@class\": \"jakarta.servlet.http.Cookie\"," + + " \"name\": \"demo\"," + + " \"value\": \"cookie1\"," + + " \"attributes\":{\"@class\":\"java.util.Collections$UnmodifiableMap\", \"HttpOnly\": \"true\"}," + + " \"comment\": null," + + " \"maxAge\": -1," + + " \"path\": null," + + " \"secure\": false," + + " \"version\": 0," + + " \"domain\": null" + + "}"; + // @formatter:on + @Test public void serializeCookie() throws JsonProcessingException, JSONException { Cookie cookie = new Cookie("demo", "cookie1"); @@ -59,7 +75,23 @@ public class CookieMixinTests extends AbstractMixinTests { assertThat(cookie).isNotNull(); assertThat(cookie.getName()).isEqualTo("demo"); assertThat(cookie.getDomain()).isEqualTo(""); - assertThat(cookie.isHttpOnly()).isEqualTo(false); + } + + @Test + public void serializeCookieWithHttpOnly() throws JsonProcessingException, JSONException { + Cookie cookie = new Cookie("demo", "cookie1"); + cookie.setHttpOnly(true); + String actualString = this.mapper.writeValueAsString(cookie); + JSONAssert.assertEquals(COOKIE_HTTP_ONLY_JSON, actualString, true); + } + + @Test + public void deserializeCookieWithHttpOnly() throws IOException { + Cookie cookie = this.mapper.readValue(COOKIE_HTTP_ONLY_JSON, Cookie.class); + assertThat(cookie).isNotNull(); + assertThat(cookie.getName()).isEqualTo("demo"); + assertThat(cookie.getDomain()).isEqualTo(""); + assertThat(cookie.isHttpOnly()).isEqualTo(true); } } diff --git a/web/src/test/java/org/springframework/security/web/session/DisableEncodeUrlFilterTests.java b/web/src/test/java/org/springframework/security/web/session/DisableEncodeUrlFilterTests.java index b53ac696dc..66b03f9356 100644 --- a/web/src/test/java/org/springframework/security/web/session/DisableEncodeUrlFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/session/DisableEncodeUrlFilterTests.java @@ -46,21 +46,11 @@ class DisableEncodeUrlFilterTests { verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeURL("/")); } - @Test - void doFilterDisablesEncodeUrl() throws Exception { - verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeUrl("/")); - } - @Test void doFilterDisablesEncodeRedirectURL() throws Exception { verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeRedirectURL("/")); } - @Test - void doFilterDisablesEncodeRedirectUrl() throws Exception { - verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeRedirectUrl("/")); - } - private void verifyDoFilterDoesNotInteractWithResponse(Consumer toInvoke) throws Exception { this.filter.doFilter(this.request, this.response, (request, response) -> { HttpServletResponse httpResponse = (HttpServletResponse) response;