From 3c95ca4f21dcfcaabdd0694e7d005a45baba953f Mon Sep 17 00:00:00 2001 From: Vrushali C Date: Wed, 9 May 2018 22:17:48 -0700 Subject: [PATCH] YARN-8247 Incorrect HTTP status code returned by ATSv2 for non-whitelisted users. Contributed by Rohith Sharma K S --- ...ineReaderWhitelistAuthorizationFilter.java | 14 ++--- ...ineReaderWhitelistAuthorizationFilter.java | 58 ++++++++++++------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/security/TimelineReaderWhitelistAuthorizationFilter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/security/TimelineReaderWhitelistAuthorizationFilter.java index 8093fcf6be7..dbe391cb68d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/security/TimelineReaderWhitelistAuthorizationFilter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/main/java/org/apache/hadoop/yarn/server/timelineservice/reader/security/TimelineReaderWhitelistAuthorizationFilter.java @@ -27,15 +27,13 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.core.Response; -import javax.ws.rs.core.Response.Status; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authorize.AccessControlList; import org.apache.hadoop.security.authorize.AuthorizationException; import org.apache.hadoop.yarn.conf.YarnConfiguration; -import org.apache.hadoop.yarn.webapp.ForbiddenException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.yarn.server.timelineservice.reader.TimelineReaderWebServicesUtils; @@ -64,9 +62,12 @@ public void destroy() { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + HttpServletRequest httpRequest = (HttpServletRequest) request; + HttpServletResponse httpResponse = (HttpServletResponse) response; + if (isWhitelistReadAuthEnabled) { UserGroupInformation callerUGI = TimelineReaderWebServicesUtils - .getCallerUserGroupInformation((HttpServletRequest) request, true); + .getCallerUserGroupInformation(httpRequest, true); if (callerUGI == null) { String msg = "Unable to obtain user name, user not authenticated"; throw new AuthorizationException(msg); @@ -76,9 +77,8 @@ public void doFilter(ServletRequest request, ServletResponse response, String userName = callerUGI.getShortUserName(); String msg = "User " + userName + " is not allowed to read TimelineService V2 data."; - Response.status(Status.FORBIDDEN).entity(msg).build(); - throw new ForbiddenException("user " + userName - + " is not allowed to read TimelineService V2 data"); + httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, msg); + return; } } if (chain != null) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/reader/TestTimelineReaderWhitelistAuthorizationFilter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/reader/TestTimelineReaderWhitelistAuthorizationFilter.java index bd4f0c2ebf9..6f0a83d1f3c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/reader/TestTimelineReaderWhitelistAuthorizationFilter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice/src/test/java/org/apache/hadoop/yarn/server/timelineservice/reader/TestTimelineReaderWhitelistAuthorizationFilter.java @@ -18,6 +18,7 @@ package org.apache.hadoop.yarn.server.timelineservice.reader; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -32,13 +33,12 @@ import javax.servlet.FilterConfig; import javax.servlet.ServletContext; import javax.servlet.ServletException; -import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.server.timelineservice.reader.security.TimelineReaderWhitelistAuthorizationFilter; -import org.apache.hadoop.yarn.webapp.ForbiddenException; import org.junit.Test; import org.mockito.Mockito; @@ -100,11 +100,11 @@ public String getName() { } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); f.doFilter(mockHsr, r, null); } - @Test(expected = ForbiddenException.class) + @Test public void checkFilterNotAllowedUser() throws ServletException, IOException { Map map = new HashMap(); map.put(YarnConfiguration.TIMELINE_SERVICE_READ_AUTH_ENABLED, "true"); @@ -115,14 +115,20 @@ public void checkFilterNotAllowedUser() throws ServletException, IOException { FilterConfig fc = new DummyFilterConfig(map); f.init(fc); HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class); + String userName = "testuser1"; Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() { @Override public String getName() { - return "testuser1"; + return userName; } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); f.doFilter(mockHsr, r, null); + + String msg = "User " + userName + + " is not allowed to read TimelineService V2 data."; + Mockito.verify(r) + .sendError(eq(HttpServletResponse.SC_FORBIDDEN), eq(msg)); } @Test @@ -143,7 +149,7 @@ public String getName() { return "user1"; } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); UserGroupInformation user1 = UserGroupInformation.createUserForTesting("user1", GROUP_NAMES); user1.doAs(new PrivilegedExceptionAction() { @@ -155,7 +161,7 @@ public Object run() throws Exception { }); } - @Test(expected = ForbiddenException.class) + @Test public void checkFilterNotAlloweGroup() throws ServletException, IOException, InterruptedException { Map map = new HashMap(); @@ -167,15 +173,16 @@ public void checkFilterNotAlloweGroup() FilterConfig fc = new DummyFilterConfig(map); f.init(fc); HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class); + String userName = "user200"; Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() { @Override public String getName() { - return "user200"; + return userName; } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); UserGroupInformation user1 = - UserGroupInformation.createUserForTesting("user200", GROUP_NAMES); + UserGroupInformation.createUserForTesting(userName, GROUP_NAMES); user1.doAs(new PrivilegedExceptionAction() { @Override public Object run() throws Exception { @@ -183,6 +190,10 @@ public Object run() throws Exception { return null; } }); + String msg = "User " + userName + + " is not allowed to read TimelineService V2 data."; + Mockito.verify(r) + .sendError(eq(HttpServletResponse.SC_FORBIDDEN), eq(msg)); } @Test @@ -205,7 +216,7 @@ public String getName() { return "user90"; } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); UserGroupInformation user1 = UserGroupInformation.createUserForTesting("user90", GROUP_NAMES); user1.doAs(new PrivilegedExceptionAction() { @@ -235,7 +246,7 @@ public String getName() { return "user90"; } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); UserGroupInformation user1 = UserGroupInformation.createUserForTesting("user90", GROUP_NAMES); user1.doAs(new PrivilegedExceptionAction() { @@ -247,7 +258,7 @@ public Object run() throws Exception { }); } - @Test(expected = ForbiddenException.class) + @Test public void checkFilterAllowNoOneWhenAdminAclsEmptyAndUserAclsEmpty() throws ServletException, IOException, InterruptedException { // check that users in admin acl list are allowed to read @@ -258,15 +269,16 @@ public void checkFilterAllowNoOneWhenAdminAclsEmptyAndUserAclsEmpty() FilterConfig fc = new DummyFilterConfig(map); f.init(fc); HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class); + String userName = "user88"; Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() { @Override public String getName() { - return "user88"; + return userName; } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); UserGroupInformation user1 = - UserGroupInformation.createUserForTesting("user88", GROUP_NAMES); + UserGroupInformation.createUserForTesting(userName, GROUP_NAMES); user1.doAs(new PrivilegedExceptionAction() { @Override public Object run() throws Exception { @@ -274,6 +286,10 @@ public Object run() throws Exception { return null; } }); + String msg = "User " + userName + + " is not allowed to read TimelineService V2 data."; + Mockito.verify(r) + .sendError(eq(HttpServletResponse.SC_FORBIDDEN), eq(msg)); } @Test @@ -293,7 +309,7 @@ public String getName() { return "user437"; } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); UserGroupInformation user1 = UserGroupInformation.createUserForTesting("user437", GROUP_NAMES); user1.doAs(new PrivilegedExceptionAction() { @@ -327,7 +343,7 @@ public String getName() { } }); - ServletResponse r = Mockito.mock(ServletResponse.class); + HttpServletResponse r = Mockito.mock(HttpServletResponse.class); UserGroupInformation user1 = // both username and group name are not part of admin and // read allowed users @@ -348,7 +364,7 @@ public String getName() { return "user27"; } }); - ServletResponse r2 = Mockito.mock(ServletResponse.class); + HttpServletResponse r2 = Mockito.mock(HttpServletResponse.class); UserGroupInformation user2 = UserGroupInformation.createUserForTesting("user27", GROUP_NAMES); user2.doAs(new PrivilegedExceptionAction() { @@ -366,7 +382,7 @@ public String getName() { return "user2"; } }); - ServletResponse r3 = Mockito.mock(ServletResponse.class); + HttpServletResponse r3 = Mockito.mock(HttpServletResponse.class); UserGroupInformation user3 = UserGroupInformation.createUserForTesting("user2", GROUP_NAMES); user3.doAs(new PrivilegedExceptionAction() {