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