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(); }