Merge branch 'claude/relaxed-faraday-h4zd09' of https://github.com/pablovolenski/vienalatina

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

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}).")
 
 

apps/board/templates/login.html

@@ -9,9 +9,13 @@
 		cuenta que se usa para publicar en el sitio.
 	</p>
 	<a class="btn" href="{{ url_for('auth.start', next=next) }}">Entrar con Gitea</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 %}
 	<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.

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. #}
+<article class="card card--center">
+	<h1>Todavía no</h1>
+	<p class="muted">
+		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.
+	</p>
+	<p><a class="linkish" href="{{ url_for('auth.login') }}">Volver a entrar</a></p>
+</article>
+{% elif sent %}
 <article class="card card--center">
 	<h1>Revisa tu correo</h1>
 	{# Says the same thing whether or not that address belongs to a member.

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. #}
+<article class="card card--center">
+	<h1>Todavía no podemos guardar tu contraseña</h1>
+	<p class="muted">
+		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.
+	</p>
+</article>
+{% 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. #}
 <article class="card card--center">

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

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
+This was optional when the members area only created accounts. **It is not
-else changes. With it, they can create the Gitea account from inside the members
+optional any more**, because the same token is what lets a member choose their
-area and hand over a one-time password.
+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
+Leaving it empty is a supported configuration, not a half-finished one, and
-*Dar de alta* form checks for the token when it renders, says plainly that this
+every screen that depends on it checks **before** asking anyone to do work: the
-server cannot create accounts, and links to Gitea's own create-user page. You
+*Dar de alta* form says this server cannot create accounts and links to Gitea's
-then add that username here, with the checkbox already off.
+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
 

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