Repository: kylin Updated Branches: refs/heads/master da0e6b04a -> c86f30f88
KYLIN-3239 Refactor the ACL code about checkPermission and hasPermission Signed-off-by: Billy Liu <billy...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/c86f30f8 Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/c86f30f8 Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/c86f30f8 Branch: refs/heads/master Commit: c86f30f88a0713dabee8f9ca5dd99be9b063214f Parents: da0e6b0 Author: GuangYaoLee92 <1012461...@qq.com> Authored: Wed Feb 7 12:45:35 2018 +0800 Committer: Billy Liu <billy...@apache.org> Committed: Fri Feb 9 21:37:52 2018 +0800 ---------------------------------------------------------------------- .../rest/controller/ProjectController.java | 10 +-- .../apache/kylin/rest/service/CubeService.java | 29 ++++---- .../apache/kylin/rest/service/ModelService.java | 20 ++--- .../kylin/rest/service/ProjectService.java | 10 +-- .../org/apache/kylin/rest/util/AclEvaluate.java | 78 +++++++++++++------- 5 files changed, 75 insertions(+), 72 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/c86f30f8/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java index 902ed24..4eedb8e1 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java +++ b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java @@ -38,7 +38,6 @@ 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.stereotype.Controller; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; @@ -97,14 +96,7 @@ public class ProjectController extends BasicController { if (projectInstance == null) { continue; } - - boolean hasProjectPermission = false; - try { - hasProjectPermission = aclEvaluate.hasProjectReadPermission(projectInstance); - } catch (AccessDeniedException e) { - //ignore to continue - } - + boolean hasProjectPermission = aclEvaluate.hasProjectReadPermission(projectInstance); if (hasProjectPermission) { readableProjects.add(projectInstance); } http://git-wip-us.apache.org/repos/asf/kylin/blob/c86f30f8/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java b/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java index 4618b48..953fa58 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java @@ -132,14 +132,13 @@ public class CubeService extends BasicService implements InitializingBean { public List<CubeInstance> listAllCubes(final String cubeName, final String projectName, final String modelName, boolean exactMatch) { List<CubeInstance> cubeInstances = null; - ProjectInstance project = (null != projectName) ? getProjectManager().getProject(projectName) : null; - if (null == project) { + if (null == projectName) { cubeInstances = getCubeManager().listAllCubes(); aclEvaluate.checkIsGlobalAdmin(); } else { cubeInstances = listAllCubes(projectName); - aclEvaluate.hasProjectReadPermission(project); + aclEvaluate.checkProjectReadPermission(projectName); } List<CubeInstance> filterModelCubes = new ArrayList<CubeInstance>(); @@ -170,7 +169,7 @@ public class CubeService extends BasicService implements InitializingBean { } public CubeInstance updateCubeCost(CubeInstance cube, int cost) throws IOException { - aclEvaluate.hasProjectWritePermission(cube.getProjectInstance()); + aclEvaluate.checkProjectWritePermission(cube); if (cube.getCost() == cost) { // Do nothing return cube; @@ -258,7 +257,7 @@ public class CubeService extends BasicService implements InitializingBean { public CubeDesc updateCubeAndDesc(CubeInstance cube, CubeDesc desc, String newProjectName, boolean forceUpdate) throws IOException { - aclEvaluate.hasProjectWritePermission(cube.getProjectInstance()); + aclEvaluate.checkProjectWritePermission(cube); Message msg = MsgPicker.getMsg(); final List<CubingJob> cubingJobs = jobService.listJobsByRealizationName(cube.getName(), null, @@ -287,7 +286,7 @@ public class CubeService extends BasicService implements InitializingBean { } public void deleteCube(CubeInstance cube) throws IOException { - aclEvaluate.hasProjectWritePermission(cube.getProjectInstance()); + aclEvaluate.checkProjectWritePermission(cube); Message msg = MsgPicker.getMsg(); final List<CubingJob> cubingJobs = jobService.listJobsByRealizationName(cube.getName(), null, @@ -316,7 +315,7 @@ public class CubeService extends BasicService implements InitializingBean { * @throws JobException */ public CubeInstance purgeCube(CubeInstance cube) throws IOException { - aclEvaluate.hasProjectOperationPermission(cube.getProjectInstance()); + aclEvaluate.checkProjectOperationPermission(cube); Message msg = MsgPicker.getMsg(); String cubeName = cube.getName(); @@ -345,7 +344,7 @@ public class CubeService extends BasicService implements InitializingBean { * @throws JobException */ public CubeInstance disableCube(CubeInstance cube) throws IOException { - aclEvaluate.hasProjectWritePermission(cube.getProjectInstance()); + aclEvaluate.checkProjectWritePermission(cube); Message msg = MsgPicker.getMsg(); String cubeName = cube.getName(); @@ -359,7 +358,7 @@ public class CubeService extends BasicService implements InitializingBean { } public void checkEnableCubeCondition(CubeInstance cube) { - aclEvaluate.hasProjectWritePermission(cube.getProjectInstance()); + aclEvaluate.checkProjectWritePermission(cube); Message msg = MsgPicker.getMsg(); String cubeName = cube.getName(); @@ -447,7 +446,7 @@ public class CubeService extends BasicService implements InitializingBean { } public void updateCubeNotifyList(CubeInstance cube, List<String> notifyList) throws IOException { - aclEvaluate.hasProjectOperationPermission(cube.getProjectInstance()); + aclEvaluate.checkProjectOperationPermission(cube); CubeDesc desc = cube.getDescriptor(); desc.setNotifyList(notifyList); getCubeDescManager().updateCubeDesc(desc); @@ -455,7 +454,7 @@ public class CubeService extends BasicService implements InitializingBean { public CubeInstance rebuildLookupSnapshot(CubeInstance cube, String segmentName, String lookupTable) throws IOException { - aclEvaluate.hasProjectOperationPermission(cube.getProjectInstance()); + aclEvaluate.checkProjectOperationPermission(cube); CubeSegment seg = cube.getSegment(segmentName, SegmentStatusEnum.READY); getCubeManager().buildSnapshotTable(seg, lookupTable); @@ -463,7 +462,7 @@ public class CubeService extends BasicService implements InitializingBean { } public CubeInstance deleteSegment(CubeInstance cube, String segmentName) throws IOException { - aclEvaluate.hasProjectOperationPermission(cube.getProjectInstance()); + aclEvaluate.checkProjectOperationPermission(cube); Message msg = MsgPicker.getMsg(); CubeSegment toDelete = null; @@ -663,12 +662,12 @@ public class CubeService extends BasicService implements InitializingBean { } public void deleteDraft(Draft draft) throws IOException { - aclEvaluate.hasProjectWritePermission(getProjectManager().getProject(draft.getProject())); + aclEvaluate.checkProjectWritePermission(draft.getProject()); getDraftManager().delete(draft.getUuid()); } public CubeDesc updateCube(CubeInstance cube, CubeDesc desc, ProjectInstance project) throws IOException { - aclEvaluate.hasProjectWritePermission(cube.getProjectInstance()); + aclEvaluate.checkProjectWritePermission(cube); Message msg = MsgPicker.getMsg(); String projectName = project.getName(); @@ -703,7 +702,7 @@ public class CubeService extends BasicService implements InitializingBean { if (null == project) { aclEvaluate.checkIsGlobalAdmin(); } else { - aclEvaluate.hasProjectReadPermission(getProjectManager().getProject(project)); + aclEvaluate.checkProjectReadPermission(project); } List<Draft> result = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/kylin/blob/c86f30f8/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java index 463107b..23c0085 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java @@ -37,7 +37,6 @@ import org.apache.kylin.metadata.model.JoinsTree; import org.apache.kylin.metadata.model.ModelDimensionDesc; import org.apache.kylin.metadata.model.TableDesc; import org.apache.kylin.metadata.model.TblColRef; -import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.rest.exception.BadRequestException; import org.apache.kylin.rest.exception.ForbiddenException; import org.apache.kylin.rest.msg.Message; @@ -85,13 +84,12 @@ public class ModelService extends BasicService { public List<DataModelDesc> listAllModels(final String modelName, final String projectName, boolean exactMatch) throws IOException { List<DataModelDesc> models; - ProjectInstance project = (null != projectName) ? getProjectManager().getProject(projectName) : null; - if (null == project) { + if (null == projectName) { aclEvaluate.checkIsGlobalAdmin(); models = getDataModelManager().getModels(); } else { - aclEvaluate.hasProjectReadPermission(project); + aclEvaluate.checkProjectReadPermission(projectName); models = getDataModelManager().getModels(projectName); } @@ -128,7 +126,7 @@ public class ModelService extends BasicService { } public DataModelDesc createModelDesc(String projectName, DataModelDesc desc) throws IOException { - aclEvaluate.hasProjectWritePermission(getProjectManager().getProject(projectName)); + aclEvaluate.checkProjectWritePermission(projectName); Message msg = MsgPicker.getMsg(); if (getDataModelManager().getDataModelDesc(desc.getName()) != null) { throw new BadRequestException(String.format(msg.getDUPLICATE_MODEL_NAME(), desc.getName())); @@ -147,7 +145,7 @@ public class ModelService extends BasicService { } public void dropModel(DataModelDesc desc) throws IOException { - aclEvaluate.hasProjectWritePermission(desc.getProjectInstance()); + aclEvaluate.checkProjectWritePermission(desc.getProjectInstance().getName()); Message msg = MsgPicker.getMsg(); //check cube desc exist List<CubeDesc> cubeDescs = getCubeDescManager().listAllDesc(); @@ -369,11 +367,10 @@ public class ModelService extends BasicService { } public DataModelDesc getModel(final String modelName, final String projectName) throws IOException { - ProjectInstance project = (null != projectName) ? getProjectManager().getProject(projectName) : null; - if (null == project) { + if (null == projectName) { aclEvaluate.checkIsGlobalAdmin(); } else { - aclEvaluate.hasProjectReadPermission(project); + aclEvaluate.checkProjectReadPermission(projectName); } return getDataModelManager().getDataModelDesc(modelName); @@ -387,11 +384,10 @@ public class ModelService extends BasicService { } public List<Draft> listModelDrafts(String modelName, String projectName) throws IOException { - ProjectInstance project = (null != projectName) ? getProjectManager().getProject(projectName) : null; - if (null == project) { + if (null == projectName) { aclEvaluate.checkIsGlobalAdmin(); } else { - aclEvaluate.hasProjectReadPermission(project); + aclEvaluate.checkProjectReadPermission(projectName); } List<Draft> result = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/kylin/blob/c86f30f8/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java b/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java index 417fb10..7d56fff 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/ProjectService.java @@ -39,7 +39,6 @@ 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.access.prepost.PostFilter; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.context.SecurityContextHolder; @@ -168,14 +167,7 @@ public class ProjectService extends BasicService { if (projectInstance == null) { continue; } - - boolean hasProjectPermission = false; - try { - hasProjectPermission = aclEvaluate.hasProjectReadPermission(projectInstance); - } catch (AccessDeniedException e) { - //ignore to continue - } - + boolean hasProjectPermission = aclEvaluate.hasProjectReadPermission(projectInstance); if (hasProjectPermission) { readableProjects.add(projectInstance); } http://git-wip-us.apache.org/repos/asf/kylin/blob/c86f30f8/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java b/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java index 8ef7423..0632c88 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java +++ b/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java @@ -28,6 +28,7 @@ import org.apache.kylin.job.execution.ExecutableManager; import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.metadata.project.ProjectManager; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.AccessDeniedException; import org.springframework.stereotype.Component; @Component("aclEvaluate") @@ -51,52 +52,51 @@ public class AclEvaluate { return getProjectInstance(projectName); } - public boolean checkProjectAdminPermission(String projectName) { + public void checkProjectAdminPermission(String projectName) { ProjectInstance projectInstance = getProjectInstance(projectName); - return aclUtil.hasProjectAdminPermission(projectInstance); + aclUtil.hasProjectAdminPermission(projectInstance); } //for raw project - public boolean checkProjectReadPermission(String projectName) { + public void checkProjectReadPermission(String projectName) { ProjectInstance projectInstance = getProjectInstance(projectName); - return aclUtil.hasProjectReadPermission(projectInstance); + aclUtil.hasProjectReadPermission(projectInstance); } - public boolean checkProjectWritePermission(String projectName) { + public void checkProjectWritePermission(String projectName) { ProjectInstance projectInstance = getProjectInstance(projectName); - return aclUtil.hasProjectWritePermission(projectInstance); + aclUtil.hasProjectWritePermission(projectInstance); } - public boolean checkProjectOperationPermission(String projectName) { + public void checkProjectOperationPermission(String projectName) { ProjectInstance projectInstance = getProjectInstance(projectName); - return aclUtil.hasProjectOperationPermission(projectInstance); + aclUtil.hasProjectOperationPermission(projectInstance); } //for cube acl entity - public boolean checkProjectReadPermission(CubeInstance cube) { - return aclUtil.hasProjectReadPermission(cube.getProjectInstance()); + public void checkProjectReadPermission(CubeInstance cube) { + aclUtil.hasProjectReadPermission(cube.getProjectInstance()); } - - public boolean checkProjectWritePermission(CubeInstance cube) { - return aclUtil.hasProjectWritePermission(cube.getProjectInstance()); + public void checkProjectWritePermission(CubeInstance cube) { + aclUtil.hasProjectWritePermission(cube.getProjectInstance()); } - public boolean checkProjectOperationPermission(CubeInstance cube) { - return aclUtil.hasProjectOperationPermission(cube.getProjectInstance()); + public void checkProjectOperationPermission(CubeInstance cube) { + aclUtil.hasProjectOperationPermission(cube.getProjectInstance()); } //for job acl entity - public boolean checkProjectReadPermission(JobInstance job) { - return aclUtil.hasProjectReadPermission(getProjectByJob(job)); + public void checkProjectReadPermission(JobInstance job) { + aclUtil.hasProjectReadPermission(getProjectByJob(job)); } - public boolean checkProjectWritePermission(JobInstance job) { - return aclUtil.hasProjectWritePermission(getProjectByJob(job)); + public void checkProjectWritePermission(JobInstance job) { + aclUtil.hasProjectWritePermission(getProjectByJob(job)); } - public boolean checkProjectOperationPermission(JobInstance job) { - return aclUtil.hasProjectOperationPermission(getProjectByJob(job)); + public void checkProjectOperationPermission(JobInstance job) { + aclUtil.hasProjectOperationPermission(getProjectByJob(job)); } // ACL util's method, so that you can use AclEvaluate @@ -109,23 +109,47 @@ public class AclEvaluate { } public boolean hasProjectReadPermission(ProjectInstance project) { - return aclUtil.hasProjectReadPermission(project); + boolean _hasProjectReadPermission = false; + try { + _hasProjectReadPermission = aclUtil.hasProjectReadPermission(project); + } catch (AccessDeniedException e) { + //ignore to continue + } + return _hasProjectReadPermission; } public boolean hasProjectOperationPermission(ProjectInstance project) { - return aclUtil.hasProjectOperationPermission(project); + boolean _hasProjectOperationPermission = false; + try { + _hasProjectOperationPermission = aclUtil.hasProjectOperationPermission(project); + } catch (AccessDeniedException e) { + //ignore to continue + } + return _hasProjectOperationPermission; } public boolean hasProjectWritePermission(ProjectInstance project) { - return aclUtil.hasProjectWritePermission(project); + boolean _hasProjectWritePermission = false; + try { + _hasProjectWritePermission = aclUtil.hasProjectWritePermission(project); + } catch (AccessDeniedException e) { + //ignore to continue + } + return _hasProjectWritePermission; } public boolean hasProjectAdminPermission(ProjectInstance project) { - return aclUtil.hasProjectAdminPermission(project); + boolean _hasProjectAdminPermission = false; + try { + _hasProjectAdminPermission = aclUtil.hasProjectAdminPermission(project); + } catch (AccessDeniedException e) { + //ignore to continue + } + return _hasProjectAdminPermission; } - public boolean checkIsGlobalAdmin() { - return aclUtil.checkIsGlobalAdmin(); + public void checkIsGlobalAdmin() { + aclUtil.checkIsGlobalAdmin(); } } \ No newline at end of file
