From 3bd086ab0eda9d097bc9b0110335c43e9db0defb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 17:17:41 +0000 Subject: [PATCH 1/2] Say accounts cannot be created before the form is filled in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running without GITEA_ADMIN_TOKEN is the safer configuration and is documented as such, but the member form did not know: it offered "Crear también su cuenta" ticked by default and reported "Falta GITEA_ADMIN_TOKEN" only on submit, after three fields had been filled in. That is the shape of failure this project has lost the most time to — something that cannot happen, going unsaid until someone has relied on it. The form now reads the config when it renders and says so, with a link to Gitea's create-user page. The checkbox is disabled rather than hidden, because "you cannot do this here" is more use than an option that quietly is not there. The refusal itself stays in the handler: a disabled input is a courtesy, and a hand-crafted POST still meets the same error. Verified: 104 checks. The form states the limit with no token and is unchanged with one, and submitting create_account anyway still creates no member. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn --- apps/board/members.py | 13 ++++++++-- apps/board/templates/member_new.html | 15 +++++++++++ apps/board/tests/test_members.py | 37 ++++++++++++++++++++++++++++ docs/server-setup.md | 5 ++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/apps/board/members.py b/apps/board/members.py index 9406a1f..3a6876d 100644 --- a/apps/board/members.py +++ b/apps/board/members.py @@ -18,7 +18,7 @@ import json import re import sqlite3 -from flask import (Blueprint, Response, abort, flash, g, redirect, +from flask import (Blueprint, Response, abort, current_app, flash, g, redirect, render_template, request, url_for) from . import gitea @@ -121,7 +121,16 @@ def index(): @admin_required def new(): if request.method == "GET": - return render_template("member_new.html", can_make_admin=g.member["role"] == "owner") + return render_template( + "member_new.html", + can_make_admin=g.member["role"] == "owner", + # Without a site-admin token the server cannot create Gitea accounts, + # which is the documented safer configuration rather than a fault. + # The form says so before it is filled in; offering a ticked checkbox + # and reporting the problem on submit wastes the work of filling it. + can_create_accounts=bool(current_app.config.get("ADMIN_TOKEN")), + gitea_url=current_app.config["GITEA_URL"].rstrip("/"), + ) login = request.form.get("login", "").strip() display_name = request.form.get("display_name", "").strip() diff --git a/apps/board/templates/member_new.html b/apps/board/templates/member_new.html index 60e22c8..e9ed03d 100644 --- a/apps/board/templates/member_new.html +++ b/apps/board/templates/member_new.html @@ -17,10 +17,25 @@ + {% if can_create_accounts %} + {% else %} + {# Disabled rather than hidden: "you cannot do this here" is more use than + an option that quietly is not there. The handler refuses it either way. #} + +

+ Este servidor no puede crear cuentas (no tiene un token de administración + de Gitea). Crea la cuenta primero en + Gitea y luego da de alta + aquí ese mismo usuario. +

+ {% endif %} {% if can_make_admin %} diff --git a/apps/board/tests/test_members.py b/apps/board/tests/test_members.py index cb12958..d488f99 100644 --- a/apps/board/tests/test_members.py +++ b/apps/board/tests/test_members.py @@ -182,3 +182,40 @@ def test_the_export_is_only_your_own_writing(client, db, make_member, sign_in): body = client.get("/comunidad/mis-datos").get_data(as_text=True) assert "Mío" in body assert "Suyo" not in body + + +def test_the_form_says_so_when_accounts_cannot_be_created(app, client, owner_id, sign_in): + """Without a site-admin token the server cannot create Gitea accounts. That + is the documented safer setup, not a fault — but it has to be said before + someone fills the form, not after they submit it.""" + app.config["ADMIN_TOKEN"] = "" + sign_in(owner_id) + + body = client.get("/comunidad/miembros/nuevo").get_data(as_text=True) + assert "no puede crear cuentas" in body + assert 'name="create_account" disabled' in body + assert 'name="create_account" checked' not in body + + +def test_with_a_token_the_form_is_unchanged(app, client, owner_id, sign_in): + app.config["ADMIN_TOKEN"] = "admintoken" + sign_in(owner_id) + + body = client.get("/comunidad/miembros/nuevo").get_data(as_text=True) + assert 'name="create_account" checked' in body + assert "no puede crear cuentas" not in body + + +def test_ticking_it_anyway_still_creates_nothing(app, client, db, post, owner_id, sign_in): + """A disabled input is a courtesy, not a permission — the refusal lives in + the handler, where a hand-crafted POST also meets it.""" + app.config["ADMIN_TOKEN"] = "" + sign_in(owner_id) + + response = post("/comunidad/miembros/nuevo", { + "login": "maria", "display_name": "María", "email": "m@example.com", + "role": "user", "create_account": "on", + }, follow_redirects=True) + + assert "GITEA_ADMIN_TOKEN" in response.get_data(as_text=True) + assert db.execute("SELECT 1 FROM members WHERE gitea_login = 'maria'").fetchone() is None diff --git a/docs/server-setup.md b/docs/server-setup.md index fae2d64..5ad53e8 100644 --- a/docs/server-setup.md +++ b/docs/server-setup.md @@ -346,6 +346,11 @@ board's environment — the compose file, `docker inspect`, a shell in the container — can use it. If you would rather not have that on the box, leave `GITEA_ADMIN_TOKEN` empty and create accounts in Gitea by hand. +Leaving it empty is a supported configuration, not a half-finished one: the +*Dar de alta* form checks for the token when it renders, says plainly that this +server cannot create accounts, and links to Gitea's own create-user page. You +then add that username here, with the checkbox already off. + ### 11.3 Build and run ```sh From e95c732c6f30def140a547e7d3400bed819669fc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 17:49:39 +0000 Subject: [PATCH 2/2] Let members get a password of their own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now an admin created an account, the password appeared once on screen, and that was the only copy. Gitea's Forgot password answers "Account recovery is disabled because no email is set up", because this server has never been able to send mail. Every route a member had to a password of their own was closed, which made the members area unusable by anyone not standing next to the owner. Now: the admin enters a username and an email, the server creates the Gitea account with a random password nobody ever sees — the admin included — and the member is emailed a link to a Viena Latina page where they choose their own. The same mechanism gives "¿Olvidaste tu contraseña?" that works, replacing Gitea's dead page. Nobody leaves the site for either, which is possible because PATCH /admin/users/{username} accepts a password. A link is enough to take an account, so it is treated as a credential: single use, short-lived, stored only as a SHA-256, and invalidated when a newer one is issued so an older email stops working. SHA-256 rather than a password hash because these are 32 random bytes, not something a person chose — there is no dictionary to slow down. Recovery answers identically for a known and an unknown address, or the form becomes a way to enumerate members one address at a time, and stops after three tries so it cannot be used to mail-bomb somebody using this server's reputation. Rejecting a password deliberately does not spend the token. A typo must not lock somebody out of an account they have never reached. If the mail fails the admin is shown the link instead. The account exists either way, so the difference is between a delayed invitation and a person who simply never gets in. Found while testing: the rate limit did not work at all. created_at is written by SQLite as "2026-09-25 15:00:00" and I compared it against Python's "2026-09-25T15:00:00+00:00"; a space sorts before T, so every row looked older than any threshold and the limit silently never fired. It is now compared inside SQLite, where the format and the clock are the same one. Also drops two tabs from the sign-in page: OpenID, which nobody here will use, and Register, which contradicted DISABLE_REGISTRATION and invited people to try something the server then refused. Verified: 126 checks. Tokens are hashed at rest, work once, expire, die when reissued, and are refused for a suspended member; a short password and a Gitea rejection both leave the link usable; recovery is indistinguishable for known and unknown addresses and stops at three; creating a member emails them and shows no password; a failed send surfaces the link. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn --- apps/board/app.py | 9 + apps/board/auth.py | 82 ++++++- apps/board/gitea.py | 41 ++++ apps/board/invites.py | 105 +++++++++ apps/board/mail.py | 114 ++++++++++ apps/board/members.py | 29 ++- apps/board/schema.sql | 23 ++ apps/board/templates/login.html | 3 + apps/board/templates/member_created.html | 38 ++-- apps/board/templates/recover.html | 34 +++ apps/board/templates/set_password.html | 39 ++++ apps/board/tests/conftest.py | 5 + apps/board/tests/test_invites.py | 264 +++++++++++++++++++++++ apps/board/tests/test_members.py | 12 +- docs/server-setup.md | 82 +++++++ infra/board/.env.example | 14 ++ infra/gitea/docker-compose.yml | 9 + 17 files changed, 881 insertions(+), 22 deletions(-) create mode 100644 apps/board/invites.py create mode 100644 apps/board/mail.py create mode 100644 apps/board/templates/recover.html create mode 100644 apps/board/templates/set_password.html create mode 100644 apps/board/tests/test_invites.py diff --git a/apps/board/app.py b/apps/board/app.py index d10ef80..fc67b0b 100644 --- a/apps/board/app.py +++ b/apps/board/app.py @@ -55,6 +55,15 @@ def create_app(overrides: dict | None = None) -> Flask: # publishing from Decap as far as the pipeline is concerned. CONTENT_REPO=os.environ.get("CONTENT_REPO", "pablo/vienalatina"), CONTENT_BRANCH=os.environ.get("CONTENT_BRANCH", "main"), + # Outgoing mail. Without MAIL_HOST the app still runs, but nobody + # can be invited or recover a password, so the admin screens say so + # rather than failing at the moment somebody presses send. + MAIL_HOST=os.environ.get("MAIL_HOST", ""), + MAIL_PORT=os.environ.get("MAIL_PORT", "587"), + MAIL_SECURITY=os.environ.get("MAIL_SECURITY", "starttls"), + MAIL_USER=os.environ.get("MAIL_USER", ""), + MAIL_PASSWORD=os.environ.get("MAIL_PASSWORD", ""), + MAIL_FROM=os.environ.get("MAIL_FROM", "Viena Latina "), URL_PREFIX=URL_PREFIX, COOLDOWN_SECONDS=int(os.environ.get("BOARD_COOLDOWN_SECONDS", "20")), SESSION_COOKIE_HTTPONLY=True, diff --git a/apps/board/auth.py b/apps/board/auth.py index 17d6660..0587997 100644 --- a/apps/board/auth.py +++ b/apps/board/auth.py @@ -14,11 +14,15 @@ import secrets from flask import (Blueprint, current_app, flash, g, redirect, render_template, request, session, url_for) -from . import gitea, tokens +from . import gitea, invites, mail, tokens from .db import get_db bp = Blueprint("auth", __name__) +# Gitea enforces its own minimum as well; this one is stricter so the member is +# told before the round trip rather than after it, in their own language. +PASSWORD_MIN = 10 + def redirect_uri() -> str: return current_app.config["BASE_URL"].rstrip("/") + url_for("auth.callback") @@ -129,6 +133,82 @@ def callback(): return redirect(target) +def invite_url(token: str) -> str: + return current_app.config["BASE_URL"].rstrip("/") + url_for( + "auth.set_password", token=token + ) + + +@bp.route("/invitacion/", methods=["GET", "POST"]) +def set_password(token: str): + """Where a member chooses their own password, from an invite or a reset. + + One page for both, because they differ only in the wording and how long the + link lived. The token is the only credential: somebody arriving here is not + signed in and cannot be. + """ + member = invites.lookup(token) + if member is None: + # Deliberately one message for every reason it might fail — expired, + # already used, never existed. Distinguishing them tells whoever holds + # a stale link something about the account it points at. + return render_template("set_password.html", member=None, token=token, + minimum=PASSWORD_MIN), 400 + + if request.method == "GET": + return render_template("set_password.html", member=member, token=token, + minimum=PASSWORD_MIN) + + password = request.form.get("password", "") + confirm = request.form.get("confirm", "") + if password != confirm: + flash("Las dos contraseñas no coinciden.", "error") + elif len(password) < PASSWORD_MIN: + flash(f"La contraseña necesita al menos {PASSWORD_MIN} caracteres.", "error") + else: + try: + gitea.admin_set_password(member["gitea_login"], password) + except gitea.GiteaError as exc: + flash(str(exc), "error") + else: + # Only now: a token that set a password is spent, but one whose + # password Gitea rejected has to keep working or the member is + # locked out by a typo. + invites.consume(member["invite_id"]) + flash("Contraseña guardada. Ya puedes entrar.", "ok") + return redirect(url_for("auth.login")) + + return render_template("set_password.html", member=member, token=token, + minimum=PASSWORD_MIN), 400 + + +@bp.route("/recuperar", methods=["GET", "POST"]) +def recover(): + """Replaces Gitea's recovery page, which is dead without a mailer.""" + if request.method == "GET": + return render_template("recover.html") + + email = request.form.get("email", "").strip() + member = get_db().execute( + """SELECT * FROM members + WHERE email = ? COLLATE NOCASE AND active = 1 + AND role IN ('owner', 'admin', 'user')""", + (email,), + ).fetchone() + + if member is not None and not invites.rate_limited(member["id"]): + try: + mail.send_reset(member["email"], member["display_name"], + invite_url(invites.issue(member["id"], "reset"))) + except (mail.MailFailed, mail.MailNotConfigured) as exc: + current_app.logger.warning("Reset mail failed: %s", exc) + + # The same answer either way, whatever happened above. Saying "no account + # with that address" would turn this form into a way to find out who is a + # member, one address at a time. + return render_template("recover.html", sent=True) + + @bp.route("/logout", methods=["POST"]) def logout(): # Drop the Gitea token too. Signing out should stop the server being able to diff --git a/apps/board/gitea.py b/apps/board/gitea.py index 1b3d17b..eaa35e6 100644 --- a/apps/board/gitea.py +++ b/apps/board/gitea.py @@ -157,6 +157,47 @@ def admin_create_user(login: str, email: str, full_name: str, password: str) -> raise GiteaError(f"Gitea rechazó la creación del usuario ({response.status_code}).") +def admin_set_password(login: str, password: str) -> None: + """Set a member's password on their behalf, after they chose it here. + + This is what lets the whole invitation and reset flow stay inside + /comunidad/ instead of handing people to Gitea, whose own recovery page is + dead without a mailer anyway. + + `login_name` and `source_id` are sent although nothing about them changes: + Gitea's EditUserOption has historically treated them as required, and + omitting them has been reported to move a local account onto a different + authentication source. For a local user they are the username and 0, so + sending them is a no-op that avoids the question. + """ + token = current_app.config.get("ADMIN_TOKEN") + if not token: + raise GiteaError( + "Falta GITEA_ADMIN_TOKEN: el servidor no puede cambiar contraseñas." + ) + response = requests.patch( + _api(f"/admin/users/{quote(login, safe='')}"), + headers={"Authorization": f"token {token}"}, + json={ + "password": password, + "must_change_password": False, + "login_name": login, + "source_id": 0, + }, + timeout=TIMEOUT, + ) + if response.status_code in (200, 201): + return + if response.status_code == 422: + # Gitea enforces its own minimum length and complexity, and its message + # is in the admin's language rather than the member's, so it is not + # passed through. + raise GiteaError("Gitea rechazó esa contraseña. Prueba con una más larga.") + if response.status_code in (401, 403): + raise GiteaError("El token de administración de Gitea no es válido.") + raise GiteaError(f"Gitea rechazó el cambio de contraseña ({response.status_code}).") + + # --- content (the member's own token) ------------------------------------ def _contents_url(path: str) -> str: diff --git a/apps/board/invites.py b/apps/board/invites.py new file mode 100644 index 0000000..fab9329 --- /dev/null +++ b/apps/board/invites.py @@ -0,0 +1,105 @@ +"""One-time links for setting a password. + +Used for two things that are the same mechanism with different clocks: the +invitation a new member gets, and the reset an existing one asks for. + +A token is a bearer credential — whoever holds it can set the password on that +account — so the rules are deliberately strict: single use, short-lived, and +only ever stored as a hash. +""" + +from __future__ import annotations + +import hashlib +import secrets +from datetime import datetime, timedelta, timezone + +from .db import get_db + +INVITE_LIFETIME = timedelta(days=7) +RESET_LIFETIME = timedelta(hours=1) + +# How many resets one member may ask for before the rest are quietly dropped. +# Without this, the "forgot password" form is a way to mail-bomb somebody using +# your server's good name. +RESET_WINDOW = timedelta(minutes=15) +RESET_LIMIT = 3 + + +def _hash(token: str) -> str: + return hashlib.sha256(token.encode("utf-8")).hexdigest() + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +def issue(member_id: int, purpose: str) -> str: + """Create a token, store its hash, return the token itself — once. + + Any earlier unused token for the same member and purpose is marked used. + A member who asks for a second reset should not leave a first one lying in + an inbox, still working. + """ + lifetime = INVITE_LIFETIME if purpose == "invite" else RESET_LIFETIME + token = secrets.token_urlsafe(32) + db = get_db() + db.execute( + """UPDATE invites SET used_at = datetime('now') + WHERE member_id = ? AND purpose = ? AND used_at IS NULL""", + (member_id, purpose), + ) + db.execute( + """INSERT INTO invites (member_id, token_hash, purpose, expires_at) + VALUES (?, ?, ?, ?)""", + (member_id, _hash(token), purpose, (_now() + lifetime).isoformat()), + ) + return token + + +def rate_limited(member_id: int) -> bool: + """Compared entirely inside SQLite, on purpose. + + `created_at` is written by SQLite's own `datetime('now')`, which formats as + `2026-09-25 15:00:00` — a space, no offset. Python's `.isoformat()` produces + `2026-09-25T15:00:00+00:00`. Compared as strings, a space sorts before `T`, + so every stored row looks older than any Python-generated threshold and the + limit silently never fires. Letting SQLite compare its own format to its own + clock keeps the two conventions from ever meeting. + """ + row = get_db().execute( + """SELECT COUNT(*) AS n FROM invites + WHERE member_id = ? AND purpose = 'reset' + AND created_at > datetime('now', ?)""", + (member_id, f"-{int(RESET_WINDOW.total_seconds() // 60)} minutes"), + ).fetchone() + return row["n"] >= RESET_LIMIT + + +def lookup(token: str): + """The member this token belongs to, or None if it is no good. + + One return value for every kind of failure — unknown, used, expired — so a + caller cannot accidentally tell the holder which it was. + """ + if not token: + return None + row = get_db().execute( + """SELECT i.id AS invite_id, i.purpose, i.expires_at, m.* + FROM invites i JOIN members m ON m.id = i.member_id + WHERE i.token_hash = ? AND i.used_at IS NULL""", + (_hash(token),), + ).fetchone() + if row is None: + return None + if datetime.fromisoformat(row["expires_at"]) <= _now(): + return None + if not row["active"] or row["role"] == "tombstone": + return None + return row + + +def consume(invite_id: int) -> None: + get_db().execute( + "UPDATE invites SET used_at = datetime('now') WHERE id = ?", (invite_id,) + ) diff --git a/apps/board/mail.py b/apps/board/mail.py new file mode 100644 index 0000000..1b84a19 --- /dev/null +++ b/apps/board/mail.py @@ -0,0 +1,114 @@ +"""Sending mail, through the association's own mailbox. + +`smtplib` from the standard library rather than a mail framework: this sends +two kinds of message, both short and both plain text. A dependency would buy +templating and queueing that nothing here asks for. + +Plain text only, no HTML alternative. An invitation is a sentence and a link — +HTML would add a second body to keep in step with the first, another place for +an escaping mistake, and a slightly worse chance of landing in the inbox rather +than the spam folder. +""" + +from __future__ import annotations + +import smtplib +import ssl +from email.message import EmailMessage + +from flask import current_app + + +class MailNotConfigured(RuntimeError): + """No SMTP host is set, so this server cannot send anything.""" + + +class MailFailed(RuntimeError): + """The mail server refused or could not be reached.""" + + +def configured() -> bool: + return bool(current_app.config.get("MAIL_HOST")) + + +def send(to: str, subject: str, body: str) -> None: + if not configured(): + raise MailNotConfigured( + "No hay servidor de correo configurado (MAIL_HOST)." + ) + + config = current_app.config + message = EmailMessage() + message["From"] = config["MAIL_FROM"] + message["To"] = to + message["Subject"] = subject + message.set_content(body) + + host, port = config["MAIL_HOST"], int(config["MAIL_PORT"]) + security = (config.get("MAIL_SECURITY") or "starttls").lower() + + try: + # 465 speaks TLS from the first byte; 587 starts in the clear and + # upgrades. Getting this pair wrong is the usual reason a mailbox that + # works in a mail client fails here, so it is configuration rather than + # a guess from the port number. + if security == "ssl": + server = smtplib.SMTP_SSL(host, port, timeout=20, + context=ssl.create_default_context()) + else: + server = smtplib.SMTP(host, port, timeout=20) + + with server: + if security == "starttls": + server.starttls(context=ssl.create_default_context()) + if config.get("MAIL_USER"): + server.login(config["MAIL_USER"], config["MAIL_PASSWORD"]) + server.send_message(message) + except (smtplib.SMTPException, OSError, ssl.SSLError) as exc: + # The caller decides what to do about it — for an invitation that means + # showing the admin the link so the member is not stranded. + current_app.logger.warning("Mail to %s failed: %s", to, exc) + raise MailFailed(str(exc)) from exc + + +def send_invite(to: str, name: str, url: str) -> None: + send( + to, + "Tu acceso a Viena Latina", + f"""Hola {name}, + +Te damos de alta en el área de la comunidad de Viena Latina. + +Elige tu contraseña aquí: + +{url} + +El enlace sirve una sola vez y caduca en 7 días. Si caduca, pide a un +administrador que te envíe uno nuevo. + +Si no esperabas este correo, puedes ignorarlo. + +— Viena Latina +""", + ) + + +def send_reset(to: str, name: str, url: str) -> None: + send( + to, + "Restablecer tu contraseña — Viena Latina", + f"""Hola {name}, + +Alguien pidió restablecer la contraseña de tu cuenta. + +Si fuiste tú, elige una nueva aquí: + +{url} + +El enlace sirve una sola vez y caduca en 1 hora. + +Si no fuiste tú, ignora este correo: tu contraseña no ha cambiado. + +— Viena Latina +""", + ) diff --git a/apps/board/members.py b/apps/board/members.py index 3a6876d..e4d58f7 100644 --- a/apps/board/members.py +++ b/apps/board/members.py @@ -21,7 +21,7 @@ import sqlite3 from flask import (Blueprint, Response, abort, current_app, flash, g, redirect, render_template, request, url_for) -from . import gitea +from . import auth, gitea, invites, mail from .db import TOMBSTONE_LOGIN, get_db from .security import admin_required, login_required, owner_required @@ -152,17 +152,20 @@ def new(): flash("Ese usuario ya es miembro.", "error") return redirect(url_for("members.new")) - password = None if create_account: - password = gitea.generate_password() + # A random password nobody ever sees, not even the admin creating the + # account. It exists only so the Gitea account is not passwordless + # until the invitation is used — and because nobody knows it, the + # invitation is the only way in, which is the point. try: - gitea.admin_create_user(login, email, display_name or login, password) + gitea.admin_create_user(login, email, display_name or login, + gitea.generate_password()) except gitea.GiteaError as exc: flash(str(exc), "error") return redirect(url_for("members.new")) try: - db.execute( + cursor = db.execute( """INSERT INTO members (gitea_login, display_name, email, role, created_by) VALUES (?, ?, ?, ?, ?)""", (login, display_name or login, email, role, g.member["id"]), @@ -171,8 +174,20 @@ def new(): flash("No se pudo dar de alta a ese miembro.", "error") return redirect(url_for("members.new")) - # Shown once and never stored: Gitea has the hash, we have nothing. - return render_template("member_created.html", login=login, password=password, + invite_link = None + if create_account: + link = auth.invite_url(invites.issue(cursor.lastrowid, "invite")) + try: + mail.send_invite(email, display_name or login, link) + except (mail.MailFailed, mail.MailNotConfigured): + # The account exists and the member cannot reach it. Showing the + # admin the link is the difference between a delayed invitation and + # a person who simply never gets in. + invite_link = link + + return render_template("member_created.html", login=login, + email=email, created=create_account, + invite_link=invite_link, role_label=ROLE_LABELS[role]) diff --git a/apps/board/schema.sql b/apps/board/schema.sql index 873b030..4be14bd 100644 --- a/apps/board/schema.sql +++ b/apps/board/schema.sql @@ -89,3 +89,26 @@ CREATE TABLE IF NOT EXISTS content_cache ( generated INTEGER NOT NULL DEFAULT 0 CHECK (generated IN (0, 1)), updated_at TEXT NOT NULL DEFAULT (datetime('now')) ); + +-- One-time links: invitations to set a first password, and password resets. +-- +-- A token here is enough to take over an account, so only its SHA-256 lives in +-- this table. A database backup that leaks is then a list of useless hashes +-- rather than a set of live keys. +-- +-- SHA-256 rather than a password hash on purpose: these are 32 random bytes +-- from secrets.token_urlsafe, not something a person chose. There is no +-- dictionary to run against them, so the slow hashing that protects weak +-- passwords buys nothing and costs a round trip on every click. +CREATE TABLE IF NOT EXISTS invites ( + id INTEGER PRIMARY KEY, + member_id INTEGER NOT NULL REFERENCES members(id) ON DELETE CASCADE, + token_hash TEXT NOT NULL UNIQUE, + purpose TEXT NOT NULL CHECK (purpose IN ('invite', 'reset')), + created_at TEXT NOT NULL DEFAULT (datetime('now')), + expires_at TEXT NOT NULL, + used_at TEXT +); + +CREATE INDEX IF NOT EXISTS invites_open + ON invites(member_id, purpose) WHERE used_at IS NULL; diff --git a/apps/board/templates/login.html b/apps/board/templates/login.html index 156e005..cc7d0a1 100644 --- a/apps/board/templates/login.html +++ b/apps/board/templates/login.html @@ -9,6 +9,9 @@ cuenta que se usa para publicar en el sitio.

Entrar con Gitea +

+ ¿Olvidaste tu contraseña? +

¿No tienes cuenta? Pídesela a un administrador: las cuentas se crean a mano, no hay registro abierto. diff --git a/apps/board/templates/member_created.html b/apps/board/templates/member_created.html index ecb7bcb..6b8bad9 100644 --- a/apps/board/templates/member_created.html +++ b/apps/board/templates/member_created.html @@ -5,19 +5,33 @@

{{ login }} ya es {{ role_label|lower }}

- {% if password %} -

Esta contraseña se muestra una sola vez. Cópiala ahora y - entrégasela en persona o por un canal privado.

- -

{{ password }}

- -

- No se guarda en ningún sitio: el servidor sólo conserva el hash, igual que - con cualquier contraseña. Si se pierde, hay que restablecerla desde Gitea. - La persona tendrá que cambiarla la primera vez que entre. -

- {% else %} + {% if not created %}

No se creó ninguna cuenta nueva: ya existía. Puede entrar con la que tenía.

+ + {% elif invite_link %} + {# The account exists but the invitation never left the building. Showing the + link is the difference between a delayed invitation and a person who + simply never gets in. #} +

+ No se pudo enviar el correo. Pásale este enlace por un canal privado — + sirve una sola vez y caduca en 7 días. +

+

{{ invite_link }}

+

+ Revisa la configuración de correo del servidor para que la próxima + invitación salga sola. +

+ + {% else %} +

+ Le enviamos un correo a {{ email }} con un enlace + para elegir su contraseña. +

+

+ El enlace caduca en 7 días y sirve una sola vez. Nadie conoce su contraseña + — ni tú ni el servidor — hasta que la elija. Si no llega, que mire en spam, + o pídele que use «¿olvidaste tu contraseña?». +

{% endif %}
diff --git a/apps/board/templates/recover.html b/apps/board/templates/recover.html new file mode 100644 index 0000000..b575393 --- /dev/null +++ b/apps/board/templates/recover.html @@ -0,0 +1,34 @@ +{% extends "base.html" %} +{% block title %}Recuperar contraseña{% endblock %} + +{% block main %} +{% if sent %} +
+

Revisa tu correo

+ {# Says the same thing whether or not that address belongs to a member. + Confirming it would turn this form into a way to find out who is one. #} +

+ Si esa dirección pertenece a un miembro, le enviamos un enlace para elegir + una contraseña nueva. Caduca en una hora. +

+

Volver a entrar

+
+{% else %} +
+ +

¿Olvidaste tu contraseña?

+

+ Escribe el correo con el que te dieron de alta y te enviamos un enlace para + elegir una nueva. +

+ + + + +
+ + Cancelar +
+
+{% endif %} +{% endblock %} diff --git a/apps/board/templates/set_password.html b/apps/board/templates/set_password.html new file mode 100644 index 0000000..cb7c140 --- /dev/null +++ b/apps/board/templates/set_password.html @@ -0,0 +1,39 @@ +{% extends "base.html" %} +{% block title %}Elige tu contraseña{% endblock %} + +{% block main %} +{% if member is none %} +{# One message for expired, already used and never existed alike. Which one it + was would tell whoever holds a stale link something about the account. #} +
+

Este enlace ya no sirve

+

+ Los enlaces caducan y sólo se pueden usar una vez. Pide uno nuevo desde + ¿olvidaste tu contraseña?, + o a un administrador si aún no habías entrado nunca. +

+
+{% else %} +
+ +

Hola, {{ member.display_name }}

+

+ Elige una contraseña para entrar al área de la comunidad. Tu usuario es + {{ member.gitea_login }}. +

+ + + +

Al menos {{ minimum }} caracteres.

+ + + + +
+ +
+
+{% endif %} +{% endblock %} diff --git a/apps/board/tests/conftest.py b/apps/board/tests/conftest.py index 0e62710..049f6eb 100644 --- a/apps/board/tests/conftest.py +++ b/apps/board/tests/conftest.py @@ -83,6 +83,11 @@ def post(client): """POST with a valid CSRF token, so tests exercise authorisation rather than repeatedly rediscovering that the CSRF hook works.""" def _post(url, data=None, **kwargs): + # A real anonymous visitor gets a CSRF token when the form renders, so + # signed-out pages (invitations, password recovery) need one here too — + # otherwise every such test fails on the hook rather than on its subject. + with client.session_transaction() as session: + session.setdefault("csrf", "token-for-tests") payload = dict(data or {}) payload.setdefault("csrf_token", "token-for-tests") return client.post(url, data=payload, **kwargs) diff --git a/apps/board/tests/test_invites.py b/apps/board/tests/test_invites.py new file mode 100644 index 0000000..6e16f97 --- /dev/null +++ b/apps/board/tests/test_invites.py @@ -0,0 +1,264 @@ +"""Invitations, password resets, and the rules that keep a link from being a +permanent key to somebody's account. + +A token here is a bearer credential: whoever holds it sets the password. So +most of these tests are about the ways a token must *stop* working, and about +what the pages give away to somebody who is only guessing. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +import pytest + +from apps.board import gitea, invites, mail + + +@pytest.fixture +def outbox(monkeypatch): + """Mail captured rather than sent. No SMTP anywhere in the suite.""" + sent = [] + monkeypatch.setattr(mail, "send", lambda to, subject, body: sent.append( + {"to": to, "subject": subject, "body": body})) + return sent + + +@pytest.fixture +def passwords(monkeypatch): + """Gitea's password API stubbed; the calls are what matters.""" + changed = [] + monkeypatch.setattr(gitea, "admin_set_password", + lambda login, password: changed.append((login, password))) + return changed + + +def link_in(message: str) -> str: + for word in message.split(): + if "/comunidad/invitacion/" in word: + return word + raise AssertionError("no invite link in the message") + + +def token_in(message: str) -> str: + return link_in(message).rsplit("/", 1)[1] + + +# --- the tokens themselves ------------------------------------------------ + +def test_the_database_never_holds_the_token_itself(app, db, make_member): + """A leaked backup should be a list of useless hashes, not live keys.""" + with app.test_request_context(): + member_id = make_member("maria") + token = invites.issue(member_id, "invite") + + stored = db.execute("SELECT token_hash FROM invites").fetchone()["token_hash"] + assert token not in stored + assert len(stored) == 64 # sha256 hex, not the 43-char token + + +def test_a_token_works_once(app, db, make_member): + with app.test_request_context(): + member_id = make_member("maria") + token = invites.issue(member_id, "invite") + + found = invites.lookup(token) + assert found["id"] == member_id + + invites.consume(found["invite_id"]) + assert invites.lookup(token) is None + + +def test_an_expired_token_is_refused(app, db, make_member): + with app.test_request_context(): + member_id = make_member("maria") + token = invites.issue(member_id, "reset") + db.execute( + "UPDATE invites SET expires_at = ? WHERE member_id = ?", + ((datetime.now(timezone.utc) - timedelta(minutes=1)).isoformat(), member_id), + ) + assert invites.lookup(token) is None + + +def test_issuing_a_new_token_kills_the_old_one(app, db, make_member): + """Asking for a second reset should not leave the first one live in an + inbox somebody else can read.""" + with app.test_request_context(): + member_id = make_member("maria") + first = invites.issue(member_id, "reset") + second = invites.issue(member_id, "reset") + + assert invites.lookup(first) is None + assert invites.lookup(second) is not None + + +def test_a_suspended_member_cannot_use_their_link(app, db, make_member): + with app.test_request_context(): + member_id = make_member("expulsada") + token = invites.issue(member_id, "invite") + db.execute("UPDATE members SET active = 0 WHERE id = ?", (member_id,)) + assert invites.lookup(token) is None + + +def test_a_made_up_token_is_refused(app): + with app.test_request_context(): + assert invites.lookup("not-a-real-token") is None + assert invites.lookup("") is None + + +# --- setting the password ------------------------------------------------- + +def test_a_member_sets_their_own_password(app, client, db, post, make_member, passwords): + with app.test_request_context(): + member_id = make_member("maria") + token = invites.issue(member_id, "invite") + + response = post(f"/comunidad/invitacion/{token}", + {"password": "una-contrasena-larga", "confirm": "una-contrasena-larga"}) + + assert response.status_code == 302 + assert passwords == [("maria", "una-contrasena-larga")] + assert db.execute("SELECT used_at FROM invites").fetchone()["used_at"] is not None + + +def test_a_short_password_is_refused_and_the_link_survives( + app, client, db, post, make_member, passwords): + """Rejecting the password must not spend the token, or a typo locks the + member out of an account they have never reached.""" + with app.test_request_context(): + member_id = make_member("maria") + token = invites.issue(member_id, "invite") + + response = post(f"/comunidad/invitacion/{token}", + {"password": "corta", "confirm": "corta"}) + + assert response.status_code == 400 + assert passwords == [] + assert db.execute("SELECT used_at FROM invites").fetchone()["used_at"] is None + assert client.get(f"/comunidad/invitacion/{token}").status_code == 200 + + +def test_mismatched_passwords_are_refused(app, post, make_member, passwords): + with app.test_request_context(): + token = invites.issue(make_member("maria"), "invite") + + response = post(f"/comunidad/invitacion/{token}", + {"password": "una-contrasena-larga", "confirm": "otra-cosa-larga"}) + assert response.status_code == 400 + assert passwords == [] + + +def test_a_rejection_from_gitea_leaves_the_link_usable( + app, db, post, make_member, monkeypatch): + def refuse(login, password): + raise gitea.GiteaError("Gitea rechazó esa contraseña.") + monkeypatch.setattr(gitea, "admin_set_password", refuse) + + with app.test_request_context(): + token = invites.issue(make_member("maria"), "invite") + + response = post(f"/comunidad/invitacion/{token}", + {"password": "una-contrasena-larga", "confirm": "una-contrasena-larga"}) + assert response.status_code == 400 + assert db.execute("SELECT used_at FROM invites").fetchone()["used_at"] is None + + +def test_a_dead_link_says_nothing_about_the_account(client): + body = client.get("/comunidad/invitacion/inventado").get_data(as_text=True) + assert "ya no sirve" in body + # Not "expired", not "already used", not "unknown" — those distinctions tell + # the holder of a stale link something about the account behind it. + assert "caducado" not in body + + +# --- recovery ------------------------------------------------------------- + +def test_recovery_emails_a_member(app, client, post, db, make_member, outbox): + make_member("maria") + response = post("/comunidad/recuperar", {"email": "maria@example.com"}) + + assert response.status_code == 200 + assert len(outbox) == 1 + assert outbox[0]["to"] == "maria@example.com" + with app.test_request_context(): + assert invites.lookup(token_in(outbox[0]["body"])) is not None + + +def test_recovery_answers_the_same_for_an_unknown_address(client, post, outbox): + """Otherwise the form is a way to find out who is a member, one address at + a time.""" + known = post("/comunidad/recuperar", {"email": "maria@example.com"}) + unknown = post("/comunidad/recuperar", {"email": "nadie@example.com"}) + + assert known.status_code == unknown.status_code == 200 + assert known.get_data() == unknown.get_data() + assert outbox == [] + + +def test_recovery_stops_after_a_few_tries(client, post, make_member, outbox): + """A reset form with no limit is a way to mail-bomb somebody using your + server's reputation.""" + make_member("maria") + for _ in range(6): + post("/comunidad/recuperar", {"email": "maria@example.com"}) + + assert len(outbox) == invites.RESET_LIMIT + + +def test_a_suspended_member_gets_no_reset(client, post, make_member, outbox): + make_member("expulsada", active=0) + post("/comunidad/recuperar", {"email": "expulsada@example.com"}) + assert outbox == [] + + +# --- inviting from the members screen ------------------------------------- + +def test_creating_a_member_emails_them_instead_of_showing_a_password( + app, client, post, owner_id, sign_in, outbox, monkeypatch): + monkeypatch.setattr(gitea, "admin_create_user", + lambda login, email, name, password: None) + sign_in(owner_id) + + response = post("/comunidad/miembros/nuevo", { + "login": "maria", "display_name": "María", "email": "m@example.com", + "role": "user", "create_account": "on", + }) + page = response.get_data(as_text=True) + + assert len(outbox) == 1 + assert outbox[0]["to"] == "m@example.com" + assert "m@example.com" in page + # The admin never sees a password, so there is none to pass on or mislay. + assert "/comunidad/invitacion/" not in page + + +def test_when_mail_fails_the_admin_is_given_the_link( + app, client, post, owner_id, sign_in, monkeypatch): + """Otherwise the account exists and the member simply never gets in.""" + monkeypatch.setattr(gitea, "admin_create_user", + lambda login, email, name, password: None) + + def explode(to, subject, body): + raise mail.MailFailed("connection refused") + monkeypatch.setattr(mail, "send", explode) + sign_in(owner_id) + + response = post("/comunidad/miembros/nuevo", { + "login": "maria", "display_name": "María", "email": "m@example.com", + "role": "user", "create_account": "on", + }) + page = response.get_data(as_text=True) + + assert "No se pudo enviar el correo" in page + assert "/comunidad/invitacion/" in page + + +def test_linking_an_existing_account_sends_nothing( + app, client, post, owner_id, sign_in, outbox): + """They already have a password; an unexpected invitation would be noise.""" + sign_in(owner_id) + post("/comunidad/miembros/nuevo", { + "login": "maria", "display_name": "María", "email": "m@example.com", + "role": "user", "create_account": "", + }) + assert outbox == [] diff --git a/apps/board/tests/test_members.py b/apps/board/tests/test_members.py index d488f99..13f4e64 100644 --- a/apps/board/tests/test_members.py +++ b/apps/board/tests/test_members.py @@ -101,18 +101,26 @@ def test_an_admin_cannot_demote_another_admin(client, post, make_member, sign_in assert response.status_code == 403 -def test_creating_a_user_shows_the_password_once(client, monkeypatch, post, owner_id, sign_in): +def test_the_password_is_never_shown_to_the_admin( + client, monkeypatch, post, owner_id, sign_in): + """It used to be, printed once for the admin to pass on. Now the member is + emailed a link and chooses their own, so the generated password exists only + to keep the Gitea account from being reachable before they do — and nobody, + the admin included, ever learns it.""" created = {} monkeypatch.setattr(gitea, "admin_create_user", lambda login, email, name, password: created.update( login=login, password=password)) + monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None) sign_in(owner_id) + response = post("/comunidad/miembros/nuevo", { "login": "maria", "display_name": "María", "email": "m@example.com", "role": "user", "create_account": "on", }) + assert created["login"] == "maria" - assert created["password"].encode() in response.data + assert created["password"].encode() not in response.data def test_a_rejected_gitea_call_creates_no_member(client, monkeypatch, db, post, owner_id, sign_in): diff --git a/docs/server-setup.md b/docs/server-setup.md index 5ad53e8..4fb9f6d 100644 --- a/docs/server-setup.md +++ b/docs/server-setup.md @@ -579,3 +579,85 @@ So the logout page says plainly what is and is not closed, and offers the link that finishes the job. On a shared computer, use it — or close the browser, which also works. The members-area cookie is already a browser-session cookie, so it does not survive that either way. + +## 13. Email: invitations and passwords + +Until this is configured, an admin can add members but **nobody else can get +in**. The password was shown once to the admin, and Gitea's *Forgot password* +answers "Account recovery is disabled because no email is set up". That was the +state the members area shipped in; this section is what fixes it. + +### 13.1 What happens now + +An admin enters a username and an email. The server creates the Gitea account +with a random password **nobody ever sees, the admin included**, and emails the +member a link. The link opens a Viena Latina page where they choose their own +password, and only then can the account be used. + +The same mechanism powers *¿Olvidaste tu contraseña?* on the sign-in page, which +replaces Gitea's dead recovery page. Nobody leaves the site for either. + +### 13.2 SMTP settings + +These are the details of the `hola@vienalatina.com` mailbox. If that mailbox is +part of the old Hetzner shared hosting, they are in the Konsole panel under the +email account. + +```sh +sudo nano /srv/board/.env +``` + +``` +MAIL_HOST= +MAIL_PORT=587 +MAIL_SECURITY=starttls +MAIL_USER=hola@vienalatina.com +MAIL_PASSWORD= +MAIL_FROM=Viena Latina +``` + +**Port and security go together.** 465 means `MAIL_SECURITY=ssl`; 587 means +`starttls`. Mismatching the pair is the usual reason a mailbox that works +perfectly in a mail client fails here, and the error it produces is a timeout +rather than anything that names the cause. + +```sh +cd /srv/board && sudo docker compose up -d --force-recreate +``` + +Test it by adding a member with an address you can read. If the mail cannot be +sent, the screen says so and shows you the invitation link to pass on by hand — +the account is created either way, so a mail problem delays somebody rather +than stranding them. + +### 13.3 What the links are, and why they expire + +A link is enough to set the password on that account, so it is treated as a +credential: + +- **single use** — following it and choosing a password spends it +- **invitations last 7 days, resets 1 hour** +- **only a hash is stored**, so a leaked database backup is a list of useless + hashes rather than a set of live keys +- asking for a new link **invalidates the previous one**, so an older email + sitting in an inbox stops working +- recovery answers identically for an address that belongs to a member and one + that does not, and stops after three attempts in fifteen minutes + +If a member says a link does not work, the fix is always to send another. There +is deliberately no way to find out *why* one failed from the page itself: that +distinction would tell whoever holds a stale link something about the account +behind it. + +### 13.4 The sign-in page loses two tabs + +`GITEA__openid__ENABLE_OPENID_SIGNIN=false` and +`GITEA__service__SHOW_REGISTRATION_BUTTON=false` in +`/srv/gitea/docker-compose.yml`. OpenID is sign-in with an external identity +URL, which nobody here will use, and the register button contradicts +`DISABLE_REGISTRATION` — it invited people to try something the server then +refused. + +```sh +cd /srv/gitea && sudo docker compose up -d +``` diff --git a/infra/board/.env.example b/infra/board/.env.example index 2c90d1d..08df4ed 100644 --- a/infra/board/.env.example +++ b/infra/board/.env.example @@ -18,3 +18,17 @@ BOARD_OWNER=pablo # the members area. Without it, everything works except account creation, and # admins add people who already have a Gitea login. GITEA_ADMIN_TOKEN= + +# Outgoing mail, for invitations and password resets. Without it an admin can +# still add members, but nobody can be invited and nobody can recover a +# password — which is the whole reason members could not get in before. +# +# These are the SMTP details of the hola@vienalatina.com mailbox. Port 465 means +# MAIL_SECURITY=ssl; port 587 means starttls. Using the wrong one of the pair is +# the usual reason a mailbox that works in a mail client fails here. +MAIL_HOST= +MAIL_PORT=587 +MAIL_SECURITY=starttls +MAIL_USER=hola@vienalatina.com +MAIL_PASSWORD= +MAIL_FROM=Viena Latina diff --git a/infra/gitea/docker-compose.yml b/infra/gitea/docker-compose.yml index 1398553..04f42e4 100644 --- a/infra/gitea/docker-compose.yml +++ b/infra/gitea/docker-compose.yml @@ -30,6 +30,15 @@ services: # visitors get, which is exactly those two pages. # Install the theme first: bash scripts/gitea-theme.sh - GITEA__ui__DEFAULT_THEME=vienalatina + + # Two tabs on the sign-in page that should not be offered here. + # OpenID is sign-in with an external identity URL, which nobody in + # this association will ever use; the register button contradicts + # DISABLE_REGISTRATION above, which is worse than useless — it invites + # people to try something the server then refuses. + - GITEA__openid__ENABLE_OPENID_SIGNIN=false + - GITEA__openid__ENABLE_OPENID_SIGNUP=false + - GITEA__service__SHOW_REGISTRATION_BUTTON=false volumes: - ./data:/data - /etc/timezone:/etc/timezone:ro