HBASE-21974 Change Admin#grant/revoke parameter from UserPermission to user and Permission

Signed-off-by: Guanghao Zhang <zghao@apache.org>

hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java

@@ -52,7 +52,7 @@ import org.apache.hadoop.hbase.regionserver.wal.FailedLogCloseException;
 import org.apache.hadoop.hbase.replication.ReplicationException;
 import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
 import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
-import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException;
 import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
 import org.apache.hadoop.hbase.snapshot.SnapshotCreationException;
@@ -2806,20 +2806,21 @@ public interface Admin extends Abortable, Closeable {
 
   /**
    * Grants user specific permissions
-   *
+   * @param userName user name
-   * @param userPermission user and permissions
+   * @param permission the specific permission
    * @param mergeExistingPermissions If set to false, later granted permissions will override
    *          previous granted permissions. otherwise, it'll merge with previous granted
    *          permissions.
    * @throws IOException if a remote or network exception occurs
    */
-  void grant(UserPermission userPermission, boolean mergeExistingPermissions) throws IOException;
+  void grant(String userName, Permission permission, boolean mergeExistingPermissions)
+      throws IOException;
 
   /**
    * Revokes user specific permissions
-   *
+   * @param userName user name
-   * @param userPermission user and permissions
+   * @param permission the specific permission
    * @throws IOException if a remote or network exception occurs
    */
-  void revoke(UserPermission userPermission) throws IOException;
+  void revoke(String userName, Permission permission) throws IOException;
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java

@@ -41,7 +41,7 @@ import org.apache.hadoop.hbase.quotas.QuotaSettings;
 import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshotView;
 import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
 import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
-import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.yetus.audience.InterfaceAudience;
 
 /**
@@ -1305,16 +1305,19 @@ public interface AsyncAdmin {
 
   /**
    * Grants user specific permissions
-   * @param userPermission user and permissions
+   * @param userName user name
+   * @param permission the specific permission
    * @param mergeExistingPermissions If set to false, later granted permissions will override
    *          previous granted permissions. otherwise, it'll merge with previous granted
    *          permissions.
    */
-  CompletableFuture<Void> grant(UserPermission userPermission, boolean mergeExistingPermissions);
+  CompletableFuture<Void> grant(String userName, Permission permission,
+      boolean mergeExistingPermissions);
 
   /**
    * Revokes user specific permissions
-   * @param userPermission user and permissions
+   * @param userName user name
+   * @param permission the specific permission
    */
-  CompletableFuture<Void> revoke(UserPermission userPermission);
+  CompletableFuture<Void> revoke(String userName, Permission permission);
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java

@@ -41,7 +41,7 @@ import org.apache.hadoop.hbase.quotas.QuotaSettings;
 import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot;
 import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
 import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
-import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.hadoop.hbase.util.FutureUtils;
 import org.apache.yetus.audience.InterfaceAudience;
 
@@ -790,13 +790,13 @@ class AsyncHBaseAdmin implements AsyncAdmin {
   }
 
   @Override
-  public CompletableFuture<Void> grant(UserPermission userPermission,
+  public CompletableFuture<Void> grant(String userName, Permission permission,
       boolean mergeExistingPermissions) {
-    return wrap(rawAdmin.grant(userPermission, mergeExistingPermissions));
+    return wrap(rawAdmin.grant(userName, permission, mergeExistingPermissions));
   }
 
   @Override
-  public CompletableFuture<Void> revoke(UserPermission userPermission) {
+  public CompletableFuture<Void> revoke(String userName, Permission permission) {
-    return wrap(rawAdmin.revoke(userPermission));
+    return wrap(rawAdmin.revoke(userName, permission));
   }
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java

@@ -90,6 +90,7 @@ import org.apache.hadoop.hbase.regionserver.wal.FailedLogCloseException;
 import org.apache.hadoop.hbase.replication.ReplicationException;
 import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
 import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
+import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.hadoop.hbase.security.access.ShadedAccessControlUtil;
 import org.apache.hadoop.hbase.security.access.UserPermission;
 import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils;
@@ -4458,13 +4459,13 @@ public class HBaseAdmin implements Admin {
   }
 
   @Override
-  public void grant(UserPermission userPermission, boolean mergeExistingPermissions)
+  public void grant(String userName, Permission permission, boolean mergeExistingPermissions)
       throws IOException {
     executeCallable(new MasterCallable<Void>(getConnection(), getRpcControllerFactory()) {
       @Override
       protected Void rpcCall() throws Exception {
-        GrantRequest req =
+        GrantRequest req = ShadedAccessControlUtil
-            ShadedAccessControlUtil.buildGrantRequest(userPermission, mergeExistingPermissions);
+            .buildGrantRequest(new UserPermission(userName, permission), mergeExistingPermissions);
         this.master.grant(getRpcController(), req);
         return null;
       }
@@ -4472,11 +4473,12 @@ public class HBaseAdmin implements Admin {
   }
 
   @Override
-  public void revoke(UserPermission userPermission) throws IOException {
+  public void revoke(String userName, Permission permission) throws IOException {
     executeCallable(new MasterCallable<Void>(getConnection(), getRpcControllerFactory()) {
       @Override
       protected Void rpcCall() throws Exception {
-        RevokeRequest req = ShadedAccessControlUtil.buildRevokeRequest(userPermission);
+        RevokeRequest req =
+            ShadedAccessControlUtil.buildRevokeRequest(new UserPermission(userName, permission));
         this.master.revoke(getRpcController(), req);
         return null;
       }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java

@@ -84,6 +84,7 @@ import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot;
 import org.apache.hadoop.hbase.replication.ReplicationException;
 import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
 import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
+import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.hadoop.hbase.security.access.ShadedAccessControlUtil;
 import org.apache.hadoop.hbase.security.access.UserPermission;
 import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils;
@@ -3739,21 +3740,24 @@ class RawAsyncHBaseAdmin implements AsyncAdmin {
   }
 
   @Override
-  public CompletableFuture<Void> grant(UserPermission userPermission,
+  public CompletableFuture<Void> grant(String userName, Permission permission,
       boolean mergeExistingPermissions) {
     return this.<Void> newMasterCaller()
-        .action((controller, stub) -> this.<GrantRequest, GrantResponse, Void> call(controller,
+        .action(
-          stub, ShadedAccessControlUtil.buildGrantRequest(userPermission, mergeExistingPermissions),
+          (controller, stub) -> this.<GrantRequest, GrantResponse, Void> call(controller, stub,
-          (s, c, req, done) -> s.grant(c, req, done), resp -> null))
+            ShadedAccessControlUtil.buildGrantRequest(new UserPermission(userName, permission),
+              mergeExistingPermissions),
+            (s, c, req, done) -> s.grant(c, req, done), resp -> null))
         .call();
   }
 
   @Override
-  public CompletableFuture<Void> revoke(UserPermission userPermission) {
+  public CompletableFuture<Void> revoke(String userName, Permission permission) {
     return this.<Void> newMasterCaller()
-        .action((controller, stub) -> this.<RevokeRequest, RevokeResponse, Void> call(controller,
+        .action(
-          stub, ShadedAccessControlUtil.buildRevokeRequest(userPermission),
+          (controller, stub) -> this.<RevokeRequest, RevokeResponse, Void> call(controller, stub,
-          (s, c, req, done) -> s.revoke(c, req, done), resp -> null))
+            ShadedAccessControlUtil.buildRevokeRequest(new UserPermission(userName, permission)),
+            (s, c, req, done) -> s.revoke(c, req, done), resp -> null))
         .call();
   }
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java

@@ -93,10 +93,8 @@ public class AccessControlClient {
   private static void grant(Connection connection, final TableName tableName,
       final String userName, final byte[] family, final byte[] qual, boolean mergeExistingPermissions,
       final Permission.Action... actions) throws Throwable {
-    // TODO: Priority is not used.
+    connection.getAdmin().grant(userName, new TablePermission(tableName, family, qual, actions),
-    UserPermission userPermission =
+      mergeExistingPermissions);
-        new UserPermission(userName, new TablePermission(tableName, family, qual, actions));
-    connection.getAdmin().grant(userPermission, mergeExistingPermissions);
   }
 
   /**
@@ -128,9 +126,8 @@ public class AccessControlClient {
    */
   private static void grant(Connection connection, final String namespace, final String userName,
       boolean mergeExistingPermissions, final Permission.Action... actions) throws Throwable {
-    UserPermission userPermission =
+    connection.getAdmin().grant(userName, new NamespacePermission(namespace, actions),
-        new UserPermission(userName, new NamespacePermission(namespace, actions));
+      mergeExistingPermissions);
-    connection.getAdmin().grant(userPermission, mergeExistingPermissions);
   }
 
   /**
@@ -160,8 +157,7 @@ public class AccessControlClient {
    */
   private static void grant(Connection connection, final String userName,
       boolean mergeExistingPermissions, final Permission.Action... actions) throws Throwable {
-    UserPermission userPermission = new UserPermission(userName, new GlobalPermission(actions));
+    connection.getAdmin().grant(userName, new GlobalPermission(actions), mergeExistingPermissions);
-    connection.getAdmin().grant(userPermission, mergeExistingPermissions);
   }
 
   /**
@@ -198,9 +194,8 @@ public class AccessControlClient {
   public static void revoke(Connection connection, final TableName tableName,
       final String username, final byte[] family, final byte[] qualifier,
       final Permission.Action... actions) throws Throwable {
-    UserPermission userPermission =
+    connection.getAdmin().revoke(username,
-        new UserPermission(username, new TablePermission(tableName, family, qualifier, actions));
+      new TablePermission(tableName, family, qualifier, actions));
-    connection.getAdmin().revoke(userPermission);
   }
 
   /**
@@ -213,9 +208,7 @@ public class AccessControlClient {
    */
   public static void revoke(Connection connection, final String namespace,
       final String userName, final Permission.Action... actions) throws Throwable {
-    UserPermission userPermission =
+    connection.getAdmin().revoke(userName, new NamespacePermission(namespace, actions));
-        new UserPermission(userName, new NamespacePermission(namespace, actions));
-    connection.getAdmin().revoke(userPermission);
   }
 
   /**
@@ -224,8 +217,7 @@ public class AccessControlClient {
    */
   public static void revoke(Connection connection, final String userName,
       final Permission.Action... actions) throws Throwable {
-    UserPermission userPermission = new UserPermission(userName, new GlobalPermission(actions));
+    connection.getAdmin().revoke(userName, new GlobalPermission(actions));
-    connection.getAdmin().revoke(userPermission);
   }
 
   /**

hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java

@@ -491,7 +491,7 @@ public class AccessControlUtil {
    * @param userShortName the short name of the user to grant permissions
    * @param actions the permissions to be granted
    * @throws ServiceException
-   * @deprecated Use {@link Admin#grant(UserPermission, boolean)} instead.
+   * @deprecated Use {@link Admin#grant(String, Permission, boolean)} instead.
    */
   @Deprecated
   public static void grant(RpcController controller,
@@ -520,7 +520,7 @@ public class AccessControlUtil {
    * @param q optional qualifier
    * @param actions the permissions to be granted
    * @throws ServiceException
-   * @deprecated Use {@link Admin#grant(UserPermission, boolean)} instead.
+   * @deprecated Use {@link Admin#grant(String, Permission, boolean)} instead.
    */
   @Deprecated
   public static void grant(RpcController controller,
@@ -548,7 +548,7 @@ public class AccessControlUtil {
    * @param namespace the short name of the user to grant permissions
    * @param actions the permissions to be granted
    * @throws ServiceException
-   * @deprecated Use {@link Admin#grant(UserPermission, boolean)} instead.
+   * @deprecated Use {@link Admin#grant(String, Permission, boolean)} instead.
    */
   @Deprecated
   public static void grant(RpcController controller,
@@ -574,7 +574,7 @@ public class AccessControlUtil {
    * @param userShortName the short name of the user to revoke permissions
    * @param actions the permissions to be revoked
    * @throws ServiceException on failure
-   * @deprecated Use {@link Admin#grant(UserPermission, boolean)} instead.
+   * @deprecated Use {@link Admin#revoke(String, Permission)} instead.
    */
   @Deprecated
   public static void revoke(RpcController controller,
@@ -604,7 +604,7 @@ public class AccessControlUtil {
    * @param q optional qualifier
    * @param actions the permissions to be revoked
    * @throws ServiceException on failure
-   * @deprecated Use {@link Admin#grant(UserPermission, boolean)} instead.
+   * @deprecated Use {@link Admin#revoke(String, Permission)} instead.
    */
   @Deprecated
   public static void revoke(RpcController controller,
@@ -631,7 +631,7 @@ public class AccessControlUtil {
    * @param namespace optional table name
    * @param actions the permissions to be revoked
    * @throws ServiceException on failure
-   * @deprecated Use {@link Admin#grant(UserPermission, boolean)} instead.
+   * @deprecated Use {@link Admin#revoke(String, Permission)} instead.
    */
   @Deprecated
   public static void revoke(RpcController controller,

hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java

@@ -24,7 +24,9 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.Map;
+import java.util.Objects;
 
+import org.apache.hadoop.hbase.TableName;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -241,4 +243,77 @@ public class Permission extends VersionedWritable {
   public Scope getAccessScope() {
     return scope;
   }
+
+  /**
+   * Build a global permission
+   * @return global permission builder
+   */
+  public static Builder newBuilder() {
+    return new Builder();
+  }
+
+  /**
+   * Build a namespace permission
+   * @param namespace the specific namespace
+   * @return namespace permission builder
+   */
+  public static Builder newBuilder(String namespace) {
+    return new Builder(namespace);
+  }
+
+  /**
+   * Build a table permission
+   * @param tableName the specific table name
+   * @return table permission builder
+   */
+  public static Builder newBuilder(TableName tableName) {
+    return new Builder(tableName);
+  }
+
+  public static final class Builder {
+    private String namespace;
+    private TableName tableName;
+    private byte[] family;
+    private byte[] qualifier;
+    private Action[] actions;
+
+    private Builder() {
+    }
+
+    private Builder(String namespace) {
+      this.namespace = namespace;
+    }
+
+    private Builder(TableName tableName) {
+      this.tableName = tableName;
+    }
+
+    public Builder withFamily(byte[] family) {
+      Objects.requireNonNull(tableName, "The tableName can't be NULL");
+      this.family = family;
+      return this;
+    }
+
+    public Builder withQualifier(byte[] qualifier) {
+      Objects.requireNonNull(tableName, "The tableName can't be NULL");
+      this.qualifier = qualifier;
+      return this;
+    }
+
+    public Builder withActions(Action... actions) {
+      this.actions = actions;
+      return this;
+    }
+
+    public Permission build() {
+      if (namespace != null) {
+        return new NamespacePermission(namespace, actions);
+      } else if (tableName != null) {
+        return new TablePermission(tableName, family, qualifier, actions);
+      } else {
+        return new GlobalPermission(actions);
+      }
+    }
+  }
+
 }

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

@@ -2054,7 +2054,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
   /* ---- Protobuf AccessControlService implementation ---- */
 
   /**
-   * @deprecated Use {@link Admin#grant(UserPermission, boolean)} instead.
+   * @deprecated Use {@link Admin#grant(String, Permission, boolean)} instead.
    */
   @Deprecated
   @Override
@@ -2077,7 +2077,8 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
         preGrantOrRevoke(caller, "grant", perm);
 
         // regionEnv is set at #start. Hopefully not null at this point.
-        regionEnv.getConnection().getAdmin().grant(perm, request.getMergeExistingPermissions());
+        regionEnv.getConnection().getAdmin().grant(perm.getUser(), perm.getPermission(),
+          request.getMergeExistingPermissions());
         if (AUDITLOG.isTraceEnabled()) {
           // audit log should store permission changes in addition to auth results
           AUDITLOG.trace("Granted permission " + perm.toString());
@@ -2095,7 +2096,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
   }
 
   /**
-   * @deprecated Use {@link Admin#revoke(UserPermission)} instead.
+   * @deprecated Use {@link Admin#revoke(String, Permission)} instead.
    */
   @Deprecated
   @Override
@@ -2116,7 +2117,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
         }
         preGrantOrRevoke(caller, "revoke", perm);
         // regionEnv is set at #start. Hopefully not null here.
-        regionEnv.getConnection().getAdmin().revoke(perm);
+        regionEnv.getConnection().getAdmin().revoke(perm.getUser(), perm.getPermission());
         if (AUDITLOG.isTraceEnabled()) {
           // audit log should record all permission changes
           AUDITLOG.trace("Revoked permission " + perm.toString());

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

@@ -378,8 +378,7 @@ public class SecureTestUtil {
       @Override
       public Void call() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(util.getConfiguration())) {
-          connection.getAdmin().grant(new UserPermission(user, new GlobalPermission(actions)),
+          connection.getAdmin().grant(user, new GlobalPermission(actions), false);
-            false);
         }
         return null;
       }
@@ -398,7 +397,7 @@ public class SecureTestUtil {
       public Void call() throws Exception {
         Configuration conf = util.getConfiguration();
         try (Connection connection = ConnectionFactory.createConnection(conf, caller)) {
-          connection.getAdmin().grant(new UserPermission(user, new GlobalPermission(actions)),
+          connection.getAdmin().grant(user, Permission.newBuilder().withActions(actions).build(),
             false);
         }
         return null;
@@ -417,7 +416,7 @@ public class SecureTestUtil {
       @Override
       public Void call() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(util.getConfiguration())) {
-          connection.getAdmin().revoke(new UserPermission(user, new GlobalPermission(actions)));
+          connection.getAdmin().revoke(user, new GlobalPermission(actions));
         }
         return null;
       }
@@ -436,7 +435,7 @@ public class SecureTestUtil {
       public Void call() throws Exception {
         Configuration conf = util.getConfiguration();
         try (Connection connection = ConnectionFactory.createConnection(conf, caller)) {
-          connection.getAdmin().revoke(new UserPermission(user, new GlobalPermission(actions)));
+          connection.getAdmin().revoke(user, Permission.newBuilder().withActions(actions).build());
         }
         return null;
       }
@@ -455,7 +454,7 @@ public class SecureTestUtil {
       public Void call() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(util.getConfiguration())) {
           connection.getAdmin()
-              .grant(new UserPermission(user, new NamespacePermission(namespace, actions)), false);
+              .grant(user, new NamespacePermission(namespace, actions), false);
         }
         return null;
       }
@@ -475,8 +474,8 @@ public class SecureTestUtil {
       public Void call() throws Exception {
         Configuration conf = util.getConfiguration();
         try (Connection connection = ConnectionFactory.createConnection(conf, caller)) {
-          connection.getAdmin()
+          connection.getAdmin().grant(user,
-            .grant(new UserPermission(user, new NamespacePermission(namespace, actions)), false);
+            Permission.newBuilder(namespace).withActions(actions).build(), false);
         }
         return null;
       }
@@ -536,8 +535,7 @@ public class SecureTestUtil {
       @Override
       public Void call() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(util.getConfiguration())) {
-          connection.getAdmin()
+          connection.getAdmin().revoke(user, new NamespacePermission(namespace, actions));
-              .revoke(new UserPermission(user, new NamespacePermission(namespace, actions)));
         }
         return null;
       }
@@ -557,8 +555,8 @@ public class SecureTestUtil {
       public Void call() throws Exception {
         Configuration conf = util.getConfiguration();
         try (Connection connection = ConnectionFactory.createConnection(conf, caller)) {
-          connection.getAdmin()
+          connection.getAdmin().revoke(user,
-            .revoke(new UserPermission(user, new NamespacePermission(namespace, actions)));
+            Permission.newBuilder(namespace).withActions(actions).build());
         }
         return null;
       }
@@ -577,8 +575,7 @@ public class SecureTestUtil {
       @Override
       public Void call() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(util.getConfiguration())) {
-          connection.getAdmin().grant(
+          connection.getAdmin().grant(user, new TablePermission(table, family, qualifier, actions),
-            new UserPermission(user, new TablePermission(table, family, qualifier, actions)),
             false);
         }
         return null;
@@ -599,8 +596,8 @@ public class SecureTestUtil {
       public Void call() throws Exception {
         Configuration conf = util.getConfiguration();
         try (Connection connection = ConnectionFactory.createConnection(conf, caller)) {
-          connection.getAdmin().grant(
+          connection.getAdmin().grant(user, Permission.newBuilder(table).withFamily(family)
-            new UserPermission(user, new TablePermission(table, family, qualifier, actions)),
+              .withQualifier(qualifier).withActions(actions).build(),
             false);
         }
         return null;
@@ -662,8 +659,8 @@ public class SecureTestUtil {
       @Override
       public Void call() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(util.getConfiguration())) {
-          connection.getAdmin().revoke(
+          connection.getAdmin().revoke(user,
-            new UserPermission(user, new TablePermission(table, family, qualifier, actions)));
+            new TablePermission(table, family, qualifier, actions));
         }
         return null;
       }
@@ -683,8 +680,8 @@ public class SecureTestUtil {
       public Void call() throws Exception {
         Configuration conf = util.getConfiguration();
         try (Connection connection = ConnectionFactory.createConnection(conf, caller)) {
-          connection.getAdmin().revoke(
+          connection.getAdmin().revoke(user, Permission.newBuilder(table).withFamily(family)
-            new UserPermission(user, new TablePermission(table, family, qualifier, actions)));
+              .withQualifier(qualifier).withActions(actions).build());
         }
         return null;
       }

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

@@ -1168,9 +1168,8 @@ public class TestAccessController extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         try (Connection conn = ConnectionFactory.createConnection(conf)) {
-          conn.getAdmin().grant(new UserPermission(USER_RO.getShortName(),
+          conn.getAdmin().grant(USER_RO.getShortName(),
-              new TablePermission(TEST_TABLE, TEST_FAMILY, Action.READ)),
+            new TablePermission(TEST_TABLE, TEST_FAMILY, Action.READ), false);
-            false);
         }
         return null;
       }
@@ -1179,9 +1178,9 @@ public class TestAccessController extends SecureTestUtil {
     AccessTestAction revokeAction = new AccessTestAction() {
       @Override
       public Object run() throws Exception {
-        try(Connection conn = ConnectionFactory.createConnection(conf)) {
+        try (Connection conn = ConnectionFactory.createConnection(conf)) {
-            conn.getAdmin().revoke(new UserPermission(USER_RO.getShortName(),
+          conn.getAdmin().revoke(USER_RO.getShortName(), Permission.newBuilder(TEST_TABLE)
-                            new TablePermission(TEST_TABLE, TEST_FAMILY, Action.READ)));
+              .withFamily(TEST_FAMILY).withActions(Action.READ).build());
         }
         return null;
       }

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

@@ -362,9 +362,8 @@ public class TestNamespaceCommands extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(conf)) {
-          connection.getAdmin().grant(
+          connection.getAdmin().grant(testUser,
-            new UserPermission(testUser, new NamespacePermission(TEST_NAMESPACE, Action.WRITE)),
+            new NamespacePermission(TEST_NAMESPACE, Action.WRITE), false);
-            false);
         }
         return null;
       }
@@ -373,9 +372,8 @@ public class TestNamespaceCommands extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         try (Connection conn = ConnectionFactory.createConnection(conf)) {
-          conn.getAdmin().grant(
+          conn.getAdmin().grant(USER_GROUP_NS_ADMIN.getShortName(),
-            new UserPermission(USER_GROUP_NS_ADMIN.getShortName(), TEST_NAMESPACE, Action.READ),
+            new NamespacePermission(TEST_NAMESPACE, Action.READ), false);
-            false);
         }
         return null;
       }
@@ -385,8 +383,8 @@ public class TestNamespaceCommands extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(conf)) {
-          connection.getAdmin().revoke(
+          connection.getAdmin().revoke(testUser,
-            new UserPermission(testUser, new NamespacePermission(TEST_NAMESPACE, Action.WRITE)));
+            new NamespacePermission(TEST_NAMESPACE, Action.WRITE));
         }
         return null;
       }
@@ -395,8 +393,8 @@ public class TestNamespaceCommands extends SecureTestUtil {
       @Override
       public Object run() throws Exception {
         try (Connection connection = ConnectionFactory.createConnection(conf)) {
-          connection.getAdmin().revoke(new UserPermission(USER_GROUP_NS_ADMIN.getShortName(),
+          connection.getAdmin().revoke(USER_GROUP_NS_ADMIN.getShortName(),
-              new NamespacePermission(TEST_NAMESPACE, Action.READ)));
+            new NamespacePermission(TEST_NAMESPACE, Action.READ));
         }
         return null;
       }

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

@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.security.access;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.security.access.Permission.Action;
+import org.apache.hadoop.hbase.testclassification.SecurityTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ SecurityTests.class, SmallTests.class })
+public class TestPermissionBuilder {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+      HBaseClassTestRule.forClass(TestPermissionBuilder.class);
+
+  @Test
+  public void testBuildGlobalPermission() {
+    // check global permission with empty action
+    Permission permission = Permission.newBuilder().build();
+    assertTrue(permission instanceof GlobalPermission);
+    assertEquals(0, permission.getActions().length);
+
+    // check global permission with ADMIN action
+    permission = Permission.newBuilder().withActions(Action.ADMIN).build();
+    assertTrue(permission instanceof GlobalPermission);
+    assertEquals(1, permission.getActions().length);
+    assertTrue(permission.getActions()[0] == Action.ADMIN);
+
+    byte[] qualifier = Bytes.toBytes("q");
+    try {
+      permission = Permission.newBuilder().withQualifier(qualifier)
+          .withActions(Action.CREATE, Action.READ).build();
+      fail("Should throw NPE");
+    } catch (NullPointerException e) {
+      // catch NPE because set family but table name is null
+    }
+  }
+
+  @Test
+  public void testBuildNamespacePermission() {
+    String namespace = "ns";
+    // check namespace permission with CREATE and READ actions
+    Permission permission =
+        Permission.newBuilder(namespace).withActions(Action.CREATE, Action.READ).build();
+    assertTrue(permission instanceof NamespacePermission);
+    NamespacePermission namespacePermission = (NamespacePermission) permission;
+    assertEquals(namespace, namespacePermission.getNamespace());
+    assertEquals(2, permission.getActions().length);
+    assertEquals(Action.READ, permission.getActions()[0]);
+    assertEquals(Action.CREATE, permission.getActions()[1]);
+
+    byte[] family = Bytes.toBytes("f");
+    try {
+      permission = Permission.newBuilder(namespace).withFamily(family)
+          .withActions(Action.CREATE, Action.READ).build();
+      fail("Should throw NPE");
+    } catch (NullPointerException e) {
+      // catch NPE because set family but table name is null
+    }
+  }
+
+  @Test
+  public void testBuildTablePermission() {
+    TableName tableName = TableName.valueOf("ns", "table");
+    byte[] family = Bytes.toBytes("f");
+    byte[] qualifier = Bytes.toBytes("q");
+    // check table permission without family or qualifier
+    Permission permission =
+        Permission.newBuilder(tableName).withActions(Action.WRITE, Action.READ).build();
+    assertTrue(permission instanceof TablePermission);
+    assertEquals(2, permission.getActions().length);
+    assertEquals(Action.READ, permission.getActions()[0]);
+    assertEquals(Action.WRITE, permission.getActions()[1]);
+    TablePermission tPerm = (TablePermission) permission;
+    assertEquals(tableName, tPerm.getTableName());
+    assertEquals(null, tPerm.getFamily());
+    assertEquals(null, tPerm.getQualifier());
+
+    // check table permission with family
+    permission =
+        Permission.newBuilder(tableName).withFamily(family).withActions(Action.EXEC).build();
+    assertTrue(permission instanceof TablePermission);
+    assertEquals(1, permission.getActions().length);
+    assertEquals(Action.EXEC, permission.getActions()[0]);
+    tPerm = (TablePermission) permission;
+    assertEquals(tableName, tPerm.getTableName());
+    assertTrue(Bytes.equals(family, tPerm.getFamily()));
+    assertTrue(Bytes.equals(null, tPerm.getQualifier()));
+
+    // check table permission with family and qualifier
+    permission =
+        Permission.newBuilder(tableName).withFamily(family).withQualifier(qualifier).build();
+    assertTrue(permission instanceof TablePermission);
+    assertEquals(0, permission.getActions().length);
+    tPerm = (TablePermission) permission;
+    assertEquals(tableName, tPerm.getTableName());
+    assertTrue(Bytes.equals(family, tPerm.getFamily()));
+    assertTrue(Bytes.equals(qualifier, tPerm.getQualifier()));
+  }
+}

hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java

@@ -59,7 +59,7 @@ import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot;
 import org.apache.hadoop.hbase.replication.ReplicationException;
 import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
 import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
-import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.hadoop.hbase.thrift2.ThriftUtilities;
 import org.apache.hadoop.hbase.thrift2.generated.TColumnFamilyDescriptor;
 import org.apache.hadoop.hbase.thrift2.generated.THBaseService;
@@ -1418,12 +1418,12 @@ public class ThriftAdmin implements Admin {
   }
 
   @Override
-  public void grant(UserPermission userPermission, boolean mergeExistingPermissions) {
+  public void grant(String userName, Permission permission, boolean mergeExistingPermissions) {
     throw new NotImplementedException("grant not supported in ThriftAdmin");
   }
 
   @Override
-  public void revoke(UserPermission userPermission) {
+  public void revoke(String userName, Permission permission) {
     throw new NotImplementedException("revoke not supported in ThriftAdmin");
   }
 }