sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pras...@apache.org
Subject git commit: SENTRY-411: Alter table set location does not strictly check for URI privileges (Sravya Tirukkovalur via Prasad Mujumdar)
Date Thu, 28 Aug 2014 18:09:23 GMT
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;
 


Mime
View raw message