Repository: incubator-falcon Updated Branches: refs/heads/master 82cea4fb9 -> aace8f484
FALCON-857 Authorization failure results in internal server error. Contributed by Venkatesh Seetharam Project: http://git-wip-us.apache.org/repos/asf/incubator-falcon/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-falcon/commit/9ae2f327 Tree: http://git-wip-us.apache.org/repos/asf/incubator-falcon/tree/9ae2f327 Diff: http://git-wip-us.apache.org/repos/asf/incubator-falcon/diff/9ae2f327 Branch: refs/heads/master Commit: 9ae2f3275bffbf73fae41abcfa0c9a56de08d002 Parents: 82cea4f Author: Venkatesh Seetharam <venkat...@apache.org> Authored: Mon Nov 3 15:38:17 2014 -0800 Committer: Venkatesh Seetharam <venkat...@apache.org> Committed: Mon Nov 3 15:45:04 2014 -0800 ---------------------------------------------------------------------- CHANGES.txt | 3 + .../security/FalconAuthorizationFilter.java | 20 ++- .../security/FalconAuthorizationFilterTest.java | 122 +++++++++++++------ 3 files changed, 101 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/9ae2f327/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 999bc04..6ce0cf9 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -125,6 +125,9 @@ Trunk (Unreleased) OPTIMIZATIONS BUG FIXES + FALCON-857 Authorization failure results in internal server error + (Venkatesh Seetharam) + FALCON-850 Cluster summary UI page results in 400 Bad Request (Balu Vellanki via Venkatesh Seetharam) http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/9ae2f327/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java ---------------------------------------------------------------------- diff --git a/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java b/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java index 2af4b9a..c82e0bf 100644 --- a/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java +++ b/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java @@ -19,7 +19,6 @@ package org.apache.falcon.security; import org.apache.falcon.FalconException; -import org.apache.falcon.FalconWebException; import org.apache.hadoop.security.authorize.AuthorizationException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,7 +30,7 @@ import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.core.Response; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; /** @@ -62,6 +61,10 @@ public class FalconAuthorizationFilter implements Filter { public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException { + int errorCode = HttpServletResponse.SC_FORBIDDEN; + String errorMessage = null; + HttpServletResponse httpResponse = (HttpServletResponse) response; + if (isAuthorizationEnabled) { HttpServletRequest httpRequest = (HttpServletRequest) request; RequestParts requestParts = getUserRequest(httpRequest); @@ -72,11 +75,20 @@ public class FalconAuthorizationFilter implements Filter { requestParts.getAction(), requestParts.getEntityType(), requestParts.getEntityName(), CurrentUser.getProxyUGI()); } catch (AuthorizationException e) { - throw FalconWebException.newException(e.getMessage(), Response.Status.FORBIDDEN); + errorMessage = e.getMessage(); + } catch (IllegalArgumentException e) { + errorMessage = e.getMessage(); + errorCode = HttpServletResponse.SC_BAD_REQUEST; } } - filterChain.doFilter(request, response); + if (errorMessage == null) { // continue processing if there was no exception + filterChain.doFilter(request, response); + } else { + if (!httpResponse.isCommitted()) { + httpResponse.sendError(errorCode, errorMessage); + } + } } @Override http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/9ae2f327/prism/src/test/java/org/apache/falcon/security/FalconAuthorizationFilterTest.java ---------------------------------------------------------------------- diff --git a/prism/src/test/java/org/apache/falcon/security/FalconAuthorizationFilterTest.java b/prism/src/test/java/org/apache/falcon/security/FalconAuthorizationFilterTest.java index 385b005..03dc792 100644 --- a/prism/src/test/java/org/apache/falcon/security/FalconAuthorizationFilterTest.java +++ b/prism/src/test/java/org/apache/falcon/security/FalconAuthorizationFilterTest.java @@ -89,31 +89,70 @@ public class FalconAuthorizationFilterTest { {"/metadata/lineage/vertices/all"}, {"/metadata/lineage/vertices/_1"}, {"/metadata/lineage/vertices/properties/_1"}, + {"metadata/discovery/process_entity/sample-process/relations"}, + {"metadata/discovery/process_entity/list?cluster=primary-cluster"}, }; } @Test (dataProvider = "resourceWithNoEntity") public void testDoFilter(String resource) throws Exception { - Filter filter = new FalconAuthorizationFilter(); - synchronized (StartupProperties.get()) { - filter.init(mockConfig); + boolean[] enabledFlags = {false, true}; + for (boolean enabled : enabledFlags) { + StartupProperties.get().setProperty( + "falcon.security.authorization.enabled", String.valueOf(enabled)); + + Filter filter = new FalconAuthorizationFilter(); + synchronized (StartupProperties.get()) { + filter.init(mockConfig); + } + + StringBuffer requestUrl = new StringBuffer("http://localhost"; + resource); + Mockito.when(mockRequest.getRequestURL()).thenReturn(requestUrl); + Mockito.when(mockRequest.getRequestURI()).thenReturn("/api" + resource); + Mockito.when(mockRequest.getPathInfo()).thenReturn(resource); + + try { + filter.doFilter(mockRequest, mockResponse, mockChain); + } finally { + filter.destroy(); + } } + } - try { - boolean[] enabledFlags = {false, true}; - for (boolean enabled : enabledFlags) { - StartupProperties.get().setProperty( - "falcon.security.authorization.enabled", String.valueOf(enabled)); + @DataProvider(name = "invalidResource") + private Object[][] createBadOptions() { + return new Object[][] { + {"/admin1/version"}, + {"/entities1/list/feed"}, + {"/metadata1/lineage/vertices/all"}, + {"/foo/bar"}, + }; + } - StringBuffer requestUrl = new StringBuffer("http://localhost"; + resource); - Mockito.when(mockRequest.getRequestURL()).thenReturn(requestUrl); - Mockito.when(mockRequest.getRequestURI()).thenReturn("/api" + resource); - Mockito.when(mockRequest.getPathInfo()).thenReturn(resource); + @Test (dataProvider = "invalidResource") + public void testDoFilterBadResource(String resource) throws Exception { + boolean[] enabledFlags = {false, true}; + for (boolean enabled : enabledFlags) { + StartupProperties.get().setProperty( + "falcon.security.authorization.enabled", String.valueOf(enabled)); + Filter filter = new FalconAuthorizationFilter(); + synchronized (StartupProperties.get()) { + filter.init(mockConfig); + } + + StringBuffer requestUrl = new StringBuffer("http://localhost"; + resource); + Mockito.when(mockRequest.getRequestURL()).thenReturn(requestUrl); + Mockito.when(mockRequest.getRequestURI()).thenReturn("/api" + resource); + Mockito.when(mockRequest.getPathInfo()).thenReturn(resource); + + try { filter.doFilter(mockRequest, mockResponse, mockChain); + + // todo: verify the response error code to 400 + } finally { + filter.destroy(); } - } finally { - filter.destroy(); } } @@ -128,48 +167,51 @@ public class FalconAuthorizationFilterTest { @Test (dataProvider = "resourceWithEntity") public void testDoFilterForEntity(String resource) throws Exception { - Filter filter = new FalconAuthorizationFilter(); - synchronized (StartupProperties.get()) { - filter.init(mockConfig); - } - - try { - boolean[] enabledFlags = {false, true}; - for (boolean enabled : enabledFlags) { - StartupProperties.get().setProperty( - "falcon.security.authorization.enabled", String.valueOf(enabled)); + boolean[] enabledFlags = {false, true}; + for (boolean enabled : enabledFlags) { + StartupProperties.get().setProperty( + "falcon.security.authorization.enabled", String.valueOf(enabled)); + + Filter filter = new FalconAuthorizationFilter(); + synchronized (StartupProperties.get()) { + filter.init(mockConfig); + } - String uri = resource + processEntity.getName(); - StringBuffer requestUrl = new StringBuffer("http://localhost"; + uri); - Mockito.when(mockRequest.getRequestURL()).thenReturn(requestUrl); - Mockito.when(mockRequest.getRequestURI()).thenReturn("/api" + uri); - Mockito.when(mockRequest.getPathInfo()).thenReturn(uri); + String uri = resource + processEntity.getName(); + StringBuffer requestUrl = new StringBuffer("http://localhost"; + uri); + Mockito.when(mockRequest.getRequestURL()).thenReturn(requestUrl); + Mockito.when(mockRequest.getRequestURI()).thenReturn("/api" + uri); + Mockito.when(mockRequest.getPathInfo()).thenReturn(uri); + try { filter.doFilter(mockRequest, mockResponse, mockChain); + } finally { + filter.destroy(); } - } finally { - filter.destroy(); } } - @Test (expectedExceptions = Exception.class) + @Test public void testDoFilterForEntityWithInvalidEntity() throws Exception { CurrentUser.authenticate("falcon"); + + StartupProperties.get().setProperty("falcon.security.authorization.enabled", "true"); + Filter filter = new FalconAuthorizationFilter(); synchronized (StartupProperties.get()) { filter.init(mockConfig); } - try { - StartupProperties.get().setProperty("falcon.security.authorization.enabled", "true"); - - String uri = "/entities/suspend/process/bad-entity"; - StringBuffer requestUrl = new StringBuffer("http://localhost"; + uri); - Mockito.when(mockRequest.getRequestURL()).thenReturn(requestUrl); - Mockito.when(mockRequest.getRequestURI()).thenReturn("/api" + uri); - Mockito.when(mockRequest.getPathInfo()).thenReturn(uri); + String uri = "/entities/suspend/process/bad-entity"; + StringBuffer requestUrl = new StringBuffer("http://localhost"; + uri); + Mockito.when(mockRequest.getRequestURL()).thenReturn(requestUrl); + Mockito.when(mockRequest.getRequestURI()).thenReturn("/api" + uri); + Mockito.when(mockRequest.getPathInfo()).thenReturn(uri); + try { filter.doFilter(mockRequest, mockResponse, mockChain); + + // todo: verify the response error code to 403 } finally { filter.destroy(); }
