diff --git a/backend/app/api_admin.py b/backend/app/api_admin.py index 51c1b3c..f2edf23 100644 --- a/backend/app/api_admin.py +++ b/backend/app/api_admin.py @@ -672,6 +672,73 @@ def make_router(config: Config) -> APIRouter: "has_more": len(rows) == limit, } + @router.get("/api/admin/outbound-emails") + async def list_outbound_emails( + request: Request, + kind: str | None = None, + status: str | None = None, + to_address: str | None = None, + limit: int = Query(default=100, ge=1, le=500), + before_id: int | None = None, + ) -> dict[str, Any]: + """v0.18.0 Slice 4: read-only inspection of the + `outbound_emails` audit table. + + Answers questions like "did this person ever get their + invite?" without grepping VM logs. Filterable by kind + ('otc' | 'invite' | 'notification' | 'bundle' | 'digest'), + status ('sent' | 'failed' | 'deferred' | 'bounced'), and + to_address; the latter is exact-match because the audit + question is usually "the specific person who said they + didn't receive it." Per the proposal, no admin UI ships + with v0.18.0 — operator queries via curl + jq for now. + """ + auth.require_admin(request) + clauses: list[str] = [] + args: list[Any] = [] + if kind: + clauses.append("kind = ?") + args.append(kind) + if status: + clauses.append("status = ?") + args.append(status) + if to_address: + clauses.append("LOWER(to_address) = LOWER(?)") + args.append(to_address) + if before_id is not None: + clauses.append("id < ?") + args.append(before_id) + where = ("WHERE " + " AND ".join(clauses)) if clauses else "" + rows = db.conn().execute( + f""" + SELECT id, to_address, from_address, subject, kind, sent_at, + status, error, notification_id, message_id + FROM outbound_emails + {where} + ORDER BY id DESC + LIMIT ? + """, + (*args, limit), + ).fetchall() + return { + "items": [ + { + "id": r["id"], + "to_address": r["to_address"], + "from_address": r["from_address"], + "subject": r["subject"], + "kind": r["kind"], + "sent_at": r["sent_at"], + "status": r["status"], + "error": r["error"], + "notification_id": r["notification_id"], + "message_id": r["message_id"], + } + for r in rows + ], + "has_more": len(rows) == limit, + } + @router.get("/api/admin/permission-events") async def list_permission_events( request: Request, diff --git a/backend/app/digest.py b/backend/app/digest.py index 1e91de9..9a3ea13 100644 --- a/backend/app/digest.py +++ b/backend/app/digest.py @@ -191,6 +191,7 @@ def assemble_for_user( cfg, email, subject, body, unsubscribe_mailto=cfg.unsubscribe_mailto, unsubscribe_url=unsubscribe_url, + kind="digest", ) if not sent: return False diff --git a/backend/app/email.py b/backend/app/email.py index 5e150b2..79c64fb 100644 --- a/backend/app/email.py +++ b/backend/app/email.py @@ -281,6 +281,8 @@ def _send_one(user: Any, notif_id: int, payload: dict, category: str) -> None: body, unsubscribe_mailto=cfg.unsubscribe_mailto, unsubscribe_url=unsubscribe_url, + kind="notification", + notification_id=notif_id, ) if not sent: return @@ -344,6 +346,8 @@ def _deliver( *, unsubscribe_mailto: str | None = None, unsubscribe_url: str | None = None, + kind: str = "notification", + notification_id: int | None = None, ) -> bool: """Build the envelope via the shared `build_envelope` helper and hand it to SMTP. @@ -353,6 +357,13 @@ def _deliver( backward-compatibility with tests that read those directly. Newer tests can assert on the header surface by inspecting `envelope["message"]`. + + v0.18.0 Slice 4: also writes one row to `outbound_emails` + capturing the attempt. status='sent' on success, 'failed' on + SMTP exception, 'deferred' on the dev-fallback path (no + SMTP_HOST configured — the send didn't happen, but the row + records the attempt so the admin endpoint can answer "did the + framework try?"). """ msg = build_envelope( to_address=to_address, @@ -369,10 +380,21 @@ def _deliver( "subject": subject, "body": body, "message": msg, + "kind": kind, } _SENT.append(envelope) + message_id = msg["Message-ID"] if not cfg.smtp_host: log.info("email (stdout fallback): to=%s subject=%s", to_address, subject) + record_outbound( + to_address=to_address, + from_address=cfg.from_address, + subject=subject, + kind=kind, + status="deferred", + message_id=message_id, + notification_id=notification_id, + ) return True try: smtp = smtplib.SMTP(cfg.smtp_host, cfg.smtp_port, timeout=30) @@ -384,12 +406,78 @@ def _deliver( smtp.send_message(msg) finally: smtp.quit() + record_outbound( + to_address=to_address, + from_address=cfg.from_address, + subject=subject, + kind=kind, + status="sent", + message_id=message_id, + notification_id=notification_id, + ) return True - except Exception: + except Exception as exc: log.exception("email send failed: to=%s subject=%s", to_address, subject) + record_outbound( + to_address=to_address, + from_address=cfg.from_address, + subject=subject, + kind=kind, + status="failed", + error=f"{type(exc).__name__}: {exc}", + message_id=message_id, + notification_id=notification_id, + ) return False +def record_outbound( + *, + to_address: str, + from_address: str, + subject: str, + kind: str, + status: str, + error: str | None = None, + notification_id: int | None = None, + message_id: str | None = None, +) -> int | None: + """v0.18.0 Slice 4: write one row to `outbound_emails`. + + Returns the inserted row's id, or `None` if the DB connection + isn't initialized (which happens in unit tests that don't boot + the full app — the write is best-effort and never raises). + """ + try: + cur = db.conn().execute( + """ + INSERT INTO outbound_emails + (to_address, from_address, subject, kind, sent_at, status, + error, notification_id, message_id) + VALUES (?, ?, ?, ?, datetime('now'), ?, ?, ?, ?) + """, + ( + to_address, + from_address, + subject, + kind, + status, + error, + notification_id, + message_id, + ), + ) + return cur.lastrowid + except RuntimeError: + # db.conn() raises RuntimeError if init() hasn't been called. + # Pure-helper unit tests for build_envelope hit this path; the + # audit row is best-effort and not part of the contract. + return None + except Exception: + log.exception("outbound_emails write failed: to=%s subject=%s", to_address, subject) + return None + + # --------------------------------------------------------------------------- # Quiet-hours release pass — called from the digest job # --------------------------------------------------------------------------- @@ -512,6 +600,7 @@ def _send_bundle(cfg: EmailConfig, user: Any, emailable: list) -> int: body, unsubscribe_mailto=cfg.unsubscribe_mailto, unsubscribe_url=unsubscribe_url, + kind="bundle", ) if not sent: return 0 diff --git a/backend/app/email_invite.py b/backend/app/email_invite.py index 31a13dd..32ce6f9 100644 --- a/backend/app/email_invite.py +++ b/backend/app/email_invite.py @@ -33,7 +33,7 @@ import logging import smtplib from email.utils import formataddr -from .email import EmailConfig, _SENT +from .email import EmailConfig, _SENT, record_outbound from .email_envelope import build_envelope log = logging.getLogger(__name__) @@ -85,14 +85,23 @@ def send_invite_email( } _SENT.append(envelope) + message_id = msg["Message-ID"] if not cfg.enabled: log.info("invite email disabled (EMAIL_ENABLED=0): to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="deferred", message_id=message_id, + ) return True if not cfg.smtp_host: # Dev fallback: surface the claim URL at INFO so the operator can # complete a claim flow without an SMTP relay. In production # SMTP_HOST is always set per OHM's overlay. log.info("invite email (stdout fallback): to=%s claim_url=%s", to_address, claim_url) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="deferred", message_id=message_id, + ) return True try: @@ -105,9 +114,18 @@ def send_invite_email( smtp.send_message(msg) finally: smtp.quit() + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="sent", message_id=message_id, + ) return True - except Exception: + except Exception as exc: log.exception("invite email send failed: to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="failed", + error=f"{type(exc).__name__}: {exc}", message_id=message_id, + ) return False diff --git a/backend/app/email_otc.py b/backend/app/email_otc.py index 2d8b461..52ccfee 100644 --- a/backend/app/email_otc.py +++ b/backend/app/email_otc.py @@ -25,7 +25,7 @@ import logging import smtplib from email.utils import formataddr -from .email import EmailConfig, _SENT +from .email import EmailConfig, _SENT, record_outbound from .email_envelope import build_envelope log = logging.getLogger(__name__) @@ -66,14 +66,23 @@ def send_otc_email(to_address: str, code: str) -> bool: } _SENT.append(envelope) + message_id = msg["Message-ID"] if not cfg.enabled: log.info("otc email disabled (EMAIL_ENABLED=0): to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="deferred", message_id=message_id, + ) return True if not cfg.smtp_host: # Dev fallback: surface the code at INFO so the operator can # complete a sign-in flow without an SMTP relay. In production # SMTP_HOST is always set per OHM's overlay. log.info("otc email (stdout fallback): to=%s code=%s", to_address, code) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="deferred", message_id=message_id, + ) return True try: @@ -86,9 +95,18 @@ def send_otc_email(to_address: str, code: str) -> bool: smtp.send_message(msg) finally: smtp.quit() + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="sent", message_id=message_id, + ) return True - except Exception: + except Exception as exc: log.exception("otc email send failed: to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="failed", + error=f"{type(exc).__name__}: {exc}", message_id=message_id, + ) return False diff --git a/backend/migrations/020_outbound_emails.sql b/backend/migrations/020_outbound_emails.sql new file mode 100644 index 0000000..cb1783b --- /dev/null +++ b/backend/migrations/020_outbound_emails.sql @@ -0,0 +1,30 @@ +-- v0.18.0 Slice 4: outbound_emails audit table. +-- +-- Per the v0.18.0 email + webhook hygiene proposal §3, every send +-- helper writes a row to this table before returning, regardless +-- of outcome. status='sent' on success, 'failed' on exception, +-- 'deferred' on the dev-fallback path (no SMTP_HOST configured). +-- +-- The table is queried by `GET /api/admin/outbound-emails` to +-- answer "did this person ever get their invite?" without having +-- to grep VM logs, and by the v0.18.0 Slice 5 bounce-correlation +-- hook (which looks up message_id when a POST lands at +-- /api/webhooks/email-bounce and marks the matching row +-- status='bounced'). + +CREATE TABLE IF NOT EXISTS outbound_emails ( + id INTEGER PRIMARY KEY, + to_address TEXT NOT NULL, + from_address TEXT NOT NULL, + subject TEXT NOT NULL, + kind TEXT NOT NULL, -- 'otc' | 'invite' | 'notification' | 'bundle' | 'digest' | 'rfc-invite' + sent_at TEXT NOT NULL, -- ISO 8601, time the send was attempted + status TEXT NOT NULL, -- 'sent' | 'failed' | 'deferred' | 'bounced' + error TEXT, -- exception class + message if status='failed' + notification_id INTEGER, -- nullable FK to notifications.id for the watcher path + message_id TEXT -- the Message-ID header value, for bounce correlation +); + +CREATE INDEX IF NOT EXISTS idx_outbound_emails_to ON outbound_emails(to_address); +CREATE INDEX IF NOT EXISTS idx_outbound_emails_sent_at ON outbound_emails(sent_at); +CREATE INDEX IF NOT EXISTS idx_outbound_emails_message ON outbound_emails(message_id); diff --git a/backend/tests/test_outbound_emails_vertical.py b/backend/tests/test_outbound_emails_vertical.py new file mode 100644 index 0000000..51eebed --- /dev/null +++ b/backend/tests/test_outbound_emails_vertical.py @@ -0,0 +1,231 @@ +"""End-to-end integration tests for the v0.18.0 Slice 4 +outbound_emails audit table + admin endpoint. + +The release adds: + * `backend/migrations/020_outbound_emails.sql` — the audit table. + * `record_outbound()` in `email.py` — the write helper every send + path calls before returning, capturing status='sent' / 'failed' + / 'deferred' (the dev-fallback path when SMTP_HOST is unset). + * `GET /api/admin/outbound-emails` — admin-only listing, filterable + by kind / status / to_address. + +These tests prove: + * Sending OTC / invite / notification mail writes one row per send + (status='deferred' under tests since SMTP_HOST is unset). + * The Message-ID on the row matches the envelope's Message-ID + header (the seam Slice 5 uses for bounce correlation). + * `kind` is populated per send path. + * `GET /api/admin/outbound-emails` lists rows newest-first, + accepts filters, refuses non-admins. +""" +from __future__ import annotations + +from test_propose_vertical import ( # noqa: F401 + FakeGitea, + app_with_fake_gitea, + provision_user_row, + sign_in_as, + tmp_env, +) + + +# --------------------------------------------------------------------------- +# Write-on-send wiring +# --------------------------------------------------------------------------- + + +def test_otc_send_writes_outbound_row_with_message_id(app_with_fake_gitea): + from fastapi.testclient import TestClient + from app import db, email as email_mod + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + email_mod.reset_sent_envelopes() + r = client.post("/auth/otc/request", json={"email": "newcomer@ex.co"}) + assert r.status_code == 200 + + # Audit row landed. + rows = db.conn().execute( + "SELECT id, to_address, kind, status, message_id, error " + "FROM outbound_emails WHERE to_address = 'newcomer@ex.co'" + ).fetchall() + assert len(rows) == 1 + row = rows[0] + assert row["kind"] == "otc" + # No SMTP_HOST in tests -> 'deferred', not 'sent'. + assert row["status"] == "deferred" + assert row["error"] is None + # Message-ID matches the envelope's header (the seam Slice 5 uses). + envelopes = [e for e in email_mod.sent_envelopes() if e.get("kind") == "otc"] + assert envelopes + envelope_mid = envelopes[-1]["message"]["Message-ID"] + assert row["message_id"] == envelope_mid + + +def test_invite_send_writes_outbound_row(app_with_fake_gitea): + from fastapi.testclient import TestClient + from app import db + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=500, login="adminQ", role="admin") + sign_in_as( + client, user_id=500, gitea_login="adminQ", + display_name="Admin Q", role="admin", + email="adminq@test", + ) + r = client.post( + "/api/admin/users", + json={ + "email": "invitee@ex.co", + "first_name": "Inv", "last_name": "Itee", + "role": "contributor", "custom_message": "", + }, + ) + assert r.status_code == 200, r.text + + rows = db.conn().execute( + "SELECT kind, status, message_id FROM outbound_emails " + "WHERE to_address = 'invitee@ex.co'" + ).fetchall() + assert len(rows) == 1 + assert rows[0]["kind"] == "invite" + assert rows[0]["status"] == "deferred" + assert rows[0]["message_id"] is not None + + +def test_notification_send_writes_outbound_row_with_notification_id(app_with_fake_gitea): + """Watcher notifications carry a `notification_id` FK so the + admin can join through to the notifications table to see what + triggered the send.""" + from fastapi.testclient import TestClient + from app import db, email as email_mod + from test_notifications_vertical import PITCH + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="alice", role="contributor") + provision_user_row(user_id=1, login="ben", role="owner") + sign_in_as( + client, user_id=2, gitea_login="alice", + display_name="Alice", role="contributor", email="alice@test", + ) + r = client.post("/api/rfcs/propose", json={ + "title": "OHM", "slug": "ohm", "pitch": PITCH, "tags": [], + }) + email_mod.reset_sent_envelopes() + # Wipe pre-merge audit rows so the assertion below is unambiguous. + db.conn().execute("DELETE FROM outbound_emails") + sign_in_as( + client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner", email="ben@test", + ) + merge_r = client.post(f"/api/proposals/{r.json()['pr_number']}/merge") + assert merge_r.status_code == 200, merge_r.text + + rows = db.conn().execute( + "SELECT kind, status, notification_id, message_id " + "FROM outbound_emails WHERE to_address = 'alice@test'" + ).fetchall() + assert rows, "no outbound_emails row for alice@test" + # At least one notification kind, with a populated FK. + notif_rows = [r for r in rows if r["kind"] == "notification"] + assert notif_rows + for nr in notif_rows: + assert nr["status"] == "deferred" + assert nr["notification_id"] is not None + assert nr["message_id"] is not None + + +# --------------------------------------------------------------------------- +# Admin endpoint +# --------------------------------------------------------------------------- + + +def test_admin_outbound_emails_lists_rows(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + # Generate a few rows. + client.post("/auth/otc/request", json={"email": "one@ex.co"}) + + provision_user_row(user_id=600, login="adminR", role="admin") + sign_in_as( + client, user_id=600, gitea_login="adminR", + display_name="Admin R", role="admin", email="adminr@test", + ) + client.post("/api/admin/users", json={ + "email": "two@ex.co", "first_name": "T", "last_name": "Wo", + "role": "contributor", "custom_message": "", + }) + + r = client.get("/api/admin/outbound-emails") + assert r.status_code == 200, r.text + items = r.json()["items"] + kinds = {it["kind"] for it in items} + assert "otc" in kinds + assert "invite" in kinds + # Newest-first. + ids = [it["id"] for it in items] + assert ids == sorted(ids, reverse=True) + + +def test_admin_outbound_emails_filters_by_kind(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + client.post("/auth/otc/request", json={"email": "filter1@ex.co"}) + + provision_user_row(user_id=601, login="adminS", role="admin") + sign_in_as( + client, user_id=601, gitea_login="adminS", + display_name="Admin S", role="admin", email="admins@test", + ) + client.post("/api/admin/users", json={ + "email": "filter2@ex.co", "first_name": "F", "last_name": "Two", + "role": "contributor", "custom_message": "", + }) + + r = client.get("/api/admin/outbound-emails?kind=otc") + assert r.status_code == 200 + items = r.json()["items"] + assert items + assert all(it["kind"] == "otc" for it in items) + + +def test_admin_outbound_emails_filters_by_to_address(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + client.post("/auth/otc/request", json={"email": "TARGET@ex.co"}) + client.post("/auth/otc/request", json={"email": "other@ex.co"}) + + provision_user_row(user_id=602, login="adminT", role="admin") + sign_in_as( + client, user_id=602, gitea_login="adminT", + display_name="Admin T", role="admin", email="admint@test", + ) + + # to_address filter is case-insensitive. + r = client.get("/api/admin/outbound-emails?to_address=target@ex.co") + assert r.status_code == 200 + items = r.json()["items"] + assert items + assert all(it["to_address"].lower() == "target@ex.co" for it in items) + + +def test_admin_outbound_emails_refuses_non_admin(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=700, login="contribU", role="contributor") + sign_in_as( + client, user_id=700, gitea_login="contribU", + display_name="Contrib U", role="contributor", + ) + r = client.get("/api/admin/outbound-emails") + assert r.status_code == 403