KYLIN-4714 Failed to revoke role access of the project
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java
index aa0d549..eaf108e 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java
@@ -306,8 +306,8 @@
updater.update(record);
try {
- crud.save(record);
- return acl; // here we are done
+ AclRecord newRecord = crud.save(record);
+ return new MutableAclRecord(newRecord); // here we are done
} catch (WriteConflictException ise) {
if (retry <= 0) {
diff --git a/server/src/test/java/org/apache/kylin/rest/controller/AccessControllerTest.java b/server/src/test/java/org/apache/kylin/rest/controller/AccessControllerTest.java
index d0f4c66..60f5eb2 100644
--- a/server/src/test/java/org/apache/kylin/rest/controller/AccessControllerTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/controller/AccessControllerTest.java
@@ -154,7 +154,6 @@
accessRequest.setSid("ADMIN");
aes = accessController.revoke(CUBE_INSTANCE, "a24ca905-1fc6-4f67-985c-38fa5aeafd92", accessRequest);
assertEquals(0, aes.size());
-
}
@Test
@@ -264,4 +263,109 @@
AccessRequest groupAccessRequest = getAccessRequest(sid, permission, false);
accessController.grant(PROJECT_INSTANCE, uuid, groupAccessRequest);
}
+
+ @Test
+ public void testIndexInAclOfResponse() throws IOException {
+ swichToAdmin();
+ List<ProjectInstance> projects = projectController.getProjects(10000, 0);
+ assertTrue(projects.size() > 0);
+ ProjectInstance project = projects.get(0);
+ ManagedUser user = new ManagedUser("user_0", "kylin", false, "all_users");
+ userService.createUser(user);
+ user = new ManagedUser("user_1", "kylin", false, "all_users");
+ userService.createUser(user);
+ user = new ManagedUser("user_2", "kylin", false, "all_users");
+ userService.createUser(user);
+ List<AccessEntryResponse> aes = accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid());
+ assertEquals(0, aes.size());
+
+ AccessRequest accessRequest = getAccessRequest("user_0", ADMINISTRATION, true);
+ aes = accessController.grant(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(1, aes.size());
+
+ int aeId = 0;
+ for (AccessEntryResponse ae : aes) {
+ aeId = (Integer) ae.getId();
+ }
+ accessRequest = new AccessRequest();
+ accessRequest.setAccessEntryId(aeId);
+ accessRequest.setPermission(READ);
+
+ aes = accessController.update(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(1, aes.size());
+
+ accessRequest = getAccessRequest("user_1", ADMINISTRATION, true);
+ aes = accessController.grant(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(2, aes.size());
+
+ accessRequest = getAccessRequest("user_2", ADMINISTRATION, true);
+ aes = accessController.grant(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(3, aes.size());
+
+
+ accessRequest = new AccessRequest();
+ accessRequest.setAccessEntryId(1);
+ accessRequest.setSid("user_1");
+ accessRequest.setPrincipal(true);
+ aes = accessController.revoke(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(2, aes.size());
+
+ accessRequest = getAccessRequest("user_1", ADMINISTRATION, true);
+ aes = accessController.grant(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(3, aes.size());
+
+ accessRequest = new AccessRequest();
+ accessRequest.setAccessEntryId(1);
+ accessRequest.setSid("user_1");
+ accessRequest.setPrincipal(true);
+ aes = accessController.revoke(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(2, aes.size());
+
+ accessRequest = new AccessRequest();
+ accessRequest.setAccessEntryId(0);
+ accessRequest.setSid("user_0");
+ accessRequest.setPrincipal(true);
+ aes = accessController.revoke(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(1, aes.size());
+
+ accessRequest = new AccessRequest();
+ accessRequest.setAccessEntryId(0);
+ accessRequest.setSid("user_2");
+ accessRequest.setPrincipal(true);
+ aes = accessController.revoke(PROJECT_INSTANCE, project.getUuid(), accessRequest);
+ assertTrue(checkAccessEntryResponse(aes, accessController.getAccessEntities(PROJECT_INSTANCE,
+ project.getUuid())));
+ assertEquals(0, aes.size());
+ }
+
+ private boolean checkAccessEntryResponse(List<AccessEntryResponse> left, List<AccessEntryResponse> right) {
+ for (int i = 0; i < left.size(); i++) {
+ AccessEntryResponse leftAer = left.get(i);
+ AccessEntryResponse rightAer = right.get(i);
+ if (!(leftAer.getId().equals(rightAer.getId())
+ && leftAer.getPermission().getMask() == rightAer.getPermission().getMask()
+ && leftAer.getSid().equals(rightAer.getSid())
+ && leftAer.isGranting() == rightAer.isGranting())) {
+ return false;
+ }
+ }
+ return true;
+ }
}
diff --git a/server/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java
index b21abcc..17c7278 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java
@@ -214,4 +214,57 @@
accessService.revokeProjectPermission("ANALYST", MetadataConstants.TYPE_USER);
Assert.assertEquals(0, accessService.getAcl(ae).getEntries().size());
}
+
+ @Test
+ public void testIndexInAclRecord() throws IOException {
+ List<ProjectInstance> projects = projectService.listProjects(10000, 0);
+ assertTrue(projects.size() > 0);
+ ProjectInstance project = projects.get(0);
+ RootPersistentEntity ae = accessService.getAclEntity(PROJECT_INSTANCE, project.getUuid());
+ final Map<Sid, Permission> sidToPerm = new HashMap<>();
+ for (int i = 0; i < 3; i++) {
+ sidToPerm.put(new PrincipalSid("user_" + i), AclPermission.ADMINISTRATION);
+ }
+ accessService.batchGrant(ae, sidToPerm);
+ Assert.assertEquals(3, accessService.getAcl(ae).getEntries().size());
+
+ // check revoke
+ MutableAclRecord newRecord = accessService.revoke(ae, 1);
+ Assert.assertTrue(checkIndexInAclRecord(newRecord, accessService.getAcl(ae)));
+ Assert.assertEquals(2, accessService.getAcl(ae).getEntries().size());
+
+ // check grant
+ PrincipalSid sid = new PrincipalSid("user_1");
+ newRecord = accessService.grant(ae, AclPermission.ADMINISTRATION, sid);
+ Assert.assertTrue(checkIndexInAclRecord(newRecord, accessService.getAcl(ae)));
+ Assert.assertEquals(3, accessService.getAcl(ae).getEntries().size());
+
+ // check update
+ newRecord = accessService.update(ae, 2, AclPermission.OPERATION);
+ Assert.assertTrue(checkIndexInAclRecord(newRecord, accessService.getAcl(ae)));
+ Assert.assertEquals(3, accessService.getAcl(ae).getEntries().size());
+
+ newRecord = accessService.revoke(ae, 0);
+ Assert.assertTrue(checkIndexInAclRecord(newRecord, accessService.getAcl(ae)));
+ Assert.assertEquals(2, accessService.getAcl(ae).getEntries().size());
+
+ newRecord = accessService.revoke(ae, 0);
+ Assert.assertTrue(checkIndexInAclRecord(newRecord, accessService.getAcl(ae)));
+ Assert.assertEquals(1, accessService.getAcl(ae).getEntries().size());
+
+ newRecord = accessService.revoke(ae, 0);
+ Assert.assertTrue(checkIndexInAclRecord(newRecord, accessService.getAcl(ae)));
+ Assert.assertEquals(0, accessService.getAcl(ae).getEntries().size());
+ }
+
+ private boolean checkIndexInAclRecord(MutableAclRecord left, MutableAclRecord right) {
+ for (int i = 0; i < left.getEntries().size(); i++) {
+ AccessControlEntry leftAce = left.getEntries().get(i);
+ AccessControlEntry rightAce = right.getEntries().get(i);
+ if (!(leftAce.equals(rightAce) && leftAce.getId().equals(rightAce.getId()))) {
+ return false;
+ }
+ }
+ return true;
+ }
}
