trevato/forge

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

More
Forks1g fMapg aRuns11g rEventsFlows

GET /api/v1/admin/people, /admin/invites and /admin/accounts carry an ETag and answer a repeat with 304 until who has access changes #9229

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9224-get-api-v1-admin-people-admin-invites-an → main 2 commits fork

  • Merged. scout/9224-get-api-v1-admin-people-admin-invites-an was merged into main as f54a702.

Review

Sign in to review this change.
2 files changed +217 −3
src/http/api/admin.ts +18 −3
@@ -27,6 +27,7 @@ import { lastOwnerSays, soleOwnedOrgs } from "../../core/orgs.ts";
2727 import { listPeople } from "../../core/roles.ts";
2828 import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts";
2929 import { getUserByName } from "../../core/users.ts";
30 import { holds, weakEtag } from "../etag.ts";
3031 import { offsetPage, offsetQuery } from "../offset_page.ts";
3132 import { body, json, str } from "../respond.ts";
3233 import { offsetRefused } from "../api.ts";
@@ -38,6 +39,20 @@ import type { Router } from "../router.ts";
3839 export const accountAdmin = (ctx: Ctx, scope: "ro" | "admin"): Response | undefined =>
3940 ctx.auth.user?.isAdmin === true ? requireScope(ctx, scope) : notFound();
4041
42 /**
43 * Who can reach the forge, polled often and changed rarely (#9224): the tag
44 * is of the exact bytes sent, so a person, invite or page change moves it and
45 * two viewers shown different bodies never share one. Called only past the
46 * access and query checks, which carry no tag.
47 */
48 const taggedJson = (ctx: Ctx, data: unknown): Response => {
49 const text = JSON.stringify(data);
50 const etag = weakEtag(text);
51 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
52 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
53 return new Response(text, { headers: { "content-type": "application/json", ...headers } });
54 };
55
4156 export function adminRoutes(router: Router<Handler>): void {
4257 // ---- pinned forges (#8697): one deploy, and a change only by Update ----
4358 const pinnedFail = (error: string): Response =>
@@ -297,7 +312,7 @@ export function adminRoutes(router: Router<Handler>): void {
297312 const query = offsetQuery(ctx.url.searchParams);
298313 if ("error" in query) return offsetRefused(query.error);
299314 const listed = timePhase(ctx.request, "invites", () => listInviteLinks(ctx.app, ctx.auth.user));
300 return listed.ok ? timePhase(ctx.request, "render", () => json(offsetPage(listed.value, query.limit, query.offset))) : notFound();
315 return listed.ok ? timePhase(ctx.request, "render", () => taggedJson(ctx, offsetPage(listed.value, query.limit, query.offset))) : notFound();
301316 });
302317
303318 router.add("DELETE", "/api/v1/admin/invites/:id", (ctx) => {
@@ -314,7 +329,7 @@ export function adminRoutes(router: Router<Handler>): void {
314329 const query = offsetQuery(ctx.url.searchParams);
315330 if ("error" in query) return offsetRefused(query.error);
316331 const listed = timePhase(ctx.request, "accounts", () => listAccounts(ctx.app, ctx.auth.user));
317 return listed.ok ? timePhase(ctx.request, "render", () => json(offsetPage(listed.value, query.limit, query.offset))) : notFound();
332 return listed.ok ? timePhase(ctx.request, "render", () => taggedJson(ctx, offsetPage(listed.value, query.limit, query.offset))) : notFound();
318333 });
319334
320335 router.add("POST", "/api/v1/admin/accounts/:username/suspend", async (ctx) => {
@@ -358,7 +373,7 @@ export function adminRoutes(router: Router<Handler>): void {
358373 const query = offsetQuery(ctx.url.searchParams);
359374 if ("error" in query) return offsetRefused(query.error);
360375 const people = timePhase(ctx.request, "people", () => listPeople(ctx.app.db));
361 return timePhase(ctx.request, "render", () => json(offsetPage(people, query.limit, query.offset)));
376 return timePhase(ctx.request, "render", () => taggedJson(ctx, offsetPage(people, query.limit, query.offset)));
362377 });
363378
364379 // Make a person a member (with an optional starter allowance, dollars a month) or a guest again.
test/admin-people-etag.test.ts added +199
@@ -0,0 +1,199 @@
1 /** GET /api/v1/admin/people, /admin/invites and /admin/accounts carry a weak ETag and answer a repeat with 304 until who has access changes (#9224). */
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 { redeemInvite } from "../src/core/invites.ts";
8 import { createToken } from "../src/core/tokens.ts";
9 import { createUser, type User } from "../src/core/users.ts";
10 import { serve, type Servers } from "../src/http/server.ts";
11 import { quietLogs } from "./helpers/globals.ts";
12 import { proveAll } from "./helpers/verified.ts";
13
14 quietLogs();
15
16 const ROUTES = ["people", "invites", "accounts"] as const;
17 type Route = (typeof ROUTES)[number];
18
19 let dataDir = "";
20 let app: App, servers: Servers;
21 let sam: User, bob: User;
22 let adminTok = "", roTok = "", rwTok = "", bobTok = "";
23 const url = (route: Route, query = "") => `${servers.url}/api/v1/admin/${route}${query}`;
24 const get = (route: Route, headers: Record<string, string> = {}, query = "") => fetch(url(route, query), { headers });
25 const asSam = (route: Route, headers: Record<string, string> = {}, query = "") =>
26 get(route, { authorization: `Bearer ${adminTok}`, ...headers }, query);
27 const tags = async (): Promise<Record<Route, string>> => {
28 const out = {} as Record<Route, string>;
29 for (const route of ROUTES) {
30 const r = await asSam(route);
31 expect(r.status).toBe(200);
32 await r.text();
33 out[route] = r.headers.get("etag")!;
34 }
35 return out;
36 };
37 /** Each named route's tag moved and settles to a 304 again; the rest held still. */
38 async function moved(before: Record<Route, string>, what: string, which: Route[]): Promise<Record<Route, string>> {
39 const after = await tags();
40 for (const route of ROUTES) {
41 if (which.includes(route)) expect(after[route], `${what}: ${route}`).not.toBe(before[route]);
42 const again = await asSam(route, { "if-none-match": after[route] });
43 expect(again.status, `${what}: ${route}`).toBe(304);
44 }
45 return after;
46 }
47 const mk = async (username: string) => {
48 const r = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@f.dev`, password: "a-good-passphrase", adminIfFirst: username === "sam" });
49 if (!r.ok) throw new Error("user");
50 return r.value;
51 };
52 const post = (path: string, body: unknown, method = "POST") =>
53 fetch(`${servers.url}/api/v1/admin/${path}`, {
54 method,
55 headers: { "content-type": "application/json", authorization: `Bearer ${adminTok}` },
56 body: JSON.stringify(body),
57 });
58
59 beforeAll(async () => {
60 dataDir = await mkdtemp(join(tmpdir(), "forge-adminpeopleetag-"));
61 app = createApp({ dataDir, port: 0 });
62 sam = await mk("sam");
63 bob = await mk("bob");
64 await mk("carol");
65 proveAll(app);
66 servers = await serve(app);
67 adminTok = createToken(app.db, sam.id, "admin", "admin").token;
68 roTok = createToken(app.db, sam.id, "ro", "ro").token;
69 rwTok = createToken(app.db, sam.id, "rw", "rw").token;
70 bobTok = createToken(app.db, bob.id, "bob", "rw").token;
71 });
72 afterAll(async () => {
73 servers.stop();
74 app.close();
75 await rm(dataDir, { recursive: true, force: true });
76 });
77
78 test("each route answers 200 with a weak tag, private no-cache and vary over the same body; a repeat is a bodiless 304 with the floor headers", async () => {
79 const made = await post("invites", { days: 7 });
80 expect(made.status).toBe(201);
81 for (const route of ROUTES) {
82 const res = await asSam(route);
83 expect(res.status, route).toBe(200);
84 const tag = res.headers.get("etag")!;
85 expect(tag, route).toMatch(/^W\//);
86 expect(res.headers.get("cache-control"), route).toBe("private, no-cache");
87 expect(res.headers.get("vary"), route).toContain("authorization, cookie");
88 expect(res.headers.get("content-type"), route).toBe("application/json");
89 const text = await res.text();
90 expect(JSON.parse(text).items.length, route).toBeGreaterThan(0);
91 const again = await asSam(route, { "if-none-match": tag });
92 expect(again.status, route).toBe(304);
93 expect(again.headers.get("etag"), route).toBe(tag);
94 expect(again.headers.get("cache-control"), route).toBe("private, no-cache");
95 expect(again.headers.get("vary"), route).toContain("authorization, cookie");
96 expect(again.headers.get("x-content-type-options"), route).toBe("nosniff");
97 expect(again.headers.get("content-security-policy"), route).not.toBeNull();
98 expect(again.headers.get("referrer-policy"), route).not.toBeNull();
99 expect(await again.text(), route).toBe("");
100 }
101 });
102
103 test("a person added, made admin, suspended or removed, and an invite made, used or revoked move the tag", async () => {
104 let t = await tags();
105
106 const dave = await mk("dave");
107 t = await moved(t, "person added", ["people", "accounts"]);
108
109 app.db.query("UPDATE user SET is_admin = 1 WHERE id = ?1").run(dave.id);
110 t = await moved(t, "made admin", ["people"]);
111 app.db.query("UPDATE user SET is_admin = 0 WHERE id = ?1").run(dave.id);
112 t = await moved(t, "admin taken back", ["people"]);
113
114 const suspended = await post("accounts/carol/suspend", {});
115 expect(suspended.status).toBe(200);
116 t = await moved(t, "suspended", ["accounts"]);
117
118 const made = await post("invites", { days: 7 });
119 expect(made.status).toBe(201);
120 const invite = (await made.json()) as { id: string; token: string };
121 t = await moved(t, "invite made", ["invites"]);
122
123 const erin = await mk("erin");
124 expect(redeemInvite(app.db, invite.token, erin.id)).toBe(true);
125 t = await moved(t, "invite used", ["invites"]);
126
127 const other = (await (await post("invites", { days: 3 })).json()) as { id: string };
128 t = await tags();
129 const revoked = await fetch(url("invites") + `/${other.id}`, { method: "DELETE", headers: { authorization: `Bearer ${adminTok}` } });
130 expect(revoked.status).toBe(200);
131 t = await moved(t, "invite revoked", ["invites"]);
132
133 const removed = await fetch(url("accounts") + "/dave", {
134 method: "DELETE",
135 headers: { "content-type": "application/json", authorization: `Bearer ${adminTok}` },
136 body: JSON.stringify({ confirm: true }),
137 });
138 expect(removed.status).toBeLessThan(300);
139 await removed.text();
140 await moved(t, "person removed", ["people", "accounts"]);
141 });
142
143 test("limit and cursor move the tag, and one page's tag buys another page nothing", async () => {
144 for (const route of ROUTES) {
145 const whole = await asSam(route);
146 const wholeTag = whole.headers.get("etag")!;
147 await whole.text();
148 const first = await asSam(route, {}, "?limit=1");
149 expect(first.status, route).toBe(200);
150 const firstTag = first.headers.get("etag")!;
151 await first.text();
152 const second = await asSam(route, {}, "?limit=1&cursor=1");
153 const secondTag = second.headers.get("etag")!;
154 const secondBody = await second.text();
155 expect(firstTag, route).not.toBe(wholeTag);
156 expect(secondTag, route).not.toBe(firstTag);
157 const replay = await asSam(route, { "if-none-match": firstTag }, "?limit=1&cursor=1");
158 expect(replay.status, route).toBe(200);
159 expect(await replay.text(), route).toBe(secondBody);
160 const own = await asSam(route, { "if-none-match": secondTag }, "?limit=1&cursor=1");
161 expect(own.status, route).toBe(304);
162 }
163 });
164
165 test("a non-admin, a token short of the scope, and a bad limit or cursor answer as before, with no tag, even holding If-None-Match: *", async () => {
166 const star = { "if-none-match": "*" };
167 for (const route of ROUTES) {
168 const anon = await get(route, star);
169 expect(anon.status, route).not.toBe(304);
170 expect(anon.status, route).toBeGreaterThanOrEqual(400);
171 expect(anon.headers.get("etag"), route).toBeNull();
172
173 const plain = await get(route, { authorization: `Bearer ${bobTok}` });
174 const notAdmin = await get(route, { authorization: `Bearer ${bobTok}`, ...star });
175 expect(notAdmin.status, route).toBeGreaterThanOrEqual(400);
176 expect(notAdmin.status, route).toBe(plain.status);
177 expect(await notAdmin.text(), route).toBe(await plain.text());
178 expect(notAdmin.headers.get("etag"), route).toBeNull();
179
180 for (const query of ["?limit=0", "?limit=abc", "?limit=999", "?cursor=0", "?cursor=x"]) {
181 const bad = await asSam(route, star, query);
182 expect(bad.status, `${route}${query}`).toBe(422);
183 expect(bad.headers.get("etag"), `${route}${query}`).toBeNull();
184 expect(bad.headers.get("cache-control"), `${route}${query}`).not.toBe("private, no-cache");
185 }
186 }
187
188 // people asks for the admin scope; invites and accounts for ro.
189 const shortOfAdmin = await get("people", { authorization: `Bearer ${roTok}`, ...star });
190 expect(shortOfAdmin.status).toBe(403);
191 expect(shortOfAdmin.headers.get("etag")).toBeNull();
192 const rwPeople = await get("people", { authorization: `Bearer ${rwTok}`, ...star });
193 expect(rwPeople.status).toBe(403);
194 expect(rwPeople.headers.get("etag")).toBeNull();
195 for (const route of ["invites", "accounts"] as const) {
196 const ro = await get(route, { authorization: `Bearer ${roTok}`, ...star });
197 expect(ro.status, route).toBe(304);
198 }
199 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs