From ae3185961e62c182b5a7de9df8a89330f0c07e57 Mon Sep 17 00:00:00 2001 From: GuangYaoLee92 <1012461549@qq.com> Date: Wed, 7 Feb 2018 12:45:35 +0800 Subject: [PATCH] KYLIN-3239 Refactor the ACL code about checkPermission and hasPermission --- .../kylin/rest/controller/ProjectController.java | 10 +-- .../org/apache/kylin/rest/service/CubeService.java | 29 ++++---- .../apache/kylin/rest/service/ModelService.java | 20 +++--- .../apache/kylin/rest/service/ProjectService.java | 10 +-- .../org/apache/kylin/rest/util/AclEvaluate.java | 78 ++++++++++++++-------- 5 files changed, 75 insertions(+), 72 deletions(-) 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 902ed24f7..4eedb8e10 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); } 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 c01308ddc..8b8a6ec0d 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 @@ -131,14 +131,13 @@ public class CubeService extends BasicService implements InitializingBean { public List listAllCubes(final String cubeName, final String projectName, final String modelName, boolean exactMatch) { List 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 filterModelCubes = new ArrayList(); @@ -169,7 +168,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; @@ -257,7 +256,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 cubingJobs = jobService.listJobsByRealizationName(cube.getName(), null, @@ -286,7 +285,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 cubingJobs = jobService.listJobsByRealizationName(cube.getName(), null, @@ -315,7 +314,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(); @@ -344,7 +343,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(); @@ -358,7 +357,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(); @@ -446,7 +445,7 @@ public class CubeService extends BasicService implements InitializingBean { } public void updateCubeNotifyList(CubeInstance cube, List notifyList) throws IOException { - aclEvaluate.hasProjectOperationPermission(cube.getProjectInstance()); + aclEvaluate.checkProjectOperationPermission(cube); CubeDesc desc = cube.getDescriptor(); desc.setNotifyList(notifyList); getCubeDescManager().updateCubeDesc(desc); @@ -454,7 +453,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); @@ -462,7 +461,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; @@ -662,12 +661,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(); @@ -702,7 +701,7 @@ public class CubeService extends BasicService implements InitializingBean { if (null == project) { aclEvaluate.checkIsGlobalAdmin(); } else { - aclEvaluate.hasProjectReadPermission(getProjectManager().getProject(project)); + aclEvaluate.checkProjectReadPermission(project); } List result = new ArrayList<>(); 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 463107bf7..23c0085f2 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 listAllModels(final String modelName, final String projectName, boolean exactMatch) throws IOException { List 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 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 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 result = new ArrayList<>(); 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 417fb10e0..7d56fff0f 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); } 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 8ef7423c5..0632c8869 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 -- 2.15.1.windows.2