HADOOP-6715. Fixes AccessControlList.toString() to return a descriptive String representation of the ACL. Contributed by Ravi Gummadi

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@984652 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -188,6 +188,9 @@ Trunk (unreleased changes)
     HADOOP-6706. Improves the sasl failure handling due to expired tickets,
     and other server detected failures. (Jitendra Pandey and ddas via ddas)
 
+    HADOOP-6715. Fixes AccessControlList.toString() to return a descriptive
+    String representation of the ACL. (Ravi Gummadi via amareshwari)
+
 Release 0.21.0 - Unreleased
 
   INCOMPATIBLE CHANGES

src/java/org/apache/hadoop/http/HttpServer.java

@@ -709,8 +709,8 @@ public class HttpServer implements FilterContainer {
       if (!adminsAcl.isUserAllowed(remoteUserUGI)) {
         response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User "
             + remoteUser + " is unauthorized to access this page. "
-            + "Only \"" + adminsAcl.toString()
-            + "\" can access this page.");
+            + "AccessControlList for accessing this page : "
+            + adminsAcl.toString());
         return false;
       }
     }

src/java/org/apache/hadoop/security/authorize/AccessControlList.java

@@ -17,11 +17,16 @@
  */
 package org.apache.hadoop.security.authorize;
 
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
 import java.util.Set;
 import java.util.TreeSet;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.io.Text;
+import org.apache.hadoop.io.Writable;
 import org.apache.hadoop.security.UserGroupInformation;
 
 /**
@@ -29,10 +34,11 @@ import org.apache.hadoop.security.UserGroupInformation;
  */
 @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"})
 @InterfaceStability.Evolving
-public class AccessControlList {
+public class AccessControlList implements Writable {
   
   // Indicates an ACL string that represents access to all users
   public static final String WILDCARD_ACL_VALUE = "*";
+  private static final int INITIAL_CAPACITY = 256;
 
   // Set of users who are granted access.
   private Set<String> users;
@@ -51,11 +57,17 @@ public class AccessControlList {
    * @param aclString String representation of the ACL
    */
   public AccessControlList(String aclString) {
+    buildACL(aclString);
+  }
+
+  // build ACL from the given string
+  private void buildACL(String aclString) {
     users = new TreeSet<String>();
     groups = new TreeSet<String>();
     if (isWildCardACLValue(aclString)) {
       allAllowed = true;
     } else {
+      allAllowed = false;
       String[] userGroupStrings = aclString.split(" ", 2);
       
       if (userGroupStrings.length >= 1) {
@@ -184,31 +196,104 @@ public class AccessControlList {
       }
     }
   }
-  
+
+  /**
+   * Returns descriptive way of users and groups that are part of this ACL.
+   * Use {@link #getAclString()} to get the exact String that can be given to
+   * the constructor of AccessControlList to create a new instance.
+   */
   @Override
   public String toString() {
-    StringBuilder sb = new StringBuilder();
-    boolean first = true;
-    for(String user: users) {
-      if (!first) {
-        sb.append(",");
-      } else {
-        first = false;
-      }
-      sb.append(user);
+    String str = null;
+
+    if (allAllowed) {
+      str = "All users are allowed";
     }
-    if (!groups.isEmpty()) {
+    else if (users.isEmpty() && groups.isEmpty()) {
+      str = "No users are allowed";
+    }
+    else {
+      String usersStr = null;
+      String groupsStr = null;
+      if (!users.isEmpty()) {
+        usersStr = users.toString();
+      }
+      if (!groups.isEmpty()) {
+        groupsStr = groups.toString();
+      }
+
+      if (!users.isEmpty() && !groups.isEmpty()) {
+        str = "Users " + usersStr + " and members of the groups "
+            + groupsStr + " are allowed";
+      }
+      else if (!users.isEmpty()) {
+        str = "Users " + usersStr + " are allowed";
+      }
+      else {// users is empty array and groups is nonempty
+        str = "Members of the groups "
+            + groupsStr + " are allowed";
+      }
+    }
+
+    return str;
+  }
+
+  /**
+   * Returns the access control list as a String that can be used for building a
+   * new instance by sending it to the constructor of {@link AccessControlList}.
+   */
+  public String getAclString() {
+    StringBuilder sb = new StringBuilder(INITIAL_CAPACITY);
+    if (allAllowed) {
+      sb.append('*');
+    }
+    else {
+      sb.append(getUsersString());
       sb.append(" ");
+      sb.append(getGroupsString());
     }
-    first = true;
-    for(String group: groups) {
+    return sb.toString();
+  }
+
+  /**
+   * Serializes the AccessControlList object
+   */
+  public void write(DataOutput out) throws IOException {
+    String aclString = getAclString();
+    Text.writeString(out, aclString);
+  }
+
+  /**
+   * Deserializes the AccessControlList object
+   */
+  public void readFields(DataInput in) throws IOException {
+    String aclString = Text.readString(in);
+    buildACL(aclString);
+  }
+
+  // Returns comma-separated concatenated single String of the set 'users'
+  private String getUsersString() {
+    return getString(users);
+  }
+
+  // Returns comma-separated concatenated single String of the set 'groups'
+  private String getGroupsString() {
+    return getString(groups);
+  }
+
+  // Returns comma-separated concatenated single String of all strings of
+  // the given set
+  private String getString(Set<String> strings) {
+    StringBuilder sb = new StringBuilder(INITIAL_CAPACITY);
+    boolean first = true;
+    for(String str: strings) {
       if (!first) {
         sb.append(",");
       } else {
         first = false;
       }
-      sb.append(group);
+      sb.append(str);
     }
-    return sb.toString();    
+    return sb.toString();
   }
 }

src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java

@@ -43,7 +43,46 @@ public class TestAccessControlList extends TestCase {
     acl = new AccessControlList("*  ");
     assertTrue(acl.isAllAllowed());
   }
-  
+
+  // Check if AccessControlList.toString() works as expected.
+  // Also validate if getAclString() for various cases.
+  public void testAclString() {
+    AccessControlList acl;
+
+    acl = new AccessControlList("*");
+    assertTrue(acl.toString().equals("All users are allowed"));
+    validateGetAclString(acl);
+
+    acl = new AccessControlList(" ");
+    assertTrue(acl.toString().equals("No users are allowed"));
+
+    acl = new AccessControlList("user1,user2");
+    assertTrue(acl.toString().equals("Users [user1, user2] are allowed"));
+    validateGetAclString(acl);
+
+    acl = new AccessControlList("user1,user2 ");// with space
+    assertTrue(acl.toString().equals("Users [user1, user2] are allowed"));
+    validateGetAclString(acl);
+
+    acl = new AccessControlList(" group1,group2");
+    assertTrue(acl.toString().equals(
+        "Members of the groups [group1, group2] are allowed"));
+    validateGetAclString(acl);
+
+    acl = new AccessControlList("user1,user2 group1,group2");
+    assertTrue(acl.toString().equals(
+        "Users [user1, user2] and " +
+        "members of the groups [group1, group2] are allowed"));
+    validateGetAclString(acl);
+  }
+
+  // Validates if getAclString() is working as expected. i.e. if we can build
+  // a new ACL instance from the value returned by getAclString().
+  private void validateGetAclString(AccessControlList acl) {
+    assertTrue(acl.toString().equals(
+        new AccessControlList(acl.getAclString()).toString()));
+  }
+
   public void testAccessControlList() throws Exception {
     AccessControlList acl;
     Set<String> users;
@@ -99,22 +138,22 @@ public class TestAccessControlList extends TestCase {
     AccessControlList acl;
     Set<String> users;
     Set<String> groups;
-    acl = new AccessControlList("");
+    acl = new AccessControlList(" ");
     assertEquals(0, acl.getUsers().size());
     assertEquals(0, acl.getGroups().size());
-    assertEquals("", acl.toString());
+    assertEquals(" ", acl.getAclString());
     
     acl.addUser("drwho");
     users = acl.getUsers();
     assertEquals(users.size(), 1);
     assertEquals(users.iterator().next(), "drwho");
-    assertEquals("drwho", acl.toString());
+    assertEquals("drwho ", acl.getAclString());
     
     acl.addGroup("tardis");
     groups = acl.getGroups();
     assertEquals(groups.size(), 1);
     assertEquals(groups.iterator().next(), "tardis");
-    assertEquals("drwho tardis", acl.toString());
+    assertEquals("drwho tardis", acl.getAclString());
     
     acl.addUser("joe");
     acl.addGroup("users");
@@ -128,7 +167,7 @@ public class TestAccessControlList extends TestCase {
     iter = groups.iterator();
     assertEquals(iter.next(), "tardis");
     assertEquals(iter.next(), "users");
-    assertEquals("drwho,joe tardis,users", acl.toString());
+    assertEquals("drwho,joe tardis,users", acl.getAclString());
 
     acl.removeUser("joe");
     acl.removeGroup("users");
@@ -138,20 +177,20 @@ public class TestAccessControlList extends TestCase {
     groups = acl.getGroups();
     assertEquals(groups.size(), 1);
     assertFalse(groups.contains("users"));
-    assertEquals("drwho tardis", acl.toString());
+    assertEquals("drwho tardis", acl.getAclString());
     
     acl.removeGroup("tardis");
     groups = acl.getGroups();
     assertEquals(0, groups.size());
     assertFalse(groups.contains("tardis"));
-    assertEquals("drwho", acl.toString());
+    assertEquals("drwho ", acl.getAclString());
     
     acl.removeUser("drwho");
     assertEquals(0, users.size());
     assertFalse(users.contains("drwho"));
     assertEquals(0, acl.getGroups().size());
     assertEquals(0, acl.getUsers().size());
-    assertEquals("", acl.toString());
+    assertEquals(" ", acl.getAclString());
   }
   
   /**
@@ -211,10 +250,10 @@ public class TestAccessControlList extends TestCase {
 
     acl.addUser("drwho");
     assertTrue(acl.isAllAllowed());
-    assertFalse(acl.toString().contains("drwho"));
+    assertFalse(acl.getAclString().contains("drwho"));
     acl.addGroup("tardis");
     assertTrue(acl.isAllAllowed());
-    assertFalse(acl.toString().contains("tardis"));
+    assertFalse(acl.getAclString().contains("tardis"));
    
     acl.removeUser("drwho");
     assertTrue(acl.isAllAllowed());