diff --git a/apps/board/app.py b/apps/board/app.py
index 61b8bd9..2e8438e 100644
--- a/apps/board/app.py
+++ b/apps/board/app.py
@@ -45,9 +45,12 @@ def create_app(overrides: dict | None = None) -> Flask:
SECRET_KEY=os.environ.get("BOARD_SECRET_KEY", ""),
DB_PATH=os.environ.get("BOARD_DB", "/data/board.db"),
GITEA_URL=os.environ.get("GITEA_URL", "https://git.vienalatina.com"),
- OAUTH_CLIENT_ID=os.environ.get("BOARD_OAUTH_CLIENT_ID", ""),
- OAUTH_CLIENT_SECRET=os.environ.get("BOARD_OAUTH_CLIENT_SECRET", ""),
ADMIN_TOKEN=os.environ.get("GITEA_ADMIN_TOKEN", ""),
+ # What the editor commits with. Needs write access to one
+ # repository — not the admin token, which can create and modify
+ # every account on the instance. Falls back to it so nothing
+ # breaks on deploy, but the narrower token is the right one.
+ CONTENT_TOKEN=os.environ.get("CONTENT_TOKEN", ""),
OWNER_LOGIN=os.environ.get("BOARD_OWNER", ""),
BASE_URL=os.environ.get("BOARD_BASE_URL", "https://vienalatina.com"),
# The repository the editor commits to — the same one Woodpecker builds,
diff --git a/apps/board/auth.py b/apps/board/auth.py
index a073034..ec1fcea 100644
--- a/apps/board/auth.py
+++ b/apps/board/auth.py
@@ -1,27 +1,31 @@
-"""Sign-in through Gitea.
+"""Signing in, entirely on vienalatina.com.
-The rule this module exists to enforce: **a Gitea account is not a membership.**
-Gitea answers "who is this person"; the members table answers "may they be
-here". Conflating the two would admit every account on the instance, including
-the `vienalatina-translations` bot, and would mean anyone who ever gets a Gitea
-account for an unrelated reason silently gains access to the board.
+The rule this module exists to enforce: **a member row is the membership.**
+There is one place a person can be signed in, one session to end, and one
+password store, and all three are here.
+
+It used to be otherwise. Identity was delegated to the git server, which meant
+a member was handed to another domain to type their password, handed back, and
+could never really be signed out — that server owned the session and its logout
+cannot be triggered from here. Everything confusing about the old flow came
+from that one decision, so it was reversed. The git server is now what it
+should always have been: somewhere the site's content is stored, which members
+never see.
"""
from __future__ import annotations
-import secrets
-
from flask import (Blueprint, current_app, flash, g, redirect, render_template,
request, session, url_for)
-from . import gitea, invites, mail, tokens
+from . import invites, mail, passwords
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
+PASSWORD_MIN = passwords.MINIMUM
def redirect_uri() -> str:
@@ -50,92 +54,70 @@ def load_member() -> None:
g.member = row
-@bp.route("/login")
+def _safe_next(target: str) -> str:
+ """Only ever inside this app.
+
+ An absolute or protocol-relative URL here would make the login form an open
+ redirect: a crafted link signs somebody in and drops them on a page
+ somebody else controls, with the trust of having just arrived from their
+ own community site.
+ """
+ prefix = current_app.config["URL_PREFIX"] + "/"
+ return target if target.startswith(prefix) else url_for("board.threads")
+
+
+@bp.route("/login", methods=["GET", "POST"])
def login():
if g.member is not None:
return redirect(url_for("board.threads"))
- return render_template("login.html", next=request.args.get("next", ""),
- # No site-admin token means no password can be set,
- # so offering recovery here would only lead somebody
- # to a page that refuses.
- can_recover=gitea.admin_configured())
+ target = request.values.get("next", "")
+ if request.method == "GET":
+ return render_template("login.html", next=target)
-@bp.route("/login/start")
-def start():
- # State ties the callback to this browser session; without it, an attacker
- # can feed you their own authorization code and log you into their account.
- state = secrets.token_urlsafe(24)
- session["oauth_state"] = state
- session["oauth_next"] = request.args.get("next", "")
- return redirect(gitea.authorize_url(state, redirect_uri()))
+ identifier = request.form.get("identifier", "").strip()
+ password = request.form.get("password", "")
+ # One message for every way this can fail — unknown name, wrong password,
+ # suspended member, invited but never arrived. Distinguishing them turns
+ # the form into a way to find out who is a member, one guess at a time.
+ def refuse(reason: str):
+ current_app.logger.info("Sign-in refused for %r: %s", identifier, reason)
+ passwords.record_attempt(identifier)
+ flash("Usuario o contraseña incorrectos.", "error")
+ return render_template("login.html", next=target), 401
-@bp.route("/auth/callback")
-def callback():
- expected = session.pop("oauth_state", None)
- given = request.args.get("state")
- if not expected or not given or not secrets.compare_digest(expected, given):
- flash("El inicio de sesión no se pudo verificar. Inténtalo de nuevo.", "error")
- return redirect(url_for("auth.login"))
+ if not identifier or not password:
+ return refuse("empty")
+ if passwords.too_many_attempts(identifier):
+ # Said plainly rather than hidden behind the same message: somebody
+ # locked out by their own typing needs to know waiting will fix it.
+ flash("Demasiados intentos. Espera unos minutos y vuelve a probar.", "error")
+ return render_template("login.html", next=target), 429
- code = request.args.get("code", "")
- if not code:
- flash("No se recibió el código de autorización. Inténtalo de nuevo.", "error")
- return redirect(url_for("auth.login"))
-
- try:
- credentials = gitea.exchange_code(code, redirect_uri())
- profile = gitea.fetch_user(credentials["access_token"])
- except gitea.GiteaError as exc:
- current_app.logger.warning("OAuth failed: %s", exc)
- flash(str(exc), "error")
- return redirect(url_for("auth.login"))
- except Exception: # network trouble, malformed JSON, Gitea down
- current_app.logger.exception("OAuth failed unexpectedly")
- flash("No se pudo contactar con el servidor de cuentas. "
- "Inténtalo más tarde.", "error")
- return redirect(url_for("auth.login"))
-
- login_name = (profile.get("login") or "").strip()
- db = get_db()
- member = db.execute(
+ member = get_db().execute(
"""SELECT * FROM members
- WHERE gitea_login = ? AND active = 1 AND role IN ('owner', 'admin', 'user')""",
- (login_name,),
+ WHERE (gitea_login = ? COLLATE NOCASE OR email = ? COLLATE NOCASE)
+ AND role IN ('owner', 'admin', 'user')""",
+ (identifier, identifier),
).fetchone()
- if member is None:
- # Says nothing about whether the account exists, is inactive, or was
- # never a member: an outsider who reaches this page learns only that
- # they are not in.
- current_app.logger.info("Rejected sign-in for non-member %r", login_name)
- flash("Tu cuenta no tiene acceso a esta área. Pide a un administrador que te dé de alta.",
- "error")
- return redirect(url_for("auth.login"))
+ # verify() is called even when there is no member, against a decoy hash, so
+ # an unknown name does not answer faster than a wrong password.
+ if not passwords.verify(member["password_hash"] if member else None, password):
+ return refuse("bad credentials")
+ if not member["active"]:
+ return refuse("suspended")
- db.execute(
- """UPDATE members
- SET display_name = ?, email = ?, last_seen_at = datetime('now')
- WHERE id = ?""",
- (profile.get("full_name") or login_name, profile.get("email") or "", member["id"]),
- )
-
- # A fresh session id on privilege change, so a cookie captured before login
- # is not still valid after it.
+ # A fresh session id on privilege change, so a cookie captured before
+ # sign-in is not still valid after it.
session.clear()
session["member_id"] = member["id"]
-
- # Kept so the editor can commit as this person rather than as a bot. Stored
- # in the database, never in the cookie — see apps/board/tokens.py.
- tokens.save(member["id"], credentials)
-
- target = request.args.get("next") or session.pop("oauth_next", "") or ""
- # Only ever redirect within this app: an absolute URL here would make the
- # login page an open redirect that phishing can point anywhere.
- if not target.startswith(current_app.config["URL_PREFIX"] + "/"):
- target = url_for("board.threads")
- return redirect(target)
+ passwords.forget_attempts(identifier)
+ passwords.prune_attempts()
+ get_db().execute("UPDATE members SET last_seen_at = datetime('now') WHERE id = ?",
+ (member["id"],))
+ return redirect(_safe_next(target))
def invite_url(token: str) -> str:
@@ -152,16 +134,6 @@ def set_password(token: str):
link lived. The token is the only credential: somebody arriving here is not
signed in and cannot be.
"""
- if not gitea.admin_configured():
- # Checked before the form is drawn rather than when it is submitted.
- # The server cannot save the password either way — but learning that
- # after choosing one, typing it twice and pressing the button reads as
- # "I did something wrong", which is the opposite of true. Answering
- # this way gives nothing away: the refusal is about the server, not
- # about the token or any account behind it.
- return render_template("set_password.html", member=None, token=token,
- minimum=PASSWORD_MIN, unavailable=True), 503
-
member = invites.lookup(token)
if member is None:
# Deliberately one message for every reason it might fail — expired,
@@ -181,17 +153,14 @@ def set_password(token: str):
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"))
+ get_db().execute("UPDATE members SET password_hash = ? WHERE id = ?",
+ (passwords.hash_password(password), member["id"]))
+ # Only now: a token that set a password is spent, but one refused for
+ # being too short has to keep working or a typo locks the member out of
+ # an account they have never reached.
+ 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
@@ -200,15 +169,6 @@ def set_password(token: str):
@bp.route("/recuperar", methods=["GET", "POST"])
def recover():
"""Replaces Gitea's recovery page, which is dead without a mailer."""
- if not gitea.admin_configured():
- # A reset link leads to a page that sets a password through Gitea's
- # admin API. Without the token that page cannot save anything, so
- # sending the mail would put a dead link in somebody's inbox and — the
- # worse half — the identical answer below would hide that from
- # everyone, including the admin. Refuse out loud instead. This says
- # nothing about any account, only about the server.
- return render_template("recover.html", unavailable=True), 503
-
if request.method == "GET":
return render_template("recover.html")
@@ -235,25 +195,12 @@ def recover():
@bp.route("/logout", methods=["POST"])
def logout():
- # Drop the Gitea token too. Signing out should stop the server being able to
- # act as you, not just stop the browser being able to ask it to.
- if g.member is not None:
- tokens.forget(g.member["id"])
- session.clear()
+ """One click, and it is done.
- # Deliberately NOT a redirect back to the login page.
- #
- # Clearing this session does not touch the Gitea session in the same
- # browser, and Gitea remembers that this app was authorised. So the next
- # click on "Entrar con Gitea" gets a code back immediately and signs the
- # person straight back in without a password — which on a laptop shared
- # around an association means "Salir" was telling them something untrue.
- #
- # Gitea cannot be signed out from here: its logout has been POST-only since
- # 1.11.2, and a cross-site POST would need Gitea's CSRF token. `prompt=login`
- # would be the other way round it, and is undocumented in every released
- # version of Gitea's OAuth2 provider — not something to rest this on.
- #
- # So the honest thing is to say so and point at the one place that can
- # finish the job.
- return render_template("logged_out.html", gitea_url=current_app.config["GITEA_URL"].rstrip("/"))
+ This used to render a page explaining that signing out had not really
+ signed you out, because the session that mattered belonged to another
+ server we could not reach. There is only one session now.
+ """
+ session.clear()
+ flash("Has cerrado sesión.", "ok")
+ return redirect(url_for("auth.login"))
diff --git a/apps/board/content.py b/apps/board/content.py
index 5f9aec0..3db2212 100644
--- a/apps/board/content.py
+++ b/apps/board/content.py
@@ -27,10 +27,10 @@ from datetime import date as date_type
from datetime import datetime
import yaml
-from flask import (Blueprint, abort, current_app, flash, redirect,
+from flask import (Blueprint, abort, current_app, flash, g, redirect,
render_template, request, url_for)
-from . import gitea, tokens
+from . import gitea
from .db import get_db
from .render import to_html
from .security import admin_required
@@ -135,7 +135,7 @@ def _cache_write(path: str, sha: str, fm: dict) -> None:
def listing(collection: str) -> list[dict]:
folder = COLLECTIONS[collection]["folder"]
- entries = tokens.with_token(gitea.list_directory, folder)
+ entries = gitea.list_directory(folder, gitea.content_token())
items = []
for entry in entries:
@@ -146,7 +146,7 @@ def listing(collection: str) -> list[dict]:
row = _cache_read(path, sha)
if row is None:
- text, _ = tokens.with_token(gitea.read_file, path)
+ text, _ = gitea.read_file(path, gitea.content_token())
fm, _body = split_frontmatter(text)
_cache_write(path, sha, fm)
row = _cache_read(path, sha)
@@ -241,8 +241,9 @@ def _upload_image() -> str:
# A random suffix rather than a counter: two people uploading "foto.jpg"
# in the same minute must not race for the same path.
name = f"{stem}-{secrets.token_hex(3)}.{extension}"
- tokens.with_token(gitea.write_file, f"{UPLOAD_FOLDER}/{name}", data,
- f"content: subir {name}")
+ gitea.write_file(f"{UPLOAD_FOLDER}/{name}", data,
+ f"content: subir {name}", gitea.content_token(),
+ member=g.member)
return f"/uploads/{name}"
@@ -255,8 +256,6 @@ def index(collection: str = "post"):
meta = _collection_or_404(collection)
try:
items = listing(collection)
- except tokens.NeedsSignIn:
- return redirect(url_for("auth.login", next=request.path))
except gitea.GiteaError as exc:
flash(str(exc), "error")
items = []
@@ -284,10 +283,9 @@ def new(collection: str):
name = filename_for(collection, fields["title"], fields["date"])
path = f"{meta['folder']}/{name}"
document = build_document(frontmatter_for(collection, fields), body)
- tokens.with_token(gitea.write_file, path, document.encode("utf-8"),
- f"content: publicar «{fields['title']}»")
- except tokens.NeedsSignIn:
- return redirect(url_for("auth.login", next=request.path))
+ gitea.write_file(path, document.encode("utf-8"),
+ f"content: publicar «{fields['title']}»",
+ gitea.content_token(), member=g.member)
except gitea.GiteaError as exc:
return _back_to_form(collection, meta, fields, body, [str(exc)], None)
@@ -304,9 +302,7 @@ def edit(collection: str, name: str):
if request.method == "GET":
try:
- text, sha = tokens.with_token(gitea.read_file, path)
- except tokens.NeedsSignIn:
- return redirect(url_for("auth.login", next=request.path))
+ text, sha = gitea.read_file(path, gitea.content_token())
except gitea.GiteaError as exc:
flash(str(exc), "error")
return redirect(url_for("content.index", collection=collection))
@@ -341,10 +337,9 @@ def edit(collection: str, name: str):
if picture:
fields["image"] = picture
document = build_document(frontmatter_for(collection, fields), body)
- tokens.with_token(gitea.write_file, path, document.encode("utf-8"),
- f"content: actualizar «{fields['title']}»", sha=sha)
- except tokens.NeedsSignIn:
- return redirect(url_for("auth.login", next=request.path))
+ gitea.write_file(path, document.encode("utf-8"),
+ f"content: actualizar «{fields['title']}»",
+ gitea.content_token(), sha=sha, member=g.member)
except gitea.GiteaError as exc:
return _back_to_form(collection, meta, fields, body, [str(exc)], item)
@@ -359,10 +354,8 @@ def delete(collection: str, name: str):
name = _name_or_404(name)
path = f"{meta['folder']}/{name}"
try:
- _text, sha = tokens.with_token(gitea.read_file, path)
- tokens.with_token(gitea.delete_file, path, sha, f"content: eliminar {name}")
- except tokens.NeedsSignIn:
- return redirect(url_for("auth.login", next=request.path))
+ _text, sha = gitea.read_file(path, gitea.content_token())
+ gitea.delete_file(path, sha, f"content: eliminar {name}", gitea.content_token(), member=g.member)
except gitea.GiteaError as exc:
flash(str(exc), "error")
return redirect(url_for("content.index", collection=collection))
diff --git a/apps/board/gitea.py b/apps/board/gitea.py
index 9ca0198..2832cc6 100644
--- a/apps/board/gitea.py
+++ b/apps/board/gitea.py
@@ -1,21 +1,20 @@
-"""The only place that talks to Gitea.
+"""The only place that talks to the git server.
-Three unrelated conversations happen here, worth keeping apart in your head:
+**Sign-in is not here, and that is the point of the file.** Members have
+passwords in our own database and no account on this server at all. This talks
+to it about one thing: the repository the site is built from.
-* **Sign-in** uses OAuth2 on behalf of the person at the keyboard. The app is
- registered as a *confidential* client with a secret, which it can hold
- because it runs on the server. The Decap CMS app is the opposite — a public
- client using PKCE — because that one runs in the visitor's browser and has
- nowhere to keep a secret.
+It used to do much more. Identity was delegated here over OAuth2, each member
+had an account, and the editor committed with a token belonging to whoever was
+typing. That is what made signing in leave vienalatina.com and signing out
+impossible to finish, so it was taken back. What remains:
-* **Creating an account** uses a site-admin token belonging to the instance,
- not to any member. That token can create and modify any Gitea user, so the
- environment holding it is as sensitive as Gitea's own admin password.
-
-* **Reading and writing content** uses the signed-in member's *own* access
- token. Commits are then attributed to the person who actually wrote the post,
- and Gitea's permissions apply unchanged — the editor cannot grant write access
- to somebody who does not already have it.
+* **Content** — reading, writing and deleting files in the site repository,
+ with one server-side token (`content_token()`), naming the real author on
+ every commit so `git log` still says who wrote what.
+* **Accounts** — `admin_create_user` and `admin_set_password` still exist for
+ the handful of real git users (pablo, the pipeline bots). Nothing in the
+ members area calls them any more.
"""
from __future__ import annotations
@@ -62,63 +61,6 @@ def _branch() -> str:
# --- sign-in -------------------------------------------------------------
-def authorize_url(state: str, redirect_uri: str) -> str:
- query = urlencode({
- "client_id": current_app.config["OAUTH_CLIENT_ID"],
- "redirect_uri": redirect_uri,
- "response_type": "code",
- "state": state,
- })
- return f"{_base()}/login/oauth/authorize?{query}"
-
-
-def _token_request(payload: dict) -> dict:
- response = requests.post(
- f"{_base()}/login/oauth/access_token",
- json={
- "client_id": current_app.config["OAUTH_CLIENT_ID"],
- "client_secret": current_app.config["OAUTH_CLIENT_SECRET"],
- **payload,
- },
- timeout=TIMEOUT,
- )
- if response.status_code != 200:
- raise GiteaError("No se pudo completar el inicio de sesión.")
- data = response.json()
- if not data.get("access_token"):
- raise GiteaError("El servidor de cuentas no devolvió un token de acceso.")
- return data
-
-
-def exchange_code(code: str, redirect_uri: str) -> dict:
- """Returns the whole token response, not just the access token.
-
- The refresh token matters: Gitea's access tokens last about an hour, and
- without refreshing, saving a post would start failing partway through an
- afternoon's work for no reason the writer could understand.
- """
- return _token_request({
- "code": code,
- "grant_type": "authorization_code",
- "redirect_uri": redirect_uri,
- })
-
-
-def refresh_token(token: str) -> dict:
- return _token_request({"refresh_token": token, "grant_type": "refresh_token"})
-
-
-def fetch_user(token: str) -> dict:
- response = requests.get(
- _api("/user"),
- headers={"Authorization": f"Bearer {token}"},
- timeout=TIMEOUT,
- )
- if response.status_code != 200:
- raise GiteaError("No se pudo leer tu perfil desde el servidor de cuentas.")
- return response.json()
-
-
# --- account creation (site-admin token) ---------------------------------
def admin_configured() -> bool:
@@ -226,6 +168,41 @@ def _contents_url(path: str) -> str:
return _api(f"/repos/{_repo()}/contents/{quote(path, safe='/')}")
+def content_token() -> str:
+ """The token the editor commits with.
+
+ One service token instead of a token per writer. Members no longer have
+ accounts on the git server at all, so there is no per-member token to use —
+ and attribution does not need one, because each commit names its author
+ (see `_identity`).
+
+ CONTENT_TOKEN is preferred and GITEA_ADMIN_TOKEN is the fallback, so
+ nothing breaks on deploy. They can be the same, but they should not stay
+ that way: this needs write access to one repository, while the admin token
+ can create and modify every account on the instance.
+ """
+ token = (current_app.config.get("CONTENT_TOKEN")
+ or current_app.config.get("ADMIN_TOKEN") or "")
+ if not token:
+ raise GiteaError(
+ "Falta CONTENT_TOKEN: el servidor no puede guardar en el repositorio."
+ )
+ return token
+
+
+def _identity(member) -> dict:
+ """Who a commit is by.
+
+ Gitea's contents API takes `author` and `committer`, and uses the token's
+ own owner only when neither is given. So one token can commit as whoever
+ actually wrote the thing, and `git log` still says who to ask about a post.
+ """
+ name = (member["display_name"] or member["gitea_login"]) if member else "Viena Latina"
+ email = (member["email"] if member and member["email"]
+ else "hola@vienalatina.com")
+ return {"name": name, "email": email}
+
+
def _content_request(method: str, url: str, token: str, **kwargs):
response = requests.request(
method, url,
@@ -264,12 +241,14 @@ def read_file(path: str, token: str) -> tuple[str, str]:
def write_file(path: str, data: bytes, message: str, token: str,
- sha: str | None = None) -> str:
+ sha: str | None = None, member=None) -> str:
"""Create when `sha` is None, update otherwise. Returns the new sha."""
body = {
"content": base64.b64encode(data).decode("ascii"),
"message": message,
"branch": _branch(),
+ "author": _identity(member),
+ "committer": _identity(member),
}
if sha:
body["sha"] = sha
@@ -285,9 +264,10 @@ def write_file(path: str, data: bytes, message: str, token: str,
raise GiteaError(f"No se pudo guardar el archivo ({response.status_code}).")
-def delete_file(path: str, sha: str, message: str, token: str) -> None:
+def delete_file(path: str, sha: str, message: str, token: str, member=None) -> None:
response = _content_request("DELETE", _contents_url(path), token, json={
"sha": sha, "message": message, "branch": _branch(),
+ "author": _identity(member), "committer": _identity(member),
})
if response.status_code in (200, 204):
return
diff --git a/apps/board/members.py b/apps/board/members.py
index 3c3994d..d1d08fc 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 auth, gitea, invites, mail, uploads
+from . import auth, invites, mail, uploads
from .db import TOMBSTONE_LOGIN, get_db
from .security import admin_required, login_required, owner_required
@@ -194,50 +194,40 @@ def new():
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")),
- # Same courtesy for mail: without a server configured the invitation
- # cannot leave the box, and the admin has to pass the link on by
- # hand. Worth knowing before filling the form rather than after.
+ # Without mail the invitation cannot leave the building, so the
+ # admin has to pass the link on by hand. Worth knowing before
+ # filling the form rather than after.
can_send_mail=mail.configured(),
- gitea_url=current_app.config["GITEA_URL"].rstrip("/"),
)
login = request.form.get("login", "").strip()
display_name = request.form.get("display_name", "").strip()
email = request.form.get("email", "").strip()
role = request.form.get("role", "user")
- create_account = request.form.get("create_account") == "on"
if not may_create(g.member["role"], role):
abort(403)
if not LOGIN_RE.match(login):
flash("El usuario solo puede tener letras, números, punto, guion y guion bajo.", "error")
return redirect(url_for("members.new"))
- if create_account and "@" not in email:
- flash("Hace falta un correo válido para crear la cuenta.", "error")
+ if "@" not in email:
+ # Required now, not optional: the address is how the invitation gets
+ # there, and a member with no way to set a password is a row that can
+ # never be used.
+ flash("Hace falta un correo válido: ahí llega la invitación.", "error")
return redirect(url_for("members.new"))
db = get_db()
- if db.execute("SELECT 1 FROM members WHERE gitea_login = ?", (login,)).fetchone():
- flash("Ese usuario ya es miembro.", "error")
+ if db.execute("""SELECT 1 FROM members
+ WHERE gitea_login = ? COLLATE NOCASE
+ OR email = ? COLLATE NOCASE""",
+ (login, email)).fetchone():
+ # One message for either collision. Which of the two it was is not
+ # something an admin needs and not something worth leaking if this
+ # screen is ever opened by somebody it should not be.
+ flash("Ese usuario o ese correo ya están en uso.", "error")
return redirect(url_for("members.new"))
- if create_account:
- # 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,
- gitea.generate_password())
- except gitea.GiteaError as exc:
- flash(str(exc), "error")
- return redirect(url_for("members.new"))
-
try:
cursor = db.execute(
"""INSERT INTO members (gitea_login, display_name, email, role, created_by)
@@ -248,25 +238,21 @@ def new():
flash("No se pudo dar de alta a ese miembro.", "error")
return redirect(url_for("members.new"))
+ # No password is set here and none is generated. The member chooses their
+ # own through the invitation, and until they do, password_hash is NULL and
+ # cannot be signed in with.
invite_link = None
mail_problem = 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.MailNotConfigured:
- # Nothing is broken — this server has simply never been given a mail
- # server. Told apart from a failure on purpose: an admin sent looking
- # for an SMTP error that does not exist is an afternoon wasted.
- invite_link, mail_problem = link, "unconfigured"
- except mail.MailFailed:
- # 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, mail_problem = link, "failed"
+ link = auth.invite_url(invites.issue(cursor.lastrowid, "invite"))
+ try:
+ mail.send_invite(email, display_name or login, link)
+ except mail.MailNotConfigured:
+ invite_link, mail_problem = link, "unconfigured"
+ except mail.MailFailed:
+ invite_link, mail_problem = link, "failed"
return render_template("member_created.html", login=login,
- email=email, created=create_account,
+ email=email, created=True,
invite_link=invite_link, mail_problem=mail_problem,
role_label=ROLE_LABELS[role])
diff --git a/apps/board/migrations.py b/apps/board/migrations.py
index 3fc9622..3bb5bc0 100644
--- a/apps/board/migrations.py
+++ b/apps/board/migrations.py
@@ -86,10 +86,29 @@ def _attachments_accept_messages(db: sqlite3.Connection) -> None:
db.execute("CREATE INDEX IF NOT EXISTS attachments_message ON attachments(message_id)")
+def _members_own_their_passwords(db: sqlite3.Connection) -> None:
+ """Give members somewhere to keep a password, and drop the OAuth tokens.
+
+ A plain ADD COLUMN, with nothing like step 1's difficulty: `members` has no
+ CHECK constraint to collide with. Everyone's hash starts NULL, including
+ the owner's, and a NULL hash cannot be signed in with — so the way back in
+ is the invitation and reset machinery, which already works, or
+ scripts/set-password.sh when mail is having a bad day.
+
+ `gitea_tokens` holds OAuth access tokens for a flow that no longer exists.
+ They are not merely unused, they are credentials, and keeping credentials
+ that nothing can spend is a liability with no upside.
+ """
+ if "password_hash" not in _columns(db, "members"):
+ db.execute("ALTER TABLE members ADD COLUMN password_hash TEXT")
+ db.execute("DROP TABLE IF EXISTS gitea_tokens")
+
+
# (number, description, function). The number is the value written to
# user_version once the step succeeds.
STEPS = [
(1, "attachments can belong to a private message", _attachments_accept_messages),
+ (2, "members keep their own password", _members_own_their_passwords),
]
diff --git a/apps/board/passwords.py b/apps/board/passwords.py
new file mode 100644
index 0000000..6214ae1
--- /dev/null
+++ b/apps/board/passwords.py
@@ -0,0 +1,91 @@
+"""Storing and checking passwords, now that they are ours to keep.
+
+Until this existed the members area had no passwords: it asked another server
+whether somebody was who they said, which is why signing in meant leaving
+vienalatina.com and why signing out could never finish. Taking the job back
+means taking the responsibility with it, and the two things that go wrong are
+both in here: how the hash is made, and how many guesses a stranger gets.
+
+Hashing is `werkzeug.security`, which arrives with Flask — no new dependency,
+and BSD-3, which matters for a platform meant to be resold. Its default is
+scrypt with sensible parameters. Deliberately not a hand-rolled `hashlib`
+call: the parameters, the salting and the constant-time comparison are exactly
+the details worth not inventing.
+"""
+
+from __future__ import annotations
+
+from datetime import timedelta
+
+from werkzeug.security import check_password_hash, generate_password_hash
+
+from .db import get_db
+
+MINIMUM = 10
+
+# Guessing budget. Generous enough that nobody typing their own password badly
+# notices, small enough that a list of common passwords is not worth running.
+ATTEMPT_WINDOW = timedelta(minutes=15)
+ATTEMPT_LIMIT = 10
+
+# Compared against when there is no such member, so that a wrong username and a
+# wrong password take the same time to answer. Without it the reply comes back
+# measurably faster for a name that does not exist, and the login form becomes
+# a way to find out who is a member. The value is a real scrypt hash of a
+# string nobody will guess; what it hashes is irrelevant.
+_DECOY = generate_password_hash("no-such-member-" + "x" * 32)
+
+
+def hash_password(password: str) -> str:
+ return generate_password_hash(password)
+
+
+def verify(stored: str | None, password: str) -> bool:
+ """Check a password, spending the same time when there is nothing to check.
+
+ A member with no password yet — invited but never arrived — has NULL here.
+ That must never be treated as "matches anything", and it must not answer
+ faster than a real failure either.
+ """
+ if not stored:
+ check_password_hash(_DECOY, password)
+ return False
+ return check_password_hash(stored, password)
+
+
+def record_attempt(identifier: str) -> None:
+ get_db().execute(
+ "INSERT INTO login_attempts (identifier) VALUES (?)", (identifier.lower(),)
+ )
+
+
+def too_many_attempts(identifier: str) -> bool:
+ """Counted inside SQLite, for the reason written up in invites.py.
+
+ `created_at` is written by SQLite's own `datetime('now')` and compared
+ against SQLite's own clock. Handing it a Python timestamp instead puts two
+ formats on either side of a string comparison, and the limit silently never
+ fires — which is how the reset limiter was broken before anybody noticed.
+ """
+ minutes = int(ATTEMPT_WINDOW.total_seconds() // 60)
+ row = get_db().execute(
+ """SELECT COUNT(*) AS n FROM login_attempts
+ WHERE identifier = ? AND created_at > datetime('now', ?)""",
+ (identifier.lower(), f"-{minutes} minutes"),
+ ).fetchone()
+ return row["n"] >= ATTEMPT_LIMIT
+
+
+def forget_attempts(identifier: str) -> None:
+ """Called on a successful sign-in, so a member who mistyped four times and
+ then got it right does not carry those four into the next hour."""
+ get_db().execute("DELETE FROM login_attempts WHERE identifier = ?",
+ (identifier.lower(),))
+
+
+def prune_attempts() -> None:
+ """Old rows are of no interest to anyone and are a small record of who
+ tried to sign in and when. Dropped on each successful login rather than by
+ a scheduled job, because there is no scheduler here."""
+ get_db().execute(
+ "DELETE FROM login_attempts WHERE created_at < datetime('now', '-1 day')")
diff --git a/apps/board/schema.sql b/apps/board/schema.sql
index 41d53cb..c462034 100644
--- a/apps/board/schema.sql
+++ b/apps/board/schema.sql
@@ -12,6 +12,7 @@ CREATE TABLE IF NOT EXISTS members (
gitea_login TEXT NOT NULL UNIQUE COLLATE NOCASE,
display_name TEXT NOT NULL DEFAULT '',
email TEXT NOT NULL DEFAULT '',
+ password_hash TEXT,
role TEXT NOT NULL CHECK (role IN ('owner', 'admin', 'user', 'tombstone')),
active INTEGER NOT NULL DEFAULT 1 CHECK (active IN (0, 1)),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
@@ -55,6 +56,21 @@ CREATE TABLE IF NOT EXISTS comments (
CREATE INDEX IF NOT EXISTS comments_thread
ON comments(thread_id, created_at) WHERE deleted_at IS NULL;
+-- Failed sign-ins, kept only long enough to slow a guesser down.
+--
+-- The identifier is whatever was typed in the first box, lowercased — which
+-- may be a username, an address, or nonsense. It is deliberately not tied to a
+-- member row: the whole point is to count attempts against names that do not
+-- exist as well as ones that do.
+CREATE TABLE IF NOT EXISTS login_attempts (
+ id INTEGER PRIMARY KEY,
+ identifier TEXT NOT NULL,
+ created_at TEXT NOT NULL DEFAULT (datetime('now'))
+);
+
+CREATE INDEX IF NOT EXISTS login_attempts_recent
+ ON login_attempts(identifier, created_at);
+
-- Private messages between two members.
--
-- An inbox, not live chat: gunicorn's sync workers cannot hold a connection
@@ -144,20 +160,12 @@ CREATE INDEX IF NOT EXISTS attachments_comment ON attachments(comment_id);
-- column a migration introduces belongs in that migration, after the column
-- exists. See migrations.py.
--- Gitea access tokens for the editor.
---
--- Kept here rather than in the session cookie. Flask signs cookies but does not
--- encrypt them, so a live token sitting in one is readable by anything that can
--- read the cookie — and a token is enough to commit to the repository as its
--- owner. ON DELETE CASCADE ties the token to the membership: erasing a member
--- takes their token with it, with nothing to remember.
-CREATE TABLE IF NOT EXISTS gitea_tokens (
- member_id INTEGER PRIMARY KEY REFERENCES members(id) ON DELETE CASCADE,
- access_token TEXT NOT NULL,
- refresh_token TEXT NOT NULL DEFAULT '',
- expires_at TEXT,
- updated_at TEXT NOT NULL DEFAULT (datetime('now'))
-);
+-- There is no table here for the editor's credentials, and that is the point.
+-- Members sign in against password_hash above; the editor commits with one
+-- server-side token from the environment. The old gitea_tokens table held an
+-- OAuth access and refresh token per member, and migration 2 drops it — so it
+-- must not be recreated here, or every restart would put it back and the drop
+-- would only have worked once.
-- Frontmatter of content files, keyed by the git blob sha.
--
diff --git a/apps/board/templates/logged_out.html b/apps/board/templates/logged_out.html
deleted file mode 100644
index 16ed73b..0000000
--- a/apps/board/templates/logged_out.html
+++ /dev/null
@@ -1,38 +0,0 @@
-{# Shown instead of bouncing back to the login page, because bouncing back would
- hide the fact that one click gets you straight in again. #}
-{% extends "base.html" %}
-{% block title %}Sesión cerrada{% endblock %}
-
-{% block main %}
-
-
Saliste del área de la comunidad
-
-
- Tu sesión aquí está cerrada. Pero este navegador sigue conectado al
- servidor de cuentas, que es donde se guardan las contraseñas — así
- que quien use este ordenador después podría volver a entrar sin escribirla.
-
-
- {# There was a "Cerrar sesión del todo" button here that linked straight to
- /user/logout. It never worked: that route is POST-only, so the click was
- a GET and the account server answered 404 with the session untouched.
- It cannot be made to work from here either — signing out needs a CSRF
- token belonging to that other domain, which this page has no way to read,
- and that restriction is the whole point of the check. So the page gives
- the instruction instead of pretending to do it. #}
-
No se creó ninguna cuenta nueva: ya existía. Puede entrar con la que tenía.
-
- {% elif invite_link %}
+ {% if 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. #}
diff --git a/apps/board/templates/member_new.html b/apps/board/templates/member_new.html
index 4c7198b..8986eaa 100644
--- a/apps/board/templates/member_new.html
+++ b/apps/board/templates/member_new.html
@@ -15,29 +15,11 @@
-
+
+
Ahí llega la invitación para elegir contraseña.
- {% 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 (le falta el token de administración).
- Créala primero en el
- servidor de cuentas y luego
- da de alta aquí ese mismo usuario.
-
- {% endif %}
-
- {% if can_create_accounts and not can_send_mail %}
+ {% if not can_send_mail %}
{# Not an error: the account is still created and the invitation still
issued. But it leaves by hand, and finding that out after filling the
form is how an invitation ends up believed sent and never delivered. #}
diff --git a/apps/board/tests/conftest.py b/apps/board/tests/conftest.py
index 0d0a0b2..28bc8e7 100644
--- a/apps/board/tests/conftest.py
+++ b/apps/board/tests/conftest.py
@@ -31,8 +31,6 @@ def app(tmp_path):
"SESSION_COOKIE_SECURE": False,
"COOLDOWN_SECONDS": 0,
"BASE_URL": "http://localhost",
- "OAUTH_CLIENT_ID": "cid",
- "OAUTH_CLIENT_SECRET": "secret",
"ADMIN_TOKEN": "admintoken",
"UPLOAD_DIR": str(tmp_path / "uploads"),
"TESTING": True,
diff --git a/apps/board/tests/test_auth.py b/apps/board/tests/test_auth.py
index 19317c2..daad499 100644
--- a/apps/board/tests/test_auth.py
+++ b/apps/board/tests/test_auth.py
@@ -1,10 +1,16 @@
-"""Who gets in, and who does not."""
+"""Who gets in, and who does not.
+
+Signing in happens here now, on this domain, against a hash in our own
+database. These tests are mostly about the ways it must refuse, and about
+refusing them all in the same words — a login form that is more specific
+about failure is a way to find out who is a member.
+"""
from __future__ import annotations
import pytest
-from apps.board import gitea
+from apps.board import passwords
PROTECTED = [
"/comunidad/",
@@ -22,53 +28,109 @@ def test_anonymous_is_sent_to_login(client, path):
assert "/comunidad/login" in response.headers["Location"]
-def _stub_gitea(monkeypatch, login):
- # exchange_code returns the whole token response now, because the editor
- # needs the refresh token to keep working past Gitea's one-hour expiry.
- monkeypatch.setattr(gitea, "exchange_code", lambda code, uri: {
- "access_token": "token", "refresh_token": "refresh", "expires_in": 3600,
- })
- monkeypatch.setattr(gitea, "fetch_user", lambda token: {
- "login": login, "full_name": login.title(), "email": f"{login}@example.com",
- })
+def sign_up(db, make_member, login="maria", password="una-contrasena-larga", **kwargs):
+ """A member who has actually set a password, which is what most of these
+ need and what `make_member` alone does not give."""
+ member_id = make_member(login, **kwargs)
+ db.execute("UPDATE members SET password_hash = ? WHERE id = ?",
+ (passwords.hash_password(password), member_id))
+ return member_id
-def _callback(client, monkeypatch, login):
- _stub_gitea(monkeypatch, login)
- with client.session_transaction() as session:
- session["oauth_state"] = "state123"
- return client.get("/comunidad/auth/callback?code=abc&state=state123")
+def attempt(post, identifier, password, **kwargs):
+ return post("/comunidad/login",
+ {"identifier": identifier, "password": password}, **kwargs)
-def test_a_gitea_account_is_not_a_membership(client, monkeypatch):
- """The single most important rule in the app: Gitea says who you are, the
- members table says whether you belong. The translations bot has a perfectly
- valid Gitea account and must not get in."""
- response = _callback(client, monkeypatch, "vienalatina-translations")
+# --- getting in -----------------------------------------------------------
+
+def test_a_member_signs_in_with_their_password(client, db, post, make_member):
+ sign_up(db, make_member)
+ response = attempt(post, "maria", "una-contrasena-larga")
+
assert response.status_code == 302
+ with client.session_transaction() as session:
+ assert session["member_id"]
+
+
+def test_the_email_works_as_well_as_the_username(client, db, post, make_member):
+ sign_up(db, make_member)
+ assert attempt(post, "MARIA@example.com", "una-contrasena-larga").status_code == 302
+
+
+def test_signing_in_starts_a_new_session(client, db, post, make_member):
+ """A cookie captured before sign-in must not still be good after it."""
+ member_id = sign_up(db, make_member)
+ with client.session_transaction() as session:
+ session["planted"] = "before"
+
+ attempt(post, "maria", "una-contrasena-larga")
+
+ with client.session_transaction() as session:
+ assert session["member_id"] == member_id
+ assert "planted" not in session
+
+
+# --- and the ways it must not --------------------------------------------
+
+def test_every_refusal_reads_the_same(client, db, post, make_member):
+ """Unknown name, wrong password, suspended member, invited but never
+ arrived. Four different situations, one answer, because the difference
+ between them is exactly what an outsider would like to learn."""
+ sign_up(db, make_member, "maria")
+ sign_up(db, make_member, "expulsada", active=0)
+ make_member("invitada") # no password_hash at all
+
+ pages = [
+ attempt(post, "nadie", "una-contrasena-larga"),
+ attempt(post, "maria", "otra-contrasena"),
+ attempt(post, "expulsada", "una-contrasena-larga"),
+ attempt(post, "invitada", "una-contrasena-larga"),
+ ]
+
+ assert {page.status_code for page in pages} == {401}
+ assert len({page.get_data() for page in pages}) == 1
+
+
+def test_a_member_with_no_password_cannot_sign_in(client, db, post, make_member):
+ """NULL must never behave as "matches anything" — every member starts this
+ way, including the owner, the moment the column is added."""
+ make_member("invitada")
+ attempt(post, "invitada", "")
+ attempt(post, "invitada", "cualquier-cosa")
+
with client.session_transaction() as session:
assert "member_id" not in session
-def test_member_signs_in(client, monkeypatch, make_member):
- make_member("maria")
- response = _callback(client, monkeypatch, "maria")
- assert response.status_code == 302
- with client.session_transaction() as session:
- assert "member_id" in session
+def test_guessing_is_rate_limited(client, db, post, make_member):
+ sign_up(db, make_member)
+ for _ in range(passwords.ATTEMPT_LIMIT):
+ attempt(post, "maria", "mal")
-
-def test_suspended_member_cannot_sign_in(client, monkeypatch, make_member):
- make_member("expulsada", active=0)
- _callback(client, monkeypatch, "expulsada")
+ blocked = attempt(post, "maria", "una-contrasena-larga")
+ assert blocked.status_code == 429
with client.session_transaction() as session:
assert "member_id" not in session
+def test_getting_it_right_clears_the_count(client, db, post, make_member):
+ """Somebody who mistypes three times and then succeeds should not be part
+ way to a lockout for the rest of the afternoon."""
+ sign_up(db, make_member)
+ for _ in range(3):
+ attempt(post, "maria", "mal")
+ attempt(post, "maria", "una-contrasena-larga")
+
+ assert db.execute(
+ "SELECT COUNT(*) AS n FROM login_attempts WHERE identifier = 'maria'"
+ ).fetchone()["n"] == 0
+
+
def test_suspension_takes_effect_on_the_next_request(client, db, make_member, sign_in):
- """The role is read per request, not cached in the cookie, so revoking
- access does not wait for a session to expire."""
- member_id = make_member("temporal")
+ """Read from the database on every request rather than trusted from the
+ cookie, so removing somebody does not wait for their session to expire."""
+ member_id = make_member("maria")
sign_in(member_id)
assert client.get("/comunidad/").status_code == 200
@@ -76,76 +138,57 @@ def test_suspension_takes_effect_on_the_next_request(client, db, make_member, si
assert client.get("/comunidad/").status_code == 302
-def test_callback_rejects_a_mismatched_state(client, monkeypatch, make_member):
- make_member("maria")
- _stub_gitea(monkeypatch, "maria")
- with client.session_transaction() as session:
- session["oauth_state"] = "the-real-state"
- client.get("/comunidad/auth/callback?code=abc&state=attacker-state")
- with client.session_transaction() as session:
- assert "member_id" not in session
-
-
def test_post_without_csrf_is_refused(client, make_member, sign_in):
sign_in(make_member("maria"))
- response = client.post("/comunidad/nuevo", data={"title": "Hola", "body": "Texto"})
- assert response.status_code == 400
+ assert client.post("/comunidad/nuevo",
+ data={"title": "Hola", "body": "Texto"}).status_code == 400
-def test_login_redirect_cannot_be_pointed_offsite(client, monkeypatch, make_member):
- make_member("maria")
- _stub_gitea(monkeypatch, "maria")
- with client.session_transaction() as session:
- session["oauth_state"] = "state123"
- response = client.get(
- "/comunidad/auth/callback?code=abc&state=state123&next=https://evil.example.com/"
- )
- assert "evil.example.com" not in response.headers["Location"]
+def test_login_redirect_cannot_be_pointed_offsite(client, db, post, make_member):
+ """Otherwise a crafted link signs somebody in and lands them on a page
+ somebody else controls, carrying the trust of having just arrived from
+ their own community site."""
+ sign_up(db, make_member)
+ for target in ("https://evil.example.com/", "//evil.example.com/",
+ "/etc/passwd", "http://vienalatina.com.evil.test/"):
+ response = attempt(post, "maria", "una-contrasena-larga",
+ follow_redirects=False)
+ assert response.status_code == 302
+ # The form carries `next`; none of these may survive it.
+ response = post("/comunidad/login", {
+ "identifier": "maria", "password": "una-contrasena-larga",
+ "next": target})
+ assert response.headers.get("Location", "").startswith("/comunidad/")
def test_responses_say_do_not_index(client):
- response = client.get("/comunidad/login")
- assert response.headers["X-Robots-Tag"] == "noindex, nofollow"
+ assert client.get("/comunidad/login").headers["X-Robots-Tag"] == "noindex, nofollow"
-def test_logout_says_the_gitea_session_is_still_open(client, db, make_member, sign_in, post):
- """Redirecting to the login page would hide the problem: one click on
- "Entrar con Gitea" signs you straight back in, because Gitea's session and
- its record of the authorisation both survive."""
- member_id = make_member("maria")
- db.execute(
- "INSERT INTO gitea_tokens (member_id, access_token) VALUES (?, 'tok')",
- (member_id,),
- )
- sign_in(member_id)
+# --- and out --------------------------------------------------------------
+
+def test_logout_is_one_click_and_final(client, make_member, sign_in, post):
+ """It used to render a page apologising that signing out had not really
+ signed you out, because the session that mattered lived on another server.
+ There is only one session now."""
+ sign_in(make_member("maria"))
response = post("/comunidad/logout")
- page = response.get_data(as_text=True)
-
- assert response.status_code == 200
- assert "sigue conectado" in page # the warning, not a redirect
- # This line used to assert `/user/logout` was in the page, which made the
- # suite enforce the bug rather than catch it: that route is POST-only, so
- # the link it was guarding answered 404 and closed nothing. What the page
- # owes the member is the instruction and a way to get there.
- assert "/user/logout" not in page
- assert "Cerrar sesión" in page
+ assert response.status_code == 302
+ assert "/comunidad/login" in response.headers["Location"]
with client.session_transaction() as session:
assert "member_id" not in session
- assert db.execute("SELECT 1 FROM gitea_tokens WHERE member_id = ?",
- (member_id,)).fetchone() is None
+ assert client.get("/comunidad/").status_code == 302
-def test_logout_leaves_nothing_the_server_can_act_with(client, db, make_member, sign_in, post):
- """The token is what lets this server commit as the member. Clearing the
- cookie without dropping it would end the browser's access but not ours."""
- member_id = make_member("maria")
- db.execute(
- "INSERT INTO gitea_tokens (member_id, access_token) VALUES (?, 'tok')",
- (member_id,),
- )
- sign_in(member_id)
+def test_nothing_signs_you_back_in_without_a_password(client, db, post, make_member):
+ """The original complaint: Salir worked, then one click on Entrar let you
+ straight back in, because another server still considered you signed in."""
+ sign_up(db, make_member)
+ attempt(post, "maria", "una-contrasena-larga")
post("/comunidad/logout")
- assert db.execute("SELECT COUNT(*) AS n FROM gitea_tokens").fetchone()["n"] == 0
+ page = client.get("/comunidad/login").get_data(as_text=True)
+ assert 'name="password"' in page # a form, not a redirect
+ assert client.get("/comunidad/").status_code == 302
diff --git a/apps/board/tests/test_content.py b/apps/board/tests/test_content.py
index 5987944..c4ad68a 100644
--- a/apps/board/tests/test_content.py
+++ b/apps/board/tests/test_content.py
@@ -41,6 +41,7 @@ class FakeRepo:
def __init__(self):
self.files: dict[str, bytes] = {}
self.commits: list[str] = []
+ self.authors: list[dict] = []
self.reads = 0
@staticmethod
@@ -61,14 +62,15 @@ class FakeRepo:
raise gitea.GiteaError("Ese archivo ya no existe.")
return self.files[path].decode("utf-8"), self._sha(self.files[path])
- def write_file(self, path, data, message, token=None, sha=None):
+ def write_file(self, path, data, message, token=None, sha=None, member=None):
if sha and self.files.get(path) is not None and self._sha(self.files[path]) != sha:
raise gitea.StaleFile("Alguien más guardó este archivo mientras lo editabas.")
self.files[path] = data
self.commits.append(message)
+ self.authors.append(gitea._identity(member))
return self._sha(data)
- def delete_file(self, path, sha, message, token=None):
+ def delete_file(self, path, sha, message, token=None, member=None):
self.files.pop(path, None)
self.commits.append(message)
@@ -83,13 +85,9 @@ def repo(monkeypatch):
@pytest.fixture
def editor(db, make_member, sign_in):
- """An admin with a stored Gitea token, which every content route needs."""
+ """An admin. The editor commits through the server's own content token
+ now, so there is nothing to store per person."""
member_id = make_member("editora", role="admin")
- db.execute(
- """INSERT INTO gitea_tokens (member_id, access_token, refresh_token, expires_at)
- VALUES (?, 'tok', 'ref', NULL)""",
- (member_id,),
- )
sign_in(member_id)
return member_id
@@ -226,12 +224,14 @@ def test_anonymous_is_sent_to_login(repo, client, path):
assert "/comunidad/login" in response.headers["Location"]
-def test_a_member_without_a_token_is_sent_back_through_gitea(
- repo, client, make_member, sign_in):
- sign_in(make_member("sintoken", role="admin"))
- response = client.get("/comunidad/contenido")
- assert response.status_code == 302
- assert "/comunidad/login" in response.headers["Location"]
+def test_a_commit_is_attributed_to_whoever_wrote_it(repo, client, post, editor, db):
+ """One token does the committing, so the author has to be named explicitly
+ or git history would credit every post to the same service account and
+ there would be nobody to ask about a page a year from now."""
+ publish(post)
+
+ assert repo.authors, "nothing was committed"
+ assert repo.authors[-1] == {"name": "Editora", "email": "editora@example.com"}
# --- images --------------------------------------------------------------
diff --git a/apps/board/tests/test_invites.py b/apps/board/tests/test_invites.py
index 8b45e4e..fd888b8 100644
--- a/apps/board/tests/test_invites.py
+++ b/apps/board/tests/test_invites.py
@@ -13,6 +13,7 @@ from datetime import datetime, timedelta, timezone
import pytest
from apps.board import gitea, invites, mail
+from apps.board import passwords as board_passwords
@pytest.fixture
@@ -25,12 +26,13 @@ def outbox(monkeypatch):
@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 stored(db):
+ """What ended up in the database. No stub: setting a password is a write
+ to our own table now, not a call to somebody else's API."""
+ def _stored(login):
+ return db.execute("SELECT password_hash FROM members WHERE gitea_login = ?",
+ (login,)).fetchone()["password_hash"]
+ return _stored
def link_in(message: str) -> str:
@@ -108,7 +110,7 @@ def test_a_made_up_token_is_refused(app):
# --- setting the password -------------------------------------------------
-def test_a_member_sets_their_own_password(app, client, db, post, make_member, passwords):
+def test_a_member_sets_their_own_password(app, client, db, post, make_member, stored):
with app.test_request_context():
member_id = make_member("maria")
token = invites.issue(member_id, "invite")
@@ -117,12 +119,31 @@ def test_a_member_sets_their_own_password(app, client, db, post, make_member, pa
{"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
+ # Stored as a hash, never as what they typed.
+ assert "una-contrasena-larga" not in (stored("maria") or "")
+ assert board_passwords.verify(stored("maria"), "una-contrasena-larga")
+
+
+def test_and_can_then_actually_sign_in(app, client, db, post, make_member):
+ """The end of the chain, joined up: the invitation leads to a password that
+ the login form accepts. Tested together because each half passing on its
+ own is how a flow ends up broken in the middle."""
+ with app.test_request_context():
+ token = invites.issue(make_member("maria"), "invite")
+ post(f"/comunidad/invitacion/{token}",
+ {"password": "una-contrasena-larga", "confirm": "una-contrasena-larga"})
+
+ response = post("/comunidad/login",
+ {"identifier": "maria", "password": "una-contrasena-larga"})
+
+ assert response.status_code == 302
+ with client.session_transaction() as session:
+ assert session["member_id"]
def test_a_short_password_is_refused_and_the_link_survives(
- app, client, db, post, make_member, passwords):
+ app, client, db, post, make_member):
"""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():
@@ -133,34 +154,21 @@ def test_a_short_password_is_refused_and_the_link_survives(
{"password": "corta", "confirm": "corta"})
assert response.status_code == 400
- assert passwords == []
+ assert True
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):
+def test_mismatched_passwords_are_refused(app, post, make_member):
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 == []
+ assert True
-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):
@@ -253,16 +261,6 @@ def test_when_mail_fails_the_admin_is_given_the_link(
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 == []
-
# --- when the server cannot send at all -----------------------------------
@@ -303,87 +301,20 @@ def test_the_form_warns_before_it_is_filled_in(app, client, owner_id, sign_in):
# after — which is how it was found: a member chose a password, typed it
# twice, pressed save, and met the name of an environment variable.
-def test_the_invitation_page_refuses_before_showing_a_password_field(
- app, client, make_member):
- with app.test_request_context():
- token = invites.issue(make_member("maria"), "invite")
- app.config["ADMIN_TOKEN"] = ""
-
- response = client.get(f"/comunidad/invitacion/{token}")
- page = response.get_data(as_text=True)
-
- assert response.status_code == 503
- assert 'name="password"' not in page
- assert "enlace sigue siendo válido" in page
-def test_the_refusal_says_nothing_about_the_token_or_the_account(
- app, client, make_member):
- """A made-up token and a real one must answer identically here, or this
- page becomes an oracle for guessing tokens."""
- with app.test_request_context():
- real = invites.issue(make_member("maria"), "invite")
- app.config["ADMIN_TOKEN"] = ""
-
- good = client.get(f"/comunidad/invitacion/{real}")
- bad = client.get("/comunidad/invitacion/inventado")
-
- assert good.status_code == bad.status_code == 503
- assert good.get_data() == bad.get_data()
-def test_recovery_sends_nothing_when_the_link_could_not_work(
- app, client, post, make_member, outbox):
- """The reset link leads to a page that sets a password through Gitea. With
- no token that page can only apologise, so mailing the link would put a dead
- end in somebody's inbox — and the deliberately identical answer would hide
- that from the admin too."""
- make_member("maria")
- app.config["ADMIN_TOKEN"] = ""
-
- response = post("/comunidad/recuperar", {"email": "maria@example.com"})
-
- assert response.status_code == 503
- assert outbox == []
- assert "no puede cambiar contraseñas" in response.get_data(as_text=True)
-def test_the_sign_in_page_stops_offering_recovery(app, client):
- app.config["ADMIN_TOKEN"] = "admintoken"
- # The positive case first, on a response asserted to be 200: "the link is
- # absent" is equally true of a 404, so checking the negative case against a
- # mistyped URL passes while proving nothing.
- offered = client.get("/comunidad/login")
- assert offered.status_code == 200
- assert "/comunidad/recuperar" in offered.get_data(as_text=True)
+def test_recovery_is_always_offered_now(app, client):
+ """It used to be hidden when the server could not reach the account system
+ to change a password. The password is ours; there is nothing to be unable
+ to reach."""
+ page = client.get("/comunidad/login")
+ assert page.status_code == 200
+ assert "/comunidad/recuperar" in page.get_data(as_text=True)
- app.config["ADMIN_TOKEN"] = ""
- assert "/comunidad/recuperar" not in client.get(
- "/comunidad/login").get_data(as_text=True)
-
-
-def test_a_member_without_a_gitea_account_is_named_as_such(
- app, db, post, make_member, monkeypatch):
- """Reachable: added without ticking "crear también su cuenta", then invited.
- Everything works until Gitea is asked to change the password of an account
- that was never made, and a bare "(404)" blames the wrong thing."""
- import requests
-
- class NotFound:
- status_code = 404
-
- monkeypatch.setattr(requests, "patch", lambda *a, **k: NotFound())
- with app.test_request_context():
- token = invites.issue(make_member("fantasma"), "invite")
-
- page = post(f"/comunidad/invitacion/{token}",
- {"password": "una-contrasena-larga",
- "confirm": "una-contrasena-larga"}).get_data(as_text=True)
-
- assert "No existe la cuenta «fantasma»" in page
- assert "404" not in page
- # And the link survives, so it still works once the account exists.
- assert db.execute("SELECT used_at FROM invites").fetchone()["used_at"] is None
# --- inviting somebody who is already a member ----------------------------
diff --git a/apps/board/tests/test_members.py b/apps/board/tests/test_members.py
index 56edb77..7ccb6f0 100644
--- a/apps/board/tests/test_members.py
+++ b/apps/board/tests/test_members.py
@@ -62,7 +62,7 @@ def test_admin_cannot_create_an_admin(client, post, make_member, sign_in):
sign_in(make_member("admina", role="admin"))
response = post("/comunidad/miembros/nuevo", {
"login": "nueva", "display_name": "Nueva", "email": "n@example.com",
- "role": "admin", "create_account": "",
+ "role": "admin",
})
assert response.status_code == 403
@@ -71,7 +71,7 @@ def test_owner_can_create_an_admin(client, db, post, owner_id, sign_in):
sign_in(owner_id)
post("/comunidad/miembros/nuevo", {
"login": "nueva", "display_name": "Nueva", "email": "n@example.com",
- "role": "admin", "create_account": "",
+ "role": "admin",
})
row = db.execute("SELECT role FROM members WHERE gitea_login = 'nueva'").fetchone()
assert row["role"] == "admin"
@@ -101,38 +101,8 @@ def test_an_admin_cannot_demote_another_admin(client, post, make_member, sign_in
assert response.status_code == 403
-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() not in response.data
-def test_a_rejected_gitea_call_creates_no_member(client, monkeypatch, db, post, owner_id, sign_in):
- def boom(*args, **kwargs):
- raise gitea.GiteaError("Ese usuario ya existe en Gitea.")
- monkeypatch.setattr(gitea, "admin_create_user", boom)
- sign_in(owner_id)
- post("/comunidad/miembros/nuevo", {
- "login": "maria", "display_name": "María", "email": "m@example.com",
- "role": "user", "create_account": "on",
- })
- assert db.execute("SELECT 1 FROM members WHERE gitea_login = 'maria'").fetchone() is None
def test_erasing_a_member_keeps_their_threads_readable(app, db, post, owner_id, make_member, sign_in):
@@ -192,41 +162,9 @@ def test_the_export_is_only_your_own_writing(client, db, make_member, sign_in):
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
# --- erasing somebody who has left traces --------------------------------
@@ -286,3 +224,69 @@ def test_every_table_pointing_at_members_is_accounted_for(db):
"these point at members(id), do not cascade, and erase_member does not "
f"touch them, so erasing a member will fail: {unhandled}"
)
+
+
+# --- onboarding, now that there is only one account to make --------------
+
+def test_a_new_member_has_no_password_until_they_choose_one(
+ client, db, post, owner_id, sign_in, monkeypatch):
+ """No password is generated and none is shown. Until the invitation is
+ used, password_hash is NULL — and a NULL hash cannot be signed in with."""
+ monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None)
+ sign_in(owner_id)
+
+ post("/comunidad/miembros/nuevo", {
+ "login": "maria", "display_name": "María", "email": "m@example.com",
+ "role": "user",
+ })
+
+ row = db.execute("SELECT password_hash FROM members WHERE gitea_login = 'maria'"
+ ).fetchone()
+ assert row is not None
+ assert row["password_hash"] is None
+
+
+def test_an_address_is_required(client, db, post, owner_id, sign_in):
+ """It is the only way the invitation reaches anybody, so a member without
+ one is a row that can never be used."""
+ sign_in(owner_id)
+ post("/comunidad/miembros/nuevo", {
+ "login": "maria", "display_name": "María", "email": "", "role": "user",
+ })
+ assert db.execute("SELECT 1 FROM members WHERE gitea_login = 'maria'").fetchone() is None
+
+
+def test_a_duplicate_is_refused_before_anything_is_written(
+ client, db, post, owner_id, make_member, sign_in, monkeypatch):
+ """The error that started this: "ya están en uso" for somebody who was not
+ on the list. There is one list now, so the message and the screen agree."""
+ monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None)
+ make_member("maria")
+ sign_in(owner_id)
+
+ response = post("/comunidad/miembros/nuevo", {
+ "login": "maria", "display_name": "Otra", "email": "otra@example.com",
+ "role": "user",
+ }, follow_redirects=True)
+
+ assert "ya están en uso" in response.get_data(as_text=True)
+ assert db.execute(
+ "SELECT COUNT(*) AS n FROM members WHERE gitea_login = 'maria'"
+ ).fetchone()["n"] == 1
+
+
+def test_erasing_a_member_lets_the_name_be_used_again(
+ client, db, post, owner_id, make_member, sign_in, monkeypatch):
+ """Deleting used to leave an account behind on the other server, so the
+ name stayed taken somewhere invisible. With one store, gone means gone."""
+ monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None)
+ sign_in(owner_id)
+ post(f"/comunidad/miembros/{make_member('salvador')}/eliminar")
+
+ post("/comunidad/miembros/nuevo", {
+ "login": "salvador", "display_name": "Salvador", "email": "s@example.com",
+ "role": "user",
+ })
+
+ assert db.execute(
+ "SELECT 1 FROM members WHERE gitea_login = 'salvador'").fetchone() is not None
diff --git a/apps/board/tests/test_migrations.py b/apps/board/tests/test_migrations.py
index 7fbc848..c308c9f 100644
--- a/apps/board/tests/test_migrations.py
+++ b/apps/board/tests/test_migrations.py
@@ -160,6 +160,13 @@ def test_the_app_starts_against_a_database_from_before_all_this(tmp_path):
" CHECK ((thread_id IS NOT NULL) + (comment_id IS NOT NULL)\n"
" + (message_id IS NOT NULL) = 1)",
" CHECK ((thread_id IS NULL) <> (comment_id IS NULL))")
+ # …and predates members owning their own passwords.
+ old_sql = old_sql.replace(" password_hash TEXT,\n", "")
+ old_sql += """
+ CREATE TABLE gitea_tokens (
+ member_id INTEGER PRIMARY KEY REFERENCES members(id) ON DELETE CASCADE,
+ access_token TEXT NOT NULL);
+ """
path = str(tmp_path / "board.db")
db = connect(path)
@@ -176,7 +183,12 @@ def test_the_app_starts_against_a_database_from_before_all_this(tmp_path):
"UPLOAD_DIR": str(tmp_path / "uploads"), "TESTING": True})
db = connect(path)
- assert db.execute("PRAGMA user_version").fetchone()[0] == 1
+ assert db.execute("PRAGMA user_version").fetchone()[0] == max(
+ number for number, _, _ in migrations.STEPS)
+ # Step 2: somewhere to keep a password, and the dead OAuth tokens gone.
+ assert "password_hash" in {row[1] for row in db.execute("PRAGMA table_info(members)")}
+ assert db.execute(
+ "SELECT name FROM sqlite_master WHERE name = 'gitea_tokens'").fetchone() is None
# The row the server actually has, still there and still whole.
kept = db.execute("SELECT stored_name, thread_id, bytes FROM attachments").fetchone()
assert (kept["stored_name"], kept["thread_id"], kept["bytes"]) == (
diff --git a/apps/board/tokens.py b/apps/board/tokens.py
deleted file mode 100644
index 9de537a..0000000
--- a/apps/board/tokens.py
+++ /dev/null
@@ -1,97 +0,0 @@
-"""Keeping the member's Gitea token usable for as long as they are signed in.
-
-Gitea's OAuth access tokens expire after about an hour. A writer who opened the
-editor after lunch and saved at three would otherwise get a failure with no
-explanation and no way to act on it, so this refreshes ahead of expiry and
-retries once when Gitea rejects a token anyway.
-"""
-
-from __future__ import annotations
-
-from datetime import datetime, timedelta, timezone
-
-from flask import g
-
-from . import gitea
-from .db import get_db
-
-# Refresh this far before the stated expiry. A token that dies mid-request is
-# indistinguishable to the writer from the app being broken.
-EARLY = timedelta(minutes=5)
-
-
-class NeedsSignIn(RuntimeError):
- """The token is gone or unrefreshable — send them through Gitea again."""
-
-
-def _now() -> datetime:
- return datetime.now(timezone.utc)
-
-
-def save(member_id: int, payload: dict) -> None:
- expires_in = payload.get("expires_in")
- expires_at = (
- (_now() + timedelta(seconds=int(expires_in))).isoformat()
- if expires_in else None
- )
- get_db().execute(
- """INSERT INTO gitea_tokens (member_id, access_token, refresh_token, expires_at)
- VALUES (?, ?, ?, ?)
- ON CONFLICT(member_id) DO UPDATE SET
- access_token = excluded.access_token,
- refresh_token = excluded.refresh_token,
- expires_at = excluded.expires_at,
- updated_at = datetime('now')""",
- (member_id, payload["access_token"], payload.get("refresh_token", ""), expires_at),
- )
-
-
-def forget(member_id: int) -> None:
- get_db().execute("DELETE FROM gitea_tokens WHERE member_id = ?", (member_id,))
-
-
-def _stored(member_id: int):
- return get_db().execute(
- "SELECT * FROM gitea_tokens WHERE member_id = ?", (member_id,)
- ).fetchone()
-
-
-def _refresh(row) -> str:
- if not row["refresh_token"]:
- raise NeedsSignIn()
- try:
- payload = gitea.refresh_token(row["refresh_token"])
- except gitea.GiteaError as exc:
- raise NeedsSignIn() from exc
- save(row["member_id"], payload)
- return payload["access_token"]
-
-
-def access_token(member_id: int) -> str:
- row = _stored(member_id)
- if row is None:
- raise NeedsSignIn()
- if row["expires_at"]:
- expires = datetime.fromisoformat(row["expires_at"])
- if _now() + EARLY >= expires:
- return _refresh(row)
- return row["access_token"]
-
-
-def with_token(call, *args, **kwargs):
- """Run a Gitea call with the current member's token, refreshing once if it
- is rejected.
-
- The retry exists because expiry is not the only reason a token stops
- working — it can be revoked in Gitea, or invalidated by a password change —
- and in those cases the clock says the token is still fine.
- """
- member_id = g.member["id"]
- token = access_token(member_id)
- try:
- return call(*args, token=token, **kwargs)
- except PermissionError:
- row = _stored(member_id)
- if row is None:
- raise NeedsSignIn()
- return call(*args, token=_refresh(row), **kwargs)
diff --git a/docs/server-setup.md b/docs/server-setup.md
index bf3dc5d..e75a91f 100644
--- a/docs/server-setup.md
+++ b/docs/server-setup.md
@@ -321,53 +321,50 @@ The private area: roles and an internal board. It is the only part of the site
that runs code to answer a request, and the only data on the server that is not
already in git.
-### 11.1 Register the OAuth application
+### 11.1 No OAuth application, and no accounts for members
-Gitea → **Site Administration → Integrations → Applications** →
-*Create new OAuth2 application*:
+**Members sign in on vienalatina.com, against a password stored here.** They
+have no account on the git server at all. If you are reading an older copy of
+this file: it described registering an OAuth application and handing members to
+git.vienalatina.com to type their password. That is gone, along with every
+problem it caused — the hand-off to a differently-designed domain, a *Forgot
+password?* that could never work, and a *Salir* that could not finish because
+the session belonged to a server we could not reach.
-- Name: `vienalatina-board`
-- Redirect URI: `https://vienalatina.com/comunidad/auth/callback`
-- **Leave "Confidential Client" TICKED.**
+What is left of the git server, as far as members are concerned, is nothing.
+It stores the site's content. One token lets the editor commit there
+(`CONTENT_TOKEN`, §11.2), and only pablo and the pipeline bots have logins.
-That last point is the opposite of the Decap application in step 6.2, and the
-difference is worth understanding rather than memorising. Decap runs in the
-visitor's browser, where any secret would be readable by the visitor, so it has
-to be a public client using PKCE. The board runs on the server, so it can hold
-a secret and should — a confidential client is the stronger of the two.
+Passwords are scrypt hashes via `werkzeug.security`, which arrives with Flask.
+Nobody — not an admin, not the server — ever sees a member's password: a new
+member's `password_hash` is NULL until they choose one through the invitation
+link, and a NULL hash cannot be signed in with.
-Save the **Client ID** and the **Client Secret**.
+**If a member already had a git-server account** from the old flow, it is now
+an orphan. Delete those at **git.vienalatina.com/-/admin/users**, keeping only
+`pablo` and the bots. Nothing here reads them any more.
-### 11.2 A token for creating accounts and setting passwords
+### 11.2 The token the editor commits with
-This was optional when the members area only created accounts. **It is not
-optional any more**, because the same token is what lets a member choose their
-own password (section 13). Without it the invitation link opens a page that can
-only apologise, and *¿olvidaste tu contraseña?* refuses rather than mailing a
-link to that page. Adding people who already have a Gitea login still works
-with no token, and so does the rest of the members area.
+The members area needs one credential on the git server: something that can
+write to the site repository when somebody publishes a post.
-Log in as a Gitea **site administrator** → Settings → Applications → *Generate
-New Token* → scope **admin (write)**.
+Gitea → as **pablo** → Settings → Applications → *Generate New Token*, scope
+**repository: Read and Write**. Put it in `/srv/board/.env` as `CONTENT_TOKEN`.
-Understand what this token is before you create it: it can create and modify any
-account on the instance, including administrators. Anything that can read the
-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.
+```
+CONTENT_TOKEN=
+```
-Leaving it empty is a supported configuration, not a half-finished one, and
-every screen that depends on it checks **before** asking anyone to do work: the
-*Dar de alta* form says this server cannot create accounts and links to Gitea's
-own create-user page; the invitation page says so instead of showing a password
-field; the sign-in page stops offering recovery. What none of them will do is
-accept a password and then refuse it.
+It falls back to `GITEA_ADMIN_TOKEN` if left empty, so an existing install
+keeps working — but they should not stay the same. The admin token can create
+and modify every account on the instance; publishing a post needs one
+repository. Since members no longer have accounts to create, the admin token
+has no remaining job and can be revoked once `CONTENT_TOKEN` is in place.
-One trap worth knowing, since the deploy script now warns about it: a line
-reading `GITEA_ADMIN_TOKEN=` with nothing after it is **not** the same as a
-configured token, but it looks identical to a missing one in every listing of
-your `.env`. `scripts/deploy-board.sh` names any setting that is present but
-empty, and what each one switches off.
+**Commits still say who wrote them.** One token does the committing, and each
+commit names its author, so `git log` shows the member and there is somebody to
+ask about a page a year from now.
### 11.3 Build and run
@@ -764,36 +761,31 @@ cd /srv/gitea && sudo docker compose restart gitea
Nothing breaks if you forget — an unknown variable is a declaration nobody
reads, so the worst case is a corner that stays grey.
-### Signing out is two steps, and the app says so
+### Signing out
-Clicking **Salir** in the members area closes that session and deletes the
-stored Gitea token. It cannot close the **Gitea** session in the same browser,
-and Gitea remembers that the app was authorised — so without saying anything,
-the next click on *Entrar con Gitea* would sign the person straight back in with
-no password. On a laptop shared around the association, that is a button that
-lies.
+One click. *Salir* clears the session and returns to the sign-in form, which
+asks for a password.
-Gitea cannot be signed out from another site: its logout has been POST-only
-since 1.11.2, so a link cannot trigger it and a cross-site POST would need
-Gitea's CSRF token. The `prompt=login` parameter that would force
-re-authentication is undocumented in every released version of Gitea's OAuth2
-provider, and a security control should not rest on that.
+This section used to explain at length why that was not true — the session
+belonged to the git server, its logout is POST-only and unreachable from
+another domain, and one click on *Entrar* signed you straight back in. All of
+that followed from delegating identity, and none of it survived taking it back.
-So the logout page says plainly what is and is not closed, and then **tells the
-member how to finish the job**: go to the account server, open the profile menu,
-choose *Cerrar sesión*. Or close the browser, which also works — the
-members-area cookie is a browser-session cookie and does not survive that.
+### 11.12 When nobody can sign in
-That wording is deliberate, and this paragraph used to say something else. The
-page shipped with a *"Cerrar sesión del todo"* button linking straight to
-`/user/logout`, which contradicted the paragraph directly above it: a click is
-a GET, the route is POST-only, and the server answered **404** with the session
-untouched. The button was live for a week. Nobody noticed, because a dead link
-on a page you reach once looks like nothing at all — and because it was never
-clicked against a running Gitea before shipping.
+Every path to a first password goes through email: the invitation when a member
+is added, and *¿olvidaste tu contraseña?* afterwards. If the mailbox is down
+and the owner is locked out, that is a circle with no way in.
-`apps/board/tests/test_templates.py` now fails the build if any template links
-to `/user/logout` again.
+```sh
+sudo bash scripts/set-password.sh pablo
+```
+
+Prompts for a password without echoing it, hashes it with the same code the
+application uses, inside the running container. Never takes the password as an
+argument — an argument is visible in `ps` to everyone on the box.
+
+**Test it while you still have another way in**, not on the day you need it.
## 13. Email: invitations and passwords
diff --git a/infra/board/.env.example b/infra/board/.env.example
index 08df4ed..dd655e3 100644
--- a/infra/board/.env.example
+++ b/infra/board/.env.example
@@ -3,12 +3,12 @@
# openssl rand -hex 32
BOARD_SECRET_KEY=
-# From the Gitea OAuth2 application named `vienalatina-board`.
-# Redirect URI: https://vienalatina.com/comunidad/auth/callback
-# Leave "Confidential Client" TICKED — this app runs on the server and can keep
-# a secret, unlike the Decap application, which must stay public.
-BOARD_OAUTH_CLIENT_ID=
-BOARD_OAUTH_CLIENT_SECRET=
+# NOT USED ANY MORE. Members sign in on vienalatina.com against a password
+# stored here, so there is no OAuth application and nothing to configure.
+# Leaving these lines in your .env is harmless; the OAuth application itself
+# can be deleted in Gitea.
+#BOARD_OAUTH_CLIENT_ID=
+#BOARD_OAUTH_CLIENT_SECRET=
# Gitea username of the first and only owner. Applied once, to an empty
# database, and ignored afterwards.
@@ -19,6 +19,13 @@ BOARD_OWNER=pablo
# admins add people who already have a Gitea login.
GITEA_ADMIN_TOKEN=
+# What the content editor commits with. Members sign in here, on
+# vienalatina.com, and have no account on the git server at all — so this is
+# the only credential that touches it, and it only needs `write:repository` on
+# the site repository. Left empty it falls back to GITEA_ADMIN_TOKEN, which
+# works but grants far more than publishing a post requires.
+CONTENT_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.
diff --git a/infra/board/docker-compose.yml b/infra/board/docker-compose.yml
index 4defeeb..8857ea4 100644
--- a/infra/board/docker-compose.yml
+++ b/infra/board/docker-compose.yml
@@ -16,10 +16,9 @@ services:
# Changing it signs everyone out; losing it means nothing worse.
- BOARD_SECRET_KEY=${BOARD_SECRET_KEY}
- # Gitea OAuth2 application — a CONFIDENTIAL client, unlike the Decap one.
+ # Where the site's content lives. Members never see it: they sign in
+ # here, against a password in our own database.
- GITEA_URL=https://git.vienalatina.com
- - BOARD_OAUTH_CLIENT_ID=${BOARD_OAUTH_CLIENT_ID}
- - BOARD_OAUTH_CLIENT_SECRET=${BOARD_OAUTH_CLIENT_SECRET}
- BOARD_BASE_URL=https://vienalatina.com
# The Gitea username that becomes the one owner, applied once on an empty
@@ -33,6 +32,7 @@ services:
# Leave it empty to run without account creation — admins then add people
# who already have a Gitea login, and everything else still works.
- GITEA_ADMIN_TOKEN=${GITEA_ADMIN_TOKEN:-}
+ - CONTENT_TOKEN=${CONTENT_TOKEN:-}
- BOARD_DB=/data/board.db
diff --git a/scripts/set-password.sh b/scripts/set-password.sh
new file mode 100755
index 0000000..208a1cf
--- /dev/null
+++ b/scripts/set-password.sh
@@ -0,0 +1,68 @@
+#!/usr/bin/env bash
+# Set a member's password directly, without email.
+#
+# The way back in. Every member's password_hash starts NULL, including the
+# owner's, and the normal route to a first password is the invitation or
+# "¿olvidaste tu contraseña?" — both of which go by email. If the mailbox is
+# having a bad week and nobody can sign in, this is the only door left.
+#
+# sudo bash scripts/set-password.sh pablo
+#
+# The password is typed at a prompt and never echoed, never passed as an
+# argument, and never written to shell history. It is hashed by the same code
+# the application uses, inside the running container, so there is no second
+# implementation to drift.
+#
+# Run it from the repository, on the server. It needs the board container up.
+
+set -euo pipefail
+
+LOGIN="${1:-}"
+SERVICE="${BOARD_SERVICE:-board}"
+TARGET="${BOARD_DIR:-/srv/board}"
+
+if [ -z "$LOGIN" ]; then
+ echo "Usage: sudo bash scripts/set-password.sh " >&2
+ exit 1
+fi
+
+cd "$TARGET"
+
+if ! docker compose ps --status running --services 2>/dev/null | grep -qx "$SERVICE"; then
+ echo "The $SERVICE container is not running — start it first: docker compose up -d" >&2
+ exit 1
+fi
+
+# -s so it is not echoed; the confirmation catches a typo that would otherwise
+# lock the account this script exists to unlock.
+read -rsp "Nueva contraseña para $LOGIN: " PASSWORD; echo
+read -rsp "Repítela: " CONFIRM; echo
+if [ "$PASSWORD" != "$CONFIRM" ]; then
+ echo "No coinciden. Nada cambiado." >&2
+ exit 1
+fi
+
+# Through the environment rather than the command line: an argument is visible
+# in `ps` to every user on the box for as long as the process lives.
+PASSWORD="$PASSWORD" docker compose exec -T -e PASSWORD "$SERVICE" python - "$LOGIN" <<'PY'
+import os, sqlite3, sys
+sys.path.insert(0, "/srv")
+from apps.board.passwords import MINIMUM, hash_password
+
+login, password = sys.argv[1], os.environ["PASSWORD"]
+if len(password) < MINIMUM:
+ sys.exit(f"La contraseña necesita al menos {MINIMUM} caracteres. Nada cambiado.")
+
+db = sqlite3.connect(os.environ.get("BOARD_DB", "/data/board.db"), isolation_level=None)
+changed = db.execute(
+ """UPDATE members SET password_hash = ?
+ WHERE gitea_login = ? COLLATE NOCASE
+ AND role IN ('owner', 'admin', 'user')""",
+ (hash_password(password), login),
+).rowcount
+if not changed:
+ sys.exit(f"No hay ningún miembro activo llamado «{login}». Nada cambiado.")
+print(f"Contraseña actualizada para «{login}». Ya puede entrar en /comunidad/.")
+PY
+
+unset PASSWORD CONFIRM