Repository: sentry Updated Branches: refs/heads/master 8028cef7e -> f255c3421
SENTRY-2359: Object owner is unable to grant privileges: SentryAccessDeniedException (Kalyan Kumar Kalvagadda reviewed by Lina li) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/f255c342 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/f255c342 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/f255c342 Branch: refs/heads/master Commit: f255c34218a0a07d46ecab3166e5c996dc07960d Parents: 8028cef Author: Kalyan Kumar Kalvagadda <kkal...@cloudera.com> Authored: Wed Aug 22 11:57:59 2018 -0500 Committer: Kalyan Kumar Kalvagadda <kkal...@cloudera.com> Committed: Wed Aug 22 11:57:59 2018 -0500 ---------------------------------------------------------------------- .../db/service/model/MSentryPrivilege.java | 1 + .../db/service/persistent/SentryStore.java | 38 +++++++++----- .../TestOwnerPrivilegesWithGrantOption.java | 54 ++++++++++++++++++++ 3 files changed, 81 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/f255c342/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java index 1decef2..eba40d2 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java @@ -341,6 +341,7 @@ public class MSentryPrivilege { // check action implies if (!action.equalsIgnoreCase(AccessConstants.ALL) + && !action.equalsIgnoreCase(AccessConstants.OWNER) && !action.equalsIgnoreCase(other.action) && !action.equalsIgnoreCase(AccessConstants.ACTION_ALL)) { return false; http://git-wip-us.apache.org/repos/asf/sentry/blob/f255c342/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 0ef6a20..80c5270 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -2012,6 +2012,12 @@ public class SentryStore implements SentryStoreInterface { return mSentryUser.getPrivileges(); } + private Set<MSentryPrivilege> getMSentryPrivilegesByUserNameIfExists(String userName) + throws Exception { + MSentryUser mSentryUser = getMSentryUserByName(userName, false); + return mSentryUser != null ? mSentryUser.getPrivileges() : Collections.emptySet(); + } + /** * Gets sentry privilege objects for a given userName from the persistence layer * @param userName : userName to look up @@ -2999,7 +3005,7 @@ public class SentryStore implements SentryStoreInterface { */ private void grantOptionCheck(PersistenceManager pm, String grantorPrincipal, TSentryPrivilege privilege) - throws SentryUserException { + throws Exception { MSentryPrivilege mPrivilege = convertToMSentryPrivilege(privilege); if (grantorPrincipal == null) { throw new SentryInvalidInputException("grantorPrincipal should not be null"); @@ -3021,20 +3027,28 @@ public class SentryStore implements SentryStoreInterface { if (!isAdminGroup) { boolean hasGrant = false; - // get all privileges for group and user + Set<MSentryPrivilege> privilegeSet = new HashSet<>(); + Set<MSentryPrivilege> enityPrivilegeSet = null; + // Collect the privileges granted to all roles to that user. Set<MSentryRole> roles = getRolesForGroups(pm, groups); roles.addAll(getRolesForUsers(pm, Sets.newHashSet(grantorPrincipal))); for (MSentryRole role : roles) { - Set<MSentryPrivilege> privilegeSet = role.getPrivileges(); - if (privilegeSet != null && !privilegeSet.isEmpty()) { - // if role has a privilege p with grant option - // and mPrivilege is a child privilege of p - for (MSentryPrivilege p : privilegeSet) { - if (p.getGrantOption() && p.implies(mPrivilege)) { - hasGrant = true; - break; - } - } + enityPrivilegeSet = role.getPrivileges(); + if(enityPrivilegeSet != null && !enityPrivilegeSet.isEmpty()) { + privilegeSet.addAll(enityPrivilegeSet); + } + } + // Collect the privileges granted to user + enityPrivilegeSet = getMSentryPrivilegesByUserNameIfExists(grantorPrincipal.trim()); + if(enityPrivilegeSet != null && !enityPrivilegeSet.isEmpty()) { + privilegeSet.addAll(enityPrivilegeSet); + + } + // Compare the privileges that user has with the privilege he/she is trying to grant. + for (MSentryPrivilege p : privilegeSet) { + if (p.getGrantOption() && p.implies(mPrivilege)) { + hasGrant = true; + break; } } http://git-wip-us.apache.org/repos/asf/sentry/blob/f255c342/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivilegesWithGrantOption.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivilegesWithGrantOption.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivilegesWithGrantOption.java index b3d98cb..c2ccb24 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivilegesWithGrantOption.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivilegesWithGrantOption.java @@ -18,13 +18,17 @@ package org.apache.sentry.tests.e2e.dbprovider; import com.google.common.collect.Lists; + import java.sql.Connection; import java.sql.Statement; + import org.apache.sentry.service.common.ServiceConstants.SentryPrincipalType; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; + public class TestOwnerPrivilegesWithGrantOption extends TestOwnerPrivileges { @BeforeClass public static void setup() throws Exception { @@ -97,4 +101,54 @@ public class TestOwnerPrivilegesWithGrantOption extends TestOwnerPrivileges { } } + @Test + public void testPermissionGrantToRoleByOwner() throws Exception { + String ownerRole = "owner_role"; + String newOwnerRole = "new_owner_role"; + dbNames = new String[]{DB1}; + roles = new String[]{"admin_role", ownerRole}; + + // create required roles, and assign them to USERGROUP1 + setupUserRoles(roles, statement); + + // create test DB + statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE"); + statement.execute("CREATE DATABASE " + DB1); + + statement.execute("CREATE ROLE " + newOwnerRole); + statement.execute("GRANT ROLE " + newOwnerRole + " to GROUP " + USERGROUP2); + + // setup privileges for USER1 + statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE " + ownerRole); + statement.execute("USE " + DB1); + + // USER1_1 create table + Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1); + Statement statementUSER1_1 = connectionUSER1_1.createStatement(); + statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1 + + " (under_col int comment 'the under column')"); + + // Verify that the user who created the table has owner privilege on the table created. + verifyTableOwnerPrivilegeExistForPrincipal(statement, SentryPrincipalType.USER, + Lists.newArrayList(USER1_1), + DB1, tableName1, 1); + + // Owner granting privileges to another user + try { + statementUSER1_1 + .execute("GRANT ALL ON " + DB1 + "." + tableName1 + " TO ROLE " + newOwnerRole); + } catch (Exception ex) { + Assert.fail("Exception received while granting permissions"); + } + + // Making sure that user who is granted all permissions can drop the table. + Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1); + Statement statementUSER2_1 = connectionUSER2_1.createStatement(); + try { + statementUSER2_1 + .execute("DROP TABLE " + DB1 + "." + tableName1 ); + } catch (Exception ex) { + Assert.fail("Exception received while dropping the table"); + } + } }