Repository: hbase
Updated Branches:
  refs/heads/master d08bafad1 -> 22fa1cd3d


HBASE-17472: Correct the semantic of permission grant

Signed-off-by: zhangduo <zhang...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/22fa1cd3
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/22fa1cd3
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/22fa1cd3

Branch: refs/heads/master
Commit: 22fa1cd3df3ed16ddbc0336ac2e52964c1e22665
Parents: d08bafa
Author: huzheng <open...@gmail.com>
Authored: Wed Jan 25 15:20:42 2017 +0800
Committer: zhangduo <zhang...@apache.org>
Committed: Mon Feb 20 20:13:43 2017 +0800

----------------------------------------------------------------------
 .../security/access/AccessControlClient.java    |  96 +++++++++----
 .../security/access/AccessControlUtil.java      |  30 ++--
 .../hbase/security/access/TablePermission.java  |  25 ++--
 .../protobuf/generated/AccessControlProtos.java | 139 +++++++++++++++----
 .../src/main/protobuf/AccessControl.proto       |   1 +
 .../security/access/AccessControlLists.java     |  39 +++++-
 .../hbase/security/access/AccessController.java |   2 +-
 .../hbase/security/access/SecureTestUtil.java   |  18 +--
 .../security/access/TestAccessController.java   | 125 ++++++++++++++++-
 .../security/access/TestNamespaceCommands.java  |   4 +-
 .../security/access/TestTablePermissions.java   |   2 +-
 11 files changed, 391 insertions(+), 90 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java
----------------------------------------------------------------------
diff --git 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java
 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java
index 96db2d1..eeac9c7 100644
--- 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java
+++ 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java
@@ -85,58 +85,106 @@ public class AccessControlClient {
    * @param userName
    * @param family
    * @param qual
+   * @param mergeExistingPermissions If set to false, later granted 
permissions will override
+   *          previous granted permissions. otherwise, it'll merge with 
previous granted
+   *          permissions.
    * @param actions
    * @throws Throwable
    */
-  public static void grant(Connection connection, final TableName tableName,
-      final String userName, final byte[] family, final byte[] qual,
+  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.
-    HBaseRpcController controller
-      = ((ClusterConnection) 
connection).getRpcControllerFactory().newController();
-    controller.setPriority(tableName);
-    */
+    // TODO: Priority is not used.
     try (Table table = connection.getTable(ACL_TABLE_NAME)) {
       AccessControlUtil.grant(null, getAccessControlServiceStub(table), 
userName, tableName,
-        family, qual, actions);
+        family, qual, mergeExistingPermissions, actions);
     }
   }
 
   /**
-   * Grants permission on the specified namespace for the specified user.
+   * Grants permission on the specified table for the specified user.
+   * If permissions for a specified user exists, later granted permissions 
will override previous granted permissions.
    * @param connection The Connection instance to use
+   * @param tableName
+   * @param userName
+   * @param family
+   * @param qual
+   * @param actions
+   * @throws Throwable
+   */
+  public static void grant(Connection connection, final TableName tableName, 
final String userName,
+      final byte[] family, final byte[] qual, final Permission.Action... 
actions) throws Throwable {
+    grant(connection, tableName, userName, family, qual, true, actions);
+  }
+
+  /**
+   * Grants permission on the specified namespace for the specified user.
+   * @param connection
    * @param namespace
    * @param userName
+   * @param mergeExistingPermissions If set to false, later granted 
permissions will override
+   *          previous granted permissions. otherwise, it'll merge with 
previous granted
+   *          permissions.
    * @param actions
    * @throws Throwable
    */
-  public static void grant(Connection connection, final String namespace,
-      final String userName, final Permission.Action... actions) throws 
Throwable {
-    /* TODO: Pass an rpcController.
-    HBaseRpcController controller
-      = ((ClusterConnection) 
connection).getRpcControllerFactory().newController();
-    */
+  private static void grant(Connection connection, final String namespace, 
final String userName,
+      boolean mergeExistingPermissions, final Permission.Action... actions) 
throws Throwable {
+    // TODO: Pass an rpcController.
     try (Table table = connection.getTable(ACL_TABLE_NAME)) {
       AccessControlUtil.grant(null, getAccessControlServiceStub(table), 
userName, namespace,
-        actions);
+        mergeExistingPermissions, actions);
     }
   }
 
   /**
+   * Grants permission on the specified namespace for the specified user.
+   * If permissions on the specified namespace exists, later granted 
permissions will override previous granted
+   * permissions.
    * @param connection The Connection instance to use
-   * Grant global permissions for the specified user.
+   * @param namespace
+   * @param userName
+   * @param actions
+   * @throws Throwable
    */
-  public static void grant(Connection connection, final String userName,
-       final Permission.Action... actions) throws Throwable {
-    /* TODO: Pass an rpcController
-    HBaseRpcController controller
-      = ((ClusterConnection) 
connection).getRpcControllerFactory().newController();
-    */
+  public static void grant(Connection connection, final String namespace, 
final String userName,
+      final Permission.Action... actions) throws Throwable {
+    grant(connection, namespace, userName, true, actions);
+  }
+
+  /**
+   * Grants permission on the specified namespace for the specified user.
+   * @param connection
+   * @param userName
+   * @param mergeExistingPermissions If set to false, later granted 
permissions will override
+   *          previous granted permissions. otherwise, it'll merge with 
previous granted
+   *          permissions.
+   * @param actions
+   * @throws Throwable
+   */
+  private static void grant(Connection connection, final String userName,
+      boolean mergeExistingPermissions, final Permission.Action... actions) 
throws Throwable {
+    // TODO: Pass an rpcController
     try (Table table = connection.getTable(ACL_TABLE_NAME)) {
-      AccessControlUtil.grant(null, getAccessControlServiceStub(table), 
userName, actions);
+      AccessControlUtil.grant(null, getAccessControlServiceStub(table), 
userName,
+              mergeExistingPermissions, actions);
     }
   }
 
+  /**
+   * Grant global permissions for the specified user.
+   * If permissions for the specified user exists, later granted permissions 
will override previous granted
+   * permissions.
+   * @param connection
+   * @param userName
+   * @param actions
+   * @throws Throwable
+   */
+  public static void grant(Connection connection, final String userName,
+      final Permission.Action... actions) throws Throwable {
+    grant(connection, userName, true, actions);
+  }
+
   public static boolean isAccessControllerRunning(Connection connection)
       throws MasterNotRunningException, ZooKeeperConnectionException, 
IOException {
     try (Admin admin = connection.getAdmin()) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
----------------------------------------------------------------------
diff --git 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
index 11c68b3..1d26366 100644
--- 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
+++ 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlUtil.java
@@ -53,7 +53,7 @@ public class AccessControlUtil {
    */
   public static AccessControlProtos.GrantRequest buildGrantRequest(
       String username, TableName tableName, byte[] family, byte[] qualifier,
-      AccessControlProtos.Permission.Action... actions) {
+      boolean mergeExistingPermissions, 
AccessControlProtos.Permission.Action... actions) {
     AccessControlProtos.Permission.Builder ret =
         AccessControlProtos.Permission.newBuilder();
     AccessControlProtos.TablePermission.Builder permissionBuilder =
@@ -79,7 +79,7 @@ public class AccessControlUtil {
           AccessControlProtos.UserPermission.newBuilder()
               .setUser(ByteString.copyFromUtf8(username))
               .setPermission(ret)
-      ).build();
+      ).setMergeExistingPermissions(mergeExistingPermissions).build();
   }
 
   /**
@@ -91,7 +91,7 @@ public class AccessControlUtil {
    * @return A {@link AccessControlProtos} GrantRequest
    */
   public static AccessControlProtos.GrantRequest buildGrantRequest(
-      String username, String namespace,
+      String username, String namespace, boolean mergeExistingPermissions,
       AccessControlProtos.Permission.Action... actions) {
     AccessControlProtos.Permission.Builder ret =
         AccessControlProtos.Permission.newBuilder();
@@ -110,7 +110,7 @@ public class AccessControlUtil {
           AccessControlProtos.UserPermission.newBuilder()
               .setUser(ByteString.copyFromUtf8(username))
               .setPermission(ret)
-      ).build();
+      ).setMergeExistingPermissions(mergeExistingPermissions).build();
   }
 
   /**
@@ -177,8 +177,8 @@ public class AccessControlUtil {
    * @param actions the permissions to be granted
    * @return A {@link AccessControlProtos} GrantRequest
    */
-  public static AccessControlProtos.GrantRequest buildGrantRequest(
-      String username, AccessControlProtos.Permission.Action... actions) {
+  public static AccessControlProtos.GrantRequest buildGrantRequest(String 
username,
+      boolean mergeExistingPermissions, 
AccessControlProtos.Permission.Action... actions) {
     AccessControlProtos.Permission.Builder ret =
         AccessControlProtos.Permission.newBuilder();
     AccessControlProtos.GlobalPermission.Builder permissionBuilder =
@@ -193,7 +193,7 @@ public class AccessControlUtil {
           AccessControlProtos.UserPermission.newBuilder()
               .setUser(ByteString.copyFromUtf8(username))
               .setPermission(ret)
-      ).build();
+      ).setMergeExistingPermissions(mergeExistingPermissions).build();
   }
 
   public static AccessControlProtos.UsersAndPermissions 
toUsersAndPermissions(String user,
@@ -490,14 +490,14 @@ public class AccessControlUtil {
    * @throws ServiceException
    */
   public static void grant(RpcController controller,
-      AccessControlService.BlockingInterface protocol, String userShortName,
+      AccessControlService.BlockingInterface protocol, String userShortName, 
boolean mergeExistingPermissions,
       Permission.Action... actions) throws ServiceException {
     List<AccessControlProtos.Permission.Action> permActions =
         Lists.newArrayListWithCapacity(actions.length);
     for (Permission.Action a : actions) {
       permActions.add(toPermissionAction(a));
     }
-    AccessControlProtos.GrantRequest request = buildGrantRequest(userShortName,
+    AccessControlProtos.GrantRequest request = 
buildGrantRequest(userShortName, mergeExistingPermissions,
         permActions.toArray(new 
AccessControlProtos.Permission.Action[actions.length]));
     protocol.grant(controller, request);
   }
@@ -518,14 +518,16 @@ public class AccessControlUtil {
    */
   public static void grant(RpcController controller,
       AccessControlService.BlockingInterface protocol, String userShortName, 
TableName tableName,
-      byte[] f, byte[] q, Permission.Action... actions) throws 
ServiceException {
+      byte[] f, byte[] q, boolean mergeExistingPermissions, 
Permission.Action... actions)
+      throws ServiceException {
     List<AccessControlProtos.Permission.Action> permActions =
         Lists.newArrayListWithCapacity(actions.length);
     for (Permission.Action a : actions) {
       permActions.add(toPermissionAction(a));
     }
-    AccessControlProtos.GrantRequest request = 
buildGrantRequest(userShortName, tableName, f, q,
-        permActions.toArray(new 
AccessControlProtos.Permission.Action[actions.length]));
+    AccessControlProtos.GrantRequest request =
+        buildGrantRequest(userShortName, tableName, f, q, 
mergeExistingPermissions,
+          permActions.toArray(new 
AccessControlProtos.Permission.Action[actions.length]));
     protocol.grant(controller, request);
   }
 
@@ -541,13 +543,13 @@ public class AccessControlUtil {
    */
   public static void grant(RpcController controller,
       AccessControlService.BlockingInterface protocol, String userShortName, 
String namespace,
-      Permission.Action... actions) throws ServiceException {
+      boolean mergeExistingPermissions, Permission.Action... actions) throws 
ServiceException {
     List<AccessControlProtos.Permission.Action> permActions =
         Lists.newArrayListWithCapacity(actions.length);
     for (Permission.Action a : actions) {
       permActions.add(toPermissionAction(a));
     }
-    AccessControlProtos.GrantRequest request = 
buildGrantRequest(userShortName, namespace,
+    AccessControlProtos.GrantRequest request = 
buildGrantRequest(userShortName, namespace, mergeExistingPermissions,
         permActions.toArray(new 
AccessControlProtos.Permission.Action[actions.length]));
     protocol.grant(controller, request);
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java
----------------------------------------------------------------------
diff --git 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java
 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java
index e9ecea4..4804b30 100644
--- 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java
+++ 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java
@@ -305,6 +305,21 @@ public class TablePermission extends Permission {
     return super.implies(action);
   }
 
+  public boolean tableFieldsEqual(TablePermission other){
+    if (!(((table == null && other.getTableName() == null) ||
+           (table != null && table.equals(other.getTableName()))) &&
+         ((family == null && other.getFamily() == null) ||
+           Bytes.equals(family, other.getFamily())) &&
+         ((qualifier == null && other.getQualifier() == null) ||
+          Bytes.equals(qualifier, other.getQualifier())) &&
+         ((namespace == null && other.getNamespace() == null) ||
+          (namespace != null && namespace.equals(other.getNamespace())))
+    )) {
+      return false;
+    }
+    return true;
+  }
+
   @Override
   
@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="NP_NULL_ON_SOME_PATH",
     justification="Passed on construction except on constructor not to be 
used")
@@ -314,15 +329,7 @@ public class TablePermission extends Permission {
     }
     TablePermission other = (TablePermission)obj;
 
-    if (!(((table == null && other.getTableName() == null) ||
-           (table != null && table.equals(other.getTableName()))) &&
-        ((family == null && other.getFamily() == null) ||
-         Bytes.equals(family, other.getFamily())) &&
-        ((qualifier == null && other.getQualifier() == null) ||
-         Bytes.equals(qualifier, other.getQualifier())) &&
-        ((namespace == null && other.getNamespace() == null) ||
-         (namespace != null && namespace.equals(other.getNamespace())))
-       )) {
+    if(!this.tableFieldsEqual(other)){
       return false;
     }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AccessControlProtos.java
----------------------------------------------------------------------
diff --git 
a/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AccessControlProtos.java
 
b/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AccessControlProtos.java
index cd1dda1..b72e6e5 100644
--- 
a/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AccessControlProtos.java
+++ 
b/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AccessControlProtos.java
@@ -5569,6 +5569,16 @@ public final class AccessControlProtos {
      * <code>required .hbase.pb.UserPermission user_permission = 1;</code>
      */
     
org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.UserPermissionOrBuilder
 getUserPermissionOrBuilder();
+
+    // optional bool merge_existing_permissions = 2 [default = false];
+    /**
+     * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+     */
+    boolean hasMergeExistingPermissions();
+    /**
+     * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+     */
+    boolean getMergeExistingPermissions();
   }
   /**
    * Protobuf type {@code hbase.pb.GrantRequest}
@@ -5634,6 +5644,11 @@ public final class AccessControlProtos {
               bitField0_ |= 0x00000001;
               break;
             }
+            case 16: {
+              bitField0_ |= 0x00000002;
+              mergeExistingPermissions_ = input.readBool();
+              break;
+            }
           }
         }
       } catch (com.google.protobuf.InvalidProtocolBufferException e) {
@@ -5696,8 +5711,25 @@ public final class AccessControlProtos {
       return userPermission_;
     }
 
+    // optional bool merge_existing_permissions = 2 [default = false];
+    public static final int MERGE_EXISTING_PERMISSIONS_FIELD_NUMBER = 2;
+    private boolean mergeExistingPermissions_;
+    /**
+     * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+     */
+    public boolean hasMergeExistingPermissions() {
+      return ((bitField0_ & 0x00000002) == 0x00000002);
+    }
+    /**
+     * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+     */
+    public boolean getMergeExistingPermissions() {
+      return mergeExistingPermissions_;
+    }
+
     private void initFields() {
       userPermission_ = 
org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.UserPermission.getDefaultInstance();
+      mergeExistingPermissions_ = false;
     }
     private byte memoizedIsInitialized = -1;
     public final boolean isInitialized() {
@@ -5722,6 +5754,9 @@ public final class AccessControlProtos {
       if (((bitField0_ & 0x00000001) == 0x00000001)) {
         output.writeMessage(1, userPermission_);
       }
+      if (((bitField0_ & 0x00000002) == 0x00000002)) {
+        output.writeBool(2, mergeExistingPermissions_);
+      }
       getUnknownFields().writeTo(output);
     }
 
@@ -5735,6 +5770,10 @@ public final class AccessControlProtos {
         size += com.google.protobuf.CodedOutputStream
           .computeMessageSize(1, userPermission_);
       }
+      if (((bitField0_ & 0x00000002) == 0x00000002)) {
+        size += com.google.protobuf.CodedOutputStream
+          .computeBoolSize(2, mergeExistingPermissions_);
+      }
       size += getUnknownFields().getSerializedSize();
       memoizedSerializedSize = size;
       return size;
@@ -5763,6 +5802,11 @@ public final class AccessControlProtos {
         result = result && getUserPermission()
             .equals(other.getUserPermission());
       }
+      result = result && (hasMergeExistingPermissions() == 
other.hasMergeExistingPermissions());
+      if (hasMergeExistingPermissions()) {
+        result = result && (getMergeExistingPermissions()
+            == other.getMergeExistingPermissions());
+      }
       result = result &&
           getUnknownFields().equals(other.getUnknownFields());
       return result;
@@ -5780,6 +5824,10 @@ public final class AccessControlProtos {
         hash = (37 * hash) + USER_PERMISSION_FIELD_NUMBER;
         hash = (53 * hash) + getUserPermission().hashCode();
       }
+      if (hasMergeExistingPermissions()) {
+        hash = (37 * hash) + MERGE_EXISTING_PERMISSIONS_FIELD_NUMBER;
+        hash = (53 * hash) + hashBoolean(getMergeExistingPermissions());
+      }
       hash = (29 * hash) + getUnknownFields().hashCode();
       memoizedHashCode = hash;
       return hash;
@@ -5896,6 +5944,8 @@ public final class AccessControlProtos {
           userPermissionBuilder_.clear();
         }
         bitField0_ = (bitField0_ & ~0x00000001);
+        mergeExistingPermissions_ = false;
+        bitField0_ = (bitField0_ & ~0x00000002);
         return this;
       }
 
@@ -5932,6 +5982,10 @@ public final class AccessControlProtos {
         } else {
           result.userPermission_ = userPermissionBuilder_.build();
         }
+        if (((from_bitField0_ & 0x00000002) == 0x00000002)) {
+          to_bitField0_ |= 0x00000002;
+        }
+        result.mergeExistingPermissions_ = mergeExistingPermissions_;
         result.bitField0_ = to_bitField0_;
         onBuilt();
         return result;
@@ -5951,6 +6005,9 @@ public final class AccessControlProtos {
         if (other.hasUserPermission()) {
           mergeUserPermission(other.getUserPermission());
         }
+        if (other.hasMergeExistingPermissions()) {
+          setMergeExistingPermissions(other.getMergeExistingPermissions());
+        }
         this.mergeUnknownFields(other.getUnknownFields());
         return this;
       }
@@ -6103,6 +6160,39 @@ public final class AccessControlProtos {
         return userPermissionBuilder_;
       }
 
+      // optional bool merge_existing_permissions = 2 [default = false];
+      private boolean mergeExistingPermissions_ ;
+      /**
+       * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+       */
+      public boolean hasMergeExistingPermissions() {
+        return ((bitField0_ & 0x00000002) == 0x00000002);
+      }
+      /**
+       * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+       */
+      public boolean getMergeExistingPermissions() {
+        return mergeExistingPermissions_;
+      }
+      /**
+       * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+       */
+      public Builder setMergeExistingPermissions(boolean value) {
+        bitField0_ |= 0x00000002;
+        mergeExistingPermissions_ = value;
+        onChanged();
+        return this;
+      }
+      /**
+       * <code>optional bool merge_existing_permissions = 2 [default = 
false];</code>
+       */
+      public Builder clearMergeExistingPermissions() {
+        bitField0_ = (bitField0_ & ~0x00000002);
+        mergeExistingPermissions_ = false;
+        onChanged();
+        return this;
+      }
+
       // @@protoc_insertion_point(builder_scope:hbase.pb.GrantRequest)
     }
 
@@ -10432,29 +10522,30 @@ public final class AccessControlProtos {
       "\0132-.hbase.pb.UsersAndPermissions.UserPer" +
       "missions\032J\n\017UserPermissions\022\014\n\004user\030\001 \002(" +
       "\014\022)\n\013permissions\030\002 \003(\0132\024.hbase.pb.Permis" +
-      "sion\"A\n\014GrantRequest\0221\n\017user_permission\030" +
-      "\001 \002(\0132\030.hbase.pb.UserPermission\"\017\n\rGrant" +
-      "Response\"B\n\rRevokeRequest\0221\n\017user_permis" +
-      "sion\030\001 \002(\0132\030.hbase.pb.UserPermission\"\020\n\016" +
-      "RevokeResponse\"\205\001\n\031GetUserPermissionsReq" +
-      "uest\022\'\n\004type\030\001 \001(\0162\031.hbase.pb.Permission" +
-      ".Type\022\'\n\ntable_name\030\002 \001(\0132\023.hbase.pb.Tab",
-      "leName\022\026\n\016namespace_name\030\003 \001(\014\"O\n\032GetUse" +
-      "rPermissionsResponse\0221\n\017user_permission\030" +
-      "\001 \003(\0132\030.hbase.pb.UserPermission\"C\n\027Check" +
-      "PermissionsRequest\022(\n\npermission\030\001 \003(\0132\024" +
-      ".hbase.pb.Permission\"\032\n\030CheckPermissions" +
-      "Response2\311\002\n\024AccessControlService\0228\n\005Gra" +
-      "nt\022\026.hbase.pb.GrantRequest\032\027.hbase.pb.Gr" +
-      "antResponse\022;\n\006Revoke\022\027.hbase.pb.RevokeR" +
-      "equest\032\030.hbase.pb.RevokeResponse\022_\n\022GetU" +
-      "serPermissions\022#.hbase.pb.GetUserPermiss",
-      "ionsRequest\032$.hbase.pb.GetUserPermission" +
-      "sResponse\022Y\n\020CheckPermissions\022!.hbase.pb" +
-      ".CheckPermissionsRequest\032\".hbase.pb.Chec" +
-      "kPermissionsResponseBI\n*org.apache.hadoo" +
-      "p.hbase.protobuf.generatedB\023AccessContro" +
-      "lProtosH\001\210\001\001\240\001\001"
+      "sion\"l\n\014GrantRequest\0221\n\017user_permission\030" +
+      "\001 \002(\0132\030.hbase.pb.UserPermission\022)\n\032merge" +
+      "_existing_permissions\030\002 \001(\010:\005false\"\017\n\rGr" +
+      "antResponse\"B\n\rRevokeRequest\0221\n\017user_per" +
+      "mission\030\001 \002(\0132\030.hbase.pb.UserPermission\"" +
+      "\020\n\016RevokeResponse\"\205\001\n\031GetUserPermissions" +
+      "Request\022\'\n\004type\030\001 \001(\0162\031.hbase.pb.Permiss",
+      "ion.Type\022\'\n\ntable_name\030\002 \001(\0132\023.hbase.pb." +
+      "TableName\022\026\n\016namespace_name\030\003 \001(\014\"O\n\032Get" +
+      "UserPermissionsResponse\0221\n\017user_permissi" +
+      "on\030\001 \003(\0132\030.hbase.pb.UserPermission\"C\n\027Ch" +
+      "eckPermissionsRequest\022(\n\npermission\030\001 \003(" +
+      "\0132\024.hbase.pb.Permission\"\032\n\030CheckPermissi" +
+      "onsResponse2\311\002\n\024AccessControlService\0228\n\005" +
+      "Grant\022\026.hbase.pb.GrantRequest\032\027.hbase.pb" +
+      ".GrantResponse\022;\n\006Revoke\022\027.hbase.pb.Revo" +
+      "keRequest\032\030.hbase.pb.RevokeResponse\022_\n\022G",
+      "etUserPermissions\022#.hbase.pb.GetUserPerm" +
+      "issionsRequest\032$.hbase.pb.GetUserPermiss" +
+      "ionsResponse\022Y\n\020CheckPermissions\022!.hbase" +
+      ".pb.CheckPermissionsRequest\032\".hbase.pb.C" +
+      "heckPermissionsResponseBI\n*org.apache.ha" +
+      "doop.hbase.protobuf.generatedB\023AccessCon" +
+      "trolProtosH\001\210\001\001\240\001\001"
     };
     com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner 
assigner =
       new 
com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner() {
@@ -10508,7 +10599,7 @@ public final class AccessControlProtos {
           internal_static_hbase_pb_GrantRequest_fieldAccessorTable = new
             com.google.protobuf.GeneratedMessage.FieldAccessorTable(
               internal_static_hbase_pb_GrantRequest_descriptor,
-              new java.lang.String[] { "UserPermission", });
+              new java.lang.String[] { "UserPermission", 
"MergeExistingPermissions", });
           internal_static_hbase_pb_GrantResponse_descriptor =
             getDescriptor().getMessageTypes().get(7);
           internal_static_hbase_pb_GrantResponse_fieldAccessorTable = new

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-protocol/src/main/protobuf/AccessControl.proto
----------------------------------------------------------------------
diff --git a/hbase-protocol/src/main/protobuf/AccessControl.proto 
b/hbase-protocol/src/main/protobuf/AccessControl.proto
index e67540b..cc0d4a5 100644
--- a/hbase-protocol/src/main/protobuf/AccessControl.proto
+++ b/hbase-protocol/src/main/protobuf/AccessControl.proto
@@ -79,6 +79,7 @@ message UsersAndPermissions {
 
 message GrantRequest {
   required UserPermission user_permission = 1;
+  optional bool merge_existing_permissions = 2 [default = false];
 }
 
 message GrantResponse {

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
index cbdcea9..f06330c 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
@@ -150,8 +150,8 @@ public class AccessControlLists {
    * @param t acl table instance. It is closed upon method return.
    * @throws IOException in the case of an error accessing the metadata table
    */
-  static void addUserPermission(Configuration conf, UserPermission userPerm, 
Table t)
-      throws IOException {
+  static void addUserPermission(Configuration conf, UserPermission userPerm, 
Table t,
+                                boolean mergeExistingPermissions) throws 
IOException {
     Permission.Action[] actions = userPerm.getActions();
     byte[] rowKey = userPermissionRowKey(userPerm);
     Put p = new Put(rowKey);
@@ -163,10 +163,34 @@ public class AccessControlLists {
       throw new IOException(msg);
     }
 
-    byte[] value = new byte[actions.length];
-    for (int i = 0; i < actions.length; i++) {
-      value[i] = actions[i].code();
+    Set<Permission.Action> actionSet = new TreeSet<Permission.Action>();
+    if(mergeExistingPermissions){
+      List<UserPermission> perms = getUserPermissions(conf, rowKey);
+      UserPermission currentPerm = null;
+      for (UserPermission perm : perms) {
+        if (Bytes.equals(perm.getUser(), userPerm.getUser())
+                && ((userPerm.isGlobal() && 
ACL_TABLE_NAME.equals(perm.getTableName()))
+                || perm.tableFieldsEqual(userPerm))) {
+          currentPerm = perm;
+          break;
+        }
+      }
+
+      if(currentPerm != null && currentPerm.getActions() != null){
+        actionSet.addAll(Arrays.asList(currentPerm.getActions()));
+      }
+    }
+
+    // merge current action with new action.
+    actionSet.addAll(Arrays.asList(actions));
+
+    // serialize to byte array.
+    byte[] value = new byte[actionSet.size()];
+    int index = 0;
+    for (Permission.Action action : actionSet) {
+      value[index++] = action.code();
     }
+
     p.addImmutable(ACL_LIST_FAMILY, key, value);
     if (LOG.isDebugEnabled()) {
       LOG.debug("Writing permission with rowKey "+
@@ -181,6 +205,11 @@ public class AccessControlLists {
     }
   }
 
+  static void addUserPermission(Configuration conf, UserPermission userPerm, 
Table t)
+          throws IOException{
+    addUserPermission(conf, userPerm, t, false);
+  }
+
   /**
    * Removes a previously granted permission from the stored access control
    * lists.  The {@link TablePermission} being removed must exactly match what

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index d89e053..ec41490 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -2246,7 +2246,7 @@ public class AccessController extends 
BaseMasterAndRegionObserver
           @Override
           public Void run() throws Exception {
             AccessControlLists.addUserPermission(regionEnv.getConfiguration(), 
perm,
-                regionEnv.getTable(AccessControlLists.ACL_TABLE_NAME));
+              regionEnv.getTable(AccessControlLists.ACL_TABLE_NAME), 
request.getMergeExistingPermissions());
             return null;
           }
         });

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
index 067e1f5..b0c68ba 100644
--- 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
+++ 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
@@ -371,7 +371,7 @@ public class SecureTestUtil {
             BlockingRpcChannel service = 
acl.coprocessorService(HConstants.EMPTY_START_ROW);
             AccessControlService.BlockingInterface protocol =
                 AccessControlService.newBlockingStub(service);
-            AccessControlUtil.grant(null, protocol, user, actions);
+            AccessControlUtil.grant(null, protocol, user, false, actions);
           }
         }
         return null;
@@ -417,7 +417,7 @@ public class SecureTestUtil {
             BlockingRpcChannel service = 
acl.coprocessorService(HConstants.EMPTY_START_ROW);
             AccessControlService.BlockingInterface protocol =
                 AccessControlService.newBlockingStub(service);
-            AccessControlUtil.grant(null, protocol, user, namespace, actions);
+            AccessControlUtil.grant(null, protocol, user, namespace, false, 
actions);
           }
         }
         return null;
@@ -439,7 +439,7 @@ public class SecureTestUtil {
         try {
           AccessControlClient.grant(connection, namespace, user, actions);
         } catch (Throwable t) {
-          t.printStackTrace();
+          LOG.error("grant failed: ", t);
         }
         return null;
       }
@@ -460,7 +460,7 @@ public class SecureTestUtil {
         try {
           AccessControlClient.revoke(connection, namespace, user, actions);
         } catch (Throwable t) {
-          t.printStackTrace();
+          LOG.error("revoke failed: ", t);
         }
         return null;
       }
@@ -506,7 +506,7 @@ public class SecureTestUtil {
             BlockingRpcChannel service = 
acl.coprocessorService(HConstants.EMPTY_START_ROW);
             AccessControlService.BlockingInterface protocol =
                 AccessControlService.newBlockingStub(service);
-            AccessControlUtil.grant(null, protocol, user, table, family, 
qualifier, actions);
+            AccessControlUtil.grant(null, protocol, user, table, family, 
qualifier, false, actions);
           }
         }
         return null;
@@ -528,7 +528,7 @@ public class SecureTestUtil {
         try {
           AccessControlClient.grant(connection, table, user, family, 
qualifier, actions);
         } catch (Throwable t) {
-          t.printStackTrace();
+          LOG.error("grant failed: ", t);
         }
         return null;
       }
@@ -549,7 +549,7 @@ public class SecureTestUtil {
         try {
           AccessControlClient.grant(connection, user, actions);
         } catch (Throwable t) {
-          t.printStackTrace();
+          LOG.error("grant failed: ", t);
         }
         return null;
       }
@@ -594,7 +594,7 @@ public class SecureTestUtil {
         try {
           AccessControlClient.revoke(connection, table, user, family, 
qualifier, actions);
         } catch (Throwable t) {
-          t.printStackTrace();
+          LOG.error("revoke failed: ", t);
         }
         return null;
       }
@@ -615,7 +615,7 @@ public class SecureTestUtil {
         try {
           AccessControlClient.revoke(connection, user, actions);
         } catch (Throwable t) {
-          t.printStackTrace();
+          LOG.error("revoke failed: ", t);
         }
         return null;
       }

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 29fe573..0377190 100644
--- 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -1191,7 +1191,7 @@ public class TestAccessController extends SecureTestUtil {
           AccessControlService.BlockingInterface protocol =
             AccessControlService.newBlockingStub(service);
           AccessControlUtil.grant(null, protocol, USER_RO.getShortName(), 
TEST_TABLE, TEST_FAMILY,
-              null, Action.READ);
+              null, false, Action.READ);
         }
         return null;
       }
@@ -2404,6 +2404,129 @@ public class TestAccessController extends 
SecureTestUtil {
     }
   }
 
+  @Test(timeout = 180000)
+  public void testAccessControlClientMultiGrantRevoke() throws Exception {
+    User testGrantRevoke =
+        User.createUserForTesting(conf, "testGrantRevoke", new String[0]);
+    AccessTestAction getAction = new AccessTestAction() {
+      @Override
+      public Object run() throws Exception {
+        try(Connection conn = ConnectionFactory.createConnection(conf);
+            Table t = conn.getTable(TEST_TABLE)) {
+          return t.get(new Get(TEST_ROW));
+        }
+      }
+    };
+
+    AccessTestAction putAction = new AccessTestAction() {
+      @Override
+      public Object run() throws Exception {
+        Put p = new Put(TEST_ROW);
+        p.addColumn(TEST_FAMILY, TEST_QUALIFIER, Bytes.toBytes(1));
+        try(Connection conn = ConnectionFactory.createConnection(conf);
+            Table t = conn.getTable(TEST_TABLE)) {
+          t.put(p);
+          return null;
+        }
+      }
+    };
+
+    verifyDenied(getAction, testGrantRevoke);
+    verifyDenied(putAction, testGrantRevoke);
+
+    // Grant global READ permissions to testGrantRevoke.
+    String userName = testGrantRevoke.getShortName();
+    try {
+      grantGlobalUsingAccessControlClient(TEST_UTIL, systemUserConnection, 
userName,
+        Permission.Action.READ);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.grant. ", e);
+    }
+    verifyAllowed(getAction, testGrantRevoke);
+    verifyDenied(putAction, testGrantRevoke);
+
+    // Grant global READ permissions to testGrantRevoke.
+    try {
+      grantGlobalUsingAccessControlClient(TEST_UTIL, systemUserConnection, 
userName,
+              Permission.Action.WRITE);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.grant. ", e);
+    }
+    verifyAllowed(getAction, testGrantRevoke);
+    verifyAllowed(putAction, testGrantRevoke);
+
+    // Revoke global READ permission to testGrantRevoke.
+    try {
+      revokeGlobalUsingAccessControlClient(TEST_UTIL, systemUserConnection, 
userName,
+              Permission.Action.READ, Permission.Action.WRITE);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.revoke ", e);
+    }
+    verifyDenied(getAction, testGrantRevoke);
+    verifyDenied(putAction, testGrantRevoke);
+
+    // Grant table READ & WRITE permissions to testGrantRevoke
+    try {
+      grantOnTableUsingAccessControlClient(TEST_UTIL, systemUserConnection, 
userName, TEST_TABLE,
+        null, null, Permission.Action.READ);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.grant. ", e);
+    }
+    verifyAllowed(getAction, testGrantRevoke);
+    verifyDenied(putAction, testGrantRevoke);
+
+    // Grant table WRITE permissions to testGrantRevoke
+    try {
+      grantOnTableUsingAccessControlClient(TEST_UTIL, systemUserConnection, 
userName, TEST_TABLE,
+        null, null, Action.WRITE);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.grant. ", e);
+    }
+    verifyAllowed(getAction, testGrantRevoke);
+    verifyAllowed(putAction, testGrantRevoke);
+
+    // Revoke table READ & WRITE permission to testGrantRevoke.
+    try {
+      revokeFromTableUsingAccessControlClient(TEST_UTIL, systemUserConnection, 
userName, TEST_TABLE, null, null,
+              Permission.Action.READ, Permission.Action.WRITE);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.revoke ", e);
+    }
+    verifyDenied(getAction, testGrantRevoke);
+    verifyDenied(putAction, testGrantRevoke);
+
+    // Grant Namespace READ permissions to testGrantRevoke
+    String namespace = TEST_TABLE.getNamespaceAsString();
+    try {
+      grantOnNamespaceUsingAccessControlClient(TEST_UTIL, 
systemUserConnection, userName,
+        namespace, Permission.Action.READ);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.grant. ", e);
+    }
+    verifyAllowed(getAction, testGrantRevoke);
+    verifyDenied(putAction, testGrantRevoke);
+
+    // Grant Namespace WRITE permissions to testGrantRevoke
+    try {
+      grantOnNamespaceUsingAccessControlClient(TEST_UTIL, 
systemUserConnection, userName,
+              namespace, Permission.Action.WRITE);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.grant. ", e);
+    }
+    verifyAllowed(getAction, testGrantRevoke);
+    verifyAllowed(putAction, testGrantRevoke);
+
+    // Revoke table READ & WRITE permission to testGrantRevoke.
+    try {
+      revokeFromNamespaceUsingAccessControlClient(TEST_UTIL, 
systemUserConnection, userName,
+              TEST_TABLE.getNamespaceAsString(), Permission.Action.READ, 
Permission.Action.WRITE);
+    } catch (Throwable e) {
+      LOG.error("error during call of AccessControlClient.revoke ", e);
+    }
+    verifyDenied(getAction, testGrantRevoke);
+    verifyDenied(putAction, testGrantRevoke);
+  }
+
   @Test (timeout=180000)
   public void testAccessControlClientGrantRevokeOnNamespace() throws Exception 
{
     // Create user for testing, who has no READ privileges by default.

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
index de98092..3c37d4b 100644
--- 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
+++ 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java
@@ -359,7 +359,7 @@ public class TestNamespaceCommands extends SecureTestUtil {
               acl.coprocessorService(HConstants.EMPTY_START_ROW);
           AccessControlService.BlockingInterface protocol =
             AccessControlService.newBlockingStub(service);
-          AccessControlUtil.grant(null, protocol, testUser, TEST_NAMESPACE, 
Action.WRITE);
+          AccessControlUtil.grant(null, protocol, testUser, TEST_NAMESPACE, 
false, Action.WRITE);
         } finally {
           acl.close();
           connection.close();
@@ -377,7 +377,7 @@ public class TestNamespaceCommands extends SecureTestUtil {
           AccessControlService.BlockingInterface protocol =
             AccessControlService.newBlockingStub(service);
           AccessControlUtil.grant(null, protocol, 
USER_GROUP_NS_ADMIN.getShortName(),
-            TEST_NAMESPACE, Action.READ);
+            TEST_NAMESPACE, false, Action.READ);
         }
         return null;
       }

http://git-wip-us.apache.org/repos/asf/hbase/blob/22fa1cd3/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java
----------------------------------------------------------------------
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 602d18f..4d2cb0b 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
@@ -458,7 +458,7 @@ public class TestTablePermissions {
     assertEquals("Should have 1 permission for user3", 1, user3Perms.size());
     assertEquals("user3 should have ADMIN, READ, CREATE permission",
                  new Permission.Action[] {
-                    Permission.Action.ADMIN, Permission.Action.READ, 
Permission.Action.CREATE
+                    Permission.Action.READ, Permission.Action.CREATE, 
Permission.Action.ADMIN
                  },
                  user3Perms.get(0).getActions());
   }

Reply via email to