MAPREDUCE-4317. Job view ACL checks are too permissive (kkambatl via tucu)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-1@1357736 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -51,6 +51,8 @@ Release 1.2.0 - unreleased
     HDFS-3595. Update the regular expression in TestEditLogLoading for the
     error message change by HDFS-3521.  (Colin Patrick McCabe via szetszwo)
 
+    MAPREDUCE-4317. Job view ACL checks are too permissive (kkambatl via tucu)
+
 Release 1.1.0 - unreleased
 
   INCOMPATIBLE CHANGES

src/mapred/org/apache/hadoop/mapred/JSPUtil.java

@@ -101,36 +101,43 @@ class JSPUtil {
     final JobInProgress job = jt.getJob(jobid);
     JobWithViewAccessCheck myJob = new JobWithViewAccessCheck(job);
 
+    if (!jt.areACLsEnabled() || job == null) {
+      return myJob;
+    }
+    
     String user = request.getRemoteUser();
-    if (user != null && job != null && jt.areACLsEnabled()) {
-      final UserGroupInformation ugi =
-        UserGroupInformation.createRemoteUser(user);
-      try {
-        ugi.doAs(new PrivilegedExceptionAction<Void>() {
-          public Void run() throws IOException, ServletException {
-
-            // checks job view permission
-            jt.getACLsManager().checkAccess(job, ugi,
-                Operation.VIEW_JOB_DETAILS);
-            return null;
-          }
-        });
-      } catch (AccessControlException e) {
-        String errMsg = "User " + ugi.getShortUserName() +
-            " failed to view " + jobid + "!<br><br>" + e.getMessage() +
-            "<hr><a href=\"jobtracker.jsp\">Go back to JobTracker</a><br>";
-        JSPUtil.setErrorAndForward(errMsg, request, response);
-        myJob.setViewAccess(false);
-      } catch (InterruptedException e) {
-        String errMsg = " Interrupted while trying to access " + jobid +
-        "<hr><a href=\"jobtracker.jsp\">Go back to JobTracker</a><br>";
-        JSPUtil.setErrorAndForward(errMsg, request, response);
-        myJob.setViewAccess(false);
-      }
+    if (user == null) {
+      JSPUtil.setErrorAndForward("Null user", request, response);
+      myJob.setViewAccess(false);
+      return myJob;
+    }
+    
+    final UserGroupInformation ugi = 
+      UserGroupInformation.createRemoteUser(user);
+    try {
+      ugi.doAs(new PrivilegedExceptionAction<Void>() {
+        public Void run() throws IOException, ServletException {
+          // checks job view permission
+          jt.getACLsManager().checkAccess(job, ugi,
+              Operation.VIEW_JOB_DETAILS);
+          return null;
+        }
+      });
+    } catch (AccessControlException e) {
+      String errMsg = "User " + ugi.getShortUserName() +
+          " failed to view " + jobid + "!<br><br>" + e.getMessage() +
+          "<hr><a href=\"jobtracker.jsp\">Go back to JobTracker</a><br>";
+      JSPUtil.setErrorAndForward(errMsg, request, response);
+      myJob.setViewAccess(false);
+    } catch (InterruptedException e) {
+      String errMsg = " Interrupted while trying to access " + jobid +
+      "<hr><a href=\"jobtracker.jsp\">Go back to JobTracker</a><br>";
+      JSPUtil.setErrorAndForward(errMsg, request, response);
+      myJob.setViewAccess(false);
     }
     return myJob;
   }
-
+  
   /**
    * Sets error code SC_UNAUTHORIZED in response and forwards to
    * error page which contains error message and a back link.

src/test/org/apache/hadoop/mapred/TestWebUIAuthorization.java

@@ -20,13 +20,20 @@ package org.apache.hadoop.mapred;
 import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.net.URL;
 import java.net.HttpURLConnection;
+import java.net.URL;
 import java.net.URLEncoder;
+import java.security.PrivilegedExceptionAction;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.examples.SleepJob;
 import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.Path;
@@ -35,20 +42,11 @@ import org.apache.hadoop.mapred.JobHistory.Keys;
 import org.apache.hadoop.mapred.JobHistory.TaskAttempt;
 import org.apache.hadoop.mapred.QueueManager.QueueACL;
 import org.apache.hadoop.mapreduce.JobContext;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.examples.SleepJob;
 import org.apache.hadoop.security.Groups;
 import org.apache.hadoop.security.ShellBasedUnixGroupsMapping;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.junit.Test;
 
-import java.security.PrivilegedExceptionAction;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
-
 public class TestWebUIAuthorization extends ClusterMapReduceTestCase {
 
   private static final Log LOG = LogFactory.getLog(
@@ -87,7 +85,13 @@ public class TestWebUIAuthorization extends ClusterMapReduceTestCase {
   static int getHttpStatusCode(String urlstring, String userName,
       String method) throws IOException {
     LOG.info("Accessing " + urlstring + " as user " + userName);
-    URL url = new URL(urlstring + "&user.name=" + userName);
+    URL url = null;
+    if (userName == null) {
+      url = new URL(urlstring);
+    } else {
+      url = new URL(urlstring + "&user.name=" + userName);
+    }
+
     HttpURLConnection connection = (HttpURLConnection)url.openConnection();
     connection.setRequestMethod(method);
     if (method.equals("POST")) {
@@ -908,9 +912,16 @@ public class TestWebUIAuthorization extends ClusterMapReduceTestCase {
     String taskGraphServlet = jtURL + "/taskgraph?type=map&jobid="
         + jobid.toString();
     validateViewJob(taskGraphServlet, "GET");
+    assertEquals("Incorrect return code for null user",
+        HttpURLConnection.HTTP_UNAUTHORIZED,
+        getHttpStatusCode(taskGraphServlet, null, "GET"));
+    
     taskGraphServlet = jtURL + "/taskgraph?type=reduce&jobid="
         + jobid.toString();
     validateViewJob(taskGraphServlet, "GET");
+    assertEquals("Incorrect return code for null user",
+        HttpURLConnection.HTTP_UNAUTHORIZED,
+        getHttpStatusCode(taskGraphServlet, null, "GET"));
   }
 
   // validate access of jobdetails.jsp