This is an automated email from the ASF dual-hosted git repository. tdsilva pushed a commit to branch 4.14-HBase-1.3 in repository https://gitbox.apache.org/repos/asf/phoenix.git
commit 138c495f2f6875bfca812ab975a2db4a63385e39 Author: Kiran Kumar Maturi <maturi.ki...@gmail.com> AuthorDate: Thu Jun 20 09:03:41 2019 +0530 PHOENIX-5269 use AccessChecker to check for user permisssions --- .../apache/phoenix/end2end/PermissionsCacheIT.java | 105 +++++++++++++++++++++ .../coprocessor/PhoenixAccessController.java | 91 ++++++++++++++++-- pom.xml | 2 +- 3 files changed, 190 insertions(+), 8 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/PermissionsCacheIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/PermissionsCacheIT.java new file mode 100644 index 0000000..c2f7ce2 --- /dev/null +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/PermissionsCacheIT.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.phoenix.end2end; + +import static org.junit.Assert.assertTrue; + +import java.security.PrivilegedExceptionAction; +import java.sql.Connection; +import java.util.Collections; +import java.util.List; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.AuthUtil; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.security.access.AccessControlLists; +import org.apache.hadoop.hbase.security.access.Permission.Action; +import org.apache.hadoop.hbase.security.access.TablePermission; +import org.apache.hadoop.hbase.zookeeper.ZKUtil; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; +import org.apache.phoenix.util.SchemaUtil; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.google.common.collect.ListMultimap; + +public class PermissionsCacheIT extends BasePermissionsIT { + + + public PermissionsCacheIT(boolean isNamespaceMapped) throws Exception { + super(isNamespaceMapped); + } + + @Test + public void testPermissionsCachedWithAccessChecker() throws Throwable { + if (!isNamespaceMapped) { + return; + } + startNewMiniCluster(); + final String schema = generateUniqueName(); + final String tableName = generateUniqueName(); + final String phoenixTableName = SchemaUtil.getTableName(schema, tableName); + try (Connection conn = getConnection()) { + grantPermissions(regularUser1.getShortName(), PHOENIX_NAMESPACE_MAPPED_SYSTEM_TABLES, + Action.READ, Action.EXEC); + grantPermissions(regularUser1.getShortName(), Collections.singleton("SYSTEM:SEQUENCE"), + Action.WRITE, Action.READ, Action.EXEC); + superUser1.runAs(new PrivilegedExceptionAction<Void>() { + @Override + public Void run() throws Exception { + try { + verifyAllowed(createSchema(schema), superUser1); + grantPermissions(regularUser1.getShortName(), schema, Action.CREATE); + grantPermissions(AuthUtil.toGroupEntry(GROUP_SYSTEM_ACCESS), schema, + Action.CREATE); + } catch (Throwable e) { + if (e instanceof Exception) { + throw (Exception) e; + } else { + throw new Exception(e); + } + } + return null; + } + }); + verifyAllowed(createTable(phoenixTableName), regularUser1); + HBaseTestingUtility utility = getUtility(); + Configuration conf = utility.getConfiguration(); + ZooKeeperWatcher zkw = HBaseTestingUtility.getZooKeeperWatcher(utility); + String aclZnodeParent = conf.get("zookeeper.znode.acl.parent", "acl"); + String aclZNode = ZKUtil.joinZNode(zkw.baseZNode, aclZnodeParent); + String tableZNode = ZKUtil.joinZNode(aclZNode, "@" + schema); + byte[] data = ZKUtil.getData(zkw, tableZNode); + ListMultimap<String, TablePermission> userPermissions = + AccessControlLists.readPermissions(data, conf); + assertTrue("User permissions not found in cache:", + userPermissions.containsKey(regularUser1.getName())); + List<TablePermission> tablePermissions = userPermissions.get(regularUser1.getName()); + for (TablePermission tablePerm : tablePermissions) { + assertTrue("Table create permission don't exist", tablePerm.implies(Action.CREATE)); + } + } catch (Exception e) { + System.out.println("Exception occurred: " + e); + throw e; + } + finally { + revokeAll(); + } + } + +} diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java index befa87d..8346601 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java @@ -26,11 +26,11 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import com.google.protobuf.ByteString; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.AuthUtil; +import org.apache.hadoop.hbase.CompoundConfiguration; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HColumnDescriptor; @@ -43,6 +43,8 @@ import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.coprocessor.BaseMasterAndRegionObserver; import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.ObserverContext; +import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.RegionServerCoprocessorEnvironment; import org.apache.hadoop.hbase.ipc.PayloadCarryingRpcController; import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; @@ -52,12 +54,15 @@ import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; +import org.apache.hadoop.hbase.security.access.AccessChecker; import org.apache.hadoop.hbase.security.access.AccessControlClient; import org.apache.hadoop.hbase.security.access.AuthResult; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.Permission.Action; +import org.apache.hadoop.hbase.security.access.TableAuthManager; import org.apache.hadoop.hbase.security.access.UserPermission; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.apache.phoenix.coprocessor.PhoenixMetaDataCoprocessorHost.PhoenixMetaDataControllerEnvironment; import org.apache.phoenix.query.QueryServices; import org.apache.phoenix.query.QueryServicesOptions; @@ -66,13 +71,16 @@ import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.PTableType; import org.apache.phoenix.util.MetaDataUtil; +import com.google.protobuf.ByteString; import com.google.protobuf.RpcCallback; public class PhoenixAccessController extends BaseMetaDataEndpointObserver { private PhoenixMetaDataControllerEnvironment env; private volatile ArrayList<BaseMasterAndRegionObserver> accessControllers; + private AccessChecker accessChecker; private boolean accessCheckEnabled; + private boolean hbaseAccessControllerEnabled; private UserProvider userProvider; public static final Log LOG = LogFactory.getLog(PhoenixAccessController.class); private static final Log AUDITLOG = @@ -89,6 +97,10 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { for (BaseMasterAndRegionObserver cp : coprocessors) { if (cp instanceof AccessControlService.Interface) { accessControllers.add(cp); + if (cp.getClass().getName().equals( + org.apache.hadoop.hbase.security.access.AccessController.class.getName())) { + hbaseAccessControllerEnabled = true; + } } } } @@ -118,6 +130,23 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { throw new IllegalArgumentException( "Not a valid environment, should be loaded by PhoenixMetaDataControllerEnvironment"); } + CompoundConfiguration compoundConf = new CompoundConfiguration(); + compoundConf.add(env.getConfiguration()); + ZooKeeperWatcher zk = null; + if (env instanceof MasterCoprocessorEnvironment) { + // if running on HMaster + MasterCoprocessorEnvironment mEnv = (MasterCoprocessorEnvironment) env; + zk = mEnv.getMasterServices().getZooKeeper(); + } else if (env instanceof RegionServerCoprocessorEnvironment) { + RegionServerCoprocessorEnvironment rsEnv = (RegionServerCoprocessorEnvironment) env; + zk = rsEnv.getRegionServerServices().getZooKeeper(); + } else if (env instanceof RegionCoprocessorEnvironment) { + // if running at region + RegionCoprocessorEnvironment regionEnv = (RegionCoprocessorEnvironment) env; + compoundConf.addStringMap(regionEnv.getRegion().getTableDesc().getConfiguration()); + zk = regionEnv.getRegionServerServices().getZooKeeper(); + } + accessChecker = new AccessChecker(env.getConfiguration(), zk); // set the user-provider. this.userProvider = UserProvider.instantiate(env.getConfiguration()); // init superusers and add the server principal (if using security) @@ -126,7 +155,11 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { } @Override - public void stop(CoprocessorEnvironment env) throws IOException {} + public void stop(CoprocessorEnvironment env) throws IOException { + if(accessChecker.getAuthManager() != null) { + TableAuthManager.release(accessChecker.getAuthManager()); + } + } @Override public void preCreateTable(ObserverContext<PhoenixMetaDataControllerEnvironment> ctx, String tenantId, @@ -400,9 +433,10 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { * @throws IOException */ private List<UserPermission> getUserPermissions(final TableName tableName) throws IOException { - return User.runAsLoginUser(new PrivilegedExceptionAction<List<UserPermission>>() { - @Override - public List<UserPermission> run() throws Exception { + List<UserPermission> userPermissions = + User.runAsLoginUser(new PrivilegedExceptionAction<List<UserPermission>>() { + @Override + public List<UserPermission> run() throws Exception { final List<UserPermission> userPermissions = new ArrayList<UserPermission>(); try (Connection connection = ConnectionFactory.createConnection(env.getConfiguration())) { // Merge permissions from all accessController coprocessors loaded in memory @@ -412,8 +446,39 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { userPermissions.addAll(AccessControlClient.getUserPermissions(connection, tableName.getNameAsString())); userPermissions.addAll(AccessControlClient.getUserPermissions( connection, AuthUtil.toGroupEntry(tableName.getNamespaceAsString()))); + } + } + } catch (Throwable e) { + if (e instanceof Exception) { + throw (Exception) e; + } else if (e instanceof Error) { + throw (Error) e; + } + throw new Exception(e); + } + return userPermissions; + } + }); + getUserDefinedPermissions(tableName, userPermissions); + return userPermissions; + } + + private void getUserDefinedPermissions(final TableName tableName, + final List<UserPermission> userPermissions) throws IOException { + User.runAsLoginUser(new PrivilegedExceptionAction<List<UserPermission>>() { + @Override + public List<UserPermission> run() throws Exception { + final List<UserPermission> userPermissions = new ArrayList<UserPermission>(); + try (Connection connection = + ConnectionFactory.createConnection(env.getConfiguration())) { + for (BaseMasterAndRegionObserver service : getAccessControllers()) { + if (service.getClass().getName().equals( + org.apache.hadoop.hbase.security.access.AccessController.class + .getName())) { + continue; } else { - getUserPermsFromUserDefinedAccessController(userPermissions, connection, (AccessControlService.Interface) service); + getUserPermsFromUserDefinedAccessController(userPermissions, connection, + (AccessControlService.Interface) service); } } } catch (Throwable e) { @@ -478,8 +543,12 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { User user = getActiveUser(); AuthResult result = null; List<Action> requiredAccess = new ArrayList<Action>(); + List<UserPermission> userPermissions = new ArrayList<>(); + if(permissions.length > 0) { + getUserDefinedPermissions(tableName, userPermissions); + } for (Action permission : permissions) { - if (hasAccess(getUserPermissions(tableName), tableName, permission, user)) { + if (hasAccess(userPermissions, tableName, permission, user)) { result = AuthResult.allow(request, "Table permission granted", user, permission, tableName, null, null); } else { result = AuthResult.deny(request, "Insufficient permissions", user, permission, tableName, null, null); @@ -507,6 +576,10 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { return true; } if (perms != null) { + if (hbaseAccessControllerEnabled + && accessChecker.getAuthManager().userHasAccess(user, table, action)) { + return true; + } List<UserPermission> permissionsForUser = getPermissionForUser(perms, user.getShortName().getBytes()); if (permissionsForUser != null) { for (UserPermission permissionForUser : permissionsForUser) { @@ -517,6 +590,10 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { if (groupNames != null) { for (String group : groupNames) { List<UserPermission> groupPerms = getPermissionForUser(perms,(AuthUtil.toGroupEntry(group)).getBytes()); + if (hbaseAccessControllerEnabled && accessChecker.getAuthManager() + .groupHasAccess(group, table, action)) { + return true; + } if (groupPerms != null) for (UserPermission permissionForUser : groupPerms) { if (permissionForUser.implies(action)) { return true; } } diff --git a/pom.xml b/pom.xml index 917d7d5..a0acf39 100644 --- a/pom.xml +++ b/pom.xml @@ -66,7 +66,7 @@ <top.dir>${project.basedir}</top.dir> <!-- Hadoop Versions --> - <hbase.version>1.3.1</hbase.version> + <hbase.version>1.3.5</hbase.version> <hadoop-two.version>2.7.1</hadoop-two.version> <!-- Dependency versions -->