hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From da...@apache.org
Subject hive git commit: HIVE-15120: Storage based auth: allow option to enforce write checks for external tables (Daniel Dai, reviewed by Thejas Nair)
Date Tue, 08 Nov 2016 23:50:36 GMT
Repository: hive
Updated Branches:
  refs/heads/branch-1 fd55bd351 -> 6b64b9806


HIVE-15120: Storage based auth: allow option to enforce write checks for external tables (Daniel
Dai, reviewed by Thejas Nair)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/6b64b980
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/6b64b980
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/6b64b980

Branch: refs/heads/branch-1
Commit: 6b64b9806bb2c651878a3fdee267bfdc8c21c2c1
Parents: fd55bd3
Author: Daniel Dai <daijy@hortonworks.com>
Authored: Tue Nov 8 15:50:20 2016 -0800
Committer: Daniel Dai <daijy@hortonworks.com>
Committed: Tue Nov 8 15:50:20 2016 -0800

----------------------------------------------------------------------
 .../org/apache/hadoop/hive/conf/HiveConf.java   |  5 +++++
 ...edMetastoreAuthorizationProviderWithACL.java |  6 ++++++
 .../TestMetastoreAuthorizationProvider.java     | 21 +++++++++++++++++++-
 ...rageBasedMetastoreAuthorizationProvider.java |  6 ++++++
 .../StorageBasedAuthorizationProvider.java      |  9 +++++++--
 5 files changed, 44 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/6b64b980/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
index 1051369..93cf71c 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
@@ -534,6 +534,11 @@ public class HiveConf extends Configuration {
         "for operations like drop-partition (disallow the drop-partition if the user in\n"
+
         "question doesn't have permissions to delete the corresponding directory\n" +
         "on the storage)."),
+    METASTORE_AUTHORIZATION_EXTERNALTABLE_DROP_CHECK("hive.metastore.authorization.storage.check.externaltable.drop",
true,
+        "Should StorageBasedAuthorization check permission of the storage before dropping
external table.\n" +
+        "StorageBasedAuthorization already does this check for managed table. For external
table however,\n" +
+        "anyone who has read permission of the directory could drop external table, which
is surprising.\n" +
+        "The flag is set to false by default to maintain backward compatibility."),
     METASTORE_EVENT_CLEAN_FREQ("hive.metastore.event.clean.freq", "0s",
         new TimeValidator(TimeUnit.SECONDS),
         "Frequency at which timer task runs to purge expired events in metastore."),

http://git-wip-us.apache.org/repos/asf/hive/blob/6b64b980/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
b/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
index 8baa539..ffdae50 100644
--- a/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
+++ b/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
@@ -191,6 +191,12 @@ public class TestStorageBasedMetastoreAuthorizationProviderWithACL
   }
 
   @Override
+  protected void disallowDropOnTable(String tblName, String userName, String location)
+      throws Exception {
+    disallowWriteAccessViaAcl(userName, location);
+  }
+
+  @Override
   protected void allowDropOnDb(String dbName, String userName, String location)
       throws Exception {
     allowWriteAccessViaAcl(userName, location);

http://git-wip-us.apache.org/repos/asf/hive/blob/6b64b980/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
index 4529ce3..4247bb1 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
@@ -19,6 +19,7 @@
 package org.apache.hadoop.hive.ql.security;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 
@@ -298,7 +299,20 @@ public class TestMetastoreAuthorizationProvider extends TestCase {
 
     allowDropOnTable(tblName, userName, tbl.getSd().getLocation());
     allowDropOnDb(dbName,userName,db.getLocationUri());
-    driver.run("drop database if exists "+getTestDbName()+" cascade");
+    ret = driver.run("drop database if exists "+getTestDbName()+" cascade");
+    assertEquals(0,ret.getResponseCode());
+
+    InjectableDummyAuthenticator.injectUserName(userName);
+    InjectableDummyAuthenticator.injectGroupNames(Arrays.asList(ugi.getGroupNames()));
+    InjectableDummyAuthenticator.injectMode(true);
+    allowCreateDatabase(userName);
+    driver.run("create database " + dbName);
+    allowCreateInDb(dbName, userName, dbLocn);
+    tbl.setTableType("EXTERNAL_TABLE");
+    msc.createTable(tbl);
+    disallowDropOnTable(tblName, userName, tbl.getSd().getLocation());
+    ret = driver.run("drop table "+tbl.getTableName());
+    assertEquals(1,ret.getResponseCode());
 
   }
 
@@ -338,6 +352,11 @@ public class TestMetastoreAuthorizationProvider extends TestCase {
     driver.run("grant drop on table "+tblName+" to user "+userName);
   }
 
+  protected void disallowDropOnTable(String tblName, String userName, String location)
+      throws Exception {
+    driver.run("revoke drop on table "+tblName+" from user "+userName);
+  }
+
   protected void allowDropOnDb(String dbName, String userName, String location)
       throws Exception {
     driver.run("grant drop on database "+dbName+" to user "+userName);

http://git-wip-us.apache.org/repos/asf/hive/blob/6b64b980/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
index 78ff780..272c924 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
@@ -77,6 +77,12 @@ public class TestStorageBasedMetastoreAuthorizationProvider extends
   }
 
   @Override
+  protected void disallowDropOnTable(String tblName, String userName, String location)
+      throws Exception {
+    setPermissions(location,"-r--r--r--");
+  }
+
+  @Override
   protected void allowDropOnDb(String dbName, String userName, String location)
       throws Exception {
     setPermissions(location,"-rwxr--r-t");

http://git-wip-us.apache.org/repos/asf/hive/blob/6b64b980/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
index 89e3513..4c0f83d 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
@@ -35,6 +35,7 @@ import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.hive.common.FileUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.HiveMetaStore.HMSHandler;
 import org.apache.hadoop.hive.metastore.TableType;
 import org.apache.hadoop.hive.metastore.Warehouse;
@@ -177,8 +178,12 @@ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProvider
 
     Path path = table.getDataLocation();
     // authorize drops if there was a drop privilege requirement, and
-    // table is not external (external table data is not dropped)
-    if (privExtractor.hasDropPrivilege() && table.getTableType() != TableType.EXTERNAL_TABLE)
{
+    // table is not external (external table data is not dropped) or
+    // "hive.metastore.authorization.storage.check.externaltable.drop"
+    // set to true
+    if (privExtractor.hasDropPrivilege() && (table.getTableType() != TableType.EXTERNAL_TABLE
||
+        getConf().getBoolean(HiveConf.ConfVars.METASTORE_AUTHORIZATION_EXTERNALTABLE_DROP_CHECK.varname,
+        HiveConf.ConfVars.METASTORE_AUTHORIZATION_EXTERNALTABLE_DROP_CHECK.defaultBoolVal)))
{
       checkDeletePermission(path, getConf(), authenticator.getUserName());
     }
 


Mime
View raw message