From d7a2d5bf01a8726dd6806b000fa59cd081a86d31 Mon Sep 17 00:00:00 2001 From: Ashwin Ramachandran Date: Tue, 11 Aug 2026 11:53:35 +0530 Subject: [PATCH] The claim page says whose account it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Somebody following a setup or reset link was asked to choose a password with no indication of which account it belonged to. A link is shared out of band, so the person holding it may have been sent the wrong one, or two of them. That was tolerable while the only link set a FIRST password on an account the recipient was expecting. It is not now that a link can REPLACE a working one: following the wrong link silently locks somebody out of their own account, and nothing on the page would have told them. It discloses nothing new. Whoever holds the link already holds a token whose payload is base64url and carries the same address in the clear — the module note says so. This turns something they could decode into something they can check. Asserted on both purposes and on the re-render after a rejected password, which is the render most easily forgotten and the one somebody is staring at when they are already confused. Mutation-checked: drop the name and it fails. Spec: ACE-108 --- packages/agami-core/src/onboarding.py | 28 +++++++++++++++++++++++---- tests/test_onboarding.py | 17 ++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/agami-core/src/onboarding.py b/packages/agami-core/src/onboarding.py index d4c9e46..8630f64 100644 --- a/packages/agami-core/src/onboarding.py +++ b/packages/agami-core/src/onboarding.py @@ -179,11 +179,26 @@ def is_pending(user: dict[str, Any]) -> bool: } -def claim_page_html(token: str, purpose: str = _SETUP_PURPOSE, error: str = "") -> str: - """The choose-a-password page reached from a valid link, worded for what the link is for.""" +def claim_page_html( + token: str, purpose: str = _SETUP_PURPOSE, error: str = "", username: str = "" +) -> str: + """The choose-a-password page reached from a valid link, worded for what the link is for. + + **It names the account.** Without that, somebody following a link is asked to choose a password + with no way to tell WHOSE it is — and the link is shared out-of-band, so the person holding it may + have been sent the wrong one, or two of them. That was tolerable while the only link set a first + password on an account the recipient was expecting; it is not now that a link can REPLACE a working + password, where following the wrong one silently locks somebody out of their own account. + + It discloses nothing new: whoever holds the link already holds a token whose payload is base64url + and carries this same address in the clear (the module note says so). Showing it turns something + they could decode into something they can check. + """ words = _WORDING[purpose] alert = f'
{ui.esc(error)}
' if error else "" + whose = f'

for {ui.esc(username)}

' if username else "" body = f""" {alert}
@@ -300,7 +315,7 @@ async def claim(request: Request) -> Response: actionable = _actionable(token) if actionable is None: return HTMLResponse(setup_invalid_html(), status_code=400) - return HTMLResponse(claim_page_html(token, actionable[1])) + return HTMLResponse(claim_page_html(token, actionable[1], username=actionable[0])) form = await _form(request) token = form.get("token", "") @@ -311,7 +326,12 @@ async def claim(request: Request) -> Response: password = form.get("password", "") if not _MIN_PASSWORD_LEN <= len(password) <= _MAX_PASSWORD_LEN: return HTMLResponse( - claim_page_html(token, purpose, error=f"Use at least {_MIN_PASSWORD_LEN} characters."), + claim_page_html( + token, + purpose, + error=f"Use at least {_MIN_PASSWORD_LEN} characters.", + username=username, + ), status_code=400, ) # Off the event loop. Argon2 is deliberately slow and memory-hard, and this handler is `async` on a diff --git a/tests/test_onboarding.py b/tests/test_onboarding.py index 3ca71c5..170b828 100644 --- a/tests/test_onboarding.py +++ b/tests/test_onboarding.py @@ -144,6 +144,23 @@ def test_claim_link_is_single_use(client, env): s.close() +def test_the_claim_page_says_whose_account_it_is(client, env): + """A link is shared out-of-band, so the person holding it may have been sent the wrong one — or + two. While the only link set a FIRST password that was tolerable; a reset link replaces a working + one, and following the wrong one locks somebody out of their own account. + + Asserted on both purposes and on the re-render after a rejected password, because that last one is + the render most easily forgotten and the one somebody is staring at when they are confused. + """ + setup = onboarding.mint_setup_token(PENDING) + assert PENDING in client.get("/claim", params={"token": setup}).text + reset = _reset_token(env, ADMIN_USER) + assert ADMIN_USER in client.get("/claim", params={"token": reset}).text + # ...and when the password is refused for being too short. + again = client.post("/claim", data={"token": setup, "password": "short"}) + assert again.status_code == 400 and PENDING in again.text + + def test_claim_rejects_a_bad_token(client): assert client.get("/claim", params={"token": "nope"}, follow_redirects=False).status_code == 400 assert (