amadan
web: three screens, not one scroll — and two buttons, because two files
Three things Paul hit in one pass, all of them the same kind of mistake: assuming the browser would do what the code implied. **The kit appeared under the passphrase form.** It rendered into the form's own status box, so the fields you had just used sat above the recovery code you were meant to be reading and saving. The passphrase step and the kit are now separate screens, with the working screen between them, and only one is ever visible. They cannot be three URLs, and it is worth saying why: the kit exists only in memory. The recovery code is not stored by anyone, including us, so a redirect between choosing a passphrase and seeing it would be a redirect that throws it away. The document swaps instead. **Nothing happened for several seconds.** Creating a vault is two seconds of deliberate Argon2id plus two round trips, and the only sign of life was a button that had gone grey. There is now a working screen that names each task as it starts — and the names come from the crypto module's own progress messages, so the list cannot drift out of step with the work it describes. A progress list that is really a timer is a lie with a spinner on it. **"Save both documents" saved one.** Browsers refuse a second programmatic download from a single gesture, and Safari refuses it silently, so the button delivered one file and reported success. There is a button per document now, each marking itself saved. That is also the truer shape. DESIGN.md asks for one gesture and its reason for two documents is that they belong in two different places — an interface that hands them over as a single action quietly argues against the thing the split exists to encourage. Recorded on discussion 19 rather than changed silently. Two new browser walks: one that fails if the kit ever offers fewer than two save buttons or if clicking one marks the other, and one that fails if the working screen's progress list never marks a task — a list that cannot show progress is decoration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 files changed,
+257
−78
internal/contra/static/pages/signup.js+98 −51internal/contra/templates/signup_passphrase.html+59 −24internal/contratest/browser_test.go+99 −2internal/contratest/jsmodules_test.go+1 −1
diff --git a/internal/contra/static/pages/signup.js b/internal/contra/static/pages/signup.js| index 2c29782..93bd056 100644 |
| --- a/internal/contra/static/pages/signup.js |
| +++ b/internal/contra/static/pages/signup.js |
| @@ -1,12 +1,12 @@ |
| -// Signup, one step per screen. |
| +// Signup, one screen per step. |
| // |
| // The screen this replaced did six seconds of work behind one button and, when |
| // anything went wrong, printed whichever DOMException happened to surface. So |
| -// the shape here is deliberate: each step is small, each says what it is |
| -// doing, and each failure names the operation it died in and leaves the whole |
| -// error in the console. |
| +// the shape here is deliberate: each step is a screen of its own, each says |
| +// what it is doing while it does it, and each failure names the operation it |
| +// died in and leaves the whole error in the console. |
| -import { signIn, signup, AlreadyRegisteredError } from "../crypto/account.js"; |
| +import { signup, AlreadyRegisteredError } from "../crypto/account.js"; |
| import { accountKeyCard, recoverySheet } from "../crypto/kit.js"; |
| import { formatCode, TAG_ACCOUNT_KEY, TAG_RECOVERY } from "../crypto/code.js"; |
| import { net, RequestError } from "../crypto/net.js"; |
| @@ -32,14 +32,24 @@ function replace(node, ...children) { |
| } |
| function say(form, text, kind = "working") { |
| - form.querySelector("[data-status]") |
| - .replaceChildren(el("p", { "data-kind": kind }, text)); |
| + const box = form.querySelector("[data-status]"); |
| + if (box) replace(box, el("p", { "data-kind": kind }, text)); |
| } |
| function busy(form, on) { |
| for (const c of form.querySelectorAll("input, button")) c.disabled = on; |
| } |
| +// show switches which screen of this document is the screen. One at a time, |
| +// and back to the top each time — a screen that appears underneath the one |
| +// you were reading is not a new screen, it is a longer old one. |
| +function show(name) { |
| + for (const s of document.querySelectorAll("[data-screen]")) { |
| + s.hidden = s.dataset.screen !== name; |
| + } |
| + window.scrollTo(0, 0); |
| +} |
| + |
| // report is the failure path every step shares. |
| // |
| // A bare DOMException message — "Data provided to an operation does not meet |
| @@ -93,36 +103,74 @@ if (beginForm) { |
| // ------------------------------------------------- step 2: the passphrase |
| -// The kit is shown exactly once, here. The recovery code is not stored by |
| -// anyone, including us — only a sealed copy of the vault key that it opens — |
| -// so this is the single moment it exists outside the person's hands. |
| -function showKit(form, accountKeyPrinted, recoveryPrinted, accountIdHex, cached) { |
| - const box = form.querySelector("[data-status]"); |
| - // Filtered, because replaceChildren stringifies a null child into the word |
| - // "null" on the page. el() skips nulls among its own children; this is the |
| - // one call that does not go through it. |
| - replace(box, |
| - el("h2", {}, "Your Emergency Kit"), |
| +// download hands the browser one file. Both documents are built here and the |
| +// bytes never leave: a kit that touched the server would put the recovery |
| +// code somewhere it must never be, for the length of one request. |
| +function download(bytes, filename) { |
| + const url = URL.createObjectURL(new Blob([bytes], { type: "application/pdf" })); |
| + const a = el("a", { href: url, download: filename }); |
| + document.body.append(a); |
| + a.click(); |
| + a.remove(); |
| + // Revoked on a later tick: some browsers have not started reading the blob |
| + // by the time click() returns. |
| + setTimeout(() => URL.revokeObjectURL(url), 30_000); |
| +} |
| + |
| +// The kit is shown exactly once. The recovery code is not stored by anyone, |
| +// including us — only a sealed copy of the vault key that it opens — so this |
| +// is the single moment it exists outside the person's hands. |
| +function showKit(accountKeyPrinted, recoveryPrinted, accountIdHex, cached) { |
| + const body = document.querySelector("[data-kit-body]"); |
| + const short = accountIdHex.slice(0, 8); |
| + |
| + // One button per document, not one button for both. |
| + // |
| + // Browsers refuse a second programmatic download from a single gesture — |
| + // Safari silently, which is how "Save both documents" delivered one file |
| + // and said nothing. Two buttons is also the truer shape: the whole reason |
| + // there are two documents is that they belong in two different places, and |
| + // handing them over as one action quietly argues the opposite. |
| + const saver = (label, make, name) => { |
| + const button = el("button", { type: "button", "data-save": name }, label); |
| + button.addEventListener("click", () => { |
| + download(make(), `contra-${name}-${short}.pdf`); |
| + button.replaceChildren(document.createTextNode(label + " — saved")); |
| + button.setAttribute("data-saved", ""); |
| + }); |
| + return button; |
| + }; |
| + |
| + replace(body, |
| el("p", {}, |
| "Two documents, shown once. Nobody can reissue the recovery code, " + |
| - "because nobody stores it."), |
| - el("h3", {}, "Account Key card — store somewhere safe"), |
| + "because nobody stores it. Save them to two different places."), |
| + |
| + el("h2", {}, "Account Key card — store somewhere safe"), |
| el("p", {}, |
| "Keep it where you can find it and nobody else goes: a drawer, a " + |
| "wallet, a folder with your documents. It is useless to anyone who " + |
| "does not also know your passphrase. A new device asks for it."), |
| el("pre", { "data-kit": "account-key" }, accountKeyPrinted), |
| - el("h3", {}, "Recovery sheet — store securely"), |
| + el("p", {}, saver("Save the Account Key card", |
| + () => accountKeyCard(accountIdHex, accountKeyPrinted), "account-key")), |
| + |
| + el("h2", {}, "Recovery sheet — store securely"), |
| el("p", {}, |
| "Keep it where you would keep a passport or a will: a safe, a deposit " + |
| "box, a trusted person's house. Never photograph it. This sheet alone " + |
| "can open your vault."), |
| el("pre", { "data-kit": "recovery" }, recoveryPrinted), |
| + el("p", {}, saver("Save the Recovery sheet", |
| + () => recoverySheet(accountIdHex, recoveryPrinted), "recovery-sheet")), |
| + |
| cached ? null : el("p", { "data-kind": "warning" }, |
| "This browser will not remember your Account Key, so it will ask for " + |
| "the card next time. That is safe — just keep the card."), |
| + |
| el("p", {}, |
| - el("button", { type: "button", "data-save-kit": "" }, "Save both documents")), |
| + "If you lose both the kit and your passphrase, the data is gone. There " + |
| + "is no reset link and no way for us to let you back in."), |
| el("p", {}, |
| el("label", {}, |
| el("input", { type: "checkbox", "data-confirmed": "" }), |
| @@ -131,27 +179,25 @@ function showKit(form, accountKeyPrinted, recoveryPrinted, accountIdHex, cached) |
| el("a", { href: "/signup/passkey" }, "Continue")), |
| ); |
| - box.querySelector("[data-save-kit]").addEventListener("click", () => { |
| - for (const [bytes, name] of [ |
| - [accountKeyCard(accountIdHex, accountKeyPrinted), "account-key"], |
| - [recoverySheet(accountIdHex, recoveryPrinted), "recovery-sheet"], |
| - ]) { |
| - const url = URL.createObjectURL(new Blob([bytes], { type: "application/pdf" })); |
| - const a = el("a", { href: url, download: `contra-${name}-${accountIdHex.slice(0, 8)}.pdf` }); |
| - document.body.append(a); |
| - a.click(); |
| - a.remove(); |
| - setTimeout(() => URL.revokeObjectURL(url), 30_000); |
| - } |
| - }); |
| - |
| - // No way onward until the person says they have the documents. Walking |
| - // past this screen without the recovery code is the one mistake in this |
| - // product that cannot be undone later. |
| - const onward = box.querySelector("[data-onward]"); |
| - box.querySelector("[data-confirmed]").addEventListener("change", (e) => { |
| + // No way onward until the person says they have the documents. Walking past |
| + // this screen without the recovery code is the one mistake in this product |
| + // that cannot be undone later. |
| + const onward = body.querySelector("[data-onward]"); |
| + body.querySelector("[data-confirmed]").addEventListener("change", (e) => { |
| onward.hidden = !e.target.checked; |
| }); |
| + |
| + show("kit"); |
| +} |
| + |
| +// markProgress moves the working screen's list along. The names come from the |
| +// crypto module itself, so the list cannot drift out of step with the work it |
| +// claims to be describing. |
| +function markProgress(task) { |
| + for (const li of document.querySelectorAll("[data-progress] li")) { |
| + if (li.dataset.task === task) li.setAttribute("data-state", "doing"); |
| + else if (li.getAttribute("data-state") === "doing") li.setAttribute("data-state", "done"); |
| + } |
| } |
| const passphraseForm = document.querySelector("[data-passphrase]"); |
| @@ -165,34 +211,35 @@ if (passphraseForm) { |
| return; |
| } |
| busy(passphraseForm, true); |
| + show("working"); |
| let step = "start"; |
| try { |
| - const kit = await signup(passphrase, passphraseForm.dataset.kdfParams, storageFor(), net, (m) => { |
| - step = m; |
| - say(passphraseForm, m + "…"); |
| - }); |
| - showKit(passphraseForm, |
| + const kit = await signup(passphrase, passphraseForm.dataset.kdfParams, |
| + storageFor(), net, (m) => { |
| + step = m; |
| + markProgress(m); |
| + }); |
| + showKit( |
| await formatCode(TAG_ACCOUNT_KEY, kit.accountKey), |
| await formatCode(TAG_RECOVERY, kit.recoveryCode), |
| toHex(kit.accountId), kit.cached); |
| } catch (err) { |
| + // Back to the form: it is the screen with somewhere to put an error and |
| + // something to try again with. |
| + show("form"); |
| + busy(passphraseForm, false); |
| if (err instanceof AlreadyRegisteredError) { |
| - say(passphraseForm, |
| - `${err.message} Sign in instead.`, "error"); |
| - busy(passphraseForm, false); |
| + say(passphraseForm, `${err.message} Sign in instead.`, "error"); |
| return; |
| } |
| if (err instanceof RequestError && err.status === 403) { |
| say(passphraseForm, |
| "That link has expired or was already used. Start again from /signup.", |
| "error"); |
| - busy(passphraseForm, false); |
| return; |
| } |
| report(passphraseForm, step, err); |
| } |
| }); |
| } |
| - |
| -export { signIn }; |
diff --git a/internal/contra/templates/signup_passphrase.html b/internal/contra/templates/signup_passphrase.html| index ccca82e..9c8419e 100644 |
| --- a/internal/contra/templates/signup_passphrase.html |
| +++ b/internal/contra/templates/signup_passphrase.html |
| @@ -2,29 +2,64 @@ |
| {{define "brand"}}<a rst-shell-brand href="/">CONTRA</a>{{end}} |
| {{define "head"}}<script type="module" src="{{asset "static/pages/signup.js"}}"></script>{{end}} |
| +{{/* Three screens in one document, one visible at a time. |
| + They cannot be three URLs: the Emergency Kit exists only in memory, on |
| + purpose — the recovery code is never stored by anyone, so a redirect |
| + between choosing a passphrase and seeing the kit would be a redirect that |
| + throws it away. So the document swaps instead, and each screen is a whole |
| + screen rather than something appended under the last one. */}} |
| {{define "content"}} |
| -<h1>Choose a passphrase</h1> |
| -<p rst-steps>Step 2 of 3 · Your address is confirmed</p> |
| - |
| -<p>Your passphrase never leaves this page. Your vault key is generated here, |
| -in your browser, and the server only ever receives sealed copies of it that it |
| -cannot open.</p> |
| - |
| -<p>This is the slow step. Stretching a passphrase deliberately takes a couple |
| -of seconds, and that is what makes it expensive to guess.</p> |
| - |
| -<form data-passphrase data-kdf-params="{{.KDFParams}}" method="post" action="/auth/complete"> |
| - <p> |
| - <label for="passphrase">Passphrase</label> |
| - <input id="passphrase" name="passphrase" type="password" |
| - autocomplete="new-password" required minlength="12"> |
| - </p> |
| - <p> |
| - <label for="passphrase_again">Passphrase again</label> |
| - <input id="passphrase_again" name="passphrase_again" type="password" |
| - autocomplete="new-password" required minlength="12"> |
| - </p> |
| - <p><button type="submit">Create my vault</button></p> |
| - <div data-status aria-live="polite"></div> |
| -</form> |
| + |
| +<section data-screen="form"> |
| + <h1>Choose a passphrase</h1> |
| + <p rst-steps>Step 2 of 3 · Your address is confirmed</p> |
| + |
| + <p>Your passphrase never leaves this page. Your vault key is generated here, |
| + in your browser, and the server only ever receives sealed copies of it that |
| + it cannot open.</p> |
| + |
| + <form data-passphrase data-kdf-params="{{.KDFParams}}" method="post" action="/auth/complete"> |
| + <p> |
| + <label for="passphrase">Passphrase</label> |
| + <input id="passphrase" name="passphrase" type="password" |
| + autocomplete="new-password" required minlength="12"> |
| + </p> |
| + <p> |
| + <label for="passphrase_again">Passphrase again</label> |
| + <input id="passphrase_again" name="passphrase_again" type="password" |
| + autocomplete="new-password" required minlength="12"> |
| + </p> |
| + <p><button type="submit">Create my vault</button></p> |
| + <div data-status aria-live="polite"></div> |
| + </form> |
| +</section> |
| + |
| +<section data-screen="working" hidden> |
| + <h1>Creating your vault</h1> |
| + <p rst-steps>Step 2 of 3 · This takes a few seconds</p> |
| + |
| + <p>Stretching your passphrase is deliberately slow — a second of work per |
| + guess is what makes guessing expensive. Everything below is happening in |
| + this browser.</p> |
| + |
| + {{/* The names match the progress messages the crypto module emits, so the |
| + list is driven by what is actually happening rather than by a timer |
| + pretending to know. */}} |
| + <ol data-progress> |
| + <li data-task="stretch your passphrase">Stretching your passphrase</li> |
| + <li data-task="build your keys">Building your keys</li> |
| + <li data-task="seal your vault key">Sealing your vault key</li> |
| + <li data-task="create your account">Creating your account</li> |
| + <li data-task="open your vault">Opening your vault</li> |
| + </ol> |
| + |
| + <p>Please keep this tab open.</p> |
| +</section> |
| + |
| +<section data-screen="kit" hidden> |
| + <h1>Your Emergency Kit</h1> |
| + <p rst-steps>Step 2 of 3 · Save these before you go on</p> |
| + <div data-kit-body></div> |
| +</section> |
| + |
| {{end}} |
diff --git a/internal/contratest/browser_test.go b/internal/contratest/browser_test.go| index d9ea76e..edec930 100644 |
| --- a/internal/contratest/browser_test.go |
| +++ b/internal/contratest/browser_test.go |
| @@ -137,7 +137,7 @@ func TestBrowserSignupAndSignIn(t *testing.T) { |
| // Argon2id at 64 MiB takes a couple of seconds in a real engine, so the |
| // helper waits on the kit appearing rather than on a timer. |
| signUpInBrowser(t, rig, box, email, passphrase) |
| - rig.Screen("[data-status]", "emergency kit") |
| + rig.Screen(`[data-screen="kit"]`, "emergency kit") |
| // Both printed values must actually be on the page: this is the only |
| // moment either exists outside the user's hands. |
| @@ -259,6 +259,16 @@ func TestBrowserEmergencyKit(t *testing.T) { |
| signUpInBrowser(t, rig, box, email, passphrase) |
| + // The passphrase form must be OFF the screen by now. It appearing above |
| + // the kit was the bug: two steps stacked in one scroll, with the fields |
| + // you just used still sitting there. |
| + var formVisible bool |
| + rig.Run(chromedp.Evaluate( |
| + `!document.querySelector('[data-screen="form"]').hidden`, &formVisible)) |
| + if formVisible { |
| + t.Error("the passphrase form is still on screen behind the kit") |
| + } |
| + |
| // Both printed values are on screen, and they are different values. |
| var accountKey, recovery string |
| rig.Run( |
| @@ -299,5 +309,92 @@ func TestBrowserEmergencyKit(t *testing.T) { |
| chromedp.Click(`[data-confirmed]`, chromedp.ByQuery), |
| chromedp.WaitVisible(`[data-onward] a`, chromedp.ByQuery), |
| ) |
| - rig.Screen("[data-status]", "emergency kit confirmed") |
| + rig.Screen(`[data-screen="kit"]`, "emergency kit confirmed") |
| +} |
| + |
| +// The kit offers each document its own button. |
| +// |
| +// A single "save both" cannot work: browsers refuse the second programmatic |
| +// download from one gesture, and Safari refuses it silently — which is how |
| +// that button delivered one file and reported success. Two buttons is what |
| +// actually delivers two files, and it is also the shape that matches the |
| +// reason there are two documents. |
| +func TestBrowserKitSavesBothDocumentsSeparately(t *testing.T) { |
| + rig, box := newRig(t) |
| + signUpInBrowser(t, rig, box, "two-files@example.invalid", |
| + "a generated test passphrase 8f3a1c") |
| + |
| + var buttons int |
| + rig.Run(chromedp.Evaluate( |
| + `document.querySelectorAll('[data-kit-body] [data-save]').length`, &buttons)) |
| + if buttons != 2 { |
| + t.Fatalf("the kit offers %d save buttons, want one per document", buttons) |
| + } |
| + |
| + // Each builds a real PDF for its own document, and they are different |
| + // documents — a pair of buttons that saved the same file twice would look |
| + // identical from the outside. |
| + var sizes []int |
| + rig.Run(chromedp.Evaluate(`(async () => { |
| + const { accountKeyCard, recoverySheet } = await import("/static/crypto/kit.js"); |
| + const id = "b22c6a5eae1be9d6a773f474fd0532b0"; |
| + const card = document.querySelector('[data-kit="account-key"]').textContent; |
| + const sheet = document.querySelector('[data-kit="recovery"]').textContent; |
| + if (card === sheet) throw new Error("the two documents carry the same code"); |
| + const a = accountKeyCard(id, card); |
| + const b = recoverySheet(id, sheet); |
| + const head = (x) => new TextDecoder().decode(x.slice(0, 5)); |
| + if (head(a) !== "%PDF-" || head(b) !== "%PDF-") throw new Error("not a PDF"); |
| + return [a.length, b.length]; |
| + })()`, &sizes, awaitPromise)) |
| + if len(sizes) != 2 || sizes[0] < 2000 || sizes[1] < 2000 { |
| + t.Errorf("the kit documents look wrong: %v bytes", sizes) |
| + } |
| + |
| + // Clicking one marks that one saved, and only that one. |
| + rig.Run( |
| + chromedp.Click(`[data-kit-body] [data-save="account-key"]`, chromedp.ByQuery), |
| + chromedp.WaitVisible(`[data-save="account-key"][data-saved]`, chromedp.ByQuery), |
| + ) |
| + var recoverySaved bool |
| + rig.Run(chromedp.Evaluate( |
| + `document.querySelector('[data-save="recovery-sheet"]').hasAttribute("data-saved")`, |
| + &recoverySaved)) |
| + if recoverySaved { |
| + t.Error("saving one document marked the other saved too") |
| + } |
| +} |
| + |
| +// The working screen is a screen, not a spinner: it names what is happening |
| +// while a couple of seconds of Argon2id go by, so nobody concludes it hung. |
| +func TestBrowserShowsProgressWhileCreating(t *testing.T) { |
| + rig, box := newRig(t) |
| + |
| + rig.Run( |
| + chromedp.Navigate(rig.Origin+"/signup"), |
| + chromedp.SendKeys(`input[name=email]`, "progress@example.invalid", chromedp.ByQuery), |
| + chromedp.Click(`button[type=submit]`, chromedp.ByQuery), |
| + chromedp.WaitVisible(`[data-sent]`, chromedp.ByQuery), |
| + ) |
| + rig.Run( |
| + chromedp.Navigate(rig.Origin+box.linkFor(t, "progress@example.invalid")), |
| + chromedp.WaitVisible(`[data-passphrase]`, chromedp.ByQuery), |
| + chromedp.SendKeys(`input[name=passphrase]`, "a generated test passphrase 8f3a1c", chromedp.ByQuery), |
| + chromedp.SendKeys(`input[name=passphrase_again]`, "a generated test passphrase 8f3a1c", chromedp.ByQuery), |
| + chromedp.Click(`button[type=submit]`, chromedp.ByQuery), |
| + // The working screen appears immediately, long before the kit. |
| + chromedp.WaitVisible(`[data-screen="working"] [data-progress]`, chromedp.ByQuery), |
| + ) |
| + rig.Screen(`[data-screen="working"]`, "creating your vault") |
| + |
| + // And at least one task is marked as under way rather than the list just |
| + // sitting there inert. |
| + var doing int |
| + rig.Run(chromedp.Evaluate( |
| + `document.querySelectorAll('[data-progress] li[data-state]').length`, &doing)) |
| + if doing == 0 { |
| + t.Error("the progress list never marked a task, so it is decoration") |
| + } |
| + |
| + rig.Run(chromedp.WaitVisible(`[data-kit="recovery"]`, chromedp.ByQuery)) |
| } |
diff --git a/internal/contratest/jsmodules_test.go b/internal/contratest/jsmodules_test.go| index 0936de2..99594e2 100644 |
| --- a/internal/contratest/jsmodules_test.go |
| +++ b/internal/contratest/jsmodules_test.go |
| @@ -48,7 +48,7 @@ var jsHighWater = map[string]int{ |
| // a crypto module, and these only move text between a form and one. |
| "pages/passkey.js": 170, |
| "pages/signin.js": 49, |
| - "pages/signup.js": 198, |
| + "pages/signup.js": 245, |
| "pages/vault.js": 170, |
| } |