Repository: sentry Updated Branches: refs/heads/master 10217aab5 -> 360e9c304
SENTRY-2258: Remove user when it is not associated with other objects (Na Li, reviewed by Sergio Pena, 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/360e9c30 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/360e9c30 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/360e9c30 Branch: refs/heads/master Commit: 360e9c304cf67beb63a02f9b33417730456f04a5 Parents: 10217aa Author: lina.li <[email protected]> Authored: Wed Jun 6 14:51:39 2018 -0500 Committer: lina.li <[email protected]> Committed: Wed Jun 6 14:51:39 2018 -0500 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 39 ++++++++++++++++++-- .../db/service/persistent/TestSentryStore.java | 20 +++++++--- 2 files changed, 49 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/360e9c30/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 b0ed2ed..7bf3e80 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 @@ -1241,7 +1241,22 @@ public class SentryStore { for (MSentryPrivilege childPriv : privilegeGraph) { revokePrivilegeFromUser(pm, tPrivilege, mUser, childPriv); } - pm.makePersistent(mUser); + + persistUser(pm, mUser); + } + + /** + * If user is stale, delete it from database. Otherwise, persist the user + * @param pm persistence manager + * @param user user to persist + */ + private void persistUser(PersistenceManager pm, MSentryUser user) { + if (isUserStale(user)) { + pm.deletePersistent(user); + return; + } + + pm.makePersistent(user); } /** @@ -1434,6 +1449,14 @@ public class SentryStore { } } + private boolean isUserStale(MSentryUser user) { + if (user.getPrivileges().isEmpty() && user.getRoles().isEmpty()) { + return true; + } + + return false; + } + private boolean isPrivilegeStall(MSentryPrivilege privilege) { if (privilege.getUsers().isEmpty() && privilege.getRoles().isEmpty()) { return true; @@ -1983,16 +2006,24 @@ public class SentryStore { Query query = pm.newQuery(MSentryUser.class); query.setFilter("this.userName == :userName"); query.setUnique(true); - List<MSentryUser> users = Lists.newArrayList(); + List<MSentryUser> usersToSave = Lists.newArrayList(); + List<MSentryUser> usersToDelete = Lists.newArrayList(); for (String userName : userNames) { userName = userName.trim(); MSentryUser user = (MSentryUser) query.execute(userName); if (user != null) { user.removeRole(role); - users.add(user); + + if (isUserStale(user)) { + usersToDelete.add(user); + } else { + usersToSave.add(user); + } } } - pm.makePersistentAll(users); + + pm.deletePersistentAll(usersToDelete); + pm.makePersistentAll(usersToSave); } return null; }); http://git-wip-us.apache.org/repos/asf/sentry/blob/360e9c30/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 e2d24e5..95dabec 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 @@ -238,7 +238,13 @@ public class TestSentryStore extends org.junit.Assert { sentryStore.alterSentryRoleAddGroups(grantor, roleName, groups); sentryStore.alterSentryRoleDeleteGroups(roleName, groups); sentryStore.alterSentryRoleAddUsers(roleName, users); + MSentryUser user = sentryStore.getMSentryUserByName(users.iterator().next()); + assertNotNull(user); + sentryStore.alterSentryRoleDeleteUsers(roleName, users); + user = sentryStore.getMSentryUserByName(users.iterator().next(), false); + assertNull(user); + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName, privilege); sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName, privilege); } @@ -3873,11 +3879,8 @@ public class TestSentryStore extends org.junit.Assert { privilege.setAction(AccessConstants.INSERT); sentryStore.alterSentryUserRevokePrivilege(grantor, userName, privilege); - user = sentryStore.getMSentryUserByName(userName); - privileges = user.getPrivileges(); - assertEquals(privileges.toString(), 0, privileges.size()); - - sentryStore.dropSentryUser(userName); + user = sentryStore.getMSentryUserByName(userName, false); + assertNull(user); } /** @@ -4031,6 +4034,11 @@ public class TestSentryStore extends org.junit.Assert { privileges = user.getPrivileges(); assertEquals(privileges.toString(), 1, privileges.size()); - sentryStore.dropSentryUser(userName); + privilege.setAction(AccessConstants.SELECT); + sentryStore.alterSentryUserRevokePrivilege(grantor, userName, privilege); + // after having ALL and revoking INSERT and SELECT, we should have NO privileges + // user should be removed automatically + user = sentryStore.getMSentryUserByName(userName, false); + assertNull(user); } }
