From d05ad19276a3a0970556379749e1ca8566f6c2be Mon Sep 17 00:00:00 2001 From: John Coyne Date: Mon, 19 Nov 2018 20:39:23 -0600 Subject: [PATCH] CookieClearingLogoutHandler enhancement Enabled the ability to pass in an array of Cookies to support clearing cookies on a different path other than the default context path Issue: gh-6078 --- .../logout/CookieClearingLogoutHandler.java | 43 ++++++++++++++----- .../CookieClearingLogoutHandlerTests.java | 34 ++++++++++++++- 2 files changed, 65 insertions(+), 12 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 0c172a2c09..c3c50de457 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-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -24,28 +24,49 @@ import org.springframework.security.core.Authentication; import org.springframework.util.Assert; /** - * A logout handler which clears a defined list of cookies, using the context path as the - * cookie path. + * A logout handler which clears either + * - A defined list of cookie names, using the context path as the cookie path + * OR + * - A given list of Cookies * * @author Luke Taylor * @since 3.1 */ public final class CookieClearingLogoutHandler implements LogoutHandler { - private final List cookiesToClear; + private final List cookiesToClear; public CookieClearingLogoutHandler(String... cookiesToClear) { Assert.notNull(cookiesToClear, "List of cookies cannot be null"); - this.cookiesToClear = Arrays.asList(cookiesToClear); + this.cookiesToClear = Arrays.asList((Object[]) cookiesToClear); + } + + /** + * @since 5.X + * @param cookiesToClear - One or more Cookie objects that must have maxAge of 0 + */ + public CookieClearingLogoutHandler(Cookie... cookiesToClear) { + Assert.notNull(cookiesToClear, "List of cookies cannot be null"); + List cookieList = new ArrayList(); + for (Cookie cookie : cookiesToClear) { + Assert.isTrue(cookie.getMaxAge() == 0, "Cookie maxAge must be 0"); + cookieList.add(cookie); + } + this.cookiesToClear = cookieList; } public void logout(HttpServletRequest request, HttpServletResponse response, Authentication authentication) { - for (String cookieName : cookiesToClear) { - Cookie cookie = new Cookie(cookieName, null); - String cookiePath = request.getContextPath() + "/"; - cookie.setPath(cookiePath); - cookie.setMaxAge(0); - response.addCookie(cookie); + for (Object cookie : cookiesToClear) { + Cookie realCookie = null; + if (cookie instanceof String) { + realCookie = new Cookie((String) cookie, null); + String cookiePath = request.getContextPath() + "/"; + realCookie.setPath(cookiePath); + realCookie.setMaxAge(0); + }else if (cookie instanceof Cookie){ + realCookie = (Cookie) cookie; + } + response.addCookie(realCookie); } } } 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 7948fb1465..4f07ae1755 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 @@ -15,7 +15,7 @@ */ package org.springframework.security.web.authentication.logout; -import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import javax.servlet.http.Cookie; @@ -60,4 +60,36 @@ public class CookieClearingLogoutHandlerTests { assertThat(c.getMaxAge()).isZero(); } } + + @Test + public void passedInCookiesAreCleared() { + MockHttpServletResponse response = new MockHttpServletResponse(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContextPath("/foo/bar"); + Cookie cookie1 = new Cookie("my_cookie",null); + cookie1.setPath("/foo"); + cookie1.setMaxAge(0); + Cookie cookie2 = new Cookie("my_cookie_too",null); + cookie2.setPath("/foo"); + cookie2.setMaxAge(0); + CookieClearingLogoutHandler handler = new CookieClearingLogoutHandler(cookie1, cookie2); + handler.logout(request, response, mock(Authentication.class)); + assertThat(response.getCookies()).hasSize(2); + for (Cookie c : response.getCookies()) { + assertThat(c.getPath()).isEqualTo("/foo"); + assertThat(c.getMaxAge()).isZero(); + } + } + + @Test(expected=IllegalArgumentException.class) + public void invalidAge() { + MockHttpServletResponse response = new MockHttpServletResponse(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContextPath("/foo/bar"); + Cookie cookie1 = new Cookie("my_cookie",null); + cookie1.setPath("/foo"); + cookie1.setMaxAge(100); + CookieClearingLogoutHandler handler = new CookieClearingLogoutHandler(cookie1); + handler.logout(request, response, mock(Authentication.class)); + } }