From e2508e7eebc6859d546391cced299e81ecf54941 Mon Sep 17 00:00:00 2001 From: meiyi Date: Thu, 28 Mar 2019 18:50:08 +0800 Subject: [PATCH] HBASE-22117 Move hasPermission/checkPermissions from region server to master --- .../java/org/apache/hadoop/hbase/client/Admin.java | 21 ++ .../org/apache/hadoop/hbase/client/AsyncAdmin.java | 19 ++ .../hadoop/hbase/client/AsyncHBaseAdmin.java | 7 + .../hbase/client/ConnectionImplementation.java | 8 + .../org/apache/hadoop/hbase/client/HBaseAdmin.java | 17 ++ .../hadoop/hbase/client/RawAsyncHBaseAdmin.java | 15 ++ .../hbase/client/ShortCircuitMasterConnection.java | 8 + .../hbase/security/access/AccessControlClient.java | 18 +- .../hbase/security/access/AccessControlUtil.java | 2 + .../security/access/ShadedAccessControlUtil.java | 13 ++ .../src/main/protobuf/AccessControl.proto | 9 + .../src/main/protobuf/Master.proto | 2 + .../hadoop/hbase/coprocessor/MasterObserver.java | 21 ++ .../hadoop/hbase/master/MasterCoprocessorHost.java | 21 ++ .../hadoop/hbase/master/MasterRpcServices.java | 44 ++++ .../hbase/security/access/AccessChecker.java | 164 ++++++++++++++ .../hbase/security/access/AccessController.java | 251 +++++++-------------- .../client/TestAsyncAccessControlAdminApi.java | 73 +++++- .../hbase/security/access/SecureTestUtil.java | 88 ++------ .../security/access/TestAccessController.java | 24 +- .../access/TestWithDisabledAuthorization.java | 67 +++--- .../hadoop/hbase/thrift2/client/ThriftAdmin.java | 6 + 22 files changed, 583 insertions(+), 315 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java index 33b44c3..f435905 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java @@ -54,6 +54,7 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException; import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException; @@ -2055,4 +2056,24 @@ public interface Admin extends Abortable, Closeable { */ List getUserPermissions(GetUserPermissionsRequest getUserPermissionsRequest) throws IOException; + + /** + * Check if the user has specific permissions + * @param userName the user name + * @param permissions the specific permission list + * @return True if user has the specific permissions + * @throws IOException if a remote or network exception occurs + */ + List hasUserPermissions(String userName, List permissions) + throws IOException; + + /** + * Check if call user has specific permissions + * @param permissions the specific permission list + * @return True if user has the specific permissions + * @throws IOException if a remote or network exception occurs + */ + default List hasUserPermissions(List permissions) throws IOException { + return hasUserPermissions(null, permissions); + } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java index 2f1f494..96ea677 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java @@ -47,6 +47,7 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.yetus.audience.InterfaceAudience; @@ -1457,4 +1458,22 @@ public interface AsyncAdmin { */ CompletableFuture> getUserPermissions(GetUserPermissionsRequest getUserPermissionsRequest); + + /** + * Check if the user has specific permissions + * @param userName the user name + * @param permissions the specific permission list + * @return True if user has the specific permissions + */ + CompletableFuture> hasUserPermissions(String userName, + List permissions); + + /** + * Check if call user has specific permissions + * @param permissions the specific permission list + * @return True if user has the specific permissions + */ + default CompletableFuture> hasUserPermissions(List permissions) { + return hasUserPermissions(null, permissions); + } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java index 668e729..1566110 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.hadoop.hbase.util.FutureUtils; import org.apache.yetus.audience.InterfaceAudience; @@ -814,4 +815,10 @@ class AsyncHBaseAdmin implements AsyncAdmin { getUserPermissions(GetUserPermissionsRequest getUserPermissionsRequest) { return wrap(rawAdmin.getUserPermissions(getUserPermissionsRequest)); } + + @Override + public CompletableFuture> hasUserPermissions(String userName, + List permissions) { + return wrap(rawAdmin.hasUserPermissions(userName, permissions)); + } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java index 6bcb499..f5b0b03 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java @@ -95,6 +95,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GetUserPermissionsRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GetUserPermissionsResponse; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.ClientService.BlockingInterface; @@ -1803,6 +1805,12 @@ class ConnectionImplementation implements ClusterConnection, Closeable { GetUserPermissionsRequest request) throws ServiceException { return stub.getUserPermissions(controller, request); } + + @Override + public HasUserPermissionsResponse hasUserPermissions(RpcController controller, + HasUserPermissionsRequest request) throws ServiceException { + return stub.hasUserPermissions(controller, request); + } }; } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java index 442fd81..20e87a3 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java @@ -86,6 +86,7 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.ShadedAccessControlUtil; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils; @@ -113,6 +114,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.AdminService; @@ -3859,6 +3861,21 @@ public class HBaseAdmin implements Admin { } @Override + public List hasUserPermissions(String userName, List permissions) + throws IOException { + return executeCallable( + new MasterCallable>(getConnection(), getRpcControllerFactory()) { + @Override + protected List rpcCall() throws Exception { + HasUserPermissionsRequest request = + ShadedAccessControlUtil.buildHasUserPermissionsRequest(userName, permissions); + return this.master.hasUserPermissions(getRpcController(), request) + .getHasUserPermissionList(); + } + }); + } + + @Override public void close() { } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java index ef48f5a..065892d 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java @@ -86,6 +86,7 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.ShadedAccessControlUtil; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils; @@ -111,6 +112,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GetUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantResponse; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.AdminService; @@ -3795,4 +3798,16 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { .collect(Collectors.toList()))) .call(); } + + @Override + public CompletableFuture> hasUserPermissions(String userName, + List permissions) { + return this.> newMasterCaller() + .action((controller, stub) -> this + .> call(controller, + stub, ShadedAccessControlUtil.buildHasUserPermissionsRequest(userName, permissions), + (s, c, req, done) -> s.hasUserPermissions(c, req, done), + resp -> resp.getHasUserPermissionList())) + .call(); + } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java index 6e80a54..8927615 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java @@ -25,6 +25,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.Get import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GetUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantResponse; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.CoprocessorServiceRequest; @@ -696,4 +698,10 @@ public class ShortCircuitMasterConnection implements MasterKeepAliveConnection { GetUserPermissionsRequest request) throws ServiceException { return stub.getUserPermissions(controller, request); } + + @Override + public HasUserPermissionsResponse hasUserPermissions(RpcController controller, + HasUserPermissionsRequest request) throws ServiceException { + return stub.hasUserPermissions(controller, request); + } } 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 046761e..01f3e49 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 @@ -392,19 +392,9 @@ public class AccessControlClient { if (StringUtils.isEmpty(tableName) || StringUtils.isEmpty(userName)) { throw new IllegalArgumentException("Table and user name can't be null or empty."); } - boolean hasPermission = false; - /** - * todo: pass an rpccontroller hbaserpccontroller controller = ((clusterconnection) - * connection).getrpccontrollerfactory().newcontroller(); - */ - try (Table table = connection.getTable(ACL_TABLE_NAME)) { - CoprocessorRpcChannel service = table.coprocessorService(HConstants.EMPTY_START_ROW); - BlockingInterface protocol = - AccessControlProtos.AccessControlService.newBlockingStub(service); - // Check whether user has permission - hasPermission = AccessControlUtil.hasPermission(null, protocol, TableName.valueOf(tableName), - columnFamily, columnQualifier, userName, actions); - } - return hasPermission; + List permissions = new ArrayList<>(1); + permissions.add(Permission.newBuilder(TableName.valueOf(tableName)).withFamily(columnFamily) + .withQualifier(columnQualifier).withActions(actions).build()); + return connection.getAdmin().hasUserPermissions(userName, permissions).get(0); } } 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 0220d89..484cee3 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 @@ -815,7 +815,9 @@ public class AccessControlUtil { * @param actions Actions * @return true if access allowed, otherwise false * @throws ServiceException + * @deprecated Use {@link Admin#hasUserPermissions(String, List)} instead. */ + @Deprecated public static boolean hasPermission(RpcController controller, AccessControlService.BlockingInterface protocol, TableName tableName, byte[] columnFamily, byte[] columnQualifier, String userName, Permission.Action[] actions) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java index be3b75e..661bcc8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/ShadedAccessControlUtil.java @@ -31,6 +31,7 @@ import org.apache.hbase.thirdparty.com.google.protobuf.ByteString; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GetUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.Permission.Type; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos; @@ -318,4 +319,16 @@ public class ShadedAccessControlUtil { } return builder.build(); } + + public static HasUserPermissionsRequest buildHasUserPermissionsRequest(String userName, + List permissions) { + HasUserPermissionsRequest.Builder builder = HasUserPermissionsRequest.newBuilder(); + if (userName != null && !userName.isEmpty()) { + builder.setUserName(ByteString.copyFromUtf8(userName)); + } + for (Permission permission : permissions) { + builder.addPermission(toPermission(permission)); + } + return builder.build(); + } } diff --git a/hbase-protocol-shaded/src/main/protobuf/AccessControl.proto b/hbase-protocol-shaded/src/main/protobuf/AccessControl.proto index af60fe2..3142a12 100644 --- a/hbase-protocol-shaded/src/main/protobuf/AccessControl.proto +++ b/hbase-protocol-shaded/src/main/protobuf/AccessControl.proto @@ -119,6 +119,15 @@ message CheckPermissionsRequest { message CheckPermissionsResponse { } +message HasUserPermissionsRequest { + optional bytes user_name = 1; + repeated Permission permission = 2; +} + +message HasUserPermissionsResponse { + repeated bool has_user_permission = 1; +} + service AccessControlService { rpc Grant(GrantRequest) returns (GrantResponse); diff --git a/hbase-protocol-shaded/src/main/protobuf/Master.proto b/hbase-protocol-shaded/src/main/protobuf/Master.proto index d883b4c..257defe 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Master.proto @@ -1033,6 +1033,8 @@ service MasterService { rpc Revoke(RevokeRequest) returns (RevokeResponse); rpc GetUserPermissions (GetUserPermissionsRequest) returns (GetUserPermissionsResponse); + + rpc HasUserPermissions (HasUserPermissionsRequest) returns (HasUserPermissionsResponse); } // HBCK Service definitions. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index bfb1ada..88b01a2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.quotas.GlobalQuotaSettings; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.SyncReplicationState; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceStability; @@ -1662,4 +1663,24 @@ public interface MasterObserver { String userName, String namespace, TableName tableName, byte[] family, byte[] qualifier) throws IOException { } + + /* + * Called before checking if user has permissions. + * @param ctx the coprocessor instance's environment + * @param userName the user name + * @param permissions the permission list + */ + default void preHasUserPermissions(ObserverContext ctx, + String userName, List permissions) throws IOException { + } + + /** + * Called after checking if user has permissions. + * @param ctx the coprocessor instance's environment + * @param userName the user name + * @param permissions the permission list + */ + default void postHasUserPermissions(ObserverContext ctx, + String userName, List permissions) throws IOException { + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index bb9fc3f..720f496 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -58,6 +58,7 @@ import org.apache.hadoop.hbase.quotas.GlobalQuotaSettings; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.SyncReplicationState; import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; @@ -1912,4 +1913,24 @@ public class MasterCoprocessorHost } }); } + + public void preHasUserPermissions(String userName, List permissions) + throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.preHasUserPermissions(this, userName, permissions); + } + }); + } + + public void postHasUserPermissions(String userName, List permissions) + throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.postHasUserPermissions(this, userName, permissions); + } + }); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index 08f89db..221ba37 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -94,6 +94,7 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.AccessChecker; +import org.apache.hadoop.hbase.security.access.AccessChecker.InputUser; import org.apache.hadoop.hbase.security.access.AccessControlLists; import org.apache.hadoop.hbase.security.access.AccessController; import org.apache.hadoop.hbase.security.access.Permission; @@ -125,6 +126,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.Get import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GetUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.GrantResponse; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsRequest; +import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.HasUserPermissionsResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.Permission.Type; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos.RevokeResponse; @@ -2628,6 +2631,47 @@ public class MasterRpcServices extends RSRpcServices } } + @Override + public HasUserPermissionsResponse hasUserPermissions(RpcController controller, + HasUserPermissionsRequest request) throws ServiceException { + try { + User caller = RpcServer.getRequestUser().orElse(null); + String userName = + request.hasUserName() ? request.getUserName().toStringUtf8() : caller.getShortName(); + List permissions = new ArrayList<>(); + for (int i = 0; i < request.getPermissionCount(); i++) { + permissions.add(ShadedAccessControlUtil.toPermission(request.getPermission(i))); + } + if (master.cpHost != null) { + master.getMasterCoprocessorHost().preHasUserPermissions(userName, permissions); + } + if (!caller.getShortName().equals(userName)) { + List groups = AccessChecker.getUserGroups(userName); + caller = new InputUser(userName, groups.toArray(new String[groups.size()])); + } + List hasUserPermissions = new ArrayList<>(); + if (accessChecker != null) { + for (Permission permission : permissions) { + boolean hasUserPermission = + accessChecker.hasUserPermission(caller, "hasUserPermissions", permission); + hasUserPermissions.add(hasUserPermission); + } + } else { + for (int i = 0; i < permissions.size(); i++) { + hasUserPermissions.add(true); + } + } + if (master.cpHost != null) { + master.getMasterCoprocessorHost().postHasUserPermissions(userName, permissions); + } + HasUserPermissionsResponse.Builder builder = + HasUserPermissionsResponse.newBuilder().addAllHasUserPermission(hasUserPermissions); + return builder.build(); + } catch (IOException ioe) { + throw new ServiceException(ioe); + } + } + private boolean containMetaWals(ServerName serverName) throws IOException { Path logDir = new Path(master.getWALRootDir(), AbstractFSWALProvider.getWALDirectoryName(serverName.toString())); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java index c4957f9..a035eb2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java @@ -26,9 +26,13 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.TreeMap; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.AuthUtil; +import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.TableName; @@ -39,12 +43,14 @@ import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.security.access.Permission.Action; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; import org.apache.hadoop.security.Groups; import org.apache.hadoop.security.HadoopKerberosName; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet; @InterfaceAudience.Private public final class AccessChecker { @@ -526,4 +532,162 @@ public final class AccessChecker { return new ArrayList<>(); } } + + /** + * Authorizes that if the current user has the given permissions. + * @param user Active user to which authorization checks should be applied + * @param request Request type + * @param permission Actions being requested + * @return True if the user has the specific permission + */ + public boolean hasUserPermission(User user, String request, Permission permission) { + if (!authorizationEnabled) { + return true; + } + if (permission instanceof TablePermission) { + TablePermission tPerm = (TablePermission) permission; + for (Permission.Action action : permission.getActions()) { + AuthResult authResult = permissionGranted(request, user, action, tPerm.getTableName(), + tPerm.getFamily(), tPerm.getQualifier()); + AccessChecker.logResult(authResult); + if (!authResult.isAllowed()) { + return false; + } + } + } else if (permission instanceof NamespacePermission) { + NamespacePermission nsPerm = (NamespacePermission) permission; + AuthResult authResult; + for (Action action : nsPerm.getActions()) { + if (getAuthManager().authorizeUserNamespace(user, nsPerm.getNamespace(), action)) { + authResult = + AuthResult.allow(request, "Namespace action allowed", user, action, null, null); + } else { + authResult = + AuthResult.deny(request, "Namespace action denied", user, action, null, null); + } + AccessChecker.logResult(authResult); + if (!authResult.isAllowed()) { + return false; + } + } + } else { + AuthResult authResult; + for (Permission.Action action : permission.getActions()) { + if (getAuthManager().authorizeUserGlobal(user, action)) { + authResult = AuthResult.allow(request, "Global action allowed", user, action, null, null); + } else { + authResult = AuthResult.deny(request, "Global action denied", user, action, null, null); + } + AccessChecker.logResult(authResult); + if (!authResult.isAllowed()) { + return false; + } + } + } + return true; + } + + private AuthResult permissionGranted(String request, User user, Action permRequest, + TableName tableName, byte[] family, byte[] qualifier) { + Map> map = makeFamilyMap(family, qualifier); + return permissionGranted(request, user, permRequest, tableName, map); + } + + /** + * Check the current user for authorization to perform a specific action against the given set of + * row data. + *

+ * Note: Ordering of the authorization checks has been carefully optimized to short-circuit the + * most common requests and minimize the amount of processing required. + *

+ * @param request User request + * @param user User name + * @param permRequest the action being requested + * @param tableName Table name + * @param families the map of column families to qualifiers present in the request + * @return an authorization result + */ + public AuthResult permissionGranted(String request, User user, Action permRequest, + TableName tableName, Map> families) { + if (!authorizationEnabled) { + return AuthResult.allow(request, "All users allowed because authorization is disabled", user, + permRequest, tableName, families); + } + // 1. All users need read access to hbase:meta table. + // this is a very common operation, so deal with it quickly. + if (TableName.META_TABLE_NAME.equals(tableName)) { + if (permRequest == Action.READ) { + return AuthResult.allow(request, "All users allowed", user, permRequest, tableName, + families); + } + } + + if (user == null) { + return AuthResult.deny(request, "No user associated with request!", null, permRequest, + tableName, families); + } + + // 2. check for the table-level, if successful we can short-circuit + if (getAuthManager().authorizeUserTable(user, tableName, permRequest)) { + return AuthResult.allow(request, "Table permission granted", user, permRequest, tableName, + families); + } + + // 3. check permissions against the requested families + if (families != null && families.size() > 0) { + // all families must pass + for (Map.Entry> family : families.entrySet()) { + // a) check for family level access + if (getAuthManager().authorizeUserTable(user, tableName, family.getKey(), permRequest)) { + continue; // family-level permission overrides per-qualifier + } + + // b) qualifier level access can still succeed + if ((family.getValue() != null) && (family.getValue().size() > 0)) { + if (family.getValue() instanceof Set) { + // for each qualifier of the family + Set familySet = (Set) family.getValue(); + for (byte[] qualifier : familySet) { + if (!getAuthManager().authorizeUserTable(user, tableName, family.getKey(), qualifier, + permRequest)) { + return AuthResult.deny(request, "Failed qualifier check", user, permRequest, + tableName, makeFamilyMap(family.getKey(), qualifier)); + } + } + } else if (family.getValue() instanceof List) { // List + List cellList = (List) family.getValue(); + for (Cell cell : cellList) { + if (!getAuthManager().authorizeUserTable(user, tableName, family.getKey(), + CellUtil.cloneQualifier(cell), permRequest)) { + return AuthResult.deny(request, "Failed qualifier check", user, permRequest, + tableName, makeFamilyMap(family.getKey(), CellUtil.cloneQualifier(cell))); + } + } + } + } else { + // no qualifiers and family-level check already failed + return AuthResult.deny(request, "Failed family check", user, permRequest, tableName, + makeFamilyMap(family.getKey(), null)); + } + } + + // all family checks passed + return AuthResult.allow(request, "All family checks passed", user, permRequest, tableName, + families); + } + + // 4. no families to check and table level access failed + return AuthResult.deny(request, "No families to check and table permission failed", user, + permRequest, tableName, families); + } + + private Map> makeFamilyMap(byte[] family, byte[] qualifier) { + if (family == null) { + return null; + } + + Map> familyMap = new TreeMap<>(Bytes.BYTES_COMPARATOR); + familyMap.put(family, qualifier != null ? ImmutableSet.of(qualifier) : null); + return familyMap; + } } 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 aaa2da5..a5ee794 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 @@ -141,7 +141,6 @@ import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; import org.apache.hbase.thirdparty.com.google.common.collect.MapMaker; import org.apache.hbase.thirdparty.com.google.common.collect.Maps; -import org.apache.hbase.thirdparty.com.google.common.collect.Sets; /** * Provides basic authorization checks for data access and administrative @@ -299,93 +298,6 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, } /** - * Check the current user for authorization to perform a specific action against the given set of - * row data. - *

- * Note: Ordering of the authorization checks has been carefully optimized to short-circuit the - * most common requests and minimize the amount of processing required. - *

- * @param request User request - * @param user User name - * @param permRequest the action being requested - * @param e the coprocessor environment - * @param tableName Table name - * @param families the map of column families to qualifiers present in the request - * @return an authorization result - */ - private AuthResult permissionGranted(String request, User user, Action permRequest, - RegionCoprocessorEnvironment e, TableName tableName, - Map> families) { - // 1. All users need read access to hbase:meta table. - // this is a very common operation, so deal with it quickly. - if (TableName.META_TABLE_NAME.equals(tableName)) { - if (permRequest == Action.READ) { - return AuthResult.allow(request, "All users allowed", user, permRequest, tableName, - families); - } - } - - if (user == null) { - return AuthResult.deny(request, "No user associated with request!", null, - permRequest, tableName, families); - } - - // 2. check for the table-level, if successful we can short-circuit - if (getAuthManager().authorizeUserTable(user, tableName, permRequest)) { - return AuthResult.allow(request, "Table permission granted", user, - permRequest, tableName, families); - } - - // 3. check permissions against the requested families - if (families != null && families.size() > 0) { - // all families must pass - for (Map.Entry> family : families.entrySet()) { - // a) check for family level access - if (getAuthManager().authorizeUserTable(user, tableName, family.getKey(), - permRequest)) { - continue; // family-level permission overrides per-qualifier - } - - // b) qualifier level access can still succeed - if ((family.getValue() != null) && (family.getValue().size() > 0)) { - if (family.getValue() instanceof Set) { - // for each qualifier of the family - Set familySet = (Set)family.getValue(); - for (byte[] qualifier : familySet) { - if (!getAuthManager().authorizeUserTable(user, tableName, - family.getKey(), qualifier, permRequest)) { - return AuthResult.deny(request, "Failed qualifier check", user, - permRequest, tableName, makeFamilyMap(family.getKey(), qualifier)); - } - } - } else if (family.getValue() instanceof List) { // List - List cellList = (List)family.getValue(); - for (Cell cell : cellList) { - if (!getAuthManager().authorizeUserTable(user, tableName, family.getKey(), - CellUtil.cloneQualifier(cell), permRequest)) { - return AuthResult.deny(request, "Failed qualifier check", user, permRequest, - tableName, makeFamilyMap(family.getKey(), CellUtil.cloneQualifier(cell))); - } - } - } - } else { - // no qualifiers and family-level check already failed - return AuthResult.deny(request, "Failed family check", user, permRequest, - tableName, makeFamilyMap(family.getKey(), null)); - } - } - - // all family checks passed - return AuthResult.allow(request, "All family checks passed", user, permRequest, - tableName, families); - } - - // 4. no families to check and table level access failed - return AuthResult.deny(request, "No families to check and table permission failed", - user, permRequest, tableName, families); - } - - /** * Check the current user for authorization to perform a specific action * against the given set of row data. * @param opType the operation type @@ -400,7 +312,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, Map> families, Action... actions) { AuthResult result = null; for (Action action: actions) { - result = permissionGranted(opType.toString(), user, action, e, + result = accessChecker.permissionGranted(opType.toString(), user, action, e.getRegion().getRegionInfo().getTable(), families); if (!result.isAllowed()) { return result; @@ -2186,76 +2098,40 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, done.run(response); } + /** + * @deprecated Use {@link Admin#hasUserPermissions(List)} instead. + */ + @Deprecated @Override public void checkPermissions(RpcController controller, - AccessControlProtos.CheckPermissionsRequest request, - RpcCallback done) { - Permission[] permissions = new Permission[request.getPermissionCount()]; - for (int i=0; i < request.getPermissionCount(); i++) { - permissions[i] = AccessControlUtil.toPermission(request.getPermission(i)); - } + AccessControlProtos.CheckPermissionsRequest request, + RpcCallback done) { AccessControlProtos.CheckPermissionsResponse response = null; try { User user = RpcServer.getRequestUser().orElse(null); TableName tableName = regionEnv.getRegion().getTableDescriptor().getTableName(); - for (Permission permission : permissions) { + List permissions = new ArrayList<>(); + for (int i = 0; i < request.getPermissionCount(); i++) { + Permission permission = AccessControlUtil.toPermission(request.getPermission(i)); + permissions.add(permission); if (permission instanceof TablePermission) { - // Check table permissions - TablePermission tperm = (TablePermission) permission; - for (Action action : permission.getActions()) { - if (!tperm.getTableName().equals(tableName)) { - throw new CoprocessorException(AccessController.class, String.format("This method " - + "can only execute at the table specified in TablePermission. " + - "Table of the region:%s , requested table:%s", tableName, - tperm.getTableName())); - } - - Map> familyMap = new TreeMap<>(Bytes.BYTES_COMPARATOR); - if (tperm.getFamily() != null) { - if (tperm.getQualifier() != null) { - Set qualifiers = Sets.newTreeSet(Bytes.BYTES_COMPARATOR); - qualifiers.add(tperm.getQualifier()); - familyMap.put(tperm.getFamily(), qualifiers); - } else { - familyMap.put(tperm.getFamily(), null); - } - } - - AuthResult result = permissionGranted("checkPermissions", user, action, regionEnv, - regionEnv.getRegion().getRegionInfo().getTable(), familyMap); - AccessChecker.logResult(result); - if (!result.isAllowed()) { - // Even if passive we need to throw an exception here, we support checking - // effective permissions, so throw unconditionally - throw new AccessDeniedException("Insufficient permissions (table=" + tableName + - (familyMap.size() > 0 ? ", family: " + result.toFamilyString() : "") + - ", action=" + action.toString() + ")"); - } - } - - } else { - // Check global permissions - - for (Action action : permission.getActions()) { - AuthResult result; - if (getAuthManager().authorizeUserGlobal(user, action)) { - result = AuthResult.allow("checkPermissions", "Global action allowed", user, - action, null, null); - } else { - result = AuthResult.deny("checkPermissions", "Global action denied", user, action, - null, null); - } - AccessChecker.logResult(result); - if (!result.isAllowed()) { - // Even if passive we need to throw an exception here, we support checking - // effective permissions, so throw unconditionally - throw new AccessDeniedException("Insufficient permissions (action=" + - action.toString() + ")"); - } + if (!tperm.getTableName().equals(tableName)) { + throw new CoprocessorException(AccessController.class, + String.format( + "This method can only execute at the table specified in " + + "TablePermission. Table of the region:%s , requested table:%s", + tableName, tperm.getTableName())); } } } + for (Permission permission : permissions) { + boolean hasPermission = + accessChecker.hasUserPermission(user, "checkPermissions", permission); + if (!hasPermission) { + throw new AccessDeniedException("Insufficient permissions " + permission.toString()); + } + } response = AccessControlProtos.CheckPermissionsResponse.getDefaultInstance(); } catch (IOException ioe) { CoprocessorRpcUtils.setControllerException(controller, ioe); @@ -2555,6 +2431,10 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, return userProvider.getCurrent(); } + /** + * @deprecated Use {@link Admin#hasUserPermissions(String, List)} instead. + */ + @Deprecated @Override public void hasPermission(RpcController controller, HasPermissionRequest request, RpcCallback done) { @@ -2568,33 +2448,10 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, AccessControlProtos.HasPermissionResponse response = null; try { User caller = RpcServer.getRequestUser().orElse(null); - // User instance for the input user name - User filterUser = accessChecker.validateCallerWithFilterUser(caller, tPerm, inputUserName); - - // Initialize family and qualifier map - Map> familyMap = new TreeMap>(Bytes.BYTES_COMPARATOR); - if (tPerm.getFamily() != null) { - if (tPerm.getQualifier() != null) { - Set qualifiers = Sets.newTreeSet(Bytes.BYTES_COMPARATOR); - qualifiers.add(tPerm.getQualifier()); - familyMap.put(tPerm.getFamily(), qualifiers); - } else { - familyMap.put(tPerm.getFamily(), null); - } - } - - // Iterate each action and check whether permission granted - boolean hasPermission = false; - for (Action action : tPerm.getActions()) { - AuthResult result = permissionGranted("hasPermission", filterUser, action, regionEnv, - tPerm.getTableName(), familyMap); - if (!result.isAllowed()) { - hasPermission = false; - // Break the loop is any action is not allowed - break; - } - hasPermission = true; - } + List permissions = Lists.newArrayList(tPerm); + preHasUserPermissions(caller, inputUserName, permissions); + boolean hasPermission = regionEnv.getConnection().getAdmin() + .hasUserPermissions(inputUserName, permissions).get(0); response = ResponseConverter.buildHasPermissionResponse(hasPermission); } catch (IOException ioe) { ResponseConverter.setControllerException(controller, ioe); @@ -2656,4 +2513,48 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, accessChecker.requirePermission(caller, "getUserPermissions", userName, Action.ADMIN); } } + + @Override + public void preHasUserPermissions(ObserverContext ctx, + String userName, List permissions) throws IOException { + preHasUserPermissions(getActiveUser(ctx), userName, permissions); + } + + private void preHasUserPermissions(User caller, String userName, List permissions) + throws IOException { + String request = "hasUserPermissions"; + for (Permission permission : permissions) { + if (!caller.getShortName().equals(userName)) { + // User should have admin privilege if checking permission for other users + if (permission instanceof TablePermission) { + TablePermission tPerm = (TablePermission) permission; + accessChecker.requirePermission(caller, request, tPerm.getTableName(), tPerm.getFamily(), + tPerm.getQualifier(), userName, Action.ADMIN); + } else if (permission instanceof NamespacePermission) { + NamespacePermission nsPerm = (NamespacePermission) permission; + accessChecker.requireNamespacePermission(caller, request, nsPerm.getNamespace(), userName, + Action.ADMIN); + } else { + accessChecker.requirePermission(caller, request, userName, Action.ADMIN); + } + } else { + // User don't need ADMIN privilege for self check. + // Setting action as null in AuthResult to display empty action in audit log + AuthResult result; + if (permission instanceof TablePermission) { + TablePermission tPerm = (TablePermission) permission; + result = AuthResult.allow(request, "Self user validation allowed", caller, null, + tPerm.getTableName(), tPerm.getFamily(), tPerm.getQualifier()); + } else if (permission instanceof NamespacePermission) { + NamespacePermission nsPerm = (NamespacePermission) permission; + result = AuthResult.allow(request, "Self user validation allowed", caller, null, + nsPerm.getNamespace()); + } else { + result = AuthResult.allow(request, "Self user validation allowed", caller, null, null, + null, null); + } + accessChecker.logResult(result); + } + } + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAccessControlAdminApi.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAccessControlAdminApi.java index 8075ae0..fbb08c6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAccessControlAdminApi.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAccessControlAdminApi.java @@ -12,14 +12,19 @@ package org.apache.hadoop.hbase.client; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.List; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.AccessControlLists; import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.SecureTestUtil; +import org.apache.hadoop.hbase.security.access.SecureTestUtil.AccessTestAction; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.SmallTests; @@ -29,6 +34,7 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.apache.hbase.thirdparty.com.google.common.collect.Lists; @RunWith(Parameterized.class) @Category({ ClientTests.class, SmallTests.class }) @@ -47,12 +53,16 @@ public class TestAsyncAccessControlAdminApi extends TestAsyncAdminBase { } @Test - public void testGrant() throws Exception { + public void test() throws Exception { TableName tableName = TableName.valueOf("test-table"); - String user = "test-user"; - UserPermission userPermission = new UserPermission(user, - Permission.newBuilder(tableName).withActions(Permission.Action.READ).build()); - // grant user table permission + String userName1 = "user1"; + String userName2 = "user2"; + User user2 = User.createUserForTesting(TEST_UTIL.getConfiguration(), userName2, new String[0]); + Permission permission = + Permission.newBuilder(tableName).withActions(Permission.Action.READ).build(); + UserPermission userPermission = new UserPermission(userName1, permission); + + // grant user1 table permission admin.grant(userPermission, false).get(); // get table permissions @@ -61,14 +71,57 @@ public class TestAsyncAccessControlAdminApi extends TestAsyncAdminBase { assertEquals(1, userPermissions.size()); assertEquals(userPermission, userPermissions.get(0)); - // get user table permissions - userPermissions = admin.getUserPermissions( - GetUserPermissionsRequest.newBuilder(tableName).withUserName(user).build()).get(); + // get table permissions + userPermissions = + admin + .getUserPermissions( + GetUserPermissionsRequest.newBuilder(tableName).withUserName(userName1).build()) + .get(); assertEquals(1, userPermissions.size()); assertEquals(userPermission, userPermissions.get(0)); - userPermissions = admin.getUserPermissions( - GetUserPermissionsRequest.newBuilder(tableName).withUserName("u").build()).get(); + userPermissions = + admin + .getUserPermissions( + GetUserPermissionsRequest.newBuilder(tableName).withUserName(userName2).build()) + .get(); assertEquals(0, userPermissions.size()); + + // has user permission + List permissions = Lists.newArrayList(permission); + boolean hasPermission = + admin.hasUserPermissions(userName1, permissions).get().get(0).booleanValue(); + assertTrue(hasPermission); + hasPermission = admin.hasUserPermissions(userName2, permissions).get().get(0).booleanValue(); + assertFalse(hasPermission); + + AccessTestAction hasPermissionAction = new AccessTestAction() { + @Override + public Object run() throws Exception { + try (AsyncConnection conn = + ConnectionFactory.createAsyncConnection(TEST_UTIL.getConfiguration()).get()) { + return conn.getAdmin().hasUserPermissions(userName1, permissions).get().get(0); + } + } + }; + try { + user2.runAs(hasPermissionAction); + fail("Should not come here"); + } catch (Exception e) { + LOG.error("Call has permission error", e); + } + + // check permission + admin.hasUserPermissions(permissions); + AccessTestAction checkPermissionsAction = new AccessTestAction() { + @Override + public Object run() throws Exception { + try (AsyncConnection conn = + ConnectionFactory.createAsyncConnection(TEST_UTIL.getConfiguration()).get()) { + return conn.getAdmin().hasUserPermissions(permissions).get().get(0); + } + } + }; + assertFalse((Boolean) user2.runAs(checkPermissionsAction)); } } 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 eecd773..341e7df 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.security.access; +import com.google.protobuf.ServiceException; import java.io.IOException; import java.lang.reflect.UndeclaredThrowableException; import java.security.PrivilegedActionException; @@ -28,9 +29,6 @@ import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; -import com.google.protobuf.BlockingRpcChannel; -import com.google.protobuf.ServiceException; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.HBaseTestingUtility; @@ -54,14 +52,9 @@ import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.MasterObserver; import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.io.hfile.HFile; -import org.apache.hadoop.hbase.protobuf.ProtobufUtil; -import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; -import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService; -import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.CheckPermissionsRequest; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; -import org.apache.hadoop.hbase.security.access.Permission.Action; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; import org.apache.hbase.thirdparty.com.google.common.collect.Maps; import org.apache.hadoop.hbase.util.JVMClusterUtil.RegionServerThread; @@ -140,36 +133,6 @@ public class SecureTestUtil { } } - public static void checkTablePerms(Configuration conf, TableName table, byte[] family, byte[] column, - Permission.Action... actions) throws IOException { - Permission[] perms = new Permission[actions.length]; - for (int i = 0; i < actions.length; i++) { - perms[i] = Permission.newBuilder(table).withFamily(family).withQualifier(column) - .withActions(actions[i]).build(); - } - - checkTablePerms(conf, table, perms); - } - - public static void checkTablePerms(Configuration conf, TableName table, Permission... perms) - throws IOException { - CheckPermissionsRequest.Builder request = CheckPermissionsRequest.newBuilder(); - for (Permission p : perms) { - request.addPermission(AccessControlUtil.toPermission(p)); - } - try (Connection connection = ConnectionFactory.createConnection(conf)) { - try (Table acl = connection.getTable(table)) { - AccessControlService.BlockingInterface protocol = - AccessControlService.newBlockingStub(acl.coprocessorService(new byte[0])); - try { - protocol.checkPermissions(null, request.build()); - } catch (ServiceException se) { - ProtobufUtil.toIOException(se); - } - } - } - } - /** * An AccessTestAction performs an action that will be examined to confirm * the results conform to expected access rights. @@ -854,25 +817,7 @@ public class SecureTestUtil { for (int i = 0; i < actions.length; i++) { perms[i] = new Permission(actions[i]); } - CheckPermissionsRequest.Builder request = CheckPermissionsRequest.newBuilder(); - for (Action a : actions) { - request.addPermission(AccessControlProtos.Permission.newBuilder() - .setType(AccessControlProtos.Permission.Type.Global) - .setGlobalPermission( - AccessControlProtos.GlobalPermission.newBuilder() - .addAction(AccessControlUtil.toPermissionAction(a)).build())); - } - try(Connection conn = ConnectionFactory.createConnection(testUtil.getConfiguration()); - Table acl = conn.getTable(AccessControlLists.ACL_TABLE_NAME)) { - BlockingRpcChannel channel = acl.coprocessorService(new byte[0]); - AccessControlService.BlockingInterface protocol = - AccessControlService.newBlockingStub(channel); - try { - protocol.checkPermissions(null, request.build()); - } catch (ServiceException se) { - ProtobufUtil.toIOException(se); - } - } + checkPermissions(testUtil.getConfiguration(), perms); } public static void checkTablePerms(HBaseTestingUtility testUtil, TableName table, byte[] family, @@ -882,26 +827,23 @@ public class SecureTestUtil { perms[i] = Permission.newBuilder(table).withFamily(family).withQualifier(column) .withActions(actions[i]).build(); } - checkTablePerms(testUtil, table, perms); + checkTablePerms(testUtil, perms); } - public static void checkTablePerms(HBaseTestingUtility testUtil, TableName table, - Permission... perms) throws IOException { - CheckPermissionsRequest.Builder request = CheckPermissionsRequest.newBuilder(); - for (Permission p : perms) { - request.addPermission(AccessControlUtil.toPermission(p)); - } + public static void checkTablePerms(HBaseTestingUtility testUtil, Permission... perms) + throws IOException { + checkPermissions(testUtil.getConfiguration(), perms); + } - try(Connection conn = ConnectionFactory.createConnection(testUtil.getConfiguration()); - Table acl = conn.getTable(table)) { - AccessControlService.BlockingInterface protocol = - AccessControlService.newBlockingStub(acl.coprocessorService(new byte[0])); - try { - protocol.checkPermissions(null, request.build()); - } catch (ServiceException se) { - ProtobufUtil.toIOException(se); + private static void checkPermissions(Configuration conf, Permission... perms) throws IOException { + try (Connection conn = ConnectionFactory.createConnection(conf)) { + List hasUserPermissions = + conn.getAdmin().hasUserPermissions(Lists.newArrayList(perms)); + for (int i = 0; i < hasUserPermissions.size(); i++) { + if (!hasUserPermissions.get(i).booleanValue()) { + throw new AccessDeniedException("Insufficient permissions " + perms[i]); + } } } } - } 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 563238c..a7ad4fd 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 @@ -1844,7 +1844,7 @@ public class TestAccessController extends SecureTestUtil { AccessTestAction multiQualifierRead = new AccessTestAction() { @Override public Void run() throws Exception { - checkTablePerms(TEST_UTIL, TEST_TABLE, + checkTablePerms(TEST_UTIL, new Permission[] { Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY).withQualifier(TEST_Q1) .withActions(Permission.Action.READ).build(), @@ -1857,8 +1857,7 @@ public class TestAccessController extends SecureTestUtil { AccessTestAction globalAndTableRead = new AccessTestAction() { @Override public Void run() throws Exception { - checkTablePerms(TEST_UTIL, TEST_TABLE, new Permission[] { - new Permission(Permission.Action.READ), + checkTablePerms(TEST_UTIL, new Permission[] { new Permission(Permission.Action.READ), Permission.newBuilder(TEST_TABLE).withActions(Permission.Action.READ).build() }); return null; } @@ -1867,7 +1866,7 @@ public class TestAccessController extends SecureTestUtil { AccessTestAction noCheck = new AccessTestAction() { @Override public Void run() throws Exception { - checkTablePerms(TEST_UTIL, TEST_TABLE, new Permission[0]); + checkTablePerms(TEST_UTIL, new Permission[0]); return null; } }; @@ -3334,7 +3333,7 @@ public class TestAccessController extends SecureTestUtil { TEST_FAMILY, TEST_QUALIFIER, Permission.Action.READ, Permission.Action.WRITE); // Verify action privilege - AccessTestAction hasPermissionAction = new AccessTestAction() { + AccessTestAction hasPermissionActionCP = new AccessTestAction() { @Override public Object run() throws Exception { try (Connection conn = ConnectionFactory.createConnection(conf); @@ -3349,6 +3348,21 @@ public class TestAccessController extends SecureTestUtil { return null; } }; + AccessTestAction hasPermissionAction = new AccessTestAction() { + @Override + public Object run() throws Exception { + try (Connection conn = ConnectionFactory.createConnection(conf)) { + Permission.Action[] actions = { Permission.Action.READ, Permission.Action.WRITE }; + conn.getAdmin().hasUserPermissions("dummy", + Arrays.asList(Permission.newBuilder(TEST_TABLE).withFamily(TEST_FAMILY) + .withQualifier(HConstants.EMPTY_BYTE_ARRAY).withActions(actions).build())); + } + return null; + } + }; + verifyAllowed(hasPermissionActionCP, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN, USER_OWNER, + USER_ADMIN_CF, user1); + verifyDenied(hasPermissionActionCP, USER_CREATE, USER_RW, USER_RO, USER_NONE, user2); verifyAllowed(hasPermissionAction, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN, USER_OWNER, USER_ADMIN_CF, user1); verifyDenied(hasPermissionAction, USER_CREATE, USER_RW, USER_RO, USER_NONE, user2); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java index bc59f70..8bc5718 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java @@ -250,9 +250,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkGlobalAdmin, SUPERUSER, USER_ADMIN); - verifyDenied(checkGlobalAdmin, USER_OWNER, USER_CREATE, USER_RW, USER_RO, USER_QUAL, - USER_NONE); + verifyAllowed(checkGlobalAdmin, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, + USER_RO, USER_QUAL, USER_NONE); AccessTestAction checkGlobalRead = new AccessTestAction() { @Override @@ -262,9 +261,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkGlobalRead, SUPERUSER, USER_ADMIN); - verifyDenied(checkGlobalRead, USER_OWNER, USER_CREATE, USER_RW, USER_RO, USER_QUAL, - USER_NONE); + verifyAllowed(checkGlobalRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, USER_RO, + USER_QUAL, USER_NONE); AccessTestAction checkGlobalReadWrite = new AccessTestAction() { @Override @@ -274,9 +272,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkGlobalReadWrite, SUPERUSER, USER_ADMIN); - verifyDenied(checkGlobalReadWrite, USER_OWNER, USER_CREATE, USER_RW, USER_RO, USER_QUAL, - USER_NONE); + verifyAllowed(checkGlobalReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, + USER_RO, USER_QUAL, USER_NONE); AccessTestAction checkTableAdmin = new AccessTestAction() { @Override @@ -287,8 +284,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkTableAdmin, SUPERUSER, USER_ADMIN, USER_OWNER); - verifyDenied(checkTableAdmin, USER_CREATE, USER_RW, USER_RO, USER_QUAL, USER_NONE); + verifyAllowed(checkTableAdmin, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, USER_RO, + USER_QUAL, USER_NONE); AccessTestAction checkTableCreate = new AccessTestAction() { @Override @@ -299,8 +296,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkTableCreate, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE); - verifyDenied(checkTableCreate, USER_RW, USER_RO, USER_QUAL, USER_NONE); + verifyAllowed(checkTableCreate, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, + USER_RO, USER_QUAL, USER_NONE); AccessTestAction checkTableRead = new AccessTestAction() { @Override @@ -311,8 +308,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkTableRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE); - verifyDenied(checkTableRead, USER_RW, USER_RO, USER_QUAL, USER_NONE); + verifyAllowed(checkTableRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, USER_RO, + USER_QUAL, USER_NONE); AccessTestAction checkTableReadWrite = new AccessTestAction() { @Override @@ -323,8 +320,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkTableReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE); - verifyDenied(checkTableReadWrite, USER_RW, USER_RO, USER_QUAL, USER_NONE); + verifyAllowed(checkTableReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, + USER_RO, USER_QUAL, USER_NONE); AccessTestAction checkColumnRead = new AccessTestAction() { @Override @@ -335,9 +332,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkColumnRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, - USER_RO); - verifyDenied(checkColumnRead, USER_QUAL, USER_NONE); + verifyAllowed(checkColumnRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, USER_RO, + USER_QUAL, USER_NONE); AccessTestAction checkColumnReadWrite = new AccessTestAction() { @Override @@ -348,9 +344,8 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkColumnReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, - USER_RW); - verifyDenied(checkColumnReadWrite, USER_RO, USER_QUAL, USER_NONE); + verifyAllowed(checkColumnReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, + USER_RO, USER_QUAL, USER_NONE); AccessTestAction checkQualifierRead = new AccessTestAction() { @Override @@ -362,8 +357,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { }; verifyAllowed(checkQualifierRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, - USER_RO, USER_QUAL); - verifyDenied(checkQualifierRead, USER_NONE); + USER_RO, USER_QUAL, USER_NONE); AccessTestAction checkQualifierReadWrite = new AccessTestAction() { @Override @@ -374,14 +368,13 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkQualifierReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, - USER_RW, USER_QUAL); - verifyDenied(checkQualifierReadWrite, USER_RO, USER_NONE); + verifyAllowed(checkQualifierReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, + USER_QUAL, USER_RO, USER_NONE); AccessTestAction checkMultiQualifierRead = new AccessTestAction() { @Override public Void run() throws Exception { - checkTablePerms(TEST_UTIL, TEST_TABLE.getTableName(), + checkTablePerms(TEST_UTIL, new Permission[] { Permission.newBuilder(TEST_TABLE.getTableName()).withFamily(TEST_FAMILY) .withQualifier(TEST_Q1).withActions(Action.READ).build(), @@ -391,14 +384,13 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyAllowed(checkMultiQualifierRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, - USER_RW, USER_RO); - verifyDenied(checkMultiQualifierRead, USER_QUAL, USER_NONE); + verifyAllowed(checkMultiQualifierRead, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, USER_RW, + USER_RO, USER_QUAL, USER_NONE); AccessTestAction checkMultiQualifierReadWrite = new AccessTestAction() { @Override public Void run() throws Exception { - checkTablePerms(TEST_UTIL, TEST_TABLE.getTableName(), + checkTablePerms(TEST_UTIL, new Permission[] { Permission.newBuilder(TEST_TABLE.getTableName()).withFamily(TEST_FAMILY) .withQualifier(TEST_Q1) @@ -411,8 +403,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { }; verifyAllowed(checkMultiQualifierReadWrite, SUPERUSER, USER_ADMIN, USER_OWNER, USER_CREATE, - USER_RW); - verifyDenied(checkMultiQualifierReadWrite, USER_RO, USER_QUAL, USER_NONE); + USER_RW, USER_RO, USER_QUAL, USER_NONE); } /** Test grants and revocations with authorization disabled */ @@ -424,7 +415,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { User tblUser = User.createUserForTesting(TEST_UTIL.getConfiguration(), "tbluser", new String[0]); - // If we check now, the test user won't have permissions + // If we check now, the test user have permissions because authorization is disabled AccessTestAction checkTableRead = new AccessTestAction() { @Override @@ -435,7 +426,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { } }; - verifyDenied(tblUser, checkTableRead); + verifyAllowed(tblUser, checkTableRead); // An actual read won't be denied @@ -469,7 +460,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { // Now the permission check will indicate revocation but the actual op will still succeed - verifyDenied(tblUser, checkTableRead); + verifyAllowed(tblUser, checkTableRead); verifyAllowed(tblUser, tableRead); } diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java index 7b96a28..dd64bb4 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java @@ -57,6 +57,7 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest; +import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException; import org.apache.hadoop.hbase.thrift2.ThriftUtilities; @@ -1111,4 +1112,9 @@ public class ThriftAdmin implements Admin { getUserPermissions(GetUserPermissionsRequest getUserPermissionsRequest) { throw new NotImplementedException("getUserPermissions not supported in ThriftAdmin"); } + + @Override + public List hasUserPermissions(String userName, List permissions) { + throw new NotImplementedException("hasUserPermissions not supported in ThriftAdmin"); + } } -- 2.7.4