HADOOP-10652: Merging r1605145 from trunk to branch-2.

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

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -100,6 +100,9 @@ Release 2.5.0 - UNRELEASED
     HADOOP-10665. Make Hadoop Authentication Handler loads case in-sensitive
     (Benoy Antony via vinayakumarb)
 
+    HADOOP-10652. Refactor Proxyusers to use AccessControlList. (Benoy
+    Antony via Arpit Agarwal)
+
   OPTIMIZATIONS
 
   BUG FIXES 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java

@@ -81,29 +81,42 @@ public class AccessControlList implements Writable {
    * @param aclString String representation of the ACL
    */
   public AccessControlList(String aclString) {
-    buildACL(aclString);
+    buildACL(aclString.split(" ", 2));
+  }
+  
+  /**
+   * Construct a new ACL from String representation of users and groups
+   * 
+   * The arguments are comma separated lists
+   * 
+   * @param users comma separated list of users
+   * @param groups comma separated list of groups
+   */
+  public AccessControlList(String users, String groups) {
+    buildACL(new String[] {users, groups});
   }
 
   /**
-   * Build ACL from the given string, format of the string is
+   * Build ACL from the given two Strings.
-   * user1,...,userN group1,...,groupN
+   * The Strings contain comma separated values.
    *
-   * @param aclString build ACL from this string
+   * @param aclString build ACL from array of Strings
    */
-  private void buildACL(String aclString) {
+  private void buildACL(String[] userGroupStrings) {
     users = new HashSet<String>();
     groups = new HashSet<String>();
-    if (isWildCardACLValue(aclString)) {
+    for (String aclPart : userGroupStrings) {
-      allAllowed = true;
+      if (aclPart != null && isWildCardACLValue(aclPart)) {
-    } else {
+        allAllowed = true;
-      allAllowed = false;
+        break;
-      String[] userGroupStrings = aclString.split(" ", 2);
+      }
-      
+    }
-      if (userGroupStrings.length >= 1) {
+    if (!allAllowed) {      
+      if (userGroupStrings.length >= 1 && userGroupStrings[0] != null) {
         users = StringUtils.getTrimmedStringCollection(userGroupStrings[0]);
       } 
       
-      if (userGroupStrings.length == 2) {
+      if (userGroupStrings.length == 2 && userGroupStrings[1] != null) {
         groups = StringUtils.getTrimmedStringCollection(userGroupStrings[1]);
         groupsMapping.cacheGroupsAdd(new LinkedList<String>(groups));
       }
@@ -294,7 +307,7 @@ public class AccessControlList implements Writable {
   @Override
   public void readFields(DataInput in) throws IOException {
     String aclString = Text.readString(in);
-    buildACL(aclString);
+    buildACL(aclString.split(" ", 2));
   }
 
   /**

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/DefaultImpersonationProvider.java

@@ -20,14 +20,13 @@ package org.apache.hadoop.security.authorize;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.regex.Pattern;
 
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.security.Groups;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.util.StringUtils;
 
@@ -39,11 +38,14 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
   private static final String CONF_GROUPS = ".groups";
   private static final String CONF_HADOOP_PROXYUSER = "hadoop.proxyuser.";
   private static final String CONF_HADOOP_PROXYUSER_RE = "hadoop\\.proxyuser\\.";
-  // list of users, groups and hosts per proxyuser
+  private static final String CONF_HADOOP_PROXYUSER_RE_USERS_GROUPS = 
-  private Map<String, Collection<String>> proxyUsers = 
+      CONF_HADOOP_PROXYUSER_RE+"[^.]*(" + Pattern.quote(CONF_USERS) +
-    new HashMap<String, Collection<String>>(); 
+      "|" + Pattern.quote(CONF_GROUPS) + ")";
-  private Map<String, Collection<String>> proxyGroups = 
+  private static final String CONF_HADOOP_PROXYUSER_RE_HOSTS = 
-    new HashMap<String, Collection<String>>();
+      CONF_HADOOP_PROXYUSER_RE+"[^.]*"+ Pattern.quote(CONF_HOSTS);
+  // acl and list of hosts per proxyuser
+  private Map<String, AccessControlList> proxyUserAcl = 
+    new HashMap<String, AccessControlList>();
   private Map<String, Collection<String>> proxyHosts = 
     new HashMap<String, Collection<String>>();
   private Configuration conf;
@@ -52,28 +54,20 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
   public void setConf(Configuration conf) {
     this.conf = conf;
 
-    // get all the new keys for users
+    // get list of users and groups per proxyuser
-    String regex = CONF_HADOOP_PROXYUSER_RE+"[^.]*\\"+CONF_USERS;
+    Map<String,String> allMatchKeys = 
-    Map<String,String> allMatchKeys = conf.getValByRegex(regex);
+        conf.getValByRegex(CONF_HADOOP_PROXYUSER_RE_USERS_GROUPS); 
     for(Entry<String, String> entry : allMatchKeys.entrySet()) {  
-      Collection<String> users = StringUtils.getTrimmedStringCollection(entry.getValue());
+      String aclKey = getAclKey(entry.getKey());
-      proxyUsers.put(entry.getKey(), users);
+      if (!proxyUserAcl.containsKey(aclKey)) {
+        proxyUserAcl.put(aclKey, new AccessControlList(
+            allMatchKeys.get(aclKey + CONF_USERS) ,
+            allMatchKeys.get(aclKey + CONF_GROUPS)));
+      }
     }
 
-    // get all the new keys for groups
+    // get hosts per proxyuser
-    regex = CONF_HADOOP_PROXYUSER_RE+"[^.]*\\"+CONF_GROUPS;
+    allMatchKeys = conf.getValByRegex(CONF_HADOOP_PROXYUSER_RE_HOSTS);
-    allMatchKeys = conf.getValByRegex(regex);
-    for(Entry<String, String> entry : allMatchKeys.entrySet()) {
-      Collection<String> groups = StringUtils.getTrimmedStringCollection(entry.getValue());
-      proxyGroups.put(entry.getKey(), groups);
-      //cache the groups. This is needed for NetGroups
-      Groups.getUserToGroupsMappingService(conf).cacheGroupsAdd(
-          new ArrayList<String>(groups));
-    }
-
-    // now hosts
-    regex = CONF_HADOOP_PROXYUSER_RE+"[^.]*\\"+CONF_HOSTS;
-    allMatchKeys = conf.getValByRegex(regex);
     for(Entry<String, String> entry : allMatchKeys.entrySet()) {
       proxyHosts.put(entry.getKey(),
           StringUtils.getTrimmedStringCollection(entry.getValue()));
@@ -88,48 +82,22 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
   @Override
   public void authorize(UserGroupInformation user, 
       String remoteAddress) throws AuthorizationException {
-
+    
-    if (user.getRealUser() == null) {
+    UserGroupInformation realUser = user.getRealUser();
+    if (realUser == null) {
       return;
     }
-    boolean userAuthorized = false;
+    
+    AccessControlList acl = proxyUserAcl.get(
+        CONF_HADOOP_PROXYUSER+realUser.getShortUserName());
+    if (acl == null || !acl.isUserAllowed(user)) {
+      throw new AuthorizationException("User: " + realUser.getUserName()
+          + " is not allowed to impersonate " + user.getUserName());
+    }
+
     boolean ipAuthorized = false;
-    UserGroupInformation superUser = user.getRealUser();
-
-    Collection<String> allowedUsers = proxyUsers.get(
-        getProxySuperuserUserConfKey(superUser.getShortUserName()));
-
-    if (isWildcardList(allowedUsers)) {
-      userAuthorized = true;
-    } else if (allowedUsers != null && !allowedUsers.isEmpty()) {
-      if (allowedUsers.contains(user.getShortUserName())) {
-        userAuthorized = true;
-      }
-    }
-
-    if (!userAuthorized){
-      Collection<String> allowedUserGroups = proxyGroups.get(
-          getProxySuperuserGroupConfKey(superUser.getShortUserName()));
-
-      if (isWildcardList(allowedUserGroups)) {
-        userAuthorized = true;
-      } else if (allowedUserGroups != null && !allowedUserGroups.isEmpty()) {
-        for (String group : user.getGroupNames()) {
-          if (allowedUserGroups.contains(group)) {
-            userAuthorized = true;
-            break;
-          }
-        }
-      }
-
-      if (!userAuthorized) {
-        throw new AuthorizationException("User: " + superUser.getUserName()
-            + " is not allowed to impersonate " + user.getUserName());
-      }
-    }
-
     Collection<String> ipList = proxyHosts.get(
-        getProxySuperuserIpConfKey(superUser.getShortUserName()));
+        getProxySuperuserIpConfKey(realUser.getShortUserName()));
 
     if (isWildcardList(ipList)) {
       ipAuthorized = true;
@@ -149,9 +117,17 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
     }
     if(!ipAuthorized) {
       throw new AuthorizationException("Unauthorized connection for super-user: "
-          + superUser.getUserName() + " from IP " + remoteAddress);
+          + realUser.getUserName() + " from IP " + remoteAddress);
     }
   }
+  
+  private String getAclKey(String key) {
+    int endIndex = key.lastIndexOf(".");
+    if (endIndex != -1) {
+      return key.substring(0, endIndex); 
+    }
+    return key;
+  }
 
   /**
    * Return true if the configuration specifies the special configuration value
@@ -193,14 +169,13 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
     return CONF_HADOOP_PROXYUSER+userName+CONF_HOSTS;
   }
 
-  @VisibleForTesting
-  public Map<String, Collection<String>> getProxyUsers() {
-    return proxyUsers;
-  }
-
   @VisibleForTesting
   public Map<String, Collection<String>> getProxyGroups() {
-    return proxyGroups;
+     Map<String,Collection<String>> proxyGroups = new HashMap<String,Collection<String>>();
+     for(Entry<String, AccessControlList> entry : proxyUserAcl.entrySet()) {
+       proxyGroups.put(entry.getKey() + CONF_GROUPS, entry.getValue().getGroups());
+     }
+     return proxyGroups;
   }
 
   @VisibleForTesting

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestProxyUsers.java

@@ -140,7 +140,6 @@ public class TestProxyUsers {
       PROXY_IP);
     ProxyUsers.refreshSuperUserGroupsConfiguration(conf);
 
-
     // First try proxying a group that's allowed
     UserGroupInformation realUserUgi = UserGroupInformation
         .createRemoteUser(REAL_USER_NAME);
@@ -362,6 +361,30 @@ public class TestProxyUsers {
      // From bad IP
      assertNotAuthorized(proxyUserUgi, "1.2.3.5");
    }
+  
+  @Test
+  public void testWithProxyGroupsAndUsersWithSpaces() throws Exception {
+    Configuration conf = new Configuration();
+    conf.set(
+        DefaultImpersonationProvider.getProxySuperuserUserConfKey(REAL_USER_NAME),
+        StringUtils.join(",", Arrays.asList(PROXY_USER_NAME + " ",AUTHORIZED_PROXY_USER_NAME, "ONEMORE")));
+
+    conf.set(
+      DefaultImpersonationProvider.getProxySuperuserGroupConfKey(REAL_USER_NAME),
+      StringUtils.join(",", Arrays.asList(GROUP_NAMES)));
+    
+    conf.set(
+      DefaultImpersonationProvider.getProxySuperuserIpConfKey(REAL_USER_NAME),
+      PROXY_IP);
+    
+    ProxyUsers.refreshSuperUserGroupsConfiguration(conf);
+    
+    Collection<String> groupsToBeProxied = 
+        ProxyUsers.getDefaultImpersonationProvider().getProxyGroups().get(
+        DefaultImpersonationProvider.getProxySuperuserGroupConfKey(REAL_USER_NAME));
+    
+    assertEquals (GROUP_NAMES.length, groupsToBeProxied.size());
+  }
 
 
   private void assertNotAuthorized(UserGroupInformation proxyUgi, String host) {