Repository: incubator-sentry Updated Branches: refs/heads/master b6c62f791 -> 7dc84d72d
SENTRY-411: Alter table set location does not strictly check for URI privileges (Sravya Tirukkovalur via Prasad Mujumdar) Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/7dc84d72 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/7dc84d72 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/7dc84d72 Branch: refs/heads/master Commit: 7dc84d72d9df267af20d9ac3746390bb333dc409 Parents: b6c62f7 Author: Prasad Mujumdar <[email protected]> Authored: Thu Aug 28 11:09:21 2014 -0700 Committer: Prasad Mujumdar <[email protected]> Committed: Thu Aug 28 11:09:21 2014 -0700 ---------------------------------------------------------------------- .../hive/authz/HiveAuthzPrivilegesMap.java | 2 +- .../metastore/MetastoreAuthzBinding.java | 41 +++++++++++--------- .../AbstractTestWithStaticConfiguration.java | 2 + .../sentry/tests/e2e/hive/TestOperations.java | 15 ++++++- .../e2e/hive/TestPrivilegesAtDatabaseScope.java | 12 ++---- .../tests/e2e/hive/TestUriPermissions.java | 2 +- 6 files changed, 44 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7dc84d72/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java index 761082a..9498a28 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java @@ -40,7 +40,7 @@ public class HiveAuthzPrivilegesMap { build(); HiveAuthzPrivileges tableDDLAndUriPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder(). addInputObjectPriviledge(AuthorizableType.Table, EnumSet.of(DBModelAction.ALL)). - addInputObjectPriviledge(AuthorizableType.URI, EnumSet.of(DBModelAction.SELECT)). + addOutputObjectPriviledge(AuthorizableType.URI, EnumSet.of(DBModelAction.ALL)). setOperationScope(HiveOperationScope.TABLE). setOperationType(HiveOperationType.DDL). build(); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7dc84d72/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java index 2ff8a08..51e3d77 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java @@ -222,6 +222,9 @@ public class MetastoreAuthzBinding extends MetaStorePreEventListener { throws InvalidOperationException, MetaException { HierarcyBuilder inputBuilder = new HierarcyBuilder(); inputBuilder.addDbToOutput(getAuthServer(), context.getTable().getDbName()); + HierarcyBuilder outputBuilder = new HierarcyBuilder(); + outputBuilder.addDbToOutput(getAuthServer(), context.getTable().getDbName()); + if (!StringUtils.isEmpty(context.getTable().getSd().getLocation())) { String uriPath; try { @@ -233,8 +236,7 @@ public class MetastoreAuthzBinding extends MetaStorePreEventListener { inputBuilder.addUriToOutput(getAuthServer(), uriPath); } authorizeMetastoreAccess(HiveOperation.CREATETABLE, inputBuilder.build(), - new HierarcyBuilder().addDbToOutput( - getAuthServer(), context.getTable().getDbName()).build()); + outputBuilder.build()); } private void authorizeDropTable(PreDropTableEvent context) @@ -262,6 +264,10 @@ public class MetastoreAuthzBinding extends MetaStorePreEventListener { HierarcyBuilder inputBuilder = new HierarcyBuilder(); inputBuilder.addTableToOutput(getAuthServer(), context.getOldTable() .getDbName(), context.getOldTable().getTableName()); + HierarcyBuilder outputBuilder = new HierarcyBuilder(); + outputBuilder.addTableToOutput(getAuthServer(), context.getOldTable() + .getDbName(), context.getOldTable().getTableName()); + // if the operation requires location change, then add URI privilege check String oldLocationUri; String newLocationUri; @@ -274,22 +280,23 @@ public class MetastoreAuthzBinding extends MetaStorePreEventListener { throw new MetaException(e.getMessage()); } if (oldLocationUri.compareTo(newLocationUri) != 0) { - inputBuilder.addUriToOutput(getAuthServer(), newLocationUri); + outputBuilder.addUriToOutput(getAuthServer(), newLocationUri); operation = HiveOperation.ALTERTABLE_LOCATION; } authorizeMetastoreAccess( operation, - inputBuilder.build(), - new HierarcyBuilder().addTableToOutput(getAuthServer(), - context.getOldTable().getDbName(), - context.getOldTable().getTableName()).build()); + inputBuilder.build(), outputBuilder.build()); + } private void authorizeAddPartition(PreAddPartitionEvent context) throws InvalidOperationException, MetaException, NoSuchObjectException { for (org.apache.hadoop.hive.metastore.api.Partition mapiPart : context.getPartitions()) { HierarcyBuilder inputBuilder = new HierarcyBuilder(); - inputBuilder.addTableToOutput(getAuthServer(), mapiPart + inputBuilder.addTableToOutput(getAuthServer(), mapiPart + .getDbName(), mapiPart.getTableName()); + HierarcyBuilder outputBuilder = new HierarcyBuilder(); + outputBuilder.addTableToOutput(getAuthServer(), mapiPart .getDbName(), mapiPart.getTableName()); // check if we need to validate URI permissions when storage location is // non-default, ie something not under the parent table @@ -307,14 +314,11 @@ public class MetastoreAuthzBinding extends MetaStorePreEventListener { throw new MetaException(e.getMessage()); } if (!partitionLocation.startsWith(tableLocation + File.separator)) { - inputBuilder.addUriToOutput(getAuthServer(), uriPath); + outputBuilder.addUriToOutput(getAuthServer(), uriPath); } } - authorizeMetastoreAccess(HiveOperation.ALTERTABLE_ADDPARTS, - inputBuilder.build(), - new HierarcyBuilder().addTableToOutput(getAuthServer(), - mapiPart.getDbName(), - mapiPart.getTableName()).build()); + authorizeMetastoreAccess(HiveOperation.ALTERTABLE_ADDPARTS, + inputBuilder.build(), outputBuilder.build()); } } @@ -341,6 +345,9 @@ public class MetastoreAuthzBinding extends MetaStorePreEventListener { */ HierarcyBuilder inputBuilder = new HierarcyBuilder().addTableToOutput( getAuthServer(), context.getDbName(), context.getTableName()); + HierarcyBuilder outputBuilder = new HierarcyBuilder().addTableToOutput( + getAuthServer(), context.getDbName(), context.getTableName()); + String partitionLocation = getSdLocation(context.getNewPartition().getSd()); if (!StringUtils.isEmpty(partitionLocation)) { String uriPath; @@ -349,13 +356,11 @@ public class MetastoreAuthzBinding extends MetaStorePreEventListener { } catch (URISyntaxException e) { throw new MetaException(e.getMessage()); } - inputBuilder.addUriToOutput(getAuthServer(), uriPath); + outputBuilder.addUriToOutput(getAuthServer(), uriPath); } authorizeMetastoreAccess( HiveOperation.ALTERPARTITION_LOCATION, - inputBuilder.build(), - new HierarcyBuilder().addTableToOutput(getAuthServer(), - context.getDbName(), context.getTableName()).build()); + inputBuilder.build(), outputBuilder.build()); } private InvalidOperationException invalidOperationException(Exception e) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7dc84d72/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java index 3a7aa41..f251ebc 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java @@ -128,6 +128,8 @@ public abstract class AbstractTestWithStaticConfiguration { protected static SentryService sentryServer; protected static Configuration sentryConf; protected static Context context; + protected final String semanticException = "SemanticException No valid privileges"; + public static void createContext() throws Exception { context = new Context(hiveServer, fileSystem, http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7dc84d72/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java index fddb343..30cbb0d 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java @@ -36,7 +36,6 @@ import com.google.common.io.Resources; public class TestOperations extends AbstractTestWithStaticConfiguration { private PolicyFile policyFile; final String tableName = "tb1"; - final String semanticException = "SemanticException No valid privileges"; static Map<String, String> privileges = new HashMap<String, String>(); static { @@ -455,7 +454,19 @@ public class TestOperations extends AbstractTestWithStaticConfiguration { statement.close(); connection.close(); - //Negative case + //Negative case: User2_1 has privileges on table but on on uri + connection = context.createConnection(USER2_1); + statement = context.createStatement(connection); + statement.execute("Use " + DB1); + context.assertSentrySemanticException(statement, "ALTER TABLE tb1 SET LOCATION '" + tabLocation + "'", + semanticException); + context.assertSentrySemanticException(statement, + "ALTER TABLE tb1 ADD IF NOT EXISTS PARTITION (b = '3') LOCATION '" + tabLocation + "/part'", + semanticException); + statement.close(); + connection.close(); + + //Negative case: User3_1 has only insert privileges on table policyFile .addPermissionsToRole("insert_db1_tb1", privileges.get("insert_db1_tb1")) .addRolesToGroup(USERGROUP3, "insert_db1_tb1", "all_uri"); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7dc84d72/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java index 653b6fb..7c9a66d 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java @@ -265,18 +265,14 @@ public class TestPrivilegesAtDatabaseScope extends AbstractTestWithStaticConfigu // test user can drop table statement.execute("DROP TABLE TAB_3"); - //negative test case: user can't create external tables + //positive test case: user can create external tables at given location assertTrue("Unable to create directory for external table test" , externalTblDir.mkdir()); statement.execute("CREATE EXTERNAL TABLE EXT_TAB_1(A STRING) STORED AS TEXTFILE LOCATION 'file:"+ externalTblDir.getAbsolutePath() + "'"); - //negative test case: user can't execute alter table set location - try { - statement.execute("ALTER TABLE TAB_2 SET LOCATION 'hdfs://nn1.example.com/hive/warehouse'"); - Assert.fail("Expected SQL exception"); - } catch (SQLException e) { - context.verifyAuthzException(e); - } + //negative test case: user can't execute alter table set location, + // as the user does not have privileges on that location + context.assertSentrySemanticException(statement, "ALTER TABLE TAB_2 SET LOCATION 'file:///tab2'", semanticException); statement.close(); connection.close(); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7dc84d72/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java index c55278c..7c7c63e 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java @@ -172,7 +172,7 @@ public class TestUriPermissions extends AbstractTestWithStaticConfiguration { @Test public void testAlterTableLocationPrivileges() throws Exception { String tabName = "tab1"; - String tabDir = "file://" + hiveServer.getProperty(HiveServerFactory.WAREHOUSE_DIR) + "/" + tabName; + String tabDir = hiveServer.getProperty(HiveServerFactory.WAREHOUSE_DIR) + "/" + tabName; Connection userConn = null; Statement userStmt = null;
