From 4f6eff101ffd1498d328bdafbbc4500a0dd43eeb Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 19:42:35 +0100 Subject: [PATCH 1/8] removed old migrtion script --- CargoHubV2/app/main.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/CargoHubV2/app/main.py b/CargoHubV2/app/main.py index 18601a7..e12bba6 100644 --- a/CargoHubV2/app/main.py +++ b/CargoHubV2/app/main.py @@ -91,20 +91,3 @@ async def api_key_middleware(request: Request, call_next): except Exception as exc: logger.exception("Unexpected error occurred in middleware") raise exc - -''' - -# script voor migrations voor later -from database import Base, engine -from models import warehouse_model, items_model, location_model, transfers_model # alle models die je wil migraten - - -# maakt alle tables -def init_db(): - Base.metadata.create_all(bind=engine) - - -if __name__ == "__main__": - init_db() - print("Tables created successfully!") -''' From 16d3050a95328b7ab09d4da1c0562b1ee4d40830 Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 20:11:26 +0100 Subject: [PATCH 2/8] api keys worden weer opgehaald uit .env --- CargoHubV2/app/main.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/CargoHubV2/app/main.py b/CargoHubV2/app/main.py index e12bba6..840a13f 100644 --- a/CargoHubV2/app/main.py +++ b/CargoHubV2/app/main.py @@ -15,7 +15,9 @@ from CargoHubV2.app.controllers import reporting_controller from CargoHubV2.app.controllers import packinglist_controller from CargoHubV2.app.controllers import docks_controller -import time + +import os +from dotenv import load_dotenv from starlette.responses import JSONResponse import logging @@ -44,6 +46,17 @@ logger = logging.getLogger("uvicorn.error") +# haalt api keys uit env variabelen +# in github werkt dit ook, uit GH secrets +# voor lokaal runnen, moet er een .env zijn met deze 3 variabelen +load_dotenv() +warehouse_manager = os.getenv("WAREHOUSE_MANAGER") +floor_manager = os.getenv("FLOOR_MANAGER") +employee = os.getenv("EMPLOYEE") +print(warehouse_manager) +print(floor_manager) +print(employee) + @app.get("/") async def root(): @@ -63,10 +76,12 @@ async def shutdown(): @app.middleware("http") async def api_key_middleware(request: Request, call_next): + # anders kan de documentatie niet bereikt worden excluded = ["/favicon.ico", "/openapi.json", "/docs"] + try: x_api_key = request.headers.get("api-key") - if request.url.path in excluded or "/get-pdf" in request.url.path: + if request.url.path in excluded: return await call_next(request) response: Response = await call_next(request) From 6fbb4d928fe917397093a1919c3aaed8bf9bcb95 Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 20:51:06 +0100 Subject: [PATCH 3/8] access decided for roles and checks in middleware --- CargoHubV2/app/main.py | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/CargoHubV2/app/main.py b/CargoHubV2/app/main.py index 840a13f..d541ba7 100644 --- a/CargoHubV2/app/main.py +++ b/CargoHubV2/app/main.py @@ -26,7 +26,7 @@ # welke port hij runt kan je bij command aanpassen # default port is localhost:8000 -# router van de controller gebruiken +# routers van de controllers gebruiken app.include_router(reporting_controller.router) app.include_router(item_groups.router) app.include_router(item_lines.router) @@ -36,7 +36,6 @@ app.include_router(transfers_controller.router) app.include_router(suppliers_controller.router) app.include_router(warehouses_controller.router) -app.include_router(load_controller.router) app.include_router(clients_controller.router) app.include_router(shipments_controller.router) app.include_router(inventories_controller.router) @@ -53,9 +52,11 @@ warehouse_manager = os.getenv("WAREHOUSE_MANAGER") floor_manager = os.getenv("FLOOR_MANAGER") employee = os.getenv("EMPLOYEE") +''' print(warehouse_manager) print(floor_manager) print(employee) +''' @app.get("/") @@ -79,6 +80,12 @@ async def api_key_middleware(request: Request, call_next): # anders kan de documentatie niet bereikt worden excluded = ["/favicon.ico", "/openapi.json", "/docs"] + w_man_only = ["v2/reports", "v2/warehouses", "v2/clients"] + all_managers = ["v2/item_groups", "v2/item_lines", "v2/item_types", "v2/items", + "v2/shipments", "v2/docks"] + all = ["v2/locations", "v2/transfers", "v2/orders", "v2/inventories", + "v2/packinglist"] + try: x_api_key = request.headers.get("api-key") if request.url.path in excluded: @@ -90,10 +97,26 @@ async def api_key_middleware(request: Request, call_next): response.status_code = 422 raise HTTPException(status_code=422, detail="Missing API key") - if x_api_key != "a1b2c3d4e5": - logger.warning("Invalid API key") - response.status_code = 403 - raise HTTPException(status_code=403, detail="Invalid API key") + if any(path in request.url.path for path in w_man_only): + + if x_api_key != warehouse_manager: + logger.warning("Invalid API key") + response.status_code = 403 + raise HTTPException(status_code=403, detail="Invalid API key") + + if any(path in request.url.path for path in all_managers): + + if x_api_key != warehouse_manager and x_api_key != floor_manager: + logger.warning("Invalid API key") + response.status_code = 403 + raise HTTPException(status_code=403, detail="Invalid API key") + + if any(path in request.url.path for path in all): + + if x_api_key != warehouse_manager and x_api_key != floor_manager and x_api_key != employee: + logger.warning("Invalid API key") + response.status_code = 403 + raise HTTPException(status_code=403, detail="Invalid API key") return response From 4c2d0f17dfffa88cf8d0389a3db28d7fa3faeadf Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 20:55:16 +0100 Subject: [PATCH 4/8] clear error messages for different api keys --- CargoHubV2/app/main.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/CargoHubV2/app/main.py b/CargoHubV2/app/main.py index d541ba7..0888d10 100644 --- a/CargoHubV2/app/main.py +++ b/CargoHubV2/app/main.py @@ -102,21 +102,27 @@ async def api_key_middleware(request: Request, call_next): if x_api_key != warehouse_manager: logger.warning("Invalid API key") response.status_code = 403 - raise HTTPException(status_code=403, detail="Invalid API key") + raise HTTPException( + status_code=403, + detail="Invalid API key, need to be Warehouse manager") if any(path in request.url.path for path in all_managers): if x_api_key != warehouse_manager and x_api_key != floor_manager: logger.warning("Invalid API key") response.status_code = 403 - raise HTTPException(status_code=403, detail="Invalid API key") + raise HTTPException( + status_code=403, + detail="Invalid API key, only Floor/Warehouse managers") if any(path in request.url.path for path in all): if x_api_key != warehouse_manager and x_api_key != floor_manager and x_api_key != employee: logger.warning("Invalid API key") response.status_code = 403 - raise HTTPException(status_code=403, detail="Invalid API key") + raise HTTPException( + status_code=403, + detail="Invalid API key, need to be employee of CargoHub") return response From 5877c4f07ab325c14cd62409bc40cca8085a4002 Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 21:02:25 +0100 Subject: [PATCH 5/8] added the env variables to yaml, through GH secrets --- .github/workflows/python-app.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index ebc3ea3..898b507 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -76,6 +76,10 @@ jobs: run: echo "PYTHONPATH=$(pwd)" >> $GITHUB_ENV - name: run CargoHubV2 server + env: + WAREHOUSE_MANAGER: ${{ secrets.WAREHOUSE_MANAGER}} + FLOOR_MANAGER: ${{ secrets.FLOOR_MANAGER }} + EMPLOYEE: ${{ secrets.EMPLOYEE }} run: | uvicorn CargoHubV2.app.main:app --port 3000 & sleep 10 From d6d0bae01ebc50dabdd27427826e8897159d3f85 Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 21:07:54 +0100 Subject: [PATCH 6/8] added suppliers to Warehouse manager only list --- CargoHubV2/app/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CargoHubV2/app/main.py b/CargoHubV2/app/main.py index 0888d10..9bf1d6b 100644 --- a/CargoHubV2/app/main.py +++ b/CargoHubV2/app/main.py @@ -80,7 +80,7 @@ async def api_key_middleware(request: Request, call_next): # anders kan de documentatie niet bereikt worden excluded = ["/favicon.ico", "/openapi.json", "/docs"] - w_man_only = ["v2/reports", "v2/warehouses", "v2/clients"] + w_man_only = ["v2/reports", "v2/warehouses", "v2/clients", "v2/suppliers"] all_managers = ["v2/item_groups", "v2/item_lines", "v2/item_types", "v2/items", "v2/shipments", "v2/docks"] all = ["v2/locations", "v2/transfers", "v2/orders", "v2/inventories", From 2ed03aca8414274bcf2f2b420ed6a74a86944efd Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 21:25:37 +0100 Subject: [PATCH 7/8] path traversal vulnerability patched for pdf endpoints --- CargoHubV2/app/controllers/packinglist_controller.py | 11 ++++++++--- CargoHubV2/app/controllers/reporting_controller.py | 9 +++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CargoHubV2/app/controllers/packinglist_controller.py b/CargoHubV2/app/controllers/packinglist_controller.py index 9d1eef9..5d02cae 100644 --- a/CargoHubV2/app/controllers/packinglist_controller.py +++ b/CargoHubV2/app/controllers/packinglist_controller.py @@ -11,6 +11,7 @@ tags=["packinglist"] ) + @router.get("/api/v2/packinglist/{order_id}") def create_packing_list( order_id: int, @@ -26,8 +27,12 @@ def create_packing_list( @router.get("/get-pdf/{filename}") def get_pdf(filename: str): PDF_DIR = Path("generated_pdfs") - pdf_path = PDF_DIR/filename + + # voorkomt path traversal + sanitized_filename = Path(filename).name + pdf_path = PDF_DIR/sanitized_filename + if pdf_path.exists(): - return FileResponse(pdf_path, media_type="application/pdf", filename=filename) + return FileResponse(pdf_path, media_type="application/pdf", filename=sanitized_filename) else: - raise HTTPException(status_code=404, detail="PDF not found") \ No newline at end of file + raise HTTPException(status_code=404, detail="PDF not found") diff --git a/CargoHubV2/app/controllers/reporting_controller.py b/CargoHubV2/app/controllers/reporting_controller.py index 3242c0a..28d9f4d 100644 --- a/CargoHubV2/app/controllers/reporting_controller.py +++ b/CargoHubV2/app/controllers/reporting_controller.py @@ -40,8 +40,13 @@ def generate_general_report( @router.get("/get-pdf/{filename}") def get_pdf(filename: str): PDF_DIR = Path("generated_pdfs") - pdf_path = PDF_DIR/filename + # voorkomt path traversal + sanitized_filename = Path(filename).name + pdf_path = PDF_DIR/sanitized_filename + if pdf_path.exists(): - return FileResponse(pdf_path, media_type="application/pdf", filename=filename) + return FileResponse( + pdf_path, + media_type="application/pdf", filename=sanitized_filename) else: raise HTTPException(status_code=404, detail="PDF not found") From 4e58b4f078aeede39eb97df3f20a3b8e0feba992 Mon Sep 17 00:00:00 2001 From: yassyass2 Date: Tue, 21 Jan 2025 21:44:35 +0100 Subject: [PATCH 8/8] checks if base path is modified --- CargoHubV2/app/controllers/packinglist_controller.py | 9 +++++++-- CargoHubV2/app/controllers/reporting_controller.py | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CargoHubV2/app/controllers/packinglist_controller.py b/CargoHubV2/app/controllers/packinglist_controller.py index 5d02cae..02e96ce 100644 --- a/CargoHubV2/app/controllers/packinglist_controller.py +++ b/CargoHubV2/app/controllers/packinglist_controller.py @@ -25,14 +25,19 @@ def create_packing_list( @router.get("/get-pdf/{filename}") -def get_pdf(filename: str): +def get_pdf(filename: str, api_key: str = Header(...)): PDF_DIR = Path("generated_pdfs") # voorkomt path traversal sanitized_filename = Path(filename).name pdf_path = PDF_DIR/sanitized_filename + if not str(pdf_path).startswith(str(PDF_DIR)): + raise HTTPException(status_code=403, detail="Base path modified") + if pdf_path.exists(): - return FileResponse(pdf_path, media_type="application/pdf", filename=sanitized_filename) + return FileResponse( + pdf_path, media_type="application/pdf", + filename=sanitized_filename) else: raise HTTPException(status_code=404, detail="PDF not found") diff --git a/CargoHubV2/app/controllers/reporting_controller.py b/CargoHubV2/app/controllers/reporting_controller.py index 28d9f4d..4804ee9 100644 --- a/CargoHubV2/app/controllers/reporting_controller.py +++ b/CargoHubV2/app/controllers/reporting_controller.py @@ -38,12 +38,15 @@ def generate_general_report( @router.get("/get-pdf/{filename}") -def get_pdf(filename: str): +def get_pdf(filename: str, api_key: str = Header(...)): PDF_DIR = Path("generated_pdfs") # voorkomt path traversal sanitized_filename = Path(filename).name pdf_path = PDF_DIR/sanitized_filename + if not str(pdf_path).startswith(str(PDF_DIR)): + raise HTTPException(status_code=403, detail="Base path modified") + if pdf_path.exists(): return FileResponse( pdf_path,