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

The caller's tokens, origins and origin apps lists name their Server-Timing phases #5019

Merged opened this

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

Merged

scout/forge:scout/5005-the-caller-s-tokens-origins-and-origin-a → main 1 commit fork 1 thread · 1 unresolved

  • Merged. scout/5005-the-caller-s-tokens-origins-and-origin-a was merged into main as 29e139a.

Review

Sign in to review this change.
2 files changed +63 −3
src/http/api.ts +9 −3
@@ -2103,9 +2103,11 @@ export function apiRoutes(router: Router<Handler>): void {
21032103 });
21042104
21052105 router.add("GET", "/api/v1/tokens", (ctx) => {
2106 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
21062107 const denied = requireAccount(ctx);
21072108 if (denied) return denied;
2108 return json(listTokens(ctx.app.db, ctx.auth.user!.id));
2109 const tokens = timePhase(ctx.request, "tokens", () => listTokens(ctx.app.db, ctx.auth.user!.id));
2110 return timePhase(ctx.request, "render", () => json(tokens));
21092111 });
21102112
21112113 router.add("DELETE", "/api/v1/tokens/:id", (ctx) => {
@@ -4493,9 +4495,11 @@ export function apiRoutes(router: Router<Handler>): void {
44934495 oauthCallbackUri(ctx.url, ctx.app.config.host);
44944496
44954497 router.add("GET", "/api/v1/user/origins", (ctx) => {
4498 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
44964499 const denied = requireAccount(ctx, { adminToken: true });
44974500 if (denied) return denied;
4498 return json(listUserOrigins(ctx.app, ctx.auth.user!).map(originJson));
4501 const origins = timePhase(ctx.request, "origins", () => listUserOrigins(ctx.app, ctx.auth.user!).map(originJson));
4502 return timePhase(ctx.request, "render", () => json(origins));
44994503 });
45004504
45014505 router.add("POST", "/api/v1/user/origins", async (ctx) => {
@@ -4567,9 +4571,11 @@ export function apiRoutes(router: Router<Handler>): void {
45674571 // this is a per-user, per-host registration; GitHub can fall back to one
45684572 // the operator configured for the whole forge.
45694573 router.add("GET", "/api/v1/user/origin-apps", (ctx) => {
4574 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
45704575 const denied = requireAccount(ctx, { adminToken: true });
45714576 if (denied) return denied;
4572 return json(listOriginApps(ctx.app, ctx.auth.user!));
4577 const apps = timePhase(ctx.request, "apps", () => listOriginApps(ctx.app, ctx.auth.user!));
4578 return timePhase(ctx.request, "render", () => json(apps));
45734579 });
45744580
45754581 router.add("POST", "/api/v1/user/origin-apps", async (ctx) => {
test/account-lists-timing.test.ts added +54
@@ -0,0 +1,54 @@
1 /** The caller's tokens, origins and origin apps lists name their phases in Server-Timing. */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { serve, type Servers } from "../src/http/server.ts";
8 import { seedAdmin } from "./helpers/admin.ts";
9
10 let dataDir = "";
11 let app: App, servers: Servers, bearer = "";
12
13 beforeAll(async () => {
14 dataDir = await mkdtemp(join(tmpdir(), "forge-altime-"));
15 app = createApp({ dataDir, port: 0 });
16 servers = await serve(app);
17 await seedAdmin(app, { username: "alice", email: "[email protected]", password: "correct-horse-battery" });
18 const minted = await fetch(`${servers.url}/api/v1/tokens`, {
19 method: "POST",
20 headers: { "content-type": "application/json" },
21 body: JSON.stringify({ adminConfirm: true, username: "alice", password: "correct-horse-battery", name: "cli", scope: "admin" }),
22 });
23 bearer = ((await minted.json()) as { token: string }).token;
24 });
25
26 afterAll(async () => {
27 servers.stop();
28 app.close();
29 await rm(dataDir, { recursive: true, force: true });
30 });
31
32 const basic = `Basic ${btoa("alice:correct-horse-battery")}`;
33 const phases = (res: Response) =>
34 (res.headers.get("server-timing") ?? "").split(", ").map((p) => p.split(";")[0]!).filter((n) => n && n !== "app");
35
36 for (const [path, read] of [
37 ["/api/v1/tokens", "tokens"],
38 ["/api/v1/user/origins", "origins"],
39 ["/api/v1/user/origin-apps", "apps"],
40 ] as const) {
41 test(`${path} names session, ${read}, render in order`, async () => {
42 const res = await fetch(`${servers.url}${path}`, { headers: { authorization: path === "/api/v1/tokens" ? basic : `Bearer ${bearer}` } });

Nit, no change needed: using Basic for /api/v1/tokens because that route refuses admin bearers is correct. A one-line comment saying so would spare the next reader from wondering why.

43 await res.arrayBuffer();
44 expect(res.status).toBe(200);
45 expect(phases(res)).toEqual(["session", read, "render"]);
46 });
47
48 test(`${path} refused still names session`, async () => {
49 const res = await fetch(`${servers.url}${path}`);
50 await res.arrayBuffer();
51 expect(res.status).toBeGreaterThanOrEqual(401);
52 expect(phases(res)).toEqual(["session"]);
53 });
54 }

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs