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 9406a1f..e4d58f7 100644 --- a/apps/board/members.py +++ b/apps/board/members.py @@ -18,10 +18,10 @@ 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 +from . import auth, gitea, invites, mail from .db import TOMBSTONE_LOGIN, get_db from .security import admin_required, login_required, owner_required @@ -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() @@ -143,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"]), @@ -162,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/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/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 cb12958..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): @@ -182,3 +190,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..4fb9f6d 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 @@ -574,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