From bee426551f415126ea881ac5da0ac8efd72a0eae Mon Sep 17 00:00:00 2001 From: Onur Kagan Ozcan Date: Thu, 12 Dec 2019 14:36:44 +0300 Subject: [PATCH 1/2] Fix remember-me cookie set/cancel inconsistency: AbstractRememberMeServices is setting remember-me cookie with checking request is secure or secure usage is independently set to a fixed flag. But when cancelling a cookie, cookie is not being marked secure or not. It produces an inconsistency when using secure flag as a part to identity of cookie. This commit intended to fix this situation. --- .../rememberme/AbstractRememberMeServices.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java index a95a555398e..2148408884b 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java @@ -53,6 +53,7 @@ * @author Luke Taylor * @author Rob Winch * @author Eddú Meléndez + * @author Onur Kagan Ozcan * @since 2.0 */ public abstract class AbstractRememberMeServices implements RememberMeServices, @@ -383,6 +384,12 @@ protected void cancelCookie(HttpServletRequest request, HttpServletResponse resp if (cookieDomain != null) { cookie.setDomain(cookieDomain); } + if (useSecureCookie == null) { + cookie.setSecure(request.isSecure()); + } + else { + cookie.setSecure(useSecureCookie); + } response.addCookie(cookie); } From d597f3c752f02202fd2dc5b5f0d96c797e67d2a4 Mon Sep 17 00:00:00 2001 From: Onur Kagan Ozcan Date: Thu, 19 Dec 2019 11:24:49 +0300 Subject: [PATCH 2/2] Add tests for remember-me cancel cookie secure flag usage --- .../AbstractRememberMeServicesTests.java | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) 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 11adf5f758c..1a515a3196e 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 @@ -268,6 +268,56 @@ public void logoutShouldCancelCookie() { assertThat(returnedCookie.getDomain()).isEqualTo("spring.io"); } + @Test + public void cancelledCookieShouldUseSecureFlag() { + MockRememberMeServices services = new MockRememberMeServices(uds); + services.setCookieDomain("spring.io"); + services.setUseSecureCookie(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContextPath("contextpath"); + request.setCookies(createLoginCookie("cookie:1:2")); + MockHttpServletResponse response = new MockHttpServletResponse(); + + services.logout(request, response, Mockito.mock(Authentication.class)); + // Try again with null Authentication + response = new MockHttpServletResponse(); + + services.logout(request, response, null); + + assertCookieCancelled(response); + + Cookie returnedCookie = response.getCookie( + AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + assertThat(returnedCookie.getDomain()).isEqualTo("spring.io"); + assertThat(returnedCookie.getSecure()).isEqualTo(true); + } + + @Test + public void cancelledCookieShouldUseRequestIsSecure() { + MockRememberMeServices services = new MockRememberMeServices(uds); + services.setCookieDomain("spring.io"); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContextPath("contextpath"); + request.setCookies(createLoginCookie("cookie:1:2")); + request.setSecure(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + services.logout(request, response, Mockito.mock(Authentication.class)); + // Try again with null Authentication + response = new MockHttpServletResponse(); + + services.logout(request, response, null); + + assertCookieCancelled(response); + + Cookie returnedCookie = response.getCookie( + AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + assertThat(returnedCookie.getDomain()).isEqualTo("spring.io"); + assertThat(returnedCookie.getSecure()).isEqualTo(true); + } + @Test(expected = CookieTheftException.class) public void cookieTheftExceptionShouldBeRethrown() { MockRememberMeServices services = new MockRememberMeServices(uds) {