From 5bc71f677f1a8a9be06c3654135cd06e49c03da1 Mon Sep 17 00:00:00 2001 From: kitos Date: Fri, 3 Jul 2026 13:57:43 +0200 Subject: [PATCH] refactor(campaigns): remove dead future-start_date scheduling code Now that only the manager sets start_date, and only at the moment of approval (which immediately activates the campaign), a draft campaign can never have a start_date. This made three things permanently unreachable: the hourly _run_scheduled_campaign_activation cron job, the activate_campaign 409/force future-date guard, and a query filter hiding tests from scheduled-but-inactive draft campaigns. Removes all three rather than leaving dead code behind. Also adds the missing manager-cannot-directly-activate router test. --- backend/app/jobs/mitre_sync_job.py | 99 ------------------- backend/app/routers/campaigns.py | 30 ++---- backend/app/services/test_crud_service.py | 18 +--- .../tests/test_campaign_approval_router.py | 7 ++ 4 files changed, 14 insertions(+), 140 deletions(-) diff --git a/backend/app/jobs/mitre_sync_job.py b/backend/app/jobs/mitre_sync_job.py index f591441..79dc38b 100644 --- a/backend/app/jobs/mitre_sync_job.py +++ b/backend/app/jobs/mitre_sync_job.py @@ -164,96 +164,6 @@ def _run_recurring_campaigns() -> None: db.close() -def _run_scheduled_campaign_activation() -> None: - """Auto-activate campaigns whose start_date has arrived. - - Finds all campaigns in 'draft' state with a start_date <= now, - activates them, creates Jira tickets, and notifies the red_tech team. - Runs every hour so campaigns activate within ~1 hour of their scheduled time. - """ - logger.info("Scheduled campaign auto-activation check starting...") - db = SessionLocal() - try: - from datetime import datetime as _dt - from app.models.campaign import Campaign - from app.models.user import User - from app.services.campaign_crud_service import activate_campaign as _activate - from app.services.notification_service import notify_role - from app.services.audit_service import log_action - - now = _dt.utcnow() - due_campaigns = ( - db.query(Campaign) - .filter( - Campaign.status == "draft", - Campaign.start_date != None, # noqa: E711 - Campaign.start_date <= now, - ) - .all() - ) - - activated = 0 - for campaign in due_campaigns: - try: - _activate(db, str(campaign.id)) - notify_role( - db, - role="red_tech", - type="campaign_activated", - title="Campaign auto-activated", - message=f'Campaign "{campaign.name}" has been automatically activated on its scheduled start date.', - entity_type="campaign", - entity_id=campaign.id, - ) - log_action( - db, - user_id=None, - action="auto_activate_campaign", - entity_type="campaign", - entity_id=campaign.id, - details={"name": campaign.name, "start_date": str(campaign.start_date)}, - ) - - # Create Jira tickets non-fatally - try: - from app.services.jira_service import ( - auto_create_campaign_issue, - auto_create_test_issue, - get_campaign_jira_key, - get_test_jira_key, - ) - # Use first admin user as actor for Jira auth - admin_user = db.query(User).filter(User.role == "admin").first() - if admin_user: - db.refresh(campaign) - campaign_jira_key = get_campaign_jira_key(db, str(campaign.id)) - if not campaign_jira_key: - campaign_jira_key = auto_create_campaign_issue(db, campaign, admin_user) - if campaign_jira_key: - for ct in campaign.campaign_tests: - if ct.test and not get_test_jira_key(db, ct.test.id): - auto_create_test_issue( - db, ct.test, admin_user, - parent_ticket_override=campaign_jira_key, - campaign_start_date=campaign.start_date, - ) - except Exception: - logger.exception("Jira auto-create failed for auto-activated campaign %s", campaign.id) - - db.commit() - activated += 1 - logger.info("Auto-activated campaign %s (%s)", campaign.id, campaign.name) - except Exception: - logger.exception("Failed to auto-activate campaign %s", campaign.id) - db.rollback() - - logger.info("Campaign auto-activation check finished — activated %d campaigns", activated) - except Exception: - logger.exception("Campaign auto-activation job failed") - finally: - db.close() - - def _run_intel_scan() -> None: """Execute an intel scan inside its own DB session.""" # Log info: "Scheduled intel scan job starting..." @@ -574,15 +484,6 @@ def start_scheduler() -> None: # Keyword argument: replace_existing replace_existing=True, ) - # Call scheduler.add_job() - scheduler.add_job( - _run_scheduled_campaign_activation, - trigger="interval", - hours=1, - id="scheduled_campaign_activation", - name="Auto-activate campaigns on start_date (hourly)", - replace_existing=True, - ) scheduler.add_job( _run_recurring_campaigns, # Keyword argument: trigger diff --git a/backend/app/routers/campaigns.py b/backend/app/routers/campaigns.py index 5118823..9e9a800 100644 --- a/backend/app/routers/campaigns.py +++ b/backend/app/routers/campaigns.py @@ -785,36 +785,18 @@ def remove_test_from_campaign( def activate_campaign( # Entry: campaign_id campaign_id: str, - force: bool = Query(False, description="Activate even if start_date is in the future"), 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("admin")), ): - """Activate a campaign, moving it from draft to active. + """Admin-only emergency override: activate a draft campaign directly, bypassing the approval queue. - If the campaign has a start_date in the future and force=False, returns a 409 - with a warning so the frontend can show a confirmation modal. If force=True, - activates immediately regardless of start_date. + Every other path to 'active' goes through /submit -> /approve, where the + manager sets start_date. A draft campaign never has a start_date set + (only /approve sets it, in the same call that moves the campaign to + 'active'), so there is no "scheduled for the future" case to guard + against here anymore. """ - from fastapi import HTTPException - campaign_obj = db.query(Campaign).filter(Campaign.id == uuid.UUID(campaign_id)).first() - if campaign_obj and campaign_obj.start_date and not force: - now = datetime.utcnow() - if campaign_obj.start_date > now: - raise HTTPException( - status_code=409, - detail={ - "code": "start_date_in_future", - "start_date": campaign_obj.start_date.strftime("%Y-%m-%d"), - "message": ( - f"This campaign is scheduled to start on " - f"{campaign_obj.start_date.strftime('%d %b %Y')}. " - f"It will activate automatically on that date. " - f"Do you want to activate it now anyway?" - ), - }, - ) - with UnitOfWork(db) as uow: # Assign campaign = crud_activate(db, campaign_id) campaign = crud_activate(db, campaign_id) diff --git a/backend/app/services/test_crud_service.py b/backend/app/services/test_crud_service.py index 47707a0..c7aba76 100644 --- a/backend/app/services/test_crud_service.py +++ b/backend/app/services/test_crud_service.py @@ -22,7 +22,7 @@ from app.models.enums import TestState from app.models.technique import Technique from app.models.test import Test from app.models.test_template import TestTemplate -from app.models.campaign import Campaign, CampaignTest +from app.models.campaign import CampaignTest from app.models.audit import AuditLog from app.utils import escape_like @@ -89,22 +89,6 @@ def list_tests( linked = db.query(CampaignTest.test_id).distinct().subquery() query = query.filter(~Test.id.in_(linked)) - # Always hide tests from scheduled campaigns that haven't started yet. - # A "scheduled-but-not-yet-active" campaign = draft status + start_date in future. - now = datetime.utcnow() - future_draft_tests = ( - db.query(CampaignTest.test_id) - .join(Campaign, Campaign.id == CampaignTest.campaign_id) - .filter( - Campaign.status == "draft", - Campaign.start_date.isnot(None), - Campaign.start_date > now, - ) - .distinct() - .subquery() - ) - query = query.filter(~Test.id.in_(future_draft_tests)) - # Return query.order_by(Test.created_at.desc()).offset(offset).limit(limit).... return query.order_by(Test.created_at.desc()).offset(offset).limit(limit).all() diff --git a/backend/tests/test_campaign_approval_router.py b/backend/tests/test_campaign_approval_router.py index 5166c31..2d79ac1 100644 --- a/backend/tests/test_campaign_approval_router.py +++ b/backend/tests/test_campaign_approval_router.py @@ -279,6 +279,13 @@ def test_admin_can_still_directly_activate_as_override(api, db, red_lead_user, r assert resp.json()["status"] == "active" +def test_manager_cannot_directly_activate_draft_campaign(api, db, red_lead_user, manager_headers): + """A manager approves via /approve — the direct /activate bypass is admin-only.""" + campaign = _make_draft_campaign(db, red_lead_user.id) + resp = api("post", f"/api/v1/campaigns/{campaign.id}/activate", manager_headers) + assert resp.status_code == 403 + + def test_create_campaign_payload_has_no_start_date_field(api, db, red_lead_headers): resp = api( "post",