From ac69fcea0f214ca80a632fcf06ecb3456a656e20 Mon Sep 17 00:00:00 2001 From: emi Date: Tue, 21 Jul 2026 03:57:20 +0000 Subject: [PATCH] Wire account verification end-to-end and fix an RFC 5322 mail bug (#1) --- accounts/users.py | 34 ++++++++++++++++++++++++--- notifications/mailer.py | 8 ++++++- tests/notifications/test_mailer.py | 37 ++++++++++++++++++++++++++++++ web/app.py | 9 ++++++-- 4 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 tests/notifications/test_mailer.py diff --git a/accounts/users.py b/accounts/users.py index 4531ef9..c713790 100644 --- a/accounts/users.py +++ b/accounts/users.py @@ -8,7 +8,7 @@ import os import secrets import uuid -from fastapi import Depends +from fastapi import Depends, Request from fastapi_users import BaseUserManager, FastAPIUsers, InvalidPasswordException, UUIDIDMixin from fastapi_users.authentication import AuthenticationBackend, CookieTransport from fastapi_users.authentication.strategy.db import DatabaseStrategy @@ -18,10 +18,16 @@ from sqlalchemy.ext.asyncio import AsyncSession from accounts.db import get_async_session from accounts.models import AccessToken, User +from notifications import mailer BASE = "/" + os.environ.get("THERMOGRAPH_BASE", "/thermograph").strip("/") -# Only used to sign reset/verification tokens — features that stay dormant until -# email is wired up. A per-boot random secret is fine while they're unused. +# The public origin, for links in emailed tokens — same var + default indexnow.py +# and discord.py already use for exactly this purpose (a link that has to resolve +# from wherever the recipient opens it, so it can't be relative to a request). +BASE_URL = os.environ.get("THERMOGRAPH_BASE_URL", "https://thermograph.org").rstrip("/") +# Signs verification tokens. THERMOGRAPH_AUTH_SECRET must be pinned before any +# such token is mailed — a per-boot random fallback would invalidate every +# outstanding link (and every session) on restart. SECRET = os.environ.get("THERMOGRAPH_AUTH_SECRET") or secrets.token_urlsafe(32) SESSION_TTL_SECONDS = int(os.environ.get("THERMOGRAPH_SESSION_TTL_DAYS", "30")) * 86400 @@ -54,6 +60,28 @@ class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]): # Guard against a scrypt/argon DoS from a megabyte "password". raise InvalidPasswordException(reason="Password is too long.") + async def on_after_register(self, user: User, request: "Request | None" = None) -> None: + # Kick off the same token-generation path a manual "resend verification" + # request would use, so there is exactly one place that builds the email + # (on_after_request_verify below) rather than two copies drifting apart. + await self.request_verify(user, request) + + async def on_after_request_verify( + self, user: User, token: str, request: "Request | None" = None + ) -> None: + # mailer.send() already no-ops on the console/disabled backends (dev, + # tests) — the enabled() check here just skips building the message. + if not mailer.enabled(): + return + link = f"{BASE_URL}{BASE}/?verify_token={token}" + mailer.send( + user.email, + "Confirm your Thermograph account", + "Welcome to Thermograph! Confirm your email address to finish setting up " + f"your account:\n\n{link}\n\n" + "If you didn't create this account, you can safely ignore this email.", + ) + async def get_user_manager(user_db=Depends(get_user_db)): yield UserManager(user_db) diff --git a/notifications/mailer.py b/notifications/mailer.py index f95e646..2194262 100644 --- a/notifications/mailer.py +++ b/notifications/mailer.py @@ -25,7 +25,7 @@ import os import smtplib import ssl from email.message import EmailMessage -from email.utils import formataddr, parseaddr +from email.utils import formataddr, formatdate, make_msgid, parseaddr log = logging.getLogger("thermograph.mail") @@ -56,6 +56,12 @@ def _build(to: str, subject: str, text: str, html: "str | None") -> EmailMessage msg["From"] = formataddr((name, addr)) if addr else MAIL_FROM msg["To"] = to msg["Subject"] = subject + # RFC 5322 requires both; without them Gmail rejects the message outright + # ("Messages missing a valid Message-ID header are not accepted") rather than + # just scoring it as spam. domain= keeps the Message-ID on our own sending + # domain rather than leaking the sending host's local/internal hostname. + msg["Message-ID"] = make_msgid(domain=addr.split("@", 1)[-1] if addr else None) + msg["Date"] = formatdate(localtime=True) if MAIL_REPLY_TO: msg["Reply-To"] = MAIL_REPLY_TO # Plain text is the body; HTML is the alternative, so text-only clients and diff --git a/tests/notifications/test_mailer.py b/tests/notifications/test_mailer.py new file mode 100644 index 0000000..7667f0b --- /dev/null +++ b/tests/notifications/test_mailer.py @@ -0,0 +1,37 @@ +"""Tests for mailer.py's message construction (no network — _build() only).""" +from notifications import mailer + + +def test_build_sets_message_id_and_date(): + """RFC 5322 requires both; Gmail rejects a message outright ("Messages + missing a valid Message-ID header are not accepted") rather than merely + spam-scoring it when either is absent.""" + msg = mailer._build("you@example.com", "Subject", "Body text.", None) + assert msg["Message-ID"] + assert msg["Message-ID"].startswith("<") and msg["Message-ID"].endswith(">") + assert msg["Date"] + + +def test_build_message_id_uses_the_sending_domain(): + """Not the sending host's own hostname (e.g. a VPS's provider-assigned + name) -- keeps the Message-ID consistent with the From/SPF/DKIM domain.""" + msg = mailer._build("you@example.com", "Subject", "Body text.", None) + assert "@thermograph.org>" in msg["Message-ID"] + + +def test_build_sets_from_to_subject(): + msg = mailer._build("you@example.com", "Hello", "Body text.", None) + assert msg["To"] == "you@example.com" + assert msg["Subject"] == "Hello" + assert "thermograph.org" in msg["From"] + + +def test_build_adds_html_alternative_when_given(): + msg = mailer._build("you@example.com", "Subject", "Body text.", "

Body

") + assert msg.get_body(preferencelist=("html",)) is not None + assert msg.get_body(preferencelist=("plain",)) is not None + + +def test_build_is_plain_text_only_without_html(): + msg = mailer._build("you@example.com", "Subject", "Body text.", None) + assert msg.get_body(preferencelist=("html",)) is None diff --git a/web/app.py b/web/app.py index acc3583..dcd7cd2 100644 --- a/web/app.py +++ b/web/app.py @@ -735,8 +735,9 @@ app.add_api_route(f"{BASE}/discord/interactions", discord_interactions, # --- accounts (fastapi-users) ---------------------------------------------- # Library-provided routers: /auth/login + /auth/logout (cookie session), -# /auth/register (signup), and /users/me (session check / profile). All under the -# same v2 prefix as the rest of the API, so the frontend calls them base-relative. +# /auth/register (signup), /auth/verify + /auth/request-verify-token (email +# confirmation), and /users/me (session check / profile). All under the same +# v2 prefix as the rest of the API, so the frontend calls them base-relative. app.include_router( users.fastapi_users.get_auth_router(users.auth_backend), prefix=f"{BASE}/api/v2/auth", tags=["auth"], @@ -745,6 +746,10 @@ app.include_router( users.fastapi_users.get_register_router(UserRead, UserCreate), prefix=f"{BASE}/api/v2/auth", tags=["auth"], ) +app.include_router( + users.fastapi_users.get_verify_router(UserRead), + prefix=f"{BASE}/api/v2/auth", tags=["auth"], +) app.include_router( users.fastapi_users.get_users_router(UserRead, UserUpdate), prefix=f"{BASE}/api/v2/users", tags=["users"],