Wire account verification end-to-end and fix an RFC 5322 mail bug (#1)
This commit is contained in:
parent
913563788b
commit
ac69fcea0f
4 changed files with 82 additions and 6 deletions
|
|
@ -8,7 +8,7 @@ import os
|
||||||
import secrets
|
import secrets
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
from fastapi import Depends
|
from fastapi import Depends, Request
|
||||||
from fastapi_users import BaseUserManager, FastAPIUsers, InvalidPasswordException, UUIDIDMixin
|
from fastapi_users import BaseUserManager, FastAPIUsers, InvalidPasswordException, UUIDIDMixin
|
||||||
from fastapi_users.authentication import AuthenticationBackend, CookieTransport
|
from fastapi_users.authentication import AuthenticationBackend, CookieTransport
|
||||||
from fastapi_users.authentication.strategy.db import DatabaseStrategy
|
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.db import get_async_session
|
||||||
from accounts.models import AccessToken, User
|
from accounts.models import AccessToken, User
|
||||||
|
from notifications import mailer
|
||||||
|
|
||||||
BASE = "/" + os.environ.get("THERMOGRAPH_BASE", "/thermograph").strip("/")
|
BASE = "/" + os.environ.get("THERMOGRAPH_BASE", "/thermograph").strip("/")
|
||||||
# Only used to sign reset/verification tokens — features that stay dormant until
|
# The public origin, for links in emailed tokens — same var + default indexnow.py
|
||||||
# email is wired up. A per-boot random secret is fine while they're unused.
|
# 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)
|
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
|
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".
|
# Guard against a scrypt/argon DoS from a megabyte "password".
|
||||||
raise InvalidPasswordException(reason="Password is too long.")
|
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)):
|
async def get_user_manager(user_db=Depends(get_user_db)):
|
||||||
yield UserManager(user_db)
|
yield UserManager(user_db)
|
||||||
|
|
|
||||||
|
|
@ -25,7 +25,7 @@ import os
|
||||||
import smtplib
|
import smtplib
|
||||||
import ssl
|
import ssl
|
||||||
from email.message import EmailMessage
|
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")
|
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["From"] = formataddr((name, addr)) if addr else MAIL_FROM
|
||||||
msg["To"] = to
|
msg["To"] = to
|
||||||
msg["Subject"] = subject
|
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:
|
if MAIL_REPLY_TO:
|
||||||
msg["Reply-To"] = MAIL_REPLY_TO
|
msg["Reply-To"] = MAIL_REPLY_TO
|
||||||
# Plain text is the body; HTML is the alternative, so text-only clients and
|
# Plain text is the body; HTML is the alternative, so text-only clients and
|
||||||
|
|
|
||||||
37
tests/notifications/test_mailer.py
Normal file
37
tests/notifications/test_mailer.py
Normal file
|
|
@ -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.", "<p>Body</p>")
|
||||||
|
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
|
||||||
|
|
@ -735,8 +735,9 @@ app.add_api_route(f"{BASE}/discord/interactions", discord_interactions,
|
||||||
|
|
||||||
# --- accounts (fastapi-users) ----------------------------------------------
|
# --- accounts (fastapi-users) ----------------------------------------------
|
||||||
# Library-provided routers: /auth/login + /auth/logout (cookie session),
|
# Library-provided routers: /auth/login + /auth/logout (cookie session),
|
||||||
# /auth/register (signup), and /users/me (session check / profile). All under the
|
# /auth/register (signup), /auth/verify + /auth/request-verify-token (email
|
||||||
# same v2 prefix as the rest of the API, so the frontend calls them base-relative.
|
# 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(
|
app.include_router(
|
||||||
users.fastapi_users.get_auth_router(users.auth_backend),
|
users.fastapi_users.get_auth_router(users.auth_backend),
|
||||||
prefix=f"{BASE}/api/v2/auth", tags=["auth"],
|
prefix=f"{BASE}/api/v2/auth", tags=["auth"],
|
||||||
|
|
@ -745,6 +746,10 @@ app.include_router(
|
||||||
users.fastapi_users.get_register_router(UserRead, UserCreate),
|
users.fastapi_users.get_register_router(UserRead, UserCreate),
|
||||||
prefix=f"{BASE}/api/v2/auth", tags=["auth"],
|
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(
|
app.include_router(
|
||||||
users.fastapi_users.get_users_router(UserRead, UserUpdate),
|
users.fastapi_users.get_users_router(UserRead, UserUpdate),
|
||||||
prefix=f"{BASE}/api/v2/users", tags=["users"],
|
prefix=f"{BASE}/api/v2/users", tags=["users"],
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue