Skip to content

Commit 939439c

Browse files
authored
[Fix-18182] [API] User can delete task definitions in unauthorized projects (#18183)
1 parent 91b75ee commit 939439c

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskDefinitionServiceImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ public Map<String, Object> deleteByCodeAndVersion(User loginUser, long projectCo
615615
}
616616
TaskDefinition taskDefinition = taskDefinitionMapper.queryByCode(taskCode);
617617

618-
if (taskDefinition == null) {
618+
if (taskDefinition == null || projectCode != taskDefinition.getProjectCode()) {
619619
log.error("Task definition does not exist, taskDefinitionCode:{}.", taskCode);
620620
putMsg(result, Status.TASK_DEFINE_NOT_EXIST, String.valueOf(taskCode));
621621
} else {

dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/TaskDefinitionServiceImplTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,35 @@ public void switchVersion() {
180180
assertEquals(Status.SUCCESS, relation.get(Constants.STATUS));
181181
}
182182

183+
@Test
184+
public void deleteByCodeAndVersion() {
185+
Project project = getProject();
186+
when(projectMapper.queryByCode(PROJECT_CODE)).thenReturn(project);
187+
when(projectService.hasProjectAndWritePerm(user, project, new HashMap<>())).thenReturn(true);
188+
189+
// cross-project privilege escalation: taskCode belongs to another project - must be rejected
190+
TaskDefinition otherProjectTask = new TaskDefinition();
191+
otherProjectTask.setProjectCode(PROJECT_CODE + 1);
192+
otherProjectTask.setCode(TASK_CODE);
193+
otherProjectTask.setVersion(VERSION + 1);
194+
when(taskDefinitionMapper.queryByCode(TASK_CODE)).thenReturn(otherProjectTask);
195+
Map<String, Object> crossProjectResult =
196+
taskDefinitionService.deleteByCodeAndVersion(user, PROJECT_CODE, TASK_CODE, VERSION);
197+
assertEquals(Status.TASK_DEFINE_NOT_EXIST, crossProjectResult.get(Constants.STATUS));
198+
Mockito.verify(taskDefinitionLogMapper, Mockito.never()).deleteByCodeAndVersion(TASK_CODE, VERSION);
199+
200+
// normal path: taskCode belongs to the project - should succeed
201+
TaskDefinition taskDefinition = new TaskDefinition();
202+
taskDefinition.setProjectCode(PROJECT_CODE);
203+
taskDefinition.setCode(TASK_CODE);
204+
taskDefinition.setVersion(VERSION + 1);
205+
when(taskDefinitionMapper.queryByCode(TASK_CODE)).thenReturn(taskDefinition);
206+
when(taskDefinitionLogMapper.deleteByCodeAndVersion(TASK_CODE, VERSION)).thenReturn(1);
207+
Map<String, Object> successResult =
208+
taskDefinitionService.deleteByCodeAndVersion(user, PROJECT_CODE, TASK_CODE, VERSION);
209+
assertEquals(Status.SUCCESS, successResult.get(Constants.STATUS));
210+
}
211+
183212
private void putMsg(Map<String, Object> result, Status status, Object... statusParams) {
184213
result.put(Constants.STATUS, status);
185214
if (statusParams != null && statusParams.length > 0) {

0 commit comments

Comments
 (0)