YARN-8247 Incorrect HTTP status code returned by ATSv2 for non-whitelisted users. Contributed by Rohith Sharma K S

Author: Vrushali C

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) {

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<String, String> map = new HashMap<String, String>();
     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<Object>() {
@@ -155,7 +161,7 @@ public Object run() throws Exception {
     });
   }
 
-  @Test(expected = ForbiddenException.class)
+  @Test
   public void checkFilterNotAlloweGroup()
       throws ServletException, IOException, InterruptedException {
     Map<String, String> map = new HashMap<String, String>();
@@ -167,22 +173,27 @@ 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<Object>() {
       @Override
       public Object run() throws Exception {
         f.doFilter(mockHsr, r, null);
         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<Object>() {
@@ -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<Object>() {
@@ -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,22 +269,27 @@ 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<Object>() {
       @Override
       public Object run() throws Exception {
         f.doFilter(mockHsr, r, null);
         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<Object>() {
@@ -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<Object>() {
@@ -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<Object>() {