HADOOP-15836. Review of AccessControlList. Contributed by BELUGA BEHR.

This commit is contained in:
Inigo Goiri 2018-10-19 13:57:43 -07:00
parent e2cecb681e
commit 00254d7b8c
2 changed files with 123 additions and 150 deletions

View File

@ -20,11 +20,13 @@
import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Collections;
import java.util.Set;
import java.util.TreeSet;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
@ -34,7 +36,6 @@
import org.apache.hadoop.io.WritableFactory;
import org.apache.hadoop.security.Groups;
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.util.StringUtils;
/**
* Class representing a configured access control list.
@ -57,9 +58,9 @@ public class AccessControlList implements Writable {
private static final int INITIAL_CAPACITY = 256;
// Set of users who are granted access.
private Collection<String> users;
private final Set<String> users = new TreeSet<>();
// Set of groups which are granted access
private Collection<String> groups;
private final Set<String> groups = new TreeSet<>();
// Whether all users are granted access.
private boolean allAllowed;
@ -85,7 +86,7 @@ public AccessControlList(String aclString) {
}
/**
* Construct a new ACL from String representation of users and groups
* Construct a new ACL from String representation of users and groups.
*
* The arguments are comma separated lists
*
@ -103,38 +104,39 @@ public AccessControlList(String users, String groups) {
* @param aclString build ACL from array of Strings
*/
private void buildACL(String[] userGroupStrings) {
users = new HashSet<String>();
groups = new HashSet<String>();
for (String aclPart : userGroupStrings) {
if (aclPart != null && isWildCardACLValue(aclPart)) {
allAllowed = true;
break;
return;
}
}
if (!allAllowed) {
if (userGroupStrings.length >= 1 && userGroupStrings[0] != null) {
users = StringUtils.getTrimmedStringCollection(userGroupStrings[0]);
String[] userList = userGroupStrings[0].split(",");
for (String user : userList) {
if (StringUtils.isNotBlank(user)) {
users.add(user.trim());
}
}
}
if (userGroupStrings.length == 2 && userGroupStrings[1] != null) {
groups = StringUtils.getTrimmedStringCollection(userGroupStrings[1]);
groupsMapping.cacheGroupsAdd(new LinkedList<String>(groups));
String[] groupList = userGroupStrings[1].split(",");
for (String group : groupList) {
if (StringUtils.isNotBlank(group)) {
groups.add(group.trim());
}
}
groupsMapping.cacheGroupsAdd(new ArrayList<>(groups));
}
}
/**
* Checks whether ACL string contains wildcard
* Checks whether ACL string contains wildcard.
*
* @param aclString check this ACL string for wildcard
* @return true if ACL string contains wildcard false otherwise
*/
private boolean isWildCardACLValue(String aclString) {
if (aclString.contains(WILDCARD_ACL_VALUE) &&
aclString.trim().equals(WILDCARD_ACL_VALUE)) {
return true;
}
return false;
return WILDCARD_ACL_VALUE.equals(aclString.trim());
}
public boolean isAllAllowed() {
@ -144,8 +146,7 @@ public boolean isAllAllowed() {
/**
* Add user to the names of users allowed for this service.
*
* @param user
* The user name
* @param user The user name
*/
public void addUser(String user) {
if (isWildCardACLValue(user)) {
@ -159,17 +160,15 @@ public void addUser(String user) {
/**
* Add group to the names of groups allowed for this service.
*
* @param group
* The group name
* @param group The group name
*/
public void addGroup(String group) {
if (isWildCardACLValue(group)) {
throw new IllegalArgumentException("Group " + group + " can not be added");
throw new IllegalArgumentException(
"Group " + group + " can not be added");
}
if (!isAllAllowed()) {
List<String> groupsList = new LinkedList<String>();
groupsList.add(group);
groupsMapping.cacheGroupsAdd(groupsList);
groupsMapping.cacheGroupsAdd(Collections.singletonList(group));
groups.add(group);
}
}
@ -177,8 +176,7 @@ public void addGroup(String group) {
/**
* Remove user from the names of users allowed for this service.
*
* @param user
* The user name
* @param user The user name
*/
public void removeUser(String user) {
if (isWildCardACLValue(user)) {
@ -192,13 +190,12 @@ public void removeUser(String user) {
/**
* Remove group from the names of groups allowed for this service.
*
* @param group
* The group name
* @param group The group name
*/
public void removeGroup(String group) {
if (isWildCardACLValue(group)) {
throw new IllegalArgumentException("Group " + group
+ " can not be removed");
throw new IllegalArgumentException(
"Group " + group + " can not be removed");
}
if (!isAllAllowed()) {
groups.remove(group);
@ -207,18 +204,20 @@ public void removeGroup(String group) {
/**
* Get the names of users allowed for this service.
* @return the set of user names. the set must not be modified.
*
* @return an unmodifiable set of user names in alphabetic order.
*/
public Collection<String> getUsers() {
return users;
return Collections.unmodifiableSet(users);
}
/**
* Get the names of user groups allowed for this service.
* @return the set of group names. the set must not be modified.
*
* @return an unmodifiable set of group names in alphabetic order.
*/
public Collection<String> getGroups() {
return groups;
return Collections.unmodifiableSet(groups);
}
/**
@ -230,7 +229,8 @@ public Collection<String> getGroups() {
public final boolean isUserInList(UserGroupInformation ugi) {
if (allAllowed || users.contains(ugi.getShortUserName())) {
return true;
} else if (!groups.isEmpty()) {
}
if (!groups.isEmpty()) {
for (String group : ugi.getGroups()) {
if (groups.contains(group)) {
return true;
@ -326,7 +326,7 @@ public void readFields(DataInput in) throws IOException {
* @return comma separated list of users
*/
private String getUsersString() {
return getString(users);
return String.join(",", users);
}
/**
@ -335,26 +335,6 @@ private String getUsersString() {
* @return comma separated list of groups
*/
private String getGroupsString() {
return getString(groups);
}
/**
* Returns comma-separated concatenated single String of all strings of
* the given set
*
* @param strings set of strings to concatenate
*/
private String getString(Collection<String> strings) {
StringBuilder sb = new StringBuilder(INITIAL_CAPACITY);
boolean first = true;
for(String str: strings) {
if (!first) {
sb.append(",");
} else {
first = false;
}
sb.append(str);
}
return sb.toString();
return String.join(",", groups);
}
}

View File

@ -21,9 +21,11 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import org.apache.hadoop.classification.InterfaceAudience;
@ -37,9 +39,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import com.google.common.collect.Iterables;
@InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"})
@InterfaceStability.Evolving
@ -202,14 +202,14 @@ public void testAclString() {
validateGetAclString(acl);
acl = new AccessControlList(" group1,group2");
assertTrue(acl.toString().equals(
"Members of the groups [group1, group2] are allowed"));
assertEquals("Members of the groups [group1, group2] are allowed",
acl.toString());
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"));
assertEquals("Users [user1, user2] and members of the groups "
+ "[group1, group2] are allowed", acl.toString());
validateGetAclString(acl);
}
@ -228,45 +228,42 @@ public void testAccessControlList() throws Exception {
acl = new AccessControlList("drwho tardis");
users = acl.getUsers();
assertEquals(users.size(), 1);
assertEquals(users.iterator().next(), "drwho");
assertEquals(1, users.size());
assertEquals("drwho", Iterables.getOnlyElement(users));
groups = acl.getGroups();
assertEquals(groups.size(), 1);
assertEquals(groups.iterator().next(), "tardis");
assertEquals(1, groups.size());
assertEquals("tardis", Iterables.getOnlyElement(groups));
acl = new AccessControlList("drwho");
users = acl.getUsers();
assertEquals(users.size(), 1);
assertEquals(users.iterator().next(), "drwho");
assertEquals(1, users.size());
assertEquals("drwho", Iterables.getOnlyElement(users));
groups = acl.getGroups();
assertEquals(groups.size(), 0);
assertEquals(0, groups.size());
acl = new AccessControlList("drwho ");
users = acl.getUsers();
assertEquals(users.size(), 1);
assertEquals(users.iterator().next(), "drwho");
assertEquals(1, users.size());
assertEquals("drwho", Iterables.getOnlyElement(users));
groups = acl.getGroups();
assertEquals(groups.size(), 0);
assertEquals(0, groups.size());
acl = new AccessControlList(" tardis");
users = acl.getUsers();
assertEquals(users.size(), 0);
assertEquals(0, users.size());
groups = acl.getGroups();
assertEquals(groups.size(), 1);
assertEquals(groups.iterator().next(), "tardis");
assertEquals(1, groups.size());
assertEquals("tardis", Iterables.getOnlyElement(groups));
Iterator<String> iter;
acl = new AccessControlList("drwho,joe tardis, users");
users = acl.getUsers();
assertEquals(users.size(), 2);
iter = users.iterator();
assertEquals(iter.next(), "drwho");
assertEquals(iter.next(), "joe");
assertEquals(2, users.size());
assertTrue(users.contains("drwho"));
assertTrue(users.contains("joe"));
groups = acl.getGroups();
assertEquals(groups.size(), 2);
iter = groups.iterator();
assertEquals(iter.next(), "tardis");
assertEquals(iter.next(), "users");
assertEquals(2, groups.size());
assertTrue(groups.contains("tardis"));
assertTrue(groups.contains("users"));
}
/**
@ -284,49 +281,45 @@ public void testAddRemoveAPI() {
acl.addUser("drwho");
users = acl.getUsers();
assertEquals(users.size(), 1);
assertEquals(users.iterator().next(), "drwho");
assertEquals(1, users.size());
assertEquals("drwho", Iterables.getOnlyElement(users));
assertEquals("drwho ", acl.getAclString());
acl.addGroup("tardis");
groups = acl.getGroups();
assertEquals(groups.size(), 1);
assertEquals(groups.iterator().next(), "tardis");
assertEquals(1, groups.size());
assertEquals("tardis", Iterables.getOnlyElement(groups));
assertEquals("drwho tardis", acl.getAclString());
acl.addUser("joe");
acl.addGroup("users");
users = acl.getUsers();
assertEquals(users.size(), 2);
Iterator<String> iter = users.iterator();
assertEquals(iter.next(), "drwho");
assertEquals(iter.next(), "joe");
assertEquals(2, users.size());
assertTrue(users.contains("drwho"));
assertTrue(users.contains("joe"));
groups = acl.getGroups();
assertEquals(groups.size(), 2);
iter = groups.iterator();
assertEquals(iter.next(), "tardis");
assertEquals(iter.next(), "users");
assertEquals("drwho,joe tardis,users", acl.getAclString());
assertEquals(2, groups.size());
assertTrue(groups.contains("tardis"));
assertTrue(groups.contains("users"));
acl.removeUser("joe");
acl.removeGroup("users");
users = acl.getUsers();
assertEquals(users.size(), 1);
assertFalse(users.contains("joe"));
assertEquals(1, users.size());
assertEquals("drwho", Iterables.getOnlyElement(users));
groups = acl.getGroups();
assertEquals(groups.size(), 1);
assertFalse(groups.contains("users"));
assertEquals(1, groups.size());
assertEquals("tardis", Iterables.getOnlyElement(groups));
assertEquals("drwho tardis", acl.getAclString());
acl.removeGroup("tardis");
groups = acl.getGroups();
assertEquals(0, groups.size());
assertFalse(groups.contains("tardis"));
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.getAclString());