From aceba1f1cf63625c00daaa0b05f30de0a5a7999d Mon Sep 17 00:00:00 2001 From: Jeremy Waters Date: Mon, 11 Jan 2016 13:16:55 -0500 Subject: [PATCH] Add support for colons in remember-me token values We have an issue where token strings that contain a colon break the existing decoding strategy, which tokenizes on colons. This change urlencodes the individual tokens when creating the cookie string; and urldecodes them decoding the cookie and extracting the tokens. This also eliminates the need for existing code to deal with openid tokens which contain urls, and thus colons. Fixes gh-3355 --- .../AbstractRememberMeServices.java | 28 +++++++++++++------ .../AbstractRememberMeServicesTests.java | 4 +-- 2 files changed, 21 insertions(+), 11 deletions(-) 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 56283d3990..fd4824d492 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 @@ -15,7 +15,11 @@ */ package org.springframework.security.web.authentication.rememberme; +import java.io.UnsupportedEncodingException; import java.lang.reflect.Method; +import java.net.URLDecoder; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; @@ -226,13 +230,14 @@ public abstract class AbstractRememberMeServices implements RememberMeServices, String[] tokens = StringUtils.delimitedListToStringArray(cookieAsPlainText, DELIMITER); - if ((tokens[0].equalsIgnoreCase("http") || tokens[0].equalsIgnoreCase("https")) - && tokens[1].startsWith("//")) { - // Assume we've accidentally split a URL (OpenID identifier) - String[] newTokens = new String[tokens.length - 1]; - newTokens[0] = tokens[0] + ":" + tokens[1]; - System.arraycopy(tokens, 2, newTokens, 1, newTokens.length - 1); - tokens = newTokens; + for (int i = 0; i < tokens.length; i++) { + try { + tokens[i] = URLDecoder.decode(tokens[i], StandardCharsets.UTF_8.name()); + } catch (UnsupportedEncodingException uee) { + throw new InvalidCookieException( + "Unable to decode Cookie token using UTF-8; value was '" + tokens[i] + + "'"); + } } return tokens; @@ -247,8 +252,13 @@ public abstract class AbstractRememberMeServices implements RememberMeServices, protected String encodeCookie(String[] cookieTokens) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < cookieTokens.length; i++) { - sb.append(cookieTokens[i]); - + try { + sb.append(URLEncoder.encode(cookieTokens[i], StandardCharsets.UTF_8.name())); + } catch (UnsupportedEncodingException uee) { + throw new InvalidCookieException( + "Unable to encode Cookie token using UTF-8; value was '" + cookieTokens[i] + + "'"); + } if (i < cookieTokens.length - 1) { sb.append(DELIMITER); } 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 946335a009..4a06f6f5f9 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 @@ -88,7 +88,7 @@ public class AbstractRememberMeServicesTests { @Test public void cookieShouldBeCorrectlyEncodedAndDecoded() throws Exception { - String[] cookie = new String[] { "name", "cookie", "tokens", "blah" }; + String[] cookie = new String[] { "name:with:colon", "cookie", "tokens", "blah" }; MockRememberMeServices services = new MockRememberMeServices(uds); String encoded = services.encodeCookie(cookie); @@ -97,7 +97,7 @@ public class AbstractRememberMeServicesTests { String[] decoded = services.decodeCookie(encoded); assertThat(decoded.length).isEqualTo(4); - assertThat(decoded[0]).isEqualTo("name"); + assertThat(decoded[0]).isEqualTo("name:with:colon"); assertThat(decoded[1]).isEqualTo("cookie"); assertThat(decoded[2]).isEqualTo("tokens"); assertThat(decoded[3]).isEqualTo("blah");