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 <prasadm@cloudera.com>
Authored: Thu Aug 28 11:09:21 2014 -0700
Committer: Prasad Mujumdar <prasadm@cloudera.com>
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;
|