{text.replace(chr(10) * 2, '
').strip()}
" + # Same logo header as the reset mail, so the test shows what users will get. + html = LOGO_HTML + f"{text.replace(chr(10) * 2, '
').strip()}
" try: await send_mail(cfg, body.to.strip(), "Ultimo Email test message", text, html) except MailerError as exc: diff --git a/backend/app/identity/assets/ultimo-logo.png b/backend/app/identity/assets/ultimo-logo.png new file mode 100644 index 0000000..0965a0e Binary files /dev/null and b/backend/app/identity/assets/ultimo-logo.png differ diff --git a/backend/app/identity/mailer.py b/backend/app/identity/mailer.py index a2a77b4..e1be7f0 100644 --- a/backend/app/identity/mailer.py +++ b/backend/app/identity/mailer.py @@ -15,6 +15,8 @@ import logging from dataclasses import dataclass from email.message import EmailMessage from email.utils import formataddr, make_msgid +from functools import cache +from pathlib import Path import aiosmtplib @@ -26,6 +28,22 @@ logger = logging.getLogger(__name__) SENDER_NAME = "Ultimo Email" _TIMEOUT = 20.0 +# The logo travels inside the message (multipart/related, referenced by cid:), +# not as a link: it shows even where remote images are blocked, and it is a PNG +# because mail clients (Gmail first) do not display SVG. Rendered at twice the +# size it is shown at, for sharp high-DPI screens. +_LOGO_FILE = Path(__file__).parent / "assets" / "ultimo-logo.png" +_LOGO_CID = "ultimo-logo@ultimo.email" +LOGO_HTML = ( + f'Someone asked to change the password of the Ultimo Email account {h}.
\n" "To choose a new password, open this link within 30 minutes:
\n" f'\n' diff --git a/backend/app/identity/routes.py b/backend/app/identity/routes.py index 2141558..00a8a30 100644 --- a/backend/app/identity/routes.py +++ b/backend/app/identity/routes.py @@ -5,6 +5,7 @@ from __future__ import annotations import logging import re +import uuid from typing import Annotated from fastapi import APIRouter, BackgroundTasks, Depends, Request, Response @@ -16,6 +17,7 @@ from app.config import get_settings from app.db import get_session from app.events import get_event_bus from app.identity import totp as totp_mod +from app.identity import trusted_devices from app.identity.cookies import ( clear_auth_cookies, new_csrf_token, @@ -133,6 +135,9 @@ async def register( @router.post("/login", response_model=UserOut) async def login(body: LoginRequest, request: Request, response: Response, db: DbSession) -> UserOut: + async def trusted(user_id: uuid.UUID) -> bool: + return await trusted_devices.is_trusted(request, response, user_id) + try: user = await authenticate( db, @@ -140,6 +145,7 @@ async def login(body: LoginRequest, request: Request, response: Response, db: Db password=body.password, ip=_client_ip(request), totp=body.totp, + trusted_device=trusted, ) except RateLimited as exc: response.headers["Retry-After"] = str(exc.retry_after) @@ -154,6 +160,10 @@ async def login(body: LoginRequest, request: Request, response: Response, db: Db ) if user is None: raise_api_error(401, "invalid_credentials", "incorrect handle or password") + if body.remember_device and body.totp and user.totp_secret is not None: + # Only after a code was actually checked: a trusted sign-in cannot extend + # the trust to anything new. + await trusted_devices.trust(request, response, user.id) await _establish_session(request, response, user) # Sync v2 §3/§5: a login marks the user active (session heat) and promotes # their accounts' tier to hot. @@ -208,6 +218,7 @@ async def totp_disable(body: TotpDisableRequest, user: CurrentUser, db: DbSessio raise_api_error(401, "invalid_credentials", "incorrect password") user.totp_secret = None await db.commit() + await trusted_devices.revoke_all(user.id) # trust was in place of a code that no longer exists return MessageOut() @@ -301,6 +312,9 @@ async def password_reset(body: PasswordResetIn, db: DbSession) -> MessageOut: user.password_hash = hash_password(body.password) await db.commit() await destroy_all_sessions(user.id) # force re-login everywhere after reset + # Whoever reset the password may be the owner taking the account back: no + # device keeps skipping the code on the old trust. + await trusted_devices.revoke_all(user.id) return MessageOut() diff --git a/backend/app/identity/schemas.py b/backend/app/identity/schemas.py index 4778af5..1fb5f96 100644 --- a/backend/app/identity/schemas.py +++ b/backend/app/identity/schemas.py @@ -19,6 +19,8 @@ class LoginRequest(BaseModel): handle: str = Field(min_length=1, max_length=320) password: str = Field(min_length=1, max_length=1024) totp: str | None = None # required when the account has 2FA enabled + # With a correct code: don't ask this browser for one again (trusted device). + remember_device: bool = False class TotpSetupResponse(BaseModel): diff --git a/backend/app/identity/service.py b/backend/app/identity/service.py index 69e26c0..65a9a21 100644 --- a/backend/app/identity/service.py +++ b/backend/app/identity/service.py @@ -2,6 +2,8 @@ from __future__ import annotations +import uuid +from collections.abc import Awaitable, Callable from dataclasses import dataclass from sqlalchemy import select @@ -152,12 +154,20 @@ async def _check_rate_limit(handle: str, ip: str) -> None: async def authenticate( - db: AsyncSession, *, handle_input: str, password: str, ip: str, totp: str | None = None + db: AsyncSession, + *, + handle_input: str, + password: str, + ip: str, + totp: str | None = None, + trusted_device: Callable[[uuid.UUID], Awaitable[bool]] | None = None, ) -> User | None: """Verify credentials (constant-time). Returns the user or None; may rate-limit. When the account has TOTP enabled, raises :class:`TotpRequired` (no code given) - or :class:`TotpInvalid` (wrong code) after the password check passes. + or :class:`TotpInvalid` (wrong code) after the password check passes — unless + ``trusted_device`` says this browser was trusted for the account at an earlier + 2FA sign-in, in which case the password is enough. """ handle = resolve_handle(handle_input) await _check_rate_limit(handle, ip) @@ -176,6 +186,8 @@ async def authenticate( from app.identity.totp import decrypt_totp_secret from app.identity.totp import verify as verify_totp + if not totp and trusted_device is not None and await trusted_device(user.id): + return user if not totp: raise TotpRequired if not verify_totp(decrypt_totp_secret(user.totp_secret), totp): diff --git a/backend/app/identity/trusted_devices.py b/backend/app/identity/trusted_devices.py new file mode 100644 index 0000000..f8adc9a --- /dev/null +++ b/backend/app/identity/trusted_devices.py @@ -0,0 +1,91 @@ +"""Trusted devices: "don't ask for the 2FA code on this device" (SPEC §6.1). + +Ticking the box at a successful 2FA sign-in stores a random token in Redis +(``trustdev:{token}`` -> user id) and hands it to the browser in its own +HttpOnly cookie. A later sign-in to the *same* account from that browser may +then skip the code — the password is still required. Other accounts signing in +on the same browser are unaffected. + +The trust lasts TRUST_DAYS and is renewed each time it is used, so a device in +regular use stays trusted while one left alone for that long has to show a code +again. A per-user index (``trustdev-user:{user_id}``) lets everything be revoked +at once: on a password reset, when 2FA is turned off, and when an administrator +blocks or deletes the account. Signing out does not revoke it — that is the +point of the box. +""" + +from __future__ import annotations + +import secrets +import uuid + +from fastapi import Request, Response + +from app.redis_client import aw, get_redis + +TRUST_DAYS = 90 +_TTL = TRUST_DAYS * 24 * 3600 +_PREFIX = "trustdev:" +_INDEX = "trustdev-user:" + +COOKIE_SECURE = "__Host-trusted-device" +COOKIE_INSECURE = "trusted_device" + + +def _cookie_name(request: Request) -> str: + # Same split as the session cookie: the __Host- prefix needs HTTPS. + return COOKIE_SECURE if request.url.scheme == "https" else COOKIE_INSECURE + + +def read_cookie(request: Request) -> str: + return request.cookies.get(COOKIE_SECURE) or request.cookies.get(COOKIE_INSECURE, "") + + +async def trust(request: Request, response: Response, user_id: uuid.UUID) -> None: + """Remember this browser for ``user_id`` and set the cookie.""" + token = secrets.token_urlsafe(32) + redis = get_redis() + await redis.set(_PREFIX + token, str(user_id), ex=_TTL) + await aw(redis.sadd(_INDEX + str(user_id), token)) + await redis.expire(_INDEX + str(user_id), _TTL) + response.set_cookie( + _cookie_name(request), + token, + max_age=_TTL, + httponly=True, + secure=request.url.scheme == "https", + samesite="lax", + path="/", + ) + + +async def is_trusted(request: Request, response: Response, user_id: uuid.UUID) -> bool: + """Whether this browser is trusted for ``user_id``; renews the trust if so.""" + token = read_cookie(request) + if not token: + return False + redis = get_redis() + if await redis.get(_PREFIX + token) != str(user_id): + return False + await redis.expire(_PREFIX + token, _TTL) + await redis.expire(_INDEX + str(user_id), _TTL) + response.set_cookie( + _cookie_name(request), + token, + max_age=_TTL, + httponly=True, + secure=request.url.scheme == "https", + samesite="lax", + path="/", + ) + return True + + +async def revoke_all(user_id: uuid.UUID) -> None: + """Forget every device trusted for ``user_id``.""" + redis = get_redis() + index = _INDEX + str(user_id) + tokens: set[str] = await aw(redis.smembers(index)) + if tokens: + await redis.delete(*(_PREFIX + t for t in tokens)) + await redis.delete(index) diff --git a/backend/app/identity/user_admin.py b/backend/app/identity/user_admin.py index 0f81c62..f832958 100644 --- a/backend/app/identity/user_admin.py +++ b/backend/app/identity/user_admin.py @@ -21,6 +21,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.accounts.service import delete_account from app.config import get_settings from app.events import get_event_bus +from app.identity import trusted_devices from app.identity.sessions import destroy_all_sessions from app.models import Account, Message, User from app.plugins.db import plugin_session @@ -79,6 +80,7 @@ async def _sign_out_everywhere(user_id: uuid.UUID) -> None: """Revoke every session, and tell the user's open tabs so they leave at once instead of showing already-loaded mail until their next request.""" await destroy_all_sessions(user_id) + await trusted_devices.revoke_all(user_id) await get_event_bus().publish("user.signed_out", {"user_id": str(user_id)}) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 7d2e1a9..e820bec 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -53,11 +53,12 @@ where = ["."] include = ["app*", "workers*"] # Non-Python files that a *non-editable* install (the Docker image) must carry: -# the account presets config (SPEC §6.2) and the shared Alembic env used by the -# per-plugin migration runner (app/plugins/migrate.py). Without these, a -# `pip install ./backend` produces a package that fails at runtime. +# the account presets config (SPEC §6.2), the shared Alembic env used by the +# per-plugin migration runner (app/plugins/migrate.py) and the logo embedded in +# system mail (app/identity/mailer.py). Without these, a `pip install ./backend` +# produces a package that fails at runtime. [tool.setuptools.package-data] -app = ["accounts/presets.yaml", "plugins/alembic/*"] +app = ["accounts/presets.yaml", "plugins/alembic/*", "identity/assets/*.png"] [tool.ruff] line-length = 100 diff --git a/backend/tests/identity/test_password_reset.py b/backend/tests/identity/test_password_reset.py index ded426a..a3d6f57 100644 --- a/backend/tests/identity/test_password_reset.py +++ b/backend/tests/identity/test_password_reset.py @@ -307,3 +307,26 @@ async def test_admin_configures_system_mail_without_ever_seeing_the_password( test = await c.post("/api/admin/mail/test", json={"to": "me@example.com"}) assert test.status_code == 200 assert ctx.sent.mails[-1][:2] == ("me@example.com", "Ultimo Email test message") + + +async def test_reset_mail_carries_the_logo_inline(monkeypatch: pytest.MonkeyPatch) -> None: + """The logo is a PNG inside the message (cid:), not a remote image or SVG.""" + import aiosmtplib + + from app.identity import mailer + + sent: list[object] = [] + + async def fake_send(msg: object, **kwargs: object) -> None: + sent.append(msg) + + monkeypatch.setattr(aiosmtplib, "send", fake_send) + subject, text, html = mailer.reset_mail("https://mail.example.com/x", "a@test.local") + await mailer.send_mail(CFG, "to@example.com", subject, text, html) + msg = sent[0] + parts = {p.get_content_type(): p for p in msg.walk()} # type: ignore[attr-defined] + assert {"multipart/alternative", "text/plain", "multipart/related", "text/html"} <= set(parts) + logo = parts["image/png"] + assert logo["Content-ID"] == "{t("auth.totpPrompt")}
+ )}