Skip to content

Commit 6d25e92

Browse files
AntonioABLimaAdam-Aghili
authored andcommitted
fix: enforce IDOR protection on v2 workflow job endpoints (#12398)
* fix: enforce ownership check and pass user_id in workflow job creation - Add _assert_job_owner helper that raises 403 for non-owners (legacy jobs with user_id=None are allowed through) - Move ownership check before job type check in stop_workflow to avoid leaking job.type to non-owners - Pass user_id to create_job in both sync and background execution paths - Add user_id parameter to JobService.create_job signature * test: add ownership and legacy job coverage for workflow endpoints - Add TestWorkflowIDORProtection class with tests for 403 on cross-user access - Add test for stop_workflow with legacy user_id=None job (should not return 403) * fix: pass user_id to create_job in knowledge_bases ingestion endpoint Prevents IDOR vulnerability where ingestion jobs created without user_id would bypass ownership checks, matching the fix applied to workflow jobs. * refactor: move job ownership check to JobService layer Moves _assert_job_owner from workflow.py into JobService.assert_job_owner so any future job-consuming endpoint can reuse the check without duplicating logic. Both get_workflow_status and stop_workflow now delegate to the service. * test: fix mocks for assert_job_owner after service layer refactor MagicMock blocks attributes starting with 'assert' by default. Added explicit mock_service.assert_job_owner = MagicMock() to each test that mocks get_job_service so the ownership check is a no-op for tests not focused on IDOR behavior. * refactor: enforce job ownership at SQL level, remove assert_job_owner Move IDOR ownership check from application layer into the DB query. `get_job_by_job_id` now accepts an optional `user_id` and filters by `job_id AND (user_id = ? OR user_id IS NULL)`, so unauthorized access returns 404 instead of 403. Removes `JobService.assert_job_owner`. - Strengthen legacy-job test assertions (!=403 → ==200) - Add missing GET test for non-WORKFLOW job type → 404
1 parent a84a980 commit 6d25e92

5 files changed

Lines changed: 459 additions & 12 deletions

File tree

src/backend/base/langflow/api/v1/knowledge_bases.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,12 @@ async def ingest_files_to_knowledge_base(
367367

368368
# Create job record in database for both async and sync paths
369369
await job_service.create_job(
370-
job_id=job_id, flow_id=job_id, job_type=JobType.INGESTION, asset_id=asset_id, asset_type="knowledge_base"
370+
job_id=job_id,
371+
flow_id=job_id,
372+
job_type=JobType.INGESTION,
373+
asset_id=asset_id,
374+
asset_type="knowledge_base",
375+
user_id=current_user.id,
371376
)
372377

373378
# Always use async path: fire and forget the ingestion logic wrapped in status updates

src/backend/base/langflow/api/v2/workflow.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ async def execute_sync_workflow(
378378

379379
# Execute graph - component errors are caught and returned in response body
380380
job_service = get_job_service()
381-
await job_service.create_job(job_id=job_id, flow_id=flow_id_str)
381+
await job_service.create_job(job_id=job_id, flow_id=flow_id_str, user_id=api_key_user.id)
382382
try:
383383
task_result, execution_session_id = await job_service.execute_with_status(
384384
job_id=job_id,
@@ -467,6 +467,7 @@ async def execute_workflow_background(
467467
await job_service.create_job(
468468
job_id=job_id,
469469
flow_id=flow_id_str,
470+
user_id=api_key_user.id,
470471
)
471472

472473
await task_service.fire_and_forget_task(
@@ -535,7 +536,7 @@ async def get_workflow_status(
535536

536537
job_service = get_job_service()
537538
try:
538-
job = await job_service.get_job_by_job_id(job_id=job_id)
539+
job = await job_service.get_job_by_job_id(job_id=job_id, user_id=api_key_user.id)
539540
except Exception as exc:
540541
raise HTTPException(
541542
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
@@ -648,7 +649,7 @@ async def get_workflow_status(
648649
)
649650
async def stop_workflow(
650651
request: WorkflowStopRequest,
651-
api_key_user: Annotated[UserRead, Depends(api_key_security)], # noqa: ARG001
652+
api_key_user: Annotated[UserRead, Depends(api_key_security)],
652653
) -> WorkflowStopResponse:
653654
"""Stop a running workflow execution by job_id.
654655
@@ -673,7 +674,7 @@ async def stop_workflow(
673674

674675
try:
675676
# 1. Fetch Job
676-
job = await job_service.get_job_by_job_id(job_id)
677+
job = await job_service.get_job_by_job_id(job_id, user_id=api_key_user.id)
677678
except Exception as exc:
678679
raise HTTPException(
679680
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
@@ -695,6 +696,18 @@ async def stop_workflow(
695696
},
696697
)
697698

699+
# Verify this is a workflow job
700+
if job.type != JobType.WORKFLOW:
701+
raise HTTPException(
702+
status_code=status.HTTP_404_NOT_FOUND,
703+
detail={
704+
"error": "Job not found",
705+
"code": "JOB_NOT_FOUND",
706+
"message": f"Job {job_id} is not a workflow job (type: {job.type})",
707+
"job_id": str(job_id),
708+
},
709+
)
710+
698711
if job.status == JobStatus.CANCELLED:
699712
return WorkflowStopResponse(job_id=str(job_id), message=f"Job {job_id} is already cancelled.")
700713

src/backend/base/langflow/services/database/models/jobs/crud.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
from sqlmodel.ext.asyncio.session import AsyncSession
1010

11-
from sqlmodel import col, select
11+
from sqlmodel import col, or_, select
1212

1313
from langflow.services.database.models.jobs.model import Job, JobStatus
1414

@@ -37,17 +37,21 @@ async def get_jobs_by_flow_id(db: AsyncSession, flow_id: UUID, page: int = 1, si
3737
return list(result.all())
3838

3939

40-
async def get_job_by_job_id(db: AsyncSession, job_id: UUID) -> Job | None:
40+
async def get_job_by_job_id(db: AsyncSession, job_id: UUID, user_id: UUID | None = None) -> Job | None:
4141
"""Get a single job by its UUID.
4242
4343
Args:
4444
db: Async database session
4545
job_id: The job ID to fetch
46+
user_id: When provided, restricts the result to jobs owned by this user
47+
or legacy jobs with no owner (user_id IS NULL).
4648
4749
Returns:
48-
Job object or None if not found
50+
Job object or None if not found (or not accessible by the given user)
4951
"""
5052
statement = select(Job).where(Job.job_id == job_id)
53+
if user_id is not None:
54+
statement = statement.where(or_(Job.user_id == user_id, col(Job.user_id).is_(None)))
5155
result = await db.exec(statement)
5256
return result.first()
5357

src/backend/base/langflow/services/jobs/service.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,22 @@ async def get_jobs_by_flow_id(self, flow_id: UUID | str, page: int = 1, page_siz
4747
async with session_scope() as session:
4848
return await get_jobs_by_flow_id(session, flow_id, page=page, size=page_size)
4949

50-
async def get_job_by_job_id(self, job_id: UUID | str) -> Job | None:
50+
async def get_job_by_job_id(self, job_id: UUID | str, user_id: UUID | None = None) -> Job | None:
5151
"""Get job for a specific job ID.
5252
5353
Args:
5454
job_id: The job ID to filter jobs by
55+
user_id: When provided, restricts the result to jobs owned by this user
56+
or legacy jobs with no owner (user_id IS NULL).
5557
5658
Returns:
57-
Job object for the specified job ID
59+
Job object for the specified job ID, or None if not found or not accessible
5860
"""
5961
if isinstance(job_id, str):
6062
job_id = UUID(job_id)
6163

6264
async with session_scope() as session:
63-
return await get_job_by_job_id(session, job_id)
65+
return await get_job_by_job_id(session, job_id, user_id=user_id)
6466

6567
async def create_job(
6668
self,
@@ -69,6 +71,7 @@ async def create_job(
6971
job_type: JobType = JobType.WORKFLOW,
7072
asset_id: UUID | None = None,
7173
asset_type: str | None = None,
74+
user_id: UUID | None = None,
7275
) -> Job:
7376
"""Create a new job record with QUEUED status.
7477
@@ -78,6 +81,7 @@ async def create_job(
7881
job_type: The job type
7982
asset_id: The asset ID
8083
asset_type: The asset type
84+
user_id: The user ID who owns this job
8185
8286
Returns:
8387
Created Job object
@@ -96,6 +100,7 @@ async def create_job(
96100
type=job_type,
97101
asset_id=asset_id,
98102
asset_type=asset_type,
103+
user_id=user_id,
99104
)
100105
session.add(job)
101106
await session.flush()

0 commit comments

Comments
 (0)