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 1218368..36a5042 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 @@ -430,6 +430,28 @@ public class AccessController extends BaseMasterAndRegionObserver } } + public void requireNamespacePermission(String request, String namespace, Action... permissions) + throws IOException { + User user = getActiveUser(); + AuthResult result = null; + + for (Action permission : permissions) { + if (authManager.authorize(user, namespace, permission)) { + result = AuthResult.allow(request, "Namespace permission granted", user, + permission, namespace); + break; + } else { + // rest of the world + result = AuthResult.deny(request, "Insufficient permissions", user, + permission, namespace); + } + } + logResult(result); + if (!result.isAllowed()) { + throw new AccessDeniedException("Insufficient permissions " + result.toContextString()); + } + } + /** * Authorizes that the current user has global privileges for the given action. * @param perm The action being requested @@ -476,8 +498,7 @@ public class AccessController extends BaseMasterAndRegionObserver private void requireGlobalPermission(String request, Action perm, TableName tableName, Map> familyMap) throws IOException { User user = getActiveUser(); - if (authManager.authorize(user, perm) || (tableName != null && - authManager.authorize(user, tableName.getNamespaceAsString(), perm))) { + if (authManager.authorize(user, perm)) { logResult(AuthResult.allow(request, "Global check allowed", user, perm, tableName, familyMap)); } else { logResult(AuthResult.deny(request, "Global check failed", user, perm, tableName, familyMap)); @@ -860,7 +881,7 @@ public class AccessController extends BaseMasterAndRegionObserver for (byte[] family: families) { familyMap.put(family, null); } - requireGlobalPermission("createTable", Action.CREATE, desc.getTableName(), familyMap); + requireNamespacePermission("createTable", desc.getTableName().getNamespaceAsString(), Action.CREATE); } @Override diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java index 0aeb346..860c5d8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java @@ -273,7 +273,9 @@ public class TestAccessController extends SecureTestUtil { // Test deleted the table, no problem LOG.info("Test deleted table " + TEST_TABLE.getTableName()); } + // Verify all table/namespace permissions are erased assertEquals(0, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size()); + assertEquals(0, AccessControlLists.getNamespacePermissions(conf, TEST_TABLE.getTableName().getNameAsString()).size()); } @Test diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java index 0f28c66..e6cce0a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java @@ -50,10 +50,6 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; -import java.util.List; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; @Category({SecurityTests.class, MediumTests.class}) public class TestNamespaceCommands extends SecureTestUtil { @@ -71,6 +67,8 @@ public class TestNamespaceCommands extends SecureTestUtil { private static User USER_CREATE; // user with permission on namespace for testing all operations. private static User USER_NSP_WRITE; + // user with admin permission on namespace. + private static User USER_NSP_ADMIN; private static String TEST_TABLE = TestNamespace + ":testtable"; private static byte[] TEST_FAMILY = Bytes.toBytes("f1"); @@ -84,6 +82,7 @@ public class TestNamespaceCommands extends SecureTestUtil { USER_RW = User.createUserForTesting(conf, "rw_user", new String[0]); USER_CREATE = User.createUserForTesting(conf, "create_user", new String[0]); USER_NSP_WRITE = User.createUserForTesting(conf, "namespace_write", new String[0]); + USER_NSP_ADMIN = User.createUserForTesting(conf, "namespace_admin", new String[0]); UTIL.startMiniCluster(); // Wait for the ACL table to become available @@ -96,7 +95,11 @@ public class TestNamespaceCommands extends SecureTestUtil { UTIL.getHBaseAdmin().createNamespace(NamespaceDescriptor.create(TestNamespace).build()); grantOnNamespace(UTIL, USER_NSP_WRITE.getShortName(), - TestNamespace, Permission.Action.WRITE, Permission.Action.CREATE); + TestNamespace, Permission.Action.WRITE); + grantOnNamespace(UTIL, USER_CREATE.getShortName(), + TestNamespace, Permission.Action.CREATE); + grantOnNamespace(UTIL, USER_NSP_ADMIN.getShortName(), + TestNamespace, Permission.Action.ADMIN); } @AfterClass @@ -118,7 +121,7 @@ public class TestNamespaceCommands extends SecureTestUtil { assertTrue(result != null); ListMultimap perms = AccessControlLists.getNamespacePermissions(conf, TestNamespace); - assertEquals(2, perms.size()); + assertEquals(4, perms.size()); List namespacePerms = perms.get(userTestNamespace); assertTrue(perms.containsKey(userTestNamespace)); assertEquals(1, namespacePerms.size()); @@ -134,7 +137,7 @@ public class TestNamespaceCommands extends SecureTestUtil { Permission.Action.WRITE); perms = AccessControlLists.getNamespacePermissions(conf, TestNamespace); - assertEquals(1, perms.size()); + assertEquals(3, perms.size()); } finally { acl.close(); } @@ -152,7 +155,7 @@ public class TestNamespaceCommands extends SecureTestUtil { // verify that superuser or hbase admin can modify namespaces. verifyAllowed(modifyNamespace, SUPERUSER); // all others should be denied - verifyDenied(modifyNamespace, USER_NSP_WRITE, USER_CREATE, USER_RW); + verifyDenied(modifyNamespace, USER_NSP_WRITE, USER_CREATE, USER_RW, USER_NSP_ADMIN); } @Test @@ -196,9 +199,9 @@ public class TestNamespaceCommands extends SecureTestUtil { // Only HBase super user should be able to grant and revoke permissions to // namespaces verifyAllowed(grantAction, SUPERUSER); - verifyDenied(grantAction, USER_CREATE, USER_RW); + verifyDenied(grantAction, USER_CREATE, USER_RW, USER_NSP_ADMIN); verifyAllowed(revokeAction, SUPERUSER); - verifyDenied(revokeAction, USER_CREATE, USER_RW); + verifyDenied(revokeAction, USER_CREATE, USER_RW, USER_NSP_ADMIN); } @Test @@ -212,11 +215,7 @@ public class TestNamespaceCommands extends SecureTestUtil { return null; } }; - - // Only users with create permissions on namespace should be able to create a new table - verifyAllowed(createTable, SUPERUSER, USER_NSP_WRITE); - - // all others should be denied - verifyDenied(createTable, USER_CREATE, USER_RW); + verifyDenied(createTable, USER_RW, USER_NSP_WRITE, USER_NSP_ADMIN); + verifyAllowed(createTable, SUPERUSER, USER_CREATE); } }