Repository: sentry
Updated Branches:
  refs/heads/master 8e0505703 -> 4e473e9d3


SENTRY-2433: Dropping object privileges does not include update of dropping 
user privileges (Na Li, reviewed by Kalyan Kumar Kalvagadda)


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/4e473e9d
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/4e473e9d
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/4e473e9d

Branch: refs/heads/master
Commit: 4e473e9d3965c28d9b120776e48c0d4593d719b2
Parents: 8e05057
Author: lina.li <lina...@cloudera.com>
Authored: Thu Oct 25 16:50:30 2018 -0500
Committer: lina.li <lina...@cloudera.com>
Committed: Tue Oct 30 14:19:04 2018 -0500

----------------------------------------------------------------------
 .../sentry/hdfs/UpdateableAuthzPermissions.java |  2 +-
 .../org/apache/sentry/hdfs/SentryPlugin.java    |  8 ++-
 .../persistent/NotificationProcessor.java       |  8 ++-
 .../e2e/dbprovider/TestOwnerPrivileges.java     | 60 ++++++++++++++++++++
 4 files changed, 73 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/4e473e9d/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
----------------------------------------------------------------------
diff --git 
a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
 
b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
index c87d205..5726c0e 100644
--- 
a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
+++ 
b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
@@ -180,7 +180,7 @@ public class UpdateableAuthzPermissions implements 
AuthzPermissions, Updateable<
         perms.addPrivilegeInfo(pInfo);
         perms.addParentChildMappings(pUpdate.getAuthzObj());
         for (Map.Entry<TPrivilegePrincipal, String> dMap : 
pUpdate.getDelPrivileges().entrySet()) {
-          if (dMap.getKey().getValue().equals(PermissionsUpdate.ALL_ROLES)) {
+          if (dMap.getKey().getValue().equals(PermissionsUpdate.ALL_PRIVS)) {
             // Remove all privileges
             perms.delPrivilegeInfo(pUpdate.getAuthzObj());
             perms.removeParentChildMappings(pUpdate.getAuthzObj());

http://git-wip-us.apache.org/repos/asf/sentry/blob/4e473e9d/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
----------------------------------------------------------------------
diff --git 
a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
index 0f3c162..0c934c9 100644
--- 
a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
+++ 
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
@@ -457,9 +457,13 @@ public class SentryPlugin implements 
SentryPolicyStorePlugin, SigUtils.SigListen
         + failure.toString(), failure);
       throw new SentryPluginException(failure.getMessage(), failure);
     }
+
+    // The value of TPrivilegePrincipal being PermissionsUpdate.ALL_PRIVS 
indicates that all privileges
+    // associated with this authorizable should be deleted, including both 
role and user, i.e.,
+    // the key value of TPrivilegePrincipalType.ROLE is ignored.
     update.addPrivilegeUpdate(authzObj).putToDelPrivileges(
-            new 
TPrivilegePrincipal(TPrivilegePrincipalType.ROLE,PermissionsUpdate.ALL_ROLES),
-            PermissionsUpdate.ALL_ROLES);
+            new 
TPrivilegePrincipal(TPrivilegePrincipalType.ROLE,PermissionsUpdate.ALL_PRIVS),
+            PermissionsUpdate.ALL_PRIVS);
 
     LOGGER.debug("onDropSentryPrivilege, Authz Perm preUpdate [ {} ]", 
authzObj);
     if (LOGGER.isTraceEnabled()) {

http://git-wip-us.apache.org/repos/asf/sentry/blob/4e473e9d/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
----------------------------------------------------------------------
diff --git 
a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
 
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
index 7b7d0e1..ab262d0 100644
--- 
a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
+++ 
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
@@ -131,9 +131,13 @@ final class NotificationProcessor {
       throws SentryInvalidInputException {
     PermissionsUpdate update = new 
PermissionsUpdate(SentryConstants.INIT_CHANGE_ID, false);
     String authzObj = SentryServiceUtil.getAuthzObj(authorizable);
+
+    // The value of TPrivilegePrincipal being PermissionsUpdate.ALL_PRIVS 
indicates that all privileges
+    // associated with this authorizable should be deleted, including both 
role and user, i.e.,
+    // the key value of TPrivilegePrincipalType.ROLE is ignored.
     update.addPrivilegeUpdate(authzObj)
-        .putToDelPrivileges(new 
TPrivilegePrincipal(TPrivilegePrincipalType.ROLE, PermissionsUpdate.ALL_ROLES),
-                PermissionsUpdate.ALL_ROLES);
+        .putToDelPrivileges(new 
TPrivilegePrincipal(TPrivilegePrincipalType.ROLE, PermissionsUpdate.ALL_PRIVS),
+                PermissionsUpdate.ALL_PRIVS);
     return update;
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/4e473e9d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
----------------------------------------------------------------------
diff --git 
a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
 
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
index 29d2256..55cbb91 100644
--- 
a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
+++ 
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
@@ -34,7 +34,9 @@ import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
+import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsAction;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hive.service.cli.HiveSQLException;
 import org.apache.sentry.tests.e2e.hdfs.TestHDFSIntegrationBase;
 import org.apache.sentry.tests.e2e.hive.StaticUserGroup;
@@ -882,10 +884,68 @@ public class TestOwnerPrivileges extends 
TestHDFSIntegrationBase {
         + " (under_col int comment 'the under column', value string)");
     statementUSER1_1.execute("DROP TABLE " + DB1 + "." + tableName1);
 
+    // verify privileges created for new table is gone
+    verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, 
SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
+        DB1, tableName1, 0);
+
+    verifyHdfsAcl(Lists.newArrayList(USER1_1), null, DB1, tableName1, null, 
false);
+
+    statementAdmin.close();
+    connection.close();
+  }
+
+  /**
+   * Verify that the user who creases external table and then drops it has no 
owner privilege on this table
+   * and makes sure that HDFS ACLs are updated accordingly.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testDropExternalTable() throws Throwable {
+    dbNames = new String[]{DB1};
+    String uriAllRole = "all_uri_role";
+    roles = new String[]{"admin_role", "create_db1", uriAllRole};
+    String externalPath = "'file:///tmp/external/p1'";
+    String externalPathWithoutQuote = "/tmp/external/p1";
+
+    // create the external path
+    FsPermission pathPermission = new FsPermission((short) 0777);
+    miniDFS.getFileSystem().mkdir(new Path("/tmp/external/p1"), 
pathPermission);
+
+    // create required roles
+    setupUserRoles(roles, statementAdmin);
+
+    // create test DB
+    statementAdmin.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+    statementAdmin.execute("CREATE DATABASE " + DB1);
+
+    // setup privileges for USER1
+    statementAdmin.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE 
create_db1");
+    statementAdmin.execute("GRANT ALL ON URI " + externalPath + " TO ROLE " + 
uriAllRole);
+
+    // USER1 create table
+    Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, 
USER1_1);
+    Statement statementUSER1_1 = connectionUSER1_1.createStatement();
+    statementUSER1_1.execute("CREATE EXTERNAL TABLE " + DB1 + "." + tableName1
+        + " (s string) partitioned by (month int) location " + externalPath);
+
     // verify privileges created for new table
     verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, 
SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
+        DB1, tableName1, 1);
+
+    // verify ACL is not created for new table exists for USER1_1 because the 
path is outside of sentry managed directory and
+    // Update of URI permission for HDFS is not created
+    verifyHdfsAcl(Lists.newArrayList(USER1_1), null, null, null, 
externalPathWithoutQuote, false);
+
+    statementUSER1_1.execute("DROP TABLE " + DB1 + "." + tableName1);
+
+    // verify privileges created for new table is gone
+    verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, 
SentryPrincipalType.USER, Lists.newArrayList(USER1_1),
         DB1, tableName1, 0);
 
+    statementUSER1_1.close();
+    connectionUSER1_1.close();
+
     statementAdmin.close();
     connection.close();
   }

Reply via email to