You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by am...@apache.org on 2019/06/14 18:40:33 UTC

[sentry] branch master updated: SENTRY-2240: User can DROP function under a database that he/she has no access (Arjun Mishra reviewed by Na Li)

This is an automated email from the ASF dual-hosted git repository.

amishra pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sentry.git


The following commit(s) were added to refs/heads/master by this push:
     new 941ce04  SENTRY-2240: User can DROP function under a database that he/she has no access (Arjun Mishra reviewed by Na Li)
941ce04 is described below

commit 941ce0443de53938d7f9085a05ab928fd4c180b1
Author: amishra <am...@cloudera.com>
AuthorDate: Fri Jun 14 13:37:43 2019 -0500

    SENTRY-2240: User can DROP function under a database that he/she has no access (Arjun Mishra reviewed by Na Li)
---
 .../sentry/binding/hive/HiveAuthzBindingHook.java  |  8 +--
 .../hive/authz/HiveAuthzBindingHookBase.java       |  5 ++
 .../binding/hive/authz/HiveAuthzPrivilegesMap.java | 12 +++-
 .../e2e/hive/TestPrivilegesAtFunctionScope.java    | 81 ++++++++++++++++++----
 4 files changed, 85 insertions(+), 21 deletions(-)

diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
index e87d0f6..643868f 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
@@ -222,13 +222,9 @@ public class HiveAuthzBindingHook extends HiveAuthzBindingHookBase {
             }
           }
         }
-
-        // create/drop function is allowed with any database
-        currDB = Database.ALL;
-        break;
       case HiveParser.TOK_DROPFUNCTION:
-        // create/drop function is allowed with any database
-        currDB = Database.ALL;
+        currTab = extractTable((ASTNode)ast.getChild(0));
+        currDB = extractDatabase((ASTNode)ast.getChild(0));
         break;
 
       case HiveParser.TOK_LOAD:
diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
index ed278c8..de88705 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
@@ -458,6 +458,11 @@ public abstract class HiveAuthzBindingHookBase extends AbstractSemanticAnalyzerH
           entityHierarchy.addAll(getAuthzHierarchyFromEntity(writeEntity));
           outputHierarchy.add(entityHierarchy);
         }
+      } else if (currDB != Database.ALL) { //This will be true with DROP FUNCTION commands
+        List<DBModelAuthorizable> entityHierarchy = new ArrayList<DBModelAuthorizable>();
+        entityHierarchy.add(hiveAuthzBinding.getAuthServer());
+        entityHierarchy.add(currDB);
+        inputHierarchy.add(entityHierarchy);
       }
       break;
     case CONNECT:
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 1aaa9b3..d6828c4 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
@@ -226,13 +226,19 @@ public class HiveAuthzPrivilegesMap {
         .setOperationScope(HiveOperationScope.DATABASE).setOperationType(HiveOperationType.DML)
         .build();
 
-    HiveAuthzPrivileges functionPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder().
+    HiveAuthzPrivileges createFunctionPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder().
         addInputObjectPriviledge(AuthorizableType.URI, EnumSet.of(DBModelAction.ALL)).
         addOutputObjectPriviledge(AuthorizableType.URI, EnumSet.of(DBModelAction.ALL)).
         setOperationScope(HiveOperationScope.FUNCTION).
         setOperationType(HiveOperationType.DATA_LOAD).
         build();
 
+    HiveAuthzPrivileges dropFunctionPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder().
+        addInputObjectPriviledge(AuthorizableType.Db, EnumSet.of(DBModelAction.DROP)).
+        setOperationScope(HiveOperationScope.FUNCTION).
+        setOperationType(HiveOperationType.DATA_LOAD).
+        build();
+
     HiveAuthzPrivileges anyPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder().
         addInputObjectPriviledge(AuthorizableType.Column, EnumSet.of(DBModelAction.SELECT,
             DBModelAction.INSERT, DBModelAction.ALTER, DBModelAction.CREATE, DBModelAction.DROP,
@@ -316,8 +322,8 @@ public class HiveAuthzPrivilegesMap {
 
     // CREATEFUNCTION
     // DROPFUNCTION
-    hiveAuthzStmtPrivMap.put(HiveOperation.CREATEFUNCTION, functionPrivilege);
-    hiveAuthzStmtPrivMap.put(HiveOperation.DROPFUNCTION, functionPrivilege);
+    hiveAuthzStmtPrivMap.put(HiveOperation.CREATEFUNCTION, createFunctionPrivilege);
+    hiveAuthzStmtPrivMap.put(HiveOperation.DROPFUNCTION, dropFunctionPrivilege);
 
     // SHOWCOLUMNS
     hiveAuthzStmtPrivMap.put(HiveOperation.SHOWCOLUMNS, columnMetaDataPrivilege);
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
index bd0f978..c6e14a5 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
@@ -100,10 +100,12 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
     context.close();
 
     policyFile
-        .addRolesToGroup(USERGROUP1, "db1_all", "UDF2_JAR", "UDF1_JAR", "UDF_JAR", "data_read")
+        .addRolesToGroup(USERGROUP1, "db1_all", "db2_all", "UDF2_JAR", "UDF1_JAR", "UDF_JAR", "data_read")
         .addRolesToGroup(USERGROUP2, "db1_tab1", "UDF_JAR")
         .addRolesToGroup(USERGROUP3, "db1_tab1")
+        .addRolesToGroup(USERGROUP4, "db1_all","db2_all")
         .addPermissionsToRole("db1_all", "server=server1->db=" + DB1)
+        .addPermissionsToRole("db2_all", "server=server1->db=" + DB2)
         .addPermissionsToRole("db1_tab1", "server=server1->db=" + DB1 + "->table=" + tableName1)
         .addPermissionsToRole("UDF_JAR", "server=server1->uri=file://" + udfLocation)
         .addPermissionsToRole("UDF1_JAR", "server=server1->uri=file://" + udf1Location)
@@ -117,23 +119,43 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
    * @throws Exception
    */
   @Test
+  @SlowE2ETest
   public void testCreateDropPermUdf() throws Exception {
     setUpContext();
     // user1 has URI privilege for the Jar and should be able create the perm function.
     Connection connection = context.createConnection(USER1_1);
     Statement statement = context.createStatement(connection);
 
-    statement.execute("CREATE FUNCTION db_2.test_1 AS 'org.apache.sentry.tests.e2e.hive.TestUDF'");
+    try {
+      statement.execute("CREATE FUNCTION db_2.test_1 AS 'org.apache.sentry.tests.e2e.hive.TestUDF'");
+    } catch (Exception e) {
+      fail("CREATE TEMPORARY FUNCTION should not fail for user1");
+    }
     statement.close();
     connection.close();
 
     connection = context.createConnection(USER3_1);
     statement = context.createStatement(connection);
 
-    // user3 does not have URI privilege but still should be able drop the perm function.
-    statement.execute("DROP FUNCTION IF EXISTS db_2.test_1");
+    try {
+      statement.execute("DROP FUNCTION IF EXISTS db_2.test_1");
+      fail("DROP TEMPORARY FUNCTION should fail for user3");
+    } catch(SQLException e) {
+      context.verifyAuthzException(e);
+    }
+
     statement.close();
     connection.close();
+
+    //user4 has privileges to drop function
+    connection = context.createConnection(USER4_1);
+    statement = context.createStatement(connection);
+    try {
+      statement.execute("DROP FUNCTION IF EXISTS db_2.test_1");
+    } catch(SQLException e) {
+      fail("DROP TEMPORARY FUNCTION should not fail for user4");
+    }
+    context.close();
   }
 
   /**
@@ -141,6 +163,7 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
    * @throws Exception
    */
   @Test
+  @SlowE2ETest
   public void testCreateDropTempUdf() throws Exception {
     setUpContext();
     // user1 should be able create/drop temp functions
@@ -168,11 +191,31 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
     connection = context.createConnection(USER3_1);
     statement = context.createStatement(connection);
 
-    // user3 does not have URI privilege but still should be able drop the temp function.
-    statement.execute("DROP FUNCTION IF EXISTS test_2");
+    // user3 does not have URI privilege and should not be allowed drop the temp function.
+    try {
+      statement.execute("DROP FUNCTION IF EXISTS db_1.test_2");
+      fail("DROP FUNCTION should fail for user3");
+    } catch (SQLException e) {
+      context.verifyAuthzException(e);
+    }
+
+    statement.close();
+    connection.close();
+
+    //user4 has privileges to drop function
+    connection = context.createConnection(USER4_1);
+    statement = context.createStatement(connection);
+    try {
+      statement.execute("DROP FUNCTION IF EXISTS db_1.test_2");
+    } catch(SQLException e) {
+      fail("DROP TEMPORARY FUNCTION should not fail for user4");
+    }
+
+    context.close();
   }
 
   @Test
+  @SlowE2ETest
   public void testAndVerifyFuncPrivilegesPart1() throws Exception {
     setUpContext();
     // user1 should be able create/drop temp functions
@@ -195,7 +238,11 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
     connection = context.createConnection(USER1_1);
     statement = context.createStatement(connection);
     statement.execute("USE " + DB1);
-    statement.execute("DROP TEMPORARY FUNCTION IF EXISTS printf_test");
+    try {
+      statement.execute("DROP TEMPORARY FUNCTION IF EXISTS printf_test");
+    } catch (SQLException e) {
+      fail("user1 has privileges to drop db1 functions");
+    }
 
     LOGGER.info("Testing select from perm func printf_test_perm");
     try {
@@ -213,7 +260,11 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
     connection = context.createConnection(USER1_1);
     statement = context.createStatement(connection);
     statement.execute("USE " + DB1);
-    statement.execute("DROP FUNCTION IF EXISTS printf_test_perm");
+    try {
+      statement.execute("DROP FUNCTION IF EXISTS printf_test_perm");
+    } catch (SQLException e) {
+      fail("user1 does have permissions to drop db1 functions");
+    }
 
     // test perm UDF with 'using file' syntax
     LOGGER.info("Testing select from perm func printf_test_perm_use_file");
@@ -231,6 +282,7 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
   }
 
   @Test
+  @SlowE2ETest
   public void testAndVerifyFuncPrivilegesPart2() throws Exception {
     setUpContext();
     // user2 has select privilege on one of the tables in db1, should be able create/drop temp functions
@@ -245,8 +297,6 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
     } catch (Exception ex) {
       LOGGER.error("test perm func printf_test_2 failed with reason: ", ex);
       fail("Fail to test temp func printf_test_2");
-    } finally {
-      statement.execute("DROP TEMPORARY FUNCTION IF EXISTS printf_test_2");
     }
 
     try {
@@ -256,8 +306,6 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
     } catch (Exception ex) {
       LOGGER.error("test perm func printf_test_2_perm failed with reason: ", ex);
       fail("Fail to test temp func printf_test_2_perm");
-    } finally {
-      statement.execute("DROP FUNCTION IF EXISTS printf_test_2_perm");
     }
 
     // USER2 doesn't have URI perm on dataFile
@@ -271,10 +319,18 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
     } catch (SQLException e) {
       context.verifyAuthzException(e);
     }
+
+    connection = context.createConnection(ADMIN1);
+    statement = context.createStatement(connection);
+    statement.execute("USE " + DB1);
+    statement.execute("DROP TEMPORARY FUNCTION IF EXISTS printf_test_2");
+    statement.execute("DROP FUNCTION IF EXISTS printf_test_2_perm");
+
     context.close();
   }
 
   @Test
+  @SlowE2ETest
   public void testAndVerifyFuncPrivilegesPart3() throws Exception {
     setUpContext();
     // user3 shouldn't be able to create/drop temp functions since it doesn't have permission for jar
@@ -316,6 +372,7 @@ public class TestPrivilegesAtFunctionScope extends AbstractTestWithStaticConfigu
    * @throws Exception
    */
   @Test
+  @SlowE2ETest
   public void testAndVerifyFuncPrivilegesPart4() throws Exception {
     setUpContext();
     Connection connection = context.createConnection(USER1_1);