trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns11g rEventsFlows

/verify, /recover and /claim name their Server-Timing phases and answer under 50 ms #4324

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/4317-verify-recover-and-claim-name-their-serv → main 3 commits fork

  • Merged. scout/4317-verify-recover-and-claim-name-their-serv was merged into main as 007da1a.

Review

Sign in to review this change.
5 files changed +77 −13
src/web/pages/claim.ts +1 −1
@@ -230,7 +230,7 @@ const fragmentJs = /* js */ `
230230 export async function claimFragmentPage(ctx: Ctx): Promise<Response> {
231231 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
232232 const spawned = await timePhaseAsync(ctx.request, "spawned", () => spawnedOrg(ctx.app));
233 if (spawned === undefined) return dead(ctx, "unknown");
233 if (spawned === undefined) return timePhase(ctx.request, "render", () => dead(ctx, "unknown"));
234234 const { spawner } = timePhase(ctx.request, "owners", () => owners(ctx, spawned));
235235 const rendering = performance.now();
236236 const response = page({
src/web/pages/passkeys.ts +8 −6
@@ -234,9 +234,11 @@ const NOT_VALID = "This link has expired, been used, or is not valid. Ask for a
234234
235235 // GET /recover — the request form, or (with ?token=) the new-passkey step.
236236 export function recoverPage(ctx: Ctx): Response {
237 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
238 const render = (draw: () => Response): Response => timePhase(ctx.request, "render", draw);
237239 const token = ctx.url.searchParams.get("token");
238240 if (token === null) {
239 return page({
241 return render(() => page({
240242 ctx,
241243 title: "Recover your account",
242244 body: html`<div class="page-head"><h1>Recover your account</h1></div>
@@ -246,12 +248,12 @@ export function recoverPage(ctx: Ctx): Response {
246248 <input id="recover-email" name="email" type="email" maxlength="254" autocomplete="email" required /></div>
247249 <div class="form-actions"><button class="btn" data-variant="primary">Send the link</button></div>
248250 </form>`,
249 });
251 }));
250252 }
251 const user = recoveryAccount(ctx.app, token);
253 const user = timePhase(ctx.request, "link", () => recoveryAccount(ctx.app, token));
252254 if (user === undefined)
253 return page({ ctx, title: "Link not valid", body: html`<div class="page-head"><h1>Link not valid</h1></div><p>${NOT_VALID}</p><p><a href="/recover">Ask for a new link</a></p>` }, 400);
254 return page({
255 return render(() => page({ ctx, title: "Link not valid", body: html`<div class="page-head"><h1>Link not valid</h1></div><p>${NOT_VALID}</p><p><a href="/recover">Ask for a new link</a></p>` }, 400));
256 return render(() => page({
255257 ctx,
256258 title: "Register a new passkey",
257259 scriptJs: passkeyJs,
@@ -262,7 +264,7 @@ export function recoverPage(ctx: Ctx): Response {
262264 <p class="error-note" id="passkey-recover-note" role="status"></p>
263265 <noscript><p class="muted small">Registering a passkey needs JavaScript.</p></noscript>
264266 <div class="form-actions"><button type="button" id="passkey-recover" class="btn" data-variant="primary">Register and sign in</button></div>`,
265 });
267 }));
266268 }
267269
268270 // POST /recover — ask for a link. The same page for every address.
src/web/pages/site.ts +4 −3
@@ -561,8 +561,9 @@ export async function signinSubmit(ctx: Ctx): Promise<Response> {
561561
562562 // GET /verify?token= — the link in a verification email.
563563 export function verifyPage(ctx: Ctx): Response {
564 const done = verifyEmail(ctx.app, ctx.url.searchParams.get("token") ?? "");
565 return page(
564 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
565 const done = timePhase(ctx.request, "verify", () => verifyEmail(ctx.app, ctx.url.searchParams.get("token") ?? ""));
566 return timePhase(ctx.request, "render", () => page(
566567 {
567568 ctx,
568569 title: done.ok ? "Email verified" : "Link not valid",
@@ -575,7 +576,7 @@ export function verifyPage(ctx: Ctx): Response {
575576 <p><a href="/settings">Settings</a></p>`,
576577 },
577578 done.ok ? 200 : 400,
578 );
579 ));
579580 }
580581
581582 export const EDGE_NAME_MESSAGE = "Sign up under the name your platform sign-in gave you.";
test/server-timing-sweep.test.ts −3
@@ -30,13 +30,10 @@ const UNNAMED: Array<[path: string, issue: string]> = [
3030 ["/alice/forge/settings/labels", "#3390"],
3131 ["/alice/forge/settings/prompts", "#3390"],
3232 ["/alice/forge/tells", "#3322"],
33 ["/claim", "#3390"],
3433 ["/live/apps", "#3390"],
3534 ["/live/on-you", "#3390"],
36 ["/recover", "#3390"],
3735 ["/signin/parent", "#3390"],
3836 ["/signin/parent/callback", "#3390"],
39 ["/verify", "#3390"],
4037 ];
4138
4239 /** Long-lived streams and live fragments: no page, no phases to name. */
test/verify-timing.test.ts added +64
@@ -0,0 +1,64 @@
1 /** `/verify`, `/recover` and `/claim` name their phases in Server-Timing and answer under 50 ms, signed in (#4317). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtempSync, rmSync } from "node:fs";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { createUser } from "../src/core/users.ts";
8 import { serve, type Servers } from "../src/http/server.ts";
9 import { quietLogs } from "./helpers/globals.ts";
10
11 quietLogs();
12
13 let root = "";
14 let app: App;
15 let servers: Servers;
16 let cookie = "";
17
18 beforeAll(async () => {
19 root = mkdtempSync(join(tmpdir(), "forge-verify-timing-"));
20 app = createApp({ dataDir: root, port: 0 });
21 const u = await createUser(app.db, {
22 origin: "seed",
23 trusted: true,
24 username: "alice",
25 email: "[email protected]",
26 password: "a-good-passphrase",
27 adminIfFirst: true,
28 });
29 if (!u.ok) throw new Error(u.error);
30 servers = await serve(app);
31 const signin = await fetch(`${servers.url}/signin`, {
32 method: "POST",
33 headers: { origin: servers.url },
34 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
35 redirect: "manual",
36 });
37 cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
38 });
39 afterAll(() => {
40 servers.stop();
41 app.close();
42 rmSync(root, { recursive: true, force: true });
43 });
44
45 const PAGES: [string, string[]][] = [
46 ["/verify?token=nope", ["session", "verify", "render"]],
47 ["/recover", ["session", "render"]],
48 ["/recover?token=nope", ["session", "link", "render"]],
49 ["/claim", ["session", "render"]],
50 ];
51
52 for (const [path, names] of PAGES)
53 test(`GET ${path} names its phases and answers under 50 ms`, async () => {
54 const get = () => fetch(`${servers.url}${path}`, { headers: { cookie }, redirect: "manual" });
55 await (await get()).text(); // warm
56 const res = await get();
57 await res.text();
58 const parts = (res.headers.get("server-timing") ?? "").split(", ");
59 for (const name of ["app", ...names])
60 expect(parts.find((p) => p.startsWith(`${name};`))).toMatch(/^[a-z]+;dur=\d+\.\d$/);
61 const dur = (n: string) => Number(parts.find((p) => p.startsWith(`${n};`))!.split("=")[1]);
62 expect(dur("app")).toBeLessThan(50);
63 expect(names.reduce((s, n) => s + dur(n), 0)).toBeLessThanOrEqual(dur("app") + 1);
64 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs