Public: anyone can read this; the crew can change it.
The OAuth consent page's Server-Timing names its phases: session, each read, render #3737
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3736-the-oauth-consent-page-s-server-timing-n → main 1 commit fork
2 files
changed +92
−2
src/web/pages/oauth.ts +9 −2
| @@ -14,6 +14,7 @@ import { | ||
| 14 | 14 | tokenName, |
| 15 | 15 | type AuthRequest, |
| 16 | 16 | } from "../../core/oauth.ts"; |
| 17 | import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts"; | |
| 17 | 18 | import type { Ctx } from "../../http/context.ts"; |
| 18 | 19 | import { pageHead } from "../chrome.ts"; |
| 19 | 20 | import { field, readForm } from "../form.ts"; |
| @@ -180,13 +181,19 @@ export function authorizePage(ctx: Ctx): Response { | ||
| 180 | 181 | `/signin?next=${encodeURIComponent(ctx.url.pathname + ctx.url.search)}`, |
| 181 | 182 | ), |
| 182 | 183 | ); |
| 184 | notePhase(ctx.request, "session", takeSessionTime(ctx.request)); | |
| 183 | 185 | const values = fromQuery(ctx); |
| 184 | const request = parseAuthRequest(ctx.app.db, (n) => values[n as keyof Params] ?? ""); | |
| 186 | const request = timePhase(ctx.request, "request", () => | |
| 187 | parseAuthRequest(ctx.app.db, (n) => values[n as keyof Params] ?? ""), | |
| 188 | ); | |
| 185 | 189 | if (!request.ok) return refused(ctx, request.error); |
| 186 | return allowReturnTo( | |
| 190 | const rendering = performance.now(); | |
| 191 | const response = allowReturnTo( | |
| 187 | 192 | guard(page({ ctx, title: "Connect", body: consent(ctx, request.value, values) })), |
| 188 | 193 | request.value.redirectUri, |
| 189 | 194 | ); |
| 195 | notePhase(ctx.request, "render", performance.now() - rendering); | |
| 196 | return response; | |
| 190 | 197 | } |
| 191 | 198 | |
| 192 | 199 | export async function authorizeSubmit(ctx: Ctx): Promise<Response> { |
test/oauth-timing.test.ts added +83
| @@ -0,0 +1,83 @@ | ||
| 1 | /** `GET /oauth/authorize` names its phases in Server-Timing (#3736). */ | |
| 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 | const REDIRECT = "https://claude.ai/api/mcp/auth_callback"; | |
| 15 | let root = ""; | |
| 16 | let app: App; | |
| 17 | let servers: Servers; | |
| 18 | let cookie = ""; | |
| 19 | let clientId = ""; | |
| 20 | ||
| 21 | beforeAll(async () => { | |
| 22 | root = mkdtempSync(join(tmpdir(), "forge-oauth-timing-")); | |
| 23 | app = createApp({ dataDir: root, port: 0 }); | |
| 24 | const u = await createUser(app.db, { | |
| 25 | origin: "seed", | |
| 26 | trusted: true, | |
| 27 | username: "alice", | |
| 28 | email: "[email protected]", | |
| 29 | password: "a-good-passphrase", | |
| 30 | adminIfFirst: true, | |
| 31 | }); | |
| 32 | if (!u.ok) throw new Error(u.error); | |
| 33 | servers = await serve(app); | |
| 34 | cookie = `forge_session=${createSession(app.db, u.value.id).token}`; | |
| 35 | const res = await fetch(`${servers.url}/oauth/register`, { | |
| 36 | method: "POST", | |
| 37 | headers: { "content-type": "application/json" }, | |
| 38 | body: JSON.stringify({ client_name: "Timed", redirect_uris: [REDIRECT] }), | |
| 39 | }); | |
| 40 | clientId = ((await res.json()) as any).client_id; | |
| 41 | }); | |
| 42 | afterAll(() => { | |
| 43 | servers.stop(); | |
| 44 | app.close(); | |
| 45 | rmSync(root, { recursive: true, force: true }); | |
| 46 | }); | |
| 47 | ||
| 48 | const names = (res: Response): string[] => | |
| 49 | (res.headers.get("server-timing") ?? "").split(", ").map((p) => p.split(";")[0]!); | |
| 50 | ||
| 51 | const authorize = (over: Record<string, string> = {}, headers: Record<string, string> = { cookie }) => | |
| 52 | fetch( | |
| 53 | `${servers.url}/oauth/authorize?${new URLSearchParams({ | |
| 54 | response_type: "code", | |
| 55 | client_id: clientId, | |
| 56 | redirect_uri: REDIRECT, | |
| 57 | code_challenge: "c".repeat(43), | |
| 58 | code_challenge_method: "S256", | |
| 59 | state: "s", | |
| 60 | ...over, | |
| 61 | })}`, | |
| 62 | { redirect: "manual", headers }, | |
| 63 | ); | |
| 64 | ||
| 65 | test("the consent page names session, request and render", async () => { | |
| 66 | const res = await authorize(); | |
| 67 | expect(res.status).toBe(200); | |
| 68 | expect(names(res)).toEqual(expect.arrayContaining(["session", "request", "render"])); | |
| 69 | }); | |
| 70 | ||
| 71 | test("a refused request names session and request", async () => { | |
| 72 | const res = await authorize({ client_id: "nope" }); | |
| 73 | expect(res.status).toBe(400); | |
| 74 | expect(names(res)).toEqual(expect.arrayContaining(["session", "request"])); | |
| 75 | expect(names(res)).not.toContain("render"); | |
| 76 | }); | |
| 77 | ||
| 78 | test("the signed-out redirect is unchanged", async () => { | |
| 79 | const res = await authorize({}, {}); | |
| 80 | expect(res.status).toBe(303); | |
| 81 | expect(res.headers.get("location")).toStartWith("/signin?next="); | |
| 82 | expect(names(res)).not.toContain("request"); | |
| 83 | }); |
new above
offline