diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java index b2a4736..c98bcb2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java @@ -22,10 +22,8 @@ import java.io.ByteArrayInputStream; import java.io.DataInput; import java.io.DataInputStream; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -41,6 +39,7 @@ import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.TableName; @@ -64,8 +63,6 @@ import org.apache.hadoop.hbase.filter.RegexStringComparator; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; -import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService; -import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.regionserver.BloomType; import org.apache.hadoop.hbase.regionserver.InternalScanner; import org.apache.hadoop.hbase.regionserver.Region; @@ -77,11 +74,6 @@ import org.apache.hadoop.io.Text; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; -import com.google.protobuf.ByteString; -import com.google.protobuf.CodedInputStream; -import com.google.protobuf.Message; -import com.google.protobuf.RpcController; -import com.google.protobuf.ServiceException; /** * Maintains lists of permission grants to users and groups to allow for @@ -153,9 +145,10 @@ public class AccessControlLists { * Stores a new user permission grant in the access control lists table. * @param conf the configuration * @param userPerm the details of the permission to be granted + * @param t acl table * @throws IOException in the case of an error accessing the metadata table */ - static void addUserPermission(Configuration conf, UserPermission userPerm) + static void addUserPermission(Configuration conf, UserPermission userPerm, Table t) throws IOException { Permission.Action[] actions = userPerm.getActions(); byte[] rowKey = userPermissionRowKey(userPerm); @@ -179,8 +172,13 @@ public class AccessControlLists { Bytes.toString(key)+": "+Bytes.toStringBinary(value) ); } - // TODO: Pass in a Connection rather than create one each time. - try (Connection connection = ConnectionFactory.createConnection(conf)) { + if (t != null) { + try { + t.put(p); + } finally { + t.close(); + } + } else try (Connection connection = ConnectionFactory.createConnection(conf)) { try (Table table = connection.getTable(ACL_TABLE_NAME)) { table.put(p); } @@ -198,9 +196,10 @@ public class AccessControlLists { * * @param conf the configuration * @param userPerm the details of the permission to be revoked + * @param t acl table * @throws IOException if there is an error accessing the metadata table */ - static void removeUserPermission(Configuration conf, UserPermission userPerm) + static void removeUserPermission(Configuration conf, UserPermission userPerm, Table t) throws IOException { Delete d = new Delete(userPermissionRowKey(userPerm)); byte[] key = userPermissionKey(userPerm); @@ -209,8 +208,13 @@ public class AccessControlLists { LOG.debug("Removing permission "+ userPerm.toString()); } d.addColumns(ACL_LIST_FAMILY, key); - // TODO: Pass in a Connection rather than create one each time. - try (Connection connection = ConnectionFactory.createConnection(conf)) { + if (t != null) { + try { + t.delete(d); + } finally { + t.close(); + } + } else try (Connection connection = ConnectionFactory.createConnection(conf)) { try (Table table = connection.getTable(ACL_TABLE_NAME)) { table.delete(d); } @@ -220,15 +224,20 @@ public class AccessControlLists { /** * Remove specified table from the _acl_ table. */ - static void removeTablePermissions(Configuration conf, TableName tableName) + static void removeTablePermissions(Configuration conf, TableName tableName, Table t) throws IOException{ Delete d = new Delete(tableName.getName()); if (LOG.isDebugEnabled()) { LOG.debug("Removing permissions of removed table "+ tableName); } - // TODO: Pass in a Connection rather than create one each time. - try (Connection connection = ConnectionFactory.createConnection(conf)) { + if (t != null) { + try { + t.delete(d); + } finally { + t.close(); + } + } else try (Connection connection = ConnectionFactory.createConnection(conf)) { try (Table table = connection.getTable(ACL_TABLE_NAME)) { table.delete(d); } @@ -238,7 +247,7 @@ public class AccessControlLists { /** * Remove specified namespace from the acl table. */ - static void removeNamespacePermissions(Configuration conf, String namespace) + static void removeNamespacePermissions(Configuration conf, String namespace, Table t) throws IOException{ Delete d = new Delete(Bytes.toBytes(toNamespaceEntry(namespace))); @@ -246,54 +255,67 @@ public class AccessControlLists { LOG.debug("Removing permissions of removed namespace "+ namespace); } - try (Connection connection = ConnectionFactory.createConnection(conf)) { + if (t != null) { + try { + t.delete(d); + } finally { + t.close(); + } + } else try (Connection connection = ConnectionFactory.createConnection(conf)) { try (Table table = connection.getTable(ACL_TABLE_NAME)) { table.delete(d); } } } + static private void removeTablePermissions(TableName tableName, byte[] column, Table table, + boolean closeTable) throws IOException { + Scan scan = new Scan(); + scan.addFamily(ACL_LIST_FAMILY); + + String columnName = Bytes.toString(column); + scan.setFilter(new QualifierFilter(CompareOp.EQUAL, new RegexStringComparator( + String.format("(%s%s%s)|(%s%s)$", + ACL_KEY_DELIMITER, columnName, ACL_KEY_DELIMITER, + ACL_KEY_DELIMITER, columnName)))); + + Set qualifierSet = new TreeSet(Bytes.BYTES_COMPARATOR); + ResultScanner scanner = null; + try { + scanner = table.getScanner(scan); + for (Result res : scanner) { + for (byte[] q : res.getFamilyMap(ACL_LIST_FAMILY).navigableKeySet()) { + qualifierSet.add(q); + } + } + + if (qualifierSet.size() > 0) { + Delete d = new Delete(tableName.getName()); + for (byte[] qualifier : qualifierSet) { + d.addColumns(ACL_LIST_FAMILY, qualifier); + } + table.delete(d); + } + } finally { + if (scanner != null) scanner.close(); + if (closeTable) table.close(); + } + } + /** * Remove specified table column from the acl table. */ - static void removeTablePermissions(Configuration conf, TableName tableName, byte[] column) - throws IOException{ - + static void removeTablePermissions(Configuration conf, TableName tableName, byte[] column, + Table t) throws IOException { if (LOG.isDebugEnabled()) { LOG.debug("Removing permissions of removed column " + Bytes.toString(column) + " from table "+ tableName); } - // TODO: Pass in a Connection rather than create one each time. - try (Connection connection = ConnectionFactory.createConnection(conf)) { + if (t != null) { + removeTablePermissions(tableName, column, t, true); + } else try (Connection connection = ConnectionFactory.createConnection(conf)) { try (Table table = connection.getTable(ACL_TABLE_NAME)) { - Scan scan = new Scan(); - scan.addFamily(ACL_LIST_FAMILY); - - String columnName = Bytes.toString(column); - scan.setFilter(new QualifierFilter(CompareOp.EQUAL, new RegexStringComparator( - String.format("(%s%s%s)|(%s%s)$", - ACL_KEY_DELIMITER, columnName, ACL_KEY_DELIMITER, - ACL_KEY_DELIMITER, columnName)))); - - Set qualifierSet = new TreeSet(Bytes.BYTES_COMPARATOR); - ResultScanner scanner = table.getScanner(scan); - try { - for (Result res : scanner) { - for (byte[] q : res.getFamilyMap(ACL_LIST_FAMILY).navigableKeySet()) { - qualifierSet.add(q); - } - } - } finally { - scanner.close(); - } - - if (qualifierSet.size() > 0) { - Delete d = new Delete(tableName.getName()); - for (byte[] qualifier : qualifierSet) { - d.addColumns(ACL_LIST_FAMILY, qualifier); - } - table.delete(d); - } + removeTablePermissions(tableName, column, t, false); } } } @@ -442,12 +464,12 @@ public class AccessControlLists { static ListMultimap getTablePermissions(Configuration conf, TableName tableName) throws IOException { - return getPermissions(conf, tableName != null ? tableName.getName() : null); + return getPermissions(conf, tableName != null ? tableName.getName() : null, null); } static ListMultimap getNamespacePermissions(Configuration conf, String namespace) throws IOException { - return getPermissions(conf, Bytes.toBytes(toNamespaceEntry(namespace))); + return getPermissions(conf, Bytes.toBytes(toNamespaceEntry(namespace)), null); } /** @@ -460,24 +482,32 @@ public class AccessControlLists { *

*/ static ListMultimap getPermissions(Configuration conf, - byte[] entryName) throws IOException { + byte[] entryName, Table t) throws IOException { if (entryName == null) entryName = ACL_GLOBAL_NAME; // for normal user tables, we just read the table row from _acl_ ListMultimap perms = ArrayListMultimap.create(); - // TODO: Pass in a Connection rather than create one each time. - try (Connection connection = ConnectionFactory.createConnection(conf)) { - try (Table table = connection.getTable(ACL_TABLE_NAME)) { - Get get = new Get(entryName); - get.addFamily(ACL_LIST_FAMILY); - Result row = table.get(get); - if (!row.isEmpty()) { - perms = parsePermissions(entryName, row); - } else { - LOG.info("No permissions found in " + ACL_TABLE_NAME + " for acl entry " - + Bytes.toString(entryName)); + Get get = new Get(entryName); + get.addFamily(ACL_LIST_FAMILY); + Result row = null; + if (t == null) { + try (Connection connection = ConnectionFactory.createConnection(conf)) { + try (Table table = connection.getTable(ACL_TABLE_NAME)) { + row = table.get(get); } } + } else { + try { + row = t.get(get); + } finally { + t.close(); + } + } + if (!row.isEmpty()) { + perms = parsePermissions(entryName, row); + } else { + LOG.info("No permissions found in " + ACL_TABLE_NAME + " for acl entry " + + Bytes.toString(entryName)); } return perms; @@ -501,7 +531,7 @@ public class AccessControlLists { Configuration conf, byte[] entryName) throws IOException { ListMultimap allPerms = getPermissions( - conf, entryName); + conf, entryName, null); List perms = new ArrayList(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 1163c44..cea3333 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -272,7 +272,8 @@ public class AccessController extends BaseMasterAndRegionObserver for (byte[] entry: entries) { try { ListMultimap perms = - AccessControlLists.getPermissions(conf, entry); + AccessControlLists.getPermissions(conf, entry, + regionEnv.getTable(e.getRegion().getRegionInfo().getTable())); byte[] serialized = AccessControlLists.writePermissionsAsBytes(perms, conf); zkw.writeToZookeeper(entry, serialized); } catch (IOException ex) { @@ -1033,7 +1034,7 @@ public class AccessController extends BaseMasterAndRegionObserver @Override public Void run() throws Exception { AccessControlLists.addUserPermission(c.getEnvironment().getConfiguration(), - userperm); + userperm, c.getEnvironment().getTable(AccessControlLists.ACL_TABLE_NAME)); return null; } }); @@ -1055,7 +1056,8 @@ public class AccessController extends BaseMasterAndRegionObserver User.runAsLoginUser(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { - AccessControlLists.removeTablePermissions(conf, tableName); + AccessControlLists.removeTablePermissions(conf, tableName, + c.getEnvironment().getTable(AccessControlLists.ACL_TABLE_NAME)); return null; } }); @@ -1091,7 +1093,8 @@ public class AccessController extends BaseMasterAndRegionObserver List perms = tableAcls.get(tableName); if (perms != null) { for (UserPermission perm : perms) { - AccessControlLists.addUserPermission(conf, perm); + AccessControlLists.addUserPermission(conf, perm, + ctx.getEnvironment().getTable(AccessControlLists.ACL_TABLE_NAME)); } } tableAcls.remove(tableName); @@ -1119,7 +1122,8 @@ public class AccessController extends BaseMasterAndRegionObserver public Void run() throws Exception { UserPermission userperm = new UserPermission(Bytes.toBytes(owner), htd.getTableName(), null, Action.values()); - AccessControlLists.addUserPermission(conf, userperm); + AccessControlLists.addUserPermission(conf, userperm, + c.getEnvironment().getTable(AccessControlLists.ACL_TABLE_NAME)); return null; } }); @@ -1155,7 +1159,8 @@ public class AccessController extends BaseMasterAndRegionObserver User.runAsLoginUser(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { - AccessControlLists.removeTablePermissions(conf, tableName, columnFamily); + AccessControlLists.removeTablePermissions(conf, tableName, columnFamily, + ctx.getEnvironment().getTable(AccessControlLists.ACL_TABLE_NAME)); return null; } }); @@ -1383,7 +1388,8 @@ public class AccessController extends BaseMasterAndRegionObserver User.runAsLoginUser(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { - AccessControlLists.removeNamespacePermissions(conf, namespace); + AccessControlLists.removeNamespacePermissions(conf, namespace, + ctx.getEnvironment().getTable(AccessControlLists.ACL_TABLE_NAME)); return null; } }); @@ -2232,7 +2238,9 @@ public class AccessController extends BaseMasterAndRegionObserver User.runAsLoginUser(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { - AccessControlLists.addUserPermission(regionEnv.getConfiguration(), perm); + AccessControlLists.addUserPermission(regionEnv.getConfiguration(), perm, + regionEnv == null ? null : + regionEnv.getTable(regionEnv.getRegion().getRegionInfo().getTable())); return null; } }); @@ -2284,7 +2292,9 @@ public class AccessController extends BaseMasterAndRegionObserver User.runAsLoginUser(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { - AccessControlLists.removeUserPermission(regionEnv.getConfiguration(), perm); + AccessControlLists.removeUserPermission(regionEnv.getConfiguration(), perm, + regionEnv == null ? null : + regionEnv.getTable(regionEnv.getRegion().getRegionInfo().getTable())); return null; } }); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java index f7def51..0da4af6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java @@ -114,9 +114,9 @@ public class TestTablePermissions { @After public void tearDown() throws Exception { Configuration conf = UTIL.getConfiguration(); - AccessControlLists.removeTablePermissions(conf, TEST_TABLE); - AccessControlLists.removeTablePermissions(conf, TEST_TABLE2); - AccessControlLists.removeTablePermissions(conf, AccessControlLists.ACL_TABLE_NAME); + AccessControlLists.removeTablePermissions(conf, TEST_TABLE, null); + AccessControlLists.removeTablePermissions(conf, TEST_TABLE2, null); + AccessControlLists.removeTablePermissions(conf, AccessControlLists.ACL_TABLE_NAME, null); } /** @@ -174,14 +174,14 @@ public class TestTablePermissions { // add some permissions AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("george"), TEST_TABLE, null, (byte[])null, - UserPermission.Action.READ, UserPermission.Action.WRITE)); + UserPermission.Action.READ, UserPermission.Action.WRITE), null); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("hubert"), TEST_TABLE, null, (byte[])null, - UserPermission.Action.READ)); + UserPermission.Action.READ), null); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("humphrey"), TEST_TABLE, TEST_FAMILY, TEST_QUALIFIER, - UserPermission.Action.READ)); + UserPermission.Action.READ), null); // retrieve the same ListMultimap perms = @@ -237,7 +237,7 @@ public class TestTablePermissions { // table 2 permissions AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("hubert"), TEST_TABLE2, null, (byte[])null, - TablePermission.Action.READ, TablePermission.Action.WRITE)); + TablePermission.Action.READ, TablePermission.Action.WRITE), null); // check full load Map> allPerms = @@ -269,19 +269,19 @@ public class TestTablePermissions { Configuration conf = UTIL.getConfiguration(); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("albert"), TEST_TABLE, null, - (byte[])null, TablePermission.Action.READ)); + (byte[])null, TablePermission.Action.READ), null); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("betty"), TEST_TABLE, null, (byte[])null, TablePermission.Action.READ, - TablePermission.Action.WRITE)); + TablePermission.Action.WRITE), null); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("clark"), TEST_TABLE, TEST_FAMILY, - TablePermission.Action.READ)); + TablePermission.Action.READ), null); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("dwight"), TEST_TABLE, TEST_FAMILY, TEST_QUALIFIER, - TablePermission.Action.WRITE)); + TablePermission.Action.WRITE), null); // verify permissions survive changes in table metadata ListMultimap preperms = @@ -397,13 +397,13 @@ public class TestTablePermissions { // add some permissions AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("user1"), - Permission.Action.READ, Permission.Action.WRITE)); + Permission.Action.READ, Permission.Action.WRITE), null); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("user2"), - Permission.Action.CREATE)); + Permission.Action.CREATE), null); AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("user3"), - Permission.Action.ADMIN, Permission.Action.READ, Permission.Action.CREATE)); + Permission.Action.ADMIN, Permission.Action.READ, Permission.Action.CREATE), null); ListMultimap perms = AccessControlLists.getTablePermissions(conf, null); List user1Perms = perms.get("user1"); @@ -439,7 +439,7 @@ public class TestTablePermissions { assertTrue(authManager.authorize(currentUser, Permission.Action.ADMIN)); for (int i=1; i<=50; i++) { AccessControlLists.addUserPermission(conf, new UserPermission(Bytes.toBytes("testauth"+i), - Permission.Action.ADMIN, Permission.Action.READ, Permission.Action.WRITE)); + Permission.Action.ADMIN, Permission.Action.READ, Permission.Action.WRITE), null); // make sure the system user still shows as authorized assertTrue("Failed current user auth check on iter "+i, authManager.authorize(currentUser, Permission.Action.ADMIN));