From 95b5ac50c64a421e4f3c6412aa11303ad4611790 Mon Sep 17 00:00:00 2001 From: kitos Date: Fri, 3 Jul 2026 12:27:25 +0200 Subject: [PATCH] test(campaigns): promote cookie-safe request helper to shared conftest fixture Moves the local _post cookie-clearing helper into a shared 'api' fixture in conftest.py so later router tests in this plan (Tasks 10/11) don't have to reinvent or forget the TestClient cookie-vs-Authorization-header gotcha. Also adds a one-line comment at both require_any_role("manager") call sites clarifying admin passthrough is automatic. --- backend/app/routers/campaigns.py | 2 + backend/tests/conftest.py | 22 +++++++ .../tests/test_campaign_approval_router.py | 62 +++++++------------ 3 files changed, 48 insertions(+), 38 deletions(-) diff --git a/backend/app/routers/campaigns.py b/backend/app/routers/campaigns.py index d798946..fc5c213 100644 --- a/backend/app/routers/campaigns.py +++ b/backend/app/routers/campaigns.py @@ -446,6 +446,7 @@ def approve_campaign_endpoint( campaign_id: str, payload: ApprovePayload, db: Session = Depends(get_db), + # admin passes automatically via require_any_role's built-in bypass — do not add "admin" here current_user: User = Depends(require_any_role("manager")), ) -> dict: """Manager approves a pending campaign, fixing its start date and activating it.""" @@ -477,6 +478,7 @@ def reject_campaign_endpoint( campaign_id: str, payload: RejectPayload, db: Session = Depends(get_db), + # admin passes automatically via require_any_role's built-in bypass — do not add "admin" here current_user: User = Depends(require_any_role("manager")), ) -> dict: """Manager rejects a pending campaign, returning it to draft with a reason.""" diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 686aae6..f3952ca 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -328,3 +328,25 @@ def manager_token(client, manager_user): def manager_headers(manager_token): """Return authorization headers for manager user.""" return {"Authorization": f"Bearer {manager_token}"} + + +@pytest.fixture(scope="function") +def api(client): + """Issue an authenticated request while avoiding stale-cookie role bleed. + + ``client``'s cookie jar persists across requests within a test, and + ``get_current_user`` prefers the ``aegis_token`` cookie over the + ``Authorization`` header. A test that uses more than one role's + ``*_headers`` fixture (e.g. submit as red_lead, then approve as + manager) would otherwise have its *first* request silently + authenticate as whichever role's fixture happens to log in last, + since pytest resolves all fixtures before the test body runs. Use + this instead of ``client.post``/``client.get`` directly whenever a + test mixes more than one role. + + Usage: ``api("post", url, headers, json=payload)``. + """ + def _request(method: str, url: str, headers: dict, **kwargs): + client.cookies.clear() + return getattr(client, method)(url, headers=headers, **kwargs) + return _request diff --git a/backend/tests/test_campaign_approval_router.py b/backend/tests/test_campaign_approval_router.py index 03e9b70..844a1a0 100644 --- a/backend/tests/test_campaign_approval_router.py +++ b/backend/tests/test_campaign_approval_router.py @@ -6,20 +6,6 @@ from app.models.test import Test from app.models.enums import TestState -def _post(client, url, headers, **kwargs): - """POST while forcing auth via the Authorization header. - - The login endpoint also sets an HttpOnly ``aegis_token`` cookie, and the - shared ``TestClient`` cookie jar persists across requests within a test. - ``get_current_user`` prefers the cookie over the ``Authorization`` - header, so once a second role's headers fixture logs in (setting its own - cookie), a request made with an *earlier* role's headers would silently - authenticate as the later role unless the leftover cookie is cleared. - """ - client.cookies.clear() - return client.post(url, headers=headers, **kwargs) - - def _make_draft_campaign(db, owner_id): tech = Technique(mitre_id="T1059", name="Command Line", tactic="execution", platforms=["windows"]) db.add(tech) @@ -36,25 +22,25 @@ def _make_draft_campaign(db, owner_id): return campaign -def test_lead_can_submit_own_campaign(client, db, red_lead_user, red_lead_headers): +def test_lead_can_submit_own_campaign(api, db, red_lead_user, red_lead_headers): campaign = _make_draft_campaign(db, red_lead_user.id) - resp = _post(client, f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) + resp = api("post", f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) assert resp.status_code == 200 assert resp.json()["status"] == "pending_approval" -def test_red_tech_cannot_submit(client, db, red_lead_user, red_tech_headers): +def test_red_tech_cannot_submit(api, db, red_lead_user, red_tech_headers): campaign = _make_draft_campaign(db, red_lead_user.id) - resp = _post(client, f"/api/v1/campaigns/{campaign.id}/submit", red_tech_headers) + resp = api("post", f"/api/v1/campaigns/{campaign.id}/submit", red_tech_headers) assert resp.status_code == 403 -def test_manager_can_approve_and_sets_start_date(client, db, red_lead_user, red_lead_headers, manager_headers): +def test_manager_can_approve_and_sets_start_date(api, db, red_lead_user, red_lead_headers, manager_headers): campaign = _make_draft_campaign(db, red_lead_user.id) - _post(client, f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) + api("post", f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) - resp = _post( - client, + resp = api( + "post", f"/api/v1/campaigns/{campaign.id}/approve", manager_headers, json={"start_date": "2026-09-01T00:00:00"}, @@ -65,12 +51,12 @@ def test_manager_can_approve_and_sets_start_date(client, db, red_lead_user, red_ assert body["start_date"] is not None -def test_lead_cannot_approve(client, db, red_lead_user, red_lead_headers): +def test_lead_cannot_approve(api, db, red_lead_user, red_lead_headers): campaign = _make_draft_campaign(db, red_lead_user.id) - _post(client, f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) + api("post", f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) - resp = _post( - client, + resp = api( + "post", f"/api/v1/campaigns/{campaign.id}/approve", red_lead_headers, json={"start_date": "2026-09-01T00:00:00"}, @@ -78,12 +64,12 @@ def test_lead_cannot_approve(client, db, red_lead_user, red_lead_headers): assert resp.status_code == 403 -def test_admin_can_approve_as_manager_backup(client, db, red_lead_user, red_lead_headers, auth_headers): +def test_admin_can_approve_as_manager_backup(api, db, red_lead_user, red_lead_headers, auth_headers): campaign = _make_draft_campaign(db, red_lead_user.id) - _post(client, f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) + api("post", f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) - resp = _post( - client, + resp = api( + "post", f"/api/v1/campaigns/{campaign.id}/approve", auth_headers, json={"start_date": "2026-09-01T00:00:00"}, @@ -91,12 +77,12 @@ def test_admin_can_approve_as_manager_backup(client, db, red_lead_user, red_lead assert resp.status_code == 200 -def test_manager_can_reject_with_reason(client, db, red_lead_user, red_lead_headers, manager_headers): +def test_manager_can_reject_with_reason(api, db, red_lead_user, red_lead_headers, manager_headers): campaign = _make_draft_campaign(db, red_lead_user.id) - _post(client, f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) + api("post", f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) - resp = _post( - client, + resp = api( + "post", f"/api/v1/campaigns/{campaign.id}/reject", manager_headers, json={"reason": "Needs more detail"}, @@ -105,12 +91,12 @@ def test_manager_can_reject_with_reason(client, db, red_lead_user, red_lead_head assert resp.json()["status"] == "draft" -def test_manager_reject_without_reason_rejected_by_validation(client, db, red_lead_user, red_lead_headers, manager_headers): +def test_manager_reject_without_reason_rejected_by_validation(api, db, red_lead_user, red_lead_headers, manager_headers): campaign = _make_draft_campaign(db, red_lead_user.id) - _post(client, f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) + api("post", f"/api/v1/campaigns/{campaign.id}/submit", red_lead_headers) - resp = _post( - client, + resp = api( + "post", f"/api/v1/campaigns/{campaign.id}/reject", manager_headers, json={"reason": ""},