From 77bf8d5507e90f62a130cd7e1d181e45eed80f05 Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Mon, 30 Jun 2014 08:51:11 -0700 Subject: [PATCH] HBASE-11432 [AccessController] Remove cell first strategy --- .../org/apache/hadoop/hbase/client/Mutation.java | 23 ------ .../java/org/apache/hadoop/hbase/client/Query.java | 23 ------ .../hbase/security/access/AccessControlFilter.java | 10 --- .../hbase/security/access/AccessController.java | 7 +- .../hadoop/hbase/security/access/TestCellACLs.java | 84 ---------------------- .../security/access/TestScanEarlyTermination.java | 28 -------- .../hbase/util/MultiThreadedReaderWithACL.java | 1 - 7 files changed, 2 insertions(+), 174 deletions(-) diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java index a51a2e2..e8535b7 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java @@ -455,29 +455,6 @@ public abstract class Mutation extends OperationWithAttributes implements Row, C } /** - * @return true if ACLs should be evaluated on the cell level first - */ - public boolean getACLStrategy() { - byte[] bytes = getAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY); - if (bytes != null) { - return Bytes.equals(bytes, AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY_CELL_FIRST); - } - return false; - } - - /** - * @param cellFirstStrategy true if ACLs should be evaluated on the cell - * level first, false if ACL should first be checked at the CF and table - * levels - */ - public void setACLStrategy(boolean cellFirstStrategy) { - if (cellFirstStrategy) { - setAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY, - AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY_CELL_FIRST); - } - } - - /** * Subclasses should override this method to add the heap size of their own fields. * @return the heap size to add (will be aligned). */ diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/client/Query.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Query.java index 5bec128..fe141c9 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/client/Query.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/client/Query.java @@ -103,27 +103,4 @@ public abstract class Query extends OperationWithAttributes { setAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL, ProtobufUtil.toUsersAndPermissions(permMap).toByteArray()); } - - /** - * @return true if ACLs should be evaluated on the cell level first - */ - public boolean getACLStrategy() { - byte[] bytes = getAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY); - if (bytes != null) { - return Bytes.equals(bytes, AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY_CELL_FIRST); - } - return false; - } - - /** - * @param cellFirstStrategy true if ACLs should be evaluated on the cell - * level first, false if ACL should first be checked at the CF and table - * levels - */ - public void setACLStrategy(boolean cellFirstStrategy) { - if (cellFirstStrategy) { - setAttribute(AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY, - AccessControlConstants.OP_ATTRIBUTE_ACL_STRATEGY_CELL_FIRST); - } - } } diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlFilter.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlFilter.java index f750cc8..164c0fd 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlFilter.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlFilter.java @@ -53,8 +53,6 @@ class AccessControlFilter extends FilterBase { CHECK_TABLE_AND_CF_ONLY, /** Cell permissions can override table or CF permissions */ CHECK_CELL_DEFAULT, - /** Cell permissions must authorize */ - CHECK_CELL_FIRST, }; private TableAuthManager authManager; @@ -131,14 +129,6 @@ class AccessControlFilter extends FilterBase { } } break; - // Cell permissions must authorize - case CHECK_CELL_FIRST: { - if (authManager.authorize(user, table, cell, Permission.Action.READ) && - authManager.authorize(user, table, family, qualifier, Permission.Action.READ)) { - return ReturnCode.INCLUDE; - } - } - break; default: throw new RuntimeException("Unhandled strategy " + strategy); } diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 455e8c2..b949610 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -1372,8 +1372,7 @@ public class AccessController extends BaseRegionObserver // filter) but that's the price of backwards compatibility. if (hasFamilyQualifierPermission(user, Action.READ, env, families)) { Filter ourFilter = new AccessControlFilter(authManager, user, table, - query.getACLStrategy() ? AccessControlFilter.Strategy.CHECK_CELL_FIRST : - AccessControlFilter.Strategy.CHECK_TABLE_AND_CF_ONLY, + AccessControlFilter.Strategy.CHECK_TABLE_AND_CF_ONLY, cfVsMaxVersions); // wrap any existing filter if (filter != null) { @@ -1400,9 +1399,7 @@ public class AccessController extends BaseRegionObserver // allowed. We will not throw an AccessDeniedException. This is a // behavioral change since 0.96. Filter ourFilter = new AccessControlFilter(authManager, user, table, - query.getACLStrategy() ? AccessControlFilter.Strategy.CHECK_CELL_FIRST : - AccessControlFilter.Strategy.CHECK_CELL_DEFAULT, - cfVsMaxVersions); + AccessControlFilter.Strategy.CHECK_CELL_DEFAULT, cfVsMaxVersions); // wrap any existing filter if (filter != null) { ourFilter = new FilterList(FilterList.Operator.MUST_PASS_ALL, diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java index be1104d..16454c8 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLs.java @@ -447,90 +447,6 @@ public class TestCellACLs extends SecureTestUtil { }, USER_OTHER); } - @Test - public void testCellStrategy() throws Exception { - // Set up our test actions - AccessTestAction readQ1Default = new AccessTestAction() { - @Override - public Object run() throws Exception { - HTable t = new HTable(conf, TEST_TABLE.getTableName()); - try { - return t.get(new Get(TEST_ROW).addColumn(TEST_FAMILY, TEST_Q1)); - } finally { - t.close(); - } - } - }; - AccessTestAction readQ2Default = new AccessTestAction() { - @Override - public Object run() throws Exception { - HTable t = new HTable(conf, TEST_TABLE.getTableName()); - try { - return t.get(new Get(TEST_ROW).addColumn(TEST_FAMILY, TEST_Q2)); - } finally { - t.close(); - } - } - }; - AccessTestAction readQ1CellFirst = new AccessTestAction() { - @Override - public Object run() throws Exception { - HTable t = new HTable(conf, TEST_TABLE.getTableName()); - try { - Get get = new Get(TEST_ROW).addColumn(TEST_FAMILY, TEST_Q1); - get.setACLStrategy(true); - return t.get(get); - } finally { - t.close(); - } - } - }; - - // Add test data - verifyAllowed(new AccessTestAction() { - @Override - public Object run() throws Exception { - HTable t = new HTable(conf, TEST_TABLE.getTableName()); - try { - Put p; - // The empty permission set on Q1 - p = new Put(TEST_ROW).add(TEST_FAMILY, TEST_Q1, ZERO); - p.setACL(USER_OTHER.getShortName(), new Permission()); - t.put(p); - // Read permissions on Q2 - p = new Put(TEST_ROW).add(TEST_FAMILY, TEST_Q2, ZERO); - p.setACL(USER_OTHER.getShortName(), new Permission(Action.READ)); - t.put(p); - } finally { - t.close(); - } - return null; - } - }, USER_OWNER); - - // A read by USER_OTHER will be denied with the default cell strategy as - // there is no visibility without a grant and a cell ACL giving - // explicit permission - verifyDenied(readQ1Default, USER_OTHER); - - // A read will be allowed by the default cell strategy if there is a cell - // ACL giving explicit permission. - verifyAllowed(readQ2Default, USER_OTHER); - - // Grant read access to USER_OTHER - grantOnTable(TEST_UTIL, USER_OTHER.getShortName(), TEST_TABLE.getTableName(), - TEST_FAMILY, null, Action.READ); - - // A read by USER_OTHER will now be allowed with the default cell strategy - // because we have a CF level grant and we take the union of permissions. - verifyAllowed(readQ1Default, USER_OTHER); - - // A read by USER_OTHER will be denied with the cell first strategy - // because the empty perm set for USER_OTHER in the cell ACL there - // revokes access. - verifyDenied(readQ1CellFirst, USER_OTHER); - } - @After public void tearDown() throws Exception { // Clean the _acl_ table diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java index 8a28c95..ab19142 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestScanEarlyTermination.java @@ -267,33 +267,5 @@ public class TestScanEarlyTermination extends SecureTestUtil { } } }, USER_OTHER); - - // A scan of FAMILY1 and FAMILY2 will produce combined results. If we use - // a cell first strategy then cell ACLs come into effect. In FAMILY2, that - // cell ACL on Q1 now grants access and the empty permission set on Q2 now - // denies access. - verifyAllowed(new AccessTestAction() { - @Override - public Object run() throws Exception { - // force a new RS connection - conf.set("testkey", UUID.randomUUID().toString()); - HTable t = new HTable(conf, TEST_TABLE.getTableName()); - try { - Scan scan = new Scan(); - scan.setACLStrategy(true); - Result result = t.getScanner(scan).next(); - if (result != null) { - assertTrue("Improper exclusion", result.containsColumn(TEST_FAMILY1, TEST_Q1)); - assertTrue("Improper exclusion", result.containsColumn(TEST_FAMILY2, TEST_Q1)); - assertFalse("Improper inclusion", result.containsColumn(TEST_FAMILY2, TEST_Q2)); - return result.listCells(); - } - return null; - } finally { - t.close(); - } - } - }, USER_OTHER); - } } diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java index a15c7e0..030ad1d 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java @@ -95,7 +95,6 @@ public class MultiThreadedReaderWithACL extends MultiThreadedReader { public Object run() throws Exception { HTableInterface localTable = null; try { - get.setACLStrategy(true); Result result = null; int specialPermCellInsertionFactor = Integer.parseInt(dataGenerator.getArgs()[2]); int mod = ((int) keyToRead % userNames.length); -- 1.9.1