You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by li...@apache.org on 2018/06/06 20:19:07 UTC
sentry git commit: SENTRY-2245: Remove privileges that do not
associate with a role or a user (Na Li, reviewed by Sergio Pena,
Kalyan Kumar Kalvagadda)
Repository: sentry
Updated Branches:
refs/heads/master 360e9c304 -> 315a8913a
SENTRY-2245: Remove privileges that do not associate with a role or a user (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/315a8913
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/315a8913
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/315a8913
Branch: refs/heads/master
Commit: 315a8913aa9dfc5fad7a9863c766267fd6f7877b
Parents: 360e9c3
Author: lina.li <li...@cloudera.com>
Authored: Wed Jun 6 15:18:31 2018 -0500
Committer: lina.li <li...@cloudera.com>
Committed: Wed Jun 6 15:18:31 2018 -0500
----------------------------------------------------------------------
.../db/service/persistent/SentryStore.java | 67 +++++++-------------
.../db/service/persistent/TestSentryStore.java | 10 +++
2 files changed, 34 insertions(+), 43 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/sentry/blob/315a8913/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 7bf3e80..e6b71b5 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
@@ -843,11 +843,11 @@ public class SentryStore {
findMatchPrivilege(mRole.getPrivileges(), convertToMSentryPrivilege(tNotAll));
if (mSelect != null) {
mSelect.removeRole(mRole);
- pm.makePersistent(mSelect);
+ persistPrivilege(pm, mSelect);
}
if (mInsert != null) {
mInsert.removeRole(mRole);
- pm.makePersistent(mInsert);
+ persistPrivilege(pm, mInsert);
}
} else {
// If Grant is for Either INSERT/SELECT and ALL already exists..
@@ -1072,11 +1072,11 @@ public class SentryStore {
findMatchPrivilege(mUser.getPrivileges(), convertToMSentryPrivilege(tNotAll));
if (mSelect != null) {
mSelect.removeUser(mUser);
- pm.makePersistent(mSelect);
+ persistPrivilege(pm, mSelect);
}
if (mInsert != null) {
mInsert.removeUser(mUser);
- pm.makePersistent(mInsert);
+ persistPrivilege(pm, mInsert);
}
} else {
// If Grant is for Either INSERT/SELECT and ALL already exists..
@@ -1421,11 +1421,7 @@ public class SentryStore {
persistedPriv.removeUser(mUser);
}
- if (isPrivilegeStall(persistedPriv)) {
- pm.deletePersistent(persistedPriv);
- } else {
- pm.makePersistent(persistedPriv);
- }
+ persistPrivilege(pm, persistedPriv);
}
} else {
@@ -1457,7 +1453,16 @@ public class SentryStore {
return false;
}
- private boolean isPrivilegeStall(MSentryPrivilege privilege) {
+ private void persistPrivilege(PersistenceManager pm, MSentryPrivilege privilege) {
+ if (isPrivilegeStale(privilege)) {
+ pm.deletePersistent(privilege);
+ return;
+ }
+
+ pm.makePersistent(privilege);
+ }
+
+ private boolean isPrivilegeStale(MSentryPrivilege privilege) {
if (privilege.getUsers().isEmpty() && privilege.getRoles().isEmpty()) {
return true;
}
@@ -1465,7 +1470,7 @@ public class SentryStore {
return false;
}
- private boolean isPrivilegeStall(MSentryGMPrivilege privilege) {
+ private boolean isPrivilegeStale(MSentryGMPrivilege privilege) {
if (privilege.getRoles().isEmpty()) {
return true;
}
@@ -1481,21 +1486,13 @@ public class SentryStore {
persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(persistedPriv), pm);
if (persistedPriv != null && !persistedPriv.getRoles().isEmpty()) {
persistedPriv.removeRole(mRole);
- if (isPrivilegeStall(persistedPriv)) {
- pm.deletePersistent(persistedPriv);
- } else {
- pm.makePersistent(persistedPriv);
- }
+ persistPrivilege(pm, persistedPriv);
}
currentPrivilege.setAction(AccessConstants.ALL);
persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(currentPrivilege), pm);
if (persistedPriv != null && mRole.getPrivileges().contains(persistedPriv)) {
persistedPriv.removeRole(mRole);
- if (isPrivilegeStall(persistedPriv)) {
- pm.deletePersistent(persistedPriv);
- } else {
- pm.makePersistent(persistedPriv);
- }
+ persistPrivilege(pm, persistedPriv);
// add decomposted actions
for (String addAction : addActions) {
@@ -1520,21 +1517,13 @@ public class SentryStore {
persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(persistedPriv), pm);
if (persistedPriv != null && !persistedPriv.getUsers().isEmpty()) {
persistedPriv.removeUser(mUser);
- if (isPrivilegeStall(persistedPriv)) {
- pm.deletePersistent(persistedPriv);
- } else {
- pm.makePersistent(persistedPriv);
- }
+ persistPrivilege(pm, persistedPriv);
}
currentPrivilege.setAction(AccessConstants.ALL);
persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(currentPrivilege), pm);
if (persistedPriv != null && mUser.getPrivileges().contains(persistedPriv)) {
persistedPriv.removeUser(mUser);
- if (isPrivilegeStall(persistedPriv)) {
- pm.deletePersistent(persistedPriv);
- } else {
- pm.makePersistent(persistedPriv);
- }
+ persistPrivilege(pm, persistedPriv);
// add decomposted actions
for (String addAction : addActions) {
@@ -1567,11 +1556,7 @@ public class SentryStore {
MSentryPrivilege persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(mPrivilege), pm);
if (persistedPriv != null && !persistedPriv.getRoles().isEmpty()) {
persistedPriv.removeRole(mRole);
- if (isPrivilegeStall(persistedPriv)) {
- pm.deletePersistent(persistedPriv);
- } else {
- pm.makePersistent(persistedPriv);
- }
+ persistPrivilege(pm, persistedPriv);
}
}
}
@@ -1592,11 +1577,7 @@ public class SentryStore {
MSentryPrivilege persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(mPrivilege), pm);
if (persistedPriv != null && !persistedPriv.getUsers().isEmpty()) {
persistedPriv.removeUser(mUser);
- if (isPrivilegeStall(persistedPriv)) {
- pm.deletePersistent(persistedPriv);
- } else {
- pm.makePersistent(persistedPriv);
- }
+ persistPrivilege(pm, persistedPriv);
}
}
}
@@ -1873,7 +1854,7 @@ public class SentryStore {
private void removeStaledPrivileges(PersistenceManager pm, List<MSentryPrivilege> privilegesCopy) {
List<MSentryPrivilege> stalePrivileges = new ArrayList<>(0);
for (MSentryPrivilege privilege : privilegesCopy) {
- if (isPrivilegeStall(privilege)) {
+ if (isPrivilegeStale(privilege)) {
stalePrivileges.add(privilege);
}
}
@@ -1885,7 +1866,7 @@ public class SentryStore {
private void removeStaledGMPrivileges(PersistenceManager pm, List<MSentryGMPrivilege> privilegesCopy) {
List<MSentryGMPrivilege> stalePrivileges = new ArrayList<>(0);
for (MSentryGMPrivilege privilege : privilegesCopy) {
- if (isPrivilegeStall(privilege)) {
+ if (isPrivilegeStale(privilege)) {
stalePrivileges.add(privilege);
}
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/315a8913/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 95dabec..12c6d91 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
@@ -3978,6 +3978,8 @@ public class TestSentryStore extends org.junit.Assert {
// second round
privilege.setAction(AccessConstants.ALL);
sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName, privilege);
+ List<MSentryPrivilege> totalPrivileges = sentryStore.getAllMSentryPrivileges();
+ assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
role = sentryStore.getMSentryRoleByName(roleName);
privileges = role.getPrivileges();
assertEquals(privileges.toString(), 1, privileges.size());
@@ -3985,6 +3987,8 @@ public class TestSentryStore extends org.junit.Assert {
privilege.setAction(AccessConstants.INSERT);
sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName, privilege);
// after having ALL and revoking INSERT, we should have (SELECT)
+ totalPrivileges = sentryStore.getAllMSentryPrivileges();
+ assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
role = sentryStore.getMSentryRoleByName(roleName);
privileges = role.getPrivileges();
assertEquals(privileges.toString(), 1, privileges.size());
@@ -4023,6 +4027,9 @@ public class TestSentryStore extends org.junit.Assert {
// second round
privilege.setAction(AccessConstants.ALL);
sentryStore.alterSentryUserGrantPrivilege(grantor, userName, privilege);
+ List<MSentryPrivilege> totalPrivileges = sentryStore.getAllMSentryPrivileges();
+ assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
+
user = sentryStore.getMSentryUserByName(userName);
privileges = user.getPrivileges();
assertEquals(privileges.toString(), 1, privileges.size());
@@ -4030,6 +4037,9 @@ public class TestSentryStore extends org.junit.Assert {
privilege.setAction(AccessConstants.INSERT);
sentryStore.alterSentryUserRevokePrivilege(grantor, userName, privilege);
// after having ALL and revoking INSERT, we should have (SELECT)
+ totalPrivileges = sentryStore.getAllMSentryPrivileges();
+ assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
+
user = sentryStore.getMSentryUserByName(userName);
privileges = user.getPrivileges();
assertEquals(privileges.toString(), 1, privileges.size());