This is an automated email from the ASF dual-hosted git repository.
chufenggao pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git
The following commit(s) were added to refs/heads/dev by this push:
new 7336afaa65 [Fix-12916] Add permission check when query or download log
(#12917)
7336afaa65 is described below
commit 7336afaa65e3c4bb4596081cf0049b5166506a0c
Author: rickchengx <[email protected]>
AuthorDate: Fri Nov 25 17:59:28 2022 +0800
[Fix-12916] Add permission check when query or download log (#12917)
---
.../api/controller/LoggerController.java | 4 +-
.../api/service/LoggerService.java | 6 +-
.../api/service/impl/LoggerServiceImpl.java | 10 ++-
.../service/impl/ProcessInstanceServiceImpl.java | 6 +-
.../api/service/LoggerServiceTest.java | 80 ++++++++++++++++++----
.../api/service/ProcessInstanceServiceTest.java | 2 +-
.../dolphinscheduler/dao/mapper/ProjectMapper.java | 7 ++
.../dolphinscheduler/dao/mapper/ProjectMapper.xml | 12 ++++
8 files changed, 103 insertions(+), 24 deletions(-)
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/LoggerController.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/LoggerController.java
index 0909d4d85c..bd2a79285d 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/LoggerController.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/LoggerController.java
@@ -81,7 +81,7 @@ public class LoggerController extends BaseController {
@RequestParam(value =
"taskInstanceId") int taskInstanceId,
@RequestParam(value =
"skipLineNum") int skipNum,
@RequestParam(value = "limit") int
limit) {
- return loggerService.queryLog(taskInstanceId, skipNum, limit);
+ return loggerService.queryLog(loginUser, taskInstanceId, skipNum,
limit);
}
/**
@@ -101,7 +101,7 @@ public class LoggerController extends BaseController {
@AccessLogAnnotation(ignoreRequestArgs = "loginUser")
public ResponseEntity downloadTaskLog(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser,
@RequestParam(value =
"taskInstanceId") int taskInstanceId) {
- byte[] logBytes = loggerService.getLogBytes(taskInstanceId);
+ byte[] logBytes = loggerService.getLogBytes(loginUser, taskInstanceId);
return ResponseEntity
.ok()
.header(HttpHeaders.CONTENT_DISPOSITION,
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/LoggerService.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/LoggerService.java
index 33477c509e..db1cb7c465 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/LoggerService.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/LoggerService.java
@@ -31,20 +31,22 @@ public interface LoggerService {
/**
* view log
*
+ * @param loginUser login user
* @param taskInstId task instance id
* @param skipLineNum skip line number
* @param limit limit
* @return log string data
*/
- Result<ResponseTaskLog> queryLog(int taskInstId, int skipLineNum, int
limit);
+ Result<ResponseTaskLog> queryLog(User loginUser, int taskInstId, int
skipLineNum, int limit);
/**
* get log size
*
+ * @param loginUser login user
* @param taskInstId task instance id
* @return log byte array
*/
- byte[] getLogBytes(int taskInstId);
+ byte[] getLogBytes(User loginUser, int taskInstId);
/**
* query log
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/LoggerServiceImpl.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/LoggerServiceImpl.java
index 239c9669f1..227f74c12d 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/LoggerServiceImpl.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/LoggerServiceImpl.java
@@ -77,6 +77,7 @@ public class LoggerServiceImpl extends BaseServiceImpl
implements LoggerService
/**
* view log
*
+ * @param loginUser login user
* @param taskInstId task instance id
* @param skipLineNum skip line number
* @param limit limit
@@ -84,7 +85,7 @@ public class LoggerServiceImpl extends BaseServiceImpl
implements LoggerService
*/
@Override
@SuppressWarnings("unchecked")
- public Result<ResponseTaskLog> queryLog(int taskInstId, int skipLineNum,
int limit) {
+ public Result<ResponseTaskLog> queryLog(User loginUser, int taskInstId,
int skipLineNum, int limit) {
TaskInstance taskInstance =
taskInstanceDao.findTaskInstanceById(taskInstId);
@@ -96,6 +97,8 @@ public class LoggerServiceImpl extends BaseServiceImpl
implements LoggerService
logger.error("Host of task instance is null, taskInstanceId:{}.",
taskInstId);
return Result.error(Status.TASK_INSTANCE_HOST_IS_NULL);
}
+ Project project =
projectMapper.queryProjectByTaskInstanceId(taskInstId);
+ projectService.checkProjectAndAuthThrowException(loginUser, project,
VIEW_LOG);
Result<ResponseTaskLog> result = new
Result<>(Status.SUCCESS.getCode(), Status.SUCCESS.getMsg());
String log = queryLog(taskInstance, skipLineNum, limit);
int lineNum = log.split("\\r\\n").length;
@@ -106,15 +109,18 @@ public class LoggerServiceImpl extends BaseServiceImpl
implements LoggerService
/**
* get log size
*
+ * @param loginUser login user
* @param taskInstId task instance id
* @return log byte array
*/
@Override
- public byte[] getLogBytes(int taskInstId) {
+ public byte[] getLogBytes(User loginUser, int taskInstId) {
TaskInstance taskInstance =
taskInstanceDao.findTaskInstanceById(taskInstId);
if (taskInstance == null ||
StringUtils.isBlank(taskInstance.getHost())) {
throw new ServiceException("task instance is null or host is
null");
}
+ Project project =
projectMapper.queryProjectByTaskInstanceId(taskInstId);
+ projectService.checkProjectAndAuthThrowException(loginUser, project,
DOWNLOAD_LOG);
return getLogBytes(taskInstance);
}
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java
index 05415f899a..4c3fceefe3 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java
@@ -375,7 +375,7 @@ public class ProcessInstanceServiceImpl extends
BaseServiceImpl implements Proce
}
List<TaskInstance> taskInstanceList =
taskInstanceDao.findValidTaskListByProcessId(processId,
processInstance.getTestFlag());
- addDependResultForTaskList(taskInstanceList);
+ addDependResultForTaskList(loginUser, taskInstanceList);
Map<String, Object> resultMap = new HashMap<>();
resultMap.put(PROCESS_INSTANCE_STATE,
processInstance.getState().toString());
resultMap.put(TASK_LIST, taskInstanceList);
@@ -388,12 +388,12 @@ public class ProcessInstanceServiceImpl extends
BaseServiceImpl implements Proce
/**
* add dependent result for dependent task
*/
- private void addDependResultForTaskList(List<TaskInstance>
taskInstanceList) throws IOException {
+ private void addDependResultForTaskList(User loginUser, List<TaskInstance>
taskInstanceList) throws IOException {
for (TaskInstance taskInstance : taskInstanceList) {
if
(TASK_TYPE_DEPENDENT.equalsIgnoreCase(taskInstance.getTaskType())) {
logger.info("DEPENDENT type task instance need to set
dependent result, taskCode:{}, taskInstanceId:{}",
taskInstance.getTaskCode(), taskInstance.getId());
- Result<ResponseTaskLog> logResult = loggerService.queryLog(
+ Result<ResponseTaskLog> logResult =
loggerService.queryLog(loginUser,
taskInstance.getId(),
Constants.LOG_QUERY_SKIP_LINE_NUMBER, Constants.LOG_QUERY_LIMIT);
if (logResult.getCode() == Status.SUCCESS.ordinal()) {
String log = logResult.getData().getMessage();
diff --git
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/LoggerServiceTest.java
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/LoggerServiceTest.java
index ca34d74730..000a5955a4 100644
---
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/LoggerServiceTest.java
+++
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/LoggerServiceTest.java
@@ -21,6 +21,7 @@ import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationCon
import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.VIEW_LOG;
import org.apache.dolphinscheduler.api.enums.Status;
+import org.apache.dolphinscheduler.api.exceptions.ServiceException;
import org.apache.dolphinscheduler.api.service.impl.LoggerServiceImpl;
import org.apache.dolphinscheduler.api.utils.Result;
import org.apache.dolphinscheduler.common.constants.Constants;
@@ -78,60 +79,111 @@ public class LoggerServiceTest {
private LogClient logClient;
@Test
- public void testQueryDataSourceList() {
+ public void testQueryLog() {
+ User loginUser = new User();
+ loginUser.setId(1);
TaskInstance taskInstance = new TaskInstance();
+ taskInstance.setExecutorId(loginUser.getId() + 1);
Mockito.when(taskInstanceDao.findTaskInstanceById(1)).thenReturn(taskInstance);
- Result result = loggerService.queryLog(2, 1, 1);
+ Result result = loggerService.queryLog(loginUser, 2, 1, 1);
// TASK_INSTANCE_NOT_FOUND
Assertions.assertEquals(Status.TASK_INSTANCE_NOT_FOUND.getCode(),
result.getCode().intValue());
try {
// HOST NOT FOUND OR ILLEGAL
- result = loggerService.queryLog(1, 1, 1);
+ result = loggerService.queryLog(loginUser, 1, 1, 1);
} catch (RuntimeException e) {
Assertions.assertTrue(true);
logger.error("testQueryDataSourceList error {}", e.getMessage());
}
Assertions.assertEquals(Status.TASK_INSTANCE_HOST_IS_NULL.getCode(),
result.getCode().intValue());
- // SUCCESS
+ // PROJECT_NOT_EXIST
taskInstance.setHost("127.0.0.1:8080");
taskInstance.setLogPath("/temp/log");
+ try {
+ Mockito.doThrow(new
ServiceException(Status.PROJECT_NOT_EXIST)).when(projectService)
+ .checkProjectAndAuthThrowException(loginUser, null,
VIEW_LOG);
+ loggerService.queryLog(loginUser, 1, 1, 1);
+ } catch (ServiceException serviceException) {
+ Assertions.assertEquals(Status.PROJECT_NOT_EXIST.getCode(),
serviceException.getCode());
+ }
+
+ // USER_NO_OPERATION_PERM
+ Project project = getProject(1);
+
Mockito.when(projectMapper.queryProjectByTaskInstanceId(1)).thenReturn(project);
+ try {
+ Mockito.doThrow(new
ServiceException(Status.USER_NO_OPERATION_PERM)).when(projectService)
+ .checkProjectAndAuthThrowException(loginUser, project,
VIEW_LOG);
+ loggerService.queryLog(loginUser, 1, 1, 1);
+ } catch (ServiceException serviceException) {
+ Assertions.assertEquals(Status.USER_NO_OPERATION_PERM.getCode(),
serviceException.getCode());
+ }
+
+ // SUCCESS
+
Mockito.doNothing().when(projectService).checkProjectAndAuthThrowException(loginUser,
project, VIEW_LOG);
Mockito.when(taskInstanceDao.findTaskInstanceById(1)).thenReturn(taskInstance);
- result = loggerService.queryLog(1, 1, 1);
+ result = loggerService.queryLog(loginUser, 1, 1, 1);
Assertions.assertEquals(Status.SUCCESS.getCode(),
result.getCode().intValue());
}
@Test
public void testGetLogBytes() {
+ User loginUser = new User();
+ loginUser.setId(1);
TaskInstance taskInstance = new TaskInstance();
+ taskInstance.setExecutorId(loginUser.getId() + 1);
Mockito.when(taskInstanceDao.findTaskInstanceById(1)).thenReturn(taskInstance);
// task instance is null
try {
- loggerService.getLogBytes(2);
- } catch (RuntimeException e) {
- Assertions.assertTrue(true);
+ loggerService.getLogBytes(loginUser, 2);
+ } catch (ServiceException e) {
+ Assertions.assertEquals(new ServiceException("task instance is
null or host is null").getMessage(),
+ e.getMessage());
logger.error("testGetLogBytes error: {}", "task instance is null");
}
// task instance host is null
try {
- loggerService.getLogBytes(1);
- } catch (RuntimeException e) {
- Assertions.assertTrue(true);
+ loggerService.getLogBytes(loginUser, 1);
+ } catch (ServiceException e) {
+ Assertions.assertEquals(new ServiceException("task instance is
null or host is null").getMessage(),
+ e.getMessage());
logger.error("testGetLogBytes error: {}", "task instance host is
null");
}
- // success
+ // PROJECT_NOT_EXIST
taskInstance.setHost("127.0.0.1:8080");
taskInstance.setLogPath("/temp/log");
+ try {
+ Mockito.doThrow(new
ServiceException(Status.PROJECT_NOT_EXIST)).when(projectService)
+ .checkProjectAndAuthThrowException(loginUser, null,
DOWNLOAD_LOG);
+ loggerService.queryLog(loginUser, 1, 1, 1);
+ } catch (ServiceException serviceException) {
+ Assertions.assertEquals(Status.PROJECT_NOT_EXIST.getCode(),
serviceException.getCode());
+ }
+
+ // USER_NO_OPERATION_PERM
+ Project project = getProject(1);
+
Mockito.when(projectMapper.queryProjectByTaskInstanceId(1)).thenReturn(project);
+ try {
+ Mockito.doThrow(new
ServiceException(Status.USER_NO_OPERATION_PERM)).when(projectService)
+ .checkProjectAndAuthThrowException(loginUser, project,
DOWNLOAD_LOG);
+ loggerService.queryLog(loginUser, 1, 1, 1);
+ } catch (ServiceException serviceException) {
+ Assertions.assertEquals(Status.USER_NO_OPERATION_PERM.getCode(),
serviceException.getCode());
+ }
+
+ // SUCCESS
+
Mockito.doNothing().when(projectService).checkProjectAndAuthThrowException(loginUser,
project, DOWNLOAD_LOG);
Mockito.when(logClient.getLogBytes(Mockito.anyString(),
Mockito.anyInt(), Mockito.anyString()))
.thenReturn(new byte[0]);
- loggerService.getLogBytes(1);
-
+
Mockito.when(projectMapper.queryProjectByTaskInstanceId(1)).thenReturn(project);
+ byte[] result = loggerService.getLogBytes(loginUser, 1);
+ Assertions.assertEquals(90, result.length);
}
@Test
diff --git
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessInstanceServiceTest.java
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessInstanceServiceTest.java
index f6bbefd8e4..abf29bbc5b 100644
---
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessInstanceServiceTest.java
+++
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessInstanceServiceTest.java
@@ -414,7 +414,7 @@ public class ProcessInstanceServiceTest {
.thenReturn(Optional.of(processInstance));
when(taskInstanceDao.findValidTaskListByProcessId(processInstance.getId(),
processInstance.getTestFlag()))
.thenReturn(taskInstanceList);
- when(loggerService.queryLog(taskInstance.getId(), 0,
4098)).thenReturn(res);
+ when(loggerService.queryLog(loginUser, taskInstance.getId(), 0,
4098)).thenReturn(res);
Map<String, Object> successRes =
processInstanceService.queryTaskListByProcessId(loginUser, projectCode, 1);
Assertions.assertEquals(Status.SUCCESS,
successRes.get(Constants.STATUS));
}
diff --git
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.java
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.java
index 59c635f711..c43a2415f0 100644
---
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.java
+++
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.java
@@ -143,4 +143,11 @@ public interface ProjectMapper extends BaseMapper<Project>
{
* @return projectList
*/
List<Project> queryAllProjectForDependent();
+
+ /**
+ * query the project by task instance id
+ * @param taskInstanceId
+ * @return project
+ */
+ Project queryProjectByTaskInstanceId(@Param("taskInstanceId") int
taskInstanceId);
}
diff --git
a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml
b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml
index d01d970ee1..6996d6f754 100644
---
a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml
+++
b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml
@@ -185,4 +185,16 @@
select code, name
from t_ds_project
</select>
+
+ <select id="queryProjectByTaskInstanceId"
resultType="org.apache.dolphinscheduler.dao.entity.Project">
+ select
+ <include refid="baseSqlV2">
+ <property name="alias" value="p"/>
+ </include>
+ from t_ds_task_instance ti
+ join t_ds_process_instance pi on ti.process_instance_id = pi.id
+ join t_ds_process_definition pd on pi.process_definition_code = pd.code
+ join t_ds_project p on p.code = pd.project_code
+ where ti.id = #{taskInstanceId}
+ </select>
</mapper>
