From e11016a8e9c570969a170ca5dea83a90e7b6714d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 19:20:44 +0000 Subject: [PATCH] Refuse before the form, not after the password MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A member followed his invitation link, chose a password, typed it twice, pressed save, and was shown the name of an environment variable. The server knew from the first byte of that request that it could not save anything: without GITEA_ADMIN_TOKEN it cannot set a password in Gitea. It asked him to do the work anyway. Three places had the same shape, all now checked up front through a new gitea.admin_configured(), mirroring mail.configured(): - the invitation page answers 503 with an explanation and no password field, identically for a real and an invented token so it cannot be used to probe for live ones - /recuperar refuses instead of mailing a link to a page that could only apologise — and its deliberately identical answer would have hidden that from the admin as well as the member - the sign-in page stops offering recovery it cannot complete Also: a 404 from admin_set_password now names the real cause. A member added without "crear también su cuenta" has no Gitea account, so the password change is aimed at nothing, and "Gitea rechazó el cambio de contraseña (404)" blames Gitea for an account that was never made. deploy-board.sh warns about settings that are present but empty. The previous check looked for missing names, and GITEA_ADMIN_TOKEN= has a name — which is why the deploy that led to this said nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn --- apps/board/auth.py | 25 ++++++- apps/board/gitea.py | 21 ++++++ apps/board/templates/login.html | 4 ++ apps/board/templates/recover.html | 16 ++++- apps/board/templates/set_password.html | 15 ++++- apps/board/tests/test_invites.py | 90 ++++++++++++++++++++++++++ docs/server-setup.md | 27 +++++--- scripts/deploy-board.sh | 20 ++++++ 8 files changed, 207 insertions(+), 11 deletions(-) diff --git a/apps/board/auth.py b/apps/board/auth.py index 0587997..1749863 100644 --- a/apps/board/auth.py +++ b/apps/board/auth.py @@ -54,7 +54,11 @@ def load_member() -> None: def login(): if g.member is not None: return redirect(url_for("board.threads")) - return render_template("login.html", next=request.args.get("next", "")) + 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()) @bp.route("/login/start") @@ -147,6 +151,16 @@ 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, @@ -185,6 +199,15 @@ 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") diff --git a/apps/board/gitea.py b/apps/board/gitea.py index eaa35e6..9f174a1 100644 --- a/apps/board/gitea.py +++ b/apps/board/gitea.py @@ -121,6 +121,18 @@ def fetch_user(token: str) -> dict: # --- account creation (site-admin token) --------------------------------- +def admin_configured() -> bool: + """Whether this server holds a site-admin token. + + Without one it can neither create accounts nor set passwords. That is a + supported configuration — server-setup.md §11.2 says why somebody might + choose it — and it is why both calls below refuse before reaching Gitea. + Exposed as a predicate so a screen can say so *before* asking somebody to + fill in a form that cannot be saved, the way `mail.configured()` is used. + """ + return bool(current_app.config.get("ADMIN_TOKEN")) + + def generate_password() -> str: # Shown once to the admin, then changed by the member on first login. # Punctuation is left out on purpose: this gets read aloud or copied by @@ -195,6 +207,15 @@ def admin_set_password(login: str, password: str) -> None: raise GiteaError("Gitea rechazó esa contraseña. Prueba con una más larga.") if response.status_code in (401, 403): raise GiteaError("El token de administración de Gitea no es válido.") + if response.status_code == 404: + # Reachable: a member can be added here without ticking "crear también + # su cuenta", and then invited. Everything works right up to this call, + # which is asked to change the password of an account that was never + # made. A bare "(404)" sends the admin looking at the wrong thing. + raise GiteaError( + f"No existe la cuenta «{login}» en Gitea, así que no se le puede " + "poner contraseña. Pide a un administrador que la cree." + ) raise GiteaError(f"Gitea rechazó el cambio de contraseña ({response.status_code}).") diff --git a/apps/board/templates/login.html b/apps/board/templates/login.html index cc7d0a1..477646b 100644 --- a/apps/board/templates/login.html +++ b/apps/board/templates/login.html @@ -9,9 +9,13 @@ cuenta que se usa para publicar en el sitio.

Entrar con Gitea + {# 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 %}

¿Olvidaste tu contraseña?

+ {% endif %}

¿No tienes cuenta? Pídesela a un administrador: las cuentas se crean a mano, no hay registro abierto. diff --git a/apps/board/templates/recover.html b/apps/board/templates/recover.html index b575393..255076a 100644 --- a/apps/board/templates/recover.html +++ b/apps/board/templates/recover.html @@ -2,7 +2,21 @@ {% block title %}Recuperar contraseña{% endblock %} {% block main %} -{% if sent %} +{% if unavailable %} +{# Refusing out loud rather than accepting the address and sending nothing. + Silence here would be indistinguishable from success — that is the point of + the identical answer below — so the one case where the server knows in + advance that it cannot help has to be said plainly. #} +

+

Todavía no

+

+ Este servidor aún no puede cambiar contraseñas, así que un enlace de + recuperación no serviría de nada. Escribe a un administrador y te darán + acceso a mano. +

+

Volver a entrar

+
+{% elif sent %}

Revisa tu correo

{# Says the same thing whether or not that address belongs to a member. diff --git a/apps/board/templates/set_password.html b/apps/board/templates/set_password.html index cb7c140..c0ea90e 100644 --- a/apps/board/templates/set_password.html +++ b/apps/board/templates/set_password.html @@ -2,7 +2,20 @@ {% block title %}Elige tu contraseña{% endblock %} {% block main %} -{% if member is none %} +{% if unavailable %} +{# The link is fine. The server is not — with no Gitea admin token it cannot + set anybody's password. Said here, before a password field appears, because + the alternative is somebody choosing a password, typing it twice, pressing + the button and only then being shown the name of an environment variable. #} +
+

Todavía no podemos guardar tu contraseña

+

+ Tu enlace sigue siendo válido, así que guárdalo. Lo que falta está en el + servidor: aún no puede cambiar contraseñas. Avisa a un administrador y + vuelve a abrir este enlace cuando te lo confirme. +

+
+{% elif member is none %} {# One message for expired, already used and never existed alike. Which one it was would tell whoever holds a stale link something about the account. #}
diff --git a/apps/board/tests/test_invites.py b/apps/board/tests/test_invites.py index 845a99c..04b6024 100644 --- a/apps/board/tests/test_invites.py +++ b/apps/board/tests/test_invites.py @@ -294,3 +294,93 @@ def test_the_form_warns_before_it_is_filled_in(app, client, owner_id, sign_in): app.config["MAIL_HOST"] = "smtp.example.com" assert "no envía correo" not in client.get( "/comunidad/miembros/nuevo").get_data(as_text=True) + + +# --- when the server cannot set passwords at all -------------------------- +# +# Without GITEA_ADMIN_TOKEN nothing in this file can complete. The point of +# these four is that the refusal arrives *before* somebody does work, not +# 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) + + 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» en Gitea" 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 diff --git a/docs/server-setup.md b/docs/server-setup.md index 0c78681..86d6d72 100644 --- a/docs/server-setup.md +++ b/docs/server-setup.md @@ -338,11 +338,14 @@ a secret and should — a confidential client is the stronger of the two. Save the **Client ID** and the **Client Secret**. -### 11.2 Optional: a token for creating accounts +### 11.2 A token for creating accounts and setting passwords -Without it, admins can add people who already have a Gitea login, and nothing -else changes. With it, they can create the Gitea account from inside the members -area and hand over a one-time password. +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. Log in as a Gitea **site administrator** → Settings → Applications → *Generate New Token* → scope **admin (write)**. @@ -353,10 +356,18 @@ board's environment — the compose file, `docker inspect`, a shell in the container — can use it. If you would rather not have that on the box, leave `GITEA_ADMIN_TOKEN` empty and create accounts in Gitea by hand. -Leaving it empty is a supported configuration, not a half-finished one: the -*Dar de alta* form checks for the token when it renders, says plainly that this -server cannot create accounts, and links to Gitea's own create-user page. You -then add that username here, with the checkbox already off. +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. + +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. ### 11.3 Build and run diff --git a/scripts/deploy-board.sh b/scripts/deploy-board.sh index 17c8a5e..e46d75a 100755 --- a/scripts/deploy-board.sh +++ b/scripts/deploy-board.sh @@ -53,6 +53,26 @@ if [ -n "$missing" ]; then echo fi +# Present but empty, which the check above cannot see: `NAME=` has the name. +# Worth its own warning, because an empty value is how a feature ends up +# switched off while looking configured — GITEA_ADMIN_TOKEN= reads as a settled +# decision and behaves as a missing one. Each of these disables something whole. +blank="$(grep -oE '^[A-Z][A-Z0-9_]*=[[:space:]]*$' "$TARGET/.env" | sed 's/=.*//' || true)" +if [ -n "$blank" ]; then + echo + echo "!! These are set to nothing in your .env, so their feature is off:" + while read -r name; do + case "$name" in + GITEA_ADMIN_TOKEN) note="no se pueden crear cuentas ni cambiar contraseñas" ;; + MAIL_HOST|MAIL_PASSWORD) note="no se envían invitaciones ni recuperaciones" ;; + BOARD_SECRET_KEY) note="LA APP NO ARRANCA" ;; + *) note="" ;; + esac + printf ' %-20s %s\n' "$name" "$note" + done <<< "$blank" + echo +fi + echo "==> Restarting" cd "$TARGET" docker compose up -d --force-recreate