[ 
https://issues.apache.org/jira/browse/PHOENIX-672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16263079#comment-16263079
 ] 

ASF GitHub Bot commented on PHOENIX-672:
----------------------------------------

Github user karanmehta93 commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/283#discussion_r152645916
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java ---
    @@ -4168,4 +4176,197 @@ public MutationState useSchema(UseSchemaStatement 
useSchemaStatement) throws SQL
             }
             return new MutationState(0, 0, connection);
         }
    +
    +    public MutationState grantPermission(GrantStatement grantStatement) 
throws SQLException {
    +
    +        StringBuffer grantPermLog = new StringBuffer();
    +        grantPermLog.append("Grant Permissions requested for user/group: " 
+ grantStatement.getName());
    +        if (grantStatement.getSchemaName() != null) {
    +            grantPermLog.append(" for Schema: " + 
grantStatement.getSchemaName());
    +        } else if (grantStatement.getTableName() != null) {
    +            grantPermLog.append(" for Table: " + 
grantStatement.getTableName());
    +        }
    +        grantPermLog.append(" Permissions: " + 
Arrays.toString(grantStatement.getPermsList()));
    +        logger.info(grantPermLog.toString());
    +
    +        HConnection hConnection = 
connection.getQueryServices().getAdmin().getConnection();
    +
    +        try {
    +            if (grantStatement.getSchemaName() != null) {
    +                // SYSTEM.CATALOG doesn't have any entry for "default" 
HBase namespace, hence we will bypass the check
    +                
if(!grantStatement.getSchemaName().equals(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME))
 {
    +                    
FromCompiler.getResolverForSchema(grantStatement.getSchemaName(), connection);
    +                }
    +                grantPermissionsToSchema(hConnection, grantStatement);
    +
    +            } else if (grantStatement.getTableName() != null) {
    +                PTable inputTable = PhoenixRuntime.getTable(connection,
    +                        
SchemaUtil.normalizeFullTableName(grantStatement.getTableName().toString()));
    +                if (!(PTableType.TABLE.equals(inputTable.getType()) || 
PTableType.SYSTEM.equals(inputTable.getType()))) {
    +                    throw new AccessDeniedException("Cannot GRANT 
permissions on INDEX TABLES or VIEWS");
    +                }
    +                grantPermissionsToTables(hConnection, grantStatement, 
inputTable);
    +
    +            } else {
    +                grantPermissionsToUser(hConnection, grantStatement);
    +            }
    +
    +        } catch (SQLException e) {
    +            // Bubble up the SQL Exception
    +            throw e;
    +        } catch (Throwable throwable) {
    +            // Wrap around other exceptions to PhoenixIOException (Ex: 
org.apache.hadoop.hbase.security.AccessDeniedException)
    +            throw ServerUtil.parseServerException(throwable);
    +        }
    +
    +        return new MutationState(0, 0, connection);
    +    }
    +
    +    private void grantPermissionsToTables(HConnection hConnection, 
GrantStatement grantStatement, PTable inputTable) throws Throwable {
    +
    +        org.apache.hadoop.hbase.TableName tableName = 
SchemaUtil.getPhysicalTableName
    +                (inputTable.getName().getBytes(), 
inputTable.isNamespaceMapped());
    +
    +        grantPermissionsToTable(hConnection, grantStatement, tableName);
    +
    +        for(PTable indexTable : inputTable.getIndexes()) {
    +            // Local Indexes don't correspond to new physical table, they 
are just stored in separate CF of base table.
    +            if(indexTable.getIndexType().equals(IndexType.LOCAL)) {
    +                continue;
    +            }
    +            logger.info("Granting " + 
Arrays.toString(grantStatement.getPermsList()) +
    +                    " perms to IndexTable: " + indexTable.getName() + " 
BaseTable: " + inputTable.getName());
    +            if (inputTable.isNamespaceMapped() != 
indexTable.isNamespaceMapped()) {
    +                throw new 
TablesNotInSyncException(inputTable.getTableName().getString(),
    +                        indexTable.getTableName().getString(), "Namespace 
properties");
    +            }
    +            tableName = 
SchemaUtil.getPhysicalTableName(indexTable.getName().getBytes(), 
indexTable.isNamespaceMapped());
    +            grantPermissionsToTable(hConnection, grantStatement, 
tableName);
    +        }
    +
    +        byte[] viewIndexTableBytes = 
MetaDataUtil.getViewIndexPhysicalName(inputTable.getPhysicalName().getBytes());
    +        tableName = 
org.apache.hadoop.hbase.TableName.valueOf(viewIndexTableBytes);
    +        boolean viewIndexTableExists = 
connection.getQueryServices().getAdmin().tableExists(tableName);
    +        if(!viewIndexTableExists && inputTable.isMultiTenant()) {
    +            logger.error("View Index Table not found for MultiTenant 
Table: " + inputTable.getName());
    +            throw new 
TablesNotInSyncException(inputTable.getTableName().getString(),
    +                    Bytes.toString(viewIndexTableBytes), " View Index 
table should exist for MultiTenant tables");
    +        }
    +        if(viewIndexTableExists) {
    +            grantPermissionsToTable(hConnection, grantStatement, 
tableName);
    +        }
    +
    +    }
    +
    +    private void grantPermissionsToTable(HConnection hConnection, 
GrantStatement grantStatement, org.apache.hadoop.hbase.TableName tableName)
    +            throws Throwable {
    +        AccessControlClient.grant(hConnection, tableName, 
grantStatement.getName(),
    +                null, null, grantStatement.getPermsList());
    +    }
    +
    +    private void grantPermissionsToSchema(HConnection hConnection, 
GrantStatement grantStatement)
    +            throws Throwable {
    +        AccessControlClient.grant(hConnection, 
grantStatement.getSchemaName(), grantStatement.getName(), 
grantStatement.getPermsList());
    +    }
    +
    +    private void grantPermissionsToUser(HConnection hConnection, 
GrantStatement grantStatement)
    +            throws Throwable {
    +        AccessControlClient.grant(hConnection, grantStatement.getName(), 
grantStatement.getPermsList());
    +    }
    +
    +    public MutationState revokePermission(RevokeStatement revokeStatement) 
throws SQLException {
    +
    +        StringBuffer revokePermLog = new StringBuffer();
    +        revokePermLog.append("Revoke Permissions requested for user/group: 
" + revokeStatement.getName());
    +        if (revokeStatement.getSchemaName() != null) {
    +            revokePermLog.append(" for Schema: " + 
revokeStatement.getSchemaName());
    +        } else if (revokeStatement.getTableName() != null) {
    +            revokePermLog.append(" for Table: " + 
revokeStatement.getTableName());
    +        }
    +        logger.info(revokePermLog.toString());
    +
    +        HConnection hConnection = 
connection.getQueryServices().getAdmin().getConnection();
    +
    +        try {
    +            if (revokeStatement.getSchemaName() != null) {
    +                // SYSTEM.CATALOG doesn't have any entry for "default" 
HBase namespace, hence we will bypass the check
    +                
if(!revokeStatement.getSchemaName().equals(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME))
 {
    +                    
FromCompiler.getResolverForSchema(revokeStatement.getSchemaName(), connection);
    +                }
    +                revokePermissionsFromSchema(hConnection, revokeStatement);
    +
    +            } else if (revokeStatement.getTableName() != null) {
    +                PTable inputTable = PhoenixRuntime.getTable(connection,
    +                        
SchemaUtil.normalizeFullTableName(revokeStatement.getTableName().toString()));
    +                if (!(PTableType.TABLE.equals(inputTable.getType()) || 
PTableType.SYSTEM.equals(inputTable.getType()))) {
    +                    throw new AccessDeniedException("Cannot REVOKE 
permissions from INDEX TABLES or VIEWS");
    +                }
    +                revokePermissionsFromTables(hConnection, revokeStatement, 
inputTable);
    +
    +            } else {
    +                revokePermissionsFromUser(hConnection, revokeStatement);
    +            }
    +
    +        } catch (SQLException e) {
    +            // Bubble up the SQL Exception
    +            throw e;
    +        } catch (Throwable throwable) {
    +            // Wrap around other exceptions to PhoenixIOException (Ex: 
org.apache.hadoop.hbase.security.AccessDeniedException)
    +            throw ServerUtil.parseServerException(throwable);
    +        }
    +
    +        return new MutationState(0, 0, connection);
    +    }
    +
    +
    +    private void revokePermissionsFromTables(HConnection hConnection, 
RevokeStatement revokeStatement, PTable inputTable) throws Throwable {
    +
    +        org.apache.hadoop.hbase.TableName tableName = 
SchemaUtil.getPhysicalTableName
    +                (inputTable.getName().getBytes(), 
inputTable.isNamespaceMapped());
    +
    +        revokePermissionsFromTable(hConnection, revokeStatement, 
tableName);
    +
    +        for(PTable indexTable : inputTable.getIndexes()) {
    +            // Local Indexes don't correspond to new physical table, they 
are just stored in separate CF of base table.
    +            if(indexTable.getIndexType().equals(IndexType.LOCAL)) {
    +                continue;
    +            }
    +            logger.info("Revoking perms from IndexTable: " + 
indexTable.getName() + " BaseTable: " + inputTable.getName());
    +            if (inputTable.isNamespaceMapped() != 
indexTable.isNamespaceMapped()) {
    +                throw new 
TablesNotInSyncException(inputTable.getTableName().getString(),
    +                        indexTable.getTableName().getString(), "Namespace 
properties");
    +            }
    +            tableName = 
SchemaUtil.getPhysicalTableName(indexTable.getName().getBytes(), 
indexTable.isNamespaceMapped());
    +            revokePermissionsFromTable(hConnection, revokeStatement, 
tableName);
    +        }
    +
    +        byte[] viewIndexTableBytes = 
MetaDataUtil.getViewIndexPhysicalName(inputTable.getPhysicalName().getBytes());
    +        tableName = 
org.apache.hadoop.hbase.TableName.valueOf(viewIndexTableBytes);
    +        boolean viewIndexTableExists = 
connection.getQueryServices().getAdmin().tableExists(tableName);
    +        if(!viewIndexTableExists && inputTable.isMultiTenant()) {
    +            logger.error("View Index Table not found for MultiTenant 
Table: " + inputTable.getName());
    --- End diff --
    
    Theoretically yes, This is more of a sanity check. I don't know if you have 
ever encountered cases where the index table failed to get created. Since 
Phoenix operations are not atomic, we can end up in a partial state. The HBase 
Perms API doesn't check if table exists or not, so it will put up an entry even 
if the table doesn't exist. Let me know your thoughts and we can decide.


> Add GRANT and REVOKE commands using HBase AccessController
> ----------------------------------------------------------
>
>                 Key: PHOENIX-672
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-672
>             Project: Phoenix
>          Issue Type: Task
>            Reporter: James Taylor
>            Assignee: Karan Mehta
>              Labels: namespaces, security
>             Fix For: 4.14.0
>
>         Attachments: PHOENIX-672.001.patch
>
>
> In HBase 0.98, cell-level security will be available. Take a look at 
> [this](https://communities.intel.com/community/datastack/blog/2013/10/29/hbase-cell-security)
>  excellent blog post by @apurtell. Once Phoenix works on 0.96, we should add 
> support for security to our SQL grammar.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to