You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ra...@apache.org on 2018/06/15 17:38:18 UTC
phoenix git commit: PHOENIX-4528 PhoenixAccessController checks
permissions only at table level when creating views(Karan Mehta)
Repository: phoenix
Updated Branches:
refs/heads/5.x-HBase-2.0 16fa7f661 -> 61affd431
PHOENIX-4528 PhoenixAccessController checks permissions only at table level when creating views(Karan Mehta)
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/61affd43
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/61affd43
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/61affd43
Branch: refs/heads/5.x-HBase-2.0
Commit: 61affd431b8c4a1730804f0c0d5a0035b797e178
Parents: 16fa7f6
Author: Rajeshbabu Chintaguntla <ra...@apache.org>
Authored: Fri Jun 15 10:38:05 2018 -0700
Committer: Rajeshbabu Chintaguntla <ra...@apache.org>
Committed: Fri Jun 15 10:38:05 2018 -0700
----------------------------------------------------------------------
.../phoenix/end2end/BasePermissionsIT.java | 4 +
.../phoenix/end2end/ChangePermissionsIT.java | 26 +++++-
.../coprocessor/PhoenixAccessController.java | 95 +++++++++++++-------
3 files changed, 92 insertions(+), 33 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/61affd43/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
index 9f91267..7698fca 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
@@ -748,6 +748,10 @@ public class BasePermissionsIT extends BaseTest {
}
}
+ String surroundWithDoubleQuotes(String input) {
+ return "\"" + input + "\"";
+ }
+
void validateAccessDeniedException(AccessDeniedException ade) {
String msg = ade.getMessage();
assertTrue("Exception contained unexpected message: '" + msg + "'",
http://git-wip-us.apache.org/repos/asf/phoenix/blob/61affd43/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java
index 0d764d8..106438f 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java
@@ -23,6 +23,7 @@ import org.apache.hadoop.hbase.security.User;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.query.QueryConstants;
import org.apache.phoenix.schema.TableNotFoundException;
+import org.apache.phoenix.util.SchemaUtil;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -144,7 +145,7 @@ public class ChangePermissionsIT extends BasePermissionsIT {
verifyAllowed(createSchema(SCHEMA_NAME), superUser1);
verifyAllowed(grantPermissions("C", regularUser1, SCHEMA_NAME, true), superUser1);
} else {
- verifyAllowed(grantPermissions("C", regularUser1, "\"" + QueryConstants.HBASE_DEFAULT_SCHEMA_NAME + "\"", true), superUser1);
+ verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME), true), superUser1);
}
// Create new table. Create indexes, views and view indexes on top of it. Verify the contents by querying it
@@ -235,7 +236,7 @@ public class ChangePermissionsIT extends BasePermissionsIT {
verifyAllowed(createSchema(SCHEMA_NAME), superUser1);
verifyAllowed(grantPermissions("C", regularUser1, SCHEMA_NAME, true), superUser1);
} else {
- verifyAllowed(grantPermissions("C", regularUser1, "\"" + QueryConstants.HBASE_DEFAULT_SCHEMA_NAME + "\"", true), superUser1);
+ verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME), true), superUser1);
}
// Create MultiTenant Table (View Index Table should be automatically created)
@@ -266,4 +267,25 @@ public class ChangePermissionsIT extends BasePermissionsIT {
verifyAllowed(readMultiTenantTableWithIndex(VIEW1_TABLE_NAME, "o1"), regularUser2);
verifyAllowed(readMultiTenantTableWithoutIndex(VIEW2_TABLE_NAME, "o2"), regularUser2);
}
+
+ /**
+ * Grant RX permissions on the schema to regularUser1,
+ * Creating view on a table with that schema by regularUser1 should be allowed
+ */
+ @Test
+ public void testCreateViewOnTableWithRXPermsOnSchema() throws Exception {
+
+ startNewMiniCluster();
+ grantSystemTableAccess(superUser1, regularUser1, regularUser2, regularUser3);
+
+ if(isNamespaceMapped) {
+ verifyAllowed(createSchema(SCHEMA_NAME), superUser1);
+ verifyAllowed(createTable(FULL_TABLE_NAME), superUser1);
+ verifyAllowed(grantPermissions("RX", regularUser1, SCHEMA_NAME, true), superUser1);
+ } else {
+ verifyAllowed(createTable(FULL_TABLE_NAME), superUser1);
+ verifyAllowed(grantPermissions("RX", regularUser1, surroundWithDoubleQuotes(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME), true), superUser1);
+ }
+ verifyAllowed(createView(VIEW1_TABLE_NAME, FULL_TABLE_NAME), regularUser1);
+ }
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/61affd43/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java
----------------------------------------------------------------------
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 b80dbe8..aa406fd 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
@@ -35,6 +35,7 @@ import org.apache.hadoop.hbase.CoprocessorEnvironment;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.NamespaceDescriptor;
import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ClusterConnection;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.client.ConnectionFactory;
@@ -46,7 +47,6 @@ import org.apache.hadoop.hbase.coprocessor.ObserverContext;
import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl;
import org.apache.hadoop.hbase.coprocessor.RegionCoprocessor;
import org.apache.hadoop.hbase.ipc.RpcServer;
-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.regionserver.RegionCoprocessorHost;
@@ -59,7 +59,11 @@ 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.UserPermission;
+import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
import org.apache.hadoop.hbase.util.Bytes;
+import com.google.protobuf.ByteString;
+import com.google.protobuf.RpcCallback;
+import com.google.protobuf.RpcController;
import org.apache.phoenix.coprocessor.PhoenixMetaDataCoprocessorHost.PhoenixMetaDataControllerEnvironment;
import org.apache.phoenix.query.QueryServices;
import org.apache.phoenix.query.QueryServicesOptions;
@@ -68,7 +72,6 @@ import org.apache.phoenix.schema.PTable;
import org.apache.phoenix.schema.PTableType;
import org.apache.phoenix.util.MetaDataUtil;
-import com.google.protobuf.RpcCallback;
public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
@@ -173,7 +176,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
User user = getActiveUser();
List<UserPermission> permissionForUser = getPermissionForUser(
- getUserPermissions(index.getNameAsString()), Bytes.toBytes(user.getShortName()));
+ getUserPermissions(index), Bytes.toBytes(user.getShortName()));
Set<Action> requireAccess = new HashSet<>();
Set<Action> accessExists = new HashSet<>();
if (permissionForUser != null) {
@@ -254,8 +257,8 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
@Override
public Void run() throws IOException {
try (Connection conn = ConnectionFactory.createConnection(env.getConfiguration())) {
- List<UserPermission> userPermissions = getUserPermissions(fromTable.getNameAsString());
- List<UserPermission> permissionsOnTheTable = getUserPermissions(toTable.getNameAsString());
+ List<UserPermission> userPermissions = getUserPermissions(fromTable);
+ List<UserPermission> permissionsOnTheTable = getUserPermissions(toTable);
if (userPermissions != null) {
for (UserPermission userPermission : userPermissions) {
Set<Action> requireAccess = new HashSet<Action>();
@@ -403,34 +406,26 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
}
}
- private List<UserPermission> getUserPermissions(final String tableName) throws IOException {
+ /**
+ * Gets all the permissions for a given tableName for all the users
+ * Also, get the permissions at table's namespace level and merge all of them
+ * @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 {
final List<UserPermission> userPermissions = new ArrayList<UserPermission>();
try (Connection connection = ConnectionFactory.createConnection(env.getConfiguration())) {
+ // Merge permissions from all accessController coprocessors loaded in memory
for (MasterObserver service : accessControllers) {
+ // Use AccessControlClient API's if the accessController is an instance of org.apache.hadoop.hbase.security.access.AccessController
if (service.getClass().getName().equals(org.apache.hadoop.hbase.security.access.AccessController.class.getName())) {
- userPermissions.addAll(AccessControlClient.getUserPermissions(connection, tableName));
+ userPermissions.addAll(AccessControlClient.getUserPermissions(connection, tableName.getNameAsString()));
+ userPermissions.addAll(AccessControlClient.getUserPermissions(
+ connection, AuthUtil.toGroupEntry(tableName.getNamespaceAsString())));
} else {
- AccessControlProtos.GetUserPermissionsRequest.Builder builder = AccessControlProtos.GetUserPermissionsRequest
- .newBuilder();
- builder.setTableName(ProtobufUtil.toProtoTableName(TableName.valueOf(tableName)));
- builder.setType(AccessControlProtos.Permission.Type.Table);
- AccessControlProtos.GetUserPermissionsRequest request = builder.build();
-
- ((AccessControlService.Interface)service).getUserPermissions(null, request,
- new RpcCallback<AccessControlProtos.GetUserPermissionsResponse>() {
- @Override
- public void run(AccessControlProtos.GetUserPermissionsResponse message) {
- if (message != null) {
- for (AccessControlProtos.UserPermission perm : message
- .getUserPermissionList()) {
- userPermissions.add(AccessControlUtil.toUserPermission(perm));
- }
- }
- }
- });
+ getUserPermsFromUserDefinedAccessController(userPermissions, connection, (AccessControlService.Interface) service);
}
}
} catch (Throwable e) {
@@ -443,12 +438,51 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
}
return userPermissions;
}
+
+ private void getUserPermsFromUserDefinedAccessController(final List<UserPermission> userPermissions, Connection connection, AccessControlService.Interface service) {
+
+ RpcController controller = (RpcController) ((ClusterConnection)connection)
+ .getRpcControllerFactory().newController();
+
+ AccessControlProtos.GetUserPermissionsRequest.Builder builderTablePerms = AccessControlProtos.GetUserPermissionsRequest
+ .newBuilder();
+ builderTablePerms.setTableName(ProtobufUtil.toProtoTableName(tableName));
+ builderTablePerms.setType(AccessControlProtos.Permission.Type.Table);
+ AccessControlProtos.GetUserPermissionsRequest requestTablePerms = builderTablePerms.build();
+
+ callGetUserPermissionsRequest(userPermissions, service, requestTablePerms, controller);
+
+ AccessControlProtos.GetUserPermissionsRequest.Builder builderNamespacePerms = AccessControlProtos.GetUserPermissionsRequest
+ .newBuilder();
+ builderNamespacePerms.setNamespaceName(ByteString.copyFrom(tableName.getNamespace()));
+ builderNamespacePerms.setType(AccessControlProtos.Permission.Type.Namespace);
+ AccessControlProtos.GetUserPermissionsRequest requestNamespacePerms = builderNamespacePerms.build();
+
+ callGetUserPermissionsRequest(userPermissions, service, requestNamespacePerms, controller);
+
+ }
+
+ private void callGetUserPermissionsRequest(final List<UserPermission> userPermissions, AccessControlService.Interface service
+ , AccessControlProtos.GetUserPermissionsRequest request, RpcController controller) {
+ ((AccessControlService.Interface)service).getUserPermissions(controller, request,
+ new RpcCallback<AccessControlProtos.GetUserPermissionsResponse>() {
+ @Override
+ public void run(AccessControlProtos.GetUserPermissionsResponse message) {
+ if (message != null) {
+ for (AccessControlProtos.UserPermission perm : message
+ .getUserPermissionList()) {
+ userPermissions.add(AccessControlUtil.toUserPermission(perm));
+ }
+ }
+ }
+ });
+ }
});
}
-
+
/**
* Authorizes that the current user has all the given permissions for the
- * given table
+ * given table and for the hbase namespace of the table
* @param tableName Table requested
* @throws IOException if obtaining the current user fails
* @throws AccessDeniedException if user has no authorization
@@ -458,7 +492,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
AuthResult result = null;
List<Action> requiredAccess = new ArrayList<Action>();
for (Action permission : permissions) {
- if (hasAccess(getUserPermissions(tableName.getNameAsString()), tableName, permission, user)) {
+ if (hasAccess(getUserPermissions(tableName), 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);
@@ -476,8 +510,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
/**
* Checks if the user has access to the table for the specified action.
- *
- * @param perms All table permissions
+ * @param perms All table and table's namespace permissions
* @param table tablename
* @param action action for access is required
* @return true if the user has access to the table for specified action, false otherwise
@@ -503,7 +536,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
}
}
} else if (LOG.isDebugEnabled()) {
- LOG.debug("No permissions found for table=" + table);
+ LOG.debug("No permissions found for table=" + table + " or namespace=" + table.getNamespaceAsString());
}
return false;
}