You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by th...@apache.org on 2015/10/23 08:43:02 UTC

hive git commit: HIVE-11901 : StorageBasedAuthorizationProvider requires write permission on table for SELECT statements (Chengbing Liu via Thejas Nair)

Repository: hive
Updated Branches:
  refs/heads/master 698e60dd1 -> 37a8fe062


HIVE-11901 : StorageBasedAuthorizationProvider requires write permission on table for SELECT statements (Chengbing Liu via Thejas Nair)

Signed-off-by: Thejas Nair <th...@hortonworks.com>


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

Branch: refs/heads/master
Commit: 37a8fe0624e1ece7cebff8648a3a981ffbb36e90
Parents: 698e60d
Author: Chengbing Liu <liuchengbing at qiyi dot com>
Authored: Thu Oct 22 23:42:50 2015 -0700
Committer: Thejas Nair <th...@hortonworks.com>
Committed: Thu Oct 22 23:42:50 2015 -0700

----------------------------------------------------------------------
 .../ql/security/TestClientSideAuthorizationProvider.java  |  9 +++++++++
 .../TestStorageBasedClientSideAuthorizationProvider.java  |  6 ++++++
 .../TestStorageBasedMetastoreAuthorizationReads.java      |  7 ++++++-
 .../authorization/StorageBasedAuthorizationProvider.java  | 10 +++++++---
 4 files changed, 28 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/37a8fe06/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestClientSideAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestClientSideAuthorizationProvider.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestClientSideAuthorizationProvider.java
index eedfbca..9040b1c 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestClientSideAuthorizationProvider.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestClientSideAuthorizationProvider.java
@@ -157,6 +157,10 @@ public class TestClientSideAuthorizationProvider extends TestCase {
     InjectableDummyAuthenticator.injectGroupNames(fakeGroupNames);
     InjectableDummyAuthenticator.injectMode(true);
 
+    allowSelectOnTable(tbl.getTableName(), fakeUser, tbl.getSd().getLocation());
+    ret = driver.run(String.format("select * from %s limit 10", tblName));
+    assertEquals(0,ret.getResponseCode());
+
     ret = driver.run(
         String.format("create table %s (a string) partitioned by (b string)", tblName+"mal"));
 
@@ -218,6 +222,11 @@ public class TestClientSideAuthorizationProvider extends TestCase {
     driver.run("grant drop on database "+dbName+" to user "+userName);
   }
 
+  protected void allowSelectOnTable(String tblName, String userName, String location)
+      throws Exception {
+    driver.run("grant select on table "+tblName+" to user "+userName);
+  }
+
   protected void assertNoPrivileges(CommandProcessorResponse ret){
     assertNotNull(ret);
     assertFalse(0 == ret.getResponseCode());

http://git-wip-us.apache.org/repos/asf/hive/blob/37a8fe06/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedClientSideAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedClientSideAuthorizationProvider.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedClientSideAuthorizationProvider.java
index 0da2660..e22ca9f 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedClientSideAuthorizationProvider.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedClientSideAuthorizationProvider.java
@@ -76,6 +76,12 @@ public class TestStorageBasedClientSideAuthorizationProvider extends
     setPermissions(location,"-rwxr--r--");
   }
 
+  @Override
+  protected void allowSelectOnTable(String tblName, String userName, String location)
+      throws Exception {
+    setPermissions(location,"-r--r--r--");
+  }
+
   private void setPermissions(String locn, String permissions) throws Exception {
     FileSystem fs = FileSystem.get(new URI(locn), clientHiveConf);
     fs.setPermission(new Path(locn), FsPermission.valueOf(permissions));

http://git-wip-us.apache.org/repos/asf/hive/blob/37a8fe06/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationReads.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationReads.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationReads.java
index 9cc5c17..ea631d2 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationReads.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationReads.java
@@ -28,7 +28,7 @@ import org.junit.Assert;
 import org.junit.Test;
 
 /**
- * Test cases focusing on drop table permission checks
+ * Test cases focusing on read table permission checks
  */
 public class TestStorageBasedMetastoreAuthorizationReads extends StorageBasedMetastoreTestBase {
 
@@ -38,6 +38,11 @@ public class TestStorageBasedMetastoreAuthorizationReads extends StorageBasedMet
   }
 
   @Test
+  public void testReadTableSuccessWithReadOnly() throws Exception {
+    readTableByOtherUser("-r--r--r--", true);
+  }
+
+  @Test
   public void testReadTableFailure() throws Exception {
     readTableByOtherUser("-rwxrwx---", false);
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/37a8fe06/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 8f81ef9..89e3513 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
@@ -234,9 +234,13 @@ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProvider
     // Partition itself can also be null, in cases where this gets called as a generic
     // catch-all call in cases like those with CTAS onto an unpartitioned table (see HIVE-1887)
     if ((part == null) || (part.getLocation() == null)) {
-      // this should be the case only if this is a create partition.
-      // The privilege needed on the table should be ALTER_DATA, and not CREATE
-      authorize(table, new Privilege[]{}, new Privilege[]{Privilege.ALTER_DATA});
+      if (requireCreatePrivilege(readRequiredPriv) || requireCreatePrivilege(writeRequiredPriv)) {
+        // this should be the case only if this is a create partition.
+        // The privilege needed on the table should be ALTER_DATA, and not CREATE
+        authorize(table, new Privilege[]{}, new Privilege[]{Privilege.ALTER_DATA});
+      } else {
+        authorize(table, readRequiredPriv, writeRequiredPriv);
+      }
     } else {
       authorize(part.getDataLocation(), readRequiredPriv, writeRequiredPriv);
     }