From 2e2d8667dd53d08bf4592771d669fe4b2557a3f9 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 15:15:43 -0400 Subject: [PATCH] =?UTF-8?q?password=20reset=20by=20email:=20Settings=20?= =?UTF-8?q?=E2=86=92=20Email,=20Forgot=20password=3F,=20and=20a=20test-ema?= =?UTF-8?q?il=20button?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The operator asked for self-service reset over SMTP. It reuses #5173's password_resets table, /reset-password page, one-hour single-use token and sign-out-everywhere. - Settings (rule 25, not env): a new Email group (SMTP server, port, encryption as a choice, username, password, from), General → Public address, and Security → Reset emails per account. The registry gains `choices`, `secret` (the value is never sent back, `is_set` says one is saved, an empty save keeps it) and `url` (http(s), trailing slash stripped). - mailer.py: stdlib smtplib on a worker thread, 20 s timeout, starttls | tls | none. mail_settings() is None until a server, a sender and the public address are set. Links are built from the public address because the Host header can be forged. - POST /api/auth/forgot-password: the same answer at the same speed for any address. The link is made and mailed off the request (send_later). It is throttled like a sign-in per visitor address, and capped per typed email by reset_emails_per_account; past the cap it answers the same and sends nothing. - POST /api/settings/test-email: mails the admin with the saved settings and shows the server's error if it fails. - Public config `password_reset_by_email`. Sign-in shows "Forgot password?" only then, linking to a new /forgot-password page. - docs/public-hosting.md: an "Email and forgotten passwords" section. Tests: the secret stays server-side; emailed link → reset; the same answer for unknown addresses; the cap; test email success and failure; validation units. #5266. Co-Authored-By: Claude Opus 5.5 --- docs/public-hosting.md | 32 +++++- frontend/src/router/index.ts | 11 +- frontend/src/stores/config.ts | 6 ++ frontend/src/views/ForgotPasswordView.vue | 84 +++++++++++++++ frontend/src/views/LoginView.vue | 6 ++ frontend/src/views/SettingsView.vue | 54 ++++++++++ src/inkwell/auth.py | 42 +++++++- src/inkwell/mailer.py | 102 ++++++++++++++++++ src/inkwell/password_resets.py | 49 ++++++++- src/inkwell/ratelimit.py | 7 +- src/inkwell/settings.py | 85 ++++++++++++++- src/inkwell/settings_api.py | 37 ++++++- tests/test_integration.py | 121 +++++++++++++++++++++- tests/test_settings.py | 22 ++++ 14 files changed, 640 insertions(+), 18 deletions(-) create mode 100644 frontend/src/views/ForgotPasswordView.vue create mode 100644 src/inkwell/mailer.py diff --git a/docs/public-hosting.md b/docs/public-hosting.md index 5ec2f4d..fc6337d 100644 --- a/docs/public-hosting.md +++ b/docs/public-hosting.md @@ -108,15 +108,37 @@ docker run --rm -v inkwell-data:/d -v "$PWD":/out alpine tar czf /out/media.tgz - **Session cookies are `HttpOnly` and `SameSite=Lax`**, which is also what stands in for CSRF protection: a `Lax` cookie is not sent on a cross-site POST. +## Email and forgotten passwords + +A forgotten password can be reset two ways. Either way the link works once, within +an hour, and using it signs the account out everywhere and unlinks its apps. + +- **By an admin, always.** Settings → People → Reset password makes a link, and the + admin hands it over. +- **By the person, once email is set up.** Fill in Settings → Email (SMTP server, + port, encryption, sign-in and a from address) and Settings → General → Public + address, the URL people reach this server at. Save, then press **Send test email**, + which mails you with exactly the settings a reset will use. From then on the sign-in + screen offers **Forgot password?** + +The public address is required because the emailed link has to point somewhere the +server can trust. Built from the request instead, a forged `Host` header would mail +someone a reset link to a site the forger controls. + +The SMTP password is kept in the database and never sent back to the browser; the +field shows only that one is saved. The forgot-password page answers the same way, +at the same speed, whether or not an address has an account, and +`reset_emails_per_account` (Settings → Security) caps how many reset emails one +address can be sent. + ## What it does not do Know these before you decide who gets an account. -- **No email at all.** `email_verified` exists on the user row and nothing sets it. - A forgotten password is reset by an admin: Settings → People → Reset password makes - a link that works once, within an hour, and the admin hands it over. Using it signs - the account out everywhere and unlinks its apps. An admin who forgets their own - password and has no other admin still needs a hand on the database. +- **No email verification.** `email_verified` exists on the user row and nothing sets + it. Email is used only for password resets (above). +- **An admin who forgets their own password**, with email off and no other admin, + still needs a hand on the database. - **No second factor.** A password is the whole of it. - **No per-user storage quota.** Any account can upload attachments until the volume is full. `max_attachment_mb` caps a single file, not a total. diff --git a/frontend/src/router/index.ts b/frontend/src/router/index.ts index 541aca0..ac74eb4 100644 --- a/frontend/src/router/index.ts +++ b/frontend/src/router/index.ts @@ -73,7 +73,16 @@ const router = createRouter({ meta: { title: "Create account", guestOnly: true }, }, { - // Where an admin-made password reset link lands (#5173). Not guest-only: the + // Ask for a reset link by email (#5266). Only linked from sign-in when the + // server can send mail; reached otherwise, the server says it can't. + path: "/forgot-password", + name: "forgot-password", + component: () => import("../views/ForgotPasswordView.vue"), + meta: { title: "Forgot password", guestOnly: true }, + }, + { + // Where a password reset link lands, whether an admin made it (#5173) or it was + // emailed (#5266). Not guest-only: the // link signs in whoever uses it as the account it was made for, whoever was // signed in on this browser before. path: "/reset-password", diff --git a/frontend/src/stores/config.ts b/frontend/src/stores/config.ts index 7f7bacd..ca987e5 100644 --- a/frontend/src/stores/config.ts +++ b/frontend/src/stores/config.ts @@ -43,6 +43,9 @@ export interface PublicConfig { enable_url_unfurl: boolean; // How many days a note survives in Trash before the server purges it. 0 = forever. trash_retention_days: number; + // Whether this server can email a reset link, so sign-in offers "Forgot password?" + // (#5266). Absent on an older server and on the offline desktop: no. + password_reset_by_email?: boolean; // Every client this server holds, keyed by platform id. Absent on a server that // holds none, and absent on the desktop's own offline config — the Tauri build // answers `config_get` locally and has no clients to hand out. @@ -66,6 +69,7 @@ export const useConfigStore = defineStore("config", () => { // Empty until proven otherwise: a server with no clients, and an older server // that never had the field, both correctly offer no downloads. const clients = ref>({}); + const passwordResetByEmail = ref(false); const loaded = ref(false); async function load(): Promise { @@ -78,6 +82,7 @@ export const useConfigStore = defineStore("config", () => { enableUrlUnfurl.value = cfg.enable_url_unfurl ?? true; trashRetentionDays.value = cfg.trash_retention_days ?? 30; clients.value = cfg.clients ?? {}; + passwordResetByEmail.value = cfg.password_reset_by_email ?? false; } catch { // Keep defaults if the config endpoint is unreachable. } finally { @@ -97,6 +102,7 @@ export const useConfigStore = defineStore("config", () => { enableUrlUnfurl, trashRetentionDays, clients, + passwordResetByEmail, loaded, load, reload, diff --git a/frontend/src/views/ForgotPasswordView.vue b/frontend/src/views/ForgotPasswordView.vue new file mode 100644 index 0000000..6afe546 --- /dev/null +++ b/frontend/src/views/ForgotPasswordView.vue @@ -0,0 +1,84 @@ + + + diff --git a/frontend/src/views/LoginView.vue b/frontend/src/views/LoginView.vue index 35f5bc1..92da95c 100644 --- a/frontend/src/views/LoginView.vue +++ b/frontend/src/views/LoginView.vue @@ -82,6 +82,12 @@ async function submit() { {{ error }}

Sign in + Forgot password?

diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index 54905ae..46426ff 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -6,6 +6,7 @@ import BaseButton from "../components/BaseButton.vue"; import InviteList from "../components/InviteList.vue"; import AccountList from "../components/AccountList.vue"; import { errorMessage } from "../api/errors"; +import { useUiStore } from "../stores/ui"; interface SettingItem { key: string; @@ -19,9 +20,16 @@ interface SettingItem { // input can refuse an out-of-range value before the round trip. minimum: number | null; maximum: number | null; + // Strings: the allowed values, drawn as a choice. + choices: string[] | null; + // A credential (the SMTP password). Its value never comes back; `is_set` says + // whether one is saved, and leaving the field empty keeps it. + secret: boolean; + is_set: boolean | null; } const config = useConfigStore(); +const ui = useUiStore(); const items = ref([]); const original = ref>({}); @@ -83,6 +91,21 @@ async function save() { } } +// Sends with the SAVED settings, which is what a reset email will use, so it waits +// for unsaved changes to be saved first. +const testing = ref(false); +async function sendTestEmail() { + testing.value = true; + try { + const res = await api.post<{ to: string }>("/api/settings/test-email"); + ui.showToast(`Test email sent to ${res.to}.`); + } catch (e) { + ui.showToast(errorMessage(e, "Couldn't send a test email.")); + } finally { + testing.value = false; + } +} + onMounted(load); @@ -152,6 +175,25 @@ onMounted(load); :value="Number(it.value)" @input="it.value = Number(($event.target as HTMLInputElement).value)" /> + +

{{ it.description }}

+ +
+ + Save first +
diff --git a/src/inkwell/auth.py b/src/inkwell/auth.py index 1a0ad05..264c639 100644 --- a/src/inkwell/auth.py +++ b/src/inkwell/auth.py @@ -11,17 +11,19 @@ from sqlalchemy import delete, func, select from .common import iso from .db import session_scope from .invites import INVALID as INVALID_INVITE, record_redeemer, redeem -from .password_resets import INVALID as INVALID_RESET, claim as claim_reset +from .mailer import send_later +from .password_resets import INVALID as INVALID_RESET, claim as claim_reset, mail_reset_link from .models.device_token import DeviceToken from .models.user import User from .proxy import client_address from .ratelimit import ( register_by_address, + reset_mail_by_account, sign_in_by_account, sign_in_by_address, ) from .security import dummy_verify, generate_token, hash_password, hash_token, verify_password -from .settings import get_setting, set_settings +from .settings import get_setting, mail_configured, set_settings bp = Blueprint("auth", __name__, url_prefix="/api/auth") @@ -330,6 +332,42 @@ async def me(): return jsonify(_serialize_user(user)) +# The one answer to every forgot-password request that gets as far as sending. +FORGOT_SENT = "If an account uses that address, a reset link is on its way. It works for an hour." + + +@bp.post("/forgot-password") +async def forgot_password(): + """Email a reset link to whoever owns this address (#5266). + + Says the same thing, at the same speed, whether or not the address has an + account: the link is made and sent off the request. Throttled per visitor address + like a sign-in, and per typed email by `reset_emails_per_account` — past that cap + the answer is unchanged and nothing is sent, so the cap is no oracle either. + """ + data = await request.get_json(silent=True) or {} + email = (data.get("email") or "").strip().lower() + if not email or "@" not in email: + return jsonify({"error": "a valid email is required"}), 400 + + wait = _sign_in_block("") + if wait is not None: + return _throttled(wait) + _sign_in_failed("") + + async with session_scope() as db: + if not await mail_configured(db): + return jsonify({"error": "this server can't send email; ask your admin for a reset link"}), 400 + + if reset_mail_by_account.retry_after(email) is None: + reset_mail_by_account.record(email) + send_later(mail_reset_link(email)) + logger.info("password reset requested for=%s from=%s", email, client_address()) + else: + logger.warning("password reset email capped for=%s from=%s", email, client_address()) + return jsonify({"ok": True, "message": FORGOT_SENT}) + + @bp.post("/reset-password") async def reset_password(): """Set a new password with a reset link an admin made (#5173), and sign the diff --git a/src/inkwell/mailer.py b/src/inkwell/mailer.py new file mode 100644 index 0000000..448a2a1 --- /dev/null +++ b/src/inkwell/mailer.py @@ -0,0 +1,102 @@ +"""Sending email over the SMTP server set in Settings → Email (#5266). + +Used for password reset links, and for the admin's test message. Email is an optional +feature (rule 164): with no server set, `mail_settings` answers None and everything +that would send says it is not available. A send that fails is logged and never +takes anything else down with it. + +stdlib `smtplib`, run on a worker thread, so the event loop never waits on a mail +server and nothing new is installed. +""" +from __future__ import annotations + +import asyncio +import logging +import smtplib +import ssl +from dataclasses import dataclass +from email.message import EmailMessage + +from .settings import get_setting, mail_configured + +logger = logging.getLogger(__name__) + +# Long enough for a slow relay, short enough that a dead one doesn't hold a thread. +TIMEOUT_S = 20 + + +@dataclass(frozen=True) +class MailSettings: + host: str + port: int + security: str # starttls | tls | none + username: str + password: str + sender: str + public_url: str + + +async def mail_settings(db) -> MailSettings | None: + """The saved SMTP settings, or None when email is off.""" + if not await mail_configured(db): + return None + username = await get_setting(db, "smtp_username") + return MailSettings( + host=await get_setting(db, "smtp_host"), + port=await get_setting(db, "smtp_port"), + security=await get_setting(db, "smtp_security"), + username=username, + password=await get_setting(db, "smtp_password"), + sender=await get_setting(db, "smtp_from") or username, + public_url=await get_setting(db, "public_url"), + ) + + +def build_message(cfg: MailSettings, to: str, subject: str, text: str) -> EmailMessage: + msg = EmailMessage() + msg["From"] = cfg.sender + msg["To"] = to + msg["Subject"] = subject + msg.set_content(text) + return msg + + +def _deliver(cfg: MailSettings, msg: EmailMessage) -> None: + """Hand one message to the SMTP server. Blocking; raises on any failure.""" + context = ssl.create_default_context() + if cfg.security == "tls": + server: smtplib.SMTP = smtplib.SMTP_SSL(cfg.host, cfg.port, timeout=TIMEOUT_S, context=context) + else: + server = smtplib.SMTP(cfg.host, cfg.port, timeout=TIMEOUT_S) + with server: + if cfg.security == "starttls": + server.starttls(context=context) + if cfg.username: + server.login(cfg.username, cfg.password) + server.send_message(msg) + + +async def send(cfg: MailSettings, to: str, subject: str, text: str) -> None: + """Send one plain-text email. Raises when the server refuses or can't be reached.""" + await asyncio.to_thread(_deliver, cfg, build_message(cfg, to, subject, text)) + + +# Sends started by `send_later`, held so they aren't collected mid-flight. +_running: set[asyncio.Task] = set() + + +def send_later(job) -> None: + """Run a coroutine that sends mail without making the request wait for it. + + For the forgot-password route, where waiting would also be an oracle: an address + with an account would answer as slowly as a mail server, and one without, at once. + `job` handles and logs its own failures.""" + task = asyncio.create_task(job) + _running.add(task) + task.add_done_callback(_running.discard) + + +async def drain() -> None: + """Wait for every pending send. For tests — nothing in the app calls this.""" + while _running: + await asyncio.gather(*list(_running), return_exceptions=True) diff --git a/src/inkwell/password_resets.py b/src/inkwell/password_resets.py index 8ca3f58..03c9371 100644 --- a/src/inkwell/password_resets.py +++ b/src/inkwell/password_resets.py @@ -8,19 +8,29 @@ Using the link sets a new password and signs the account out everywhere. Its web sessions end because the account's `session_epoch` moves on (see `auth`), and its device tokens are deleted, so each linked app has to sign in again. +A person can also ask for one themselves when the server can send email (#5266): +`mail_reset_link` makes the same link and mails it to the account's address. + This module is the reset itself; `auth.reset_password` redeems it and the admin route is in `accounts_api.py`, the same split as invites and for the same reason. """ from __future__ import annotations +import logging import uuid from datetime import datetime, timedelta, timezone -from sqlalchemy import delete, update +from sqlalchemy import delete, select, update from sqlalchemy.ext.asyncio import AsyncSession +from .db import session_scope +from .mailer import mail_settings, send from .models.password_reset import PasswordReset +from .models.user import User from .security import generate_token, hash_token +from .settings import get_setting + +logger = logging.getLogger(__name__) # Long enough to read a message and act on it; short enough that a link left in a # chat history is dead by the time anyone else scrolls past it. @@ -30,8 +40,9 @@ LIFETIME = timedelta(hours=1) INVALID = "invalid or expired reset link" -async def issue(db: AsyncSession, user_id: uuid.UUID, by: uuid.UUID) -> tuple[str, datetime]: - """Make a reset link for the account, returning the token and its expiry. +async def issue(db: AsyncSession, user_id: uuid.UUID, by: uuid.UUID | None) -> tuple[str, datetime]: + """Make a reset link for the account, returning the token and its expiry. `by` is + the admin who made it, or None when the person asked by email. Its earlier unused links are deleted first, so only the newest one works: an admin who makes a second link because the first went astray has closed the first. @@ -64,3 +75,35 @@ async def claim(db: AsyncSession, token: str) -> uuid.UUID | None: .values(used_at=now) .returning(PasswordReset.user_id) ) + + +async def mail_reset_link(email: str) -> None: + """Email a reset link to the account with this address, if there is one. + + Run off the request (`mailer.send_later`), so the person asking gets the same + answer at the same speed whether or not the address has an account. Logs and + swallows every failure: there is no one left to tell. + """ + try: + async with session_scope() as db: + cfg = await mail_settings(db) + user = await db.scalar(select(User).where(User.email == email)) + if cfg is None or user is None: + return + token, _ = await issue(db, user.id, None) + await db.commit() + site = await get_setting(db, "site_name") + to = user.email + link = f"{cfg.public_url}/reset-password?token={token}" + await send( + cfg, + to, + f"Reset your {site} password", + f"Someone asked to reset the password for {to} on {site}.\n\n" + f"If it was you, open this link within the hour to choose a new one:\n\n{link}\n\n" + "Setting it signs you out everywhere else. If it wasn't you, ignore this " + "email and your password stays as it is.\n", + ) + logger.info("password reset email sent to=%s", to) + except Exception: + logger.exception("password reset email failed for=%s", email) diff --git a/src/inkwell/ratelimit.py b/src/inkwell/ratelimit.py index 1cc81c7..336efd8 100644 --- a/src/inkwell/ratelimit.py +++ b/src/inkwell/ratelimit.py @@ -126,9 +126,14 @@ sign_in_by_address = SlidingWindow( register_by_address = SlidingWindow( lambda: live("register_limit_per_address"), _minutes("register_window_minutes") ) +# Reset emails, counted per address typed in whether or not it has an account, so +# the cap can't be used to learn which ones do (#5266). +reset_mail_by_account = SlidingWindow( + lambda: live("reset_emails_per_account"), _minutes("signin_window_minutes") +) def reset_all() -> None: """Drop every counter. For tests — nothing in the app calls this.""" - for window in (sign_in_by_account, sign_in_by_address, register_by_address): + for window in (sign_in_by_account, sign_in_by_address, register_by_address, reset_mail_by_account): window.clear() diff --git a/src/inkwell/settings.py b/src/inkwell/settings.py index 7f331c3..de0a331 100644 --- a/src/inkwell/settings.py +++ b/src/inkwell/settings.py @@ -26,6 +26,13 @@ class SettingDef: # lock every account out permanently. minimum: int | None = None maximum: int | None = None + # Strings only: the values allowed, shown as a choice rather than a text field. + choices: tuple[str, ...] | None = None + # A credential. The admin API never sends its value back, only whether one is + # set, and saving it empty leaves it as it was (#5266). + secret: bool = False + # An http(s) address, stored without a trailing slash. + url: bool = False # The source of truth for every user-facing setting. Add a row here and it appears @@ -34,6 +41,15 @@ REGISTRY: list[SettingDef] = [ SettingDef( "site_name", "string", "Inkwell", "Site name", "Shown in the header and the browser tab.", "General" ), + SettingDef( + "public_url", + "string", + "", + "Public address", + "Where people reach this server, like https://notes.example.com. Emailed links use it.", + "General", + url=True, + ), SettingDef( "allow_registration", "bool", @@ -77,6 +93,35 @@ REGISTRY: list[SettingDef] = [ "The server contacts the linked site; private/internal addresses are always blocked.", "Links", ), + # --- Email (#5266) --------------------------------------------------------------- + # + # Only for password reset links so far. Off until a server, a sender and the + # public address are all set (`mailer.mail_settings`). + SettingDef("smtp_host", "string", "", "SMTP server", "Leave empty to turn email off.", "Email"), + SettingDef( + "smtp_port", + "int", + 587, + "SMTP port", + "587 for STARTTLS, 465 for TLS.", + "Email", + minimum=1, + maximum=65535, + ), + SettingDef( + "smtp_security", + "string", + "starttls", + "Encryption", + "How the connection to the SMTP server is secured.", + "Email", + choices=("starttls", "tls", "none"), + ), + SettingDef("smtp_username", "string", "", "SMTP username", "Leave empty if no sign-in is needed.", "Email"), + SettingDef( + "smtp_password", "string", "", "SMTP password", "Kept on the server and never shown again.", "Email", secret=True + ), + SettingDef("smtp_from", "string", "", "From address", "Who emails come from. Defaults to the username.", "Email"), # --- Security ----------------------------------------------------------------- # # Read on paths too hot for a database round trip (the credential throttle checks @@ -151,6 +196,16 @@ REGISTRY: list[SettingDef] = [ minimum=1, maximum=10080, ), + SettingDef( + "reset_emails_per_account", + "int", + 3, + "Reset emails per account", + "How many password reset emails one address can be sent within the sign-in window.", + "Security", + minimum=1, + maximum=100, + ), ] _BY_KEY: dict[str, SettingDef] = {d.key: d for d in REGISTRY} @@ -218,18 +273,34 @@ async def get_public_config(db) -> dict: # left in Trash. A native client also reads it BEFORE linking, which is why # it belongs on the unauthenticated config rather than behind login. "trash_retention_days": await get_setting(db, "trash_retention_days"), + # Whether the sign-in screen offers "Forgot password?" (#5266). + "password_reset_by_email": await mail_configured(db), } +async def mail_configured(db) -> bool: + """Whether the server can send email: an SMTP server, someone to send as, and a + public address for the links to point at. The last is not optional: built from + the request's Host instead, a forged Host would mail someone a reset link to a + site the forger controls.""" + host = await get_setting(db, "smtp_host") + sender = await get_setting(db, "smtp_from") or await get_setting(db, "smtp_username") + return bool(host and sender and await get_setting(db, "public_url")) + + async def get_admin_settings(db) -> list[dict]: """Every registry setting with its current value + metadata, for the admin UI.""" result: list[dict] = [] for d in REGISTRY: + value = await get_setting(db, d.key) result.append( { "key": d.key, "type": d.type, - "value": await get_setting(db, d.key), + "value": "" if d.secret else value, + "is_set": bool(value) if d.secret else None, + "secret": d.secret, + "choices": list(d.choices) if d.choices else None, "default": d.default, "label": d.label, "description": d.description, @@ -258,6 +329,7 @@ _LIVE_KEYS = ( "signin_window_minutes", "register_limit_per_address", "register_window_minutes", + "reset_emails_per_account", ) _live: dict[str, Any] = {k: _BY_KEY[k].default for k in _LIVE_KEYS} @@ -304,7 +376,16 @@ def validate_updates(updates: dict) -> tuple[dict, str | None]: elif defn.type == "bool": clean[key] = _coerce_bool(val) else: - clean[key] = str(val) + text = str(val if val is not None else "").strip() + if defn.secret and not text: + continue # empty means "leave it as it is"; the UI never has the value + if defn.choices and text not in defn.choices: + return {}, f"{defn.label} must be one of: {', '.join(defn.choices)}" + if defn.url and text: + if not text.startswith(("http://", "https://")): + return {}, f"{defn.label} must start with http:// or https://" + text = text.rstrip("/") + clean[key] = text return clean, None diff --git a/src/inkwell/settings_api.py b/src/inkwell/settings_api.py index 5449413..fb73357 100644 --- a/src/inkwell/settings_api.py +++ b/src/inkwell/settings_api.py @@ -2,11 +2,17 @@ from __future__ import annotations from datetime import timedelta -from quart import Blueprint, current_app, jsonify, request +import logging + +from quart import Blueprint, current_app, g, jsonify, request from .auth import require_admin from .db import session_scope -from .settings import get_admin_settings, refresh_live, set_settings, validate_updates +from .mailer import mail_settings, send +from .models.user import User +from .settings import get_admin_settings, get_setting, refresh_live, set_settings, validate_updates + +logger = logging.getLogger(__name__) bp = Blueprint("settings", __name__, url_prefix="/api/settings") @@ -46,3 +52,30 @@ async def update_settings(): current_app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(days=int(clean["session_ttl_days"])) return jsonify({"settings": result}) + + +@bp.post("/test-email") +@require_admin +async def test_email(): + """Send a test message to the signed-in admin with the SAVED email settings, so + "Send test email" checks what reset links will actually use (#5266). An admin is + shown the server's own error, which is what fixing the settings takes.""" + async with session_scope() as db: + cfg = await mail_settings(db) + if cfg is None: + return jsonify( + {"error": "Email is off: it needs an SMTP server, a from address and the public address."} + ), 400 + admin = await db.get(User, g.user_id) + site = await get_setting(db, "site_name") + try: + await send( + cfg, + admin.email, + f"{site} test email", + f"This is a test from {site} at {cfg.public_url}. Password reset emails will arrive like this one.\n", + ) + except Exception as e: # any SMTP, socket or TLS failure; the admin needs its text + logger.warning("test email failed: %s", e) + return jsonify({"error": f"Couldn't send: {e}"}), 502 + return jsonify({"ok": True, "to": admin.email}) diff --git a/tests/test_integration.py b/tests/test_integration.py index 3ecdc3e..a315524 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -17,19 +17,22 @@ from __future__ import annotations import asyncio import hashlib +import re +import smtplib import uuid from datetime import datetime, timedelta, timezone import pytest import pytest_asyncio -from sqlalchemy import func, select, text, update +from sqlalchemy import delete, func, select, text, update -from inkwell import ratelimit +from inkwell import mailer, ratelimit from inkwell.app import create_app from inkwell.config import Config from inkwell.db import dispose_engine, session_scope from inkwell.models.invite import Invite from inkwell.models.password_reset import PasswordReset +from inkwell.models.settings import Setting from inkwell.models.share import Share from inkwell.models.label import NoteLabel from inkwell.models.note import Note @@ -476,6 +479,7 @@ async def test_the_security_group_reaches_the_admin_ui(app_client, db): "signin_window_minutes", "register_limit_per_address", "register_window_minutes", + "reset_emails_per_account", } # The UI renders a number input from these, and it cannot offer a safe range it # was never told about. @@ -1337,3 +1341,116 @@ async def test_a_share_names_someone_else_on_this_instance(app_client, db): other = await _owners_note(app_client, "another") sid = (await (await _share(app_client, nid, people["stranger"])).get_json())["shares"][-1]["id"] assert (await app_client.delete(f"/api/notes/{other}/shares/{sid}")).status_code == 404 + + +# --- Password reset by email (#5266) ------------------------------------------ +# +# The SMTP hand-off is replaced by a list; everything up to it is real. + +_MAIL_SETTINGS = { + "smtp_host": "smtp.example.test", + "smtp_from": "inkwell@example.test", + "smtp_password": "hunter2", + "public_url": "https://notes.example.test/", +} + + +async def _mail_off() -> None: + """Settings outlive the per-test truncate, so each email test starts from none.""" + async with session_scope() as fresh: + await fresh.execute(delete(Setting).where(Setting.key.in_([*_MAIL_SETTINGS, "smtp_security"]))) + await fresh.commit() + + +def _outbox(monkeypatch) -> list: + sent: list = [] + monkeypatch.setattr(mailer, "_deliver", lambda cfg, msg: sent.append(msg)) + return sent + + +async def _forgot(email: str): + return await create_app().test_client().post("/api/auth/forgot-password", json={"email": email}) + + +async def test_the_smtp_password_never_leaves_the_server(app_client, db): + await _mail_off() + await _admin_with_invite(app_client) + saved = await app_client.patch("/api/settings", json=_MAIL_SETTINGS) + assert saved.status_code == 200, await saved.get_data(as_text=True) + rows = {r["key"]: r for r in (await saved.get_json())["settings"]} + assert (rows["smtp_password"]["value"], rows["smtp_password"]["is_set"]) == ("", True) + assert rows["public_url"]["value"] == "https://notes.example.test" + + # Saving the form again sends the password field empty, which keeps it. + assert (await app_client.patch("/api/settings", json={"smtp_password": ""})).status_code == 200 + async with session_scope() as fresh: + assert await get_setting(fresh, "smtp_password") == "hunter2" + + assert (await app_client.patch("/api/settings", json={"smtp_security": "ssl3"})).status_code == 400 + assert (await app_client.patch("/api/settings", json={"public_url": "notes.example.test"})).status_code == 400 + + +async def test_a_forgotten_password_is_reset_from_an_emailed_link(app_client, db, monkeypatch): + await _mail_off() + outbox = _outbox(monkeypatch) + token = await _admin_with_invite(app_client) + assert (await _register("guest@example.test", token)).status_code == 201 + + # Off until the server can send: no link on sign-in, and the route says so. + assert (await (await app_client.get("/api/config")).get_json())["password_reset_by_email"] is False + assert (await _forgot("guest@example.test")).status_code == 400 + + await app_client.patch("/api/settings", json=_MAIL_SETTINGS) + assert (await (await app_client.get("/api/config")).get_json())["password_reset_by_email"] is True + + known = await _forgot("Guest@Example.test") + unknown = await _forgot("nobody@example.test") + assert known.status_code == unknown.status_code == 200 + assert await known.get_json() == await unknown.get_json() + await mailer.drain() + + assert [m["To"] for m in outbox] == ["guest@example.test"] + text = outbox[0].get_content() + link = re.search(r"https://notes\.example\.test/reset-password\?token=([\w-]+)", text) + assert link, text + async with session_scope() as fresh: + assert await fresh.scalar(select(PasswordReset.created_by)) is None + + reset = await create_app().test_client().post( + "/api/auth/reset-password", json={"token": link.group(1), "password": "chosen-by-email"} + ) + assert reset.status_code == 200 + assert (await reset.get_json())["email"] == "guest@example.test" + + +async def test_reset_emails_stop_at_the_cap_without_saying_so(app_client, db, monkeypatch): + await _mail_off() + outbox = _outbox(monkeypatch) + token = await _admin_with_invite(app_client) + assert (await _register("guest@example.test", token)).status_code == 201 + await app_client.patch("/api/settings", json=_MAIL_SETTINGS) + + answers = [await _forgot("guest@example.test") for _ in range(4)] + assert [a.status_code for a in answers] == [200, 200, 200, 200] + await mailer.drain() + assert len(outbox) == 3 # reset_emails_per_account defaults to 3 + + +async def test_the_test_email_goes_to_the_admin_and_reports_a_failure(app_client, db, monkeypatch): + await _mail_off() + outbox = _outbox(monkeypatch) + await _admin_with_invite(app_client) + assert (await app_client.post("/api/settings/test-email")).status_code == 400 + + await app_client.patch("/api/settings", json=_MAIL_SETTINGS) + sent = await app_client.post("/api/settings/test-email") + assert sent.status_code == 200 + assert [m["To"] for m in outbox] == ["owner@example.test"] + + def refuse(cfg, msg): + raise smtplib.SMTPAuthenticationError(535, b"bad credentials") + + monkeypatch.setattr(mailer, "_deliver", refuse) + failed = await app_client.post("/api/settings/test-email") + assert failed.status_code == 502 + assert "bad credentials" in (await failed.get_json())["error"] diff --git a/tests/test_settings.py b/tests/test_settings.py index 7ce8cef..4bf3cf3 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -23,3 +23,25 @@ def test_validate_rejects_bad_int(): clean, error = validate_updates({"session_ttl_days": "not-a-number"}) assert error is not None assert clean == {} + + +def test_an_empty_secret_keeps_what_is_saved(): + clean, error = validate_updates({"smtp_password": "", "smtp_host": "smtp.example.test"}) + assert error is None + assert clean == {"smtp_host": "smtp.example.test"} + + +def test_a_choice_must_be_one_of_its_values(): + assert validate_updates({"smtp_security": "tls"}) == ({"smtp_security": "tls"}, None) + clean, error = validate_updates({"smtp_security": "ssl"}) + assert clean == {} and error + + +def test_the_public_address_is_an_http_url_without_a_trailing_slash(): + assert validate_updates({"public_url": "https://notes.example.test/"}) == ( + {"public_url": "https://notes.example.test"}, + None, + ) + assert validate_updates({"public_url": ""}) == ({"public_url": ""}, None) + clean, error = validate_updates({"public_url": "javascript:alert(1)"}) + assert clean == {} and error