Re: [xwiki-devs] [xwiki-notifications] r28709 - platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki
On May 4, 2010, at 10:50 PM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2010-05-04 22:50:34 +0200 (Tue, 04 May 2010) New Revision: 28709
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java Log: XWIKI-5156: Session cookies are not marked as HttpOnly Fixed
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:00 UTC (rev 28708) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:34 UTC (rev 28709) @@ -245,7 +245,31 @@ LOG.debug("Adding cookie: " + cookie.getDomain() + cookie.getPath() + " " + cookie.getName() + "=" + cookie.getValue()); } - response.addCookie(cookie); + // We don't use the container's response.addCookie, since the HttpOnly cookie flag was introduced only recently + // in the servlet specification, and we're still using the older 2.4 specification as a minimal requirement for + // compatibility with as many containers as possible. Instead, we write the cookie manually as a HTTP header.
AFAIK addCookie is avail in the the 2.3 spec: http://java.sun.com/products/servlet/2.3/javadoc/javax/servlet/http/HttpServ... Thanks -Vincent
+ StringBuilder cookieValue = new StringBuilder(150); + cookieValue.append(cookie.getName() + "="); + if (StringUtils.isNotEmpty(cookie.getValue())) { + cookieValue.append("\"" + cookie.getValue() + "\""); + } + cookieValue.append("; Version=1"); + if (cookie.getMaxAge() >= 0) { + cookieValue.append("; Max-Age=" + cookie.getMaxAge()); + } else { + cookieValue.append("; Discard"); + } + if (StringUtils.isNotEmpty(cookie.getDomain())) { + // IE needs toLowerCase for the domain name + cookieValue.append("; Domain=" + cookie.getDomain().toLowerCase()); + } + if (StringUtils.isNotEmpty(cookie.getPath())) { + cookieValue.append("; Path=" + cookie.getPath()); + } + // Protect cookies from being used from JavaScript, see http://www.owasp.org/index.php/HttpOnly + cookieValue.append("; HttpOnly"); + + response.addHeader("Set-Cookie", cookieValue.toString()); }
/**
On 05/05/2010 10:13 AM, Vincent Massol wrote:
On May 4, 2010, at 10:50 PM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2010-05-04 22:50:34 +0200 (Tue, 04 May 2010) New Revision: 28709
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java Log: XWIKI-5156: Session cookies are not marked as HttpOnly Fixed
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:00 UTC (rev 28708) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:34 UTC (rev 28709) @@ -245,7 +245,31 @@ LOG.debug("Adding cookie: " + cookie.getDomain() + cookie.getPath() + " " + cookie.getName() + "=" + cookie.getValue()); } - response.addCookie(cookie); + // We don't use the container's response.addCookie, since the HttpOnly cookie flag was introduced only recently + // in the servlet specification, and we're still using the older 2.4 specification as a minimal requirement for + // compatibility with as many containers as possible. Instead, we write the cookie manually as a HTTP header.
AFAIK addCookie is avail in the the 2.3 spec: http://java.sun.com/products/servlet/2.3/javadoc/javax/servlet/http/HttpServ...
Yes, addCookie is (and it was used before). The problem is the Cookie class which doesn't have a setHttpOnly in 2.3.
Thanks -Vincent
+ StringBuilder cookieValue = new StringBuilder(150); + cookieValue.append(cookie.getName() + "="); + if (StringUtils.isNotEmpty(cookie.getValue())) { + cookieValue.append("\"" + cookie.getValue() + "\""); + } + cookieValue.append("; Version=1"); + if (cookie.getMaxAge()>= 0) { + cookieValue.append("; Max-Age=" + cookie.getMaxAge()); + } else { + cookieValue.append("; Discard"); + } + if (StringUtils.isNotEmpty(cookie.getDomain())) { + // IE needs toLowerCase for the domain name + cookieValue.append("; Domain=" + cookie.getDomain().toLowerCase()); + } + if (StringUtils.isNotEmpty(cookie.getPath())) { + cookieValue.append("; Path=" + cookie.getPath()); + } + // Protect cookies from being used from JavaScript, see http://www.owasp.org/index.php/HttpOnly + cookieValue.append("; HttpOnly"); + + response.addHeader("Set-Cookie", cookieValue.toString()); }
/**
-- Sergiu Dumitriu http://purl.org/net/sergiu/
On May 5, 2010, at 7:22 PM, Sergiu Dumitriu wrote:
On 05/05/2010 10:13 AM, Vincent Massol wrote:
On May 4, 2010, at 10:50 PM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2010-05-04 22:50:34 +0200 (Tue, 04 May 2010) New Revision: 28709
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java Log: XWIKI-5156: Session cookies are not marked as HttpOnly Fixed
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:00 UTC (rev 28708) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:34 UTC (rev 28709) @@ -245,7 +245,31 @@ LOG.debug("Adding cookie: " + cookie.getDomain() + cookie.getPath() + " " + cookie.getName() + "=" + cookie.getValue()); } - response.addCookie(cookie); + // We don't use the container's response.addCookie, since the HttpOnly cookie flag was introduced only recently + // in the servlet specification, and we're still using the older 2.4 specification as a minimal requirement for + // compatibility with as many containers as possible. Instead, we write the cookie manually as a HTTP header.
AFAIK addCookie is avail in the the 2.3 spec: http://java.sun.com/products/servlet/2.3/javadoc/javax/servlet/http/HttpServ...
Yes, addCookie is (and it was used before). The problem is the Cookie class which doesn't have a setHttpOnly in 2.3.
ok I misread. Couldn't you still use addCookie but call Cookie.getValue() add the httpOnly value to it and call setValue() to set it instead? Thanks -Vincent
Thanks -Vincent
+ StringBuilder cookieValue = new StringBuilder(150); + cookieValue.append(cookie.getName() + "="); + if (StringUtils.isNotEmpty(cookie.getValue())) { + cookieValue.append("\"" + cookie.getValue() + "\""); + } + cookieValue.append("; Version=1"); + if (cookie.getMaxAge()>= 0) { + cookieValue.append("; Max-Age=" + cookie.getMaxAge()); + } else { + cookieValue.append("; Discard"); + } + if (StringUtils.isNotEmpty(cookie.getDomain())) { + // IE needs toLowerCase for the domain name + cookieValue.append("; Domain=" + cookie.getDomain().toLowerCase()); + } + if (StringUtils.isNotEmpty(cookie.getPath())) { + cookieValue.append("; Path=" + cookie.getPath()); + } + // Protect cookies from being used from JavaScript, see http://www.owasp.org/index.php/HttpOnly + cookieValue.append("; HttpOnly"); + + response.addHeader("Set-Cookie", cookieValue.toString()); }
/**
On 05/05/2010 07:40 PM, Vincent Massol wrote:
On May 5, 2010, at 7:22 PM, Sergiu Dumitriu wrote:
On 05/05/2010 10:13 AM, Vincent Massol wrote:
On May 4, 2010, at 10:50 PM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2010-05-04 22:50:34 +0200 (Tue, 04 May 2010) New Revision: 28709
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java Log: XWIKI-5156: Session cookies are not marked as HttpOnly Fixed
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:00 UTC (rev 28708) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyPersistentLoginManager.java 2010-05-04 20:50:34 UTC (rev 28709) @@ -245,7 +245,31 @@ LOG.debug("Adding cookie: " + cookie.getDomain() + cookie.getPath() + " " + cookie.getName() + "=" + cookie.getValue()); } - response.addCookie(cookie); + // We don't use the container's response.addCookie, since the HttpOnly cookie flag was introduced only recently + // in the servlet specification, and we're still using the older 2.4 specification as a minimal requirement for + // compatibility with as many containers as possible. Instead, we write the cookie manually as a HTTP header.
AFAIK addCookie is avail in the the 2.3 spec: http://java.sun.com/products/servlet/2.3/javadoc/javax/servlet/http/HttpServ...
Yes, addCookie is (and it was used before). The problem is the Cookie class which doesn't have a setHttpOnly in 2.3.
ok I misread. Couldn't you still use addCookie but call Cookie.getValue() add the httpOnly value to it and call setValue() to set it instead?
No, since the value is escaped by the container. It will really be placed in the cookie value. The only way to keep the simple addCookie call is to upgrade to servlet 3.0.
Thanks -Vincent
Thanks -Vincent
+ StringBuilder cookieValue = new StringBuilder(150); + cookieValue.append(cookie.getName() + "="); + if (StringUtils.isNotEmpty(cookie.getValue())) { + cookieValue.append("\"" + cookie.getValue() + "\""); + } + cookieValue.append("; Version=1"); + if (cookie.getMaxAge()>= 0) { + cookieValue.append("; Max-Age=" + cookie.getMaxAge()); + } else { + cookieValue.append("; Discard"); + } + if (StringUtils.isNotEmpty(cookie.getDomain())) { + // IE needs toLowerCase for the domain name + cookieValue.append("; Domain=" + cookie.getDomain().toLowerCase()); + } + if (StringUtils.isNotEmpty(cookie.getPath())) { + cookieValue.append("; Path=" + cookie.getPath()); + } + // Protect cookies from being used from JavaScript, see http://www.owasp.org/index.php/HttpOnly + cookieValue.append("; HttpOnly"); + + response.addHeader("Set-Cookie", cookieValue.toString()); }
/**
-- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (2)
-
Sergiu Dumitriu -
Vincent Massol