This is an automated email from the ASF dual-hosted git repository. stoty pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/phoenix.git
commit f93c93168809b44f687ad5cf16fb31f5b8a3fa28 Author: Sergey Soldatov <ssolda...@cloudera.com> AuthorDate: Fri Oct 29 20:29:56 2021 +0300 PHOENIX-6579 ACL check doesn't honor the namespace mapping for mapped views. --- .../apache/phoenix/end2end/BasePermissionsIT.java | 6 +-- .../phoenix/end2end/PermissionNSEnabledIT.java | 45 ++++++++++++++++++++++ .../phoenix/coprocessor/MetaDataEndpointImpl.java | 4 +- .../coprocessor/PhoenixAccessController.java | 27 +++++++------ 4 files changed, 66 insertions(+), 16 deletions(-) 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 d1ce77a..c9fc1a7 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 @@ -99,7 +99,7 @@ public abstract class BasePermissionsIT extends BaseTest { private static final String SUPER_USER = System.getProperty("user.name"); - private static HBaseTestingUtility testUtil; + static HBaseTestingUtility testUtil; private static final Set<String> PHOENIX_SYSTEM_TABLES = new HashSet<>(Arrays.asList("SYSTEM.CATALOG", "SYSTEM.SEQUENCE", "SYSTEM.STATS", "SYSTEM.FUNCTION", "SYSTEM.MUTEX", "SYSTEM.CHILD_LINK", "SYSTEM.TRANSFORM")); @@ -365,7 +365,7 @@ public abstract class BasePermissionsIT extends BaseTest { // UG Object // 1. Instance of String --> represents GROUP name // 2. Instance of User --> represents HBase user - private AccessTestAction grantPermissions(final String actions, final Object ug, + AccessTestAction grantPermissions(final String actions, final Object ug, final String tableOrSchemaList, final boolean isSchema) throws SQLException { return grantPermissions(actions, ug, Collections.singleton(tableOrSchemaList), isSchema); } @@ -958,7 +958,7 @@ public abstract class BasePermissionsIT extends BaseTest { } } - private String surroundWithDoubleQuotes(String input) { + String surroundWithDoubleQuotes(String input) { return "\"" + input + "\""; } diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/PermissionNSEnabledIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/PermissionNSEnabledIT.java index 7a2a995..292654f 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/PermissionNSEnabledIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/PermissionNSEnabledIT.java @@ -18,8 +18,14 @@ package org.apache.phoenix.end2end; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.access.Permission; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.phoenix.exception.SQLExceptionCode; import org.apache.phoenix.util.SchemaUtil; import org.junit.BeforeClass; @@ -29,11 +35,13 @@ import org.junit.experimental.categories.Category; import java.security.PrivilegedExceptionAction; import java.sql.Connection; import java.sql.SQLException; +import java.sql.Statement; import java.util.Collections; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.SYSTEM_CATALOG_TABLE; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.SYSTEM_CHILD_LINK_TABLE; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.SYSTEM_SCHEMA_NAME; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -48,6 +56,43 @@ public class PermissionNSEnabledIT extends BasePermissionsIT { public static synchronized void doSetup() throws Exception { BasePermissionsIT.initCluster(true); } + private AccessTestAction createMappedView(final String schemaName, final String tableName) throws SQLException { + return new AccessTestAction() { + @Override + public Object run() throws Exception { + try (Connection conn = getConnection(); Statement stmt = conn.createStatement();) { + String viewStmtSQL = "CREATE VIEW \"" + schemaName + "\".\"" + tableName + "\" ( PK varchar primary key)"; + assertFalse(stmt.execute(viewStmtSQL)); + } + return null; + } + }; + } + + + @Test + public void testCreateMappedView() throws Throwable { + final String schema = generateUniqueName(); + final String tableName = generateUniqueName(); + verifyAllowed(createSchema(schema), superUser1); + grantPermissions(regularUser1.getShortName(), schema, Permission.Action.WRITE, + Permission.Action.READ, Permission.Action.EXEC, Permission.Action.ADMIN); + grantPermissions(regularUser1.getShortName(), "SYSTEM", Permission.Action.WRITE, + Permission.Action.READ, Permission.Action.EXEC); + superUser1.runAs(new PrivilegedExceptionAction<Void>() { + @Override + public Void run() throws Exception { + Admin admin = testUtil.getAdmin(); + TableDescriptorBuilder tdb = TableDescriptorBuilder.newBuilder(TableName.valueOf(schema + ":" + tableName)); + ColumnFamilyDescriptor cfd = ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("0")).build(); + tdb.setColumnFamily(cfd); + TableDescriptor td = tdb.build(); + admin.createTable(td); + return null; + } + }); + verifyAllowed(createMappedView(schema, tableName), regularUser1); + } @Test public void testSchemaPermissions() throws Throwable{ diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java index 70980ec..a5cd7b4 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java @@ -2136,7 +2136,9 @@ TABLE_FAMILY_BYTES, TABLE_SEQ_NUM_BYTES); } } else { // Mapped View - cParentPhysicalName = SchemaUtil.getTableNameAsBytes(schemaName, tableName); + cParentPhysicalName = SchemaUtil.getPhysicalHBaseTableName( + schemaName, tableName, isNamespaceMapped).getBytes(); + } parentSchemaName = parentPhysicalSchemaTableNames[1]; parentTableName = parentPhysicalSchemaTableNames[2]; 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 c6f49a5..f0217c4 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 @@ -110,7 +110,8 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { for (RegionCoprocessor cp : cpHost.findCoprocessors(RegionCoprocessor.class)) { if (cp instanceof AccessControlService.Interface && cp instanceof MasterObserver) { oldAccessControllers.add((MasterObserver)cp); - if(cp.getClass().getName().equals(org.apache.hadoop.hbase.security.access.AccessController.class.getName())) { + if (cp.getClass().getName().equals( + org.apache.hadoop.hbase.security.access.AccessController.class.getName())) { hbaseAccessControllerEnabled = true; } } @@ -128,7 +129,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { public void preGetTable(ObserverContext<PhoenixMetaDataControllerEnvironment> ctx, String tenantId, String tableName, TableName physicalTableName) throws IOException { if (!accessCheckEnabled) { return; } - if(this.execPermissionsCheckEnabled) { + if (this.execPermissionsCheckEnabled) { requireAccess("GetTable" + tenantId, physicalTableName, Action.READ, Action.EXEC); } else { requireAccess("GetTable" + tenantId, physicalTableName, Action.READ); @@ -169,7 +170,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { @Override public void stop(CoprocessorEnvironment env) throws IOException { - if(accessChecker.getAuthManager() != null) { + if (accessChecker.getAuthManager() != null) { CompatPermissionUtil.stopAccessChecker(accessChecker); } } @@ -195,7 +196,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { Set<TableName> physicalTablesChecked = new HashSet<TableName>(); if (tableType == PTableType.VIEW || tableType == PTableType.INDEX) { physicalTablesChecked.add(parentPhysicalTableName); - if(execPermissionsCheckEnabled) { + if (execPermissionsCheckEnabled) { requireAccess("Create" + tableType, parentPhysicalTableName, Action.READ, Action.EXEC); } else { requireAccess("Create" + tableType, parentPhysicalTableName, Action.READ); @@ -256,7 +257,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { if (physicalTableName != null && !parentPhysicalTableName.equals(physicalTableName) && !MetaDataUtil.isViewIndex(physicalTableName.getNameAsString())) { List<Action> actions = new ArrayList<>(Arrays.asList(Action.READ, Action.WRITE, Action.CREATE, Action.ADMIN)); - if(execPermissionsCheckEnabled) { + if (execPermissionsCheckEnabled) { actions.add(Action.EXEC); } authorizeOrGrantAccessToUsers("Create" + tableType, parentPhysicalTableName, @@ -332,7 +333,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { } } if (!requireAccess.isEmpty()) { - if(AuthUtil.isGroupPrincipal(getUserFromUP(userPermission))){ + if (AuthUtil.isGroupPrincipal(getUserFromUP(userPermission))){ AUDITLOG.warn("Users of GROUP:" + getUserFromUP(userPermission) + " will not have following access " + requireAccess + " to the newly created index " + toTable @@ -387,7 +388,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { } //checking similar permission checked during the create of the view. if (tableType == PTableType.VIEW || tableType == PTableType.INDEX) { - if(execPermissionsCheckEnabled) { + if (execPermissionsCheckEnabled) { requireAccess("Drop "+tableType, parentPhysicalTableName, Action.READ, Action.EXEC); } else { requireAccess("Drop "+tableType, parentPhysicalTableName, Action.READ); @@ -406,7 +407,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { } } if (tableType == PTableType.VIEW) { - if(execPermissionsCheckEnabled) { + if (execPermissionsCheckEnabled) { requireAccess("Alter "+tableType, parentPhysicalTableName, Action.READ, Action.EXEC); } else { requireAccess("Alter "+tableType, parentPhysicalTableName, Action.READ); @@ -454,7 +455,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { } // Check for read access in case of rebuild if (newState == PIndexState.BUILDING) { - if(execPermissionsCheckEnabled) { + if (execPermissionsCheckEnabled) { requireAccess("Rebuild:", parentPhysicalTableName, Action.READ, Action.EXEC); } else { requireAccess("Rebuild:", parentPhysicalTableName, Action.READ); @@ -480,10 +481,12 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver { // Merge permissions from all accessController coprocessors loaded in memory for (MasterObserver service : getAccessControllers()) { // 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.getNameAsString())); + if (service.getClass().getName().equals( + org.apache.hadoop.hbase.security.access.AccessController.class.getName())) { userPermissions.addAll(AccessControlClient.getUserPermissions( - connection, AuthUtil.toGroupEntry(tableName.getNamespaceAsString()))); + connection, tableName.getNameWithNamespaceInclAsString())); + userPermissions.addAll(AccessControlClient.getUserPermissions( + connection, AuthUtil.toGroupEntry(tableName.getNamespaceAsString()))); } } } catch (Throwable e) {