Repository: hbase Updated Branches: refs/heads/branch-1 964e17f7b -> 45357c078
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/45357c07 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/45357c07 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/45357c07 Branch: refs/heads/branch-1 Commit: 45357c078d566ebd2f32594d73f7ad35feebe6dc Parents: 964e17f Author: huzheng <open...@gmail.com> Authored: Thu Feb 16 18:20:41 2017 +0800 Committer: zhangduo <zhang...@apache.org> Committed: Mon Feb 20 20:24:09 2017 +0800 ---------------------------------------------------------------------- .../hadoop/hbase/protobuf/ProtobufUtil.java | 25 ++-- .../hadoop/hbase/protobuf/RequestConverter.java | 28 ++-- .../security/access/AccessControlClient.java | 85 ++++++++++-- .../hbase/security/access/TablePermission.java | 25 ++-- .../protobuf/generated/AccessControlProtos.java | 139 +++++++++++++++---- .../src/main/protobuf/AccessControl.proto | 1 + .../security/access/AccessControlLists.java | 59 +++++--- .../hbase/security/access/AccessController.java | 5 +- .../hbase/security/access/SecureTestUtil.java | 70 ++++++++-- .../security/access/TestAccessController.java | 125 ++++++++++++++++- .../security/access/TestNamespaceCommands.java | 4 +- .../security/access/TestTablePermissions.java | 3 +- 12 files changed, 462 insertions(+), 107 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/45357c07/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java index fc8c030..bfdcfbd 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java @@ -2354,15 +2354,15 @@ public final class ProtobufUtil { */ public static void grant(RpcController controller, AccessControlService.BlockingInterface protocol, String userShortName, - 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(ProtobufUtil.toPermissionAction(a)); } - AccessControlProtos.GrantRequest request = RequestConverter. - buildGrantRequest(userShortName, permActions.toArray( - new AccessControlProtos.Permission.Action[actions.length])); + AccessControlProtos.GrantRequest request = + RequestConverter.buildGrantRequest(userShortName, mergeExistingPermissions, + permActions.toArray(new AccessControlProtos.Permission.Action[actions.length])); protocol.grant(controller, request); } @@ -2382,15 +2382,16 @@ public final class ProtobufUtil { */ 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(ProtobufUtil.toPermissionAction(a)); } - AccessControlProtos.GrantRequest request = RequestConverter. - buildGrantRequest(userShortName, tableName, f, q, permActions.toArray( - new AccessControlProtos.Permission.Action[actions.length])); + AccessControlProtos.GrantRequest request = + RequestConverter.buildGrantRequest(userShortName, tableName, f, q, mergeExistingPermissions, + permActions.toArray(new AccessControlProtos.Permission.Action[actions.length])); protocol.grant(controller, request); } @@ -2406,15 +2407,15 @@ public final class ProtobufUtil { */ 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(ProtobufUtil.toPermissionAction(a)); } - AccessControlProtos.GrantRequest request = RequestConverter. - buildGrantRequest(userShortName, namespace, permActions.toArray( - new AccessControlProtos.Permission.Action[actions.length])); + AccessControlProtos.GrantRequest request = + RequestConverter.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/45357c07/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java index 5e4a163..8163130 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java @@ -1542,8 +1542,8 @@ public final class RequestConverter { * @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 = @@ -1554,11 +1554,9 @@ public final class RequestConverter { ret.setType(AccessControlProtos.Permission.Type.Global) .setGlobalPermission(permissionBuilder); return AccessControlProtos.GrantRequest.newBuilder() - .setUserPermission( - AccessControlProtos.UserPermission.newBuilder() - .setUser(ByteString.copyFromUtf8(username)) - .setPermission(ret) - ).build(); + .setUserPermission(AccessControlProtos.UserPermission.newBuilder() + .setUser(ByteString.copyFromUtf8(username)).setPermission(ret)) + .setMergeExistingPermissions(mergeExistingPermissions).build(); } /** @@ -1572,7 +1570,7 @@ public final class RequestConverter { * @return A {@link AccessControlProtos} GrantRequest */ public static AccessControlProtos.GrantRequest buildGrantRequest( - String username, TableName tableName, byte[] family, byte[] qualifier, + String username, TableName tableName, byte[] family, byte[] qualifier, boolean mergeExistingPermissions, AccessControlProtos.Permission.Action... actions) { AccessControlProtos.Permission.Builder ret = AccessControlProtos.Permission.newBuilder(); @@ -1599,7 +1597,7 @@ public final class RequestConverter { AccessControlProtos.UserPermission.newBuilder() .setUser(ByteString.copyFromUtf8(username)) .setPermission(ret) - ).build(); + ).setMergeExistingPermissions(mergeExistingPermissions).build(); } /** @@ -1610,8 +1608,8 @@ public final class RequestConverter { * @param actions the permissions to be granted * @return A {@link AccessControlProtos} GrantRequest */ - public static AccessControlProtos.GrantRequest buildGrantRequest( - String username, String namespace, + public static AccessControlProtos.GrantRequest buildGrantRequest(String username, + String namespace, boolean mergeExistingPermissions, AccessControlProtos.Permission.Action... actions) { AccessControlProtos.Permission.Builder ret = AccessControlProtos.Permission.newBuilder(); @@ -1626,11 +1624,9 @@ public final class RequestConverter { ret.setType(AccessControlProtos.Permission.Type.Namespace) .setNamespacePermission(permissionBuilder); return AccessControlProtos.GrantRequest.newBuilder() - .setUserPermission( - AccessControlProtos.UserPermission.newBuilder() - .setUser(ByteString.copyFromUtf8(username)) - .setPermission(ret) - ).build(); + .setUserPermission(AccessControlProtos.UserPermission.newBuilder() + .setUser(ByteString.copyFromUtf8(username)).setPermission(ret)) + .setMergeExistingPermissions(mergeExistingPermissions).build(); } /** http://git-wip-us.apache.org/repos/asf/hbase/blob/45357c07/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 81dfae0..5799aaf 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 @@ -90,53 +90,110 @@ 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(final Connection connection, final TableName tableName, - final String userName, final byte[] family, final byte[] qual, + public 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 { - PayloadCarryingRpcController controller - = ((ClusterConnection) connection).getRpcControllerFactory().newController(); + PayloadCarryingRpcController controller = + ((ClusterConnection) connection).getRpcControllerFactory().newController(); controller.setPriority(tableName); try (Table table = connection.getTable(ACL_TABLE_NAME)) { ProtobufUtil.grant(controller, getAccessControlServiceStub(table), userName, tableName, - family, qual, actions); + family, qual, mergeExistingPermissions, actions); } } /** + * 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 + * @param tableName + * @param userName + * @param family + * @param qual + * @param actions + * @throws Throwable + */ + public static void grant(final 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, false, actions); + } + + /** * Grants permission on the specified namespace for the specified user. - * @param connection The Connection instance to use + * @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(final Connection connection, final String namespace, - final String userName, final Permission.Action... actions) throws Throwable { - PayloadCarryingRpcController controller - = ((ClusterConnection) connection).getRpcControllerFactory().newController(); + final String userName, boolean mergeExistingPermissions, final Permission.Action... actions) + throws Throwable { + PayloadCarryingRpcController controller = + ((ClusterConnection) connection).getRpcControllerFactory().newController(); try (Table table = connection.getTable(ACL_TABLE_NAME)) { ProtobufUtil.grant(controller, getAccessControlServiceStub(table), userName, namespace, - actions); + mergeExistingPermissions, actions); } } /** + * Grants permission on the specified namespace 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 namespace + * @param userName + * @param actions + * @throws Throwable + */ + public static void grant(final Connection connection, final String namespace, + final String userName, final Permission.Action... actions) throws Throwable { + grant(connection, namespace, userName, false, actions); + } + + /** * Grant global permissions for the specified user. + * @param connection The Connection instance to use + * @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(final Connection connection, final String userName, - final Permission.Action... actions) throws Throwable { - PayloadCarryingRpcController controller - = ((ClusterConnection) connection).getRpcControllerFactory().newController(); + boolean mergeExistingPermissions, final Permission.Action... actions) throws Throwable { + PayloadCarryingRpcController controller = + ((ClusterConnection) connection).getRpcControllerFactory().newController(); try (Table table = connection.getTable(ACL_TABLE_NAME)) { - ProtobufUtil.grant(controller, getAccessControlServiceStub(table), userName, actions); + ProtobufUtil.grant(controller, getAccessControlServiceStub(table), userName, + mergeExistingPermissions, actions); } } + /** + * Grant global permissions for the specified user. + * @param connection The Connection instance to use + * @param userName + * @param actions + * @throws Throwable + */ + public static void grant(final Connection connection, final String userName, + final Permission.Action... actions) throws Throwable { + grant(connection, userName, false, actions); + } + public static boolean isAccessControllerRunning(final Connection connection) throws MasterNotRunningException, ZooKeeperConnectionException, IOException { try (Admin admin = connection.getAdmin()) { http://git-wip-us.apache.org/repos/asf/hbase/blob/45357c07/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 55eeebb..4fb7548 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 @@ -311,6 +311,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; + } else { + 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") @@ -320,15 +335,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/45357c07/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/45357c07/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/45357c07/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 50d575e..ff75958 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 @@ -139,15 +139,8 @@ public class AccessControlLists { master.createSystemTable(ACL_TABLEDESC); } - /** - * Stores a new user permission grant in the access control lists table. - * @param conf the configuration - * @param userPerm the details of the permission to be granted - * @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); @@ -159,16 +152,38 @@ 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 "+ - Bytes.toString(rowKey)+" "+ - Bytes.toString(key)+": "+Bytes.toStringBinary(value) - ); + LOG.debug("Writing permission with rowKey " + Bytes.toString(rowKey) + " " + + Bytes.toString(key) + ": " + Bytes.toStringBinary(value)); } try { t.put(p); @@ -178,6 +193,18 @@ public class AccessControlLists { } /** + * Stores a new user permission grant in the access control lists table. + * @param conf the configuration + * @param userPerm the details of the permission to be granted + * @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 { + 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 * is stored -- no wildcard matching is attempted. Ie, if user "bob" has http://git-wip-us.apache.org/repos/asf/hbase/blob/45357c07/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 7be4540..dd15c3c 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 @@ -2237,7 +2237,7 @@ public class AccessController extends BaseMasterAndRegionObserver @Override public void grant(RpcController controller, - AccessControlProtos.GrantRequest request, + final AccessControlProtos.GrantRequest request, RpcCallback<AccessControlProtos.GrantResponse> done) { final UserPermission perm = ProtobufUtil.toUserPermission(request.getUserPermission()); AccessControlProtos.GrantResponse response = null; @@ -2266,7 +2266,8 @@ 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/45357c07/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 22a9748..e208fae 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 @@ -372,7 +372,7 @@ public class SecureTestUtil { BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW); AccessControlService.BlockingInterface protocol = AccessControlService.newBlockingStub(service); - ProtobufUtil.grant(null, protocol, user, actions); + ProtobufUtil.grant(null, protocol, user, false, actions); } } return null; @@ -418,7 +418,7 @@ public class SecureTestUtil { BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW); AccessControlService.BlockingInterface protocol = AccessControlService.newBlockingStub(service); - ProtobufUtil.grant(null, protocol, user, namespace, actions); + ProtobufUtil.grant(null, protocol, user, namespace, false, actions); } } return null; @@ -440,7 +440,23 @@ public class SecureTestUtil { try { AccessControlClient.grant(connection, namespace, user, actions); } catch (Throwable t) { - t.printStackTrace(); + LOG.error("grant failed: ", t); + } + return null; + } + }); + } + + public static void grantOnNamespaceUsingAccessControlClient(final HBaseTestingUtility util, + final Connection connection, final String user, final String namespace, + final boolean mergeExistingPermissions, final Permission.Action... actions) throws Exception { + SecureTestUtil.updateACLs(util, new Callable<Void>() { + @Override + public Void call() throws Exception { + try { + AccessControlClient.grant(connection, namespace, user, mergeExistingPermissions, actions); + } catch (Throwable t) { + LOG.error("grant failed: ", t); } return null; } @@ -461,7 +477,7 @@ public class SecureTestUtil { try { AccessControlClient.revoke(connection, namespace, user, actions); } catch (Throwable t) { - t.printStackTrace(); + LOG.error("revoke failed: ", t); } return null; } @@ -507,7 +523,7 @@ public class SecureTestUtil { BlockingRpcChannel service = acl.coprocessorService(HConstants.EMPTY_START_ROW); AccessControlService.BlockingInterface protocol = AccessControlService.newBlockingStub(service); - ProtobufUtil.grant(null, protocol, user, table, family, qualifier, actions); + ProtobufUtil.grant(null, protocol, user, table, family, qualifier, false, actions); } } return null; @@ -529,13 +545,33 @@ public class SecureTestUtil { try { AccessControlClient.grant(connection, table, user, family, qualifier, actions); } catch (Throwable t) { - t.printStackTrace(); + LOG.error("grant failed: ", t); + } + return null; + } + }); + } + + public static void grantOnTableUsingAccessControlClient(final HBaseTestingUtility util, + final Connection connection, final String user, final TableName table, final byte[] family, + final byte[] qualifier, final boolean mergeExistingPermissions, + final Permission.Action... actions) throws Exception { + SecureTestUtil.updateACLs(util, new Callable<Void>() { + @Override + public Void call() throws Exception { + try { + AccessControlClient.grant(connection, table, user, family, qualifier, + mergeExistingPermissions, actions); + } catch (Throwable t) { + LOG.error("grant failed: ", t); } return null; } }); } + + /** * Grant global permissions to the given user using AccessControlClient. Will wait until all * active AccessController instances have updated their permissions caches or will @@ -550,7 +586,23 @@ public class SecureTestUtil { try { AccessControlClient.grant(connection, user, actions); } catch (Throwable t) { - t.printStackTrace(); + LOG.error("grant failed: ", t); + } + return null; + } + }); + } + + public static void grantGlobalUsingAccessControlClient(final HBaseTestingUtility util, + final Connection connection, final String user, final boolean mergeExistingPermissions, + final Permission.Action... actions) throws Exception { + SecureTestUtil.updateACLs(util, new Callable<Void>() { + @Override + public Void call() throws Exception { + try { + AccessControlClient.grant(connection, user, mergeExistingPermissions, actions); + } catch (Throwable t) { + LOG.error("grant failed: ", t); } return null; } @@ -595,7 +647,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; } @@ -616,7 +668,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/45357c07/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 1c1ab39..99112d3 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 @@ -58,7 +58,6 @@ import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; -import org.apache.hadoop.hbase.Tag; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Append; import org.apache.hadoop.hbase.client.Connection; @@ -1192,7 +1191,7 @@ public class TestAccessController extends SecureTestUtil { BlockingRpcChannel service = acl.coprocessorService(TEST_TABLE.getName()); AccessControlService.BlockingInterface protocol = AccessControlService.newBlockingStub(service); - ProtobufUtil.grant(null, protocol, USER_RO.getShortName(), TEST_TABLE, TEST_FAMILY, null, + ProtobufUtil.grant(null, protocol, USER_RO.getShortName(), TEST_TABLE, TEST_FAMILY, null, false, Action.READ); } return null; @@ -2405,6 +2404,128 @@ public class TestAccessController extends SecureTestUtil { } } + @Test(timeout = 180000) + public void testAccessControlClientGrantRevokeWithMultiActions() 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, true, + 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, true, + 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, true, 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, true, 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, true, 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, true, 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/45357c07/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 377a3f6..a20ceeb 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 @@ -357,7 +357,7 @@ public class TestNamespaceCommands extends SecureTestUtil { acl.coprocessorService(HConstants.EMPTY_START_ROW); AccessControlService.BlockingInterface protocol = AccessControlService.newBlockingStub(service); - ProtobufUtil.grant(null, protocol, testUser, TEST_NAMESPACE, Action.WRITE); + ProtobufUtil.grant(null, protocol, testUser, TEST_NAMESPACE, false, Action.WRITE); } return null; } @@ -372,7 +372,7 @@ public class TestNamespaceCommands extends SecureTestUtil { AccessControlService.BlockingInterface protocol = AccessControlService.newBlockingStub(service); ProtobufUtil.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/45357c07/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 b6d15e7..ba586f5 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 @@ -45,6 +45,7 @@ import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.security.access.Permission.Action; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.client.HTable; import org.apache.hadoop.hbase.client.Put; @@ -449,7 +450,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()); }
