Skip to content

[Fix-18283][API] Add project permission check on workflow lineage and workflow-definition list endpoints#18284

Open
ruanwenjun wants to merge 1 commit into
apache:devfrom
ruanwenjun:fix-api-project-auth
Open

[Fix-18283][API] Add project permission check on workflow lineage and workflow-definition list endpoints#18284
ruanwenjun wants to merge 1 commit into
apache:devfrom
ruanwenjun:fix-api-project-auth

Conversation

@ruanwenjun
Copy link
Copy Markdown
Member

Was this PR generated or assisted by AI?

YES. Drafted with the help of Claude Code: it scanned the API module, identified the seven controller endpoints that accepted a projectCode parameter without going through ProjectService.checkProjectAndAuthThrowException, applied the fix, and adapted the affected tests. Every change was reviewed and validated locally before commit.

Purpose of the pull request

Closes #18283.

Seven REST endpoints in the API server accept a projectCode path/query parameter but do not check whether the login user actually has access to that project. As a result any logged-in user can read project-scoped metadata for projects they have no permission to.

Affected endpoints (none of them previously called ProjectService.checkProjectAndAuthThrowException):

WorkflowLineageController

  • GET /projects/{projectCode}/lineages/query-by-name
  • GET /projects/{projectCode}/lineages/{workFlowCode}
  • GET /projects/{projectCode}/lineages/list
  • POST /projects/{projectCode}/lineages/tasks/verify-delete (this one did not even verify project existence)
  • GET /projects/{projectCode}/lineages/query-dependent-tasks

WorkflowDefinitionController

  • GET /projects/{projectCode}/workflow-definition/query-workflow-definition-list
  • GET /projects/{projectCode}/workflow-definition/query-task-definition-list

Brief change log

  • WorkflowLineageService: add User loginUser to the 5 endpoint-facing methods.
  • WorkflowLineageServiceImpl: every public method now calls ProjectService.checkProjectAndAuthThrowException(loginUser, project, WORKFLOW_DEFINITION) after looking up the project; taskDependentMsg additionally now verifies project existence.
  • WorkflowDefinitionService: add User loginUser to queryWorkflowDefinitionListByProjectCode and queryTaskDefinitionListByWorkflowDefinitionCode.
  • WorkflowDefinitionServiceImpl: both list methods now check user access; taskUsedInOtherTaskValid and workflowDefinitionUsedInOtherTaskValid thread loginUser through to the inner taskDependentMsg calls.
  • Controllers: thread loginUser into the seven endpoints' service calls.
  • Tests: adapt existing mocks to the new signatures; add a negative test (testQueryWorkFlowLineageByNameWithoutProjectPerm) confirming an unauthorized user gets USER_NO_OPERATION_PROJECT_PERM.

Verify this pull request

This change added tests and can be verified as follows:

  • The existing WorkflowTaskLineageServiceTest, WorkflowLineageServiceImplTest, WorkflowTaskLineageControllerTest, WorkflowDefinitionServiceTest, and WorkflowDefinitionControllerTest were updated to pass loginUser and continue to pass.
  • A new test, WorkflowTaskLineageServiceTest#testQueryWorkFlowLineageByNameWithoutProjectPerm, mocks ProjectService to throw USER_NO_OPERATION_PROJECT_PERM and asserts the service propagates it and never calls the underlying DAO.
  • Locally ./mvnw verify -pl dolphinscheduler-api -Dspotless.skip=true -DskipUT=false reports Tests run: 687, Failures: 0, Errors: 0 (skipped 12, all pre-existing). ./mvnw spotless:check -pl dolphinscheduler-api is clean.

Pull Request Notice

Pull Request Notice

This change is not a wire/DB/SPI break: only Java internal method signatures change. UI clients continue to call the same HTTP endpoints, but unauthorized callers now receive USER_NO_OPERATION_PROJECT_PERM instead of getting back unrelated project data. No update to docs/docs/en/guide/upgrade/incompatible.md is needed.

… workflow-definition list endpoints

The five `WorkflowLineageController` endpoints and two `WorkflowDefinitionController` list
endpoints accept a `projectCode` parameter but did not verify whether the login user has
access to that project. Any logged-in user could read project-scoped metadata for projects
they have no permission to.

Route every affected service method through `ProjectService.checkProjectAndAuthThrowException`
with the existing `WORKFLOW_DEFINITION` permission, mirroring the pattern used by sibling
endpoints (e.g. `/workflow-definition/all`, `/workflow-instances`).

- WorkflowLineageService: add `User loginUser` to the 5 endpoint-facing methods; service
  impl now calls `checkProjectAndAuthThrowException` after looking up the project, including
  `taskDependentMsg` which previously did not even verify project existence.
- WorkflowDefinitionService: add `User loginUser` to `queryWorkflowDefinitionListByProjectCode`
  and `queryTaskDefinitionListByWorkflowDefinitionCode`; both impls now verify access.
- WorkflowDefinitionServiceImpl: propagate `loginUser` through `taskUsedInOtherTaskValid` and
  `workflowDefinitionUsedInOtherTaskValid` so the inner `taskDependentMsg` calls compile
  against the new signature.
- Controllers: thread `loginUser` into the seven endpoints' service calls.
- Tests: adapt existing mocks to the new signatures; add a negative test that confirms
  `queryWorkFlowLineageByName` rejects an unauthorized user with `USER_NO_OPERATION_PROJECT_PERM`.
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [API] Missing project permission check on workflow lineage and workflow-definition list endpoints

1 participant