Repository: ambari Updated Branches: refs/heads/trunk a52976e2b -> aea31b6b8
AMBARI-11129 - Set HttpOnly and Secure flags for Ambari session cookies (tbeerbower) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/aea31b6b Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/aea31b6b Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/aea31b6b Branch: refs/heads/trunk Commit: aea31b6b8c8e26787eeb08ab41ee59ab559364d1 Parents: a52976e Author: tbeerbower <[email protected]> Authored: Fri May 15 09:34:57 2015 -0400 Committer: tbeerbower <[email protected]> Committed: Fri May 15 09:34:57 2015 -0400 ---------------------------------------------------------------------- ambari-project/pom.xml | 12 +++--- ambari-server/pom.xml | 2 +- .../server/controller/AmbariHandlerList.java | 4 +- .../ambari/server/controller/AmbariServer.java | 30 ++++++++++----- .../server/controller/AmbariSessionManager.java | 2 +- .../server/controller/ControllerModule.java | 2 +- .../controller/AmbariHandlerListTest.java | 4 +- .../server/controller/AmbariServerTest.java | 40 ++++++++++++++++++-- .../controller/AmbariSessionManagerTest.java | 10 +++-- 9 files changed, 78 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-project/pom.xml ---------------------------------------------------------------------- diff --git a/ambari-project/pom.xml b/ambari-project/pom.xml index 378a998..b1db47b 100644 --- a/ambari-project/pom.xml +++ b/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> http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/pom.xml ---------------------------------------------------------------------- diff --git a/ambari-server/pom.xml b/ambari-server/pom.xml index 8efd1ec..4c34e6a 100644 --- a/ambari-server/pom.xml +++ b/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> http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariHandlerList.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariHandlerList.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariHandlerList.java index 13fc832..d1a7fde 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariHandlerList.java +++ b/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; http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java index 4a30c0d..8320715 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java +++ b/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 http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariSessionManager.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariSessionManager.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariSessionManager.java index 721d95b..fbf9d90 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariSessionManager.java +++ b/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(); } /** http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java index 432e41a..c9daab4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java +++ b/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); http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariHandlerListTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariHandlerListTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariHandlerListTest.java index 31a7a19..03bf6c4 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariHandlerListTest.java +++ b/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); http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariServerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariServerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariServerTest.java index 484f398..6e1c97e 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariServerTest.java +++ b/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; @@ -64,6 +68,36 @@ public class AmbariServerTest { } @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 { PasswordAuthentication pa = Authenticator.requestPasswordAuthentication( http://git-wip-us.apache.org/repos/asf/ambari/blob/aea31b6b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariSessionManagerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariSessionManagerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariSessionManagerTest.java index 058baa1..94671b4 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariSessionManagerTest.java +++ b/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
