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 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
|||||||
from app.api.errors import raise_api_error
|
from app.api.errors import raise_api_error
|
||||||
from app.db import get_session
|
from app.db import get_session
|
||||||
from app.identity.deps import CurrentAdmin, require_csrf
|
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 (
|
from app.identity.schemas import (
|
||||||
AdminMailIn,
|
AdminMailIn,
|
||||||
AdminMailOut,
|
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"
|
"This is a test message from Ultimo Email.\n\n"
|
||||||
"Outgoing system mail works: password-reset links will be sent from this account.\n"
|
"Outgoing system mail works: password-reset links will be sent from this account.\n"
|
||||||
)
|
)
|
||||||
html = f"<p>{text.replace(chr(10) * 2, '</p><p>').strip()}</p>"
|
# Same logo header as the reset mail, so the test shows what users will get.
|
||||||
|
html = LOGO_HTML + f"<p>{text.replace(chr(10) * 2, '</p><p>').strip()}</p>"
|
||||||
try:
|
try:
|
||||||
await send_mail(cfg, body.to.strip(), "Ultimo Email test message", text, html)
|
await send_mail(cfg, body.to.strip(), "Ultimo Email test message", text, html)
|
||||||
except MailerError as exc:
|
except MailerError as exc:
|
||||||
|
|||||||
Binary file not shown.
|
After Width: | Height: | Size: 4.9 KiB |
@@ -15,6 +15,8 @@ import logging
|
|||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from email.message import EmailMessage
|
from email.message import EmailMessage
|
||||||
from email.utils import formataddr, make_msgid
|
from email.utils import formataddr, make_msgid
|
||||||
|
from functools import cache
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
import aiosmtplib
|
import aiosmtplib
|
||||||
|
|
||||||
@@ -26,6 +28,22 @@ logger = logging.getLogger(__name__)
|
|||||||
SENDER_NAME = "Ultimo Email"
|
SENDER_NAME = "Ultimo Email"
|
||||||
_TIMEOUT = 20.0
|
_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'<p style="text-align:center;margin:0 0 24px"><img src="cid:{_LOGO_CID}" '
|
||||||
|
'width="180" height="47" alt="Ultimo Email" style="display:inline-block;border:0"></p>'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@cache
|
||||||
|
def _logo() -> bytes:
|
||||||
|
return _LOGO_FILE.read_bytes()
|
||||||
|
|
||||||
|
|
||||||
class MailerError(Exception):
|
class MailerError(Exception):
|
||||||
"""System mail is not configured, or the SMTP server refused the message."""
|
"""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["Auto-Submitted"] = "auto-generated" # RFC 3834: do not auto-reply to this
|
||||||
msg.set_content(text)
|
msg.set_content(text)
|
||||||
msg.add_alternative(html, subtype="html")
|
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:
|
try:
|
||||||
await aiosmtplib.send(
|
await aiosmtplib.send(
|
||||||
msg,
|
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"
|
body = "font-family:Arial,Helvetica,sans-serif;font-size:15px;color:#1f2937;line-height:1.5"
|
||||||
html = (
|
html = (
|
||||||
f'<!doctype html>\n<html><body style="{body}">\n'
|
f'<!doctype html>\n<html><body style="{body}">\n{LOGO_HTML}\n'
|
||||||
f"<p>Someone asked to change the password of the Ultimo Email account <b>{h}</b>.</p>\n"
|
f"<p>Someone asked to change the password of the Ultimo Email account <b>{h}</b>.</p>\n"
|
||||||
"<p>To choose a new password, open this link within 30 minutes:</p>\n"
|
"<p>To choose a new password, open this link within 30 minutes:</p>\n"
|
||||||
f'<p><a href="{u}" style="{button}">Change password</a></p>\n'
|
f'<p><a href="{u}" style="{button}">Change password</a></p>\n'
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
|
import uuid
|
||||||
from typing import Annotated
|
from typing import Annotated
|
||||||
|
|
||||||
from fastapi import APIRouter, BackgroundTasks, Depends, Request, Response
|
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.db import get_session
|
||||||
from app.events import get_event_bus
|
from app.events import get_event_bus
|
||||||
from app.identity import totp as totp_mod
|
from app.identity import totp as totp_mod
|
||||||
|
from app.identity import trusted_devices
|
||||||
from app.identity.cookies import (
|
from app.identity.cookies import (
|
||||||
clear_auth_cookies,
|
clear_auth_cookies,
|
||||||
new_csrf_token,
|
new_csrf_token,
|
||||||
@@ -133,6 +135,9 @@ async def register(
|
|||||||
|
|
||||||
@router.post("/login", response_model=UserOut)
|
@router.post("/login", response_model=UserOut)
|
||||||
async def login(body: LoginRequest, request: Request, response: Response, db: DbSession) -> 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:
|
try:
|
||||||
user = await authenticate(
|
user = await authenticate(
|
||||||
db,
|
db,
|
||||||
@@ -140,6 +145,7 @@ async def login(body: LoginRequest, request: Request, response: Response, db: Db
|
|||||||
password=body.password,
|
password=body.password,
|
||||||
ip=_client_ip(request),
|
ip=_client_ip(request),
|
||||||
totp=body.totp,
|
totp=body.totp,
|
||||||
|
trusted_device=trusted,
|
||||||
)
|
)
|
||||||
except RateLimited as exc:
|
except RateLimited as exc:
|
||||||
response.headers["Retry-After"] = str(exc.retry_after)
|
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:
|
if user is None:
|
||||||
raise_api_error(401, "invalid_credentials", "incorrect handle or password")
|
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)
|
await _establish_session(request, response, user)
|
||||||
# Sync v2 §3/§5: a login marks the user active (session heat) and promotes
|
# Sync v2 §3/§5: a login marks the user active (session heat) and promotes
|
||||||
# their accounts' tier to hot.
|
# 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")
|
raise_api_error(401, "invalid_credentials", "incorrect password")
|
||||||
user.totp_secret = None
|
user.totp_secret = None
|
||||||
await db.commit()
|
await db.commit()
|
||||||
|
await trusted_devices.revoke_all(user.id) # trust was in place of a code that no longer exists
|
||||||
return MessageOut()
|
return MessageOut()
|
||||||
|
|
||||||
|
|
||||||
@@ -301,6 +312,9 @@ async def password_reset(body: PasswordResetIn, db: DbSession) -> MessageOut:
|
|||||||
user.password_hash = hash_password(body.password)
|
user.password_hash = hash_password(body.password)
|
||||||
await db.commit()
|
await db.commit()
|
||||||
await destroy_all_sessions(user.id) # force re-login everywhere after reset
|
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()
|
return MessageOut()
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ class LoginRequest(BaseModel):
|
|||||||
handle: str = Field(min_length=1, max_length=320)
|
handle: str = Field(min_length=1, max_length=320)
|
||||||
password: str = Field(min_length=1, max_length=1024)
|
password: str = Field(min_length=1, max_length=1024)
|
||||||
totp: str | None = None # required when the account has 2FA enabled
|
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):
|
class TotpSetupResponse(BaseModel):
|
||||||
|
|||||||
@@ -2,6 +2,8 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import uuid
|
||||||
|
from collections.abc import Awaitable, Callable
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
@@ -152,12 +154,20 @@ async def _check_rate_limit(handle: str, ip: str) -> None:
|
|||||||
|
|
||||||
|
|
||||||
async def authenticate(
|
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:
|
) -> User | None:
|
||||||
"""Verify credentials (constant-time). Returns the user or None; may rate-limit.
|
"""Verify credentials (constant-time). Returns the user or None; may rate-limit.
|
||||||
|
|
||||||
When the account has TOTP enabled, raises :class:`TotpRequired` (no code given)
|
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)
|
handle = resolve_handle(handle_input)
|
||||||
await _check_rate_limit(handle, ip)
|
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 decrypt_totp_secret
|
||||||
from app.identity.totp import verify as verify_totp
|
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:
|
if not totp:
|
||||||
raise TotpRequired
|
raise TotpRequired
|
||||||
if not verify_totp(decrypt_totp_secret(user.totp_secret), totp):
|
if not verify_totp(decrypt_totp_secret(user.totp_secret), totp):
|
||||||
|
|||||||
@@ -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)
|
||||||
@@ -21,6 +21,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
|||||||
from app.accounts.service import delete_account
|
from app.accounts.service import delete_account
|
||||||
from app.config import get_settings
|
from app.config import get_settings
|
||||||
from app.events import get_event_bus
|
from app.events import get_event_bus
|
||||||
|
from app.identity import trusted_devices
|
||||||
from app.identity.sessions import destroy_all_sessions
|
from app.identity.sessions import destroy_all_sessions
|
||||||
from app.models import Account, Message, User
|
from app.models import Account, Message, User
|
||||||
from app.plugins.db import plugin_session
|
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
|
"""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."""
|
instead of showing already-loaded mail until their next request."""
|
||||||
await destroy_all_sessions(user_id)
|
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)})
|
await get_event_bus().publish("user.signed_out", {"user_id": str(user_id)})
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -53,11 +53,12 @@ where = ["."]
|
|||||||
include = ["app*", "workers*"]
|
include = ["app*", "workers*"]
|
||||||
|
|
||||||
# Non-Python files that a *non-editable* install (the Docker image) must carry:
|
# 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
|
# the account presets config (SPEC §6.2), the shared Alembic env used by the
|
||||||
# per-plugin migration runner (app/plugins/migrate.py). Without these, a
|
# per-plugin migration runner (app/plugins/migrate.py) and the logo embedded in
|
||||||
# `pip install ./backend` produces a package that fails at runtime.
|
# system mail (app/identity/mailer.py). Without these, a `pip install ./backend`
|
||||||
|
# produces a package that fails at runtime.
|
||||||
[tool.setuptools.package-data]
|
[tool.setuptools.package-data]
|
||||||
app = ["accounts/presets.yaml", "plugins/alembic/*"]
|
app = ["accounts/presets.yaml", "plugins/alembic/*", "identity/assets/*.png"]
|
||||||
|
|
||||||
[tool.ruff]
|
[tool.ruff]
|
||||||
line-length = 100
|
line-length = 100
|
||||||
|
|||||||
@@ -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"})
|
test = await c.post("/api/admin/mail/test", json={"to": "me@example.com"})
|
||||||
assert test.status_code == 200
|
assert test.status_code == 200
|
||||||
assert ctx.sent.mails[-1][:2] == ("me@example.com", "Ultimo Email test message")
|
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"] == "<ultimo-logo@ultimo.email>"
|
||||||
|
assert logo.get_payload(decode=True).startswith(b"\x89PNG")
|
||||||
|
assert 'src="cid:ultimo-logo@ultimo.email"' in parts["text/html"].get_content()
|
||||||
|
|||||||
@@ -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"
|
||||||
@@ -29,6 +29,7 @@ export const en = {
|
|||||||
"auth.passwordHint": "at least 8 characters",
|
"auth.passwordHint": "at least 8 characters",
|
||||||
"auth.totpCode": "2FA code",
|
"auth.totpCode": "2FA code",
|
||||||
"auth.totpPrompt": "Enter the 6-digit code from your authenticator app",
|
"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.signIn": "Sign in",
|
||||||
"auth.signingIn": "Signing in…",
|
"auth.signingIn": "Signing in…",
|
||||||
"auth.create": "Create account",
|
"auth.create": "Create account",
|
||||||
|
|||||||
@@ -123,10 +123,16 @@ export const api = {
|
|||||||
method: "POST",
|
method: "POST",
|
||||||
body: JSON.stringify({ localpart, password }),
|
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<UserOut>("/auth/login", {
|
apiFetch<UserOut>("/auth/login", {
|
||||||
method: "POST",
|
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" }),
|
logout: () => apiFetch<{ status: string }>("/auth/logout", { method: "POST" }),
|
||||||
me: () => apiFetch<UserOut>("/auth/me"),
|
me: () => apiFetch<UserOut>("/auth/me"),
|
||||||
|
|||||||
@@ -11,13 +11,14 @@ export function Login() {
|
|||||||
const [password, setPassword] = useState("");
|
const [password, setPassword] = useState("");
|
||||||
const [totp, setTotp] = useState("");
|
const [totp, setTotp] = useState("");
|
||||||
const [needTotp, setNeedTotp] = useState(false);
|
const [needTotp, setNeedTotp] = useState(false);
|
||||||
|
const [rememberDevice, setRememberDevice] = useState(false);
|
||||||
const navigate = useNavigate();
|
const navigate = useNavigate();
|
||||||
const queryClient = useQueryClient();
|
const queryClient = useQueryClient();
|
||||||
// Only offer the signup link when the server would actually accept one.
|
// Only offer the signup link when the server would actually accept one.
|
||||||
const registration = useQuery({ queryKey: ["registration"], queryFn: api.registrationStatus });
|
const registration = useQuery({ queryKey: ["registration"], queryFn: api.registrationStatus });
|
||||||
|
|
||||||
const mutation = useMutation({
|
const mutation = useMutation({
|
||||||
mutationFn: () => api.login(handle, password, needTotp ? totp : undefined),
|
mutationFn: () => api.login(handle, password, needTotp ? totp : undefined, needTotp && rememberDevice),
|
||||||
onSuccess: (user) => {
|
onSuccess: (user) => {
|
||||||
queryClient.setQueryData(["me"], user);
|
queryClient.setQueryData(["me"], user);
|
||||||
navigate("/mail");
|
navigate("/mail");
|
||||||
@@ -71,6 +72,15 @@ export function Login() {
|
|||||||
autoFocus
|
autoFocus
|
||||||
/>
|
/>
|
||||||
<p className="text-xs text-app-muted">{t("auth.totpPrompt")}</p>
|
<p className="text-xs text-app-muted">{t("auth.totpPrompt")}</p>
|
||||||
|
<label className="flex items-center gap-2 pt-1 text-sm text-app-fg">
|
||||||
|
<input
|
||||||
|
type="checkbox"
|
||||||
|
checked={rememberDevice}
|
||||||
|
onChange={(e) => setRememberDevice(e.target.checked)}
|
||||||
|
className="h-4 w-4 accent-[rgb(var(--app-accent))]"
|
||||||
|
/>
|
||||||
|
{t("auth.rememberDevice")}
|
||||||
|
</label>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
<button
|
<button
|
||||||
|
|||||||
Reference in New Issue
Block a user