You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by bi...@apache.org on 2018/03/06 17:53:19 UTC

[kylin] branch master updated (1e7825b -> 56e7333)

This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git.


    from 1e7825b  bump to 2.4.0-SNAPSHOT
     new 62bfc5f  Refactor ACL, AclService, AccessService
     new 4de8bfc  KYLIN-3220, add manager for project ACL.
     new 94a1978  minor refactor about ACL.
     new 5807cdf  minor, check user/group exists when grant access.
     new 7c9892b  KYLIN-3246, add manager for user.
     new fde86c2  KYLIN-3248, add batch grant API for project ACL.
     new 56e7333  minor, add ut for ValidateUtilTest.

The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../kylin/common/persistence/JsonSerializer.java   |  11 +-
 .../kylin/common/persistence/ResourceStore.java    |   1 +
 .../apache/kylin/metadata/acl/TableACLManager.java |   2 +-
 .../kylin/metadata/cachesync/CachedCrudAssist.java |   6 +-
 .../localmeta/acl/legacy-test-domain-object        |  31 ++
 .../kylin/rest/controller/AccessController.java    |  39 +-
 ...eAclController.java => TableACLController.java} |  22 +-
 .../apache/kylin/rest/request/AccessRequest.java   |   6 +-
 .../kylin/rest/security/KylinUserManager.java      | 141 ++++++++
 .../apache/kylin/rest/security/ManagedUser.java    |   5 +
 .../kylin/rest/security/springacl/AceImpl.java     | 184 ++++++++++
 .../kylin/rest/security/springacl/AclRecord.java   | 281 +++++++++++++++
 .../springacl/LegacyAceInfo.java}                  |  26 +-
 .../rest/security/springacl/MutableAclRecord.java  | 123 +++++++
 .../security/springacl/ObjectIdentityImpl.java     | 134 +++++++
 .../{service => security/springacl}/SidInfo.java   |  31 +-
 .../apache/kylin/rest/service/AccessService.java   | 289 +++++++--------
 .../org/apache/kylin/rest/service/AclService.java  | 393 +++++++++------------
 .../kylin/rest/service/AclTableMigrationTool.java  |  30 +-
 .../kylin/rest/service/DomainObjectInfo.java       |  52 ---
 .../kylin/rest/service/KylinUserService.java       |  53 ++-
 .../apache/kylin/rest/service/QueryService.java    | 146 ++------
 .../org/apache/kylin/rest/util/ValidateUtil.java   |  32 +-
 .../apache/kylin/rest/util/ValidateUtilTest.java   |  36 --
 .../rest/controller/AccessControllerTest.java      |  21 +-
 .../kylin/rest/security/KylinUserManagerTest.java  |  51 +++
 .../kylin/rest/service/AccessServiceTest.java      |  57 ++-
 .../apache/kylin/rest/service/AclServiceTest.java  | 193 ++++++++++
 .../apache/kylin/rest/service/UserServiceTest.java |   4 -
 .../org/apache/kylin/rest/util/AclUtilTest.java    |   4 +-
 .../apache/kylin/rest/util/ValidateUtilTest.java   | 134 +++++++
 31 files changed, 1820 insertions(+), 718 deletions(-)
 create mode 100644 examples/test_case_data/localmeta/acl/legacy-test-domain-object
 rename server-base/src/main/java/org/apache/kylin/rest/controller/{TableAclController.java => TableACLController.java} (85%)
 create mode 100644 server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java
 create mode 100644 server-base/src/main/java/org/apache/kylin/rest/security/springacl/AceImpl.java
 create mode 100644 server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java
 rename server-base/src/main/java/org/apache/kylin/rest/{service/AceInfo.java => security/springacl/LegacyAceInfo.java} (68%)
 create mode 100644 server-base/src/main/java/org/apache/kylin/rest/security/springacl/MutableAclRecord.java
 create mode 100644 server-base/src/main/java/org/apache/kylin/rest/security/springacl/ObjectIdentityImpl.java
 rename server-base/src/main/java/org/apache/kylin/rest/{service => security/springacl}/SidInfo.java (67%)
 delete mode 100644 server-base/src/main/java/org/apache/kylin/rest/service/DomainObjectInfo.java
 delete mode 100644 server-base/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java
 create mode 100644 server/src/test/java/org/apache/kylin/rest/security/KylinUserManagerTest.java
 create mode 100644 server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
 create mode 100644 server/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.

[kylin] 04/07: minor, check user/group exists when grant access.

Posted by bi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 5807cdf70f8e8215566fcb20ae3b900f20f8ed27
Author: Jiatao Tao <24...@qq.com>
AuthorDate: Tue Feb 6 19:38:31 2018 +0800

    minor, check user/group exists when grant access.
---
 .../org/apache/kylin/rest/controller/AccessController.java | 14 +++++++++++---
 .../main/java/org/apache/kylin/rest/util/ValidateUtil.java |  2 +-
 .../apache/kylin/rest/controller/AccessControllerTest.java | 11 ++++++++---
 .../test/java/org/apache/kylin/rest/util/AclUtilTest.java  |  4 +++-
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java
index 7935f77..56cae10 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java
@@ -36,6 +36,7 @@ import org.apache.kylin.rest.service.ProjectService;
 import org.apache.kylin.rest.service.TableACLService;
 import org.apache.kylin.rest.service.UserService;
 import org.apache.kylin.rest.util.AclPermissionUtil;
+import org.apache.kylin.rest.util.ValidateUtil;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -75,7 +76,10 @@ public class AccessController extends BasicController implements InitializingBea
     @Qualifier("userService")
     private UserService userService;
 
-    
+    @Autowired
+    @Qualifier("validateUtil")
+    private ValidateUtil validateUtil;
+
     @Override
     public void afterPropertiesSet() throws Exception {
         // init ExternalAclProvider
@@ -138,9 +142,13 @@ public class AccessController extends BasicController implements InitializingBea
      */
     @RequestMapping(value = "/{type}/{uuid}", method = { RequestMethod.POST }, produces = { "application/json" })
     @ResponseBody
-    public List<AccessEntryResponse> grant(@PathVariable String type, @PathVariable String uuid, @RequestBody AccessRequest accessRequest) {
+    public List<AccessEntryResponse> grant(@PathVariable String type, @PathVariable String uuid, @RequestBody AccessRequest accessRequest) throws IOException {
+        boolean isPrincipal = accessRequest.isPrincipal();
+        String name = accessRequest.getSid();
+        validateUtil.checkIdentifiersExists(name, isPrincipal);
+
         AclEntity ae = accessService.getAclEntity(type, uuid);
-        Sid sid = accessService.getSid(accessRequest.getSid(), accessRequest.isPrincipal());
+        Sid sid = accessService.getSid(name, isPrincipal);
         Permission permission = AclPermissionFactory.getPermission(accessRequest.getPermission());
         Acl acl = accessService.grant(ae, permission, sid);
 
diff --git a/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java b/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java
index c250da7..1d56a71 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java
@@ -89,7 +89,7 @@ public class ValidateUtil {
     public void validateIdentifiers(String prj, String name, String type) throws IOException {
         Set<String> allIdentifiers = getAllIdentifiersInPrj(prj, type);
         if (!allIdentifiers.contains(name)) {
-            throw new RuntimeException("Operation failed, identifiers:" + name + " not exists");
+            throw new RuntimeException("Operation failed, " + type + ":" + name + " not exists in project.");
         }
     }
 
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 217b54c..dea37f5 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
@@ -34,6 +34,7 @@ import org.apache.kylin.rest.security.AclEntityType;
 import org.apache.kylin.rest.security.AclPermissionType;
 import org.apache.kylin.rest.security.ManagedUser;
 import org.apache.kylin.rest.service.CubeService;
+import org.apache.kylin.rest.service.IUserGroupService;
 import org.apache.kylin.rest.service.ProjectService;
 import org.apache.kylin.rest.service.ServiceTestBase;
 import org.apache.kylin.rest.service.UserService;
@@ -78,6 +79,10 @@ public class AccessControllerTest extends ServiceTestBase implements AclEntityTy
     @Qualifier("userService")
     UserService userService;
 
+    @Autowired
+    @Qualifier("userGroupService")
+    private IUserGroupService userGroupService;
+
     @Before
     public void setup() throws Exception {
         super.setup();
@@ -88,11 +93,11 @@ public class AccessControllerTest extends ServiceTestBase implements AclEntityTy
     }
 
     @Test
-    public void testGetUserPermissionInPrj() {
+    public void testGetUserPermissionInPrj() throws IOException {
         List<ProjectInstance> projects = projectController.getProjects(10000, 0);
         assertTrue(projects.size() > 0);
         ProjectInstance project = projects.get(0);
-        ManagedUser user = new ManagedUser("u", "kylin", false, "all_users");
+        ManagedUser user = new ManagedUser("u", "kylin", false, "all_users", "g1", "g2", "g3", "g4");
         userService.createUser(user);
 
         grantPermission("g1", READ, project.getUuid());
@@ -249,7 +254,7 @@ public class AccessControllerTest extends ServiceTestBase implements AclEntityTy
         return accessRequest;
     }
 
-    private void grantPermission(String sid, String permission, String uuid) {
+    private void grantPermission(String sid, String permission, String uuid) throws IOException {
         swichToAdmin();
         AccessRequest groupAccessRequest = getAccessRequest(sid, permission, false);
         accessController.grant(PROJECT_INSTANCE, uuid, groupAccessRequest);
diff --git a/server/src/test/java/org/apache/kylin/rest/util/AclUtilTest.java b/server/src/test/java/org/apache/kylin/rest/util/AclUtilTest.java
index b8fbe5f..18e5bf5 100644
--- a/server/src/test/java/org/apache/kylin/rest/util/AclUtilTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/util/AclUtilTest.java
@@ -32,6 +32,8 @@ import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 
+import java.io.IOException;
+
 public class AclUtilTest extends ServiceTestBase {
     @Autowired
     AccessController accessController;
@@ -40,7 +42,7 @@ public class AclUtilTest extends ServiceTestBase {
     AclUtil aclUtil;
 
     @Test
-    public void testBasic() {
+    public void testBasic() throws IOException {
         final String PROJECT = "default";
         final String ANALYST = "ANALYST";
         final String ADMIN = "ADMIN";

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.

[kylin] 02/07: KYLIN-3220, add manager for project ACL.

Posted by bi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 4de8bfc92814410155fa42b9a030bb86986e9652
Author: Jiatao Tao <24...@qq.com>
AuthorDate: Thu Feb 1 11:14:03 2018 +0800

    KYLIN-3220, add manager for project ACL.
    
    KYLIN-3220, add manager for project ACL.
    
    code review minor changes
---
 .../kylin/common/persistence/ResourceStore.java    |   1 +
 .../apache/kylin/metadata/acl/TableACLManager.java |   2 +-
 .../kylin/metadata/cachesync/CachedCrudAssist.java |   6 +-
 .../kylin/rest/security/springacl/AclRecord.java   |   5 +
 .../apache/kylin/rest/service/AccessService.java   |   2 -
 .../org/apache/kylin/rest/service/AclService.java  | 181 ++++++++++++++-------
 .../kylin/rest/service/KylinUserService.java       |  19 +++
 .../org/apache/kylin/rest/service/UserService.java |   2 +
 .../apache/kylin/rest/service/AclServiceTest.java  |   2 +-
 .../apache/kylin/rest/service/UserServiceTest.java |   7 +
 10 files changed, 161 insertions(+), 66 deletions(-)

diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java
index a6b3337..2bccd67 100644
--- a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java
+++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java
@@ -76,6 +76,7 @@ abstract public class ResourceStore {
     public static final String CUBE_STATISTICS_ROOT = "/cube_statistics";
     public static final String BAD_QUERY_RESOURCE_ROOT = "/bad_query";
     public static final String DRAFT_RESOURCE_ROOT = "/draft";
+    public static final String USER_ROOT = "/user";
 
     public static final String METASTORE_UUID_TAG = "/UUID";
 
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java b/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java
index 163d340..9f74dec 100644
--- a/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java
+++ b/core-metadata/src/main/java/org/apache/kylin/metadata/acl/TableACLManager.java
@@ -58,7 +58,7 @@ public class TableACLManager {
         logger.info("Initializing TableACLManager with config " + config);
         this.config = config;
         this.tableACLMap = new CaseInsensitiveStringCache<>(config, "table_acl");
-        this.crud = new CachedCrudAssist<TableACL>(getStore(), "/table_acl", "", TableACL.class, tableACLMap) {
+        this.crud = new CachedCrudAssist<TableACL>(getStore(), "/table_acl", "", TableACL.class, tableACLMap, true) {
             @Override
             protected TableACL initEntityAfterReload(TableACL acl, String resourceName) {
                 acl.init(resourceName);
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java b/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java
index b3c200e..be3d8d4 100644
--- a/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java
+++ b/core-metadata/src/main/java/org/apache/kylin/metadata/cachesync/CachedCrudAssist.java
@@ -51,16 +51,16 @@ abstract public class CachedCrudAssist<T extends RootPersistentEntity> {
 
     public CachedCrudAssist(ResourceStore store, String resourceRootPath, Class<T> entityType,
             SingleValueCache<String, T> cache) {
-        this(store, resourceRootPath, MetadataConstants.FILE_SURFIX, entityType, cache);
+        this(store, resourceRootPath, MetadataConstants.FILE_SURFIX, entityType, cache, false);
     }
 
     public CachedCrudAssist(ResourceStore store, String resourceRootPath, String resourcePathSuffix,
-            Class<T> entityType, SingleValueCache<String, T> cache) {
+            Class<T> entityType, SingleValueCache<String, T> cache, boolean compact) {
         this.store = store;
         this.entityType = entityType;
         this.resRootPath = resourceRootPath;
         this.resPathSuffix = resourcePathSuffix;
-        this.serializer = new JsonSerializer<T>(entityType);
+        this.serializer = new JsonSerializer<T>(entityType, compact);
         this.cache = cache;
 
         this.checkCopyOnWrite = store.getConfig().isCheckCopyOnWrite();
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java
index 3fff632..eb5b792 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java
@@ -105,6 +105,11 @@ public class AclRecord extends RootPersistentEntity implements Acl, OwnershipAcl
         }
     }
 
+    @Override
+    public String resourceName() {
+        return String.valueOf(domainObjectInfo.getIdentifier());
+    }
+
     public SidInfo getOwnerInfo() {
         return ownerInfo;
     }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
index 74a87c8..09a89c8 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
@@ -66,8 +66,6 @@ import org.springframework.transaction.annotation.Transactional;
 
 import com.google.common.base.Preconditions;
 
-/**
- */
 @Component("accessService")
 public class AccessService {
     @SuppressWarnings("unused")
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 73a6fb2..adc7c30 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
@@ -21,14 +21,22 @@ package org.apache.kylin.rest.service;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.annotation.Nullable;
+
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.persistence.JsonSerializer;
 import org.apache.kylin.common.persistence.ResourceStore;
 import org.apache.kylin.common.persistence.Serializer;
+import org.apache.kylin.common.util.AutoReadWriteLock;
+import org.apache.kylin.common.util.AutoReadWriteLock.AutoLock;
+import org.apache.kylin.metadata.cachesync.Broadcaster;
+import org.apache.kylin.metadata.cachesync.CachedCrudAssist;
+import org.apache.kylin.metadata.cachesync.CaseInsensitiveStringCache;
 import org.apache.kylin.rest.exception.BadRequestException;
 import org.apache.kylin.rest.exception.InternalErrorException;
 import org.apache.kylin.rest.msg.Message;
@@ -38,6 +46,7 @@ import org.apache.kylin.rest.security.springacl.MutableAclRecord;
 import org.apache.kylin.rest.security.springacl.ObjectIdentityImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.acls.domain.PermissionFactory;
 import org.springframework.security.acls.domain.PrincipalSid;
@@ -51,12 +60,11 @@ import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.Permission;
 import org.springframework.security.acls.model.PermissionGrantingStrategy;
 import org.springframework.security.acls.model.Sid;
-import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Component;
 
 @Component("aclService")
-public class AclService implements MutableAclService {
+public class AclService implements MutableAclService, InitializingBean {
     private static final Logger logger = LoggerFactory.getLogger(AclService.class);
 
     public static final String DIR_PREFIX = "/acl/";
@@ -69,35 +77,67 @@ public class AclService implements MutableAclService {
 
     @Autowired
     protected PermissionFactory aclPermissionFactory;
+    // cache
+    private CaseInsensitiveStringCache<AclRecord> aclMap;
+    private CachedCrudAssist<AclRecord> crud;
+    private AutoReadWriteLock lock = new AutoReadWriteLock();
 
-    //    @Autowired
-    //    protected AclAuthorizationStrategy aclAuthorizationStrategy;
+    public AclService() throws IOException {
+        KylinConfig config = KylinConfig.getInstanceFromEnv();
+        ResourceStore aclStore = ResourceStore.getStore(config);
+        this.aclMap = new CaseInsensitiveStringCache<>(config, "acl");
+        this.crud = new CachedCrudAssist<AclRecord>(aclStore, "/acl", "", AclRecord.class, aclMap, true) {
+            @Override
+            protected AclRecord initEntityAfterReload(AclRecord acl, String resourceName) {
+                acl.init(null, aclPermissionFactory, permissionGrantingStrategy);
+                return acl;
+            }
+        };
+        crud.reloadAll();
+    }
 
-    //    @Autowired
-    //    protected AuditLogger auditLogger;
+    @Override
+    public void afterPropertiesSet() throws Exception {
+        Broadcaster.getInstance(KylinConfig.getInstanceFromEnv()).registerStaticListener(new AclRecordSyncListener(), "acl");
+    }
 
-    protected ResourceStore aclStore;
+    private class AclRecordSyncListener extends Broadcaster.Listener {
 
-    public AclService() throws IOException {
-        aclStore = ResourceStore.getStore(KylinConfig.getInstanceFromEnv());
+        @Override
+        public void onEntityChange(Broadcaster broadcaster, String entity, Broadcaster.Event event, String cacheKey)
+                throws IOException {
+            try (AutoLock l = lock.lockForWrite()) {
+                if (event == Broadcaster.Event.DROP)
+                    aclMap.removeLocal(cacheKey);
+                else
+                    crud.reloadQuietly(cacheKey);
+            }
+            broadcaster.notifyProjectACLUpdate(cacheKey);
+        }
+
+        @Override
+        public void onClearAll(Broadcaster broadcaster) throws IOException {
+            try (AutoLock l = lock.lockForWrite()) {
+                aclMap.clear();
+            }
+        }
     }
 
     @Override
     public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
-        List<ObjectIdentity> oids = new ArrayList<ObjectIdentity>();
-        try {
-            List<AclRecord> allAclRecords = aclStore.getAllResources(DIR_PREFIX, AclRecord.class, SERIALIZER);
-            for (AclRecord record : allAclRecords) {
-                ObjectIdentityImpl parent = record.getParentDomainObjectInfo();
-                if (parent != null && parent.equals(parentIdentity)) {
-                    ObjectIdentityImpl child = record.getDomainObjectInfo();
-                    oids.add(child);
-                }
+        List<ObjectIdentity> oids = new ArrayList<>();
+        Collection<AclRecord> allAclRecords;
+        try (AutoLock l = lock.lockForRead()) {
+            allAclRecords = new ArrayList<>(aclMap.values());
+        }
+        for (AclRecord record : allAclRecords) {
+            ObjectIdentityImpl parent = record.getParentDomainObjectInfo();
+            if (parent != null && parent.equals(parentIdentity)) {
+                ObjectIdentityImpl child = record.getDomainObjectInfo();
+                oids.add(child);
             }
-            return oids;
-        } catch (IOException e) {
-            throw new InternalErrorException(e);
         }
+        return oids;
     }
 
     public MutableAclRecord readAcl(ObjectIdentity oid) throws NotFoundException {
@@ -128,40 +168,33 @@ public class AclService implements MutableAclService {
     @Override
     public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> oids, List<Sid> sids) throws NotFoundException {
         Map<ObjectIdentity, Acl> aclMaps = new HashMap<>();
-        try {
-            for (ObjectIdentity oid : oids) {
-                AclRecord record = aclStore.getResource(resourceKey(oid), AclRecord.class, SERIALIZER);
-                if (record == null) {
-                    Message msg = MsgPicker.getMsg();
-                    throw new NotFoundException(String.format(msg.getACL_INFO_NOT_FOUND(), oid));
-                }
+        for (ObjectIdentity oid : oids) {
+            AclRecord record = getAclRecordByCache(objID(oid));
+            if (record == null) {
+                Message msg = MsgPicker.getMsg();
+                throw new NotFoundException(String.format(msg.getACL_INFO_NOT_FOUND(), oid));
+            }
 
-                Acl parentAcl = null;
-                if (record.isEntriesInheriting() && record.getParentDomainObjectInfo() != null)
-                    parentAcl = readAclById(record.getParentDomainObjectInfo());
+            Acl parentAcl = null;
+            if (record.isEntriesInheriting() && record.getParentDomainObjectInfo() != null)
+                parentAcl = readAclById(record.getParentDomainObjectInfo());
 
-                record.init(parentAcl, aclPermissionFactory, permissionGrantingStrategy);
+            record.init(parentAcl, aclPermissionFactory, permissionGrantingStrategy);
 
-                aclMaps.put(oid, new MutableAclRecord(record));
-            }
-            return aclMaps;
-        } catch (IOException e) {
-            throw new InternalErrorException(e);
+            aclMaps.put(oid, new MutableAclRecord(record));
         }
+        return aclMaps;
     }
 
     @Override
     public MutableAcl createAcl(ObjectIdentity objectIdentity) throws AlreadyExistsException {
-        try {
-            if (aclStore.exists(resourceKey(objectIdentity))) {
+        try (AutoLock l = lock.lockForWrite()) {
+            AclRecord aclRecord = getAclRecordByCache(objID(objectIdentity));
+            if (aclRecord != null) {
                 throw new AlreadyExistsException("ACL of " + objectIdentity + " exists!");
             }
-
-            Authentication auth = SecurityContextHolder.getContext().getAuthentication();
-            PrincipalSid owner = new PrincipalSid(auth);
-            AclRecord record = new AclRecord(objectIdentity, owner);
-            record.init(null, aclPermissionFactory, permissionGrantingStrategy);
-            aclStore.putResource(resourceKey(objectIdentity), record, 0, SERIALIZER);
+            AclRecord record = newPrjACL(objectIdentity);
+            crud.save(record);
             logger.debug("ACL of " + objectIdentity + " created successfully.");
         } catch (IOException e) {
             throw new InternalErrorException(e);
@@ -171,7 +204,7 @@ public class AclService implements MutableAclService {
 
     @Override
     public void deleteAcl(ObjectIdentity objectIdentity, boolean deleteChildren) throws ChildrenExistException {
-        try {
+        try (AutoLock l = lock.lockForWrite()) {
             List<ObjectIdentity> children = findChildren(objectIdentity);
             if (!deleteChildren && children.size() > 0) {
                 Message msg = MsgPicker.getMsg();
@@ -180,7 +213,7 @@ public class AclService implements MutableAclService {
             for (ObjectIdentity oid : children) {
                 deleteAcl(oid, deleteChildren);
             }
-            aclStore.deleteResource(resourceKey(String.valueOf(objectIdentity.getIdentifier())));
+            crud.delete(objID(objectIdentity));
             logger.debug("ACL of " + objectIdentity + " deleted successfully.");
         } catch (IOException e) {
             throw new InternalErrorException(e);
@@ -190,10 +223,9 @@ public class AclService implements MutableAclService {
     // Try use the updateAclWithRetry() method family whenever possible
     @Override
     public MutableAcl updateAcl(MutableAcl mutableAcl) throws NotFoundException {
-        try {
+        try (AutoLock l = lock.lockForWrite()) {
             AclRecord record = ((MutableAclRecord) mutableAcl).getAclRecord();
-            String resPath = resourceKey(mutableAcl.getObjectIdentity());
-            aclStore.putResource(resPath, record, System.currentTimeMillis(), SERIALIZER);
+            crud.save(record);
             logger.debug("ACL of " + mutableAcl.getObjectIdentity() + " updated successfully.");
         } catch (IOException e) {
             throw new InternalErrorException(e);
@@ -202,7 +234,7 @@ public class AclService implements MutableAclService {
     }
 
     // a NULL permission means to delete the ace
-    public MutableAclRecord upsertAce(MutableAclRecord acl, final Sid sid, final Permission perm) {
+    MutableAclRecord upsertAce(MutableAclRecord acl, final Sid sid, final Permission perm) {
         return updateAclWithRetry(acl, new AclRecordUpdater() {
             @Override
             public void update(AclRecord record) {
@@ -210,8 +242,8 @@ public class AclService implements MutableAclService {
             }
         });
     }
-    
-    public MutableAclRecord inherit(MutableAclRecord acl, final MutableAclRecord parentAcl) {
+
+    MutableAclRecord inherit(MutableAclRecord acl, final MutableAclRecord parentAcl) {
         return updateAclWithRetry(acl, new AclRecordUpdater() {
             @Override
             public void update(AclRecord record) {
@@ -221,6 +253,33 @@ public class AclService implements MutableAclService {
         });
     }
 
+    @Nullable
+    private AclRecord getAclRecordByCache(String id) {
+        try (AutoLock l = lock.lockForRead()) {
+            if (aclMap.size() > 0) {
+                return aclMap.get(id);
+            }
+        }
+        
+        try (AutoLock l = lock.lockForWrite()) {
+            crud.reloadAll();
+            return aclMap.get(id);
+        } catch (IOException e) {
+            throw new RuntimeException("Can not get ACL record from cache.", e);
+        }
+    }
+
+    private AclRecord newPrjACL(ObjectIdentity objID) {
+        AclRecord acl = new AclRecord(objID, getCurrentSid());
+        acl.init(null, this.aclPermissionFactory, this.permissionGrantingStrategy);
+        acl.updateRandomUuid();
+        return acl;
+    }
+
+    private Sid getCurrentSid() {
+        return new PrincipalSid(SecurityContextHolder.getContext().getAuthentication());
+    }
+
     public interface AclRecordUpdater {
         void update(AclRecord record);
     }
@@ -231,9 +290,8 @@ public class AclService implements MutableAclService {
             AclRecord record = acl.getAclRecord();
 
             updater.update(record);
-            String resPath = resourceKey(record.getObjectIdentity());
             try {
-                aclStore.putResource(resPath, record, System.currentTimeMillis(), SERIALIZER);
+                crud.save(record);
                 return acl; // here we are done
 
             } catch (IllegalStateException ise) {
@@ -242,7 +300,8 @@ public class AclService implements MutableAclService {
                     throw ise;
                 }
 
-                logger.warn("Write conflict to update ACL " + resPath + " retry remaining " + retry + ", will retry...");
+                logger.warn("Write conflict to update ACL " + resourceKey(record.getObjectIdentity())
+                        + " retry remaining " + retry + ", will retry...");
                 acl = readAcl(acl.getObjectIdentity());
 
             } catch (IOException e) {
@@ -252,11 +311,15 @@ public class AclService implements MutableAclService {
         throw new RuntimeException("should not reach here");
     }
 
-    public static String resourceKey(ObjectIdentity domainObjId) {
-        return resourceKey(String.valueOf(domainObjId.getIdentifier()));
+    private static String resourceKey(ObjectIdentity domainObjId) {
+        return resourceKey(objID(domainObjId));
+    }
+
+    private static String objID(ObjectIdentity domainObjId) {
+        return String.valueOf(domainObjId.getIdentifier());
     }
 
-    public static String resourceKey(String domainObjId) {
+    static String resourceKey(String domainObjId) {
         return DIR_PREFIX + domainObjId;
     }
 }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
index 7e8919c..c35d737 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+import javax.annotation.Nullable;
 import javax.annotation.PostConstruct;
 
 import org.apache.kylin.common.KylinConfig;
@@ -39,7 +40,9 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
 
+import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 
 public class KylinUserService implements UserService {
 
@@ -146,6 +149,22 @@ public class KylinUserService implements UserService {
     }
 
     @Override
+    public List<String> listUsernames() throws IOException {
+        List<String> paths = new ArrayList<>();
+        paths.addAll(aclStore.listResources(ResourceStore.USER_ROOT));
+        List<String> users = Lists.transform(paths, new Function<String, String>() {
+            @Nullable
+            @Override
+            public String apply(@Nullable String input) {
+                String[] path = input.split("/");
+                Preconditions.checkArgument(path.length == 3);
+                return path[2];
+            }
+        });
+        return users;
+    }
+
+    @Override
     public List<String> listAdminUsers() throws IOException{
         List<String> adminUsers = new ArrayList<>();
         for (ManagedUser managedUser : listUsers()) {
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
index 21c4cf9..5b50456 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
@@ -32,6 +32,8 @@ public interface UserService extends UserDetailsManager {
 
     List<ManagedUser> listUsers() throws IOException;
 
+    List<String> listUsernames() throws IOException;
+
     List<String> listAdminUsers() throws IOException;
 
     //For performance consideration, list all users may be incomplete(eg. not load user's authorities until authorities has benn used).
diff --git a/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
index ad0d524..875a2a1 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
@@ -109,7 +109,7 @@ public class AclServiceTest extends ServiceTestBase {
         // inherit parent
         childAcl = aclService.inherit(childAcl, parentAcl);
         Assert.assertEquals(parentOid, childAcl.getAclRecord().getParentDomainObjectInfo());
-        Assert.assertEquals(null, childAclOutdated.getAclRecord().getParentDomainObjectInfo());
+        Assert.assertEquals(parentOid, childAclOutdated.getAclRecord().getParentDomainObjectInfo());
         
         // update permission on an outdated ACL, retry should keep things going
         PrincipalSid user1 = new PrincipalSid("user1");
diff --git a/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java
index 6304712..2b3e0f5 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.google.common.collect.Lists;
 import org.apache.kylin.rest.constant.Constant;
 import org.apache.kylin.rest.exception.InternalErrorException;
 import org.apache.kylin.rest.security.ManagedUser;
@@ -63,6 +64,12 @@ public class UserServiceTest extends ServiceTestBase {
 
     }
 
+    @Test
+    public void testGetAllUserNames() throws IOException {
+        List<String> users = userService.listUsernames();
+        List<String> expected = Lists.newArrayList("ADMIN", "ANALYST", "MODELER");
+        Assert.assertEquals(expected, users);
+    }
 
     @Test
     public void testDeleteAdmin() throws IOException {

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.

[kylin] 01/07: Refactor ACL, AclService, AccessService

Posted by bi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 62bfc5f70e9553f0c95fe160dbf63f8d20ec3d52
Author: Li Yang <li...@apache.org>
AuthorDate: Sat Jan 27 22:03:50 2018 +0800

    Refactor ACL, AclService, AccessService
---
 .../kylin/common/persistence/JsonSerializer.java   |  11 +-
 .../localmeta/acl/legacy-test-domain-object        |  31 +++
 .../apache/kylin/rest/request/AccessRequest.java   |   6 +-
 .../kylin/rest/security/springacl/AceImpl.java     | 184 +++++++++++++
 .../kylin/rest/security/springacl/AclRecord.java   | 276 +++++++++++++++++++
 .../springacl/LegacyAceInfo.java}                  |  26 +-
 .../rest/security/springacl/MutableAclRecord.java  | 123 +++++++++
 .../security/springacl/ObjectIdentityImpl.java     | 134 +++++++++
 .../{service => security/springacl}/SidInfo.java   |  31 ++-
 .../apache/kylin/rest/service/AccessService.java   | 263 +++++++-----------
 .../org/apache/kylin/rest/service/AclService.java  | 303 +++++++--------------
 .../kylin/rest/service/AclTableMigrationTool.java  |  30 +-
 .../kylin/rest/service/DomainObjectInfo.java       |  52 ----
 .../rest/controller/AccessControllerTest.java      |  10 +-
 .../kylin/rest/service/AccessServiceTest.java      |  41 ++-
 .../apache/kylin/rest/service/AclServiceTest.java  | 166 +++++++++++
 16 files changed, 1189 insertions(+), 498 deletions(-)

diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/JsonSerializer.java b/core-common/src/main/java/org/apache/kylin/common/persistence/JsonSerializer.java
index 8f0cc51..508b2ab 100644
--- a/core-common/src/main/java/org/apache/kylin/common/persistence/JsonSerializer.java
+++ b/core-common/src/main/java/org/apache/kylin/common/persistence/JsonSerializer.java
@@ -30,11 +30,17 @@ import org.apache.kylin.common.util.JsonUtil;
 public class JsonSerializer<T extends RootPersistentEntity> implements Serializer<T> {
 
     Class<T> clz;
+    boolean compact = false;
 
     public JsonSerializer(Class<T> clz) {
         this.clz = clz;
     }
 
+    public JsonSerializer(Class<T> clz, boolean compact) {
+        this.clz = clz;
+        this.compact = compact;
+    }
+    
     @Override
     public T deserialize(DataInputStream in) throws IOException {
         return JsonUtil.readValue(in, clz);
@@ -42,6 +48,9 @@ public class JsonSerializer<T extends RootPersistentEntity> implements Serialize
 
     @Override
     public void serialize(T obj, DataOutputStream out) throws IOException {
-        JsonUtil.writeValueIndent(out, obj);
+        if (compact)
+            JsonUtil.writeValue(out, obj);
+        else
+            JsonUtil.writeValueIndent(out, obj);
     }
 }
diff --git a/examples/test_case_data/localmeta/acl/legacy-test-domain-object b/examples/test_case_data/localmeta/acl/legacy-test-domain-object
new file mode 100644
index 0000000..46cae53
--- /dev/null
+++ b/examples/test_case_data/localmeta/acl/legacy-test-domain-object
@@ -0,0 +1,31 @@
+{
+  "domainObjectInfo" : {
+    "id" : "legacy-test-domain-object",
+    "type" : "org.apache.kylin.rest.service.AclServiceTest$MockAclEntity"
+  },
+  "parentDomainObjectInfo" : null,
+  "ownerInfo" : {
+    "sid" : "ADMIN",
+    "principal" : true
+  },
+  "entriesInheriting" : true,
+  "allAceInfo" : {
+    "MODELER" : {
+      "sidInfo" : {
+        "sid" : "MODELER",
+        "principal" : true
+      },
+      "permissionMask" : 1
+    },
+    "ADMIN" : {
+      "sidInfo" : {
+        "sid" : "ADMIN",
+        "principal" : true
+      },
+      "permissionMask" : 16
+    }
+  },
+  "uuid" : null,
+  "last_modified" : 1517017738360,
+  "version" : "2.3.0.20505"
+}
\ No newline at end of file
diff --git a/server-base/src/main/java/org/apache/kylin/rest/request/AccessRequest.java b/server-base/src/main/java/org/apache/kylin/rest/request/AccessRequest.java
index 0ff57b2..ba93fa7 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/request/AccessRequest.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/request/AccessRequest.java
@@ -24,7 +24,7 @@ package org.apache.kylin.rest.request;
  */
 public class AccessRequest {
 
-    private Long accessEntryId;
+    private int accessEntryId;
     private String permission;
     private String sid;
     private boolean principal;
@@ -32,11 +32,11 @@ public class AccessRequest {
     public AccessRequest() {
     }
 
-    public Long getAccessEntryId() {
+    public int getAccessEntryId() {
         return accessEntryId;
     }
 
-    public void setAccessEntryId(Long accessEntryId) {
+    public void setAccessEntryId(int accessEntryId) {
         this.accessEntryId = accessEntryId;
     }
 
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AceImpl.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AceImpl.java
new file mode 100644
index 0000000..8976a19
--- /dev/null
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AceImpl.java
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+/**
+ * Mimic org.springframework.security.acls.domain.ObjectIdentityImpl
+ * Make it Jackson friendly.
+ */
+package org.apache.kylin.rest.security.springacl;
+
+import java.io.Serializable;
+import java.util.Comparator;
+
+import org.springframework.security.acls.domain.GrantedAuthoritySid;
+import org.springframework.security.acls.domain.PrincipalSid;
+import org.springframework.security.acls.model.AccessControlEntry;
+import org.springframework.security.acls.model.Acl;
+import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.Sid;
+
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+@SuppressWarnings("serial")
+@JsonAutoDetect(fieldVisibility = Visibility.NONE, getterVisibility = Visibility.NONE, isGetterVisibility = Visibility.NONE, setterVisibility = Visibility.NONE)
+public class AceImpl implements AccessControlEntry {
+
+    public static final Comparator<AceImpl> SID_ORDER = new Comparator<AceImpl>() {
+        @Override
+        public int compare(AceImpl o1, AceImpl o2) {
+            if (o1.sidOfAuthority == null) {
+                return o2.sidOfAuthority == null ? o1.sidOfPrincipal.compareTo(o2.sidOfPrincipal) : 1;
+            } else {
+                return o2.sidOfAuthority == null ? -1 : o1.sidOfAuthority.compareTo(o2.sidOfAuthority);
+            }
+        }
+    };
+
+    // ~ Instance fields
+    // ================================================================================================
+
+    @JsonProperty("p")
+    @JsonInclude(JsonInclude.Include.NON_NULL)
+    private String sidOfPrincipal;
+    @JsonProperty("a")
+    @JsonInclude(JsonInclude.Include.NON_NULL)
+    private String sidOfAuthority;
+    @JsonProperty("m")
+    private int permissionMask;
+
+    // non-persistent fields
+    private AclRecord acl;
+    private int indexInAcl;
+    private Sid sid;
+    private Permission perm;
+
+    // ~ Constructors
+    // ===================================================================================================
+
+    // for Jackson
+    public AceImpl() {
+    }
+
+    public AceImpl(LegacyAceInfo legacy) {
+        this(legacy.getSidInfo(), legacy.getPermissionMask());
+    }
+
+    public AceImpl(Sid sid, Permission perm) {
+        this(new SidInfo(sid), perm == null ? 0 : perm.getMask());
+    }
+
+    public AceImpl(SidInfo sidInfo, int permMask) {
+        if (sidInfo.isPrincipal())
+            sidOfPrincipal = sidInfo.getSid();
+        else
+            sidOfAuthority = sidInfo.getSid();
+
+        permissionMask = permMask;
+    }
+
+    void init(AclRecord acl, int index) {
+        this.acl = acl;
+        this.indexInAcl = index;
+    }
+
+    // ~ Methods
+    // ========================================================================================================
+
+    @Override
+    public Acl getAcl() {
+        return acl;
+    }
+
+    @Override
+    public Serializable getId() {
+        return indexInAcl;
+    }
+
+    @Override
+    public Permission getPermission() {
+        if (perm == null) {
+            perm = acl.aclPermissionFactory.buildFromMask(permissionMask);
+        }
+        return perm;
+    }
+    
+    public int getPermissionMask() {
+        return permissionMask;
+    }
+
+    void setPermission(Permission perm) {
+        this.permissionMask = perm.getMask();
+        this.perm = null;
+    }
+
+    @Override
+    public Sid getSid() {
+        if (sid == null) {
+            if (sidOfPrincipal != null)
+                sid = new PrincipalSid(sidOfPrincipal);
+            else if (sidOfAuthority != null)
+                sid = new GrantedAuthoritySid(sidOfAuthority);
+            else
+                throw new IllegalStateException();
+        }
+        return sid;
+    }
+
+    @Override
+    public boolean isGranting() {
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + permissionMask;
+        result = prime * result + ((sidOfAuthority == null) ? 0 : sidOfAuthority.hashCode());
+        result = prime * result + ((sidOfPrincipal == null) ? 0 : sidOfPrincipal.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        AceImpl other = (AceImpl) obj;
+        if (permissionMask != other.permissionMask)
+            return false;
+        if (sidOfAuthority == null) {
+            if (other.sidOfAuthority != null)
+                return false;
+        } else if (!sidOfAuthority.equals(other.sidOfAuthority))
+            return false;
+        if (sidOfPrincipal == null) {
+            if (other.sidOfPrincipal != null)
+                return false;
+        } else if (!sidOfPrincipal.equals(other.sidOfPrincipal))
+            return false;
+        return true;
+    }
+
+}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java
new file mode 100644
index 0000000..3fff632
--- /dev/null
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/AclRecord.java
@@ -0,0 +1,276 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+package org.apache.kylin.rest.security.springacl;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.kylin.common.persistence.RootPersistentEntity;
+import org.springframework.security.acls.domain.PermissionFactory;
+import org.springframework.security.acls.model.AccessControlEntry;
+import org.springframework.security.acls.model.Acl;
+import org.springframework.security.acls.model.NotFoundException;
+import org.springframework.security.acls.model.ObjectIdentity;
+import org.springframework.security.acls.model.OwnershipAcl;
+import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.PermissionGrantingStrategy;
+import org.springframework.security.acls.model.Sid;
+import org.springframework.security.acls.model.UnloadedSidException;
+import org.springframework.util.Assert;
+
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/**
+ */
+@SuppressWarnings("serial")
+@JsonAutoDetect(fieldVisibility = Visibility.NONE, getterVisibility = Visibility.NONE, isGetterVisibility = Visibility.NONE, setterVisibility = Visibility.NONE)
+public class AclRecord extends RootPersistentEntity implements Acl, OwnershipAcl {
+
+    // ~ Instance fields
+    // ================================================================================================
+
+    @JsonProperty("domainObjectInfo")
+    private ObjectIdentityImpl domainObjectInfo;
+    @JsonProperty("parentDomainObjectInfo")
+    private ObjectIdentityImpl parentDomainObjectInfo;
+    @JsonProperty("ownerInfo")
+    private SidInfo ownerInfo;
+    @JsonProperty("entriesInheriting")
+    private boolean entriesInheriting;
+    @JsonProperty("entries")
+    private List<AceImpl> entries; // the list is ordered by SID, so that grant/revoke by SID is fast
+    @JsonProperty("allAceInfo")
+    @JsonInclude(JsonInclude.Include.NON_NULL)
+    private Map<String, LegacyAceInfo> legacyAceInfo;
+
+    // non-persistent fields
+    PermissionFactory aclPermissionFactory;
+    private PermissionGrantingStrategy permissionGrantingStrategy;
+    private Acl parentAcl;
+
+    // ~ Constructor / Getter / Setter
+    // ================================================================================================
+
+    // for Jackson
+    public AclRecord() {
+    }
+
+    // new empty ACL
+    public AclRecord(ObjectIdentity oid, Sid owner) {
+        this.domainObjectInfo = new ObjectIdentityImpl(oid);
+        this.ownerInfo = new SidInfo(owner);
+    }
+
+    public void init(Acl parentAcl, PermissionFactory aclPermissionFactory,
+            PermissionGrantingStrategy permissionGrantingStrategy) {
+        this.aclPermissionFactory = aclPermissionFactory;
+        this.permissionGrantingStrategy = permissionGrantingStrategy;
+        this.parentAcl = parentAcl;
+
+        if (entries == null)
+            entries = new ArrayList<>();
+
+        // convert legacy ace
+        if (legacyAceInfo != null) {
+            for (LegacyAceInfo legacy : legacyAceInfo.values()) {
+                entries.add(new AceImpl(legacy));
+            }
+            Collections.sort(entries, AceImpl.SID_ORDER);
+            legacyAceInfo = null;
+        }
+
+        for (int i = 0; i < entries.size(); i++) {
+            entries.get(i).init(this, i);
+        }
+    }
+
+    public SidInfo getOwnerInfo() {
+        return ownerInfo;
+    }
+
+    public void setOwnerInfo(SidInfo ownerInfo) {
+        this.ownerInfo = ownerInfo;
+    }
+
+    public boolean isEntriesInheriting() {
+        return entriesInheriting;
+    }
+
+    public void setEntriesInheriting(boolean entriesInheriting) {
+        this.entriesInheriting = entriesInheriting;
+    }
+
+    public ObjectIdentityImpl getDomainObjectInfo() {
+        return domainObjectInfo;
+    }
+
+    public void setDomainObjectInfo(ObjectIdentityImpl domainObjectInfo) {
+        this.domainObjectInfo = domainObjectInfo;
+    }
+
+    public ObjectIdentityImpl getParentDomainObjectInfo() {
+        return parentDomainObjectInfo;
+    }
+
+    public void setParentDomainObjectInfo(ObjectIdentityImpl parentDomainObjectInfo) {
+        this.parentDomainObjectInfo = parentDomainObjectInfo;
+    }
+
+    public Map<String, LegacyAceInfo> getAllAceInfo() {
+        return legacyAceInfo;
+    }
+
+    public void setAllAceInfo(Map<String, LegacyAceInfo> allAceInfo) {
+        this.legacyAceInfo = allAceInfo;
+    }
+
+    // ~ Methods
+    // ========================================================================================================
+
+    @Override
+    public ObjectIdentity getObjectIdentity() {
+        return domainObjectInfo;
+    }
+
+    @Override
+    public Sid getOwner() {
+        return ownerInfo.getSidObj();
+    }
+
+    @Override
+    public void setOwner(Sid newOwner) {
+        ownerInfo = new SidInfo(newOwner);
+    }
+
+    @Override
+    public Acl getParentAcl() {
+        return parentAcl;
+    }
+
+    @Override
+    public void setParent(Acl newParent) {
+        AclRecord newP = newParent instanceof MutableAclRecord //
+                ? ((MutableAclRecord) newParent).getAclRecord()
+                : (AclRecord) newParent;
+        parentDomainObjectInfo = newP.domainObjectInfo;
+        parentAcl = newP;
+    }
+
+    @Override
+    public List<AccessControlEntry> getEntries() {
+        return new ArrayList<AccessControlEntry>(entries);
+    }
+
+    public AccessControlEntry getAccessControlEntryAt(int entryIndex) {
+        return entries.get(entryIndex);
+    }
+
+    public Permission getPermission(Sid sid) {
+        synchronized (entries) {
+            int p = Collections.binarySearch(entries, new AceImpl(sid, null), AceImpl.SID_ORDER);
+            if (p >= 0) {
+                return entries.get(p).getPermission();
+            }
+            return null;
+        }
+    }
+
+    public void upsertAce(Permission permission, Sid sid) {
+        Assert.notNull(sid, "Sid required");
+
+        AceImpl ace = new AceImpl(sid, permission);
+        synchronized (entries) {
+            int p = Collections.binarySearch(entries, ace, AceImpl.SID_ORDER);
+            if (p >= 0) {
+                if (permission == null) // null permission means delete
+                    entries.remove(p);
+                else
+                    entries.get(p).setPermission(permission);
+            } else {
+                if (permission != null) { // if not delete
+                    ace.init(this, entries.size());
+                    entries.add(-p - 1, ace);
+                }
+            }
+        }
+    }
+
+    public void deleteAce(Sid sid) {
+        upsertAce(null, sid);
+    }
+
+    @Override
+    public void insertAce(int atIndexLocation, Permission permission, Sid sid, boolean granting)
+            throws NotFoundException {
+        Assert.state(granting, "Granting must be true");
+
+        // entries are strictly ordered, given index is ignored
+        upsertAce(permission, sid);
+    }
+
+    @Override
+    public void updateAce(int aceIndex, Permission permission) throws NotFoundException {
+        verifyAceIndexExists(aceIndex);
+
+        synchronized (entries) {
+            AceImpl ace = entries.get(aceIndex);
+            ace.setPermission(permission);
+        }
+    }
+
+    @Override
+    public void deleteAce(int aceIndex) throws NotFoundException {
+        verifyAceIndexExists(aceIndex);
+
+        synchronized (entries) {
+            entries.remove(aceIndex);
+        }
+    }
+
+    private void verifyAceIndexExists(int aceIndex) {
+        if (aceIndex < 0) {
+            throw new NotFoundException("aceIndex must be greater than or equal to zero");
+        }
+        if (aceIndex >= this.entries.size()) {
+            throw new NotFoundException("aceIndex must refer to an index of the AccessControlEntry list. "
+                    + "List size is " + entries.size() + ", index was " + aceIndex);
+        }
+    }
+
+    @Override
+    public boolean isGranted(List<Permission> permission, List<Sid> sids, boolean administrativeMode)
+            throws NotFoundException, UnloadedSidException {
+        Assert.notEmpty(sids, "SIDs required");
+        Assert.notEmpty(permission, "Permissions required");
+
+        return permissionGrantingStrategy.isGranted(this, permission, sids, administrativeMode);
+    }
+
+    @Override
+    public boolean isSidLoaded(List<Sid> sids) {
+        // don't support sid filtering yet
+        return true;
+    }
+
+}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AceInfo.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/LegacyAceInfo.java
similarity index 68%
rename from server-base/src/main/java/org/apache/kylin/rest/service/AceInfo.java
rename to server-base/src/main/java/org/apache/kylin/rest/security/springacl/LegacyAceInfo.java
index 0be1019..478cae3 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/AceInfo.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/LegacyAceInfo.java
@@ -16,21 +16,28 @@
  * limitations under the License.
 */
 
-package org.apache.kylin.rest.service;
+package org.apache.kylin.rest.security.springacl;
 
 import org.springframework.security.acls.model.AccessControlEntry;
 
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
 /**
- * Created by xiefan on 17-5-2.
  */
-class AceInfo {
+@JsonAutoDetect(fieldVisibility = Visibility.NONE, getterVisibility = Visibility.NONE, isGetterVisibility = Visibility.NONE, setterVisibility = Visibility.NONE)
+public class LegacyAceInfo {
+    
+    @JsonProperty("sidInfo")
     private SidInfo sidInfo;
+    @JsonProperty("permissionMask")
     private int permissionMask;
 
-    public AceInfo() {
+    public LegacyAceInfo() {
     }
 
-    public AceInfo(AccessControlEntry ace) {
+    public LegacyAceInfo(AccessControlEntry ace) {
         super();
         this.sidInfo = new SidInfo(ace.getSid());
         this.permissionMask = ace.getPermission().getMask();
@@ -40,16 +47,7 @@ class AceInfo {
         return sidInfo;
     }
 
-    public void setSidInfo(SidInfo sidInfo) {
-        this.sidInfo = sidInfo;
-    }
-
     public int getPermissionMask() {
         return permissionMask;
     }
-
-    public void setPermissionMask(int permissionMask) {
-        this.permissionMask = permissionMask;
-    }
-
 }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/MutableAclRecord.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/MutableAclRecord.java
new file mode 100644
index 0000000..9ad099c
--- /dev/null
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/MutableAclRecord.java
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+package org.apache.kylin.rest.security.springacl;
+
+import java.io.Serializable;
+import java.util.List;
+
+import org.springframework.security.acls.model.AccessControlEntry;
+import org.springframework.security.acls.model.Acl;
+import org.springframework.security.acls.model.MutableAcl;
+import org.springframework.security.acls.model.NotFoundException;
+import org.springframework.security.acls.model.ObjectIdentity;
+import org.springframework.security.acls.model.OwnershipAcl;
+import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.Sid;
+import org.springframework.security.acls.model.UnloadedSidException;
+
+/**
+ * A thin wrapper around AclRecord to work around the conflict between MutableAcl.getId() and AclEntity.getId()
+ * having different return types.
+ */
+@SuppressWarnings("serial")
+public class MutableAclRecord implements Acl, MutableAcl, OwnershipAcl {
+
+    private final AclRecord acl;
+
+    public MutableAclRecord(AclRecord acl) {
+        this.acl = acl;
+    }
+    
+    public AclRecord getAclRecord() {
+        return acl;
+    }
+    
+    @Override
+    public Serializable getId() {
+        return acl.getDomainObjectInfo().getIdentifier();
+    }
+    
+    @Override
+    public ObjectIdentity getObjectIdentity() {
+        return acl.getObjectIdentity();
+    }
+
+    @Override
+    public Sid getOwner() {
+        return acl.getOwner();
+    }
+
+    @Override
+    public Acl getParentAcl() {
+        return acl.getParentAcl();
+    }
+
+    @Override
+    public boolean isEntriesInheriting() {
+        return acl.isEntriesInheriting();
+    }
+
+    @Override
+    public void setOwner(Sid newOwner) {
+        acl.setOwner(newOwner);
+    }
+
+    @Override
+    public void setEntriesInheriting(boolean entriesInheriting) {
+        acl.setEntriesInheriting(entriesInheriting);
+    }
+
+    @Override
+    public void setParent(Acl newParent) {
+        acl.setParent(newParent);
+    }
+
+    @Override
+    public List<AccessControlEntry> getEntries() {
+        return acl.getEntries();
+    }
+    
+    @Override
+    public void insertAce(int atIndexLocation, Permission permission, Sid sid, boolean granting)
+            throws NotFoundException {
+        acl.insertAce(atIndexLocation, permission, sid, granting);
+    }
+
+    @Override
+    public void updateAce(int aceIndex, Permission permission) throws NotFoundException {
+        acl.updateAce(aceIndex, permission);
+    }
+
+    @Override
+    public void deleteAce(int aceIndex) throws NotFoundException {
+        acl.deleteAce(aceIndex);
+    }
+    
+    @Override
+    public boolean isGranted(List<Permission> permission, List<Sid> sids, boolean administrativeMode)
+            throws NotFoundException, UnloadedSidException {
+        return acl.isGranted(permission, sids, administrativeMode);
+    }
+
+    @Override
+    public boolean isSidLoaded(List<Sid> sids) {
+        return acl.isSidLoaded(sids);
+    }
+
+}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/springacl/ObjectIdentityImpl.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/ObjectIdentityImpl.java
new file mode 100644
index 0000000..a1dc9c6
--- /dev/null
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/ObjectIdentityImpl.java
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+package org.apache.kylin.rest.security.springacl;
+
+import java.io.Serializable;
+
+import org.apache.kylin.common.persistence.AclEntity;
+import org.springframework.security.acls.model.ObjectIdentity;
+import org.springframework.util.Assert;
+
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/**
+ * Mimic org.springframework.security.acls.domain.ObjectIdentityImpl
+ * Make it Jackson friendly.
+ */
+@SuppressWarnings("serial")
+@JsonAutoDetect(fieldVisibility = Visibility.NONE, getterVisibility = Visibility.NONE, isGetterVisibility = Visibility.NONE, setterVisibility = Visibility.NONE)
+public class ObjectIdentityImpl implements ObjectIdentity {
+    // ~ Instance fields
+    // ================================================================================================
+
+    @JsonProperty("type")
+    private String type;
+    @JsonProperty("id")
+    private String identifier;
+
+    // ~ Constructors
+    // ===================================================================================================
+
+    // for Jackson
+    public ObjectIdentityImpl() {
+    }
+    
+    public ObjectIdentityImpl(ObjectIdentity oid) {
+        this(oid.getType(), String.valueOf(oid.getIdentifier()));
+    }
+    
+    public ObjectIdentityImpl(String type, String identifier) {
+        Assert.hasText(type, "Type required");
+        Assert.notNull(identifier, "identifier required");
+
+        this.identifier = identifier;
+        this.type = type;
+    }
+
+    public ObjectIdentityImpl(AclEntity ae) {
+        Assert.notNull(ae, "ACL entity required");
+        this.type = ae.getClass().getName();
+        this.identifier = ae.getId();
+    }
+
+    // ~ Methods
+    // ========================================================================================================
+
+    /**
+     * Important so caching operates properly.
+     * <p>
+     * Considers an object of the same class equal if it has the same
+     * <code>classname</code> and <code>id</code> properties.
+     * <p>
+     * Numeric identities (Integer and Long values) are considered equal if they are
+     * numerically equal. Other serializable types are evaluated using a simple equality.
+     *
+     * @param arg0 object to compare
+     *
+     * @return <code>true</code> if the presented object matches this object
+     */
+    public boolean equals(Object arg0) {
+        if (arg0 == null || !(arg0 instanceof ObjectIdentity)) {
+            return false;
+        }
+
+        ObjectIdentity other = (ObjectIdentity) arg0;
+
+        if (!identifier.equals(other.getIdentifier())) {
+            return false;
+        }
+
+        return type.equals(other.getType());
+    }
+
+    public Serializable getIdentifier() {
+        return identifier;
+    }
+    
+    public String getId() {
+        return identifier;
+    }
+
+    public String getType() {
+        return type;
+    }
+
+    /**
+     * Important so caching operates properly.
+     *
+     * @return the hash
+     */
+    public int hashCode() {
+        int code = 31;
+        code ^= this.type.hashCode();
+        code ^= this.identifier.hashCode();
+
+        return code;
+    }
+
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append(this.getClass().getName()).append("[");
+        sb.append("Type: ").append(this.type);
+        sb.append("; Identifier: ").append(this.identifier).append("]");
+
+        return sb.toString();
+    }
+}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/SidInfo.java b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/SidInfo.java
similarity index 67%
rename from server-base/src/main/java/org/apache/kylin/rest/service/SidInfo.java
rename to server-base/src/main/java/org/apache/kylin/rest/security/springacl/SidInfo.java
index 0a89449..af39341 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/SidInfo.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/springacl/SidInfo.java
@@ -16,19 +16,29 @@
  * limitations under the License.
 */
 
-package org.apache.kylin.rest.service;
+package org.apache.kylin.rest.security.springacl;
 
 import org.springframework.security.acls.domain.GrantedAuthoritySid;
 import org.springframework.security.acls.domain.PrincipalSid;
 import org.springframework.security.acls.model.Sid;
 
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
 /**
- * Created by xiefan on 17-5-2.
  */
-class SidInfo {
+@JsonAutoDetect(fieldVisibility = Visibility.NONE, getterVisibility = Visibility.NONE, isGetterVisibility = Visibility.NONE, setterVisibility = Visibility.NONE)
+public class SidInfo {
+    
+    @JsonProperty("sid")
     private String sid;
+    @JsonProperty("principal")
     private boolean isPrincipal;
 
+    private transient Sid sidObj;
+    
+    // for Jackson
     public SidInfo() {
     }
 
@@ -39,23 +49,22 @@ class SidInfo {
         } else if (sid instanceof GrantedAuthoritySid) {
             this.sid = ((GrantedAuthoritySid) sid).getGrantedAuthority();
             this.isPrincipal = false;
-        }
+        } else
+            throw new IllegalStateException();
     }
 
     public String getSid() {
         return sid;
     }
 
-    public void setSid(String sid) {
-        this.sid = sid;
-    }
-
     public boolean isPrincipal() {
         return isPrincipal;
     }
 
-    public void setPrincipal(boolean isPrincipal) {
-        this.isPrincipal = isPrincipal;
+    public Sid getSidObj() {
+        if (sidObj == null) {
+            sidObj = isPrincipal ? new PrincipalSid(sid) : new GrantedAuthoritySid(sid);
+        }
+        return sidObj;
     }
-
 }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
index 8498ecd..74a87c8 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
@@ -40,17 +40,20 @@ import org.apache.kylin.rest.msg.MsgPicker;
 import org.apache.kylin.rest.response.AccessEntryResponse;
 import org.apache.kylin.rest.security.AclEntityFactory;
 import org.apache.kylin.rest.security.AclEntityType;
+import org.apache.kylin.rest.security.springacl.AclRecord;
+import org.apache.kylin.rest.security.springacl.MutableAclRecord;
+import org.apache.kylin.rest.security.springacl.ObjectIdentityImpl;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.acls.domain.BasePermission;
 import org.springframework.security.acls.domain.GrantedAuthoritySid;
-import org.springframework.security.acls.domain.ObjectIdentityImpl;
 import org.springframework.security.acls.domain.PrincipalSid;
 import org.springframework.security.acls.model.AccessControlEntry;
 import org.springframework.security.acls.model.Acl;
 import org.springframework.security.acls.model.AlreadyExistsException;
-import org.springframework.security.acls.model.MutableAcl;
 import org.springframework.security.acls.model.NotFoundException;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.Permission;
@@ -64,11 +67,11 @@ import org.springframework.transaction.annotation.Transactional;
 import com.google.common.base.Preconditions;
 
 /**
- * @author xduo
- * 
  */
 @Component("accessService")
 public class AccessService {
+    @SuppressWarnings("unused")
+    private static final Logger logger = LoggerFactory.getLogger(AccessService.class);
 
     @Autowired
     @Qualifier("aclService")
@@ -77,15 +80,15 @@ public class AccessService {
     // ~ Methods to manage acl life circle of domain objects ~
 
     @Transactional
-    public Acl init(AclEntity ae, Permission initPermission) {
-        Acl acl = null;
-        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae.getClass(), ae.getId());
+    public MutableAclRecord init(AclEntity ae, Permission initPermission) {
+        MutableAclRecord acl = null;
+        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae);
 
         try {
             // Create acl record for secured domain object.
-            acl = aclService.createAcl(objectIdentity);
+            acl = (MutableAclRecord) aclService.createAcl(objectIdentity);
         } catch (AlreadyExistsException e) {
-            acl = (MutableAcl) aclService.readAclById(objectIdentity);
+            acl = aclService.readAcl(objectIdentity);
         }
 
         if (null != initPermission) {
@@ -99,7 +102,7 @@ public class AccessService {
 
     @Transactional
     @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN + " or hasPermission(#ae, 'ADMINISTRATION')")
-    public Acl grant(AclEntity ae, Permission permission, Sid sid) {
+    public MutableAclRecord grant(AclEntity ae, Permission permission, Sid sid) {
         Message msg = MsgPicker.getMsg();
 
         if (ae == null)
@@ -109,150 +112,83 @@ public class AccessService {
         if (sid == null)
             throw new BadRequestException(msg.getSID_REQUIRED());
 
-        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae.getClass(), ae.getId());
-        MutableAcl acl = null;
-
+        MutableAclRecord acl = null;
         try {
-            acl = (MutableAcl) aclService.readAclById(objectIdentity);
+            acl = aclService.readAcl(new ObjectIdentityImpl(ae));
         } catch (NotFoundException e) {
-            acl = (MutableAcl) init(ae, null);
-        }
-
-        int indexOfAce = -1;
-        for (int i = 0; i < acl.getEntries().size(); i++) {
-            AccessControlEntry ace = acl.getEntries().get(i);
-
-            if (ace.getSid().equals(sid)) {
-                indexOfAce = i;
-            }
+            acl = init(ae, null);
         }
 
-        if (indexOfAce != -1) {
-            secureOwner(acl, indexOfAce);
-            acl.updateAce(indexOfAce, permission);
-        } else {
-            acl.insertAce(acl.getEntries().size(), permission, sid, true);
-        }
-
-        acl = aclService.updateAcl(acl);
+        secureOwner(acl, sid);
 
-        return acl;
+        return aclService.upsertAce(acl, sid, permission);
     }
 
     @Transactional
     @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN + " or hasPermission(#ae, 'ADMINISTRATION')")
-    public Acl update(AclEntity ae, Long accessEntryId, Permission newPermission) {
+    public MutableAclRecord update(AclEntity ae, int accessEntryIndex, Permission newPermission) {
         Message msg = MsgPicker.getMsg();
 
         if (ae == null)
             throw new BadRequestException(msg.getACL_DOMAIN_NOT_FOUND());
-        if (accessEntryId == null)
-            throw new BadRequestException(msg.getACE_ID_REQUIRED());
         if (newPermission == null)
             throw new BadRequestException(msg.getACL_PERMISSION_REQUIRED());
 
-        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae.getClass(), ae.getId());
-        MutableAcl acl = (MutableAcl) aclService.readAclById(objectIdentity);
-
-        int indexOfAce = -1;
-        for (int i = 0; i < acl.getEntries().size(); i++) {
-            AccessControlEntry ace = acl.getEntries().get(i);
-            if (ace.getId().equals(accessEntryId)) {
-                indexOfAce = i;
-                break;
-            }
-        }
-
-        if (indexOfAce != -1) {
-            secureOwner(acl, indexOfAce);
+        MutableAclRecord acl = aclService.readAcl(new ObjectIdentityImpl(ae));
+        Sid sid = acl.getAclRecord().getAccessControlEntryAt(accessEntryIndex).getSid();
 
-            try {
-                acl.updateAce(indexOfAce, newPermission);
-                acl = aclService.updateAcl(acl);
-            } catch (NotFoundException e) {
-                //do nothing?
-            }
-        }
+        secureOwner(acl, sid);
 
-        return acl;
+        return aclService.upsertAce(acl, sid, newPermission);
     }
 
     @Transactional
     @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN + " or hasPermission(#ae, 'ADMINISTRATION')")
-    public Acl revoke(AclEntity ae, Long accessEntryId) {
+    public MutableAclRecord revoke(AclEntity ae, int accessEntryIndex) {
         Message msg = MsgPicker.getMsg();
 
         if (ae == null)
             throw new BadRequestException(msg.getACL_DOMAIN_NOT_FOUND());
-        if (accessEntryId == null)
-            throw new BadRequestException(msg.getACE_ID_REQUIRED());
 
-        MutableAcl acl = (MutableAcl) getAcl(ae);
-        int indexOfAce = getIndexOfAce(accessEntryId, acl);
-        acl = deleteAndUpdate(acl, indexOfAce);
-        return acl;
-    }
+        MutableAclRecord acl = aclService.readAcl(new ObjectIdentityImpl(ae));
+        Sid sid = acl.getAclRecord().getAccessControlEntryAt(accessEntryIndex).getSid();
 
-    private int getIndexOfAce(Long accessEntryId, MutableAcl acl) {
-        int indexOfAce = -1;
-        List<AccessControlEntry> aces = acl.getEntries();
-        for (int i = 0; i < aces.size(); i++) {
-            if (aces.get(i).getId().equals(accessEntryId)) {
-                indexOfAce = i;
-                break;
-            }
-        }
-        return indexOfAce;
-    }
+        secureOwner(acl, sid);
 
-    private MutableAcl deleteAndUpdate(MutableAcl acl, int indexOfAce) {
-        if (indexOfAce != -1) {
-            secureOwner(acl, indexOfAce);
-            try {
-                acl.deleteAce(indexOfAce);
-                acl = aclService.updateAcl(acl);
-            } catch (NotFoundException e) {
-                throw new RuntimeException("Revoke acl fail." + e.getMessage());
-            }
-        }
-        return acl;
+        return aclService.upsertAce(acl, sid, null);
     }
 
-    @Deprecated
+    /**
+     * The method is not used at the moment
+     */
     @Transactional
     public void inherit(AclEntity ae, AclEntity parentAe) {
         Message msg = MsgPicker.getMsg();
 
-        if (ae == null) {
+        if (ae == null)
             throw new BadRequestException(msg.getACL_DOMAIN_NOT_FOUND());
-        }
-        if (parentAe == null) {
+        if (parentAe == null)
             throw new BadRequestException(msg.getPARENT_ACL_NOT_FOUND());
-        }
 
-        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae.getClass(), ae.getId());
-        MutableAcl acl = null;
+        MutableAclRecord acl = null;
         try {
-            acl = (MutableAcl) aclService.readAclById(objectIdentity);
+            acl = aclService.readAcl(new ObjectIdentityImpl(ae));
         } catch (NotFoundException e) {
-            acl = (MutableAcl) init(ae, null);
+            acl = init(ae, null);
         }
 
-        ObjectIdentity parentObjectIdentity = new ObjectIdentityImpl(parentAe.getClass(), parentAe.getId());
-        MutableAcl parentAcl = null;
+        MutableAclRecord parentAcl = null;
         try {
-            parentAcl = (MutableAcl) aclService.readAclById(parentObjectIdentity);
+            parentAcl = aclService.readAcl(new ObjectIdentityImpl(parentAe));
         } catch (NotFoundException e) {
-            parentAcl = (MutableAcl) init(parentAe, null);
+            parentAcl = init(parentAe, null);
         }
 
         if (null == acl || null == parentAcl) {
             return;
         }
 
-        acl.setEntriesInheriting(true);
-        acl.setParent(parentAcl);
-        aclService.updateAcl(acl);
+        aclService.inherit(acl, parentAcl);
     }
 
     @Transactional
@@ -268,7 +204,7 @@ public class AccessService {
         if (ae.getId() == null)
             return;
 
-        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae.getClass(), ae.getId());
+        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae);
 
         try {
             aclService.deleteAcl(objectIdentity, deleteChildren);
@@ -287,20 +223,17 @@ public class AccessService {
         return AclEntityFactory.createAclEntity(entityType, uuid);
     }
 
-    @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN +
-            " or hasPermission(#ae, 'ADMINISTRATION')" +
-            " or hasPermission(#ae, 'MANAGEMENT')" +
-            " or hasPermission(#ae, 'OPERATION')" +
-            " or hasPermission(#ae, 'READ')")
-    public Acl getAcl(AclEntity ae) {
+    @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN + " or hasPermission(#ae, 'ADMINISTRATION')"
+            + " or hasPermission(#ae, 'MANAGEMENT')" + " or hasPermission(#ae, 'OPERATION')"
+            + " or hasPermission(#ae, 'READ')")
+    public MutableAclRecord getAcl(AclEntity ae) {
         if (null == ae) {
             return null;
         }
-        ObjectIdentity objectIdentity = new ObjectIdentityImpl(ae.getClass(), ae.getId());
-        Acl acl = null;
 
+        MutableAclRecord acl = null;
         try {
-            acl = (MutableAcl) aclService.readAclById(objectIdentity);
+            acl = aclService.readAcl(new ObjectIdentityImpl(ae));
         } catch (NotFoundException e) {
             //do nothing?
         }
@@ -316,7 +249,8 @@ public class AccessService {
         }
     }
 
-    public List<AccessEntryResponse> generateAceResponsesByFuzzMatching(Acl acl, String nameSeg, boolean isCaseSensitive) {
+    public List<AccessEntryResponse> generateAceResponsesByFuzzMatching(Acl acl, String nameSeg,
+            boolean isCaseSensitive) {
         if (null == acl) {
             return Collections.emptyList();
         }
@@ -357,10 +291,10 @@ public class AccessService {
         List<String> result = new ArrayList<>();
         for (AccessControlEntry ace : acl.getEntries()) {
             String name = null;
-            if (type.equalsIgnoreCase("user") && ace.getSid() instanceof PrincipalSid) {
+            if (type.equalsIgnoreCase(MetadataConstants.TYPE_USER) && ace.getSid() instanceof PrincipalSid) {
                 name = ((PrincipalSid) ace.getSid()).getPrincipal();
             }
-            if (type.equalsIgnoreCase("group") && ace.getSid() instanceof GrantedAuthoritySid) {
+            if (type.equalsIgnoreCase(MetadataConstants.TYPE_GROUP) && ace.getSid() instanceof GrantedAuthoritySid) {
                 name = ((GrantedAuthoritySid) ace.getSid()).getGrantedAuthority();
             }
             if (!StringUtils.isBlank(name)) {
@@ -376,14 +310,16 @@ public class AccessService {
      * @param acl
      * @param indexOfAce
      */
-    private void secureOwner(MutableAcl acl, int indexOfAce) {
+    private void secureOwner(MutableAclRecord acl, Sid sid) {
         Message msg = MsgPicker.getMsg();
 
-        // Can't revoke admin permission from domain object owner
-        if (acl.getOwner().equals(acl.getEntries().get(indexOfAce).getSid())
-                && BasePermission.ADMINISTRATION.equals(acl.getEntries().get(indexOfAce).getPermission())) {
+        AclRecord record = acl.getAclRecord();
+        if (record.getOwner().equals(sid) == false)
+            return;
+
+        // prevent changing owner's admin permission
+        if (BasePermission.ADMINISTRATION.equals(record.getPermission(sid)))
             throw new ForbiddenException(msg.getREVOKE_ADMIN_PERMISSION());
-        }
     }
 
     public Object generateAllAceResponses(Acl acl) {
@@ -400,32 +336,35 @@ public class AccessService {
     }
 
     public void revokeProjectPermission(String name, String type) {
-        //revoke user's project permission
-        List<ProjectInstance> projectInstances = ProjectManager.getInstance(KylinConfig.getInstanceFromEnv()).listAllProjects();
+        Sid sid = null;
+        if (type.equalsIgnoreCase(MetadataConstants.TYPE_USER)) {
+            sid = new PrincipalSid(name);
+        } else if (type.equalsIgnoreCase(MetadataConstants.TYPE_GROUP)) {
+            sid = new GrantedAuthoritySid(name);
+        } else {
+            return;
+        }
+
+        // revoke user's project permission
+        List<ProjectInstance> projectInstances = ProjectManager.getInstance(KylinConfig.getInstanceFromEnv())
+                .listAllProjects();
         for (ProjectInstance pi : projectInstances) {
             // after KYLIN-2760, only project ACL will work, so entity type is always ProjectInstance.
             AclEntity ae = getAclEntity("ProjectInstance", pi.getUuid());
 
-            MutableAcl acl = (MutableAcl) getAcl(ae);
+            MutableAclRecord acl = getAcl(ae);
             if (acl == null) {
                 return;
             }
-            List<AccessControlEntry> aces = acl.getEntries();
-            if (aces == null) {
-                return;
-            }
 
-            int indexOfAce = -1;
-            for (int i = 0; i < aces.size(); i++) {
-                if (needRevoke(aces.get(i).getSid(), name, type)) {
-                    indexOfAce = i;
-                    break;
-                }
+            Permission perm = acl.getAclRecord().getPermission(sid);
+            if (perm != null) {
+                secureOwner(acl, sid);
+                aclService.upsertAce(acl, sid, null);
             }
-            deleteAndUpdate(acl, indexOfAce);
         }
     }
-
+    
     public String getUserPermissionInPrj(String project) {
         String grantedPermission = "";
         List<String> groups = getGroupsFromCurrentUser();
@@ -435,30 +374,31 @@ public class AccessService {
 
         // {user/group:permission}
         Map<String, Integer> projectPermissions = getProjectPermission(project);
-        Integer greaterPermission = projectPermissions.get(SecurityContextHolder.getContext().getAuthentication().getName());
+        Integer greaterPermission = projectPermissions
+                .get(SecurityContextHolder.getContext().getAuthentication().getName());
         for (String group : groups) {
             Integer groupPerm = projectPermissions.get(group);
             greaterPermission = Preconditions.checkNotNull(getGreaterPerm(groupPerm, greaterPermission));
         }
 
         switch (greaterPermission) {
-            case 16:
-                grantedPermission = "ADMINISTRATION";
-                break;
-            case 32:
-                grantedPermission = "MANAGEMENT";
-                break;
-            case 64:
-                grantedPermission = "OPERATION";
-                break;
-            case 1:
-                grantedPermission = "READ";
-                break;
-            case 0:
-                grantedPermission = "EMPTY";
-                break;
-            default:
-                throw new RuntimeException("invalid permission state:" + greaterPermission);
+        case 16:
+            grantedPermission = "ADMINISTRATION";
+            break;
+        case 32:
+            grantedPermission = "MANAGEMENT";
+            break;
+        case 64:
+            grantedPermission = "OPERATION";
+            break;
+        case 1:
+            grantedPermission = "READ";
+            break;
+        case 0:
+            grantedPermission = "EMPTY";
+            break;
+        default:
+            throw new RuntimeException("invalid permission state:" + greaterPermission);
         }
         return grantedPermission;
     }
@@ -488,7 +428,8 @@ public class AccessService {
 
     private List<String> getGroupsFromCurrentUser() {
         List<String> groups = new ArrayList<>();
-        Collection<? extends GrantedAuthority> authorities = SecurityContextHolder.getContext().getAuthentication().getAuthorities();
+        Collection<? extends GrantedAuthority> authorities = SecurityContextHolder.getContext().getAuthentication()
+                .getAuthorities();
 
         for (GrantedAuthority auth : authorities) {
             groups.add(auth.getAuthority());
@@ -522,14 +463,4 @@ public class AccessService {
         }
         return null;
     }
-
-    private boolean needRevoke(Sid sid, String name, String type) {
-        if (type.equals(MetadataConstants.TYPE_USER) && sid instanceof PrincipalSid) {
-            return ((PrincipalSid) sid).getPrincipal().equals(name);
-        } else if (type.equals(MetadataConstants.TYPE_GROUP) && sid instanceof GrantedAuthoritySid) {
-            return ((GrantedAuthoritySid) sid).getGrantedAuthority().equals(name);
-        } else {
-            return false;
-        }
-    }
 }
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 1eab0e5..73a6fb2 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
@@ -19,7 +19,6 @@
 package org.apache.kylin.rest.service;
 
 import java.io.IOException;
-import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -29,25 +28,19 @@ import java.util.Map;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.persistence.JsonSerializer;
 import org.apache.kylin.common.persistence.ResourceStore;
-import org.apache.kylin.common.persistence.RootPersistentEntity;
 import org.apache.kylin.common.persistence.Serializer;
 import org.apache.kylin.rest.exception.BadRequestException;
 import org.apache.kylin.rest.exception.InternalErrorException;
 import org.apache.kylin.rest.msg.Message;
 import org.apache.kylin.rest.msg.MsgPicker;
+import org.apache.kylin.rest.security.springacl.AclRecord;
+import org.apache.kylin.rest.security.springacl.MutableAclRecord;
+import org.apache.kylin.rest.security.springacl.ObjectIdentityImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Qualifier;
-import org.springframework.security.acls.domain.AccessControlEntryImpl;
-import org.springframework.security.acls.domain.AclAuthorizationStrategy;
-import org.springframework.security.acls.domain.AclImpl;
-import org.springframework.security.acls.domain.AuditLogger;
-import org.springframework.security.acls.domain.GrantedAuthoritySid;
-import org.springframework.security.acls.domain.ObjectIdentityImpl;
 import org.springframework.security.acls.domain.PermissionFactory;
 import org.springframework.security.acls.domain.PrincipalSid;
-import org.springframework.security.acls.model.AccessControlEntry;
 import org.springframework.security.acls.model.Acl;
 import org.springframework.security.acls.model.AlreadyExistsException;
 import org.springframework.security.acls.model.ChildrenExistException;
@@ -55,29 +48,21 @@ import org.springframework.security.acls.model.MutableAcl;
 import org.springframework.security.acls.model.MutableAclService;
 import org.springframework.security.acls.model.NotFoundException;
 import org.springframework.security.acls.model.ObjectIdentity;
+import org.springframework.security.acls.model.Permission;
 import org.springframework.security.acls.model.PermissionGrantingStrategy;
 import org.springframework.security.acls.model.Sid;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.security.util.FieldUtils;
 import org.springframework.stereotype.Component;
 
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.core.JsonParseException;
-import com.fasterxml.jackson.databind.JsonMappingException;
-
 @Component("aclService")
 public class AclService implements MutableAclService {
-
     private static final Logger logger = LoggerFactory.getLogger(AclService.class);
 
-    private final Field fieldAces = FieldUtils.getField(AclImpl.class, "aces");
-
-    private final Field fieldAcl = FieldUtils.getField(AccessControlEntryImpl.class, "acl");
-
     public static final String DIR_PREFIX = "/acl/";
+    public static final Serializer<AclRecord> SERIALIZER = new JsonSerializer<>(AclRecord.class, true);
 
-    public static final Serializer<AclRecord> SERIALIZER = new JsonSerializer<>(AclRecord.class);
+    // ============================================================================
 
     @Autowired
     protected PermissionGrantingStrategy permissionGrantingStrategy;
@@ -85,21 +70,15 @@ public class AclService implements MutableAclService {
     @Autowired
     protected PermissionFactory aclPermissionFactory;
 
-    @Autowired
-    protected AclAuthorizationStrategy aclAuthorizationStrategy;
+    //    @Autowired
+    //    protected AclAuthorizationStrategy aclAuthorizationStrategy;
 
-    @Autowired
-    protected AuditLogger auditLogger;
+    //    @Autowired
+    //    protected AuditLogger auditLogger;
 
     protected ResourceStore aclStore;
 
-    @Autowired
-    @Qualifier("userService")
-    protected UserService userService;
-
     public AclService() throws IOException {
-        fieldAces.setAccessible(true);
-        fieldAcl.setAccessible(true);
         aclStore = ResourceStore.getStore(KylinConfig.getInstanceFromEnv());
     }
 
@@ -107,13 +86,12 @@ public class AclService implements MutableAclService {
     public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
         List<ObjectIdentity> oids = new ArrayList<ObjectIdentity>();
         try {
-            List<AclRecord> allAclRecords = aclStore.getAllResources(String.valueOf(DIR_PREFIX), AclRecord.class,
-                    SERIALIZER);
+            List<AclRecord> allAclRecords = aclStore.getAllResources(DIR_PREFIX, AclRecord.class, SERIALIZER);
             for (AclRecord record : allAclRecords) {
-                DomainObjectInfo parent = record.getParentDomainObjectInfo();
-                if (parent != null && parent.getId().equals(String.valueOf(parentIdentity.getIdentifier()))) {
-                    DomainObjectInfo child = record.getDomainObjectInfo();
-                    oids.add(new ObjectIdentityImpl(child.getType(), child.getId()));
+                ObjectIdentityImpl parent = record.getParentDomainObjectInfo();
+                if (parent != null && parent.equals(parentIdentity)) {
+                    ObjectIdentityImpl child = record.getDomainObjectInfo();
+                    oids.add(child);
                 }
             }
             return oids;
@@ -122,6 +100,10 @@ public class AclService implements MutableAclService {
         }
     }
 
+    public MutableAclRecord readAcl(ObjectIdentity oid) throws NotFoundException {
+        return (MutableAclRecord) readAclById(oid);
+    }
+
     @Override
     public Acl readAclById(ObjectIdentity object) throws NotFoundException {
         Map<ObjectIdentity, Acl> aclsMap = readAclsById(Arrays.asList(object), null);
@@ -145,31 +127,22 @@ public class AclService implements MutableAclService {
 
     @Override
     public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> oids, List<Sid> sids) throws NotFoundException {
-        Message msg = MsgPicker.getMsg();
-        Map<ObjectIdentity, Acl> aclMaps = new HashMap<ObjectIdentity, Acl>();
+        Map<ObjectIdentity, Acl> aclMaps = new HashMap<>();
         try {
             for (ObjectIdentity oid : oids) {
-                AclRecord record = aclStore.getResource(getQueryKeyById(String.valueOf(oid.getIdentifier())),
-                        AclRecord.class, SERIALIZER);
-                if (record != null && record.getOwnerInfo() != null) {
-                    SidInfo owner = record.getOwnerInfo();
-                    Sid ownerSid = owner.isPrincipal() ? new PrincipalSid(owner.getSid()) : new GrantedAuthoritySid(owner.getSid());
-                    boolean entriesInheriting = record.isEntriesInheriting();
-
-                    Acl parentAcl = null;
-                    DomainObjectInfo parent = record.getParentDomainObjectInfo();
-                    if (parent != null) {
-                        ObjectIdentity parentObject = new ObjectIdentityImpl(parent.getType(), parent.getId());
-                        parentAcl = readAclById(parentObject, null);
-                    }
-
-                    AclImpl acl = new AclImpl(oid, oid.getIdentifier(), aclAuthorizationStrategy, permissionGrantingStrategy, parentAcl, null, entriesInheriting, ownerSid);
-                    genAces(sids, record, acl);
-
-                    aclMaps.put(oid, acl);
-                } else {
+                AclRecord record = aclStore.getResource(resourceKey(oid), AclRecord.class, SERIALIZER);
+                if (record == null) {
+                    Message msg = MsgPicker.getMsg();
                     throw new NotFoundException(String.format(msg.getACL_INFO_NOT_FOUND(), oid));
                 }
+
+                Acl parentAcl = null;
+                if (record.isEntriesInheriting() && record.getParentDomainObjectInfo() != null)
+                    parentAcl = readAclById(record.getParentDomainObjectInfo());
+
+                record.init(parentAcl, aclPermissionFactory, permissionGrantingStrategy);
+
+                aclMaps.put(oid, new MutableAclRecord(record));
             }
             return aclMaps;
         } catch (IOException e) {
@@ -179,22 +152,16 @@ public class AclService implements MutableAclService {
 
     @Override
     public MutableAcl createAcl(ObjectIdentity objectIdentity) throws AlreadyExistsException {
-        Acl acl = null;
-
-        try {
-            acl = readAclById(objectIdentity);
-        } catch (NotFoundException e) {
-            //do nothing?
-        }
-        if (null != acl) {
-            throw new AlreadyExistsException("ACL of " + objectIdentity + " exists!");
-        }
-        Authentication auth = SecurityContextHolder.getContext().getAuthentication();
-        PrincipalSid sid = new PrincipalSid(auth);
         try {
-            AclRecord record = new AclRecord(new DomainObjectInfo(objectIdentity), null, new SidInfo(sid), true, null);
-            aclStore.putResource(getQueryKeyById(String.valueOf(objectIdentity.getIdentifier())), record, 0,
-                    SERIALIZER);
+            if (aclStore.exists(resourceKey(objectIdentity))) {
+                throw new AlreadyExistsException("ACL of " + objectIdentity + " exists!");
+            }
+
+            Authentication auth = SecurityContextHolder.getContext().getAuthentication();
+            PrincipalSid owner = new PrincipalSid(auth);
+            AclRecord record = new AclRecord(objectIdentity, owner);
+            record.init(null, aclPermissionFactory, permissionGrantingStrategy);
+            aclStore.putResource(resourceKey(objectIdentity), record, 0, SERIALIZER);
             logger.debug("ACL of " + objectIdentity + " created successfully.");
         } catch (IOException e) {
             throw new InternalErrorException(e);
@@ -204,178 +171,92 @@ public class AclService implements MutableAclService {
 
     @Override
     public void deleteAcl(ObjectIdentity objectIdentity, boolean deleteChildren) throws ChildrenExistException {
-        Message msg = MsgPicker.getMsg();
         try {
             List<ObjectIdentity> children = findChildren(objectIdentity);
             if (!deleteChildren && children.size() > 0) {
+                Message msg = MsgPicker.getMsg();
                 throw new BadRequestException(String.format(msg.getIDENTITY_EXIST_CHILDREN(), objectIdentity));
             }
             for (ObjectIdentity oid : children) {
                 deleteAcl(oid, deleteChildren);
             }
-            aclStore.deleteResource(getQueryKeyById(String.valueOf(objectIdentity.getIdentifier())));
+            aclStore.deleteResource(resourceKey(String.valueOf(objectIdentity.getIdentifier())));
             logger.debug("ACL of " + objectIdentity + " deleted successfully.");
         } catch (IOException e) {
             throw new InternalErrorException(e);
         }
     }
 
+    // Try use the updateAclWithRetry() method family whenever possible
     @Override
     public MutableAcl updateAcl(MutableAcl mutableAcl) throws NotFoundException {
-        Message msg = MsgPicker.getMsg();
         try {
-            readAclById(mutableAcl.getObjectIdentity());
-        } catch (NotFoundException e) {
-            throw e;
-        }
-
-        try {
-            String id = getQueryKeyById(String.valueOf(mutableAcl.getObjectIdentity().getIdentifier()));
-            AclRecord record = aclStore.getResource(id, AclRecord.class, SERIALIZER);
-            if (mutableAcl.getParentAcl() != null) {
-                record.setParentDomainObjectInfo(new DomainObjectInfo(mutableAcl.getParentAcl().getObjectIdentity()));
-            }
-
-            if (record.getAllAceInfo() == null) {
-                record.setAllAceInfo(new HashMap<String, AceInfo>());
-            }
-            Map<String, AceInfo> allAceInfo = record.getAllAceInfo();
-            allAceInfo.clear();
-            for (AccessControlEntry ace : mutableAcl.getEntries()) {
-                if (ace.getSid() instanceof PrincipalSid) {
-                    PrincipalSid psid = (PrincipalSid) ace.getSid();
-                    String userName = psid.getPrincipal();
-                    if (!userService.userExists(userName))
-                        logger.error("Grant project access error," + String.format(msg.getUSER_NOT_EXIST(), userName));
-                }
-                AceInfo aceInfo = new AceInfo(ace);
-                allAceInfo.put(String.valueOf(aceInfo.getSidInfo().getSid()), aceInfo);
-            }
-            aclStore.putResourceWithoutCheck(id, record, System.currentTimeMillis(), SERIALIZER);
+            AclRecord record = ((MutableAclRecord) mutableAcl).getAclRecord();
+            String resPath = resourceKey(mutableAcl.getObjectIdentity());
+            aclStore.putResource(resPath, record, System.currentTimeMillis(), SERIALIZER);
             logger.debug("ACL of " + mutableAcl.getObjectIdentity() + " updated successfully.");
         } catch (IOException e) {
             throw new InternalErrorException(e);
         }
-        return (MutableAcl) readAclById(mutableAcl.getObjectIdentity());
+        return mutableAcl;
     }
 
-    protected void genAces(List<Sid> sids, AclRecord record, AclImpl acl) throws JsonParseException, JsonMappingException, IOException {
-        List<AceInfo> aceInfos = new ArrayList<AceInfo>();
-        Map<String, AceInfo> allAceInfos = record.getAllAceInfo();
-        if (allAceInfos != null) {
-            if (sids != null) {
-                // Just return aces in sids
-                for (Sid sid : sids) {
-                    String sidName = null;
-                    if (sid instanceof PrincipalSid) {
-                        sidName = ((PrincipalSid) sid).getPrincipal();
-                    } else if (sid instanceof GrantedAuthoritySid) {
-                        sidName = ((GrantedAuthoritySid) sid).getGrantedAuthority();
-                    }
-                    AceInfo aceInfo = allAceInfos.get(sidName);
-                    if (aceInfo != null) {
-                        aceInfos.add(aceInfo);
-                    }
-                }
-            } else {
-                aceInfos.addAll(allAceInfos.values());
-            }
-        } 
-
-        List<AccessControlEntry> newAces = new ArrayList<AccessControlEntry>();
-        for (int i = 0; i < aceInfos.size(); i++) {
-            AceInfo aceInfo = aceInfos.get(i);
-
-            if (null != aceInfo) {
-                Sid sid = aceInfo.getSidInfo().isPrincipal() ? new PrincipalSid(aceInfo.getSidInfo().getSid()) : new GrantedAuthoritySid(aceInfo.getSidInfo().getSid());
-                AccessControlEntry ace = new AccessControlEntryImpl(Long.valueOf(i), acl, sid, aclPermissionFactory.buildFromMask(aceInfo.getPermissionMask()), true, false, false);
-                newAces.add(ace);
+    // a NULL permission means to delete the ace
+    public MutableAclRecord upsertAce(MutableAclRecord acl, final Sid sid, final Permission perm) {
+        return updateAclWithRetry(acl, new AclRecordUpdater() {
+            @Override
+            public void update(AclRecord record) {
+                record.upsertAce(perm, sid);
             }
-        }
-
-        this.setAces(acl, newAces);
-    }
-
-    private void setAces(AclImpl acl, List<AccessControlEntry> aces) {
-        try {
-            fieldAces.set(acl, aces);
-        } catch (IllegalAccessException e) {
-            throw new IllegalStateException("Could not set AclImpl entries", e);
-        }
-    }
-
-    public static String getQueryKeyById(String id) {
-        return DIR_PREFIX + id;
-    }
-}
-
-@SuppressWarnings("serial")
-class AclRecord extends RootPersistentEntity {
-
-    @JsonProperty()
-    private DomainObjectInfo domainObjectInfo;
-
-    @JsonProperty()
-    private DomainObjectInfo parentDomainObjectInfo;
-
-    @JsonProperty()
-    private SidInfo ownerInfo;
-
-    @JsonProperty()
-    private boolean entriesInheriting;
-
-    @JsonProperty()
-    private Map<String, AceInfo> allAceInfo;
-
-    public AclRecord() {
+        });
     }
-
-    public AclRecord(DomainObjectInfo domainObjectInfo, DomainObjectInfo parentDomainObjectInfo, SidInfo ownerInfo, boolean entriesInheriting, Map<String, AceInfo> allAceInfo) {
-        this.domainObjectInfo = domainObjectInfo;
-        this.parentDomainObjectInfo = parentDomainObjectInfo;
-        this.ownerInfo = ownerInfo;
-        this.entriesInheriting = entriesInheriting;
-        this.allAceInfo = allAceInfo;
-    }
-
-    public SidInfo getOwnerInfo() {
-        return ownerInfo;
-    }
-
-    public void setOwnerInfo(SidInfo ownerInfo) {
-        this.ownerInfo = ownerInfo;
-    }
-
-    public boolean isEntriesInheriting() {
-        return entriesInheriting;
-    }
-
-    public void setEntriesInheriting(boolean entriesInheriting) {
-        this.entriesInheriting = entriesInheriting;
+    
+    public MutableAclRecord inherit(MutableAclRecord acl, final MutableAclRecord parentAcl) {
+        return updateAclWithRetry(acl, new AclRecordUpdater() {
+            @Override
+            public void update(AclRecord record) {
+                record.setEntriesInheriting(true);
+                record.setParent(parentAcl);
+            }
+        });
     }
 
-    public DomainObjectInfo getDomainObjectInfo() {
-        return domainObjectInfo;
+    public interface AclRecordUpdater {
+        void update(AclRecord record);
     }
 
-    public void setDomainObjectInfo(DomainObjectInfo domainObjectInfo) {
-        this.domainObjectInfo = domainObjectInfo;
-    }
+    private MutableAclRecord updateAclWithRetry(MutableAclRecord acl, AclRecordUpdater updater) {
+        int retry = 7;
+        while (retry-- > 0) {
+            AclRecord record = acl.getAclRecord();
+
+            updater.update(record);
+            String resPath = resourceKey(record.getObjectIdentity());
+            try {
+                aclStore.putResource(resPath, record, System.currentTimeMillis(), SERIALIZER);
+                return acl; // here we are done
+
+            } catch (IllegalStateException ise) {
+                if (retry <= 0) {
+                    logger.error("Retry is out, till got error, abandoning...", ise);
+                    throw ise;
+                }
 
-    public DomainObjectInfo getParentDomainObjectInfo() {
-        return parentDomainObjectInfo;
-    }
+                logger.warn("Write conflict to update ACL " + resPath + " retry remaining " + retry + ", will retry...");
+                acl = readAcl(acl.getObjectIdentity());
 
-    public void setParentDomainObjectInfo(DomainObjectInfo parentDomainObjectInfo) {
-        this.parentDomainObjectInfo = parentDomainObjectInfo;
+            } catch (IOException e) {
+                throw new InternalErrorException(e);
+            }
+        }
+        throw new RuntimeException("should not reach here");
     }
 
-    public Map<String, AceInfo> getAllAceInfo() {
-        return allAceInfo;
+    public static String resourceKey(ObjectIdentity domainObjId) {
+        return resourceKey(String.valueOf(domainObjId.getIdentifier()));
     }
 
-    public void setAllAceInfo(Map<String, AceInfo> allAceInfo) {
-        this.allAceInfo = allAceInfo;
+    public static String resourceKey(String domainObjId) {
+        return DIR_PREFIX + domainObjId;
     }
-
 }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AclTableMigrationTool.java b/server-base/src/main/java/org/apache/kylin/rest/service/AclTableMigrationTool.java
index 333d98e..33957ab 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/AclTableMigrationTool.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/AclTableMigrationTool.java
@@ -40,6 +40,10 @@ import org.apache.kylin.common.persistence.StringEntity;
 import org.apache.kylin.common.util.Bytes;
 import org.apache.kylin.rest.security.AclConstant;
 import org.apache.kylin.rest.security.ManagedUser;
+import org.apache.kylin.rest.security.springacl.AclRecord;
+import org.apache.kylin.rest.security.springacl.LegacyAceInfo;
+import org.apache.kylin.rest.security.springacl.ObjectIdentityImpl;
+import org.apache.kylin.rest.security.springacl.SidInfo;
 import org.apache.kylin.rest.util.Serializer;
 import org.apache.kylin.storage.hbase.HBaseConnection;
 import org.apache.kylin.storage.hbase.HBaseResourceStore;
@@ -53,10 +57,10 @@ public class AclTableMigrationTool {
 
     private static final Serializer<SidInfo> sidSerializer = new Serializer<SidInfo>(SidInfo.class);
 
-    private static final Serializer<DomainObjectInfo> domainObjSerializer = new Serializer<DomainObjectInfo>(
-            DomainObjectInfo.class);
+    private static final Serializer<ObjectIdentityImpl> domainObjSerializer = new Serializer<ObjectIdentityImpl>(
+            ObjectIdentityImpl.class);
 
-    private static final Serializer<AceInfo> aceSerializer = new Serializer<AceInfo>(AceInfo.class);
+    private static final Serializer<LegacyAceInfo> aceSerializer = new Serializer<LegacyAceInfo>(LegacyAceInfo.class);
 
     private static final Serializer<UserGrantedAuthority[]> ugaSerializer = new Serializer<UserGrantedAuthority[]>(
             UserGrantedAuthority[].class);
@@ -124,13 +128,13 @@ public class AclTableMigrationTool {
                     Result result = rs.next();
                     while (result != null) {
                         AclRecord record = new AclRecord();
-                        DomainObjectInfo object = getDomainObjectInfoFromRs(result);
+                        ObjectIdentityImpl object = getDomainObjectInfoFromRs(result);
                         record.setDomainObjectInfo(object);
                         record.setParentDomainObjectInfo(getParentDomainObjectInfoFromRs(result));
                         record.setOwnerInfo(getOwnerSidInfo(result));
                         record.setEntriesInheriting(getInheriting(result));
                         record.setAllAceInfo(getAllAceInfo(result));
-                        store.putResourceWithoutCheck(AclService.getQueryKeyById(object.getId()), record,
+                        store.putResourceWithoutCheck(AclService.resourceKey(object.getId()), record,
                                 System.currentTimeMillis(), AclService.SERIALIZER);
                         result = rs.next();
                     }
@@ -192,18 +196,16 @@ public class AclTableMigrationTool {
 
     }
 
-    private DomainObjectInfo getDomainObjectInfoFromRs(Result result) {
+    private ObjectIdentityImpl getDomainObjectInfoFromRs(Result result) {
         String type = new String(result.getValue(Bytes.toBytes(AclConstant.ACL_INFO_FAMILY),
                 Bytes.toBytes(AclConstant.ACL_INFO_FAMILY_TYPE_COLUMN)));
         String id = new String(result.getRow());
-        DomainObjectInfo newInfo = new DomainObjectInfo();
-        newInfo.setId(id);
-        newInfo.setType(type);
+        ObjectIdentityImpl newInfo = new ObjectIdentityImpl(type, id);
         return newInfo;
     }
 
-    private DomainObjectInfo getParentDomainObjectInfoFromRs(Result result) throws IOException {
-        DomainObjectInfo parentInfo = domainObjSerializer.deserialize(result.getValue(
+    private ObjectIdentityImpl getParentDomainObjectInfoFromRs(Result result) throws IOException {
+        ObjectIdentityImpl parentInfo = domainObjSerializer.deserialize(result.getValue(
                 Bytes.toBytes(AclConstant.ACL_INFO_FAMILY), Bytes.toBytes(AclConstant.ACL_INFO_FAMILY_PARENT_COLUMN)));
         return parentInfo;
     }
@@ -220,14 +222,14 @@ public class AclTableMigrationTool {
         return owner;
     }
 
-    private Map<String, AceInfo> getAllAceInfo(Result result) throws IOException {
-        Map<String, AceInfo> allAceInfoMap = new HashMap<>();
+    private Map<String, LegacyAceInfo> getAllAceInfo(Result result) throws IOException {
+        Map<String, LegacyAceInfo> allAceInfoMap = new HashMap<>();
         NavigableMap<byte[], byte[]> familyMap = result.getFamilyMap(Bytes.toBytes(AclConstant.ACL_ACES_FAMILY));
 
         if (familyMap != null && !familyMap.isEmpty()) {
             for (Map.Entry<byte[], byte[]> entry : familyMap.entrySet()) {
                 String sid = new String(entry.getKey());
-                AceInfo aceInfo = aceSerializer.deserialize(entry.getValue());
+                LegacyAceInfo aceInfo = aceSerializer.deserialize(entry.getValue());
                 if (null != aceInfo) {
                     allAceInfoMap.put(sid, aceInfo);
                 }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/DomainObjectInfo.java b/server-base/src/main/java/org/apache/kylin/rest/service/DomainObjectInfo.java
deleted file mode 100644
index f07a65e..0000000
--- a/server-base/src/main/java/org/apache/kylin/rest/service/DomainObjectInfo.java
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
-*/
-
-package org.apache.kylin.rest.service;
-
-import org.springframework.security.acls.model.ObjectIdentity;
-
-
-class DomainObjectInfo {
-    private String id;
-    private String type;
-
-    public DomainObjectInfo() {
-    }
-
-    public DomainObjectInfo(ObjectIdentity oid) {
-        super();
-        this.id = (String) oid.getIdentifier();
-        this.type = oid.getType();
-    }
-
-    public String getId() {
-        return id;
-    }
-
-    public void setId(String id) {
-        this.id = id;
-    }
-
-    public String getType() {
-        return type;
-    }
-
-    public void setType(String type) {
-        this.type = type;
-    }
-}
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 16bc971..217b54c 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
@@ -121,9 +121,9 @@ public class AccessControllerTest extends ServiceTestBase implements AclEntityTy
         aes = accessController.grant(CUBE_INSTANCE, "a24ca905-1fc6-4f67-985c-38fa5aeafd92", accessRequest);
         Assert.assertTrue(aes.size() == 1);
 
-        Long aeId = null;
+        int aeId = 0;
         for (AccessEntryResponse ae : aes) {
-            aeId = (Long) ae.getId();
+            aeId = (Integer) ae.getId();
         }
         Assert.assertNotNull(aeId);
 
@@ -134,7 +134,7 @@ public class AccessControllerTest extends ServiceTestBase implements AclEntityTy
         aes = accessController.update(CUBE_INSTANCE, "a24ca905-1fc6-4f67-985c-38fa5aeafd92", accessRequest);
         Assert.assertTrue(aes.size() == 1);
         for (AccessEntryResponse ae : aes) {
-            aeId = (Long) ae.getId();
+            aeId = (Integer) ae.getId();
         }
         Assert.assertNotNull(aeId);
 
@@ -167,7 +167,7 @@ public class AccessControllerTest extends ServiceTestBase implements AclEntityTy
         //revoke auth
         swichToAdmin();
         AccessRequest request = getAccessRequest(ANALYST, READ, true);
-        request.setAccessEntryId((Long) aes.get(0).getId());
+        request.setAccessEntryId((Integer) aes.get(0).getId());
         accessController.revoke(PROJECT_INSTANCE, project.getUuid(), request);
         swichToAnalyst();
         projects = projectController.getProjects(10000, 0);
@@ -215,7 +215,7 @@ public class AccessControllerTest extends ServiceTestBase implements AclEntityTy
             //correct
         }
         swichToAdmin();
-        accessRequest.setAccessEntryId((Long) aes.get(0).getId());
+        accessRequest.setAccessEntryId((Integer) aes.get(0).getId());
         accessController.revoke(PROJECT_INSTANCE, projects.get(0).getUuid(), accessRequest);
         swichToAnalyst();
         try {
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 3b9dca2..8f18ed1 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
@@ -31,7 +31,9 @@ import org.apache.kylin.metadata.project.ProjectInstance;
 import org.apache.kylin.rest.response.AccessEntryResponse;
 import org.apache.kylin.rest.security.AclPermission;
 import org.apache.kylin.rest.security.AclPermissionFactory;
+import org.apache.kylin.rest.service.AclServiceTest.MockAclEntity;
 import org.junit.Assert;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -43,7 +45,6 @@ import org.springframework.security.acls.model.Sid;
 import com.fasterxml.jackson.core.JsonProcessingException;
 
 /**
- * @author xduo
  */
 public class AccessServiceTest extends ServiceTestBase {
 
@@ -74,9 +75,9 @@ public class AccessServiceTest extends ServiceTestBase {
         Assert.assertNotNull(adminSid);
         Assert.assertNotNull(AclPermissionFactory.getPermissions());
 
-        AclEntity ae = new MockAclEntity("test-domain-object");
+        AclEntity ae = new AclServiceTest.MockAclEntity("test-domain-object");
         accessService.clean(ae, true);
-        AclEntity attachedEntity = new MockAclEntity("attached-domain-object");
+        AclEntity attachedEntity = new AclServiceTest.MockAclEntity("attached-domain-object");
         accessService.clean(attachedEntity, true);
 
         // test getAcl
@@ -97,12 +98,12 @@ public class AccessServiceTest extends ServiceTestBase {
         acl = accessService.grant(ae, AclPermission.ADMINISTRATION, modeler);
         Assert.assertEquals(accessService.generateAceResponses(acl).size(), 2);
 
-        Long modelerEntryId = null;
+        int modelerEntryId = 0;
         for (AccessControlEntry ace : acl.getEntries()) {
             PrincipalSid sid = (PrincipalSid) ace.getSid();
 
             if (sid.getPrincipal().equals("MODELER")) {
-                modelerEntryId = (Long) ace.getId();
+                modelerEntryId = (Integer) ace.getId();
                 Assert.assertTrue(ace.getPermission() == AclPermission.ADMINISTRATION);
             }
         }
@@ -116,7 +117,7 @@ public class AccessServiceTest extends ServiceTestBase {
             PrincipalSid sid = (PrincipalSid) ace.getSid();
 
             if (sid.getPrincipal().equals("MODELER")) {
-                modelerEntryId = (Long) ace.getId();
+                modelerEntryId = (Integer) ace.getId();
                 Assert.assertTrue(ace.getPermission() == AclPermission.READ);
             }
         }
@@ -147,21 +148,19 @@ public class AccessServiceTest extends ServiceTestBase {
         Assert.assertNull(attachedEntityAcl);
     }
 
-    public class MockAclEntity implements AclEntity {
-
-        private String id;
-
-        /**
-         * @param id
-         */
-        public MockAclEntity(String id) {
-            super();
-            this.id = id;
-        }
-
-        @Override
-        public String getId() {
-            return id;
+    @Ignore
+    @Test
+    public void test100000Entries() throws JsonProcessingException {
+        MockAclEntity ae = new MockAclEntity("100000Entries");
+        long time = System.currentTimeMillis();
+        for (int i = 0; i < 100000; i++) {
+            if (i % 10 == 0) {
+                long now = System.currentTimeMillis();
+                System.out.println((now - time) + " ms for last 10 entries, total " + i);
+                time = now;
+            }
+            Sid sid = accessService.getSid("USER" + i, true);
+            accessService.grant(ae, AclPermission.OPERATION, sid);
         }
     }
 }
diff --git a/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
new file mode 100644
index 0000000..ad0d524
--- /dev/null
+++ b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+package org.apache.kylin.rest.service;
+
+import java.util.List;
+
+import org.apache.kylin.common.persistence.AclEntity;
+import org.apache.kylin.rest.security.AclPermission;
+import org.apache.kylin.rest.security.springacl.AceImpl;
+import org.apache.kylin.rest.security.springacl.AclRecord;
+import org.apache.kylin.rest.security.springacl.MutableAclRecord;
+import org.apache.kylin.rest.security.springacl.ObjectIdentityImpl;
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.security.acls.domain.GrantedAuthoritySid;
+import org.springframework.security.acls.domain.PrincipalSid;
+import org.springframework.security.acls.model.AccessControlEntry;
+import org.springframework.security.acls.model.AlreadyExistsException;
+import org.springframework.security.acls.model.NotFoundException;
+import org.springframework.security.authentication.TestingAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
+
+/**
+ */
+public class AclServiceTest extends ServiceTestBase {
+
+    @Autowired
+    @Qualifier("aclService")
+    AclService aclService;
+
+    @Test
+    public void testReadFromLegacy() throws Exception {
+        ObjectIdentityImpl oid = oid("legacy-test-domain-object");
+        MutableAclRecord acl = aclService.readAcl(oid);
+        {
+            AclRecord rec = acl.getAclRecord();
+            Assert.assertEquals(oid, rec.getDomainObjectInfo());
+            Assert.assertNull(rec.getParentDomainObjectInfo());
+            Assert.assertNull(rec.getParentAcl());
+            Assert.assertEquals(new PrincipalSid("ADMIN"), rec.getOwner());
+            Assert.assertEquals(true, rec.isEntriesInheriting());
+
+            List<AccessControlEntry> entries = rec.getEntries();
+            Assert.assertEquals(2, entries.size());
+            AceImpl e0 = (AceImpl) entries.get(0);
+            Assert.assertEquals(rec, e0.getAcl());
+            Assert.assertEquals(Integer.valueOf(0), e0.getId());
+            Assert.assertEquals(new PrincipalSid("ADMIN"), e0.getSid());
+            Assert.assertEquals(16, e0.getPermissionMask());
+            AceImpl e1 = (AceImpl) entries.get(1);
+            Assert.assertEquals(rec, e1.getAcl());
+            Assert.assertEquals(Integer.valueOf(1), e1.getId());
+            Assert.assertEquals(new PrincipalSid("MODELER"), e1.getSid());
+            Assert.assertEquals(1, e1.getPermissionMask());
+        }
+
+        aclService.upsertAce(acl, new GrantedAuthoritySid("G1"), AclPermission.MANAGEMENT);
+        MutableAclRecord newAcl = aclService.readAcl(oid);
+        {
+            AclRecord rec = newAcl.getAclRecord();
+            List<AccessControlEntry> entries = rec.getEntries();
+            Assert.assertEquals(3, entries.size());
+            AceImpl e0 = (AceImpl) entries.get(0);
+            Assert.assertEquals(rec, e0.getAcl());
+            Assert.assertEquals(Integer.valueOf(0), e0.getId());
+            Assert.assertEquals(new GrantedAuthoritySid("G1"), e0.getSid());
+            Assert.assertEquals(32, e0.getPermissionMask());
+        }
+    }
+
+    @Test
+    public void testBasics() throws Exception {
+        switchToAdmin();
+        ObjectIdentityImpl parentOid = oid("parent-obj");
+        MutableAclRecord parentAcl = (MutableAclRecord) aclService.createAcl(parentOid);
+        
+        switchToAnalyst();
+        ObjectIdentityImpl childOid = oid("child-obj");
+        MutableAclRecord childAcl = (MutableAclRecord) aclService.createAcl(childOid);
+        MutableAclRecord childAclOutdated = aclService.readAcl(childOid);
+        
+        // test create on existing acl
+        try {
+            aclService.createAcl(childOid);
+            Assert.fail();
+        } catch (AlreadyExistsException ex) {
+            // expected
+        }
+        
+        // inherit parent
+        childAcl = aclService.inherit(childAcl, parentAcl);
+        Assert.assertEquals(parentOid, childAcl.getAclRecord().getParentDomainObjectInfo());
+        Assert.assertEquals(null, childAclOutdated.getAclRecord().getParentDomainObjectInfo());
+        
+        // update permission on an outdated ACL, retry should keep things going
+        PrincipalSid user1 = new PrincipalSid("user1");
+        MutableAclRecord childAcl2 = aclService.upsertAce(childAclOutdated, user1, AclPermission.ADMINISTRATION);
+        Assert.assertEquals(parentOid, childAcl2.getAclRecord().getParentDomainObjectInfo());
+        Assert.assertEquals(AclPermission.ADMINISTRATION, childAcl2.getAclRecord().getPermission(user1));
+        
+        // remove permission
+        MutableAclRecord childAcl3 = aclService.upsertAce(childAcl2, user1, null);
+        Assert.assertEquals(0, childAcl3.getAclRecord().getEntries().size());
+        
+        // delete ACL
+        aclService.deleteAcl(parentOid, true);
+        
+        try {
+            aclService.readAcl(childOid);
+            Assert.fail();
+        } catch (NotFoundException ex) {
+            // expected
+        }
+    }
+
+    private void switchToAdmin() {
+        Authentication adminAuth = new TestingAuthenticationToken("ADMIN", "ADMIN", "ROLE_ADMIN");
+        SecurityContextHolder.getContext().setAuthentication(adminAuth);
+    }
+
+    private void switchToAnalyst() {
+        Authentication analystAuth = new TestingAuthenticationToken("ANALYST", "ANALYST", "ROLE_ANALYST");
+        SecurityContextHolder.getContext().setAuthentication(analystAuth);
+    }
+
+    private ObjectIdentityImpl oid(String oid) {
+        return new ObjectIdentityImpl(new MockAclEntity(oid));
+    }
+
+    public static class MockAclEntity implements AclEntity {
+
+        private String id;
+
+        /**
+         * @param id
+         */
+        public MockAclEntity(String id) {
+            super();
+            this.id = id;
+        }
+
+        @Override
+        public String getId() {
+            return id;
+        }
+    }
+}

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.

[kylin] 07/07: minor, add ut for ValidateUtilTest.

Posted by bi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 56e73333dc6f4b7e074826070c806209d0bdab79
Author: Jiatao Tao <24...@qq.com>
AuthorDate: Sun Feb 11 16:36:43 2018 +0800

    minor, add ut for ValidateUtilTest.
---
 .../apache/kylin/rest/util/ValidateUtilTest.java   |  36 ------
 .../apache/kylin/rest/util/ValidateUtilTest.java   | 134 +++++++++++++++++++++
 2 files changed, 134 insertions(+), 36 deletions(-)

diff --git a/server-base/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java b/server-base/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java
deleted file mode 100644
index 8f3b14c..0000000
--- a/server-base/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
-*/
-
-package org.apache.kylin.rest.util;
-
-import org.junit.Assert;
-import org.junit.Test;
-
-public class ValidateUtilTest {
-
-    @Test
-    public void testIsAlphanumericUnderscore() {
-        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("a"));
-        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("123"));
-        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("abc123"));
-        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("abc123_"));
-        Assert.assertFalse(ValidateUtil.isAlphanumericUnderscore("abc@"));
-        Assert.assertFalse(ValidateUtil.isAlphanumericUnderscore(""));
-        Assert.assertFalse(ValidateUtil.isAlphanumericUnderscore(null));
-    }
-}
diff --git a/server/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java b/server/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java
new file mode 100644
index 0000000..62bd203
--- /dev/null
+++ b/server/src/test/java/org/apache/kylin/rest/util/ValidateUtilTest.java
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kylin.rest.util;
+
+import static org.apache.kylin.metadata.MetadataConstants.TYPE_GROUP;
+import static org.apache.kylin.metadata.MetadataConstants.TYPE_USER;
+
+import java.io.IOException;
+
+import org.apache.kylin.common.persistence.RootPersistentEntity;
+import org.apache.kylin.rest.security.AclPermission;
+import org.apache.kylin.rest.service.AccessService;
+import org.apache.kylin.rest.service.ServiceTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+
+import com.google.common.collect.Lists;
+
+public class ValidateUtilTest extends ServiceTestBase {
+    private final String PROJECT = "default";
+    private final String NOT_EXISTS = "not_exists";
+    private final String TABLE = "DEFAULT.TEST_KYLIN_FACT";
+
+    @Autowired
+    @Qualifier("validateUtil")
+    private ValidateUtil validateUtil;
+
+    @Autowired
+    @Qualifier("accessService")
+    private AccessService accessService;
+
+    @Test
+    public void testCheckIdentifiersExists() throws IOException {
+        validateUtil.checkIdentifiersExists("ADMIN", true);
+        validateUtil.checkIdentifiersExists("ROLE_ADMIN", false);
+
+        try {
+            validateUtil.checkIdentifiersExists(NOT_EXISTS, true);
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertEquals("Operation failed, user:not_exists not exists, please add first.", e.getMessage());
+        }
+
+        try {
+            validateUtil.checkIdentifiersExists(NOT_EXISTS, false);
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertEquals("Operation failed, group:not_exists not exists, please add first.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testGetAndValidateIdentifiers() throws IOException {
+        RootPersistentEntity ae = accessService.getAclEntity("ProjectInstance", "1eaca32a-a33e-4b69-83dd-0bb8b1f8c91b");
+        accessService.init(ae, AclPermission.ADMINISTRATION);
+        accessService.grant(ae, AclPermission.ADMINISTRATION, accessService.getSid("u1", true));
+        accessService.grant(ae, AclPermission.ADMINISTRATION, accessService.getSid("g1", false));
+
+        Assert.assertEquals(Lists.newArrayList("ADMIN", "u1"),
+                Lists.newArrayList(validateUtil.getAllIdentifiersInPrj(PROJECT, TYPE_USER)));
+        Assert.assertEquals(Lists.newArrayList("g1"),
+                Lists.newArrayList(validateUtil.getAllIdentifiersInPrj(PROJECT, TYPE_GROUP)));
+
+        validateUtil.validateIdentifiers(PROJECT, "u1", TYPE_USER);
+        try {
+            validateUtil.validateIdentifiers(PROJECT, NOT_EXISTS, TYPE_USER);
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertEquals("Operation failed, user:not_exists not exists in project.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testValidateTable() throws IOException {
+        validateUtil.validateTable(PROJECT, TABLE);
+        try {
+            validateUtil.validateTable(PROJECT, NOT_EXISTS);
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertEquals("Operation failed, table:not_exists not exists", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testValidateColumns() throws IOException {
+        validateUtil.validateColumn(PROJECT, TABLE, Lists.newArrayList("TRANS_ID"));
+        try {
+            validateUtil.validateColumn(PROJECT, TABLE, Lists.newArrayList(NOT_EXISTS));
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertEquals("Operation failed, column:not_exists not exists", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testValidateArgs() {
+        validateUtil.validateArgs(PROJECT);
+        try {
+            validateUtil.validateArgs("");
+            Assert.fail();
+        } catch (Exception e) {
+            Assert.assertTrue(e instanceof IllegalStateException);
+        }
+    }
+
+    @Test
+    public void testIsAlphanumericUnderscore() {
+        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("a"));
+        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("123"));
+        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("abc123"));
+        Assert.assertTrue(ValidateUtil.isAlphanumericUnderscore("abc123_"));
+        Assert.assertFalse(ValidateUtil.isAlphanumericUnderscore("abc@"));
+        Assert.assertFalse(ValidateUtil.isAlphanumericUnderscore(""));
+        Assert.assertFalse(ValidateUtil.isAlphanumericUnderscore(null));
+    }
+}

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.

[kylin] 05/07: KYLIN-3246, add manager for user.

Posted by bi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 7c9892b2f8163d63536d4b5e76759186584c3d60
Author: Jiatao Tao <24...@qq.com>
AuthorDate: Thu Feb 8 15:11:09 2018 +0800

    KYLIN-3246, add manager for user.
    
    minor, remove notify project ACL update.
---
 .../kylin/rest/security/KylinUserManager.java      | 141 +++++++++++++++++++++
 .../apache/kylin/rest/security/ManagedUser.java    |   5 +
 .../kylin/rest/service/KylinUserService.java       |  72 +++--------
 .../org/apache/kylin/rest/service/UserService.java |   2 -
 .../kylin/rest/security/KylinUserManagerTest.java  |  51 ++++++++
 .../apache/kylin/rest/service/UserServiceTest.java |  11 --
 6 files changed, 217 insertions(+), 65 deletions(-)

diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java b/server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java
new file mode 100644
index 0000000..c4b457d
--- /dev/null
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kylin.rest.security;
+
+import static org.apache.kylin.common.persistence.ResourceStore.USER_ROOT;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.persistence.ResourceStore;
+import org.apache.kylin.common.util.AutoReadWriteLock;
+import org.apache.kylin.metadata.cachesync.Broadcaster;
+import org.apache.kylin.metadata.cachesync.CachedCrudAssist;
+import org.apache.kylin.metadata.cachesync.CaseInsensitiveStringCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class KylinUserManager {
+
+    private static final Logger logger = LoggerFactory.getLogger(KylinUserManager.class);
+
+    public static KylinUserManager getInstance(KylinConfig config) {
+        return config.getManager(KylinUserManager.class);
+    }
+
+    // called by reflection
+    static KylinUserManager newInstance(KylinConfig config) throws IOException {
+        return new KylinUserManager(config);
+    }
+
+    // ============================================================================
+
+    private KylinConfig config;
+    // user ==> ManagedUser
+    private CaseInsensitiveStringCache<ManagedUser> userMap;
+    private CachedCrudAssist<ManagedUser> crud;
+    private AutoReadWriteLock lock = new AutoReadWriteLock();
+
+    public KylinUserManager(KylinConfig config) throws IOException {
+        logger.info("Initializing KylinUserManager with config " + config);
+        this.config = config;
+        this.userMap = new CaseInsensitiveStringCache<>(config, "user");
+        this.crud = new CachedCrudAssist<ManagedUser>(getStore(), USER_ROOT, "", ManagedUser.class, userMap, false) {
+            @Override
+            protected ManagedUser initEntityAfterReload(ManagedUser user, String resourceName) {
+                return user;
+            }
+        };
+
+        crud.reloadAll();
+        Broadcaster.getInstance(config).registerListener(new ManagedUserSyncListener(), "user");
+    }
+
+    private class ManagedUserSyncListener extends Broadcaster.Listener {
+
+        @Override
+        public void onEntityChange(Broadcaster broadcaster, String entity, Broadcaster.Event event, String cacheKey)
+                throws IOException {
+            try (AutoReadWriteLock.AutoLock l = lock.lockForWrite()) {
+                if (event == Broadcaster.Event.DROP)
+                    userMap.removeLocal(cacheKey);
+                else
+                    crud.reloadQuietly(cacheKey);
+            }
+        }
+    }
+
+    public KylinConfig getConfig() {
+        return config;
+    }
+
+    public ResourceStore getStore() {
+        return ResourceStore.getStore(this.config);
+    }
+
+    public ManagedUser get(String name) {
+        try (AutoReadWriteLock.AutoLock l = lock.lockForRead()) {
+            return userMap.get(name);
+        }
+    }
+
+    public List<ManagedUser> list() {
+        try (AutoReadWriteLock.AutoLock l = lock.lockForRead()) {
+            List<ManagedUser> users = new ArrayList<>();
+            users.addAll(userMap.values());
+            Collections.sort(users, new Comparator<ManagedUser>() {
+                @Override
+                public int compare(ManagedUser o1, ManagedUser o2) {
+                    return o1.getUsername().compareToIgnoreCase(o2.getUsername());
+                }
+            });
+            return users;
+        }
+    }
+
+    public void update(ManagedUser user) {
+        try (AutoReadWriteLock.AutoLock l = lock.lockForWrite()) {
+            ManagedUser exist = userMap.get(user.getUsername());
+            if (exist != null) {
+                user.setLastModified(exist.getLastModified());
+            }
+            crud.save(user);
+        } catch (IOException e) {
+            throw new RuntimeException("Can not update user.", e);
+        }
+    }
+
+    public void delete(String username) {
+        try (AutoReadWriteLock.AutoLock l = lock.lockForWrite()) {
+            crud.delete(username);
+        } catch (IOException e) {
+            throw new RuntimeException("Can not delete user.", e);
+        }
+    }
+
+    public boolean exists(String username) {
+        try (AutoReadWriteLock.AutoLock l = lock.lockForRead()) {
+            return userMap.containsKey(username);
+        }
+    }
+}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/ManagedUser.java b/server-base/src/main/java/org/apache/kylin/rest/security/ManagedUser.java
index 0fb2791..00e0045 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/security/ManagedUser.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/ManagedUser.java
@@ -113,6 +113,11 @@ public class ManagedUser extends RootPersistentEntity implements UserDetails {
         caterLegacy();
     }
 
+    @Override
+    public String resourceName() {
+        return username;
+    }
+
     public String getUsername() {
         return username;
     }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
index c35d737..a67fb4c 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
@@ -22,7 +22,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
-import javax.annotation.Nullable;
 import javax.annotation.PostConstruct;
 
 import org.apache.kylin.common.KylinConfig;
@@ -33,6 +32,7 @@ import org.apache.kylin.rest.constant.Constant;
 import org.apache.kylin.rest.exception.InternalErrorException;
 import org.apache.kylin.rest.msg.Message;
 import org.apache.kylin.rest.msg.MsgPicker;
+import org.apache.kylin.rest.security.KylinUserManager;
 import org.apache.kylin.rest.security.ManagedUser;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,9 +40,7 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
 
-import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.Lists;
 
 public class KylinUserService implements UserService {
 
@@ -84,29 +82,20 @@ public class KylinUserService implements UserService {
     public void updateUser(UserDetails user) {
         Preconditions.checkState(user instanceof ManagedUser, "User {} is not ManagedUser", user);
         ManagedUser managedUser = (ManagedUser) user;
-        try {
-            String id = getId(user.getUsername());
-            aclStore.putResourceWithoutCheck(id, managedUser, System.currentTimeMillis(), SERIALIZER);
-            logger.trace("update user : {}", user.getUsername());
-            setEvictCacheFlag(true);
-        } catch (IOException e) {
-            throw new InternalErrorException(e);
-        }
+        getKylinUserManager().update(managedUser);
+        logger.trace("update user : {}", user.getUsername());
+        setEvictCacheFlag(true);
     }
 
     @Override
     public void deleteUser(String userName) {
-        if (userName.equals(SUPER_ADMIN))
+        if (userName.equals(SUPER_ADMIN)) {
             throw new InternalErrorException("User " + userName + " is not allowed to be deleted.");
-
-        try {
-            String id = getId(userName);
-            aclStore.deleteResource(id);
-            logger.trace("delete user : {}", userName);
-            setEvictCacheFlag(true);
-        } catch (IOException e) {
-            throw new InternalErrorException(e);
         }
+
+        getKylinUserManager().delete(userName);
+        logger.trace("delete user : {}", userName);
+        setEvictCacheFlag(true);
     }
 
     @Override
@@ -116,12 +105,8 @@ public class KylinUserService implements UserService {
 
     @Override
     public boolean userExists(String userName) {
-        try {
-            logger.trace("judge user exist: {}", userName);
-            return aclStore.exists(getId(userName));
-        } catch (IOException e) {
-            throw new InternalErrorException(e);
-        }
+        logger.trace("judge user exist: {}", userName);
+        return getKylinUserManager().exists(userName);
     }
 
     /**
@@ -131,37 +116,17 @@ public class KylinUserService implements UserService {
     @Override
     public UserDetails loadUserByUsername(String userName) throws UsernameNotFoundException {
         Message msg = MsgPicker.getMsg();
-        try {
-            ManagedUser managedUser = aclStore.getResource(getId(userName), ManagedUser.class, SERIALIZER);
-            if (managedUser == null) {
-                throw new UsernameNotFoundException(String.format(msg.getUSER_NOT_FOUND(), userName));
-            }
-            logger.trace("load user : {}", userName);
-            return managedUser;
-        } catch (IOException e) {
-            throw new InternalErrorException(e);
+        ManagedUser managedUser = getKylinUserManager().get(userName);
+        if (managedUser == null) {
+            throw new UsernameNotFoundException(String.format(msg.getUSER_NOT_FOUND(), userName));
         }
+        logger.trace("load user : {}", userName);
+        return managedUser;
     }
 
     @Override
     public List<ManagedUser> listUsers() throws IOException {
-        return aclStore.getAllResources(DIR_PREFIX, ManagedUser.class, SERIALIZER);
-    }
-
-    @Override
-    public List<String> listUsernames() throws IOException {
-        List<String> paths = new ArrayList<>();
-        paths.addAll(aclStore.listResources(ResourceStore.USER_ROOT));
-        List<String> users = Lists.transform(paths, new Function<String, String>() {
-            @Nullable
-            @Override
-            public String apply(@Nullable String input) {
-                String[] path = input.split("/");
-                Preconditions.checkArgument(path.length == 3);
-                return path[2];
-            }
-        });
-        return users;
+        return getKylinUserManager().list();
     }
 
     @Override
@@ -183,4 +148,7 @@ public class KylinUserService implements UserService {
         return DIR_PREFIX + userName;
     }
 
+    private KylinUserManager getKylinUserManager() {
+        return KylinUserManager.getInstance(KylinConfig.getInstanceFromEnv());
+    }
 }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
index 5b50456..21c4cf9 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
@@ -32,8 +32,6 @@ public interface UserService extends UserDetailsManager {
 
     List<ManagedUser> listUsers() throws IOException;
 
-    List<String> listUsernames() throws IOException;
-
     List<String> listAdminUsers() throws IOException;
 
     //For performance consideration, list all users may be incomplete(eg. not load user's authorities until authorities has benn used).
diff --git a/server/src/test/java/org/apache/kylin/rest/security/KylinUserManagerTest.java b/server/src/test/java/org/apache/kylin/rest/security/KylinUserManagerTest.java
new file mode 100644
index 0000000..7024c4f
--- /dev/null
+++ b/server/src/test/java/org/apache/kylin/rest/security/KylinUserManagerTest.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kylin.rest.security;
+
+import java.io.IOException;
+
+import org.apache.kylin.rest.constant.Constant;
+import org.apache.kylin.rest.util.MultiNodeManagerTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+
+import com.google.common.collect.Lists;
+
+public class KylinUserManagerTest extends MultiNodeManagerTestBase {
+    @Test
+    public void testBasic() throws IOException, InterruptedException {
+        final KylinUserManager managerA = new KylinUserManager(configA);
+        final KylinUserManager managerB = new KylinUserManager(configB);
+        ManagedUser u1 = new ManagedUser("u1", "skippped", false, Lists.<GrantedAuthority> newArrayList());
+        managerA.update(u1);
+        Thread.sleep(1000);
+        ManagedUser u11 = new ManagedUser("u1", "password", false,
+                Lists.<GrantedAuthority> newArrayList(new SimpleGrantedAuthority(Constant.ROLE_ANALYST)));
+        managerB.update(u11);
+        Thread.sleep(1000);
+        Assert.assertEquals("password", managerA.get("u1").getPassword());
+        Assert.assertEquals("password", managerB.get("u1").getPassword());
+        managerB.delete("u1");
+        Thread.sleep(1000);
+        Assert.assertNull(managerA.get("u1"));
+        Assert.assertNull(managerB.get("u1"));
+    }
+}
diff --git a/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java
index 2b3e0f5..284469c 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/UserServiceTest.java
@@ -22,7 +22,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
-import com.google.common.collect.Lists;
 import org.apache.kylin.rest.constant.Constant;
 import org.apache.kylin.rest.exception.InternalErrorException;
 import org.apache.kylin.rest.security.ManagedUser;
@@ -34,9 +33,6 @@ import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UserDetails;
 
-/**
- * 
- */
 public class UserServiceTest extends ServiceTestBase {
 
     @Autowired
@@ -65,13 +61,6 @@ public class UserServiceTest extends ServiceTestBase {
     }
 
     @Test
-    public void testGetAllUserNames() throws IOException {
-        List<String> users = userService.listUsernames();
-        List<String> expected = Lists.newArrayList("ADMIN", "ANALYST", "MODELER");
-        Assert.assertEquals(expected, users);
-    }
-
-    @Test
     public void testDeleteAdmin() throws IOException {
         try {
             userService.deleteUser("ADMIN");

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.

[kylin] 03/07: minor refactor about ACL.

Posted by bi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 94a197849e9fb46b52e898adb01cd81a7bc12d78
Author: Jiatao Tao <24...@qq.com>
AuthorDate: Sun Feb 4 21:06:21 2018 +0800

    minor refactor about ACL.
    
    minor.
---
 ...eAclController.java => TableACLController.java} |  22 +++-
 .../apache/kylin/rest/service/QueryService.java    | 146 ++++++---------------
 .../org/apache/kylin/rest/util/ValidateUtil.java   |  32 +++--
 3 files changed, 74 insertions(+), 126 deletions(-)

diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/TableAclController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/TableACLController.java
similarity index 85%
rename from server-base/src/main/java/org/apache/kylin/rest/controller/TableAclController.java
rename to server-base/src/main/java/org/apache/kylin/rest/controller/TableACLController.java
index 198930c..1a97ec5 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/controller/TableAclController.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/controller/TableACLController.java
@@ -22,7 +22,11 @@ import java.io.IOException;
 import java.util.List;
 import java.util.Set;
 
+import static org.apache.kylin.metadata.MetadataConstants.TYPE_USER;
+
+import org.apache.kylin.rest.constant.Constant;
 import org.apache.kylin.rest.service.TableACLService;
+import org.apache.kylin.rest.service.UserService;
 import org.apache.kylin.rest.util.ValidateUtil;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -34,7 +38,7 @@ import org.springframework.web.bind.annotation.ResponseBody;
 
 @Controller
 @RequestMapping(value = "/acl")
-public class TableAclController extends BasicController {
+public class TableACLController extends BasicController {
 
     @Autowired
     @Qualifier("TableAclService")
@@ -44,12 +48,22 @@ public class TableAclController extends BasicController {
     @Qualifier("validateUtil")
     private ValidateUtil validateUtil;
 
+    @Autowired
+    @Qualifier("userService")
+    private UserService userService;
+
     @RequestMapping(value = "/table/{project}/{type}/{table:.+}", method = {RequestMethod.GET}, produces = {"application/json"})
     @ResponseBody
     public List<String> getUsersCanQueryTheTbl(@PathVariable String project, @PathVariable String type, @PathVariable String table) throws IOException {
         validateUtil.validateArgs(project, table);
         validateUtil.validateTable(project, table);
-        Set<String> allIdentifiers = validateUtil.getAllIdentifiers(project, type);
+        Set<String> allIdentifiers = validateUtil.getAllIdentifiersInPrj(project, type);
+        // add global admins
+        if (type.equals(TYPE_USER)) {
+            allIdentifiers.addAll(userService.listAdminUsers());
+        } else {
+            allIdentifiers.add(Constant.ROLE_ADMIN);
+        }
         return tableACLService.getCanAccessList(project, table, allIdentifiers, type);
     }
 
@@ -70,7 +84,7 @@ public class TableAclController extends BasicController {
             @PathVariable String table,
             @PathVariable String name) throws IOException {
         validateUtil.validateArgs(project, table, name);
-        validateUtil.validateIdentifiers(name, type);
+        validateUtil.validateIdentifiers(project, name, type);
         validateUtil.validateTable(project, table);
         tableACLService.addToTableACL(project, name, table, type);
     }
@@ -84,7 +98,7 @@ public class TableAclController extends BasicController {
             @PathVariable String table,
             @PathVariable String name) throws IOException {
         validateUtil.validateArgs(project, table, name);
-        validateUtil.validateIdentifiers(name, type);
+        validateUtil.validateIdentifiers(project, name, type);
         validateUtil.validateTable(project, table);
         tableACLService.deleteFromTableACL(project, name, table, type);
     }
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
index 5f67aa1..375b069 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
@@ -18,15 +18,35 @@
 
 package org.apache.kylin.rest.service;
 
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.base.CharMatcher;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Splitter;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import net.sf.ehcache.Cache;
-import net.sf.ehcache.CacheManager;
-import net.sf.ehcache.Element;
+import static org.apache.kylin.common.util.CheckUtil.checkCondition;
+
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.annotation.PostConstruct;
+
 import org.apache.calcite.avatica.ColumnMetaData.Rep;
 import org.apache.calcite.config.CalciteConnectionConfig;
 import org.apache.calcite.jdbc.CalcitePrepare;
@@ -49,7 +69,6 @@ import org.apache.kylin.common.util.DBUtils;
 import org.apache.kylin.common.util.JsonUtil;
 import org.apache.kylin.common.util.Pair;
 import org.apache.kylin.common.util.SetThreadName;
-import org.apache.kylin.cube.CubeInstance;
 import org.apache.kylin.cube.CubeManager;
 import org.apache.kylin.cube.cuboid.Cuboid;
 import org.apache.kylin.metadata.badquery.BadQueryEntry;
@@ -59,13 +78,11 @@ import org.apache.kylin.metadata.model.JoinTableDesc;
 import org.apache.kylin.metadata.model.ModelDimensionDesc;
 import org.apache.kylin.metadata.model.TableRef;
 import org.apache.kylin.metadata.project.ProjectInstance;
-import org.apache.kylin.metadata.project.RealizationEntry;
 import org.apache.kylin.metadata.querymeta.ColumnMeta;
 import org.apache.kylin.metadata.querymeta.ColumnMetaWithType;
 import org.apache.kylin.metadata.querymeta.SelectedColumnMeta;
 import org.apache.kylin.metadata.querymeta.TableMeta;
 import org.apache.kylin.metadata.querymeta.TableMetaWithType;
-import org.apache.kylin.metadata.realization.RealizationType;
 import org.apache.kylin.query.QueryConnection;
 import org.apache.kylin.query.relnode.OLAPContext;
 import org.apache.kylin.query.util.PushDownUtil;
@@ -89,43 +106,21 @@ import org.apache.kylin.rest.util.TableauInterceptor;
 import org.apache.kylin.shaded.htrace.org.apache.htrace.Sampler;
 import org.apache.kylin.shaded.htrace.org.apache.htrace.Trace;
 import org.apache.kylin.shaded.htrace.org.apache.htrace.TraceScope;
-import org.apache.kylin.storage.hybrid.HybridInstance;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
-import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Component;
 
-import javax.annotation.PostConstruct;
-import java.io.DataInputStream;
-import java.io.DataOutputStream;
-import java.io.IOException;
-import java.net.InetAddress;
-import java.net.UnknownHostException;
-import java.sql.Connection;
-import java.sql.DatabaseMetaData;
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
-import java.sql.ResultSetMetaData;
-import java.sql.SQLException;
-import java.sql.Statement;
-import java.sql.Time;
-import java.sql.Timestamp;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 
-import static org.apache.kylin.common.util.CheckUtil.checkCondition;
+import net.sf.ehcache.Cache;
+import net.sf.ehcache.CacheManager;
+import net.sf.ehcache.Element;
 
 /**
  * @author xduo
@@ -328,65 +323,10 @@ public class QueryService extends BasicService {
         logger.info(stringBuilder.toString());
     }
 
-    public void checkAuthorization(SQLResponse sqlResponse, String project) throws AccessDeniedException {
-
-        //project 
-        ProjectInstance projectInstance = getProjectManager().getProject(project);
-        try {
-            if (aclEvaluate.hasProjectReadPermission(projectInstance)) {
-                return;
-            }
-        } catch (AccessDeniedException e) {
-            logger.warn(
-                    "Current user {} has no READ permission on current project {}, please ask Administrator for permission granting.");
-            //just continue
-        }
-
-        String realizationsStr = sqlResponse.getCube();//CUBE[name=abc],HYBRID[name=xyz]
-
-        if (StringUtils.isEmpty(realizationsStr)) {
-            throw new AccessDeniedException(
-                    "Query pushdown requires having READ permission on project, please ask Administrator to grant you permissions");
-        }
-
-        String[] splits = StringUtils.split(realizationsStr, ",");
-
-        for (String split : splits) {
-
-            Iterable<String> parts = Splitter.on(CharMatcher.anyOf("[]=,")).split(split);
-            String[] partsStr = Iterables.toArray(parts, String.class);
-
-            if (RealizationType.HYBRID.toString().equals(partsStr[0])) {
-                // special care for hybrid
-                HybridInstance hybridInstance = getHybridManager().getHybridInstance(partsStr[2]);
-                Preconditions.checkNotNull(hybridInstance);
-                checkHybridAuthorization(hybridInstance);
-            } else {
-                CubeInstance cubeInstance = getCubeManager().getCube(partsStr[2]);
-                checkCubeAuthorization(cubeInstance);
-            }
-        }
-    }
-
-    private void checkCubeAuthorization(CubeInstance cube) throws AccessDeniedException {
-        Preconditions.checkState(aclEvaluate.hasCubeReadPermission(cube));
-    }
-
-    private void checkHybridAuthorization(HybridInstance hybridInstance) throws AccessDeniedException {
-        List<RealizationEntry> realizationEntries = hybridInstance.getRealizationEntries();
-        for (RealizationEntry realizationEntry : realizationEntries) {
-            String reName = realizationEntry.getRealization();
-            if (RealizationType.CUBE == realizationEntry.getType()) {
-                CubeInstance cubeInstance = getCubeManager().getCube(reName);
-                checkCubeAuthorization(cubeInstance);
-            } else if (RealizationType.HYBRID == realizationEntry.getType()) {
-                HybridInstance innerHybridInstance = getHybridManager().getHybridInstance(reName);
-                checkHybridAuthorization(innerHybridInstance);
-            }
-        }
-    }
-
     public SQLResponse doQueryWithCache(SQLRequest sqlRequest) {
+        long t = System.currentTimeMillis();
+        aclEvaluate.checkProjectReadPermission(sqlRequest.getProject());
+        logger.info("Check query permission in " + (System.currentTimeMillis() - t) + " ms.");
         return doQueryWithCache(sqlRequest, false);
     }
 
@@ -458,10 +398,6 @@ public class QueryService extends BasicService {
                 Trace.addTimelineAnnotation("response without real execution");
             }
 
-            // check authorization before return, since the response may come from cache
-            if (!sqlResponse.getIsException())
-                checkQueryAuth(sqlResponse, project);
-
             sqlResponse.setDuration(queryContext.getAccumulatedMillis());
             sqlResponse.setTraceUrl(traceUrl);
             logQuery(queryContext.getQueryId(), sqlRequest, sqlResponse);
@@ -611,12 +547,6 @@ public class QueryService extends BasicService {
         return response;
     }
 
-    protected void checkQueryAuth(SQLResponse sqlResponse, String project) throws AccessDeniedException {
-        if (!sqlResponse.getIsException() && KylinConfig.getInstanceFromEnv().isQuerySecureEnabled()) {
-            checkAuthorization(sqlResponse, project);
-        }
-    }
-
     private SQLResponse queryWithSqlMassage(SQLRequest sqlRequest) throws Exception {
         Connection conn = null;
 
diff --git a/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java b/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java
index 90d41ca..c250da7 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java
@@ -32,7 +32,6 @@ import org.apache.kylin.metadata.MetadataConstants;
 import org.apache.kylin.metadata.model.ColumnDesc;
 import org.apache.kylin.metadata.model.TableDesc;
 import org.apache.kylin.metadata.project.ProjectInstance;
-import org.apache.kylin.rest.constant.Constant;
 import org.apache.kylin.rest.service.AccessService;
 import org.apache.kylin.rest.service.IUserGroupService;
 import org.apache.kylin.rest.service.ProjectService;
@@ -54,10 +53,6 @@ public class ValidateUtil {
     private final static Pattern alphaNumUnderscorePattren = Pattern.compile("[a-zA-Z0-9_]+");
 
     @Autowired
-    @Qualifier("userService")
-    private UserService userService;
-
-    @Autowired
     @Qualifier("tableService")
     private TableService tableService;
 
@@ -70,6 +65,10 @@ public class ValidateUtil {
     private AccessService accessService;
 
     @Autowired
+    @Qualifier("userService")
+    private UserService userService;
+
+    @Autowired
     @Qualifier("userGroupService")
     private IUserGroupService userGroupService;
 
@@ -77,18 +76,25 @@ public class ValidateUtil {
         return toCheck == null ? false : alphaNumUnderscorePattren.matcher(toCheck).matches();
     }
 
-    //Identifiers may be user or user authority(you may call role or group)
-    public void validateIdentifiers(String name, String type) throws IOException {
-        if (type.equalsIgnoreCase(MetadataConstants.TYPE_USER) && !userService.userExists(name)) {
-            throw new RuntimeException("Operation failed, user:" + name + " not exists");
+    public void checkIdentifiersExists(String name, boolean isPrincipal) throws IOException {
+        if (isPrincipal && !userService.userExists(name)) {
+            throw new RuntimeException("Operation failed, user:" + name + " not exists, please add first.");
+        }
+        if (!isPrincipal && !userGroupService.exists(name)) {
+            throw new RuntimeException("Operation failed, group:" + name + " not exists, please add first.");
         }
-        if (type.equalsIgnoreCase(MetadataConstants.TYPE_GROUP) && !userGroupService.exists(name)) {
-            throw new RuntimeException("Operation failed, group:" + name + " not exists");
+    }
+
+    //Identifiers may be user or user authority(you may call role or group)
+    public void validateIdentifiers(String prj, String name, String type) throws IOException {
+        Set<String> allIdentifiers = getAllIdentifiersInPrj(prj, type);
+        if (!allIdentifiers.contains(name)) {
+            throw new RuntimeException("Operation failed, identifiers:" + name + " not exists");
         }
     }
 
     //get all users/user authorities that has permission in the project
-    public Set<String> getAllIdentifiers(String project, String type) throws IOException {
+    public Set<String> getAllIdentifiersInPrj(String project, String type) throws IOException {
         List<Sid> allSids = getAllSids(project);
         if (type.equalsIgnoreCase(MetadataConstants.TYPE_USER)) {
             return getUsersInPrj(allSids);
@@ -99,7 +105,6 @@ public class ValidateUtil {
 
     private Set<String> getAuthoritiesInPrj(List<Sid> allSids) {
         Set<String> allAuthorities = new TreeSet<>();
-        allAuthorities.add(Constant.ROLE_ADMIN);
         for (Sid sid : allSids) {
             if (sid instanceof GrantedAuthoritySid) {
                 allAuthorities.add(((GrantedAuthoritySid) sid).getGrantedAuthority());
@@ -110,7 +115,6 @@ public class ValidateUtil {
 
     private Set<String> getUsersInPrj(List<Sid> allSids) throws IOException {
         Set<String> allUsers = new TreeSet<>();
-        allUsers.addAll(userService.listAdminUsers());
         for (Sid sid : allSids) {
             if (sid instanceof PrincipalSid) {
                 allUsers.add(((PrincipalSid) sid).getPrincipal());

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.

[kylin] 06/07: KYLIN-3248, add batch grant API for project ACL.

Posted by bi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

billyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit fde86c2d67bdc08b057ab31698a7b01986518b34
Author: Jiatao Tao <24...@qq.com>
AuthorDate: Thu Feb 8 20:56:36 2018 +0800

    KYLIN-3248, add batch grant API for project ACL.
    
    minor, change the batch reqs.
---
 .../kylin/rest/controller/AccessController.java    | 25 ++++++++++++++++++++
 .../apache/kylin/rest/service/AccessService.java   | 26 ++++++++++++++++++---
 .../org/apache/kylin/rest/service/AclService.java  | 11 +++++++++
 .../kylin/rest/service/AccessServiceTest.java      | 20 ++++++++++++++++
 .../apache/kylin/rest/service/AclServiceTest.java  | 27 ++++++++++++++++++++++
 5 files changed, 106 insertions(+), 3 deletions(-)

diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java
index 56cae10..b9a00b3 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/controller/AccessController.java
@@ -20,8 +20,11 @@ package org.apache.kylin.rest.controller;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
+import com.google.common.base.Preconditions;
 import org.apache.kylin.common.persistence.AclEntity;
 import org.apache.kylin.common.util.Pair;
 import org.apache.kylin.metadata.MetadataConstants;
@@ -156,6 +159,28 @@ public class AccessController extends BasicController implements InitializingBea
     }
 
     /**
+     * Batch API.Grant a new access on a domain object to a user/role
+     */
+    @RequestMapping(value = "batch/{type}/{uuid}", method = { RequestMethod.POST }, produces = { "application/json" })
+    @ResponseBody
+    public void batchGrant(@PathVariable String type, @PathVariable String uuid,
+            @RequestBody List<Object[]> reqs) throws IOException {
+        Map<Sid, Permission> sidToPerm = new HashMap<>();
+        AclEntity ae = accessService.getAclEntity(type, uuid);
+        for (Object[] req : reqs) {
+            Preconditions.checkArgument(req.length == 3, "error access requests.");
+            String name = (String) req[0];
+            boolean isPrincipal = (boolean) req[1];
+            validateUtil.checkIdentifiersExists(name, isPrincipal);
+
+            Sid sid = accessService.getSid(name, isPrincipal);
+            Permission permission = AclPermissionFactory.getPermission((String) req[2]);
+            sidToPerm.put(sid, permission);
+        }
+        accessService.batchGrant(ae, sidToPerm);
+    }
+
+    /**
      * Update a access on a domain object
      * 
      * @param accessRequest
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
index 09a89c8..dfac275 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/AccessService.java
@@ -100,6 +100,29 @@ public class AccessService {
 
     @Transactional
     @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN + " or hasPermission(#ae, 'ADMINISTRATION')")
+    public void batchGrant(AclEntity ae, Map<Sid, Permission> sidToPerm) {
+        Message msg = MsgPicker.getMsg();
+
+        if (ae == null)
+            throw new BadRequestException(msg.getACL_DOMAIN_NOT_FOUND());
+        if (sidToPerm == null)
+            throw new BadRequestException(msg.getACL_PERMISSION_REQUIRED());
+
+        MutableAclRecord acl;
+        try {
+            acl = aclService.readAcl(new ObjectIdentityImpl(ae));
+        } catch (NotFoundException e) {
+            acl = init(ae, null);
+        }
+
+        for (Sid sid : sidToPerm.keySet()) {
+            secureOwner(acl, sid);
+        }
+        aclService.batchUpsertAce(acl, sidToPerm);
+    }
+
+    @Transactional
+    @PreAuthorize(Constant.ACCESS_HAS_ROLE_ADMIN + " or hasPermission(#ae, 'ADMINISTRATION')")
     public MutableAclRecord grant(AclEntity ae, Permission permission, Sid sid) {
         Message msg = MsgPicker.getMsg();
 
@@ -304,9 +327,6 @@ public class AccessService {
 
     /**
      * Protect admin permission granted to acl owner.
-     *
-     * @param acl
-     * @param indexOfAce
      */
     private void secureOwner(MutableAclRecord acl, Sid sid) {
         Message msg = MsgPicker.getMsg();
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 adc7c30..f3e2393 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
@@ -243,6 +243,17 @@ public class AclService implements MutableAclService, InitializingBean {
         });
     }
 
+    void batchUpsertAce(MutableAclRecord acl, final Map<Sid, Permission> sidToPerm) {
+        updateAclWithRetry(acl, new AclRecordUpdater() {
+            @Override
+            public void update(AclRecord record) {
+                for (Sid sid : sidToPerm.keySet()) {
+                    record.upsertAce(sidToPerm.get(sid), sid);
+                }
+            }
+        });
+    }
+
     MutableAclRecord inherit(MutableAclRecord acl, final MutableAclRecord parentAcl) {
         return updateAclWithRetry(acl, new AclRecordUpdater() {
             @Override
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 8f18ed1..52f5cd2 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
@@ -22,7 +22,9 @@ import static org.apache.kylin.rest.security.AclEntityType.PROJECT_INSTANCE;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.apache.kylin.common.persistence.AclEntity;
 import org.apache.kylin.common.persistence.RootPersistentEntity;
@@ -31,6 +33,7 @@ import org.apache.kylin.metadata.project.ProjectInstance;
 import org.apache.kylin.rest.response.AccessEntryResponse;
 import org.apache.kylin.rest.security.AclPermission;
 import org.apache.kylin.rest.security.AclPermissionFactory;
+import org.apache.kylin.rest.security.springacl.MutableAclRecord;
 import org.apache.kylin.rest.service.AclServiceTest.MockAclEntity;
 import org.junit.Assert;
 import org.junit.Ignore;
@@ -40,6 +43,7 @@ import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.security.acls.domain.PrincipalSid;
 import org.springframework.security.acls.model.AccessControlEntry;
 import org.springframework.security.acls.model.Acl;
+import org.springframework.security.acls.model.Permission;
 import org.springframework.security.acls.model.Sid;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -148,6 +152,22 @@ public class AccessServiceTest extends ServiceTestBase {
         Assert.assertNull(attachedEntityAcl);
     }
 
+    @Test
+    public void testBatchGrant() {
+        AclEntity ae = new AclServiceTest.MockAclEntity("batch-grant");
+        final Map<Sid, Permission> sidToPerm = new HashMap<>();
+        for (int i = 0; i < 10; i++) {
+            sidToPerm.put(new PrincipalSid("u" + i), AclPermission.ADMINISTRATION);
+        }
+        accessService.batchGrant(ae, sidToPerm);
+        MutableAclRecord acl = accessService.getAcl(ae);
+        List<AccessControlEntry> e = acl.getEntries();
+        Assert.assertEquals(10, e.size());
+        for (int i = 0; i < e.size(); i++) {
+            Assert.assertEquals(new PrincipalSid("u" + i), e.get(i).getSid());
+        }
+    }
+
     @Ignore
     @Test
     public void test100000Entries() throws JsonProcessingException {
diff --git a/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
index 875a2a1..4aabee1 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/AclServiceTest.java
@@ -18,7 +18,10 @@
 
 package org.apache.kylin.rest.service;
 
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.apache.kylin.common.persistence.AclEntity;
 import org.apache.kylin.rest.security.AclPermission;
@@ -33,8 +36,12 @@ import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.security.acls.domain.GrantedAuthoritySid;
 import org.springframework.security.acls.domain.PrincipalSid;
 import org.springframework.security.acls.model.AccessControlEntry;
+import org.springframework.security.acls.model.Acl;
 import org.springframework.security.acls.model.AlreadyExistsException;
 import org.springframework.security.acls.model.NotFoundException;
+import org.springframework.security.acls.model.ObjectIdentity;
+import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.Sid;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -132,6 +139,26 @@ public class AclServiceTest extends ServiceTestBase {
         }
     }
 
+    @Test
+    public void testBatchUpsertAce() {
+        switchToAdmin();
+        ObjectIdentity oid = oid("acl");
+        MutableAclRecord acl = (MutableAclRecord) aclService.createAcl(oid);
+        final Map<Sid, Permission> sidToPerm = new HashMap<>();
+        for (int i = 0; i < 10; i++) {
+            sidToPerm.put(new PrincipalSid("u" + i), AclPermission.ADMINISTRATION);
+        }
+        aclService.batchUpsertAce(acl, sidToPerm);
+
+        for (Acl a : aclService.readAclsById(Collections.singletonList(oid)).values()) {
+            List<AccessControlEntry> e = a.getEntries();
+            Assert.assertEquals(10, e.size());
+            for (int i = 0; i < e.size(); i++) {
+                Assert.assertEquals(new PrincipalSid("u" + i), e.get(i).getSid());
+            }
+        }
+    }
+
     private void switchToAdmin() {
         Authentication adminAuth = new TestingAuthenticationToken("ADMIN", "ADMIN", "ROLE_ADMIN");
         SecurityContextHolder.getContext().setAuthentication(adminAuth);

-- 
To stop receiving notification emails like this one, please contact
billyliu@apache.org.