phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From karanmeht...@apache.org
Subject phoenix git commit: PHOENIX-4528 PhoenixAccessController checks permissions only at table level when creating views
Date Wed, 17 Jan 2018 19:42:46 GMT
Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-1.3 f408f65aa -> c42d90aa3


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/c42d90aa
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/c42d90aa
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/c42d90aa

Branch: refs/heads/4.x-HBase-1.3
Commit: c42d90aa34adbee65840916fd2b7215218eb1666
Parents: f408f65
Author: Karan Mehta <karanmehta93@gmail.com>
Authored: Sat Jan 13 17:19:22 2018 -0800
Committer: Karan Mehta <karanmehta93@gmail.com>
Committed: Wed Jan 17 11:34:48 2018 -0800

----------------------------------------------------------------------
 .../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/c42d90aa/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/c42d90aa/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/c42d90aa/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;
     }


Mime
View raw message