HBASE-10239. Improve determinism and debugability of TestAccessController

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1553718 13f79535-47bb-0310-9956-ffa450edef68

hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java

@@ -155,6 +155,14 @@ public class AccessController extends BaseRegionObserver
 
   private volatile boolean initialized = false;
 
+  public HRegion getRegion() {
+    return regionEnv != null ? regionEnv.getRegion() : null;
+  }
+
+  public TableAuthManager getAuthManager() {
+    return authManager;
+  }
+
   void initialize(RegionCoprocessorEnvironment e) throws IOException {
     final HRegion region = e.getRegion();
     Map<byte[], ListMultimap<String,TablePermission>> tables =
@@ -1768,5 +1776,4 @@ public class AccessController extends BaseRegionObserver
   @Override
   public void postRollBackMerge(ObserverContext<RegionServerCoprocessorEnvironment> ctx,
       HRegion regionA, HRegion regionB) throws IOException { }
-
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java

@@ -24,23 +24,41 @@ import java.io.IOException;
 import java.lang.reflect.UndeclaredThrowableException;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Callable;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.Coprocessor;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.MiniHBaseCluster;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.Waiter.Predicate;
 import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.client.RetriesExhaustedWithDetailsException;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService;
 import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.CheckPermissionsRequest;
+import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.security.AccessDeniedException;
 import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.util.JVMClusterUtil.RegionServerThread;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.protobuf.BlockingRpcChannel;
 import com.google.protobuf.ServiceException;
 
 /**
  * Utility methods for testing security
  */
 public class SecureTestUtil {
+  private static final Log LOG = LogFactory.getLog(SecureTestUtil.class);
+  private static final int WAIT_TIME = 10000;
+
   public static void enableSecurity(Configuration conf) throws IOException {
     conf.set("hadoop.security.authorization", "false");
     conf.set("hadoop.security.authentication", "simple");
@@ -65,27 +83,79 @@ public class SecureTestUtil {
     conf.setInt("hfile.format.version", 3);
   }
 
-  public void verifyAllowed(User user, PrivilegedExceptionAction... actions) throws Exception {
+  public void checkTablePerms(Configuration conf, byte[] table, byte[] family, byte[] column,
-    for (PrivilegedExceptionAction action : actions) {
+      Permission.Action... actions) throws IOException {
+    Permission[] perms = new Permission[actions.length];
+    for (int i = 0; i < actions.length; i++) {
+      perms[i] = new TablePermission(TableName.valueOf(table), family, column, actions[i]);
+    }
+
+    checkTablePerms(conf, table, perms);
+  }
+
+  public void checkTablePerms(Configuration conf, byte[] table, Permission... perms) throws IOException {
+    CheckPermissionsRequest.Builder request = CheckPermissionsRequest.newBuilder();
+    for (Permission p : perms) {
+      request.addPermission(ProtobufUtil.toPermission(p));
+    }
+    HTable acl = new HTable(conf, table);
+    try {
+      AccessControlService.BlockingInterface protocol =
+        AccessControlService.newBlockingStub(acl.coprocessorService(new byte[0]));
       try {
-        user.runAs(action);
+        protocol.checkPermissions(null, request.build());
+      } catch (ServiceException se) {
+        ProtobufUtil.toIOException(se);
+      }
+    } finally {
+      acl.close();
+    }
+  }
+
+  /**
+   * An AccessTestAction performs an action that will be examined to confirm
+   * the results conform to expected access rights.
+   * <p>
+   * To indicate an action was allowed, return null or a non empty list of
+   * KeyValues.
+   * <p>
+   * To indicate the action was not allowed, either throw an AccessDeniedException
+   * or return an empty list of KeyValues.
+   */
+  static interface AccessTestAction extends PrivilegedExceptionAction<Object> { }
+
+  public void verifyAllowed(User user, AccessTestAction... actions) throws Exception {
+    for (AccessTestAction action : actions) {
+      try {
+        Object obj = user.runAs(action);
+        if (obj != null && obj instanceof List<?>) {
+          List<?> results = (List<?>) obj;
+          if (results != null && results.isEmpty()) {
+            fail("Empty non null results from action for user '" + user.getShortName() + "'");
+          }
+        }
       } catch (AccessDeniedException ade) {
         fail("Expected action to pass for user '" + user.getShortName() + "' but was denied");
       }
     }
   }
 
-  public void verifyAllowed(PrivilegedExceptionAction action, User... users) throws Exception {
+  public void verifyAllowed(AccessTestAction action, User... users) throws Exception {
     for (User user : users) {
       verifyAllowed(user, action);
     }
   }
 
-  public void verifyDenied(User user, PrivilegedExceptionAction... actions) throws Exception {
+  public void verifyDenied(User user, AccessTestAction... actions) throws Exception {
-    for (PrivilegedExceptionAction action : actions) {
+    for (AccessTestAction action : actions) {
       try {
-        user.runAs(action);
+        Object obj = user.runAs(action);
-        fail("Expected AccessDeniedException for user '" + user.getShortName() + "'");
+        if (obj != null && obj instanceof List<?>) {
+          List<?> results = (List<?>) obj;
+          if (results != null && !results.isEmpty()) {
+            fail("Expected no results for user '" + user.getShortName() + "'");
+          }
+        }
       } catch (IOException e) {
         boolean isAccessDeniedException = false;
         if(e instanceof RetriesExhaustedWithDetailsException) {
@@ -131,38 +201,211 @@ public class SecureTestUtil {
     }
   }
 
-  public void verifyDenied(PrivilegedExceptionAction action, User... users) throws Exception {
+  public void verifyDenied(AccessTestAction action, User... users) throws Exception {
     for (User user : users) {
       verifyDenied(user, action);
     }
   }
 
-  public void checkTablePerms(Configuration conf, byte[] table, byte[] family, byte[] column,
+  private static List<AccessController> getAccessControllers(MiniHBaseCluster cluster) {
-      Permission.Action... actions) throws IOException {
+    List<AccessController> result = Lists.newArrayList();
-    Permission[] perms = new Permission[actions.length];
+    for (RegionServerThread t: cluster.getLiveRegionServerThreads()) {
-    for (int i = 0; i < actions.length; i++) {
+      for (HRegion region: t.getRegionServer().getOnlineRegionsLocalContext()) {
-      perms[i] = new TablePermission(TableName.valueOf(table), family, column, actions[i]);
+        Coprocessor cp = region.getCoprocessorHost()
+          .findCoprocessor(AccessController.class.getName());
+        if (cp != null) {
+          result.add((AccessController)cp);
+        }
+      }
     }
-
+    return result;
-    checkTablePerms(conf, table, perms);
   }
 
-  public void checkTablePerms(Configuration conf, byte[] table, Permission... perms) throws IOException {
+  private static Map<AccessController,Long> getAuthManagerMTimes(MiniHBaseCluster cluster) {
-    CheckPermissionsRequest.Builder request = CheckPermissionsRequest.newBuilder();
+    Map<AccessController,Long> result = Maps.newHashMap();
-    for (Permission p : perms) {
+    for (AccessController ac: getAccessControllers(cluster)) {
-      request.addPermission(ProtobufUtil.toPermission(p));
+      result.put(ac, ac.getAuthManager().getMTime());
     }
-    HTable acl = new HTable(conf, table);
+    return result;
-    try {
+  }
-      AccessControlService.BlockingInterface protocol =
+
-        AccessControlService.newBlockingStub(acl.coprocessorService(new byte[0]));
+  @SuppressWarnings("rawtypes")
-      try {
+  private static void updateACLs(final HBaseTestingUtility util, Callable c) throws Exception {
-        protocol.checkPermissions(null, request.build());
+    // Get the current mtimes for all access controllers
-      } catch (ServiceException se) {
+    final Map<AccessController,Long> oldMTimes = getAuthManagerMTimes(util.getHBaseCluster());
-        ProtobufUtil.toIOException(se);
+
+    // Run the update action
+    c.call();
+
+    // Wait until mtimes for all access controllers have incremented
+    util.waitFor(WAIT_TIME, 100, new Predicate<IOException>() {
+      @Override
+      public boolean evaluate() throws IOException {
+        Map<AccessController,Long> mtimes = getAuthManagerMTimes(util.getHBaseCluster());
+        for (Map.Entry<AccessController,Long> e: mtimes.entrySet()) {
+          if (!oldMTimes.containsKey(e.getKey())) {
+            LOG.error("Snapshot of AccessController state does not include instance on region " +
+              e.getKey().getRegion().getRegionNameAsString());
+            // Error out the predicate, we will try again
+            return false;
+          }
+          long old = oldMTimes.get(e.getKey());
+          long now = e.getValue();
+          if (now <= old) {
+            LOG.info("AccessController on region " +
+              e.getKey().getRegion().getRegionNameAsString() + " has not updated: mtime=" +
+              now);
+            return false;
+          }
+        }
+        return true;
       }
-    } finally {
+    });
-      acl.close();
+  }
-    }
+
+  /**
+   * Grant permissions globally to the given user. Will wait until all active
+   * AccessController instances have updated their permissions caches or will
+   * throw an exception upon timeout (10 seconds).
+   */
+  public static void grantGlobal(final HBaseTestingUtility util, final String user,
+      final Permission.Action... actions) throws Exception {
+    SecureTestUtil.updateACLs(util, new Callable<Void>() {
+      @Override
+      public Void call() throws Exception {
+        HTable acl = new HTable(util.getConfiguration(), AccessControlLists.ACL_TABLE_NAME);
+        try {
+          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          AccessControlService.BlockingInterface protocol =
+              AccessControlService.newBlockingStub(service);
+          ProtobufUtil.grant(protocol, user, actions);
+        } finally {
+          acl.close();
+        }
+        return null;
+      }
+    });
+  }
+
+  /**
+   * Revoke permissions globally from the given user. Will wait until all active
+   * AccessController instances have updated their permissions caches or will
+   * throw an exception upon timeout (10 seconds).
+   */
+  public static void revokeGlobal(final HBaseTestingUtility util, final String user,
+      final Permission.Action... actions) throws Exception {
+    SecureTestUtil.updateACLs(util, new Callable<Void>() {
+      @Override
+      public Void call() throws Exception {
+        HTable acl = new HTable(util.getConfiguration(), AccessControlLists.ACL_TABLE_NAME);
+        try {
+          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          AccessControlService.BlockingInterface protocol =
+              AccessControlService.newBlockingStub(service);
+          ProtobufUtil.revoke(protocol, user, actions);
+        } finally {
+          acl.close();
+        }
+        return null;
+      }
+    });
+  }
+
+  /**
+   * Grant permissions on a namespace to the given user. Will wait until all active
+   * AccessController instances have updated their permissions caches or will
+   * throw an exception upon timeout (10 seconds).
+   */
+  public static void grantOnNamespace(final HBaseTestingUtility util, final String user,
+      final String namespace, final Permission.Action... actions) throws Exception {
+    SecureTestUtil.updateACLs(util, new Callable<Void>() {
+      @Override
+      public Void call() throws Exception {
+        HTable acl = new HTable(util.getConfiguration(), AccessControlLists.ACL_TABLE_NAME);
+        try {
+          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          AccessControlService.BlockingInterface protocol =
+              AccessControlService.newBlockingStub(service);
+          ProtobufUtil.grant(protocol, user, namespace, actions);
+        } finally {
+          acl.close();
+        }
+        return null;
+      }
+    });
+  }
+
+  /**
+   * Revoke permissions on a namespace from the given user. Will wait until all active
+   * AccessController instances have updated their permissions caches or will
+   * throw an exception upon timeout (10 seconds).
+   */
+  public static void revokeFromNamespace(final HBaseTestingUtility util, final String user,
+      final String namespace, final Permission.Action... actions) throws Exception {
+    SecureTestUtil.updateACLs(util, new Callable<Void>() {
+      @Override
+      public Void call() throws Exception {
+        HTable acl = new HTable(util.getConfiguration(), AccessControlLists.ACL_TABLE_NAME);
+        try {
+          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          AccessControlService.BlockingInterface protocol =
+              AccessControlService.newBlockingStub(service);
+          ProtobufUtil.revoke(protocol, user, namespace, actions);
+        } finally {
+          acl.close();
+        }
+        return null;
+      }
+    });
+  }
+
+  /**
+   * Grant permissions on a table to the given user. Will wait until all active
+   * AccessController instances have updated their permissions caches or will
+   * throw an exception upon timeout (10 seconds).
+   */
+  public static void grantOnTable(final HBaseTestingUtility util, final String user,
+      final TableName table, final byte[] family, final byte[] qualifier,
+      final Permission.Action... actions) throws Exception {
+    SecureTestUtil.updateACLs(util, new Callable<Void>() {
+      @Override
+      public Void call() throws Exception {
+        HTable acl = new HTable(util.getConfiguration(), AccessControlLists.ACL_TABLE_NAME);
+        try {
+          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          AccessControlService.BlockingInterface protocol =
+              AccessControlService.newBlockingStub(service);
+          ProtobufUtil.grant(protocol, user, table, family, qualifier, actions);
+        } finally {
+          acl.close();
+        }
+        return null;
+      }
+    });
+  }
+
+  /**
+   * Revoke permissions on a table from the given user. Will wait until all active
+   * AccessController instances have updated their permissions caches or will
+   * throw an exception upon timeout (10 seconds).
+   */
+  public static void revokeFromTable(final HBaseTestingUtility util, final String user,
+      final TableName table, final byte[] family, final byte[] qualifier,
+      final Permission.Action... actions) throws Exception {
+    SecureTestUtil.updateACLs(util, new Callable<Void>() {
+      @Override
+      public Void call() throws Exception {
+        HTable acl = new HTable(util.getConfiguration(), AccessControlLists.ACL_TABLE_NAME);
+        try {
+          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          AccessControlService.BlockingInterface protocol =
+              AccessControlService.newBlockingStub(service);
+          ProtobufUtil.revoke(protocol, user, table, family, qualifier, actions);
+        } finally {
+          acl.close();
+        }
+        return null;
+      }
+    });
   }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java

@@ -24,9 +24,6 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
-import java.lang.reflect.UndeclaredThrowableException;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.util.List;
 import java.util.Map;
 import java.util.NavigableMap;
@@ -60,7 +57,6 @@ import org.apache.hadoop.hbase.client.Increment;
 import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.ResultScanner;
-import org.apache.hadoop.hbase.client.RetriesExhaustedWithDetailsException;
 import org.apache.hadoop.hbase.client.Scan;
 import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment;
 import org.apache.hadoop.hbase.coprocessor.ObserverContext;
@@ -73,7 +69,6 @@ import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder;
 import org.apache.hadoop.hbase.mapreduce.LoadIncrementalHFiles;
 import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
-import org.apache.hadoop.hbase.protobuf.RequestConverter;
 import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos;
 import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService;
 import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.CheckPermissionsRequest;
@@ -82,7 +77,6 @@ import org.apache.hadoop.hbase.regionserver.HRegionServer;
 import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost;
 import org.apache.hadoop.hbase.regionserver.RegionServerCoprocessorHost;
 import org.apache.hadoop.hbase.regionserver.ScanType;
-import org.apache.hadoop.hbase.security.AccessDeniedException;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.security.access.Permission.Action;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -200,38 +194,31 @@ public class TestAccessController extends SecureTestUtil {
     RCP_ENV = rcpHost.createEnvironment(AccessController.class, ACCESS_CONTROLLER,
       Coprocessor.PRIORITY_HIGHEST, 1, conf);
 
-    // initilize access control
+    // Set up initial grants
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
-    try {
-      BlockingRpcChannel service = acl.coprocessorService(TEST_TABLE.getTableName().getName());
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
 
-      protocol.grant(null, RequestConverter.buildGrantRequest(USER_ADMIN.getShortName(),
+    SecureTestUtil.grantGlobal(TEST_UTIL, USER_ADMIN.getShortName(),
-        AccessControlProtos.Permission.Action.ADMIN,
+      Permission.Action.ADMIN,
-        AccessControlProtos.Permission.Action.CREATE,
+      Permission.Action.CREATE,
-        AccessControlProtos.Permission.Action.READ,
+      Permission.Action.READ,
-        AccessControlProtos.Permission.Action.WRITE));
+      Permission.Action.WRITE);
 
-      protocol.grant(null, RequestConverter.buildGrantRequest(USER_RW.getShortName(),
+    SecureTestUtil.grantOnTable(TEST_UTIL, USER_RW.getShortName(),
-        TEST_TABLE.getTableName(), TEST_FAMILY, null,
+      TEST_TABLE.getTableName(), TEST_FAMILY, null,
-        AccessControlProtos.Permission.Action.READ,
+      Permission.Action.READ,
-        AccessControlProtos.Permission.Action.WRITE));
+      Permission.Action.WRITE);
 
-      // USER_CREATE is USER_RW plus CREATE permissions
+    // USER_CREATE is USER_RW plus CREATE permissions
-      protocol.grant(null, RequestConverter.buildGrantRequest(USER_CREATE.getShortName(),
+    SecureTestUtil.grantOnTable(TEST_UTIL, USER_CREATE.getShortName(),
-        TEST_TABLE.getTableName(), null, null,
+      TEST_TABLE.getTableName(), null, null,
-        AccessControlProtos.Permission.Action.CREATE,
+      Permission.Action.CREATE,
-        AccessControlProtos.Permission.Action.READ,
+      Permission.Action.READ,
-        AccessControlProtos.Permission.Action.WRITE));
+      Permission.Action.WRITE);
 
-      protocol.grant(null, RequestConverter.buildGrantRequest(USER_RO.getShortName(), TEST_TABLE.getTableName(),
+    SecureTestUtil.grantOnTable(TEST_UTIL, USER_RO.getShortName(),
-        TEST_FAMILY, null, AccessControlProtos.Permission.Action.READ));
+      TEST_TABLE.getTableName(), TEST_FAMILY, null,
+      Permission.Action.READ);
 
-      assertEquals(4, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
+    assertEquals(4, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
-    } finally {
-      acl.close();
-    }
   }
 
   @After
@@ -246,101 +233,6 @@ public class TestAccessController extends SecureTestUtil {
     assertEquals(0, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
   }
 
-  /**
-   * An AccessTestAction performs an action that will be examined to confirm
-   * the results conform to expected access rights.
-   * <p>
-   * To indicate an action was allowed, return null or a non empty list of
-   * KeyValues.
-   * <p>
-   * To indicate the action was not allowed, either throw an AccessDeniedException
-   * or return an empty list of KeyValues.
-   */
-  static interface AccessTestAction extends PrivilegedExceptionAction<Object> { }
-
-  public void verifyAllowed(User user, AccessTestAction... actions) throws Exception {
-    for (AccessTestAction action : actions) {
-      try {
-        Object obj = user.runAs(action);
-        if (obj != null && obj instanceof List<?>) {
-          List<?> results = (List<?>) obj;
-          if (results != null && results.isEmpty()) {
-            fail("Empty non null results from action for user '" + user.getShortName() + "'");
-          }
-        }
-      } catch (AccessDeniedException ade) {
-        fail("Expected action to pass for user '" + user.getShortName() + "' but was denied");
-      }
-    }
-  }
-
-  public void verifyAllowed(AccessTestAction action, User... users) throws Exception {
-    for (User user : users) {
-      verifyAllowed(user, action);
-    }
-  }
-
-  public void verifyDenied(User user, AccessTestAction... actions) throws Exception {
-    for (AccessTestAction action : actions) {
-      try {
-        Object obj = user.runAs(action);
-        if (obj != null && obj instanceof List<?>) {
-          List<?> results = (List<?>) obj;
-          if (results != null && !results.isEmpty()) {
-            fail("Expected no results for user '" + user.getShortName() + "'");
-          }
-        }
-      } catch (IOException e) {
-        boolean isAccessDeniedException = false;
-        if(e instanceof RetriesExhaustedWithDetailsException) {
-          // in case of batch operations, and put, the client assembles a
-          // RetriesExhaustedWithDetailsException instead of throwing an
-          // AccessDeniedException
-          for(Throwable ex : ((RetriesExhaustedWithDetailsException) e).getCauses()) {
-            if (ex instanceof AccessDeniedException) {
-              isAccessDeniedException = true;
-              break;
-            }
-          }
-        }
-        else {
-          // For doBulkLoad calls AccessDeniedException
-          // is buried in the stack trace
-          Throwable ex = e;
-          do {
-            if (ex instanceof AccessDeniedException) {
-              isAccessDeniedException = true;
-              break;
-            }
-          } while((ex = ex.getCause()) != null);
-        }
-        if (!isAccessDeniedException) {
-          fail("Not receiving AccessDeniedException for user '" + user.getShortName() + "'");
-        }
-      } catch (UndeclaredThrowableException ute) {
-        // TODO why we get a PrivilegedActionException, which is unexpected?
-        Throwable ex = ute.getUndeclaredThrowable();
-        if (ex instanceof PrivilegedActionException) {
-          ex = ((PrivilegedActionException) ex).getException();
-        }
-        if (ex instanceof ServiceException) {
-          ServiceException se = (ServiceException)ex;
-          if (se.getCause() != null && se.getCause() instanceof AccessDeniedException) {
-            // expected result
-            return;
-          }
-        }
-        fail("Not receiving AccessDeniedException for user '" + user.getShortName() + "'");
-      }
-    }
-  }
-
-  public void verifyDenied(AccessTestAction action, User... users) throws Exception {
-    for (User user : users) {
-      verifyDenied(user, action);
-    }
-  }
-
   @Test
   public void testTableCreate() throws Exception {
     AccessTestAction createTable = new AccessTestAction() {
@@ -675,7 +567,7 @@ public class TestAccessController extends SecureTestUtil {
     
     final List<HRegion> regions = TEST_UTIL.getHBaseCluster().findRegionsForTable(TEST_TABLE.getTableName());
     
-    PrivilegedExceptionAction action = new PrivilegedExceptionAction() {
+    AccessTestAction action = new AccessTestAction() {
       @Override
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preMerge(
@@ -1513,20 +1405,12 @@ public class TestAccessController extends SecureTestUtil {
     verifyDenied(gblUser, deleteActionAll, deleteAction1, deleteAction2);
 
     // grant table read permission
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantGlobal(TEST_UTIL, gblUser.getShortName(),
-    try {
+      Permission.Action.READ);
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+    SecureTestUtil.grantOnTable(TEST_UTIL, tblUser.getShortName(),
-      AccessControlService.BlockingInterface protocol =
+      tableName, null, null,
-        AccessControlService.newBlockingStub(service);
+      Permission.Action.READ);
-      ProtobufUtil.grant(protocol, tblUser.getShortName(),
-        tableName, null, null, Permission.Action.READ);
-      ProtobufUtil.grant(protocol, gblUser.getShortName(),
-          Permission.Action.READ);
-    } finally {
-      acl.close();
-    }
 
-    Thread.sleep(100);
     // check
     verifyAllowed(tblUser, getActionAll, getAction1, getAction2);
     verifyDenied(tblUser, putActionAll, putAction1, putAction2);
@@ -1536,21 +1420,12 @@ public class TestAccessController extends SecureTestUtil {
     verifyDenied(gblUser, putActionAll, putAction1, putAction2);
     verifyDenied(gblUser, deleteActionAll, deleteAction1, deleteAction2);
 
-    // grant table write permission
+    // grant table write permission while revoking read permissions
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantGlobal(TEST_UTIL, gblUser.getShortName(),
-    try {
+      Permission.Action.WRITE);
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+    SecureTestUtil.grantOnTable(TEST_UTIL, tblUser.getShortName(),
-      AccessControlService.BlockingInterface protocol =
+      tableName, null, null,
-        AccessControlService.newBlockingStub(service);
+      Permission.Action.WRITE);
-      ProtobufUtil.grant(protocol, tblUser.getShortName(),
-        tableName, null, null, Permission.Action.WRITE);
-      ProtobufUtil.grant(protocol, gblUser.getShortName(),
-          Permission.Action.WRITE);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     verifyDenied(tblUser, getActionAll, getAction1, getAction2);
     verifyAllowed(tblUser, putActionAll, putAction1, putAction2);
@@ -1560,21 +1435,10 @@ public class TestAccessController extends SecureTestUtil {
     verifyAllowed(gblUser, putActionAll, putAction1, putAction2);
     verifyAllowed(gblUser, deleteActionAll, deleteAction1, deleteAction2);
 
-    // revoke table permission
+    // revoke table permissions
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.revokeGlobal(TEST_UTIL, gblUser.getShortName());
-    try {
+    SecureTestUtil.revokeFromTable(TEST_UTIL, tblUser.getShortName(),
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+      tableName, null, null);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, tblUser.getShortName(), tableName, null, null,
-        Permission.Action.READ, Permission.Action.WRITE);
-      ProtobufUtil.revoke(protocol, tblUser.getShortName(), tableName, null, null);
-      ProtobufUtil.revoke(protocol, gblUser.getShortName());
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     verifyDenied(tblUser, getActionAll, getAction1, getAction2);
     verifyDenied(tblUser, putActionAll, putAction1, putAction2);
@@ -1585,20 +1449,10 @@ public class TestAccessController extends SecureTestUtil {
     verifyDenied(gblUser, deleteActionAll, deleteAction1, deleteAction2);
 
     // grant column family read permission
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantGlobal(TEST_UTIL, gblUser.getShortName(),
-    try {
+      Permission.Action.READ);
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+    SecureTestUtil.grantOnTable(TEST_UTIL, tblUser.getShortName(),
-      AccessControlService.BlockingInterface protocol =
+      tableName, family1, null, Permission.Action.READ);
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, tblUser.getShortName(),
-        tableName, family1, null, Permission.Action.READ);
-      ProtobufUtil.grant(protocol, gblUser.getShortName(),
-          Permission.Action.READ);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     // Access should be denied for family2
     verifyAllowed(tblUser, getActionAll, getAction1);
@@ -1611,20 +1465,10 @@ public class TestAccessController extends SecureTestUtil {
     verifyDenied(gblUser, deleteActionAll, deleteAction1, deleteAction2);
 
     // grant column family write permission
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantGlobal(TEST_UTIL, gblUser.getShortName(),
-    try {
+      Permission.Action.WRITE);
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+    SecureTestUtil.grantOnTable(TEST_UTIL, tblUser.getShortName(),
-      AccessControlService.BlockingInterface protocol =
+      tableName, family2, null, Permission.Action.WRITE);
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, tblUser.getShortName(),
-        tableName, family2, null, Permission.Action.WRITE);
-      ProtobufUtil.grant(protocol, gblUser.getShortName(),
-          Permission.Action.WRITE);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     // READ from family1, WRITE to family2 are allowed
     verifyAllowed(tblUser, getActionAll, getAction1);
@@ -1638,18 +1482,8 @@ public class TestAccessController extends SecureTestUtil {
     verifyAllowed(gblUser, deleteActionAll, deleteAction1, deleteAction2);
 
     // revoke column family permission
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.revokeGlobal(TEST_UTIL, gblUser.getShortName());
-    try {
+    SecureTestUtil.revokeFromTable(TEST_UTIL, tblUser.getShortName(), tableName, family2, null);
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.revoke(protocol, tblUser.getShortName(), tableName, family2, null);
-      ProtobufUtil.revoke(protocol, gblUser.getShortName());
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     // Revoke on family2 should not have impact on family1 permissions
     verifyAllowed(tblUser, getActionAll, getAction1);
@@ -1739,34 +1573,15 @@ public class TestAccessController extends SecureTestUtil {
       }
     };
 
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.revokeFromTable(TEST_UTIL, user.getShortName(), tableName, family1, null);
-    try {
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.revoke(protocol, user.getShortName(), tableName, family1, null);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     verifyDenied(user, getQualifierAction);
     verifyDenied(user, putQualifierAction);
     verifyDenied(user, deleteQualifierAction);
 
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantOnTable(TEST_UTIL, user.getShortName(),
-    try {
+      tableName, family1, qualifier,
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+      Permission.Action.READ);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, user.getShortName(),
-        tableName, family1, qualifier, Permission.Action.READ);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     verifyAllowed(user, getQualifierAction);
     verifyDenied(user, putQualifierAction);
@@ -1774,55 +1589,26 @@ public class TestAccessController extends SecureTestUtil {
 
     // only grant write permission
     // TODO: comment this portion after HBASE-3583
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantOnTable(TEST_UTIL, user.getShortName(),
-    try {
+      tableName, family1, qualifier,
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+      Permission.Action.WRITE);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, user.getShortName(),
-        tableName, family1, qualifier, Permission.Action.WRITE);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     verifyDenied(user, getQualifierAction);
     verifyAllowed(user, putQualifierAction);
     verifyAllowed(user, deleteQualifierAction);
 
-    // grant both read and write permission.
+    // grant both read and write permission
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantOnTable(TEST_UTIL, user.getShortName(),
-    try {
+      tableName, family1, qualifier,
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
+      Permission.Action.READ, Permission.Action.WRITE);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, user.getShortName(),
-        tableName, family1, qualifier,
-          Permission.Action.READ, Permission.Action.WRITE);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     verifyAllowed(user, getQualifierAction);
     verifyAllowed(user, putQualifierAction);
     verifyAllowed(user, deleteQualifierAction);
 
-    // revoke family level permission won't impact column level.
+    // revoke family level permission won't impact column level
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.revokeFromTable(TEST_UTIL, user.getShortName(),
-    try {
+      tableName, family1, qualifier);
-      BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.revoke(protocol, user.getShortName(),
-        tableName, family1, qualifier);
-    } finally {
-      acl.close();
-    }
-
-    Thread.sleep(100);
 
     verifyDenied(user, getQualifierAction);
     verifyDenied(user, putQualifierAction);
@@ -1879,13 +1665,14 @@ public class TestAccessController extends SecureTestUtil {
       hasFoundUserPermission(up, perms));
 
     // grant read permission
+    SecureTestUtil.grantOnTable(TEST_UTIL, user.getShortName(),
+      tableName, family1, qualifier, Permission.Action.READ);
+
     acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
     try {
       BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
       AccessControlService.BlockingInterface protocol =
         AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, user.getShortName(),
-        tableName, family1, qualifier, Permission.Action.READ);
       perms = ProtobufUtil.getUserPermissions(protocol, tableName);
     } finally {
       acl.close();
@@ -1902,14 +1689,15 @@ public class TestAccessController extends SecureTestUtil {
       hasFoundUserPermission(upToVerify, perms));
 
     // grant read+write
+    SecureTestUtil.grantOnTable(TEST_UTIL, user.getShortName(),
+      tableName, family1, qualifier,
+      Permission.Action.WRITE, Permission.Action.READ);
+
     acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
     try {
       BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
       AccessControlService.BlockingInterface protocol =
         AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, user.getShortName(),
-        tableName, family1, qualifier,
-          Permission.Action.WRITE, Permission.Action.READ);
       perms = ProtobufUtil.getUserPermissions(protocol, tableName);
     } finally {
       acl.close();
@@ -1920,13 +1708,15 @@ public class TestAccessController extends SecureTestUtil {
     assertTrue("User should be granted permission: " + upToVerify.toString(),
       hasFoundUserPermission(upToVerify, perms));
 
+    // revoke
+    SecureTestUtil.revokeFromTable(TEST_UTIL, user.getShortName(), tableName, family1, qualifier,
+      Permission.Action.WRITE, Permission.Action.READ);
+    
     acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
     try {
       BlockingRpcChannel service = acl.coprocessorService(tableName.getName());
       AccessControlService.BlockingInterface protocol =
         AccessControlService.newBlockingStub(service);
-      ProtobufUtil.revoke(protocol, user.getShortName(), tableName, family1, qualifier,
-        Permission.Action.WRITE, Permission.Action.READ);
       perms = ProtobufUtil.getUserPermissions(protocol, tableName);
     } finally {
       acl.close();
@@ -2078,20 +1868,15 @@ public class TestAccessController extends SecureTestUtil {
     User userColumn = User.createUserForTesting(conf, "user_check_perms_family", new String[0]);
     User userQualifier = User.createUserForTesting(conf, "user_check_perms_q", new String[0]);
 
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantOnTable(TEST_UTIL, userTable.getShortName(),
-    try {
+      TEST_TABLE.getTableName(), null, null,
-      BlockingRpcChannel channel = acl.coprocessorService(new byte[0]);
+      Permission.Action.READ);
-      AccessControlService.BlockingInterface protocol =
+    SecureTestUtil.grantOnTable(TEST_UTIL, userColumn.getShortName(),
-        AccessControlService.newBlockingStub(channel);
+      TEST_TABLE.getTableName(), TEST_FAMILY, null,
-      ProtobufUtil.grant(protocol, userTable.getShortName(),
+      Permission.Action.READ);
-        TEST_TABLE.getTableName(), null, null, Permission.Action.READ);
+    SecureTestUtil.grantOnTable(TEST_UTIL, userQualifier.getShortName(),
-      ProtobufUtil.grant(protocol, userColumn.getShortName(),
+      TEST_TABLE.getTableName(), TEST_FAMILY, TEST_Q1,
-        TEST_TABLE.getTableName(), TEST_FAMILY, null, Permission.Action.READ);
+      Permission.Action.READ);
-      ProtobufUtil.grant(protocol, userQualifier.getShortName(),
-        TEST_TABLE.getTableName(), TEST_FAMILY, TEST_Q1, Permission.Action.READ);
-    } finally {
-      acl.close();
-    }
 
     AccessTestAction tableRead = new AccessTestAction() {
       @Override
@@ -2184,7 +1969,7 @@ public class TestAccessController extends SecureTestUtil {
                   .setTableName(ProtobufUtil.toProtoTableName(TEST_TABLE.getTableName()))
                   .addAction(AccessControlProtos.Permission.Action.CREATE))
       ).build();
-    acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
     try {
       BlockingRpcChannel channel = acl.coprocessorService(new byte[0]);
       AccessControlService.BlockingInterface protocol =
@@ -2301,21 +2086,13 @@ public class TestAccessController extends SecureTestUtil {
 
     // Since each RegionServer running on different user, add global
     // permissions for the new user.
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    String currentUser = User.getCurrent().getShortName();
-    try {
+    String activeUserForNewRs = currentUser + ".hfs." +
-      BlockingRpcChannel service = acl.coprocessorService(TEST_TABLE.getTableName().getName());
+      hbaseCluster.getLiveRegionServerThreads().size();
-      AccessControlService.BlockingInterface protocol =
+    SecureTestUtil.grantGlobal(TEST_UTIL, activeUserForNewRs,
-        AccessControlService.newBlockingStub(service);
+      Permission.Action.ADMIN, Permission.Action.CREATE, Permission.Action.READ,
-      String currentUser = User.getCurrent().getShortName();
+        Permission.Action.WRITE);
-      // User name for the new RegionServer we plan to add.
+
-      String activeUserForNewRs = currentUser + ".hfs."
-          + hbaseCluster.getLiveRegionServerThreads().size();
-      ProtobufUtil.grant(protocol, activeUserForNewRs,
-        Permission.Action.ADMIN, Permission.Action.CREATE,
-        Permission.Action.READ, Permission.Action.WRITE);
-    } finally {
-      acl.close();
-    }
     final HBaseAdmin admin = TEST_UTIL.getHBaseAdmin();
     HTableDescriptor htd = new HTableDescriptor(TEST_TABLE2);
     htd.addFamily(new HColumnDescriptor(TEST_FAMILY));
@@ -2380,16 +2157,9 @@ public class TestAccessController extends SecureTestUtil {
     User TABLE_ADMIN = User.createUserForTesting(conf, "UserA", new String[0]);
 
     // Grant TABLE ADMIN privs
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantOnTable(TEST_UTIL, TABLE_ADMIN.getShortName(),
-    try {
+      TEST_TABLE.getTableName(), null, null,
-      BlockingRpcChannel service = acl.coprocessorService(TEST_TABLE.getTableName().getName());
+      Permission.Action.ADMIN);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, TABLE_ADMIN.getShortName(), TEST_TABLE.getTableName(),
-        null, null, Permission.Action.ADMIN);
-    } finally {
-      acl.close();
-    }
 
     AccessTestAction listTablesAction = new AccessTestAction() {
       @Override
@@ -2429,16 +2199,9 @@ public class TestAccessController extends SecureTestUtil {
     User TABLE_ADMIN = User.createUserForTesting(conf, "TestUser", new String[0]);
 
     // Grant TABLE ADMIN privs
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantOnTable(TEST_UTIL, TABLE_ADMIN.getShortName(),
-    try {
+      TEST_TABLE.getTableName(), null, null,
-      BlockingRpcChannel service = acl.coprocessorService(TEST_TABLE.getTableName().getName());
+      Permission.Action.ADMIN);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, TABLE_ADMIN.getShortName(), TEST_TABLE.getTableName(),
-        null, null, Permission.Action.ADMIN);
-    } finally {
-      acl.close();
-    }
 
     AccessTestAction deleteTableAction = new AccessTestAction() {
       @Override
@@ -2474,20 +2237,10 @@ public class TestAccessController extends SecureTestUtil {
 
     verifyDenied(getAction, USER_NONE);
 
-    // Grant namespace READ to USER_NONE, this should supercede any table permissions
+    // Grant namespace READ to USER_NONE, this should supersede any table permissions
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
+    SecureTestUtil.grantOnNamespace(TEST_UTIL, USER_NONE.getShortName(),
-    try {
+      TEST_TABLE.getTableName().getNamespaceAsString(),
-      BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_BYTE_ARRAY);
+      Permission.Action.READ);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      AccessControlProtos.GrantRequest request = RequestConverter.
-        buildGrantRequest(USER_NONE.getShortName(),
-          TEST_TABLE.getTableName().getNamespaceAsString(),
-          AccessControlProtos.Permission.Action.READ);
-      protocol.grant(null, request);
-    } finally {
-      acl.close();
-    }
 
     // Now USER_NONE should be able to read also
     verifyAllowed(getAction, USER_NONE);

hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java

@@ -20,18 +20,14 @@ package org.apache.hadoop.hbase.security.access;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
-import java.security.PrivilegedExceptionAction;
 import java.util.List;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.Coprocessor;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
-import org.apache.hadoop.hbase.HColumnDescriptor;
 import org.apache.hadoop.hbase.HConstants;
-import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.MediumTests;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
-import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.client.Result;
@@ -44,6 +40,7 @@ import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessCont
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.security.access.Permission.Action;
 import org.apache.hadoop.hbase.util.Bytes;
+
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -53,7 +50,6 @@ import com.google.common.collect.ListMultimap;
 import com.google.protobuf.BlockingRpcChannel;
 
 @Category(MediumTests.class)
-@SuppressWarnings("rawtypes")
 public class TestNamespaceCommands extends SecureTestUtil {
   private static HBaseTestingUtility UTIL = new HBaseTestingUtility();
   private static String TestNamespace = "ns1";
@@ -61,11 +57,11 @@ public class TestNamespaceCommands extends SecureTestUtil {
   private static MasterCoprocessorEnvironment CP_ENV;
   private static AccessController ACCESS_CONTROLLER;
   
-//user with all permissions
+  // user with all permissions
   private static User SUPERUSER;
- // user with rw permissions
+  // user with rw permissions
   private static User USER_RW;
- // user with create table permissions alone
+  // user with create table permissions alone
   private static User USER_CREATE;
   // user with permission on namespace for testing all operations.
   private static User USER_NSP_WRITE;
@@ -85,20 +81,12 @@ public class TestNamespaceCommands extends SecureTestUtil {
     // Wait for the ACL table to become available
     UTIL.waitTableAvailable(AccessControlLists.ACL_TABLE_NAME.getName(), 30 * 1000);
 
-    HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
     MasterCoprocessorHost cpHost = UTIL.getMiniHBaseCluster().getMaster().getCoprocessorHost();
     cpHost.load(AccessController.class, Coprocessor.PRIORITY_HIGHEST, conf);
     ACCESS_CONTROLLER = (AccessController) cpHost.findCoprocessor(AccessController.class.getName());
-    try {
+
-      BlockingRpcChannel service =
+    SecureTestUtil.grantOnNamespace(UTIL, USER_NSP_WRITE.getShortName(),
-          acl.coprocessorService(HConstants.EMPTY_START_ROW);
+      TestNamespace, Permission.Action.WRITE);
-      AccessControlService.BlockingInterface protocol =
-        AccessControlService.newBlockingStub(service);
-      ProtobufUtil.grant(protocol, USER_NSP_WRITE.getShortName(),
-          TestNamespace, Action.WRITE);
-    } finally {
-      acl.close();
-    }
   }
   
   @AfterClass
@@ -110,12 +98,12 @@ public class TestNamespaceCommands extends SecureTestUtil {
   @Test
   public void testAclTableEntries() throws Exception {
     String userTestNamespace = "userTestNsp";
-    AccessControlService.BlockingInterface protocol = null;
     HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
     try {
-      BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+      // Grant and check state in ACL table
-      protocol = AccessControlService.newBlockingStub(service);
+      SecureTestUtil.grantOnNamespace(UTIL, userTestNamespace, TestNamespace,
-      ProtobufUtil.grant(protocol, userTestNamespace, TestNamespace, Permission.Action.WRITE);
+        Permission.Action.WRITE);
+
       Result result = acl.get(new Get(Bytes.toBytes(userTestNamespace)));
       assertTrue(result != null);
       ListMultimap<String, TablePermission> perms =
@@ -130,9 +118,11 @@ public class TestNamespaceCommands extends SecureTestUtil {
       assertEquals(null, namespacePerms.get(0).getQualifier());
       assertEquals(1, namespacePerms.get(0).getActions().length);
       assertEquals(Permission.Action.WRITE, namespacePerms.get(0).getActions()[0]);
-      // Now revoke and check.
+
-      ProtobufUtil.revoke(protocol, userTestNamespace, TestNamespace,
+      // Revoke and check state in ACL table
-          Permission.Action.WRITE);
+      SecureTestUtil.revokeFromNamespace(UTIL, userTestNamespace, TestNamespace,
+        Permission.Action.WRITE);
+
       perms = AccessControlLists.getNamespacePermissions(conf, TestNamespace);
       assertEquals(1, perms.size());
     } finally {
@@ -142,7 +132,7 @@ public class TestNamespaceCommands extends SecureTestUtil {
   
   @Test
   public void testModifyNamespace() throws Exception {
-    PrivilegedExceptionAction modifyNamespace = new PrivilegedExceptionAction() {
+    AccessTestAction modifyNamespace = new AccessTestAction() {
       public Object run() throws Exception {
         ACCESS_CONTROLLER.preModifyNamespace(ObserverContext.createAndPrepare(CP_ENV, null),
           NamespaceDescriptor.create(TestNamespace).addConfiguration("abc", "156").build());
@@ -157,14 +147,16 @@ public class TestNamespaceCommands extends SecureTestUtil {
   
   @Test
   public void testGrantRevoke() throws Exception{
-    //Only HBase super user should be able to grant and revoke permissions to
-    // namespaces.
     final String testUser = "testUser";
-    PrivilegedExceptionAction grantAction = new PrivilegedExceptionAction() {
+
+    // Test if client API actions are authorized
+
+    AccessTestAction grantAction = new AccessTestAction() {
       public Object run() throws Exception {
         HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
         try {
-          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          BlockingRpcChannel service =
+              acl.coprocessorService(HConstants.EMPTY_START_ROW);
           AccessControlService.BlockingInterface protocol =
             AccessControlService.newBlockingStub(service);
           ProtobufUtil.grant(protocol, testUser, TestNamespace, Action.WRITE);
@@ -175,11 +167,12 @@ public class TestNamespaceCommands extends SecureTestUtil {
       }
     };
 
-    PrivilegedExceptionAction revokeAction = new PrivilegedExceptionAction() {
+    AccessTestAction revokeAction = new AccessTestAction() {
       public Object run() throws Exception {
         HTable acl = new HTable(conf, AccessControlLists.ACL_TABLE_NAME);
         try {
-          BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW);
+          BlockingRpcChannel service =
+              acl.coprocessorService(HConstants.EMPTY_START_ROW);
           AccessControlService.BlockingInterface protocol =
             AccessControlService.newBlockingStub(service);
           ProtobufUtil.revoke(protocol, testUser, TestNamespace, Action.WRITE);
@@ -190,11 +183,11 @@ 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);
-
     verifyAllowed(revokeAction, SUPERUSER);
     verifyDenied(revokeAction, USER_CREATE, USER_RW);    
-    
   }
 }