AMBARI-11129 - Set HttpOnly and Secure flags for Ambari session cookies (tbeerbower)

ambari-project/pom.xml

@@ -234,22 +234,22 @@
       <dependency>
         <groupId>org.eclipse.jetty</groupId>
         <artifactId>jetty-server</artifactId>
-        <version>7.6.7.v20120910</version>
+        <version>8.1.17.v20150415</version>
       </dependency>
       <dependency>
         <groupId>org.eclipse.jetty</groupId>
         <artifactId>jetty-security</artifactId>
-        <version>7.6.7.v20120910</version>
+        <version>8.1.17.v20150415</version>
       </dependency>
       <dependency>
         <groupId>org.eclipse.jetty</groupId>
         <artifactId>jetty-servlet</artifactId>
-        <version>7.6.7.v20120910</version>
+        <version>8.1.17.v20150415</version>
       </dependency>
       <dependency>
         <groupId>org.eclipse.jetty</groupId>
         <artifactId>jetty-webapp</artifactId>
-        <version>7.6.7.v20120910</version>
+        <version>8.1.17.v20150415</version>
       </dependency>
       <!--jsp support for jetty -->
       <dependency>
@@ -295,8 +295,8 @@
       </dependency>
       <dependency>
         <groupId>javax.servlet</groupId>
-        <artifactId>servlet-api</artifactId>
-        <version>2.5</version>
+        <artifactId>javax.servlet-api</artifactId>
+        <version>3.0.1</version>
       </dependency>
       <dependency>
         <groupId>com.sun.jersey</groupId>

ambari-server/pom.xml

@@ -1605,7 +1605,7 @@
     </dependency>
     <dependency>
       <groupId>javax.servlet</groupId>
-      <artifactId>servlet-api</artifactId>
+      <artifactId>javax.servlet-api</artifactId>
     </dependency>
     <dependency>
       <groupId>com.sun.jersey</groupId>

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariHandlerList.java

@@ -234,8 +234,8 @@ public class AmbariHandlerList extends HandlerCollection implements ViewInstance
     webAppContext.setClassLoader(viewInstanceDefinition.getViewEntity().getClassLoader());
     webAppContext.setAttribute(ViewContext.CONTEXT_ATTRIBUTE, new ViewContextImpl(viewInstanceDefinition, viewRegistry));
     webAppContext.setSessionHandler(new SharedSessionHandler(sessionManager));
-    webAppContext.addFilter(new FilterHolder(persistFilter), "/*", 1);
-    webAppContext.addFilter(new FilterHolder(springSecurityFilter), "/*", 1);
+    webAppContext.addFilter(new FilterHolder(persistFilter), "/*", AmbariServer.DISPATCHER_TYPES);
+    webAppContext.addFilter(new FilterHolder(springSecurityFilter), "/*", AmbariServer.DISPATCHER_TYPES);
     webAppContext.setAllowNullPathInfo(true);
 
     return webAppContext;

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java

@@ -23,9 +23,11 @@ import java.io.File;
 import java.net.Authenticator;
 import java.net.BindException;
 import java.net.PasswordAuthentication;
+import java.util.EnumSet;
 import java.util.Map;
 
 import javax.crypto.BadPaddingException;
+import javax.servlet.DispatcherType;
 
 import org.apache.ambari.eventdb.webservice.WorkflowJsonService;
 import org.apache.ambari.server.AmbariException;
@@ -137,6 +139,11 @@ public class AmbariServer {
   // Set velocity logger
   protected static final String VELOCITY_LOG_CATEGORY = "VelocityLogger";
 
+  /**
+   * Dispatcher types for webAppContext.addFilter.
+   */
+  public static final EnumSet<DispatcherType> DISPATCHER_TYPES = EnumSet.of(DispatcherType.REQUEST);
+
   static {
     Velocity.setProperty("runtime.log.logsystem.log4j.logger", VELOCITY_LOG_CATEGORY);
   }
@@ -277,20 +284,20 @@ public class AmbariServer {
       rootServlet.setInitOrder(1);
 
       //session-per-request strategy for api and agents
-      root.addFilter(new FilterHolder(injector.getInstance(AmbariPersistFilter.class)), "/api/*", 1);
-      root.addFilter(new FilterHolder(injector.getInstance(AmbariPersistFilter.class)), "/proxy/*", 1);
-      root.addFilter(new FilterHolder(new MethodOverrideFilter()), "/api/*", 1);
-      root.addFilter(new FilterHolder(new MethodOverrideFilter()), "/proxy/*", 1);
+      root.addFilter(new FilterHolder(injector.getInstance(AmbariPersistFilter.class)), "/api/*", DISPATCHER_TYPES);
+      root.addFilter(new FilterHolder(injector.getInstance(AmbariPersistFilter.class)), "/proxy/*", DISPATCHER_TYPES);
+      root.addFilter(new FilterHolder(new MethodOverrideFilter()), "/api/*", DISPATCHER_TYPES);
+      root.addFilter(new FilterHolder(new MethodOverrideFilter()), "/proxy/*", DISPATCHER_TYPES);
 
       // register listener to capture request context
       root.addEventListener(new RequestContextListener());
 
-      agentroot.addFilter(new FilterHolder(injector.getInstance(AmbariPersistFilter.class)), "/agent/*", 1);
-      agentroot.addFilter(SecurityFilter.class, "/*", 1);
+      agentroot.addFilter(new FilterHolder(injector.getInstance(AmbariPersistFilter.class)), "/agent/*", DISPATCHER_TYPES);
+      agentroot.addFilter(SecurityFilter.class, "/*", DISPATCHER_TYPES);
 
       if (configs.getApiAuthentication()) {
-        root.addFilter(new FilterHolder(springSecurityFilter), "/api/*", 1);
-        root.addFilter(new FilterHolder(springSecurityFilter), "/proxy/*", 1);
+        root.addFilter(new FilterHolder(springSecurityFilter), "/api/*", DISPATCHER_TYPES);
+        root.addFilter(new FilterHolder(springSecurityFilter), "/proxy/*", DISPATCHER_TYPES);
       }
 
 
@@ -546,7 +553,12 @@ public class AmbariServer {
     // use AMBARISESSIONID instead of JSESSIONID to avoid conflicts with
     // other services (like HDFS) that run on the same context but a different
     // port
-    sessionManager.setSessionCookie("AMBARISESSIONID");
+    sessionManager.getSessionCookieConfig().setName("AMBARISESSIONID");
+
+    sessionManager.getSessionCookieConfig().setHttpOnly(true);
+    if (configs.getApiSSLAuthentication()) {
+      sessionManager.getSessionCookieConfig().setSecure(true);
+    }
 
     // each request that does not use AMBARISESSIONID will create a new
     // HashedSession in Jetty; these MUST be reaped after inactivity in order

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariSessionManager.java

@@ -59,7 +59,7 @@ public class AmbariSessionManager {
    * @return the session cookie
    */
   public String getSessionCookie() {
-    return sessionManager.getSessionCookie();
+    return sessionManager.getSessionCookieConfig().getName();
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java

@@ -276,7 +276,7 @@ public class ControllerModule extends AbstractModule {
 
     final SessionIdManager sessionIdManager = new HashSessionIdManager();
     final SessionManager sessionManager = new HashSessionManager();
-    sessionManager.setSessionPath("/");
+    sessionManager.getSessionCookieConfig().setPath("/");
     sessionManager.setSessionIdManager(sessionIdManager);
     bind(SessionManager.class).toInstance(sessionManager);
     bind(SessionIdManager.class).toInstance(sessionIdManager);

ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariHandlerListTest.java

@@ -69,8 +69,8 @@ public class AmbariHandlerListTest {
     Capture<FilterHolder> persistFilterCapture = new Capture<FilterHolder>();
     Capture<FilterHolder> securityFilterCapture = new Capture<FilterHolder>();
 
-    handler.addFilter(capture(persistFilterCapture), eq("/*"), eq(1));
-    handler.addFilter(capture(securityFilterCapture), eq("/*"), eq(1));
+    handler.addFilter(capture(persistFilterCapture), eq("/*"), eq(AmbariServer.DISPATCHER_TYPES));
+    handler.addFilter(capture(securityFilterCapture), eq("/*"), eq(AmbariServer.DISPATCHER_TYPES));
     handler.setAllowNullPathInfo(true);
 
     replay(handler, server);

ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariServerTest.java

@@ -18,31 +18,35 @@
 
 package org.apache.ambari.server.controller;
 
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
 
 import java.net.Authenticator;
 import java.net.InetAddress;
 import java.net.PasswordAuthentication;
 
 import org.apache.ambari.server.AmbariException;
+import org.apache.ambari.server.configuration.Configuration;
 import org.apache.ambari.server.orm.GuiceJpaInitializer;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
 import org.apache.velocity.app.Velocity;
 import org.easymock.EasyMock;
+import org.eclipse.jetty.server.SessionManager;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 
+import javax.servlet.SessionCookieConfig;
+
 public class AmbariServerTest {
 
-  private static final Logger log = LoggerFactory.getLogger(AmbariServerTest.class);
   private Injector injector;
 
 
@@ -63,6 +67,36 @@ public class AmbariServerTest {
     Assert.assertEquals(AmbariServer.VELOCITY_LOG_CATEGORY, Velocity.getProperty("runtime.log.logsystem.log4j.logger"));
   }
 
+  @Test
+  public void testConfigureSessionManager() throws Exception {
+    AmbariServer ambariServer = new AmbariServer();
+
+    Configuration configuration = createNiceMock(Configuration.class);
+    SessionManager sessionManager = createNiceMock(SessionManager.class);
+    SessionCookieConfig sessionCookieConfig = createNiceMock(SessionCookieConfig.class);
+
+    ambariServer.configs = configuration;
+
+    expect(sessionManager.getSessionCookieConfig()).andReturn(sessionCookieConfig).anyTimes();
+
+    expect(configuration.getApiSSLAuthentication()).andReturn(false);
+    sessionCookieConfig.setHttpOnly(true);
+
+    expect(configuration.getApiSSLAuthentication()).andReturn(true);
+    sessionCookieConfig.setHttpOnly(true);
+    sessionCookieConfig.setSecure(true);
+
+    replay(configuration, sessionManager, sessionCookieConfig);
+
+    // getApiSSLAuthentication == false
+    ambariServer.configureSessionManager(sessionManager);
+
+    // getApiSSLAuthentication == true
+    ambariServer.configureSessionManager(sessionManager);
+
+    verify(configuration, sessionManager, sessionCookieConfig);
+  }
+
   @Test
   public void testProxyUser() throws Exception {
 

ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariSessionManagerTest.java

@@ -21,6 +21,7 @@ package org.apache.ambari.server.controller;
 import org.eclipse.jetty.server.SessionManager;
 import org.junit.Test;
 
+import javax.servlet.SessionCookieConfig;
 import javax.servlet.http.HttpSession;
 
 import static org.easymock.EasyMock.createMockBuilder;
@@ -55,17 +56,20 @@ public class AmbariSessionManagerTest {
   @Test
   public void testGetSessionCookie() throws Exception {
     SessionManager sessionManager = createNiceMock(SessionManager.class);
+    SessionCookieConfig sessionCookieConfig = createNiceMock(SessionCookieConfig.class);
+
     AmbariSessionManager ambariSessionManager = new AmbariSessionManager();
 
     ambariSessionManager.sessionManager = sessionManager;
 
-    expect(sessionManager.getSessionCookie()).andReturn("SESSION_COOKIE").anyTimes();
+    expect(sessionCookieConfig.getName()).andReturn("SESSION_COOKIE").anyTimes();
+    expect(sessionManager.getSessionCookieConfig()).andReturn(sessionCookieConfig).anyTimes();
 
-    replay(sessionManager);
+    replay(sessionManager, sessionCookieConfig);
 
     assertEquals("SESSION_COOKIE", ambariSessionManager.getSessionCookie());
 
-    verify(sessionManager);
+    verify(sessionManager, sessionCookieConfig);
   }
 
   @Test