Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -24,28 +24,49 @@
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<String> cookiesToClear;
private final List<Object> cookiesToClear;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this to List<Function<HttpServletRequest,Cookie>>. In each of the constructors we can convert the arguments into a Function<HttpServletRequest,Cookie>.


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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this to 5.2

* @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<Object> cookieList = new ArrayList<Object>();
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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic will obviously need updated once we change this.cookiesToClear.

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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -60,4 +60,36 @@ public void configuredCookiesAreCleared() {
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));
}
}