feat(campaigns): require an operator identity to approve
approved_by was client-supplied, making approval forgeable ahead of Phase 2 pushing real spend to Meta. Add a proportionate operator-token check (no login system) via a new require_operator FastAPI dependency: MASKANX_OPERATOR_TOKENS maps name:token pairs, and approve_campaign now derives its actor solely from the resolved operator, never from the client-supplied ActorPayload.actor. Unset env resolves to "unauthenticated" so local dev and existing tests are not blocked; set-but-unrecognised tokens 401. submit and reject remain unauthenticated since neither authorises spend. Updates test_submit_then_approve_moves_through_states to assert approved_by == "unauthenticated" (env var unset in tests) instead of the previously-trusted client actor, since a forged actor must now be ignored.
This commit is contained in:
@@ -0,0 +1,76 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
"""Operator identity for state changes that authorise spend.
|
||||
|
||||
MaskanX has no login system. Approving a campaign and launching it are the
|
||||
two actions that lead to money being spent, so they must be attributable to
|
||||
a named operator rather than to a string the client chose.
|
||||
|
||||
Configure `MASKANX_OPERATOR_TOKENS` as comma-separated `name:token` pairs.
|
||||
When it is unset the dependency returns "unauthenticated" so local
|
||||
development is not blocked, and logs a warning.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
|
||||
from fastapi import Header, HTTPException, status
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
OPERATOR_TOKENS_ENV = "MASKANX_OPERATOR_TOKENS"
|
||||
UNAUTHENTICATED = "unauthenticated"
|
||||
|
||||
_warned = False
|
||||
|
||||
|
||||
class OperatorAuthError(Exception):
|
||||
"""Raised when an operator token is missing or unknown."""
|
||||
|
||||
|
||||
def _token_map() -> dict[str, str]:
|
||||
raw = (os.environ.get(OPERATOR_TOKENS_ENV) or "").strip()
|
||||
if not raw:
|
||||
return {}
|
||||
mapping: dict[str, str] = {}
|
||||
for entry in raw.split(","):
|
||||
name, _, token = entry.partition(":")
|
||||
name = name.strip()
|
||||
token = token.strip()
|
||||
if name and token:
|
||||
mapping[token] = name
|
||||
return mapping
|
||||
|
||||
|
||||
def resolve_operator(token: str | None) -> str:
|
||||
"""Return the operator name for a token, or raise OperatorAuthError."""
|
||||
global _warned
|
||||
tokens = _token_map()
|
||||
if not tokens:
|
||||
if not _warned:
|
||||
logger.warning(
|
||||
"%s is not set. Campaign approval and launch are "
|
||||
"unauthenticated; set it before running live campaigns.",
|
||||
OPERATOR_TOKENS_ENV,
|
||||
)
|
||||
_warned = True
|
||||
return UNAUTHENTICATED
|
||||
if not token or token not in tokens:
|
||||
raise OperatorAuthError("Unknown or missing operator token.")
|
||||
return tokens[token]
|
||||
|
||||
|
||||
def require_operator(
|
||||
x_maskanx_operator: str | None = Header(default=None),
|
||||
) -> str:
|
||||
"""FastAPI dependency resolving the calling operator."""
|
||||
try:
|
||||
return resolve_operator(x_maskanx_operator)
|
||||
except OperatorAuthError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail=(
|
||||
"A valid X-MaskanX-Operator header is required to approve or "
|
||||
"launch a campaign."
|
||||
),
|
||||
) from exc
|
||||
@@ -10,7 +10,7 @@ import logging
|
||||
import uuid
|
||||
from typing import Any
|
||||
|
||||
from fastapi import APIRouter, HTTPException, status as http_status
|
||||
from fastapi import APIRouter, Depends, HTTPException, status as http_status
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from ...campaigns.models import CampaignSpec
|
||||
@@ -22,6 +22,7 @@ from ...campaigns.state import (
|
||||
)
|
||||
from ...campaigns.validation import validate_campaign
|
||||
from ...meta.client import MetaClient, MetaError, access_token_from_env
|
||||
from ._operator import require_operator
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -252,8 +253,14 @@ async def submit_campaign(
|
||||
async def approve_campaign(
|
||||
campaign_id: str,
|
||||
payload: ActorPayload | None = None,
|
||||
operator: str = Depends(require_operator),
|
||||
) -> CampaignSpec:
|
||||
return await _transition(campaign_id, "approve", payload or ActorPayload())
|
||||
body = payload or ActorPayload()
|
||||
return await _transition(
|
||||
campaign_id,
|
||||
"approve",
|
||||
ActorPayload(actor=operator, reason=body.reason),
|
||||
)
|
||||
|
||||
|
||||
@router.post("/{campaign_id}/reject", response_model=CampaignSpec)
|
||||
|
||||
@@ -228,6 +228,7 @@ _KEY_REGISTRY: List[Dict[str, str]] = [
|
||||
{"key": "META_ADS_ACCESS_TOKEN", "plugin": "Meta Ads", "description": "Facebook & Instagram ads access token"},
|
||||
{"key": "META_APP_ID", "plugin": "Meta Ads", "description": "Meta app ID, enables automatic token refresh"},
|
||||
{"key": "META_APP_SECRET", "plugin": "Meta Ads", "description": "Meta app secret, enables automatic token refresh"},
|
||||
{"key": "MASKANX_OPERATOR_TOKENS", "plugin": "Campaigns", "description": "Comma-separated name:token pairs authorised to approve and launch campaigns"},
|
||||
# Analytics
|
||||
# (Google Analytics uses OAuth, no static key needed)
|
||||
# Social Media
|
||||
|
||||
@@ -105,9 +105,14 @@ def test_submit_then_approve_moves_through_states(client):
|
||||
submitted = client.post(f"/api/campaigns/{cid}/submit")
|
||||
assert submitted.json()["status"] == "pending_approval"
|
||||
|
||||
# `actor` in the body is a forged-identity attempt: approval must be
|
||||
# attributable to the resolved operator, never to a client-supplied
|
||||
# string. With MASKANX_OPERATOR_TOKENS unset (as in this test), the
|
||||
# operator dependency resolves to "unauthenticated" rather than trusting
|
||||
# the body.
|
||||
approved = client.post(f"/api/campaigns/{cid}/approve", json={"actor": "owner"})
|
||||
assert approved.json()["status"] == "approved"
|
||||
assert approved.json()["approved_by"] == "owner"
|
||||
assert approved.json()["approved_by"] == "unauthenticated"
|
||||
|
||||
|
||||
def test_approve_from_draft_is_rejected(client):
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
"""Approve and launch must be attributable to a known operator."""
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from adclaw.app.routers import _operator
|
||||
|
||||
|
||||
def _app():
|
||||
from adclaw.app.routers import campaigns as campaigns_router
|
||||
|
||||
app = FastAPI()
|
||||
app.include_router(campaigns_router.router, prefix="/api")
|
||||
return app
|
||||
|
||||
|
||||
def test_unset_env_allows_and_reports_unauthenticated(monkeypatch):
|
||||
monkeypatch.delenv("MASKANX_OPERATOR_TOKENS", raising=False)
|
||||
assert _operator.resolve_operator(None) == "unauthenticated"
|
||||
|
||||
|
||||
def test_known_token_resolves_to_its_name(monkeypatch):
|
||||
monkeypatch.setenv("MASKANX_OPERATOR_TOKENS", "owner:s3cr3t,ops:t0ken")
|
||||
assert _operator.resolve_operator("s3cr3t") == "owner"
|
||||
assert _operator.resolve_operator("t0ken") == "ops"
|
||||
|
||||
|
||||
def test_unknown_token_is_rejected(monkeypatch):
|
||||
monkeypatch.setenv("MASKANX_OPERATOR_TOKENS", "owner:s3cr3t")
|
||||
with pytest.raises(_operator.OperatorAuthError):
|
||||
_operator.resolve_operator("wrong")
|
||||
|
||||
|
||||
def test_missing_header_is_rejected_when_configured(monkeypatch):
|
||||
monkeypatch.setenv("MASKANX_OPERATOR_TOKENS", "owner:s3cr3t")
|
||||
with pytest.raises(_operator.OperatorAuthError):
|
||||
_operator.resolve_operator(None)
|
||||
|
||||
|
||||
def test_malformed_config_entries_are_ignored(monkeypatch):
|
||||
monkeypatch.setenv("MASKANX_OPERATOR_TOKENS", "bad,owner:s3cr3t,:empty,x:")
|
||||
assert _operator.resolve_operator("s3cr3t") == "owner"
|
||||
with pytest.raises(_operator.OperatorAuthError):
|
||||
_operator.resolve_operator("empty")
|
||||
|
||||
|
||||
def test_approve_uses_operator_identity_not_client_actor(monkeypatch):
|
||||
"""The body's `actor` must never override the authenticated operator."""
|
||||
monkeypatch.setenv("MASKANX_OPERATOR_TOKENS", "owner:s3cr3t")
|
||||
client = TestClient(_app())
|
||||
# Full flow is covered in test_campaign_api.py; here we assert the
|
||||
# dependency is wired so a forged actor cannot win.
|
||||
from adclaw.app.routers import campaigns as campaigns_router
|
||||
|
||||
import inspect
|
||||
|
||||
source = inspect.getsource(campaigns_router.approve_campaign)
|
||||
assert "require_operator" in source
|
||||
assert "payload.actor" not in source
|
||||
Reference in New Issue
Block a user