From 956a9bd40f955f6de525c9e1ce0bab5fafbcc439 Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Wed, 10 Dec 2014 16:19:54 +0530 Subject: [PATCH] HBASE-12348 preModifyColumn and preDeleteColumn in AC denies user to perform its operation though it has required rights --- .../hbase/security/access/AccessController.java | 5 +++-- .../hbase/security/access/TestAccessController.java | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) 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 6f3edd7..9f69b4b 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 @@ -1028,13 +1028,14 @@ public class AccessController extends BaseMasterAndRegionObserver @Override public void preModifyColumn(ObserverContext c, TableName tableName, HColumnDescriptor descriptor) throws IOException { - requirePermission("modifyColumn", tableName, null, null, Action.ADMIN, Action.CREATE); + requirePermission("modifyColumn", tableName, descriptor.getName(), null, Action.ADMIN, + Action.CREATE); } @Override public void preDeleteColumn(ObserverContext c, TableName tableName, byte[] col) throws IOException { - requirePermission("deleteColumn", tableName, null, null, Action.ADMIN, Action.CREATE); + requirePermission("deleteColumn", tableName, col, null, Action.ADMIN, 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 fb7af84..422573c 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 @@ -155,6 +155,8 @@ public class TestAccessController extends SecureTestUtil { private static User USER_CREATE; // user with no permissions private static User USER_NONE; + // user with admin rights on the column family + private static User USER_ADMIN_CF; // TODO: convert this test to cover the full matrix in // https://hbase.apache.org/book/appendix_acl_matrix.html @@ -213,6 +215,7 @@ public class TestAccessController extends SecureTestUtil { USER_OWNER = User.createUserForTesting(conf, "owner", new String[0]); USER_CREATE = User.createUserForTesting(conf, "tbl_create", new String[0]); USER_NONE = User.createUserForTesting(conf, "nouser", new String[0]); + USER_ADMIN_CF = User.createUserForTesting(conf, "col_family_admin", new String[0]); } @AfterClass @@ -261,9 +264,13 @@ public class TestAccessController extends SecureTestUtil { TEST_TABLE.getTableName(), TEST_FAMILY, null, Permission.Action.READ); - assertEquals(4, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size()); + grantOnTable(TEST_UTIL, USER_ADMIN_CF.getShortName(), + TEST_TABLE.getTableName(), TEST_FAMILY, + null, Permission.Action.ADMIN); + + assertEquals(5, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size()); try { - assertEquals(4, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size()); + assertEquals(5, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size()); } catch (Throwable e) { LOG.error("error during call of AccessControlClient.getUserPermissions. ", e); } @@ -378,7 +385,7 @@ public class TestAccessController extends SecureTestUtil { } }; - verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF); verifyDenied(action, USER_RW, USER_RO, USER_NONE); } @@ -393,7 +400,7 @@ public class TestAccessController extends SecureTestUtil { } }; - verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF); verifyDenied(action, USER_RW, USER_RO, USER_NONE); } @@ -2500,8 +2507,8 @@ public class TestAccessController extends SecureTestUtil { null, Action.ADMIN); List perms = testUserPerms.runAs(getPrivilegedAction(regex)); assertNotNull(perms); - // USER_ADMIN, USER_CREATE, USER_RW, USER_RO, testUserPerms has row each. - assertEquals(5, perms.size()); + // USER_ADMIN, USER_CREATE, USER_RW, USER_RO, testUserPerms, USER_ADMIN_CF has row each. + assertEquals(6, perms.size()); } @Test -- 1.9.2.msysgit.0