diff --git a/apps/board/members.py b/apps/board/members.py index 5d2d52a..09a3ce9 100644 --- a/apps/board/members.py +++ b/apps/board/members.py @@ -201,6 +201,45 @@ def new(): role_label=ROLE_LABELS[role]) +@bp.route("/miembros//invitar", methods=["POST"]) +@admin_required +def invite(member_id: int): + """Send a fresh invitation to somebody who is already a member. + + This exists because creating the account and inviting the person were one + action, and there are several ordinary ways to end up needing only the + second: the admin made the account by hand in the account server and added + the member with the box unticked, the mail failed the first time, the link + sat unopened for more than a week, or the address was wrong and has been + corrected. Before this, every one of those left a member with an account + nobody knows the password to and no way to reach it. + + Issuing a new token invalidates the previous one, which `invites.issue` + already guarantees — so a link that has been forwarded, or is sitting in a + mailbox somebody else can read, stops working the moment a new one is sent. + """ + target = _load(member_id) + if not target["email"]: + flash(f"{target['display_name']} no tiene correo. Añádelo primero.", "error") + return redirect(url_for("members.index")) + if not target["active"] or target["role"] == "tombstone": + abort(403) + + link = auth.invite_url(invites.issue(member_id, "invite")) + try: + mail.send_invite(target["email"], target["display_name"], link) + except (mail.MailNotConfigured, mail.MailFailed) as exc: + # The link is shown rather than withheld: it is already issued and + # valid, and the alternative is an admin who knows only that something + # did not work. + current_app.logger.warning("Invite mail to %s failed: %s", target["email"], exc) + flash(f"No se pudo enviar el correo. Pásale este enlace: {link}", "error") + return redirect(url_for("members.index")) + + flash(f"Invitación enviada a {target['email']}.", "ok") + return redirect(url_for("members.index")) + + @bp.route("/miembros//estado", methods=["POST"]) @admin_required def set_active(member_id: int): diff --git a/apps/board/templates/members.html b/apps/board/templates/members.html index 880a97a..c061cbf 100644 --- a/apps/board/templates/members.html +++ b/apps/board/templates/members.html @@ -35,6 +35,19 @@ + {# Any admin can re-send, not just the owner: the usual reason + somebody needs this is that the admin who added them made + the account by hand, and waiting for the owner to be + around defeats the point. Hidden without an address, + because the handler would only refuse. #} + {% if m.email and m.active %} +
+ + +
+ {% endif %} + {% if is_owner %}
diff --git a/apps/board/tests/test_invites.py b/apps/board/tests/test_invites.py index 59d087b..8b45e4e 100644 --- a/apps/board/tests/test_invites.py +++ b/apps/board/tests/test_invites.py @@ -384,3 +384,92 @@ def test_a_member_without_a_gitea_account_is_named_as_such( 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 ---------------------------- +# +# Creating the account and inviting the person used to be one action, so a +# member added any other way had no route in at all: no invitation was ever +# issued for them and nothing could issue one later. + +def test_an_existing_member_can_be_invited(app, client, post, owner_id, make_member, + sign_in, outbox): + """The case that prompted this: the admin made the account by hand, added + the member with the box unticked, and nobody could reach the account — + including the admin, who never knew the password.""" + member_id = make_member("salvador") + sign_in(owner_id) + + post(f"/comunidad/miembros/{member_id}/invitar") + + assert len(outbox) == 1 + assert outbox[0]["to"] == "salvador@example.com" + with app.test_request_context(): + assert invites.lookup(token_in(outbox[0]["body"])) is not None + + +def test_re_inviting_kills_the_previous_link(app, client, post, owner_id, make_member, + sign_in, outbox): + """A link that was forwarded, or is sitting in a mailbox somebody else can + read, must stop working the moment a replacement is sent.""" + member_id = make_member("salvador") + sign_in(owner_id) + + post(f"/comunidad/miembros/{member_id}/invitar") + post(f"/comunidad/miembros/{member_id}/invitar") + + with app.test_request_context(): + assert invites.lookup(token_in(outbox[0]["body"])) is None + assert invites.lookup(token_in(outbox[1]["body"])) is not None + + +def test_an_admin_can_invite_without_waiting_for_the_owner( + app, client, post, make_member, sign_in, outbox): + sign_in(make_member("admina", role="admin")) + post(f"/comunidad/miembros/{make_member('salvador')}/invitar") + assert len(outbox) == 1 + + +def test_a_plain_user_cannot_invite(client, post, make_member, sign_in, outbox): + sign_in(make_member("cualquiera")) + response = post(f"/comunidad/miembros/{make_member('salvador')}/invitar") + + assert response.status_code == 403 + assert outbox == [] + + +def test_a_member_with_no_address_is_refused_before_a_token_is_made( + app, client, db, post, owner_id, sign_in, outbox): + """Issuing the token first would invalidate a previous, working invitation + in exchange for one that cannot be delivered.""" + member_id = db.execute( + "INSERT INTO members (gitea_login, display_name, role) VALUES ('sincorreo', 'Sin', 'user')" + ).lastrowid + sign_in(owner_id) + + post(f"/comunidad/miembros/{member_id}/invitar", follow_redirects=True) + + assert outbox == [] + assert db.execute("SELECT 1 FROM invites").fetchone() is None + + +def test_a_suspended_member_cannot_be_invited(client, post, owner_id, make_member, + sign_in, outbox): + sign_in(owner_id) + response = post(f"/comunidad/miembros/{make_member('fuera', active=0)}/invitar") + + assert response.status_code == 403 + assert outbox == [] + + +def test_when_the_mail_fails_the_admin_is_handed_the_link( + app, client, post, owner_id, make_member, sign_in, monkeypatch): + def explode(to, subject, body): + raise mail.MailFailed("connection refused") + monkeypatch.setattr(mail, "send", explode) + sign_in(owner_id) + + page = post(f"/comunidad/miembros/{make_member('salvador')}/invitar", + follow_redirects=True).get_data(as_text=True) + + assert "/comunidad/invitacion/" in page diff --git a/docs/server-setup.md b/docs/server-setup.md index bc3da10..28ba5da 100644 --- a/docs/server-setup.md +++ b/docs/server-setup.md @@ -639,14 +639,26 @@ GITEA__other__SHOW_FOOTER_VERSION=false ``` `APP_NAME` lives in `app.ini`'s unnamed root section, which the environment -mapping spells `DEFAULT`. **Check it took**, because a key written to a section -that does not exist is accepted in silence: +mapping spells `DEFAULT`. + +**`/srv/gitea/docker-compose.yml` is a copy, and nothing kept it in step with +this repository.** That is worth stating plainly because it cost a week: every +Gitea setting added here — CORS, the theme, OpenID, the register button, the +footer — was committed and documented and never reached the server, because the +only thing that syncs a compose file is `deploy-board.sh`, and it syncs the +board's. The file on the server stays valid, the container stays healthy, and +the setting is simply absent. ```sh -cd /srv/gitea && sudo docker compose up -d -sudo docker compose exec gitea head -5 /data/gitea/conf/app.ini +cd ~/vienalatina && sudo bash scripts/deploy-gitea.sh ``` +That copies `infra/gitea/docker-compose.yml` across (keeping the old one as +`.bak` and printing the diff, since it may have been hand-edited), restarts, +and then **reads the settings back out of the running container** and prints +them. Treat that output as the only evidence: a line missing there is a setting +not in effect, whatever the compose file says. + That should show `APP_NAME = Viena Latina`. Hiding the version is the one with a security argument as well as a cosmetic one: it tells a passer-by exactly which advisories to try. @@ -672,17 +684,20 @@ git pull --no-rebase --no-edit gitea main bash scripts/gitea-theme.sh ``` -Then add the line the script prints to `/srv/gitea/docker-compose.yml` (it is -already in `infra/gitea/docker-compose.yml`): - -``` -- GITEA__ui__DEFAULT_THEME=vienalatina -``` +`GITEA__ui__DEFAULT_THEME=vienalatina` is already in +`infra/gitea/docker-compose.yml`, so it arrives with the sync — no hand-editing +of the server's copy: ```sh -cd /srv/gitea && sudo docker compose up -d +cd ~/vienalatina && sudo bash scripts/deploy-gitea.sh ``` +The script prints `DEFAULT_THEME` back out of the container. If it says +`vienalatina` and the screens are still grey, the setting is fine and the +*theme file* was never built — run `bash scripts/gitea-theme.sh` first. Those +are two different failures with one symptom, which is why the script names +both. + Check it in a private window at **https://git.vienalatina.com/user/login** — cream background, the Viena Latina wordmark, `#c0391c` buttons. Then browse a repository and open a commit: a theme that only looks right on the login page is @@ -807,12 +822,20 @@ behind it. ### 13.4 The sign-in page loses two tabs `GITEA__openid__ENABLE_OPENID_SIGNIN=false` and -`GITEA__service__SHOW_REGISTRATION_BUTTON=false` in -`/srv/gitea/docker-compose.yml`. OpenID is sign-in with an external identity +`GITEA__service__SHOW_REGISTRATION_BUTTON=false`, already in +`infra/gitea/docker-compose.yml`. OpenID is sign-in with an external identity URL, which nobody here will use, and the register button contradicts `DISABLE_REGISTRATION` — it invited people to try something the server then refused. +It also loses a third thing, the *Forgot password?* link, which goes to a page +that answers "Account recovery is disabled because no email is set up" and +always will: the SMTP details are the members area's, and this container has no +mailer and needs none. Recovery lives at **vienalatina.com/comunidad/recuperar** +and works. The link is hidden by a rule in the theme file rather than by +replacing the template, so a Gitea upgrade cannot quietly undo it — and if the +selector ever stops matching, the link reappears rather than the page breaking. + ```sh -cd /srv/gitea && sudo docker compose up -d +cd ~/vienalatina && sudo bash scripts/deploy-gitea.sh ``` diff --git a/infra/gitea/theme-vienalatina.overrides.css b/infra/gitea/theme-vienalatina.overrides.css index eb8b4e7..32a10fb 100644 --- a/infra/gitea/theme-vienalatina.overrides.css +++ b/infra/gitea/theme-vienalatina.overrides.css @@ -96,3 +96,22 @@ height: 26px !important; width: auto !important; } + +/* The dead "Forgot password?" link on the sign-in form. + * + * It goes to /user/forgot_password, which answers "Account recovery is + * disabled because no email is set up" and always will: the SMTP details live + * in the members area's environment, and this is a different container that + * has no mailer and does not need one. Recovery is at + * vienalatina.com/comunidad/recuperar instead, which works. + * + * Every member who forgets a password reaches the sign-in form first, so + * leaving the link there means the broken route is the one they find. Hidden + * in CSS rather than by overriding the template, because a replaced template + * has to be re-checked against every Gitea release, whereas an attribute + * selector that stops matching leaves the link visible — the state we are in + * today, not a broken page. + */ +a[href$="/user/forgot_password"] { + display: none !important; +} diff --git a/scripts/deploy-gitea.sh b/scripts/deploy-gitea.sh new file mode 100755 index 0000000..97598ab --- /dev/null +++ b/scripts/deploy-gitea.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# Put this repository's Gitea settings onto the server, and prove they landed. +# +# This script exists because there was no way to do that. `deploy-board.sh` +# syncs the members area; `/srv/gitea/` has been a hand-made copy since the day +# it was set up, so every Gitea setting added to `infra/gitea/` since then has +# been written, committed, documented — and never applied. The symptom is the +# quietest possible one: the file on the server is valid, the container is +# healthy, and the setting simply is not there. +# +# cd ~/vienalatina && git pull ... +# sudo bash scripts/deploy-gitea.sh +# +# It never touches /srv/gitea/data — that is the database, the repositories and +# app.ini, none of which belong to this repository. + +set -euo pipefail + +REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +TARGET="${GITEA_DIR:-/srv/gitea}" + +if [ ! -d "$TARGET" ]; then + echo "No $TARGET — this script updates an existing install, it does not create one." >&2 + exit 1 +fi + +echo "==> Syncing compose settings into $TARGET" +# A backup, because unlike the board's this file may have been edited by hand on +# the server, and that edit is about to be overwritten. If anything in the diff +# below is a surprise, it was a local change nobody wrote down. +if [ -f "$TARGET/docker-compose.yml" ]; then + cp "$TARGET/docker-compose.yml" "$TARGET/docker-compose.yml.bak" + if ! diff -u "$TARGET/docker-compose.yml.bak" "$REPO/infra/gitea/docker-compose.yml"; then + echo " (differences above; previous file kept as docker-compose.yml.bak)" + fi +fi +cp "$REPO/infra/gitea/docker-compose.yml" "$TARGET/docker-compose.yml" + +echo "==> Restarting" +cd "$TARGET" +docker compose up -d + +# Settling time. environment-to-ini rewrites app.ini during start-up, so +# reading it back immediately can catch the previous file. +sleep 5 + +echo +echo "==> What the container actually has (not what we sent it)" +# The whole point of this script. Every failure this session has been a setting +# that was accepted somewhere and read by nobody, so the last word belongs to +# the running container rather than to a file we just copied. +docker compose exec -T gitea sh -c ' + echo "--- APP_NAME ---" + grep -m1 "^APP_NAME" /data/gitea/conf/app.ini || echo "APP_NAME: not set (Gitea will show its own name)" + echo "--- [other] ---" + sed -n "/^\[other\]/,/^\[/p" /data/gitea/conf/app.ini | grep -v "^\[" || echo "no [other] section" + echo "--- sign-in extras ---" + grep -E "^(ENABLE_OPENID_SIGNIN|SHOW_REGISTRATION_BUTTON|DEFAULT_THEME)" /data/gitea/conf/app.ini \ + || echo "none of ENABLE_OPENID_SIGNIN / SHOW_REGISTRATION_BUTTON / DEFAULT_THEME are set" +' || echo "!! Could not read app.ini — is the container up? docker compose logs gitea" + +echo +echo "Expected: APP_NAME = Viena Latina, SHOW_FOOTER_POWERED_BY = false," +echo "SHOW_FOOTER_VERSION = false, ENABLE_OPENID_SIGNIN = false," +echo "SHOW_REGISTRATION_BUTTON = false, DEFAULT_THEME = vienalatina." +echo +echo "A line that is missing above is a setting that is NOT in effect, whatever" +echo "the compose file says. If DEFAULT_THEME is set but the screens are still" +echo "grey, the theme file was never built: bash scripts/gitea-theme.sh"