SENTRY-693: The generic model has not successfully revoke part of privileges from existed ALL privilege (Guoquan Shen, reviewed by Colin Ma)
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
index dab7d74..daeefdf 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
@@ -34,6 +34,7 @@
import org.apache.sentry.core.model.search.SearchActionFactory;
import org.apache.sentry.provider.db.generic.service.persistent.PrivilegeObject.Builder;
import org.apache.sentry.provider.db.service.model.MSentryGMPrivilege;
+import org.apache.sentry.provider.db.service.model.MSentryPrivilege;
import org.apache.sentry.provider.db.service.model.MSentryRole;
import com.google.common.base.Joiner;
@@ -144,6 +145,8 @@
MSentryGMPrivilege mPrivilege = getPrivilege(convertToPrivilege(privilege), pm);
if (mPrivilege == null) {
mPrivilege = convertToPrivilege(privilege);
+ } else {
+ mPrivilege = (MSentryGMPrivilege) pm.detachCopy(mPrivilege);
}
Set<MSentryGMPrivilege> privilegeGraph = Sets.newHashSet();
@@ -161,10 +164,9 @@
* privilege.removeRole(role) and pm.makePersistent(privilege)
* will remove other roles that shouldn't been removed
*/
- pm.retrieve(persistedPriv);
-
revokeRolePartial(mPrivilege, persistedPriv, role, pm);
}
+ pm.makePersistent(role);
}
/**
@@ -234,10 +236,16 @@
/**
* grant the left privileges to role
*/
- MSentryGMPrivilege leftPriv = new MSentryGMPrivilege(persistedPriv);
- leftPriv.setAction(ac.getValue());
- leftPriv.appendRole(role);
- pm.makePersistent(leftPriv);
+ MSentryGMPrivilege tmpPriv = new MSentryGMPrivilege(persistedPriv);
+ tmpPriv.setAction(ac.getValue());
+ MSentryGMPrivilege leftPersistedPriv = getPrivilege(tmpPriv, pm);
+ if (leftPersistedPriv == null) {
+ //leftPersistedPriv isn't exist
+ leftPersistedPriv = tmpPriv;
+ role.appendGMPrivilege(leftPersistedPriv);
+ }
+ leftPersistedPriv.appendRole(role);
+ pm.makePersistent(leftPersistedPriv);
}
}
} else if (revokeaction.implies(persistedAction)) {
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
index 8893391..189eabb 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
@@ -497,7 +497,7 @@
String grantor = ADMIN_USER;
PrivilegeObject allPrivilege = new Builder()
.setComponent(SEARCH)
- .setAction(SearchConstants.QUERY)
+ .setAction(SearchConstants.ALL)
.setService(SERVICE)
.setAuthorizables(Arrays.asList(new Collection(COLLECTION_NAME), new Field(FIELD_NAME)))
.build();
@@ -525,6 +525,58 @@
sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName)));
}
+ /**
+ * Grant update, query and all privilege to role r1
+ * Revoke query privilege from role r1
+ * there is update privilege related to role r1
+ */
+ @Test
+ public void testRevokePrivilegeWithAllPrivilegesGranted() throws Exception {
+ String roleName = "r1";
+ /**
+ * grantor is admin, there is no need to check grant option
+ */
+ String grantor = ADMIN_USER;
+ PrivilegeObject allPrivilege = new Builder()
+ .setComponent(SEARCH)
+ .setAction(SearchConstants.ALL)
+ .setService(SERVICE)
+ .setAuthorizables(Arrays.asList(new Collection(COLLECTION_NAME), new Field(FIELD_NAME)))
+ .build();
+
+ PrivilegeObject updatePrivilege = new Builder(allPrivilege)
+ .setAction(SearchConstants.UPDATE)
+ .build();
+
+ PrivilegeObject queryPrivilege = new Builder(allPrivilege)
+ .setAction(SearchConstants.QUERY)
+ .build();
+
+ sentryStore.createRole(SEARCH, roleName, grantor);
+ //grant query to role r1
+ sentryStore.alterRoleGrantPrivilege(SEARCH, roleName, queryPrivilege, grantor);
+ assertEquals(Sets.newHashSet(queryPrivilege),
+ sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName)));
+
+ //grant update to role r1
+ sentryStore.alterRoleGrantPrivilege(SEARCH, roleName, updatePrivilege, grantor);
+ assertEquals(Sets.newHashSet(queryPrivilege, updatePrivilege),
+ sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName)));
+ /**
+ * grant all action privilege to role r1, because all action includes query and update action,
+ * The role r1 only has the action all privilege
+ */
+ sentryStore.alterRoleGrantPrivilege(SEARCH, roleName, allPrivilege, grantor);
+ assertEquals(Sets.newHashSet(allPrivilege),
+ sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName)));
+ /**
+ * revoke update privilege from role r1, the query privilege has been left
+ */
+ sentryStore.alterRoleRevokePrivilege(SEARCH, roleName, updatePrivilege, grantor);
+ assertEquals(Sets.newHashSet(queryPrivilege),
+ sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName)));
+ }
+
@Test
public void testRevokeParentPrivilegeWithChildsExist() throws Exception {
String roleName = "r1";
