diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthManager.java index c27063752c58..40d146b4bd3a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AuthManager.java @@ -78,12 +78,13 @@ Set get(String name) { } } - void clear() { + void cleanUp(ListMultimap newPermissions) { synchronized (mutex) { for (Map.Entry> entry : cache.entrySet()) { - entry.getValue().clear(); + if (!newPermissions.containsKey(entry.getKey())) { + entry.getValue().clear(); + } } - cache.clear(); } } } @@ -163,7 +164,6 @@ public void refreshNamespaceCacheFromWritable(String namespace, byte[] data) thr * @param globalPerms new global permissions */ private void updateGlobalCache(ListMultimap globalPerms) { - globalCache.clear(); for (String name : globalPerms.keySet()) { for (Permission permission : globalPerms.get(name)) { // Before 2.2, the global permission which storage in zk is not right. It was saved as a @@ -175,6 +175,12 @@ private void updateGlobalCache(ListMultimap globalPerms) { } } } + for (String name : globalCache.keySet()) { + if (!globalPerms.containsKey(name)) { + globalCache.remove(name); + } + } + mtime.incrementAndGet(); } @@ -186,7 +192,6 @@ private void updateGlobalCache(ListMultimap globalPerms) { private void updateTableCache(TableName table, ListMultimap tablePerms) { PermissionCache cacheToUpdate = tableCache.getOrDefault(table, new PermissionCache<>()); - clearCache(cacheToUpdate); updateCache(tablePerms, cacheToUpdate); tableCache.put(table, cacheToUpdate); mtime.incrementAndGet(); @@ -200,16 +205,11 @@ private void updateTableCache(TableName table, ListMultimap private void updateNamespaceCache(String namespace, ListMultimap nsPerms) { PermissionCache cacheToUpdate = namespaceCache.getOrDefault(namespace, new PermissionCache<>()); - clearCache(cacheToUpdate); updateCache(nsPerms, cacheToUpdate); namespaceCache.put(namespace, cacheToUpdate); mtime.incrementAndGet(); } - private void clearCache(PermissionCache cacheToUpdate) { - cacheToUpdate.clear(); - } - @SuppressWarnings("unchecked") private void updateCache(ListMultimap newPermissions, PermissionCache cacheToUpdate) { @@ -218,6 +218,7 @@ private void updateCache(ListMultimap newPermissio cacheToUpdate.put(name, permission); } } + cacheToUpdate.cleanUp(newPermissions); } /** 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 01995cd70c45..24c710a72505 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 @@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -462,4 +463,99 @@ public void testAuthManager() throws Exception { } } } + + @Test + public void testPermissionShouldAlwaysSucceedDuringUpdate() + throws IOException, InterruptedException { + Configuration conf = UTIL.getConfiguration(); + AuthManager authManager = new AuthManager(conf); + + User user = User.createUserForTesting(conf, "test-user", new String[] {}); + AtomicBoolean authorizationSucceeded = new AtomicBoolean(true); + + // test from fine-grained to coarse-grained permissions + // test table permission + byte[] tableAclData = createPermissionsData(conf, user, Permission.newBuilder(TEST_TABLE)); + authManager.refreshTableCacheFromWritable(TEST_TABLE, tableAclData); + verifyAuthorization(() -> { + for (int i = 0; i < 1000; i++) { + try { + authManager.refreshTableCacheFromWritable(TEST_TABLE, tableAclData); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + }, () -> { + for (int i = 0; i < 1000; i++) { + if (!authManager.authorizeUserTable(user, TEST_TABLE, Permission.Action.READ)) { + authorizationSucceeded.set(false); + break; + } + } + }, authorizationSucceeded); + + // test namespace permission + byte[] nsAclData = createPermissionsData(conf, user, Permission.newBuilder(TEST_NAMESPACE)); + authManager.refreshNamespaceCacheFromWritable(TEST_NAMESPACE, nsAclData); + + verifyAuthorization(() -> { + for (int i = 0; i < 1000; i++) { + try { + authManager.refreshNamespaceCacheFromWritable(TEST_NAMESPACE, nsAclData); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + }, () -> { + for (int i = 0; i < 1000; i++) { + if (!authManager.authorizeUserNamespace(user, TEST_NAMESPACE, Permission.Action.READ)) { + authorizationSucceeded.set(false); + break; + } + } + }, authorizationSucceeded); + + // test global permission + byte[] globalAclData = createPermissionsData(conf, user, Permission.newBuilder()); + authManager.refreshTableCacheFromWritable(PermissionStorage.ACL_TABLE_NAME, globalAclData); + + verifyAuthorization(() -> { + for (int i = 0; i < 1000; i++) { + try { + authManager.refreshTableCacheFromWritable(PermissionStorage.ACL_TABLE_NAME, + globalAclData); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + }, () -> { + for (int i = 0; i < 1000; i++) { + if (!authManager.authorizeUserGlobal(user, Permission.Action.READ)) { + authorizationSucceeded.set(false); + break; + } + } + }, authorizationSucceeded); + } + + private static void verifyAuthorization(Runnable m1, Runnable m2, + AtomicBoolean authorizationSucceeded) throws InterruptedException { + Thread t1 = new Thread(m1); + Thread t2 = new Thread(m2); + t1.start(); + t2.start(); + t1.join(); + t2.join(); + assertTrue("Authorization should always succeed", authorizationSucceeded.get()); + } + + private static byte[] createPermissionsData(Configuration conf, User user, + Permission.Builder newBuilder) { + List acl = new ArrayList<>(1); + acl.add(new UserPermission(user.getShortName(), + newBuilder.withActions(Permission.Action.READ, Permission.Action.WRITE).build())); + ListMultimap permissions = ArrayListMultimap.create(); + permissions.putAll(user.getShortName(), acl); + return PermissionStorage.writePermissionsAsBytes(permissions, conf); + } }