You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by lh...@apache.org on 2010/05/14 05:17:46 UTC
svn commit: r944091 - in /incubator/shiro/trunk/web/src:
main/java/org/apache/shiro/web/mgt/ main/java/org/apache/shiro/web/servlet/
main/java/org/apache/shiro/web/session/
test/java/org/apache/shiro/web/servlet/
test/java/org/apache/shiro/web/session/
Author: lhazlewood
Date: Fri May 14 03:17:41 2010
New Revision: 944091
URL: http://svn.apache.org/viewvc?rev=944091&view=rev
Log:
SHIRO-139: Implemented Cookie HttpOnly support. Defaulted both DefaultWebSessionManager's sessionId cookie and the RememberMe cookie both to HttpOnly = true for extra security (reduced chance of XSS attacks)
Modified:
incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/mgt/CookieRememberMeManager.java
incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/Cookie.java
incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java
incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/session/DefaultWebSessionManager.java
incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/servlet/SimpleCookieTest.java
incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/session/DefaultWebSessionManagerTest.java
Modified: incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/mgt/CookieRememberMeManager.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/mgt/CookieRememberMeManager.java?rev=944091&r1=944090&r2=944091&view=diff
==============================================================================
--- incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/mgt/CookieRememberMeManager.java (original)
+++ incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/mgt/CookieRememberMeManager.java Fri May 14 03:17:41 2010
@@ -87,6 +87,7 @@ public class CookieRememberMeManager ext
*/
public CookieRememberMeManager() {
Cookie cookie = new SimpleCookie(DEFAULT_REMEMBER_ME_COOKIE_NAME);
+ cookie.setHttpOnly(true);
//One year should be long enough - most sites won't object to requiring a user to log in if they haven't visited
//in a year:
cookie.setMaxAge(Cookie.ONE_YEAR);
Modified: incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/Cookie.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/Cookie.java?rev=944091&r1=944090&r2=944091&view=diff
==============================================================================
--- incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/Cookie.java (original)
+++ incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/Cookie.java Fri May 14 03:17:41 2010
@@ -71,6 +71,10 @@ public interface Cookie {
void setVersion(int version);
+ void setHttpOnly(boolean httpOnly);
+
+ boolean isHttpOnly();
+
void saveTo(HttpServletRequest request, HttpServletResponse response);
void removeFrom(HttpServletRequest request, HttpServletResponse response);
Modified: incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java?rev=944091&r1=944090&r2=944091&view=diff
==============================================================================
--- incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java (original)
+++ incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java Fri May 14 03:17:41 2010
@@ -24,6 +24,11 @@ import org.slf4j.LoggerFactory;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.TimeZone;
/**
* TODO - Class JavaDoc
@@ -43,6 +48,22 @@ public class SimpleCookie implements Coo
*/
public static final int DEFAULT_VERSION = -1;
+ //These constants are protected on purpose so that the test case can use them
+ protected static final String NAME_VALUE_DELIMITER = "=";
+ protected static final String ATTRIBUTE_DELIMITER = "; ";
+ protected static final long DAY_MILLIS = 86400000; //1 day = 86,400,000 milliseconds
+ protected static final String GMT_TIME_ZONE_ID = "GMT";
+ protected static final String COOKIE_DATE_FORMAT_STRING = "EEE, dd-MMM-yyyy HH:mm:ss z";
+
+ protected static final String COOKIE_HEADER_NAME = "Set-Cookie";
+ protected static final String PATH_ATTRIBUTE_NAME = "Path";
+ protected static final String EXPIRES_ATTRIBUTE_NAME = "Expires";
+ protected static final String DOMAIN_ATTRIBUTE_NAME = "Domain";
+ protected static final String VERSION_ATTRIBUTE_NAME = "Version";
+ protected static final String COMMENT_ATTRIBUTE_NAME = "Comment";
+ protected static final String SECURE_ATTRIBUTE_NAME = "Secure";
+ protected static final String HTTP_ONLY_ATTRIBUTE_NAME = "HttpOnly";
+
private static final transient Logger log = LoggerFactory.getLogger(SimpleCookie.class);
private String name;
@@ -53,10 +74,12 @@ public class SimpleCookie implements Coo
private int maxAge;
private int version;
private boolean secure;
+ private boolean httpOnly;
public SimpleCookie() {
this.maxAge = DEFAULT_MAX_AGE;
this.version = DEFAULT_VERSION;
+ this.httpOnly = true; //most of the cookies ever used by Shiro should be as secure as possible.
}
public SimpleCookie(String name) {
@@ -70,9 +93,10 @@ public class SimpleCookie implements Coo
this.comment = cookie.getComment();
this.domain = cookie.getDomain();
this.path = cookie.getPath();
- this.maxAge = cookie.getMaxAge();
- this.version = cookie.getVersion();
+ this.maxAge = Math.max(DEFAULT_MAX_AGE, cookie.getMaxAge());
+ this.version = Math.max(DEFAULT_VERSION, cookie.getVersion());
this.secure = cookie.isSecure();
+ this.httpOnly = cookie.isHttpOnly();
}
public String getName() {
@@ -80,6 +104,9 @@ public class SimpleCookie implements Coo
}
public void setName(String name) {
+ if (!StringUtils.hasText(name)) {
+ throw new IllegalArgumentException("Name cannot be null/empty.");
+ }
this.name = name;
}
@@ -120,7 +147,7 @@ public class SimpleCookie implements Coo
}
public void setMaxAge(int maxAge) {
- this.maxAge = Math.max(-1, maxAge);
+ this.maxAge = Math.max(DEFAULT_MAX_AGE, maxAge);
}
public int getVersion() {
@@ -128,7 +155,7 @@ public class SimpleCookie implements Coo
}
public void setVersion(int version) {
- this.version = Math.max(-1, version);
+ this.version = Math.max(DEFAULT_VERSION, version);
}
public boolean isSecure() {
@@ -139,6 +166,14 @@ public class SimpleCookie implements Coo
this.secure = secure;
}
+ public boolean isHttpOnly() {
+ return httpOnly;
+ }
+
+ public void setHttpOnly(boolean httpOnly) {
+ this.httpOnly = httpOnly;
+ }
+
/**
* Returns the Cookie's calculated path setting. If the {@link javax.servlet.http.Cookie#getPath() path} is {@code null}, then the
* {@code request}'s {@link javax.servlet.http.HttpServletRequest#getContextPath() context path}
@@ -163,7 +198,7 @@ public class SimpleCookie implements Coo
public void saveTo(HttpServletRequest request, HttpServletResponse response) {
- String name = getName();
+ /*String name = getName();
String value = getValue();
String comment = getComment();
String domain = getDomain();
@@ -189,14 +224,125 @@ public class SimpleCookie implements Coo
cookie.setSecure(true);
}
- response.addCookie(cookie);
+ response.addCookie(cookie);*/
+
+ String headerValue = buildHeaderValue(request);
+ response.addHeader(COOKIE_HEADER_NAME, headerValue);
if (log.isDebugEnabled()) {
- log.debug("Added Cookie [{}] to path [{}] with value [{}] to the HttpServletResponse",
- new Object[]{name, path, value});
+ log.debug("Added HttpServletResponse Cookie [{}]", headerValue);
+ }
+ }
+
+ /**
+ * This implementation followed the grammar defined here for convenience:
+ * <a href="http://github.com/abarth/http-state/blob/master/notes/2009-11-07-Yui-Naruse.txt">Cookie grammar</a>.
+ *
+ * @param request the incoming request
+ * @return the 'Set-Cookie' header value for this cookie instance.
+ */
+ private String buildHeaderValue(HttpServletRequest request) {
+ String name = getName();
+ if (!StringUtils.hasText(name)) {
+ throw new IllegalStateException("Cookie name cannot be null/empty.");
+ }
+
+ StringBuffer sb = new StringBuffer(name).append(NAME_VALUE_DELIMITER);
+
+ String value = getValue();
+ if (StringUtils.hasText(value)) {
+ sb.append(value);
+ }
+
+ appendPath(sb, request);
+ appendDomain(sb);
+ appendExpires(sb);
+ appendVersion(sb);
+ appendComment(sb);
+ appendSecure(sb);
+ appendHttpOnly(sb);
+
+ return sb.toString();
+ }
+
+ private void appendPath(StringBuffer sb, HttpServletRequest request) {
+ String path = calculatePath(request);
+ if (StringUtils.hasText(path)) {
+ sb.append(ATTRIBUTE_DELIMITER);
+ sb.append(PATH_ATTRIBUTE_NAME).append(NAME_VALUE_DELIMITER).append(path);
}
}
+ private void appendDomain(StringBuffer sb) {
+ String domain = getDomain();
+ if (StringUtils.hasText(domain)) {
+ sb.append(ATTRIBUTE_DELIMITER);
+ sb.append(DOMAIN_ATTRIBUTE_NAME).append(NAME_VALUE_DELIMITER).append(domain);
+ }
+ }
+
+ private void appendExpires(StringBuffer sb) {
+ int maxAge = getMaxAge();
+ if (maxAge > DEFAULT_MAX_AGE) {
+ sb.append(ATTRIBUTE_DELIMITER);
+ Date expires;
+ if (maxAge == 0) {
+ //delete the cookie by specifying a time in the past (1 day ago):
+ expires = new Date(System.currentTimeMillis() - DAY_MILLIS);
+ } else {
+ //Value is in seconds. So take 'now' and add that many seconds, and that's our expiration date:
+ Calendar cal = Calendar.getInstance();
+ cal.add(Calendar.SECOND, maxAge);
+ expires = cal.getTime();
+ }
+ String formatted = toCookieDate(expires);
+ sb.append(EXPIRES_ATTRIBUTE_NAME).append(NAME_VALUE_DELIMITER).append(formatted);
+ }
+ }
+
+ private void appendVersion(StringBuffer sb) {
+ int version = getVersion();
+ if (version > DEFAULT_VERSION) {
+ sb.append(ATTRIBUTE_DELIMITER);
+ sb.append(VERSION_ATTRIBUTE_NAME).append(NAME_VALUE_DELIMITER).append(version);
+ }
+ }
+
+ private void appendComment(StringBuffer sb) {
+ String comment = getComment();
+ if (StringUtils.hasText(comment)) {
+ sb.append(ATTRIBUTE_DELIMITER);
+ sb.append(COMMENT_ATTRIBUTE_NAME).append(NAME_VALUE_DELIMITER).append(comment);
+ }
+ }
+
+ private void appendSecure(StringBuffer sb) {
+ if (isSecure()) {
+ sb.append(ATTRIBUTE_DELIMITER);
+ sb.append(SECURE_ATTRIBUTE_NAME); //No value for this attribute
+ }
+ }
+
+ private void appendHttpOnly(StringBuffer sb) {
+ if (isHttpOnly()) {
+ sb.append(ATTRIBUTE_DELIMITER);
+ sb.append(HTTP_ONLY_ATTRIBUTE_NAME); //No value for this attribute
+ }
+ }
+
+ /**
+ * Formats a date into a cookie date compatible string (Netscape's specification).
+ *
+ * @param date the date to format
+ * @return an HTTP 1.0/1.1 Cookie compatible date string (GMT-based).
+ */
+ private static String toCookieDate(Date date) {
+ TimeZone tz = TimeZone.getTimeZone(GMT_TIME_ZONE_ID);
+ DateFormat fmt = new SimpleDateFormat(COOKIE_DATE_FORMAT_STRING);
+ fmt.setTimeZone(tz);
+ return fmt.format(date);
+ }
+
/**
* Returns the cookie with the given name from the request or {@code null} if no cookie
* with that name could be found.
@@ -225,14 +371,15 @@ public class SimpleCookie implements Coo
cookie.setMaxAge(0);
cookie.setValue("deleteMe");
- //JSEC-94: Must set the path on the outgoing cookie (some browsers don't retain it from the
- //retrieved cookie?)
- // my testing shows none of these browsers will remove cookie if setPath() is not invoked: FF3, Chrome, IE7, Safari windows
+ // JSEC-94: Must set the path on the outgoing cookie (some browsers don't retain it from the
+ // retrieved cookie?)
+ // Testing shows none of these browsers will remove cookie if setPath() is not invoked:
+ // FF3, Chrome, IE7, Safari windows
String path = calculatePath(request);
cookie.setPath(path);
String domain = getDomain();
- if (domain != null) {
+ if (StringUtils.hasText(domain)) {
cookie.setDomain(domain);
}
Modified: incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/session/DefaultWebSessionManager.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/session/DefaultWebSessionManager.java?rev=944091&r1=944090&r2=944091&view=diff
==============================================================================
--- incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/session/DefaultWebSessionManager.java (original)
+++ incubator/shiro/trunk/web/src/main/java/org/apache/shiro/web/session/DefaultWebSessionManager.java Fri May 14 03:17:41 2010
@@ -50,7 +50,9 @@ public class DefaultWebSessionManager ex
private boolean sessionIdCookieEnabled;
public DefaultWebSessionManager() {
- this.sessionIdCookie = new SimpleCookie(ShiroHttpSession.DEFAULT_SESSION_ID_NAME);
+ Cookie cookie = new SimpleCookie(ShiroHttpSession.DEFAULT_SESSION_ID_NAME);
+ cookie.setHttpOnly(true); //more secure, protects against XSS attacks
+ this.sessionIdCookie = cookie;
this.sessionIdCookieEnabled = true;
}
Modified: incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/servlet/SimpleCookieTest.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/servlet/SimpleCookieTest.java?rev=944091&r1=944090&r2=944091&view=diff
==============================================================================
--- incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/servlet/SimpleCookieTest.java (original)
+++ incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/servlet/SimpleCookieTest.java Fri May 14 03:17:41 2010
@@ -72,16 +72,19 @@ public class SimpleCookieTest extends Te
assertTrue(cookie.getPath().equals("/somepath"));
}
- private void testContextPath(String contextPath) {
+ private void testRootContextPath(String contextPath) {
this.cookie.setValue("blah");
- javax.servlet.http.Cookie cookie = new javax.servlet.http.Cookie("test", "blah");
- cookie.setMaxAge(-1);
- cookie.setPath("/");
+ String expectedCookieValue = new StringBuffer()
+ .append("test").append(SimpleCookie.NAME_VALUE_DELIMITER).append("blah")
+ .append(SimpleCookie.ATTRIBUTE_DELIMITER)
+ .append(SimpleCookie.PATH_ATTRIBUTE_NAME).append(SimpleCookie.NAME_VALUE_DELIMITER).append(Cookie.ROOT_PATH)
+ .append(SimpleCookie.ATTRIBUTE_DELIMITER)
+ .append(SimpleCookie.HTTP_ONLY_ATTRIBUTE_NAME)
+ .toString();
expect(mockRequest.getContextPath()).andReturn(contextPath);
-
- mockResponse.addCookie(eqCookie(cookie));
+ mockResponse.addHeader(SimpleCookie.COOKIE_HEADER_NAME, expectedCookieValue);
replay(mockRequest);
replay(mockResponse);
@@ -95,14 +98,14 @@ public class SimpleCookieTest extends Te
@Test
/** Verifies fix for <a href="http://issues.apache.org/jira/browse/JSEC-34">JSEC-34</a> (1 of 2)*/
public void testEmptyContextPath() throws Exception {
- testContextPath("");
+ testRootContextPath("");
}
@Test
/** Verifies fix for <a href="http://issues.apache.org/jira/browse/JSEC-34">JSEC-34</a> (2 of 2)*/
public void testNullContextPath() throws Exception {
- testContextPath(null);
+ testRootContextPath(null);
}
private static <T extends javax.servlet.http.Cookie> T eqCookie(final T in) {
Modified: incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/session/DefaultWebSessionManagerTest.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/session/DefaultWebSessionManagerTest.java?rev=944091&r1=944090&r2=944091&view=diff
==============================================================================
--- incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/session/DefaultWebSessionManagerTest.java (original)
+++ incubator/shiro/trunk/web/src/test/java/org/apache/shiro/web/session/DefaultWebSessionManagerTest.java Fri May 14 03:17:41 2010
@@ -69,6 +69,7 @@ public class DefaultWebSessionManagerTes
expect(cookie.getPath()).andReturn("/");
expect(cookie.getVersion()).andReturn(SimpleCookie.DEFAULT_VERSION);
expect(cookie.isSecure()).andReturn(true);
+ expect(cookie.isHttpOnly()).andReturn(true);
replay(cookie);