Compare commits

..

4 Commits

Author SHA1 Message Date
e17d1e05c6 Merge branch 'claude/relaxed-faraday-h4zd09' of https://github.com/pablovolenski/vienalatina
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
2026-09-29 09:09:02 +00:00
Claude
b90cbd2a9f
One front door: the members area owns its own logins
Three complaints, one cause. Logging out could not finish because the
session belonged to another server, whose logout is POST-only and
unreachable from here. "Ese usuario ya está en uso" for somebody absent
from Miembros, because erase_member deleted our row and left the git
account standing — two stores of users, one of them showing. And the
hand-off to a differently-designed domain, with a Forgot password that
could never work. All three followed from delegating identity, so it is
no longer delegated.

Members now sign in at /comunidad/login against a scrypt hash in our own
database, via werkzeug.security, which arrives with Flask. They have no
account on the git server at all, which makes the collision impossible
rather than fixed. Logout is one click.

This removes more than it adds: the OAuth round trip, the client
registration, tokens.py with its refresh-before-expiry logic, the
gitea_tokens table, and the logged-out page that existed to apologise
for a logout that did not log you out.

The editor keeps per-writer attribution without per-writer tokens: one
CONTENT_TOKEN commits, and each commit names its author, which Gitea's
contents API supports and a test now asserts. CONTENT_TOKEN falls back
to GITEA_ADMIN_TOKEN so nothing breaks on deploy, but it only needs
write access to one repository, while the admin token can modify every
account on the instance — and now has no remaining job.

The refusals are the interesting part. Unknown name, wrong password,
suspended member and invited-but-never-arrived all answer identically,
asserted by comparing the rendered bytes. The hash check runs against a
decoy even when there is no such member, so an unknown name does not
answer faster. Attempts are rate limited, counted inside SQLite for the
reason invites.py documents. A NULL hash never matches anything.

Migration 2 adds password_hash and drops the dead OAuth tokens. Everyone
starts NULL, including the owner, so scripts/set-password.sh exists and
was tested before this could ship: it prompts without echo, never takes
the password as an argument where ps would show it, and refuses a short
one or an unknown member.

Rehearsed against a rebuilt copy of the server's database: starts, keeps
the photo, drops the tokens table, serves a login form with no redirect,
signs in, signs out, stays out.

223 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn
2026-09-29 08:25:25 +00:00
d69fb49466 Merge branch 'claude/relaxed-faraday-h4zd09' of https://github.com/pablovolenski/vienalatina 2026-09-28 16:19:41 +00:00
Claude
73ad25a15a
Fix the start-up crash Phase C shipped, and test for it
The board would not start against any existing database, so Caddy had
nothing to proxy to and answered 502. Reproduced by rebuilding the
server's database from the previous schema and starting the app the way
gunicorn does.

The cause is one line of schema.sql:

  CREATE INDEX IF NOT EXISTS attachments_message ON attachments(message_id)

IF NOT EXISTS guards the index NAME, not the column. On a database whose
attachments table predates message_id, executescript dies with "no such
column" — before any migration could add it, because init_db runs the
schema first by design. Every index on a column a migration introduces
belongs in the migration, after the column exists. It now lives there,
outside the rebuild branch so a freshly created database gets it too.

Second bug, latent and worse: the rebuild set PRAGMA foreign_keys = OFF
inside the transaction, where it is documented to be a no-op. It looked
applied and did nothing. Moved outside BEGIN. The test fixture had been
passing for the wrong reason — it left foreign keys at SQLite's default,
which is off — and now turns them on as connect() does.

Both are the same mistake in different clothes: the tests exercised
migrations.apply directly, so nothing ever ran init_db against a
database from before the change. That test exists now. It builds the
previous schema, inserts the row the server actually has, starts the
app, and asserts the photo survived and a restart is a no-op. Verified
by reinstating the bad line and watching it fail with the same error the
server gave.

230 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn
2026-09-28 16:15:56 +00:00
23 changed files with 823 additions and 817 deletions

View File

@ -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,

View File

@ -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,14 +153,11 @@ 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.
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"))
@ -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"))

View File

@ -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))

View File

@ -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

View File

@ -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,48 +194,38 @@ 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")
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")
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"))
try:
@ -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"
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])

View File

@ -45,11 +45,7 @@ def _attachments_accept_messages(db: sqlite3.Connection) -> None:
on can cascade, and switched back on after, with a check that nothing was
broken in between.
"""
if "message_id" in _columns(db, "attachments"):
return # a new database: schema.sql already created it this way
db.execute("PRAGMA foreign_keys = OFF")
try:
if "message_id" not in _columns(db, "attachments"):
db.execute("""
CREATE TABLE attachments_new (
id INTEGER PRIMARY KEY,
@ -83,14 +79,36 @@ def _attachments_accept_messages(db: sqlite3.Connection) -> None:
broken = db.execute("PRAGMA foreign_key_check").fetchall()
if broken:
raise RuntimeError(f"migration left dangling references: {broken}")
finally:
db.execute("PRAGMA foreign_keys = ON")
# Outside the branch above: a database created fresh by schema.sql has the
# column but not this index, because schema.sql cannot carry it — see the
# note there. Both paths end up with the same table and the same indexes.
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),
]
@ -101,6 +119,11 @@ def apply(db: sqlite3.Connection) -> list[str]:
for number, description, step in sorted(STEPS):
if number <= version:
continue
# Outside the transaction, deliberately: "PRAGMA foreign_keys is a
# no-op within a transaction". Setting it inside BEGIN looks like it
# worked and changes nothing, which is how a table rebuild ends up
# running with enforcement still on.
db.execute("PRAGMA foreign_keys = OFF")
db.execute("BEGIN IMMEDIATE")
try:
step(db)
@ -111,5 +134,7 @@ def apply(db: sqlite3.Connection) -> list[str]:
except Exception:
db.execute("ROLLBACK")
raise
finally:
db.execute("PRAGMA foreign_keys = ON")
done.append(f"{number}: {description}")
return done

91
apps/board/passwords.py Normal file
View File

@ -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')")

View File

@ -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
@ -137,22 +153,19 @@ CREATE TABLE IF NOT EXISTS attachments (
CREATE INDEX IF NOT EXISTS attachments_thread ON attachments(thread_id);
CREATE INDEX IF NOT EXISTS attachments_comment ON attachments(comment_id);
CREATE INDEX IF NOT EXISTS attachments_message ON attachments(message_id);
-- The index on message_id is NOT here, and that is not an oversight.
-- CREATE INDEX IF NOT EXISTS guards the index NAME, not the column: run it
-- against a database whose attachments table predates message_id and it fails
-- with "no such column", taking the whole start-up with it. Any index on a
-- 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.
--

View File

@ -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 %}
<article class="card card--center">
<h1>Saliste del área de la comunidad</h1>
<p class="muted">
Tu sesión aquí está cerrada. Pero este navegador <strong>sigue conectado al
servidor de cuentas</strong>, que es donde se guardan las contraseñas — así
que quien use este ordenador después podría volver a entrar sin escribirla.
</p>
{# 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. #}
<p>
<a class="btn" href="{{ gitea_url }}">Ir al servidor de cuentas</a>
</p>
<p class="muted small">
Allí, abre el menú de tu perfil (arriba a la derecha) y elige
<em>Cerrar sesión</em>. Cerrar el navegador del todo también sirve.
</p>
<p class="muted small">
En tu propio ordenador no hace falta: puedes volver a entrar cuando quieras.
</p>
<p><a class="linkish" href="{{ url_for('auth.login') }}">Volver a entrar</a></p>
</article>
{% endblock %}

View File

@ -2,23 +2,32 @@
{% block title %}Entrar{% endblock %}
{% block main %}
<article class="card card--center">
<form class="card card--center" method="post" action="{{ url_for('auth.login') }}">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<input type="hidden" name="next" value="{{ next }}">
<h1>Área de la comunidad</h1>
<p class="muted">
Este espacio es sólo para miembros de Viena Latina. Se entra con la misma
cuenta que se usa para publicar en el sitio.
Este espacio es sólo para miembros de Viena Latina.
</p>
<a class="btn" href="{{ url_for('auth.start', next=next) }}">Entrar</a>
{# Offered only when the server can actually complete it: without a Gitea
admin token the recovery page can do nothing but apologise. #}
{% if can_recover %}
<label for="identifier">Usuario o correo</label>
<input id="identifier" name="identifier" required autocomplete="username"
autocapitalize="none" autofocus>
<label for="password">Contraseña</label>
<input id="password" name="password" type="password" required
autocomplete="current-password">
<div class="actions">
<button class="btn" type="submit">Entrar</button>
</div>
<p class="muted small">
<a href="{{ url_for('auth.recover') }}">¿Olvidaste tu contraseña?</a>
</p>
{% endif %}
<p class="muted small">
¿No tienes cuenta? Pídesela a un administrador: las cuentas se crean a mano,
no hay registro abierto.
¿No tienes cuenta? Pídesela a un administrador: las cuentas se crean a
mano, no hay registro abierto.
</p>
</article>
</form>
{% endblock %}

View File

@ -5,10 +5,7 @@
<article class="card">
<h1>{{ login }} ya es {{ role_label|lower }}</h1>
{% if not created %}
<p>No se creó ninguna cuenta nueva: ya existía. Puede entrar con la que tenía.</p>
{% 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. #}

View File

@ -15,29 +15,11 @@
<input id="display_name" name="display_name" placeholder="María López">
<label for="email">Correo</label>
<input id="email" name="email" type="email" placeholder="maria@ejemplo.com">
<input id="email" name="email" type="email" required
placeholder="maria@ejemplo.com">
<p class="muted small">Ahí llega la invitación para elegir contraseña.</p>
{% if can_create_accounts %}
<label class="check">
<input type="checkbox" name="create_account" checked>
Crear también su cuenta (desmarca si ya tiene una)
</label>
{% 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. #}
<label class="check">
<input type="checkbox" name="create_account" disabled>
Crear también su cuenta
</label>
<p class="muted small">
Este servidor no puede crear cuentas (le falta el token de administración).
Créala primero en el
<a href="{{ gitea_url }}/admin/users/new">servidor de cuentas</a> y luego
da de alta aquí ese mismo usuario.
</p>
{% 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. #}

View File

@ -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,

View File

@ -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

View File

@ -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 --------------------------------------------------------------

View File

@ -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 ----------------------------

View File

@ -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

View File

@ -9,6 +9,7 @@ never needed for.
from __future__ import annotations
import sqlite3
from pathlib import Path
import pytest
@ -42,6 +43,10 @@ def old_db(tmp_path):
"""A database as it existed before this migration, with real rows in it."""
db = sqlite3.connect(tmp_path / "old.db", isolation_level=None)
db.row_factory = sqlite3.Row
# As connect() does in db.py. Without this the fixture runs with SQLite's
# default (off) and a migration that mishandles foreign keys passes here
# and fails on the server.
db.execute("PRAGMA foreign_keys = ON")
db.executescript(OLD_SCHEMA)
db.execute("INSERT INTO members (id, gitea_login) VALUES (1, 'salvador')")
db.execute("INSERT INTO threads (id) VALUES (7)")
@ -126,3 +131,70 @@ def test_a_fresh_database_skips_it(app, db):
done and return quietly rather than rebuilding a table it just created."""
assert "message_id" in {row[1] for row in db.execute("PRAGMA table_info(attachments)")}
assert migrations.apply(db) == []
# --- the whole start-up, not just the step -------------------------------
def test_the_app_starts_against_a_database_from_before_all_this(tmp_path):
"""The test that was missing, and the reason the site went down.
Every other test here calls `migrations.apply` directly. The failure was
one layer above it: `init_db` runs `schema.sql` *first*, and schema.sql
carried `CREATE INDEX ... ON attachments(message_id)`. IF NOT EXISTS guards
the index name, not the column — so against a real database the script died
with "no such column: message_id" before any migration could fix anything,
the app never finished starting, and Caddy answered 502.
This builds a database with the schema as it shipped, puts a real row in
it, and starts the application the way gunicorn does.
"""
from apps.board.app import create_app
from apps.board.db import connect
shipped = (Path(__file__).resolve().parents[3] / "apps/board/schema.sql")
old_sql = shipped.read_text(encoding="utf-8")
# Reduce it to the shape that predates this migration: no message_id
# anywhere, and the CHECK that goes with it.
old_sql = old_sql.replace(" message_id INTEGER REFERENCES messages(id),\n", "")
old_sql = old_sql.replace(
" 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)
db.executescript(old_sql)
db.execute("INSERT INTO members (gitea_login, display_name, role) "
"VALUES ('salvador', 'Salvador', 'user')")
db.execute("INSERT INTO threads (author_id, title, body_md) VALUES (1, 'Hola', 'T')")
db.execute("""INSERT INTO attachments
(thread_id, stored_name, original_name, content_type, bytes, uploaded_by)
VALUES (1, 'foto-abc123abc123.jpg', 'foto.jpg', 'image/jpeg', 2048, 1)""")
db.close()
create_app({"SECRET_KEY": "x", "DB_PATH": path, "OWNER_LOGIN": "salvador",
"UPLOAD_DIR": str(tmp_path / "uploads"), "TESTING": True})
db = connect(path)
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"]) == (
"foto-abc123abc123.jpg", 1, 2048)
# And starting again changes nothing, because a container restarts.
create_app({"SECRET_KEY": "x", "DB_PATH": path, "OWNER_LOGIN": "salvador",
"UPLOAD_DIR": str(tmp_path / "uploads"), "TESTING": True})
db.close()

View File

@ -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)

View File

@ -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

View File

@ -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.

View File

@ -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

68
scripts/set-password.sh Executable file
View File

@ -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 <usuario>" >&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