fix(系统设置): 用户越权访问的部分问题

--bug=1040150 --user=宋昌昌 用户越权访问安全漏洞 https://www.tapd.cn/33805590/s/1510941
This commit is contained in:
song-cc-rock 2024-04-29 14:43:18 +08:00 committed by 刘瑞斌
parent 8c0ba073b5
commit 52c1ba2376
10 changed files with 62 additions and 20 deletions

View File

@ -435,6 +435,7 @@ public class ApiScenarioController {
}
@GetMapping("versions/{scenarioId}")
@CheckOwner(resourceId = "#scenarioId", resourceType = "api_scenario")
public List<ApiScenarioDTO> getApiScenarioVersions(@PathVariable String scenarioId) {
return apiAutomationService.getApiScenarioVersions(scenarioId);
}

View File

@ -217,6 +217,7 @@ public class PermissionConstants {
public static final String PROJECT_ENTERPRISE_REPORT_COPY = "PROJECT_ENTERPRISE_REPORT:READ+COPY";
public static final String PROJECT_ENTERPRISE_REPORT_SCHEDULE = "PROJECT_ENTERPRISE_REPORT:READ+SCHEDULE";
public static final String PROJECT_ERROR_REPORT_LIBRARY_READ = "PROJECT_ERROR_REPORT_LIBRARY:READ";
public static final String PROJECT_ERROR_REPORT_LIBRARY_EXPORT = "PROJECT_ERROR_REPORT_LIBRARY:READ+EXPORT";
public static final String PROJECT_ERROR_REPORT_LIBRARY_CREATE = "PROJECT_ERROR_REPORT_LIBRARY:READ+CREATE";
public static final String PROJECT_ERROR_REPORT_LIBRARY_EDIT = "PROJECT_ERROR_REPORT_LIBRARY:READ+EDIT";

View File

@ -11,6 +11,7 @@ import io.metersphere.log.annotation.MsAuditLog;
import io.metersphere.request.ProjectRequest;
import io.metersphere.service.BaseProjectService;
import jakarta.annotation.Resource;
import org.apache.shiro.authz.annotation.Logical;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;
@ -43,6 +44,7 @@ public class BaseProjectController {
}
@GetMapping("/get/{id}")
@RequiresPermissions(value = {PermissionConstants.WORKSPACE_PROJECT_MANAGER_READ, PermissionConstants.PROJECT_MANAGER_READ}, logical = Logical.OR)
public Project getProject(@PathVariable String id) {
return baseProjectService.getProjectById(id);
}

View File

@ -20,6 +20,7 @@ import io.metersphere.i18n.Translator;
import io.metersphere.log.annotation.MsAuditLog;
import io.metersphere.request.EnvironmentRequest;
import jakarta.annotation.Resource;
import org.apache.commons.collections.CollectionUtils;
import org.apache.shiro.authz.annotation.Logical;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.json.JSONArray;
@ -60,7 +61,11 @@ public class TestEnvironmentController {
* @return
*/
@PostMapping("/list/{goPage}/{pageSize}")
@RequiresPermissions(value = PermissionConstants.PROJECT_ENVIRONMENT_READ)
public Pager<List<ApiTestEnvironmentWithBLOBs>> listByCondition(@PathVariable int goPage, @PathVariable int pageSize, @RequestBody EnvironmentRequest environmentRequest) {
if (CollectionUtils.isEmpty(environmentRequest.getProjectIds()) || !environmentRequest.getProjectIds().contains(SessionUtils.getCurrentProjectId())) {
MSException.throwException(Translator.get("check_owner_case"));
}
Page<Object> page = PageHelper.startPage(goPage, pageSize, true);
return PageUtils.setPageInfo(page, baseEnvironmentService.listByConditions(environmentRequest));
}

View File

@ -5,17 +5,19 @@ import io.metersphere.commons.constants.NoticeConstants;
import io.metersphere.commons.constants.OperLogConstants;
import io.metersphere.commons.constants.OperLogModule;
import io.metersphere.commons.constants.PermissionConstants;
import io.metersphere.commons.utils.SessionUtils;
import io.metersphere.dto.TestCaseCommentDTO;
import io.metersphere.log.annotation.MsAuditLog;
import io.metersphere.notice.annotation.SendNotice;
import io.metersphere.dto.TestCaseCommentDTO;
import io.metersphere.request.testreview.SaveCommentRequest;
import io.metersphere.security.CheckOwner;
import io.metersphere.service.TestCaseCommentService;
import io.metersphere.service.TestCaseService;
import jakarta.annotation.Resource;
import org.apache.shiro.authz.annotation.Logical;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.springframework.web.bind.annotation.*;
import jakarta.annotation.Resource;
import java.util.List;
@RequestMapping("/test/case/comment")
@ -30,6 +32,7 @@ public class TestCaseCommentController {
@MsAuditLog(module = OperLogModule.TRACK_TEST_CASE_REVIEW, type = OperLogConstants.CREATE, content = "#msClass.getLogDetails(#request.id)", msClass = TestCaseCommentService.class)
@SendNotice(taskType = NoticeConstants.TaskType.TRACK_TEST_CASE_TASK, target = "#targetClass.getTestCase(#request.caseId)", targetClass = TestCaseService.class,
event = NoticeConstants.Event.COMMENT, subject = "测试用例通知")
@CheckOwner(resourceId = "#request.caseId", resourceType = "test_case")
public TestCaseComment saveComment(@RequestBody SaveCommentRequest request) {
return testCaseCommentService.saveComment(request);
}
@ -59,13 +62,14 @@ public class TestCaseCommentController {
@RequiresPermissions(PermissionConstants.PROJECT_TRACK_REVIEW_READ_COMMENT)
@MsAuditLog(module = OperLogModule.TRACK_TEST_CASE_REVIEW, type = OperLogConstants.DELETE, beforeEvent = "#msClass.getLogDetails(#commentId)", msClass = TestCaseCommentService.class)
public void deleteComment(@PathVariable String commentId) {
testCaseCommentService.delete(commentId);
testCaseCommentService.delete(commentId, SessionUtils.getCurrentProjectId());
}
@PostMapping("/edit")
@RequiresPermissions(PermissionConstants.PROJECT_TRACK_REVIEW_READ_COMMENT)
@MsAuditLog(module = OperLogModule.TRACK_TEST_CASE_REVIEW, type = OperLogConstants.UPDATE, beforeEvent = "#msClass.getLogDetails(#request.id)", content = "#msClass.getLogDetails(#request.id)", msClass = TestCaseCommentService.class)
@CheckOwner(resourceId = "#request.caseId", resourceType = "test_case")
public TestCaseComment editComment(@RequestBody SaveCommentRequest request) {
return testCaseCommentService.edit(request);
return testCaseCommentService.edit(request, SessionUtils.getCurrentProjectId());
}
}

View File

@ -9,6 +9,7 @@ import io.metersphere.commons.constants.OperLogModule;
import io.metersphere.commons.constants.PermissionConstants;
import io.metersphere.commons.utils.PageUtils;
import io.metersphere.commons.utils.Pager;
import io.metersphere.commons.utils.SessionUtils;
import io.metersphere.dto.TestPlanCaseDTO;
import io.metersphere.log.annotation.MsAuditLog;
import io.metersphere.log.annotation.MsRequestLog;
@ -19,12 +20,12 @@ import io.metersphere.plan.request.function.TestPlanFuncCaseEditRequest;
import io.metersphere.plan.service.TestPlanTestCaseService;
import io.metersphere.request.ResetOrderRequest;
import io.metersphere.security.CheckOwner;
import jakarta.annotation.Resource;
import org.apache.commons.lang3.StringUtils;
import org.apache.shiro.authz.annotation.Logical;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.springframework.web.bind.annotation.*;
import jakarta.annotation.Resource;
import java.util.Arrays;
import java.util.List;
@ -101,7 +102,7 @@ public class TestPlanTestCaseController {
@GetMapping("/get/{caseId}")
@RequiresPermissions(PermissionConstants.PROJECT_TRACK_PLAN_READ)
public TestPlanCaseDTO getTestPlanCases(@PathVariable String caseId) {
return testPlanTestCaseService.get(caseId);
return testPlanTestCaseService.get(caseId, SessionUtils.getCurrentProjectId());
}
@PostMapping("recent/{count}")

View File

@ -9,9 +9,10 @@ import io.metersphere.commons.constants.OperLogModule;
import io.metersphere.commons.constants.PermissionConstants;
import io.metersphere.commons.utils.PageUtils;
import io.metersphere.commons.utils.Pager;
import io.metersphere.commons.utils.SessionUtils;
import io.metersphere.dto.TestReviewCaseDTO;
import io.metersphere.dto.TestReviewTestCaseEditResult;
import io.metersphere.log.annotation.MsAuditLog;
import io.metersphere.dto.TestReviewCaseDTO;
import io.metersphere.log.annotation.MsRequestLog;
import io.metersphere.request.ResetOrderRequest;
import io.metersphere.request.testplancase.TestReviewCaseBatchRequest;
@ -20,10 +21,10 @@ import io.metersphere.request.testreview.QueryCaseReviewRequest;
import io.metersphere.request.testreview.TestCaseReviewTestCaseEditRequest;
import io.metersphere.security.CheckOwner;
import io.metersphere.service.TestReviewTestCaseService;
import jakarta.annotation.Resource;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.springframework.web.bind.annotation.*;
import jakarta.annotation.Resource;
import java.util.List;
@RequestMapping("/test/review/case")
@ -103,7 +104,7 @@ public class TestReviewTestCaseController {
@GetMapping("/get/{reviewId}")
@RequiresPermissions(PermissionConstants.PROJECT_TRACK_REVIEW_READ)
public TestReviewCaseDTO get(@PathVariable String reviewId) {
return testReviewTestCaseService.get(reviewId);
return testReviewTestCaseService.get(reviewId, SessionUtils.getCurrentProjectId());
}
@GetMapping("/reviewer/status/{id}")

View File

@ -6,6 +6,7 @@ import io.metersphere.base.mapper.TestCaseMapper;
import io.metersphere.base.mapper.TestCaseTestMapper;
import io.metersphere.base.mapper.TestPlanMapper;
import io.metersphere.base.mapper.TestPlanTestCaseMapper;
import io.metersphere.base.mapper.ext.ExtCheckOwnerMapper;
import io.metersphere.base.mapper.ext.ExtTestPlanTestCaseMapper;
import io.metersphere.commons.constants.IssueRefType;
import io.metersphere.commons.constants.MicroServiceName;
@ -16,9 +17,9 @@ import io.metersphere.commons.utils.*;
import io.metersphere.constants.TestCaseCommentType;
import io.metersphere.dto.*;
import io.metersphere.excel.constants.TestPlanTestCaseStatus;
import io.metersphere.i18n.Translator;
import io.metersphere.log.vo.DetailColumn;
import io.metersphere.log.vo.OperatingLogDetails;
import io.metersphere.log.vo.StatusReference;
import io.metersphere.plan.dto.TestCaseReportStatusResultDTO;
import io.metersphere.plan.dto.TestPlanReportDataStruct;
import io.metersphere.plan.request.function.*;
@ -91,6 +92,8 @@ public class TestPlanTestCaseService {
private CustomFieldTestCaseService customFieldTestCaseService;
@Resource
private TestCaseSyncStatusService testCaseSyncStatusService;
@Resource
private ExtCheckOwnerMapper extCheckOwnerMapper;
private static final String CUSTOM_NUM = "custom_num";
private static final String NUM = "num";
@ -306,8 +309,9 @@ public class TestPlanTestCaseService {
request.setExecutor(user.getId());
}
public TestPlanCaseDTO get(String id) {
public TestPlanCaseDTO get(String id, String currentProjectId) {
TestPlanCaseDTO testPlanCaseDTO = extTestPlanTestCaseMapper.get(id);
checkPlanCaseOwner(testPlanCaseDTO.getCaseId(), currentProjectId);
ServiceUtils.buildCustomNumInfo(testPlanCaseDTO);
List<TestCaseTestDTO> testCaseTestDTOS = extTestPlanTestCaseMapper.listTestCaseTest(testPlanCaseDTO.getCaseId());
testCaseTestDTOS.forEach(this::setTestName);
@ -548,7 +552,7 @@ public class TestPlanTestCaseService {
public void calculatePlanReport(String planId, TestPlanReportDataStruct report) {
try {
List<PlanReportCaseDTO> planReportCaseDTOS = extTestPlanTestCaseMapper.selectForPlanReport(planId);
this.calculatePlanReport(planReportCaseDTOS, report);
calculatePlanReport(planReportCaseDTOS, report);
} catch (MSException e) {
LogUtil.error(e);
}
@ -574,7 +578,7 @@ public class TestPlanTestCaseService {
}
}
this.calculatePlanReport(planReportCaseDTOList, report);
calculatePlanReport(planReportCaseDTOList, report);
}
} catch (MSException e) {
LogUtil.error(e);
@ -599,7 +603,7 @@ public class TestPlanTestCaseService {
}
public List<TestPlanCaseDTO> getAllCases(String planId) {
return buildCaseInfo(this.getAllCasesByStatusList(planId, null));
return buildCaseInfo(getAllCasesByStatusList(planId, null));
}
public List<TestPlanCaseDTO> buildCaseInfo(List<TestPlanCaseDTO> cases) {
@ -647,4 +651,11 @@ public class TestPlanTestCaseService {
public int reduction(List<String> caseIds) {
return updateIsDel(caseIds, false);
}
private void checkPlanCaseOwner(String caseId, String currentProjectId) {
boolean hasPermission = extCheckOwnerMapper.checkoutOwner("test_case", currentProjectId, List.of(caseId));
if (!hasPermission) {
MSException.throwException(Translator.get("check_owner_case"));
}
}
}

View File

@ -133,8 +133,8 @@ public class TestCaseCommentService {
return "测试评审任务通知:" + user.getName() + "" + start + "" + "'" + testCaseWithBLOBs.getName() + "'" + "添加评论:" + testCaseComment.getDescription();
}
public void delete(String commentId) {
checkCommentOwner(commentId);
public void delete(String commentId, String currentProjectId) {
checkCommentOwner(commentId, currentProjectId);
testCaseCommentMapper.deleteByPrimaryKey(commentId);
mdFileService.deleteBySourceId(commentId);
}
@ -147,13 +147,13 @@ public class TestCaseCommentService {
testCaseCommentMapper.deleteByExample(example);
}
public TestCaseComment edit(SaveCommentRequest request) {
checkCommentOwner(request.getId());
public TestCaseComment edit(SaveCommentRequest request, String currentProjectId) {
checkCommentOwner(request.getId(), currentProjectId);
testCaseCommentMapper.updateByPrimaryKeySelective(request);
return testCaseCommentMapper.selectByPrimaryKey(request.getId());
}
private void checkCommentOwner(String commentId) {
private void checkCommentOwner(String commentId, String currentProjectId) {
TestCaseComment comment = testCaseCommentMapper.selectByPrimaryKey(commentId);
if (comment == null) {
MSException.throwException(Translator.get("resource_not_exist"));
@ -161,6 +161,10 @@ public class TestCaseCommentService {
if (!StringUtils.equals(comment.getAuthor(), SessionUtils.getUser().getId())) {
MSException.throwException(Translator.get("check_owner_comment"));
}
TestCaseWithBLOBs testCase = testCaseMapper.selectByPrimaryKey(comment.getCaseId());
if (testCase == null || !StringUtils.equals(testCase.getProjectId(), currentProjectId)) {
MSException.throwException(Translator.get("check_owner_comment"));
}
}
public String getLogDetails(String id) {

View File

@ -2,6 +2,7 @@ package io.metersphere.service;
import io.metersphere.base.domain.*;
import io.metersphere.base.mapper.*;
import io.metersphere.base.mapper.ext.ExtCheckOwnerMapper;
import io.metersphere.base.mapper.ext.ExtTestCaseReviewTestCaseMapper;
import io.metersphere.base.mapper.ext.ExtTestReviewCaseMapper;
import io.metersphere.commons.constants.TestCaseReviewStatus;
@ -16,6 +17,7 @@ import io.metersphere.dto.TestCaseReviewDTO;
import io.metersphere.dto.TestReviewCaseDTO;
import io.metersphere.dto.TestReviewTestCaseEditResult;
import io.metersphere.excel.converter.TestReviewCaseStatus;
import io.metersphere.i18n.Translator;
import io.metersphere.log.vo.DetailColumn;
import io.metersphere.log.vo.OperatingLogDetails;
import io.metersphere.log.vo.StatusReference;
@ -67,6 +69,8 @@ public class TestReviewTestCaseService {
TestCaseReviewTestCaseUsersMapper testCaseReviewTestCaseUsersMapper;
@Resource
TestCaseReviewTestCaseUsersService testCaseReviewTestCaseUsersService;
@Resource
ExtCheckOwnerMapper extCheckOwnerMapper;
public List<TestReviewCaseDTO> list(QueryCaseReviewRequest request) {
request.setOrders(ServiceUtils.getDefaultSortOrder(request.getOrders()));
@ -455,8 +459,9 @@ public class TestReviewTestCaseService {
return comments;
}
public TestReviewCaseDTO get(String testReviewTestCaseId) {
public TestReviewCaseDTO get(String testReviewTestCaseId, String currentProjectId) {
TestReviewCaseDTO testReviewCaseDTO = extTestReviewCaseMapper.get(testReviewTestCaseId);
checkReviewCaseOwner(testReviewCaseDTO.getCaseId(), currentProjectId);
testReviewCaseDTO.setFields(testCaseService.getCustomFieldByCaseId(testReviewCaseDTO.getCaseId()));
return testReviewCaseDTO;
}
@ -875,4 +880,11 @@ public class TestReviewTestCaseService {
testCaseReviewDTO.setStatus(TestPlanStatus.Finished.name());
}
}
private void checkReviewCaseOwner(String caseId, String currentProjectId) {
boolean hasPermission = extCheckOwnerMapper.checkoutOwner("test_case", currentProjectId, List.of(caseId));
if (!hasPermission) {
MSException.throwException(Translator.get("check_owner_case"));
}
}
}