From 27e17d58f38ba39cf6282e6195847591d3b22731 Mon Sep 17 00:00:00 2001 From: Onur Kagan Ozcan Date: Sat, 21 Dec 2019 01:16:06 +0300 Subject: [PATCH 1/3] Fix CookieClearingLogoutHandler cookie secure flag: It is better to mark cookie secure flag with request.isSecure() to ensure cookie identity is same --- .../logout/CookieClearingLogoutHandler.java | 2 ++ .../logout/CookieClearingLogoutHandlerTests.java | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java b/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java index bbbd42afff8..532a0e9e9eb 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java +++ b/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java @@ -32,6 +32,7 @@ * - A given list of Cookies * * @author Luke Taylor + * @author Onur Kagan Ozcan * @since 3.1 */ public final class CookieClearingLogoutHandler implements LogoutHandler { @@ -46,6 +47,7 @@ public CookieClearingLogoutHandler(String... cookiesToClear) { String cookiePath = request.getContextPath() + "/"; cookie.setPath(cookiePath); cookie.setMaxAge(0); + cookie.setSecure(request.isSecure()); return cookie; }; cookieList.add(f); diff --git a/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java index cd35ea18ff0..114448c851f 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java @@ -27,6 +27,7 @@ /** * @author Luke Taylor + * @author Onur Kagan Ozcan */ public class CookieClearingLogoutHandlerTests { @@ -61,6 +62,18 @@ public void configuredCookiesAreCleared() { } } + @Test + public void configuredCookieIsSecure() { + MockHttpServletResponse response = new MockHttpServletResponse(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setSecure(true); + request.setContextPath("/app"); + CookieClearingLogoutHandler handler = new CookieClearingLogoutHandler("my_cookie"); + handler.logout(request, response, mock(Authentication.class)); + assertThat(response.getCookies()).hasSize(1); + assertThat(response.getCookies()[0].getSecure()).isTrue(); + } + @Test public void passedInCookiesAreCleared() { MockHttpServletResponse response = new MockHttpServletResponse(); From bd210defe18e2bba50f5e4f50e546192cd0c9a8d Mon Sep 17 00:00:00 2001 From: Onur Kagan Ozcan Date: Sat, 21 Dec 2019 01:21:02 +0300 Subject: [PATCH 2/3] Update copyright headers --- .../web/authentication/logout/CookieClearingLogoutHandler.java | 2 +- .../authentication/logout/CookieClearingLogoutHandlerTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java b/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java index 532a0e9e9eb..3f7ba8b4ffb 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java +++ b/web/src/main/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. diff --git a/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java index 114448c851f..60a11e9eecd 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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. From 3605ca5d2ae4ccdb90741fa7820afa0e35bf06b8 Mon Sep 17 00:00:00 2001 From: Onur Kagan Ozcan Date: Sun, 12 Jan 2020 14:25:55 +0300 Subject: [PATCH 3/3] Add tests for insecure request cookie clearing process --- .../logout/CookieClearingLogoutHandlerTests.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java index 60a11e9eecd..e67169ad0dd 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/logout/CookieClearingLogoutHandlerTests.java @@ -74,6 +74,18 @@ public void configuredCookieIsSecure() { assertThat(response.getCookies()[0].getSecure()).isTrue(); } + @Test + public void configuredCookieIsNotSecure() { + MockHttpServletResponse response = new MockHttpServletResponse(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setSecure(false); + request.setContextPath("/app"); + CookieClearingLogoutHandler handler = new CookieClearingLogoutHandler("my_cookie"); + handler.logout(request, response, mock(Authentication.class)); + assertThat(response.getCookies()).hasSize(1); + assertThat(response.getCookies()[0].getSecure()).isFalse(); + } + @Test public void passedInCookiesAreCleared() { MockHttpServletResponse response = new MockHttpServletResponse();