Repository: incubator-zeppelin Updated Branches: refs/heads/master bd5be5092 -> 4818f07ad
Zeppelin enables CORS (Cross-Origin Request Sharing) by default with insecure settings (Access-Control-Allow-Origin: *) Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. Author: joelz <[email protected]> Author: djoelz <[email protected]> Closes #216 from djoelz/master and squashes the following commits: a00adc2 [djoelz] Merge pull request #1 from apache/master df324de [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. cecbab8 [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. Project: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/commit/4818f07a Tree: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/tree/4818f07a Diff: http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/diff/4818f07a Branch: refs/heads/master Commit: 4818f07ada85c3512d71aee1a9eb6be58e6909f0 Parents: bd5be50 Author: joelz <[email protected]> Authored: Wed Aug 19 10:57:40 2015 -0700 Committer: Lee moon soo <[email protected]> Committed: Wed Aug 19 18:24:44 2015 -0700 ---------------------------------------------------------------------- zeppelin-server/pom.xml | 5 ++ .../org/apache/zeppelin/server/CorsFilter.java | 16 +++- .../apache/zeppelin/server/CorsFilterTests.java | 95 ++++++++++++++++++++ .../zeppelin/socket/NotebookServerTests.java | 7 +- .../zeppelin/socket/TestHttpServletRequest.java | 2 +- zeppelin-web/src/WEB-INF/web.xml | 12 ++- 6 files changed, 124 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/4818f07a/zeppelin-server/pom.xml ---------------------------------------------------------------------- diff --git a/zeppelin-server/pom.xml b/zeppelin-server/pom.xml index cc6df97..9f908fe 100644 --- a/zeppelin-server/pom.xml +++ b/zeppelin-server/pom.xml @@ -315,6 +315,11 @@ <scope>test</scope> </dependency> + <dependency> + <groupId>org.mockito</groupId> + <artifactId>mockito-all</artifactId> + <version>1.9.0</version> + </dependency> </dependencies> <build> http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/4818f07a/zeppelin-server/src/main/java/org/apache/zeppelin/server/CorsFilter.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/CorsFilter.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/CorsFilter.java index 1524d5b..c2a137f 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/server/CorsFilter.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/CorsFilter.java @@ -18,6 +18,7 @@ package org.apache.zeppelin.server; import java.io.IOException; +import java.net.URI; import java.text.DateFormat; import java.util.Date; import java.util.Locale; @@ -40,21 +41,28 @@ public class CorsFilter implements Filter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException { + String sourceHost = request.getServerName(); + String currentHost = java.net.InetAddress.getLocalHost().getHostName(); + String origin = ""; + if (currentHost.equals(sourceHost) || "localhost".equals(sourceHost)) { + origin = ((HttpServletRequest) request).getHeader("Origin"); + } + if (((HttpServletRequest) request).getMethod().equals("OPTIONS")) { HttpServletResponse resp = ((HttpServletResponse) response); - addCorsHeaders(resp); + addCorsHeaders(resp, origin); return; } if (response instanceof HttpServletResponse) { HttpServletResponse alteredResponse = ((HttpServletResponse) response); - addCorsHeaders(alteredResponse); + addCorsHeaders(alteredResponse, origin); } filterChain.doFilter(request, response); } - private void addCorsHeaders(HttpServletResponse response) { - response.addHeader("Access-Control-Allow-Origin", "*"); + private void addCorsHeaders(HttpServletResponse response, String origin) { + response.addHeader("Access-Control-Allow-Origin", origin); response.addHeader("Access-Control-Allow-Credentials", "true"); response.addHeader("Access-Control-Allow-Headers", "authorization,Content-Type"); response.addHeader("Access-Control-Allow-Methods", "POST, GET, OPTIONS, PUT, HEAD, DELETE"); http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/4818f07a/zeppelin-server/src/test/java/org/apache/zeppelin/server/CorsFilterTests.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/server/CorsFilterTests.java b/zeppelin-server/src/test/java/org/apache/zeppelin/server/CorsFilterTests.java new file mode 100644 index 0000000..3c9152d --- /dev/null +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/server/CorsFilterTests.java @@ -0,0 +1,95 @@ +/** + * Created by joelz on 8/6/15. + * + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.zeppelin.server; + +import org.apache.zeppelin.socket.TestHttpServletRequest; +import org.junit.Assert; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.*; + +/** + * BASIC Cors rest api tests + * + * + * @author joelz + * + */ +public class CorsFilterTests { + + public static String[] headers = new String[8]; + public static Integer count = 0; + + @Test + public void ValidCorsFilterTest() throws IOException, ServletException { + CorsFilter filter = new CorsFilter(); + HttpServletResponse mockResponse = mock(HttpServletResponse.class); + FilterChain mockedFilterChain = mock(FilterChain.class); + TestHttpServletRequest mockRequest = mock(TestHttpServletRequest.class); + when(mockRequest.getHeader("Origin")).thenReturn("http://localhost:8080"); + when(mockRequest.getMethod()).thenReturn("Empty"); + when(mockRequest.getServerName()).thenReturn("localhost"); + + + doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocationOnMock) throws Throwable { + headers[count] = invocationOnMock.getArguments()[1].toString(); + count++; + return null; + } + }).when(mockResponse).addHeader(anyString(), anyString()); + + filter.doFilter(mockRequest, mockResponse, mockedFilterChain); + Assert.assertTrue(headers[0].equals("http://localhost:8080")); + } + + @Test + public void InvalidCorsFilterTest() throws IOException, ServletException { + CorsFilter filter = new CorsFilter(); + HttpServletResponse mockResponse = mock(HttpServletResponse.class); + FilterChain mockedFilterChain = mock(FilterChain.class); + TestHttpServletRequest mockRequest = mock(TestHttpServletRequest.class); + when(mockRequest.getHeader("Origin")).thenReturn("http://evillocalhost:8080"); + when(mockRequest.getMethod()).thenReturn("Empty"); + when(mockRequest.getServerName()).thenReturn("evillocalhost"); + + + doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocationOnMock) throws Throwable { + headers[count] = invocationOnMock.getArguments()[1].toString(); + count++; + return null; + } + }).when(mockResponse).addHeader(anyString(), anyString()); + + filter.doFilter(mockRequest, mockResponse, mockedFilterChain); + Assert.assertTrue(headers[0].equals("")); + } +} http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/4818f07a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTests.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTests.java b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTests.java index 3ab06f0..c262593 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTests.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTests.java @@ -19,14 +19,9 @@ */ package org.apache.zeppelin.socket; -import org.apache.zeppelin.notebook.Note; -import org.apache.zeppelin.server.ZeppelinServer; import org.junit.Assert; -import org.junit.FixMethodOrder; import org.junit.Test; -import org.junit.runners.MethodSorters; -import java.io.IOException; import java.net.UnknownHostException; /** @@ -50,4 +45,6 @@ import java.net.UnknownHostException; NotebookServer server = new NotebookServer(); Assert.assertFalse(server.checkOrigin(new TestHttpServletRequest(), "http://evillocalhost:8080")); } + + } http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/4818f07a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/TestHttpServletRequest.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/TestHttpServletRequest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/TestHttpServletRequest.java index 9ec54ba..0a176ba 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/TestHttpServletRequest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/TestHttpServletRequest.java @@ -317,7 +317,7 @@ public class TestHttpServletRequest implements HttpServletRequest { @Override public String getServerName() { - return null; + return "localhost"; } @Override http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/4818f07a/zeppelin-web/src/WEB-INF/web.xml ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/WEB-INF/web.xml b/zeppelin-web/src/WEB-INF/web.xml index f34da18..08c2639 100644 --- a/zeppelin-web/src/WEB-INF/web.xml +++ b/zeppelin-web/src/WEB-INF/web.xml @@ -17,8 +17,8 @@ --> <web-app xmlns="http://java.sun.com/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" - xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_2_5.xsd" - version="2.5"> + xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_0.xsd" + version="3.0"> <display-name>zeppelin-web</display-name> <servlet> @@ -30,7 +30,6 @@ </init-param> <load-on-startup>1</load-on-startup> </servlet> - <!-- This route is for swagger, must be different than root --> <servlet-mapping> <servlet-name>default</servlet-name> @@ -41,4 +40,11 @@ <param-name>configuration</param-name> <param-value>deployment</param-value> </context-param> + + <session-config> + <cookie-config> + <http-only>true</http-only> + <secure>true</secure> + </cookie-config> + </session-config> </web-app>
