[ 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)