Skip to content

Commit cde22d6

Browse files
Merge pull request #327 from microsoft/dev
fix: merging dev to main
2 parents c6b6251 + c892e62 commit cde22d6

14 files changed

Lines changed: 719 additions & 391 deletions

File tree

azure.yaml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,27 @@ requiredVersions:
88
azd: '>=1.18.2 != 1.23.9'
99

1010
hooks:
11+
preprovision:
12+
windows:
13+
run: |
14+
Write-Host ""
15+
Write-Host " ---------------------------------------------------------------------------------------------------" -ForegroundColor Yellow
16+
Write-Host " Note: If deployment fails or you encounter an issue, please open an issue with the deployment logs." -ForegroundColor Yellow
17+
Write-Host " https://github.com/microsoft/Container-Migration-Solution-Accelerator/issues" -ForegroundColor Cyan
18+
Write-Host " ---------------------------------------------------------------------------------------------------" -ForegroundColor Yellow
19+
Write-Host ""
20+
shell: pwsh
21+
interactive: true
22+
posix:
23+
run: |
24+
printf '\n'
25+
printf '\033[33m ---------------------------------------------------------------------------------------------------\033[0m\n'
26+
printf '\033[33m Note: If deployment fails or you encounter an issue, please open an issue with the deployment logs.\033[0m\n'
27+
printf '\033[36m https://github.com/microsoft/Container-Migration-Solution-Accelerator/issues\033[0m\n'
28+
printf '\033[33m ---------------------------------------------------------------------------------------------------\033[0m\n'
29+
printf '\n'
30+
shell: sh
31+
interactive: true
1132
postdeploy:
1233
posix:
1334
shell: sh

src/backend-api/pyproject.toml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ dependencies = [
1717
"httpx==0.28.1",
1818
"pydantic-settings==2.13.1",
1919
"python-dotenv==1.2.2",
20-
"python-multipart==0.0.27",
20+
"python-multipart==0.0.31",
2121
"protobuf==7.34.0",
2222
"sas-cosmosdb==0.1.5",
2323
"semantic-kernel[azure]==1.41.1",
@@ -34,14 +34,14 @@ omit = ["src/tests/*"]
3434
override-dependencies = [
3535
"av==16.0.0",
3636
"starlette==0.49.1",
37-
"aiohttp==3.13.4",
37+
"aiohttp==3.14.1",
3838
"azure-core==1.38.0",
3939
"urllib3==2.7.0",
4040
"requests==2.33.0",
4141
"werkzeug==3.1.6",
4242
"pygments==2.20.0",
4343
"black==26.3.1",
44-
"cryptography==46.0.7",
44+
"cryptography==48.0.1",
4545
"pyjwt==2.12.0",
4646
"pyopenssl==26.0.0",
4747
"idna==3.15",
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
from fastapi import HTTPException
2+
3+
from libs.base.typed_fastapi import TypedFastAPI
4+
from libs.models.entities import Process
5+
from libs.repositories.process_repository import ProcessRepository
6+
7+
8+
async def verify_process_ownership(
9+
app: TypedFastAPI, process_id: str, user_id: str
10+
) -> Process:
11+
"""Ensure the authenticated caller owns the requested process.
12+
13+
Fetches the Process entity and compares its stored ``user_id`` against the
14+
authenticated caller. A missing process and a process owned by a different
15+
user are treated identically and both raise a 404, so callers cannot probe
16+
for the existence of other users' processes.
17+
18+
Args:
19+
app: The FastAPI application carrying the dependency-injection context.
20+
process_id: The identifier of the process being accessed.
21+
user_id: The authenticated caller's user principal id.
22+
23+
Returns:
24+
The owned Process entity.
25+
26+
Raises:
27+
HTTPException: 404 if the process does not exist or is not owned by the
28+
authenticated caller.
29+
"""
30+
if not process_id:
31+
raise HTTPException(status_code=400, detail="Process ID is required")
32+
33+
if not user_id:
34+
raise HTTPException(status_code=401, detail="User not authenticated")
35+
36+
async with app.app_context.create_scope() as scope:
37+
process_repository = scope.get_service(ProcessRepository)
38+
process = await process_repository.get_async(process_id)
39+
40+
if not process or process.user_id != user_id:
41+
# Return 404 (not 403) so the response does not confirm the existence of
42+
# processes belonging to other users.
43+
raise HTTPException(status_code=404, detail="Process not found")
44+
45+
return process

src/backend-api/src/app/routers/router_files.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ async def upload_file(
6969
processRepository = scope.get_service(ProcessRepository)
7070
fileRepository = scope.get_service(FileRepository)
7171

72-
process_record = await processRepository.get_async(process_id)
72+
from libs.services.authorization import verify_process_ownership
73+
74+
process_record = await verify_process_ownership(app, process_id, user_id)
7375

7476
file_id = str(uuid4())
7577
file_name = re.sub(r"[^\w.-]", "_", file.filename)

src/backend-api/src/app/routers/router_process.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from libs.models.entities import Process
1212
from libs.repositories.process_repository import ProcessRepository
1313
from libs.services.auth import get_authenticated_user
14+
from libs.services.authorization import verify_process_ownership
1415
from libs.services.interfaces import ILoggerService
1516
from libs.services.process_services import ProcessService
1617
from routers.models.files import FileInfo
@@ -83,6 +84,11 @@ async def status(process_id: str, request: Request):
8384
f"Process router status endpoint called for process_id: {process_id}"
8485
)
8586

87+
# Authenticate the caller and verify they own this process
88+
authenticated_user = get_authenticated_user(request)
89+
user_id = authenticated_user.user_principal_id
90+
await verify_process_ownership(app, process_id, user_id)
91+
8692
# loading business component for process
8793
processService = app.app_context.get_service(ProcessService)
8894

@@ -99,6 +105,11 @@ async def render_status(process_id: str, request: Request):
99105
f"Process router render status endpoint called for process_id: {process_id}"
100106
)
101107

108+
# Authenticate the caller and verify they own this process
109+
authenticated_user = get_authenticated_user(request)
110+
user_id = authenticated_user.user_principal_id
111+
await verify_process_ownership(app, process_id, user_id)
112+
102113
# loading business component for process
103114
processService = app.app_context.get_service(ProcessService)
104115

@@ -138,6 +149,9 @@ async def upload_files(
138149
if not user_id:
139150
raise HTTPException(status_code=401, detail="User not authenticated")
140151

152+
# Verify the caller owns this process before touching its blob storage
153+
await verify_process_ownership(app, process_id, user_id)
154+
141155
# Make uploaded files list
142156
uploaded_files: list[FileInfo] = []
143157

@@ -195,6 +209,8 @@ async def upload_files(
195209
response.headers["Location"] = f"/process/{process_id}/"
196210

197211
return result_response
212+
except HTTPException:
213+
raise
198214
except Exception as e:
199215
logger_service.log_error(f"Error in upload_files: {str(e)}")
200216
raise HTTPException(status_code=500, detail=f"Error uploading files: {str(e)}")
@@ -233,6 +249,9 @@ async def delete_file(
233249
if not user_id:
234250
raise HTTPException(status_code=401, detail="User not authenticated")
235251

252+
# Verify the caller owns this process before deleting any file
253+
await verify_process_ownership(app, process_id, user_id)
254+
236255
# Get process service
237256
processService = app.app_context.get_service(ProcessService)
238257

@@ -274,6 +293,8 @@ async def delete_file(
274293
status_code=404,
275294
detail=f"File '{file_name}' not found for process '{process_id}'",
276295
)
296+
except HTTPException:
297+
raise
277298
except Exception as e:
278299
logger_service.log_error(f"Error in delete_file: {str(e)}")
279300
raise HTTPException(status_code=500, detail=f"Error deleting file: {str(e)}")
@@ -309,6 +330,9 @@ async def delete_process(
309330
if not user_id:
310331
raise HTTPException(status_code=401, detail="User not authenticated")
311332

333+
# Verify the caller owns this process before deleting its files
334+
await verify_process_ownership(app, process_id, user_id)
335+
312336
# Get process service
313337
processService = app.app_context.get_service(ProcessService)
314338

@@ -333,6 +357,8 @@ async def delete_process(
333357

334358
return result_response
335359

360+
except HTTPException:
361+
raise
336362
except Exception as e:
337363
logger_service.log_error(f"Error in delete_process: {str(e)}")
338364
raise HTTPException(status_code=500, detail=f"Error deleting process: {str(e)}")
@@ -365,6 +391,9 @@ async def start_processing(
365391
if not user_id:
366392
raise HTTPException(status_code=401, detail="User not authenticated")
367393

394+
# Verify the caller owns this process before queueing it for processing
395+
await verify_process_ownership(app, process_id, user_id)
396+
368397
# Get process service
369398
processService = app.app_context.get_service(ProcessService)
370399

@@ -394,6 +423,8 @@ async def start_processing(
394423
"user_id": str(user_id),
395424
"status": "queued",
396425
}
426+
except HTTPException:
427+
raise
397428
except Exception as e:
398429
logger_service.log_error(f"Error in start_processing: {str(e)}")
399430
raise HTTPException(
@@ -425,6 +456,9 @@ async def download_process_files(
425456
if not user_id:
426457
raise HTTPException(status_code=401, detail="User not authenticated")
427458

459+
# Verify the caller owns this process before returning its files
460+
await verify_process_ownership(app, process_id, user_id)
461+
428462
# Get process service
429463
processService = app.app_context.get_service(ProcessService)
430464

@@ -492,6 +526,9 @@ async def get_process_summary(
492526
if not user_id:
493527
raise HTTPException(status_code=401, detail="User not authenticated")
494528

529+
# Verify the caller owns this process before returning its summary
530+
await verify_process_ownership(app, process_id, user_id)
531+
495532
# Get process service
496533
processService = app.app_context.get_service(ProcessService)
497534

@@ -549,6 +586,9 @@ async def get_file_content(
549586
if not user_id:
550587
raise HTTPException(status_code=401, detail="User not authenticated")
551588

589+
# Verify the caller owns this process before returning file content
590+
await verify_process_ownership(app, process_id, user_id)
591+
552592
# Get process service
553593
processService = app.app_context.get_service(ProcessService)
554594

@@ -607,6 +647,9 @@ async def cancel_process(
607647
if not user_id:
608648
raise HTTPException(status_code=401, detail="User not authenticated")
609649

650+
# Verify the caller owns this process before forwarding the kill request
651+
await verify_process_ownership(app, process_id, user_id)
652+
610653
# Get processor control URL from configuration
611654
config = app.app_context.configuration
612655
processor_url = config.processor_control_url or "http://processor:8080"
@@ -704,6 +747,9 @@ async def get_cancel_status(
704747
if not user_id:
705748
raise HTTPException(status_code=401, detail="User not authenticated")
706749

750+
# Verify the caller owns this process before reading its cancel status
751+
await verify_process_ownership(app, process_id, user_id)
752+
707753
# Get processor control URL from configuration
708754
config = app.app_context.configuration
709755
processor_url = config.processor_control_url or "http://processor:8080"

src/backend-api/src/tests/routers/test_router_files.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ def _make_app(*, process_record=None, file_count=1, blob_helper=None):
2424
process_repo = MagicMock()
2525
process_repo.get_async = AsyncMock(
2626
return_value=process_record
27-
or SimpleNamespace(id="p-1", source_file_count=0, status="initialized")
27+
or SimpleNamespace(
28+
id="p-1",
29+
user_id="user-1",
30+
source_file_count=0,
31+
status="initialized",
32+
)
2833
)
2934
process_repo.update_async = AsyncMock(return_value=None)
3035

@@ -159,3 +164,38 @@ def test_returns_500_when_blob_upload_fails(self):
159164
headers=AUTH_HEADERS,
160165
)
161166
assert res.status_code == 500
167+
168+
def test_returns_404_when_caller_does_not_own_process(self):
169+
# Process exists but is owned by a different user than the caller.
170+
app, mocks = _make_app(
171+
process_record=SimpleNamespace(
172+
id="p-1",
173+
user_id="someone-else",
174+
source_file_count=0,
175+
status="initialized",
176+
)
177+
)
178+
client = TestClient(app)
179+
res = client.post(
180+
"/api/file/upload",
181+
files={"file": ("a.txt", b"x", "text/plain")},
182+
data={"process_id": VALID_PROCESS_ID},
183+
headers=AUTH_HEADERS,
184+
)
185+
assert res.status_code == 404
186+
mocks["blob_helper"].upload_blob.assert_not_awaited()
187+
mocks["process_repo"].update_async.assert_not_awaited()
188+
189+
def test_returns_404_when_process_missing(self):
190+
app, mocks = _make_app(process_record=False)
191+
# Force get_async to return None (no such process).
192+
mocks["process_repo"].get_async = AsyncMock(return_value=None)
193+
client = TestClient(app)
194+
res = client.post(
195+
"/api/file/upload",
196+
files={"file": ("a.txt", b"x", "text/plain")},
197+
data={"process_id": VALID_PROCESS_ID},
198+
headers=AUTH_HEADERS,
199+
)
200+
assert res.status_code == 404
201+
mocks["blob_helper"].upload_blob.assert_not_awaited()

0 commit comments

Comments
 (0)