bf43c60 · 4b2f6de · f88e534
@@ -38,6 +38,7 @@
import { signInFirst } from "../web/chrome.ts";
import { sessionCookie } from "../web/pages/site.ts";
import { cookieValue, type Ctx } from "./context.ts";
import { KeySet } from "./jwks.ts";
import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../core/route_timing.ts";
export const STATE_COOKIE = "forge_rp";
const SCOPE = "openid profile";
@@ -102,17 +103,19 @@
async function clientId(ctx: Ctx, issuer: string, redirectUri: string): Promise<
}
function failure(ctx: Ctx, message: string, status: number): Response {
  const response = page(
    {
      ctx,
      title: "Sign in",
      body: html`<div class="narrow">
        <div class="page-head"><h1>Could not sign you in</h1></div>
        <p role="alert">${message}</p>
        <p><a href="/signin">Back to sign in</a></p>
      </div>`,
    },
    status,
  const response = timePhase(ctx.request, "render", () =>
    page(
      {
        ctx,
        title: "Sign in",
        body: html`<div class="narrow">
          <div class="page-head"><h1>Could not sign you in</h1></div>
          <p role="alert">${message}</p>
          <p><a href="/signin">Back to sign in</a></p>
        </div>`,
      },
      status,
    ),
  );
  response.headers.set("cache-control", "no-store");
  return response;
@@ -134,6 +137,7 @@
export async function parentLinkStart(ctx: Ctx): Promise<Response> {
}
async function begin(ctx: Ctx, linkUser?: string): Promise<Response> {
  notePhase(ctx.request, "session", takeSessionTime(ctx.request));
  const issuer = parentIssuer(ctx.app);
  if (issuer === undefined) return failure(ctx, "This forge has no parent to sign in through.", 404);
  // Each start writes a row and the table is bounded: a flood must not push
@@ -174,6 +178,7 @@
async function begin(ctx: Ctx, linkUser?: string): Promise<Response> {
}
export async function parentCallback(ctx: Ctx): Promise<Response> {
  notePhase(ctx.request, "session", takeSessionTime(ctx.request));
  const q = ctx.url.searchParams;
  const state = q.get("state") ?? "";
  const code = q.get("code") ?? "";
@@ -193,19 +198,21 @@
export async function parentCallback(ctx: Ctx): Promise<Response> {
    return refuse("The parent forge did not sign you in.");
  try {
      
        sentinelcrew
        19d ago
        outdated
      
      Nit, not blocking: clientId() can call the parent (to register a client on first use), and that call sits outside the parent phase. Consider moving it inside timePhaseAsync so all the time spent talking to the parent is named.
    
    const client = await clientId(ctx, login.issuer, login.redirectUri);
      
        sentinelcrew
        18d ago
        
      
      The parent phase wraps the fetch and answerOf, so a slow parent body is attributed to the parent and not to the app. Good. If the fetch throws, nothing is noted, which is fine because the catch answers with an error page that carries render.
    
    const res = await fetch(`${login.issuer}/oauth/token`, {
      method: "POST",
      body: new URLSearchParams({
        grant_type: "authorization_code",
        code,
        client_id: client,
        redirect_uri: login.redirectUri,
        code_verifier: login.verifier,
      }),
      redirect: "error",
      signal: AbortSignal.timeout(5000),
    const { res, body } = await timePhaseAsync(ctx.request, "parent", async () => {
      const res = await fetch(`${login.issuer}/oauth/token`, {
        method: "POST",
        body: new URLSearchParams({
          grant_type: "authorization_code",
          code,
          client_id: client,
          redirect_uri: login.redirectUri,
          code_verifier: login.verifier,
        }),
        redirect: "error",
        signal: AbortSignal.timeout(5000),
      });
      return { res, body: await answerOf(res) };
    });
    const body = await answerOf(res);
    if (!res.ok || typeof body.id_token !== "string")
      return refuse("The parent forge did not accept the sign-in.", 502);
    const keys = keysOf(ctx.app, login.issuer);
@@ -722,6 +722,11 @@
function describeTarget(to: string, ctx: Ctx): Destructive | undefined {
 * table above recognises as destructive.
 */
export function confirmPage(ctx: Ctx): Response {
  notePhase(ctx.request, "session", takeSessionTime(ctx.request));
  return timePhase(ctx.request, "render", () => confirmBody(ctx));
}
function confirmBody(ctx: Ctx): Response {
  const params = ctx.url.searchParams;
  const to = params.get("to") ?? "";
  if (
@@ -0,0 +1,95 @@
/** `/-/confirm` and the parent sign-in pair name their Server-Timing phases (#5401). */
import { afterAll, beforeAll, expect, test } from "bun:test";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { createApp, type App } from "../src/app.ts";
import { createSession } from "../src/core/sessions.ts";
import { createUser } from "../src/core/users.ts";
import { serve, type Servers } from "../src/http/server.ts";
import { quietLogs } from "./helpers/globals.ts";
quietLogs();
let root = "";
let app: App;
let servers: Servers;
beforeAll(async () => {
  root = mkdtempSync(join(tmpdir(), "forge-confirm-parent-timing-"));
  app = createApp({ dataDir: root, port: 0 });
  servers = await serve(app);
});
afterAll(() => {
  servers.stop();
  app.close();
  rmSync(root, { recursive: true, force: true });
});
const phases = (res: Response): string[] =>
  (res.headers.get("server-timing") ?? "").split(",").map((p) => p.trim().split(";")[0]!);
test("GET /-/confirm names session and render", async () => {
  const res = await fetch(`${servers.url}/-/confirm?to=/nowhere`);
  await res.arrayBuffer();
  expect(res.status).toBe(404);
  expect(phases(res)).toEqual(expect.arrayContaining(["app", "session", "render"]));
});
test("GET /signin/parent names session and render", async () => {
  const res = await fetch(`${servers.url}/signin/parent`, { redirect: "manual" });
  await res.arrayBuffer();
  expect(res.status).toBe(404);
  expect(phases(res)).toEqual(expect.arrayContaining(["app", "session", "render"]));
});
test("GET /signin/parent/callback names session and render", async () => {
  const res = await fetch(`${servers.url}/signin/parent/callback`, { redirect: "manual" });
  await res.arrayBuffer();
  expect(res.status).toBe(400);
  expect(phases(res)).toEqual(expect.arrayContaining(["app", "session", "render"]));
});
test("the callback names parent for the token exchange", async () => {
  const dir = mkdtempSync(join(tmpdir(), "forge-confirm-parent-timing-"));
  const parent = createApp({ dataDir: join(dir, "p"), port: 0 });
  const parentServers = await serve(parent);
  const child = createApp({ dataDir: join(dir, "c"), port: 0, parentUrl: parentServers.url, parentLoopback: true });
  const childServers = await serve(child);
  try {
    const made = await createUser(parent.db, {
      origin: "seed",
      username: "alice",
      email: "[email protected]",
      password: "a-long-family-passphrase",
      adminIfFirst: true,
    });
    if (!made.ok) throw new Error(made.error);
    const cookie = `forge_session=${createSession(parent.db, made.value.id).token}`;
    const start = await fetch(`${childServers.url}/signin/parent`, { redirect: "manual" });
    expect(start.status).toBe(303);
    expect(phases(start)).toEqual(expect.arrayContaining(["session"]));
    const state = start.headers.getSetCookie().map((c) => c.split(";")[0]!).find((c) => c.startsWith("forge_rp="))!;
    const to = new URL(start.headers.get("location")!);
    const consent = await fetch(`${parentServers.url}/oauth/authorize`, {
      method: "POST",
      redirect: "manual",
      headers: { cookie, origin: parentServers.url },
      body: new URLSearchParams({ ...Object.fromEntries(to.searchParams), decision: "allow" }),
    });
    const back = new URL(consent.headers.get("location")!);
    const done = await fetch(`${childServers.url}${back.pathname}${back.search}`, {
      redirect: "manual",
      headers: { cookie: state },
    });
    await done.arrayBuffer();
    expect(done.status).toBe(303);
    expect(phases(done)).toEqual(expect.arrayContaining(["app", "session", "parent"]));
  } finally {
    childServers.stop();
    parentServers.stop();
    child.close();
    parent.close();
    rmSync(dir, { recursive: true, force: true });
  }
}, 30_000);
@@ -20,9 +20,6 @@
quietLogs();
/** Pages without named phases yet → the issue that will name them. */
const UNNAMED: Array<[path: string, issue: string]> = [
  ["/-/confirm", "#3390"],
  ["/signin/parent", "#3390"],
  ["/signin/parent/callback", "#3390"],
];
/** Long-lived streams and live fragments: no page, no phases to name. */
		@@ -38,6 +38,7 @@ import { signInFirst } from "../web/chrome.ts";
38	38	import { sessionCookie } from "../web/pages/site.ts";
39	39	import { cookieValue, type Ctx } from "./context.ts";
40	40	import { KeySet } from "./jwks.ts";
	41	import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../core/route_timing.ts";
41	42
42	43	export const STATE_COOKIE = "forge_rp";
43	44	const SCOPE = "openid profile";
		@@ -102,17 +103,19 @@ async function clientId(ctx: Ctx, issuer: string, redirectUri: string): Promise<
102	103	}
103	104
104	105	function failure(ctx: Ctx, message: string, status: number): Response {
105		const response = page(
106		{
107		ctx,
108		title: "Sign in",
109		body: html`<div class="narrow">
110		<div class="page-head"><h1>Could not sign you in</h1></div>
111		<p role="alert">${message}</p>
112		<p><a href="/signin">Back to sign in</a></p>
113		</div>`,
114		},
115		status,
	106	const response = timePhase(ctx.request, "render", () =>
	107	page(
	108	{
	109	ctx,
	110	title: "Sign in",
	111	body: html`<div class="narrow">
	112	<div class="page-head"><h1>Could not sign you in</h1></div>
	113	<p role="alert">${message}</p>
	114	<p><a href="/signin">Back to sign in</a></p>
	115	</div>`,
	116	},
	117	status,
	118	),
116	119	);
117	120	response.headers.set("cache-control", "no-store");
118	121	return response;
		@@ -134,6 +137,7 @@ export async function parentLinkStart(ctx: Ctx): Promise<Response> {
134	137	}
135	138
136	139	async function begin(ctx: Ctx, linkUser?: string): Promise<Response> {
	140	notePhase(ctx.request, "session", takeSessionTime(ctx.request));
137	141	const issuer = parentIssuer(ctx.app);
138	142	if (issuer === undefined) return failure(ctx, "This forge has no parent to sign in through.", 404);
139	143	// Each start writes a row and the table is bounded: a flood must not push
		@@ -174,6 +178,7 @@ async function begin(ctx: Ctx, linkUser?: string): Promise<Response> {
174	178	}
175	179
176	180	export async function parentCallback(ctx: Ctx): Promise<Response> {
	181	notePhase(ctx.request, "session", takeSessionTime(ctx.request));
177	182	const q = ctx.url.searchParams;
178	183	const state = q.get("state") ?? "";
179	184	const code = q.get("code") ?? "";
		@@ -193,19 +198,21 @@ export async function parentCallback(ctx: Ctx): Promise<Response> {
193	198	return refuse("The parent forge did not sign you in.");
194	199	try {
		sentinelcrew 19d ago outdated Nit, not blocking: `clientId()` can call the parent (to register a client on first use), and that call sits outside the `parent` phase. Consider moving it inside `timePhaseAsync` so all the time spent talking to the parent is named.
195	200	const client = await clientId(ctx, login.issuer, login.redirectUri);
		sentinelcrew 18d ago The `parent` phase wraps the fetch and `answerOf`, so a slow parent body is attributed to the parent and not to the app. Good. If the fetch throws, nothing is noted, which is fine because the catch answers with an error page that carries `render`.
196		const res = await fetch(`${login.issuer}/oauth/token`, {
197		method: "POST",
198		body: new URLSearchParams({
199		grant_type: "authorization_code",
200		code,
201		client_id: client,
202		redirect_uri: login.redirectUri,
203		code_verifier: login.verifier,
204		}),
205		redirect: "error",
206		signal: AbortSignal.timeout(5000),
	201	const { res, body } = await timePhaseAsync(ctx.request, "parent", async () => {
	202	const res = await fetch(`${login.issuer}/oauth/token`, {
	203	method: "POST",
	204	body: new URLSearchParams({
	205	grant_type: "authorization_code",
	206	code,
	207	client_id: client,
	208	redirect_uri: login.redirectUri,
	209	code_verifier: login.verifier,
	210	}),
	211	redirect: "error",
	212	signal: AbortSignal.timeout(5000),
	213	});
	214	return { res, body: await answerOf(res) };
207	215	});
208		const body = await answerOf(res);
209	216	if (!res.ok \|\| typeof body.id_token !== "string")
210	217	return refuse("The parent forge did not accept the sign-in.", 502);
211	218	const keys = keysOf(ctx.app, login.issuer);
		@@ -722,6 +722,11 @@ function describeTarget(to: string, ctx: Ctx): Destructive \| undefined {
722	722	* table above recognises as destructive.
723	723	*/
724	724	export function confirmPage(ctx: Ctx): Response {
	725	notePhase(ctx.request, "session", takeSessionTime(ctx.request));
	726	return timePhase(ctx.request, "render", () => confirmBody(ctx));
	727	}
	728
	729	function confirmBody(ctx: Ctx): Response {
725	730	const params = ctx.url.searchParams;
726	731	const to = params.get("to") ?? "";
727	732	if (
		@@ -0,0 +1,95 @@
	1	/** `/-/confirm` and the parent sign-in pair name their Server-Timing phases (#5401). */
	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 { createSession } from "../src/core/sessions.ts";
	8	import { createUser } from "../src/core/users.ts";
	9	import { serve, type Servers } from "../src/http/server.ts";
	10	import { quietLogs } from "./helpers/globals.ts";
	11
	12	quietLogs();
	13
	14	let root = "";
	15	let app: App;
	16	let servers: Servers;
	17
	18	beforeAll(async () => {
	19	root = mkdtempSync(join(tmpdir(), "forge-confirm-parent-timing-"));
	20	app = createApp({ dataDir: root, port: 0 });
	21	servers = await serve(app);
	22	});
	23	afterAll(() => {
	24	servers.stop();
	25	app.close();
	26	rmSync(root, { recursive: true, force: true });
	27	});
	28
	29	const phases = (res: Response): string[] =>
	30	(res.headers.get("server-timing") ?? "").split(",").map((p) => p.trim().split(";")[0]!);
	31
	32	test("GET /-/confirm names session and render", async () => {
	33	const res = await fetch(`${servers.url}/-/confirm?to=/nowhere`);
	34	await res.arrayBuffer();
	35	expect(res.status).toBe(404);
	36	expect(phases(res)).toEqual(expect.arrayContaining(["app", "session", "render"]));
	37	});
	38
	39	test("GET /signin/parent names session and render", async () => {
	40	const res = await fetch(`${servers.url}/signin/parent`, { redirect: "manual" });
	41	await res.arrayBuffer();
	42	expect(res.status).toBe(404);
	43	expect(phases(res)).toEqual(expect.arrayContaining(["app", "session", "render"]));
	44	});
	45
	46	test("GET /signin/parent/callback names session and render", async () => {
	47	const res = await fetch(`${servers.url}/signin/parent/callback`, { redirect: "manual" });
	48	await res.arrayBuffer();
	49	expect(res.status).toBe(400);
	50	expect(phases(res)).toEqual(expect.arrayContaining(["app", "session", "render"]));
	51	});
	52
	53	test("the callback names parent for the token exchange", async () => {
	54	const dir = mkdtempSync(join(tmpdir(), "forge-confirm-parent-timing-"));
	55	const parent = createApp({ dataDir: join(dir, "p"), port: 0 });
	56	const parentServers = await serve(parent);
	57	const child = createApp({ dataDir: join(dir, "c"), port: 0, parentUrl: parentServers.url, parentLoopback: true });
	58	const childServers = await serve(child);
	59	try {
	60	const made = await createUser(parent.db, {
	61	origin: "seed",
	62	username: "alice",
	63	email: "[email protected]",
	64	password: "a-long-family-passphrase",
	65	adminIfFirst: true,
	66	});
	67	if (!made.ok) throw new Error(made.error);
	68	const cookie = `forge_session=${createSession(parent.db, made.value.id).token}`;
	69	const start = await fetch(`${childServers.url}/signin/parent`, { redirect: "manual" });
	70	expect(start.status).toBe(303);
	71	expect(phases(start)).toEqual(expect.arrayContaining(["session"]));
	72	const state = start.headers.getSetCookie().map((c) => c.split(";")[0]!).find((c) => c.startsWith("forge_rp="))!;
	73	const to = new URL(start.headers.get("location")!);
	74	const consent = await fetch(`${parentServers.url}/oauth/authorize`, {
	75	method: "POST",
	76	redirect: "manual",
	77	headers: { cookie, origin: parentServers.url },
	78	body: new URLSearchParams({ ...Object.fromEntries(to.searchParams), decision: "allow" }),
	79	});
	80	const back = new URL(consent.headers.get("location")!);
	81	const done = await fetch(`${childServers.url}${back.pathname}${back.search}`, {
	82	redirect: "manual",
	83	headers: { cookie: state },
	84	});
	85	await done.arrayBuffer();
	86	expect(done.status).toBe(303);
	87	expect(phases(done)).toEqual(expect.arrayContaining(["app", "session", "parent"]));
	88	} finally {
	89	childServers.stop();
	90	parentServers.stop();
	91	child.close();
	92	parent.close();
	93	rmSync(dir, { recursive: true, force: true });
	94	}
	95	}, 30_000);
		@@ -20,9 +20,6 @@ quietLogs();
20	20
21	21	/** Pages without named phases yet → the issue that will name them. */
22	22	const UNNAMED: Array<[path: string, issue: string]> = [
23		["/-/confirm", "#3390"],
24		["/signin/parent", "#3390"],
25		["/signin/parent/callback", "#3390"],
26	23	];
27	24
28	25	/** Long-lived streams and live fragments: no page, no phases to name. */