Skip to content

Commit 2015f39

Browse files
okohubeleftherias
authored andcommitted
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.
1 parent 40d4dce commit 2015f39

File tree

2 files changed

+57
-0
lines changed

2 files changed

+57
-0
lines changed

web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
* @author Luke Taylor
5454
* @author Rob Winch
5555
* @author Eddú Meléndez
56+
* @author Onur Kagan Ozcan
5657
* @since 2.0
5758
*/
5859
public abstract class AbstractRememberMeServices implements RememberMeServices,
@@ -383,6 +384,12 @@ protected void cancelCookie(HttpServletRequest request, HttpServletResponse resp
383384
if (cookieDomain != null) {
384385
cookie.setDomain(cookieDomain);
385386
}
387+
if (useSecureCookie == null) {
388+
cookie.setSecure(request.isSecure());
389+
}
390+
else {
391+
cookie.setSecure(useSecureCookie);
392+
}
386393
response.addCookie(cookie);
387394
}
388395

web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,56 @@ public void logoutShouldCancelCookie() {
268268
assertThat(returnedCookie.getDomain()).isEqualTo("spring.io");
269269
}
270270

271+
@Test
272+
public void cancelledCookieShouldUseSecureFlag() {
273+
MockRememberMeServices services = new MockRememberMeServices(uds);
274+
services.setCookieDomain("spring.io");
275+
services.setUseSecureCookie(true);
276+
277+
MockHttpServletRequest request = new MockHttpServletRequest();
278+
request.setContextPath("contextpath");
279+
request.setCookies(createLoginCookie("cookie:1:2"));
280+
MockHttpServletResponse response = new MockHttpServletResponse();
281+
282+
services.logout(request, response, Mockito.mock(Authentication.class));
283+
// Try again with null Authentication
284+
response = new MockHttpServletResponse();
285+
286+
services.logout(request, response, null);
287+
288+
assertCookieCancelled(response);
289+
290+
Cookie returnedCookie = response.getCookie(
291+
AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY);
292+
assertThat(returnedCookie.getDomain()).isEqualTo("spring.io");
293+
assertThat(returnedCookie.getSecure()).isEqualTo(true);
294+
}
295+
296+
@Test
297+
public void cancelledCookieShouldUseRequestIsSecure() {
298+
MockRememberMeServices services = new MockRememberMeServices(uds);
299+
services.setCookieDomain("spring.io");
300+
301+
MockHttpServletRequest request = new MockHttpServletRequest();
302+
request.setContextPath("contextpath");
303+
request.setCookies(createLoginCookie("cookie:1:2"));
304+
request.setSecure(true);
305+
MockHttpServletResponse response = new MockHttpServletResponse();
306+
307+
services.logout(request, response, Mockito.mock(Authentication.class));
308+
// Try again with null Authentication
309+
response = new MockHttpServletResponse();
310+
311+
services.logout(request, response, null);
312+
313+
assertCookieCancelled(response);
314+
315+
Cookie returnedCookie = response.getCookie(
316+
AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY);
317+
assertThat(returnedCookie.getDomain()).isEqualTo("spring.io");
318+
assertThat(returnedCookie.getSecure()).isEqualTo(true);
319+
}
320+
271321
@Test(expected = CookieTheftException.class)
272322
public void cookieTheftExceptionShouldBeRethrown() {
273323
MockRememberMeServices services = new MockRememberMeServices(uds) {

0 commit comments

Comments
 (0)