HBASE-20639 Implement permission checking through AccessController instead of RSGroupAdminEndpoint - revert due to pending discussion
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/266b251d Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/266b251d Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/266b251d Branch: refs/heads/HBASE-19064 Commit: 266b251dfa0d99ef5e770f933ac29c8fa8ab16af Parents: fe73fe8 Author: tedyu <yuzhih...@gmail.com> Authored: Tue May 29 19:57:51 2018 -0700 Committer: tedyu <yuzhih...@gmail.com> Committed: Tue May 29 19:57:51 2018 -0700 ---------------------------------------------------------------------- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 7 ++ .../hbase/rsgroup/TestRSGroupsWithACL.java | 82 +++++++++----------- .../hbase/security/access/AccessController.java | 43 ---------- 3 files changed, 42 insertions(+), 90 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/266b251d/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java ---------------------------------------------------------------------- diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java index 8546a40..fa7537a 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java @@ -205,6 +205,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preMoveServers(hostPorts, request.getTargetGroup()); } + checkPermission("moveServers"); groupAdminServer.moveServers(hostPorts, request.getTargetGroup()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postMoveServers(hostPorts, request.getTargetGroup()); @@ -229,6 +230,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preMoveTables(tables, request.getTargetGroup()); } + checkPermission("moveTables"); groupAdminServer.moveTables(tables, request.getTargetGroup()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postMoveTables(tables, request.getTargetGroup()); @@ -248,6 +250,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preAddRSGroup(request.getRSGroupName()); } + checkPermission("addRSGroup"); groupAdminServer.addRSGroup(request.getRSGroupName()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postAddRSGroup(request.getRSGroupName()); @@ -268,6 +271,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preRemoveRSGroup(request.getRSGroupName()); } + checkPermission("removeRSGroup"); groupAdminServer.removeRSGroup(request.getRSGroupName()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postRemoveRSGroup(request.getRSGroupName()); @@ -288,6 +292,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName()); } + checkPermission("balanceRSGroup"); boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName()); builder.setBalanceRan(balancerRan); if (master.getMasterCoprocessorHost() != null) { @@ -356,6 +361,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { master.getMasterCoprocessorHost().preMoveServersAndTables(hostPorts, tables, request.getTargetGroup()); } + checkPermission("moveServersAndTables"); groupAdminServer.moveServersAndTables(hostPorts, tables, request.getTargetGroup()); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postMoveServersAndTables(hostPorts, tables, @@ -383,6 +389,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().preRemoveServers(servers); } + checkPermission("removeServers"); groupAdminServer.removeServers(servers); if (master.getMasterCoprocessorHost() != null) { master.getMasterCoprocessorHost().postRemoveServers(servers); http://git-wip-us.apache.org/repos/asf/hbase/blob/266b251d/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java ---------------------------------------------------------------------- diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java index 83d76a4..afdff71 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java @@ -23,7 +23,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; @@ -33,14 +32,9 @@ import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; -import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; -import org.apache.hadoop.hbase.coprocessor.ObserverContext; -import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; -import org.apache.hadoop.hbase.master.MasterCoprocessorHost; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.AccessControlClient; import org.apache.hadoop.hbase.security.access.AccessControlLists; -import org.apache.hadoop.hbase.security.access.AccessController; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.hbase.security.access.TableAuthManager; @@ -100,9 +94,6 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ private static byte[] TEST_FAMILY = Bytes.toBytes("f1"); private static RSGroupAdminEndpoint rsGroupAdminEndpoint; - private static AccessController accessController; - private static MasterCoprocessorEnvironment CP_ENV; - private static ObserverContext<MasterCoprocessorEnvironment> CTX; @BeforeClass public static void setupBeforeClass() throws Exception { @@ -118,15 +109,8 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ configureRSGroupAdminEndpoint(conf); TEST_UTIL.startMiniCluster(); - MasterCoprocessorHost masterCpHost = - TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); - rsGroupAdminEndpoint = - (RSGroupAdminEndpoint) masterCpHost.findCoprocessor(RSGroupAdminEndpoint.class.getName()); - accessController = - (AccessController) masterCpHost.findCoprocessor(AccessController.class.getName()); - CP_ENV = - masterCpHost.createEnvironment(accessController, Coprocessor.PRIORITY_HIGHEST, 1, conf); - CTX = ObserverContextImpl.createAndPrepare(CP_ENV); + rsGroupAdminEndpoint = (RSGroupAdminEndpoint) TEST_UTIL.getMiniHBaseCluster().getMaster(). + getMasterCoprocessorHost().findCoprocessor(RSGroupAdminEndpoint.class.getName()); // Wait for the ACL table to become available TEST_UTIL.waitUntilAllRegionsAssigned(AccessControlLists.ACL_TABLE_NAME); @@ -239,7 +223,9 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test @@ -249,57 +235,69 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testMoveServers() throws Exception { AccessTestAction action = () -> { - accessController.preMoveServers(CTX, null, null); + rsGroupAdminEndpoint.checkPermission("moveServers"); return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testMoveTables() throws Exception { AccessTestAction action = () -> { - accessController.preMoveTables(CTX, null, null); + rsGroupAdminEndpoint.checkPermission("moveTables"); return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testAddRSGroup() throws Exception { AccessTestAction action = () -> { - accessController.preAddRSGroup(CTX, null); + rsGroupAdminEndpoint.checkPermission("addRSGroup"); return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testRemoveRSGroup() throws Exception { AccessTestAction action = () -> { - accessController.preRemoveRSGroup(CTX, null); + rsGroupAdminEndpoint.checkPermission("removeRSGroup"); return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testBalanceRSGroup() throws Exception { AccessTestAction action = () -> { - accessController.preBalanceRSGroup(CTX, null); + rsGroupAdminEndpoint.checkPermission("balanceRSGroup"); return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test @@ -309,7 +307,9 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test @@ -319,30 +319,18 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ return null; }; - validateAdminPermissions(action); + verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); + verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, + USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); } @Test public void testMoveServersAndTables() throws Exception { AccessTestAction action = () -> { - accessController.preMoveServersAndTables(CTX, null, null, null); + rsGroupAdminEndpoint.checkPermission("moveServersAndTables"); return null; }; - validateAdminPermissions(action); - } - - @Test - public void testRemoveServers() throws Exception { - AccessTestAction action = () -> { - accessController.preRemoveServers(CTX, null); - return null; - }; - - validateAdminPermissions(action); - } - - private void validateAdminPermissions(AccessTestAction action) throws Exception { verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE); http://git-wip-us.apache.org/repos/asf/hbase/blob/266b251d/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java ---------------------------------------------------------------------- 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 7e824e2..2758c7e 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 @@ -95,7 +95,6 @@ import org.apache.hadoop.hbase.filter.FilterList; import org.apache.hadoop.hbase.io.hfile.HFile; import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils; import org.apache.hadoop.hbase.ipc.RpcServer; -import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService; @@ -1306,48 +1305,6 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, requirePermission(ctx, "recommissionRegionServers", Action.ADMIN); } - @Override - public void preMoveServersAndTables(final ObserverContext<MasterCoprocessorEnvironment> ctx, - Set<Address> servers, Set<TableName> tables, String targetGroup) throws IOException { - requirePermission(ctx, "moveServersAndTables", Action.ADMIN); - } - - @Override - public void preMoveServers(final ObserverContext<MasterCoprocessorEnvironment> ctx, - Set<Address> servers, String targetGroup) throws IOException { - requirePermission(ctx, "moveServers", Action.ADMIN); - } - - @Override - public void preMoveTables(final ObserverContext<MasterCoprocessorEnvironment> ctx, - Set<TableName> tables, String targetGroup) throws IOException { - requirePermission(ctx, "moveTables", Action.ADMIN); - } - - @Override - public void preAddRSGroup(final ObserverContext<MasterCoprocessorEnvironment> ctx, String name) - throws IOException { - requirePermission(ctx, "addRSGroup", Action.ADMIN); - } - - @Override - public void preRemoveRSGroup(final ObserverContext<MasterCoprocessorEnvironment> ctx, String name) - throws IOException { - requirePermission(ctx, "removeRSGroup", Action.ADMIN); - } - - @Override - public void preBalanceRSGroup(final ObserverContext<MasterCoprocessorEnvironment> ctx, - String groupName) throws IOException { - requirePermission(ctx, "balanceRSGroup", Action.ADMIN); - } - - @Override - public void preRemoveServers(final ObserverContext<MasterCoprocessorEnvironment> ctx, - Set<Address> servers) throws IOException { - requirePermission(ctx, "removeServers", Action.ADMIN); - } - /* ---- RegionObserver implementation ---- */ @Override