diff --git a/apps/board/templates/logged_out.html b/apps/board/templates/logged_out.html index d3bd326..16ed73b 100644 --- a/apps/board/templates/logged_out.html +++ b/apps/board/templates/logged_out.html @@ -13,14 +13,20 @@ que quien use este ordenador después podría volver a entrar sin escribirla.
+ {# 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. #}- Cerrar sesión del todo + Ir al servidor de cuentas
- Si esa página no te desconecta, abre el menú de tu perfil - ahí y elige Sign Out. - Cerrar el navegador también sirve. + Allí, abre el menú de tu perfil (arriba a la derecha) y elige + Cerrar sesión. Cerrar el navegador del todo también sirve.
diff --git a/apps/board/tests/test_auth.py b/apps/board/tests/test_auth.py index a5985d4..19317c2 100644 --- a/apps/board/tests/test_auth.py +++ b/apps/board/tests/test_auth.py @@ -124,7 +124,12 @@ def test_logout_says_the_gitea_session_is_still_open(client, db, make_member, si assert response.status_code == 200 assert "sigue conectado" in page # the warning, not a redirect - assert "/user/logout" in page + # 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 with client.session_transaction() as session: assert "member_id" not in session diff --git a/apps/board/tests/test_templates.py b/apps/board/tests/test_templates.py index 61f76d4..8174ec8 100644 --- a/apps/board/tests/test_templates.py +++ b/apps/board/tests/test_templates.py @@ -38,6 +38,22 @@ def test_every_post_form_carries_a_csrf_token(template): assert "csrf_token" in form, f"{template.name} has a POST form without a CSRF token" +@pytest.mark.parametrize("template", TEMPLATES, ids=lambda p: p.name) +def test_nothing_links_to_the_account_server_logout(template): + """A link there is a GET, and that route is POST-only, so the browser gets + a 404 and the session it was meant to end carries on. We shipped exactly + that and it went unnoticed for a week, because a dead link on a page nobody + reaches twice looks like nothing at all. + + It cannot be fixed by turning the link into a form either: the POST needs a + CSRF token belonging to that other domain, which is unreadable from here by + design. The page has to tell the member what to do instead.""" + html = JINJA_COMMENT.sub("", template.read_text(encoding="utf-8")) + assert "/user/logout" not in html, ( + f"{template.name} links to /user/logout, which answers 404 to a GET" + ) + + # --- the name of the software behind the login --------------------------- # # Members sign in through an OAuth provider that happens to be Gitea. They are diff --git a/docs/server-setup.md b/docs/server-setup.md index 28ba5da..1838054 100644 --- a/docs/server-setup.md +++ b/docs/server-setup.md @@ -734,10 +734,21 @@ 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. -So the logout page says plainly what is and is not closed, and offers the link -that finishes the job. On a shared computer, use it — or close the browser, -which also works. The members-area cookie is already a browser-session cookie, -so it does not survive that either way. +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. + +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. + +`apps/board/tests/test_templates.py` now fails the build if any template links +to `/user/logout` again. ## 13. Email: invitations and passwords