From 2015f392ef063ae295b6c64c5a83ca0f9762ed28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Onur=20Ka=C4=9Fan=20=C3=96zcan?= Date: Fri, 20 Dec 2019 18:04:31 +0300 Subject: [PATCH] Set secure when cancelling remember-me cookie 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. --- .../AbstractRememberMeServices.java | 7 +++ .../AbstractRememberMeServicesTests.java | 50 +++++++++++++++++++ 2 files changed, 57 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 a95a555398..2148408884 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 @@ import org.springframework.util.StringUtils; * @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 @@ public abstract class AbstractRememberMeServices implements RememberMeServices, if (cookieDomain != null) { cookie.setDomain(cookieDomain); } + if (useSecureCookie == null) { + cookie.setSecure(request.isSecure()); + } + else { + cookie.setSecure(useSecureCookie); + } response.addCookie(cookie); } 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 11adf5f758..1a515a3196 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 class AbstractRememberMeServicesTests { 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) {