From 165442d897b914f0890d0fae77ea242453765c56 Mon Sep 17 00:00:00 2001 From: Lukas Remis Date: Thu, 1 Oct 2026 21:02:44 +0200 Subject: [PATCH] Logo in system mail; "don't ask for the 2FA code on this device" System mail: the password-reset email (and the admin panel's test email) now opens with the Ultimo logo, centered. It is a PNG rendered from icons/ultimo-logo.svg and embedded in the message (multipart/related, cid:), so it shows where remote images are blocked and in clients that do not render SVG. The PNG is listed in package-data so the Docker image carries it. Trusted devices: a 2FA sign-in offers "Don't ask for the code on this device in future". Ticked with a correct code, the browser gets its own HttpOnly trusted-device cookie (token in Redis, 90 days, renewed on use); later sign-ins to the same account from that browser need only the password. Other accounts and browsers are unaffected. The trust is revoked on a password reset, when 2FA is turned off, and when an admin blocks or deletes the user; signing out keeps it. Co-Authored-By: Claude Opus 5.5 --- backend/app/api/admin.py | 5 +- backend/app/identity/assets/ultimo-logo.png | Bin 0 -> 5024 bytes backend/app/identity/mailer.py | 24 ++- backend/app/identity/routes.py | 14 ++ backend/app/identity/schemas.py | 2 + backend/app/identity/service.py | 16 +- backend/app/identity/trusted_devices.py | 91 ++++++++++++ backend/app/identity/user_admin.py | 2 + backend/pyproject.toml | 9 +- backend/tests/identity/test_password_reset.py | 23 +++ .../tests/identity/test_trusted_devices.py | 137 ++++++++++++++++++ frontend/src/i18n/en.ts | 1 + frontend/src/lib/api.ts | 10 +- frontend/src/routes/Login.tsx | 12 +- 14 files changed, 334 insertions(+), 12 deletions(-) create mode 100644 backend/app/identity/assets/ultimo-logo.png create mode 100644 backend/app/identity/trusted_devices.py create mode 100644 backend/tests/identity/test_trusted_devices.py diff --git a/backend/app/api/admin.py b/backend/app/api/admin.py index 0875763..7e143ac 100644 --- a/backend/app/api/admin.py +++ b/backend/app/api/admin.py @@ -20,7 +20,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.api.errors import raise_api_error from app.db import get_session from app.identity.deps import CurrentAdmin, require_csrf -from app.identity.mailer import MailerError, mailer_config, send_mail +from app.identity.mailer import LOGO_HTML, MailerError, mailer_config, send_mail from app.identity.schemas import ( AdminMailIn, AdminMailOut, @@ -131,7 +131,8 @@ async def test_mail(body: AdminMailTestIn, admin: CurrentAdmin, db: DbSession) - "This is a test message from Ultimo Email.\n\n" "Outgoing system mail works: password-reset links will be sent from this account.\n" ) - html = f"

{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 0000000000000000000000000000000000000000..0965a0e659f7fe62e120c23518ea9ed9e52adb7b GIT binary patch literal 5024 zcmZ{IXEYp8xAquq!f4UEAM46M}>wT67Zxi4rw>Z&5OO2_Z^E zH~J{S^?u)4_s_le$3FX%z0O)^t+V&DpXZICz9tz76A1tSAk)@@J_P{qkau%S5YgR8 zUY#(0=k9rE!Mys*Nk%?``TlEwTJFl+Bkq9QM`)$+XPXRs=FC9$h+0kr|8#|1zOrD zI@MUthlI~TkhiJ-^T5Qx%P43Dvbg*i1@@JQP{6aK_)DR3(%81M@I*ADE#g>^|8sK> zIp?PsF%Kq&E1O~VW&_559YZ~lhvV*F#Yl9aZns}3LM%@+#XG)z7{AteF$}j&I7r3u zA3`9&zh%lL+vIYfF>2)*0D!<1N5@G8h}RAc-BdF4pTq8ckj&x@1_LI{Z3bDpohJQ<;D*6u45}D8;4Oh`aFe7H5Hx$WuwHK53J}t zhb#1ZMkH1Kz9;<}?;qXo6jljYPOCbi;zY{cdX+e^1aZgi6Ia2~VJ(q9g>u(C(sG42 zqI5`hY_jy|_Kj^BHB*<2*zjipMAgKM_8{XTM*qbP=1BDqJ(?PiZDj-aZcZ0KC_JWG z=P&zCrNwtKD00ilP8hR5iMmaylsh3ho_hQJPbl?Jk35Hq=Z$nX+uP|^>5NJj@PIe> zAoI25gw0!(Jvff1iFmPBWSFSz6!GRb8vkg=ygH|ZZrP>R$^lUZ+7V?lFwYuPBBV0xBL7u+KeC@-EV% zN3SVaH5U|+4*Z5}svt%jvOT5$`3{h>ckgNmF zEzdp2?TLQ%=UR<_e8Z3> z`d#}2*X+ZyS+AqgO0TqNY*WAL-&ktk^OHx@X->Id zBBqg7(Hj~zy-zC0MnXO~YEwLqWue-X4pW?PUn}-w1}{fYKIJEBY4X4fulIWNqputH zf4c8Qp2^~8_0!kPj%2l?{#yN%E-yX(AOc>1I(WvCJ6Yk@g%feXRFTn9?{%pdvxc>Q z#>kL}-eG8)@}G zQuV4g-@NAvms#8k_S#foX2~qaEkZUZhOu^fq3G!k5R9iKa_3z1FpCz;@X%>qNY%&8 zr!l`aC9-mknRoGBvG<=Mr9{U?SI*SFXu}(W!Xd)^YbP|plFsU{GCNgq&!w7;Tzb9plXLk0igs&981CN+O-I6Xuj`;uy zr@mAd0^dxzkGb|*Vhx!_YPF5J=sj%y<^sSmqn^6LF@8>R=n?=;JHt=O(aQeDB?TDU zz9z?{tIQ9>!kp~%U%BCe84CH@M#5CjC#R00oxVbIHB&+7GYR3-^rxpN3WuiaGHH}Y z?r0SEgTK>DOg=)Jp`X|M9$68i7fixXw_@L~X+&3A~RVLu9vuyww~n{ zyayo~0g?5UK5pihp300bSNw}fI7rV(At7--Y(AB%4MJgtDUWH(+rP^aSL9W#`F-t- zt38xQW;*ah9q?RHYHGRPzf5biANPK~#a6!6Cx~+lkXZ85-%ZTw_fR}zsMqGr1VQGj zchy+9t?oxlSVKIACtXA>?_TGs*e|=CS@!sUI zq~u(b)v0PJmR|TtB{>%>1bUTbPp9cBId_}mzEwr6DdT;U}23O#sov` zs73anjN#mVL8!csh>U6Fl-J2xlP*RZLQ%X5KZ!FW^TZc2Xg|<#+`jVEB)_0ID*XyK zFLf-$Sb>a!YUq$Q&#mi-+0EOsg%obTtI9(m9>u%Qk@k#r>fBqh!?nLv?s^FU7nH+0 z9x{J12q`FRI2TkDog2>K#N^f(d0zAOXDw;FK^A4fiT$H7%&siE+9ZE%;@MQgMK>ui z&9ZE_8Z_tQ))qhi%gohJJGR7&%XyzDcD@p|CfI!qllsdj?GK#_ zIjiELg*yV}!RYI6HyppegZrl^g6UNM4C6*fhQ>Th16Y+yBKE_DP#E8o&vam>>YUV| zH}zo@oxxd~ZAVdysn1r}#2&PN44l<}BZJ-J!&2HjCP42qNE9^K?AF_>y=sj+I&+vy z#Jpg!Q-(kX{3ONJX218z+@EGqRnVpD`>0q!ZD1P&VZzTy#H z?9cYHwyjQiiQ+L%+AxQ2Y$}w6?n4&6T5;xIZT%1G}ST|6_W0oyx)QGLdEV=|884MDe%d?MR1ak#UjJ z3xG{1emEf$7(5cnEOf~eVM;1N9d3ItyXQefP7xyQzj#Cg;2UF&e`b0{W-s6I!Iu-^ zL&N?+Pbtnk-uEYFFqCcpCpqGZsWS{m1}TkRj6V!d((KTd&<&kxeAMA$qU;$@>0N}Q z=RBO7s#iW+HCfGC%l5nGSkV4U$fUf{&u2wttb}|{?fX7{6M^)Nyl(gmJ9Zktu4>ID zxH<8e-{YXRCohP!bs(z8Jq*epn8+&ca15kH0;L$g+P!3XF(&aLjgb;Qom+lJ_04TR&3NR2mZ!cLsERYPaZlHhj>&%8nd~!{F ztc)yNORdbk>*R@Jm(@N+!Sy%C+X^>Hd@_}R(?~S8dSEyt=X^F1aL&6euX>b*kL!A& z4=z&12hi!e>iEFrD0OcMO}98VSE9X z@S_5lRX!0+Z^|hO6+2*kW96MG#DYK(MO+4r!ezGTrn@y!Tdzxlo4eSM4yLoRJiAi^ zmJ&|B=)E(A!)PaWl+1j14kHseLaj(Pmi&iPh95iraUq5aF*>ibU>j7)Lu`(2cq^QF zk`Al>8$e`%U;HjmsLB{TKyA2P)eEB@px>r-a|~My0H7#7{t3C|DGmkBL>BB0lDA_=KsP1C7zYB=aRtTuly*54NgAq^9i!CtoTYM z-z_j^?hF4n;Cq&QI4eW9H{2-+sKH#vnNdgF&g4BFf{T4jtUWoie{Vk_^U!|5ii~K@ z5Aa^FJvHOm6^Nk(FDcg=`rv+|fz{5_R(dKO?{I@|9qyBOY2Dyq?5B_Po8fSutLWb3 zL+adzYZMLz)C>}|eaDYpisO?ZUaWdtc}6PuA}`RB$@v3nb8ZT5 z(t+r{0O;daJ^Q$CCF*|^1Snj*;o^y~p{EMqz*ka--bUZmB?;XYiqL_n7Evq>y|78L+02&F~gncJ09%HdxWrqCc)4fmqhF$c{mD~?4{f7=n ztyC1Qwe>fsC?0Zb?EO4?IV%MlMCaM;?oj6vEs+VZi{z*5bVvgZ=19(P6`ndncx-?J z!2jpL)YwX~FRujkP)h=uY%Au3mK!Wh8(ADY_YI@Uj7cC-NVS#zJFQ9eob&%unf@2~ b=~~Gsik+)ee*OEMx(3iz*N0ZBB3}I$J{)b@ literal 0 HcmV?d00001 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'

Ultimo Email

' +) + + +@cache +def _logo() -> bytes: + return _LOGO_FILE.read_bytes() + class MailerError(Exception): """System mail is not configured, or the SMTP server refused the message.""" @@ -67,6 +85,10 @@ async def send_mail(cfg: MailerConfig, to: str, subject: str, text: str, html: s msg["Auto-Submitted"] = "auto-generated" # RFC 3834: do not auto-reply to this msg.set_content(text) msg.add_alternative(html, subtype="html") + if f"cid:{_LOGO_CID}" in html: + html_part = msg.get_body(("html",)) + if isinstance(html_part, EmailMessage): + html_part.add_related(_logo(), maintype="image", subtype="png", cid=f"<{_LOGO_CID}>") try: await aiosmtplib.send( msg, @@ -99,7 +121,7 @@ def reset_mail(link: str, handle: str) -> tuple[str, str, str]: ) body = "font-family:Arial,Helvetica,sans-serif;font-size:15px;color:#1f2937;line-height:1.5" html = ( - f'\n\n' + f'\n\n{LOGO_HTML}\n' 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'

Change password

\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"] == "" + assert logo.get_payload(decode=True).startswith(b"\x89PNG") + assert 'src="cid:ultimo-logo@ultimo.email"' in parts["text/html"].get_content() diff --git a/backend/tests/identity/test_trusted_devices.py b/backend/tests/identity/test_trusted_devices.py new file mode 100644 index 0000000..aa87c70 --- /dev/null +++ b/backend/tests/identity/test_trusted_devices.py @@ -0,0 +1,137 @@ +"""Trusted devices: "don't ask for the code on this device" at a 2FA sign-in.""" + +from __future__ import annotations + +import uuid +from collections.abc import AsyncIterator +from dataclasses import dataclass, field + +import httpx +import pyotp +import pytest_asyncio +from httpx import ASGITransport +from sqlalchemy import delete +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from sqlalchemy.pool import NullPool + +from app.config import get_settings +from app.identity import trusted_devices +from app.identity.passwords import hash_password +from app.identity.totp import encrypt_totp_secret +from app.main import create_app +from app.models import User + +PASSWORD = "hunter2pass" + + +@dataclass +class Ctx: + session: AsyncSession + created: list[uuid.UUID] = field(default_factory=list) + + async def user(self) -> tuple[User, str]: + secret = pyotp.random_base32() + user = User( + handle=f"td{uuid.uuid4().hex[:8]}@test.local", + password_hash=hash_password(PASSWORD), + totp_secret=encrypt_totp_secret(secret), + ) + self.session.add(user) + await self.session.commit() + self.created.append(user.id) + return user, secret + + +def _browser() -> httpx.AsyncClient: + """One browser: keeps its cookies between requests.""" + return httpx.AsyncClient(transport=ASGITransport(app=create_app()), base_url="http://t") + + +@pytest_asyncio.fixture +async def ctx() -> AsyncIterator[Ctx]: + engine = create_async_engine(get_settings().database_url, poolclass=NullPool) + session = async_sessionmaker(engine, expire_on_commit=False)() + c = Ctx(session=session) + try: + yield c + finally: + for uid in c.created: + await trusted_devices.revoke_all(uid) + await session.execute(delete(User).where(User.id.in_(c.created))) + await session.commit() + await session.close() + await engine.dispose() + + +async def _login( + browser: httpx.AsyncClient, user: User, totp: str | None = None, remember: bool = False +) -> httpx.Response: + body: dict[str, object] = {"handle": user.handle, "password": PASSWORD} + if totp: + body["totp"] = totp + if remember: + body["remember_device"] = True + return await browser.post("/api/auth/login", json=body) + + +async def test_a_remembered_browser_signs_in_without_the_code(ctx: Ctx) -> None: + user, secret = await ctx.user() + async with _browser() as browser: + first = await _login(browser, user, pyotp.TOTP(secret).now(), remember=True) + assert first.status_code == 200 + assert "trusted_device" in browser.cookies + again = await _login(browser, user) + assert again.status_code == 200 + # The password is still checked. + wrong = await browser.post( + "/api/auth/login", json={"handle": user.handle, "password": "not-it-at-all"} + ) + assert wrong.json()["error"]["code"] == "invalid_credentials" + + +async def test_without_the_box_the_code_is_asked_every_time(ctx: Ctx) -> None: + user, secret = await ctx.user() + async with _browser() as browser: + assert (await _login(browser, user, pyotp.TOTP(secret).now())).status_code == 200 + assert "trusted_device" not in browser.cookies + assert (await _login(browser, user)).json()["error"]["code"] == "totp_required" + + +async def test_trust_is_per_account_and_per_browser(ctx: Ctx) -> None: + alice, alice_secret = await ctx.user() + bob, _ = await ctx.user() + async with _browser() as browser: + await _login(browser, alice, pyotp.TOTP(alice_secret).now(), remember=True) + # Another account on the same browser still needs its own code. + assert (await _login(browser, bob)).json()["error"]["code"] == "totp_required" + async with _browser() as other_browser: + assert (await _login(other_browser, alice)).json()["error"]["code"] == "totp_required" + + +async def test_turning_2fa_off_revokes_the_trust(ctx: Ctx) -> None: + user, secret = await ctx.user() + async with _browser() as browser: + await _login(browser, user, pyotp.TOTP(secret).now(), remember=True) + token = browser.cookies["trusted_device"] + await trusted_devices.revoke_all(user.id) # what totp/disable, reset and block call + assert (await _login(browser, user)).json()["error"]["code"] == "totp_required" + assert browser.cookies.get("trusted_device") == token # stale cookie, no longer honoured + + +async def test_totp_disable_endpoint_revokes(ctx: Ctx) -> None: + user, secret = await ctx.user() + async with _browser() as browser: + await _login(browser, user, pyotp.TOTP(secret).now(), remember=True) + old_token = browser.cookies["trusted_device"] + csrf = browser.cookies["csrf"] + resp = await browser.post( + "/api/auth/totp/disable", json={"password": PASSWORD}, headers={"X-CSRF": csrf} + ) + assert resp.status_code == 200 + # Turn 2FA back on: the old trust must not come back with it. + user.totp_secret = encrypt_totp_secret(pyotp.random_base32()) + ctx.session.add(user) + await ctx.session.commit() + async with _browser() as browser: + browser.cookies.set("trusted_device", old_token) + assert (await _login(browser, user)).json()["error"]["code"] == "totp_required" diff --git a/frontend/src/i18n/en.ts b/frontend/src/i18n/en.ts index 0f48d83..d835f5f 100644 --- a/frontend/src/i18n/en.ts +++ b/frontend/src/i18n/en.ts @@ -29,6 +29,7 @@ export const en = { "auth.passwordHint": "at least 8 characters", "auth.totpCode": "2FA code", "auth.totpPrompt": "Enter the 6-digit code from your authenticator app", + "auth.rememberDevice": "Don't ask for the code on this device in future", "auth.signIn": "Sign in", "auth.signingIn": "Signing in…", "auth.create": "Create account", diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 0c36fe1..d9351a3 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -123,10 +123,16 @@ export const api = { method: "POST", body: JSON.stringify({ localpart, password }), }), - login: (handle: string, password: string, totp?: string) => + // rememberDevice: with a correct 2FA code, don't ask this browser for one again. + login: (handle: string, password: string, totp?: string, rememberDevice = false) => apiFetch("/auth/login", { method: "POST", - body: JSON.stringify({ handle, password, totp: totp || undefined }), + body: JSON.stringify({ + handle, + password, + totp: totp || undefined, + remember_device: rememberDevice || undefined, + }), }), logout: () => apiFetch<{ status: string }>("/auth/logout", { method: "POST" }), me: () => apiFetch("/auth/me"), diff --git a/frontend/src/routes/Login.tsx b/frontend/src/routes/Login.tsx index d315a87..949740b 100644 --- a/frontend/src/routes/Login.tsx +++ b/frontend/src/routes/Login.tsx @@ -11,13 +11,14 @@ export function Login() { const [password, setPassword] = useState(""); const [totp, setTotp] = useState(""); const [needTotp, setNeedTotp] = useState(false); + const [rememberDevice, setRememberDevice] = useState(false); const navigate = useNavigate(); const queryClient = useQueryClient(); // Only offer the signup link when the server would actually accept one. const registration = useQuery({ queryKey: ["registration"], queryFn: api.registrationStatus }); const mutation = useMutation({ - mutationFn: () => api.login(handle, password, needTotp ? totp : undefined), + mutationFn: () => api.login(handle, password, needTotp ? totp : undefined, needTotp && rememberDevice), onSuccess: (user) => { queryClient.setQueryData(["me"], user); navigate("/mail"); @@ -71,6 +72,15 @@ export function Login() { autoFocus />

{t("auth.totpPrompt")}

+ )}