You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by pb...@apache.org on 2018/02/01 00:06:58 UTC
[26/35] phoenix git commit: PHOENIX-4528 PhoenixAccessController
checks permissions only at table level when creating views
PHOENIX-4528 PhoenixAccessController checks permissions only at table level when creating views
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/6a85b11e
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/6a85b11e
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/6a85b11e
Branch: refs/heads/4.x-cdh5.11.2
Commit: 6a85b11edc90c37e0ffe053319fe6a86f8bb00d2
Parents: 319ff01
Author: Karan Mehta <ka...@gmail.com>
Authored: Sun Jan 14 01:19:22 2018 +0000
Committer: Pedro Boado <pb...@apache.org>
Committed: Wed Jan 31 22:24:48 2018 +0000
----------------------------------------------------------------------
.../phoenix/end2end/BasePermissionsIT.java | 4 +
.../phoenix/end2end/ChangePermissionsIT.java | 26 +++++-
.../coprocessor/PhoenixAccessController.java | 91 +++++++++++++-------
3 files changed, 88 insertions(+), 33 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/6a85b11e/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 9d7ef1b..d33d538 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
@@ -746,6 +746,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/6a85b11e/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 2bf7fe1..a30f01f 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
@@ -145,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, "\"" + SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE + "\"", true), superUser1);
+ verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE), true), superUser1);
}
// Create new table. Create indexes, views and view indexes on top of it. Verify the contents by querying it
@@ -236,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, "\"" + SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE + "\"", true), superUser1);
+ verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE), true), superUser1);
}
// Create MultiTenant Table (View Index Table should be automatically created)
@@ -267,4 +267,26 @@ 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(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE), true), superUser1);
+ }
+
+ verifyAllowed(createView(VIEW1_TABLE_NAME, FULL_TABLE_NAME), regularUser1);
+ }
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/6a85b11e/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 a4bc857..7b9452d 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
@@ -27,6 +27,7 @@ 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;
@@ -166,7 +167,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) {
@@ -247,8 +248,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>();
@@ -396,36 +397,27 @@ 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 (BaseMasterAndRegionObserver 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();
-
- PayloadCarryingRpcController controller = ((ClusterConnection)connection)
- .getRpcControllerFactory().newController();
- ((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(ProtobufUtil.toUserPermission(perm));
- }
- }
- }
- });
+ getUserPermsFromUserDefinedAccessController(userPermissions, connection, (AccessControlService.Interface) service);
}
}
} catch (Throwable e) {
@@ -438,12 +430,50 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
}
return userPermissions;
}
+
+ private void getUserPermsFromUserDefinedAccessController(final List<UserPermission> userPermissions, Connection connection, AccessControlService.Interface service) {
+
+ PayloadCarryingRpcController controller = ((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, PayloadCarryingRpcController controller) {
+ 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(ProtobufUtil.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
@@ -453,7 +483,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);
@@ -471,8 +501,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
@@ -498,7 +527,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;
}