diff --git a/CHANGES.txt b/CHANGES.txt index 43c079ba90c..0d3b5091818 100644 --- a/CHANGES.txt +++ b/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 diff --git a/src/java/org/apache/hadoop/http/HttpServer.java b/src/java/org/apache/hadoop/http/HttpServer.java index cef289a0481..5906b1fd440 100644 --- a/src/java/org/apache/hadoop/http/HttpServer.java +++ b/src/java/org/apache/hadoop/http/HttpServer.java @@ -709,8 +709,8 @@ public static boolean hasAdministratorAccess( 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; } } diff --git a/src/java/org/apache/hadoop/security/authorize/AccessControlList.java b/src/java/org/apache/hadoop/security/authorize/AccessControlList.java index 6a60d2b6159..e8e0767faae 100644 --- a/src/java/org/apache/hadoop/security/authorize/AccessControlList.java +++ b/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 @@ */ @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 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(); groups = new TreeSet(); if (isWildCardACLValue(aclString)) { allAllowed = true; } else { + allAllowed = false; String[] userGroupStrings = aclString.split(" ", 2); if (userGroupStrings.length >= 1) { @@ -184,31 +196,104 @@ private static final void addToSet(Set set, String[] strings) { } } } - + + /** + * 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 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(); } } \ No newline at end of file diff --git a/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java b/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java index fa51b0e2e2f..a03a61b0c59 100644 --- a/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java +++ b/src/test/core/org/apache/hadoop/security/authorize/TestAccessControlList.java @@ -43,7 +43,46 @@ public void testWildCardAccessControlList() throws Exception { 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 users; @@ -99,22 +138,22 @@ public void testAddRemoveAPI() { AccessControlList acl; Set users; Set 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 void testAddRemoveAPI() { 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 void testAddRemoveAPI() { 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 void testAddRemoveToWildCardACL() { 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());