Repository: sentry Updated Branches: refs/heads/master 4d9bc9cea -> 3278c714c
SENTRY-2251: Update user privileges based on changes to authorizables (Kalyan Kumar Kalvagadda reviewed by Sergio Pena and Arjun Mishra) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/3278c714 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/3278c714 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/3278c714 Branch: refs/heads/master Commit: 3278c714c490d505b84b798bb9b77732f7f7ef7f Parents: 4d9bc9c Author: Kalyan Kumar Kalvagadda <kkal...@cloudera.com> Authored: Fri Jun 22 15:17:55 2018 -0500 Committer: Kalyan Kumar Kalvagadda <kkal...@cloudera.com> Committed: Fri Jun 22 15:17:55 2018 -0500 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 78 ++++++++++---------- .../db/service/persistent/TestSentryStore.java | 42 ++++++++++- 2 files changed, 80 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/3278c714/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 27b8876..aca5c2d 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 @@ -1154,11 +1154,10 @@ public class SentryStore { } if (requestedPrivToRevoke.getAction().equalsIgnoreCase(AccessConstants.ALL) || - requestedPrivToRevoke.getAction().equalsIgnoreCase(AccessConstants.ACTION_ALL)) { - if (!persistedPriv.getRoles().isEmpty()) { - if (mEntity != null) { - persistedPriv.removeEntity(mEntity); - } + requestedPrivToRevoke.getAction().equalsIgnoreCase(AccessConstants.ACTION_ALL)) { + if ((!persistedPriv.getRoles().isEmpty() || !persistedPriv.getUsers().isEmpty()) && + mEntity != null) { + persistedPriv.removeEntity(mEntity); persistPrivilege(pm, persistedPriv); } } else { @@ -2614,7 +2613,7 @@ public class SentryStore { } /** - * Drop the given privilege from all roles. + * Drop the given privilege from all entities. * * @param tAuthorizable the given authorizable object. * @throws Exception @@ -2624,17 +2623,17 @@ public class SentryStore { pm -> { pm.setDetachAllOnCommit(false); // No need to detach objects - // Drop the give privilege for all possible actions from all roles. + // Drop the give privilege for all possible actions from all entities. TSentryPrivilege tPrivilege = toSentryPrivilege(tAuthorizable); try { if (isMultiActionsSupported(tPrivilege)) { for (String privilegeAction : ALL_ACTIONS) { tPrivilege.setAction(privilegeAction); - dropPrivilegeForAllRoles(pm, new TSentryPrivilege(tPrivilege)); + dropPrivilegeForAllEntities(pm, new TSentryPrivilege(tPrivilege)); } } else { - dropPrivilegeForAllRoles(pm, new TSentryPrivilege(tPrivilege)); + dropPrivilegeForAllEntities(pm, new TSentryPrivilege(tPrivilege)); } } catch (JDODataStoreException e) { throw new SentryInvalidInputException("Failed to get privileges: " @@ -2645,7 +2644,7 @@ public class SentryStore { } /** - * Drop the given privilege from all roles. As well as persist the corresponding + * Drop the given privilege from all entities. As well as persist the corresponding * permission change to MSentryPermChange table in a single transaction. * * @param tAuthorizable the given authorizable object. @@ -2657,17 +2656,17 @@ public class SentryStore { execute(update, pm -> { pm.setDetachAllOnCommit(false); // No need to detach objects - // Drop the give privilege for all possible actions from all roles. + // Drop the give privilege for all possible actions from all entities. TSentryPrivilege tPrivilege = toSentryPrivilege(tAuthorizable); try { if (isMultiActionsSupported(tPrivilege)) { for (String privilegeAction : ALL_ACTIONS) { tPrivilege.setAction(privilegeAction); - dropPrivilegeForAllRoles(pm, new TSentryPrivilege(tPrivilege)); + dropPrivilegeForAllEntities(pm, new TSentryPrivilege(tPrivilege)); } } else { - dropPrivilegeForAllRoles(pm, new TSentryPrivilege(tPrivilege)); + dropPrivilegeForAllEntities(pm, new TSentryPrivilege(tPrivilege)); } } catch (JDODataStoreException e) { throw new SentryInvalidInputException("Failed to get privileges: " @@ -2750,7 +2749,7 @@ public class SentryStore { } /** - * Rename the privilege for all roles. Drop the old privilege name and create the new one. + * Rename the privilege for all entities. Drop the old privilege name and create the new one. * * @param oldTAuthorizable the old authorizable name needs to be renamed. * @param newTAuthorizable the new authorizable name @@ -2763,7 +2762,7 @@ public class SentryStore { pm -> { pm.setDetachAllOnCommit(false); // No need to detach objects - // Drop the give privilege for all possible actions from all roles. + // Drop the give privilege for all possible actions from all entities. TSentryPrivilege tPrivilege = toSentryPrivilege(oldTAuthorizable); TSentryPrivilege newPrivilege = toSentryPrivilege(newTAuthorizable); @@ -2773,10 +2772,10 @@ public class SentryStore { for (String privilegeAction : ALL_ACTIONS) { tPrivilege.setAction(privilegeAction); newPrivilege.setAction(privilegeAction); - renamePrivilegeForAllRoles(pm, tPrivilege, newPrivilege); + renamePrivilegeForAllEntities(pm, tPrivilege, newPrivilege); } } else { - renamePrivilegeForAllRoles(pm, tPrivilege, newPrivilege); + renamePrivilegeForAllEntities(pm, tPrivilege, newPrivilege); } } catch (JDODataStoreException e) { throw new SentryInvalidInputException("Failed to get privileges: " @@ -2787,7 +2786,7 @@ public class SentryStore { } /** - * Rename the privilege for all roles. Drop the old privilege name and create the new one. + * Rename the privilege for all entities. Drop the old privilege name and create the new one. * As well as persist the corresponding permission change to MSentryPermChange table in a * single transaction. * @@ -2804,7 +2803,7 @@ public class SentryStore { execute(update, pm -> { pm.setDetachAllOnCommit(false); // No need to detach objects - // Drop the give privilege for all possible actions from all roles. + // Drop the give privilege for all possible actions from all entities. TSentryPrivilege tPrivilege = toSentryPrivilege(oldTAuthorizable); TSentryPrivilege newPrivilege = toSentryPrivilege(newTAuthorizable); @@ -2814,10 +2813,10 @@ public class SentryStore { for (String privilegeAction : ALL_ACTIONS) { tPrivilege.setAction(privilegeAction); newPrivilege.setAction(privilegeAction); - renamePrivilegeForAllRoles(pm, tPrivilege, newPrivilege); + renamePrivilegeForAllEntities(pm, tPrivilege, newPrivilege); } } else { - renamePrivilegeForAllRoles(pm, tPrivilege, newPrivilege); + renamePrivilegeForAllEntities(pm, tPrivilege, newPrivilege); } } catch (JDODataStoreException e) { throw new SentryInvalidInputException("Failed to get privileges: " @@ -2833,45 +2832,46 @@ public class SentryStore { } // wrapper for dropOrRename - private void renamePrivilegeForAllRoles(PersistenceManager pm, + private void renamePrivilegeForAllEntities(PersistenceManager pm, TSentryPrivilege tPrivilege, TSentryPrivilege newPrivilege) throws SentryNoSuchObjectException, SentryInvalidInputException { - dropOrRenamePrivilegeForAllRoles(pm, tPrivilege, newPrivilege); + dropOrRenamePrivilegeForAllEntities(pm, tPrivilege, newPrivilege); } /** - * Drop given privilege from all roles + * Drop given privilege from all entities * @param tPrivilege * @throws SentryNoSuchObjectException * @throws SentryInvalidInputException */ - private void dropPrivilegeForAllRoles(PersistenceManager pm, + private void dropPrivilegeForAllEntities(PersistenceManager pm, TSentryPrivilege tPrivilege) throws SentryNoSuchObjectException, SentryInvalidInputException { - dropOrRenamePrivilegeForAllRoles(pm, tPrivilege, null); + dropOrRenamePrivilegeForAllEntities(pm, tPrivilege, null); } /** - * Drop given privilege from all roles Create the new privilege if asked + * Drop given privilege from all entities Create the new privilege if asked * @param tPrivilege * @param pm * @throws SentryNoSuchObjectException * @throws SentryInvalidInputException */ - private void dropOrRenamePrivilegeForAllRoles(PersistenceManager pm, + private void dropOrRenamePrivilegeForAllEntities(PersistenceManager pm, TSentryPrivilege tPrivilege, TSentryPrivilege newTPrivilege) throws SentryNoSuchObjectException, SentryInvalidInputException { - Collection<MSentryRole> roleSet = new HashSet<>(); + Collection<PrivilegeEntity> entitySet = new HashSet<>(); List<MSentryPrivilege> mPrivileges = getMSentryPrivileges(tPrivilege, pm); for (MSentryPrivilege mPrivilege : mPrivileges) { - roleSet.addAll(ImmutableSet.copyOf(mPrivilege.getRoles())); + entitySet.addAll(ImmutableSet.copyOf(mPrivilege.getRoles())); + entitySet.addAll(ImmutableSet.copyOf(mPrivilege.getUsers())); } // Dropping the privilege if (newTPrivilege == null) { - for (MSentryRole role : roleSet) { - alterSentryRevokePrivilegeCore(pm, SentryEntityType.ROLE, role.getRoleName(), tPrivilege); + for (PrivilegeEntity entity : entitySet) { + alterSentryRevokePrivilegeCore(pm, entity.getType(), entity.getEntityName(), tPrivilege); } return; } @@ -2885,18 +2885,22 @@ public class SentryStore { // dereferenced. If object has to be used even after that it should have been detached. parent = pm.detachCopy(parent); } - for (MSentryRole role : roleSet) { + for (PrivilegeEntity entity : entitySet) { + // When all the privilege associated for a user are revoked, user will be removed from the database. + // JDO object should be not used when the associated database entry is removed. Application should use + // a detached copy instead. + PrivilegeEntity detachedEntity = pm.detachCopy(entity); // 1. get privilege and child privileges Collection<MSentryPrivilege> privilegeGraph = new HashSet<>(); if (parent != null) { privilegeGraph.add(parent); - populateChildren(pm, SentryEntityType.ROLE, Sets.newHashSet(role.getRoleName()), parent, privilegeGraph); + populateChildren(pm, detachedEntity.getType(), Sets.newHashSet(detachedEntity.getEntityName()), parent, privilegeGraph); } else { - populateChildren(pm, SentryEntityType.ROLE, Sets.newHashSet(role.getRoleName()), convertToMSentryPrivilege(tPrivilege), + populateChildren(pm, detachedEntity.getType(), Sets.newHashSet(detachedEntity.getEntityName()), convertToMSentryPrivilege(tPrivilege), privilegeGraph); } // 2. revoke privilege and child privileges - alterSentryRevokePrivilegeCore(pm, SentryEntityType.ROLE, role.getRoleName(), tPrivilege); + alterSentryRevokePrivilegeCore(pm, detachedEntity.getType(), detachedEntity.getEntityName(), tPrivilege); // 3. add new privilege and child privileges with new tableName for (MSentryPrivilege mPriv : privilegeGraph) { TSentryPrivilege tPriv = convertToTSentryPrivilege(mPriv); @@ -2907,7 +2911,7 @@ public class SentryStore { tPriv.setDbName(newTPrivilege.getDbName()); tPriv.setTableName(newTPrivilege.getTableName()); } - alterSentryGrantPrivilegeCore(pm, SentryEntityType.ROLE, role.getRoleName(), tPriv); + alterSentryGrantPrivilegeCore(pm, detachedEntity.getType(), detachedEntity.getEntityName(), tPriv); } } } http://git-wip-us.apache.org/repos/asf/sentry/blob/3278c714/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java index 51048bc..475fc4e 100644 --- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java +++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java @@ -1905,17 +1905,20 @@ public class TestSentryStore extends org.junit.Assert { } /** - * Assign multiple table and SERVER privileges to roles - * drop privilege for the object verify that it's removed correctl + * Assign multiple table and SERVER privileges to roles and users + * drop privilege for the object verify that it's removed correctly * @throws Exception */ @Test public void testDropDbObject() throws Exception { String roleName1 = "list-privs-r1", roleName2 = "list-privs-r2", roleName3 = "list-privs-r3"; + String userName1 = "user-1", userName2 = "user-2"; String grantor = "g1"; sentryStore.createSentryRole(roleName1); sentryStore.createSentryRole(roleName2); sentryStore.createSentryRole(roleName3); + sentryStore.createSentryUser(userName1); + sentryStore.createSentryUser(userName2); TSentryPrivilege privilege_tbl1 = new TSentryPrivilege(); privilege_tbl1.setPrivilegeScope("TABLE"); @@ -1959,6 +1962,10 @@ public class TestSentryStore extends org.junit.Assert { sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.ROLE, roleName3, privilege3_1, null); sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.ROLE, roleName3, privilege3_2, null); + sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.USER, userName1, privilege1, null); + sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.USER, userName2, privilege2_3, null); + + sentryStore.dropPrivilege(toTSentryAuthorizable(privilege_tbl1)); assertEquals(0, sentryStore.getAllTSentryPrivilegesByRoleName(roleName1) .size()); @@ -1967,6 +1974,15 @@ public class TestSentryStore extends org.junit.Assert { assertEquals(1, sentryStore.getAllTSentryPrivilegesByRoleName(roleName3) .size()); + + try { + sentryStore.getAllTSentryPrivilegesByUserName(userName1); + fail("Should have received an exception"); + } catch (Exception e) { + + } + assertEquals(1, sentryStore.getAllTSentryPrivilegesByUserName(userName2) + .size()); sentryStore.dropPrivilege(toTSentryAuthorizable(privilege_tbl2)); assertEquals(0, sentryStore.getAllTSentryPrivilegesByRoleName(roleName1) .size()); @@ -1974,6 +1990,11 @@ public class TestSentryStore extends org.junit.Assert { .size()); assertEquals(0, sentryStore.getAllTSentryPrivilegesByRoleName(roleName3) .size()); + try { + sentryStore.getAllTSentryPrivilegesByUserName(userName2); + fail("Should have received an exception"); + } catch (Exception e) { + } } /** @@ -2320,19 +2341,23 @@ public class TestSentryStore extends org.junit.Assert { } /*** - * Create roles and assign privileges for same table rename the privileges for + * Create roles and users and assign privileges for same table rename the privileges for * the table and verify the new privileges * @throws Exception */ @Test public void testRenameTable() throws Exception { String roleName1 = "role1", roleName2 = "role2", roleName3 = "role3"; + String userName1 = "user-1", userName2 = "user-2", userName3 = "user-3"; String grantor = "g1"; String table1 = "tbl1", table2 = "tbl2"; sentryStore.createSentryRole(roleName1); sentryStore.createSentryRole(roleName2); sentryStore.createSentryRole(roleName3); + sentryStore.createSentryUser(userName1); + sentryStore.createSentryUser(userName2); + sentryStore.createSentryUser(userName3); TSentryPrivilege privilege_tbl1 = new TSentryPrivilege(); privilege_tbl1.setPrivilegeScope("TABLE"); @@ -2355,6 +2380,9 @@ public class TestSentryStore extends org.junit.Assert { sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.ROLE, roleName1, privilege_tbl1_insert, null); sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.ROLE, roleName2, privilege_tbl1_select, null); sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.ROLE, roleName3, privilege_tbl1_all, null); + sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.USER, userName1, privilege_tbl1_insert, null); + sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.USER, userName2, privilege_tbl1_select, null); + sentryStore.alterSentryGrantPrivilege(grantor, SentryEntityType.USER, userName3, privilege_tbl1_all, null); TSentryAuthorizable oldTable = toTSentryAuthorizable(privilege_tbl1); TSentryAuthorizable newTable = toTSentryAuthorizable(privilege_tbl1); @@ -2369,6 +2397,14 @@ public class TestSentryStore extends org.junit.Assert { assertTrue(table2.equalsIgnoreCase(privilege.getTableName())); } } + for (String userName : Sets.newHashSet(userName1, userName2, userName3)) { + Set<TSentryPrivilege> privilegeSet = sentryStore + .getAllTSentryPrivilegesByUserName(userName); + assertEquals(1, privilegeSet.size()); + for (TSentryPrivilege privilege : privilegeSet) { + assertTrue(table2.equalsIgnoreCase(privilege.getTableName())); + } + } } /**
