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 (