[Fix-12916] Add permission check when query or download log (#12917)

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,

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

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);
     }
 

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();

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);
+            loggerService.getLogBytes(loginUser, 2);
-        } catch (RuntimeException e) {
+        } catch (ServiceException e) {
-            Assertions.assertTrue(true);
+            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);
+            loggerService.getLogBytes(loginUser, 1);
-        } catch (RuntimeException e) {
+        } catch (ServiceException e) {
-            Assertions.assertTrue(true);
+            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

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));
     }

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);
 }

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>