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

GET /api/v1/flags carries an ETag and answers a repeat with 304 until a flag changes #9216

Merged opened this

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

Merged

scout/forge:scout/9214-get-api-v1-flags-carries-an-etag-and-ans → main 2 commits fork

  • Merged. scout/9214-get-api-v1-flags-carries-an-etag-and-ans was merged into main as 1a0b401.

Review

Sign in to review this change.
2 files changed +121 −2
src/http/api/flags.ts +10 −2
@@ -3,7 +3,8 @@
33 import { readFlags } from "../../actions/flags_view.ts";
44 import { notePhase, takeSessionTime } from "../../core/route_timing.ts";
55 import type { Handler } from "../context.ts";
6 import { apiError, json, unauthorized } from "../respond.ts";
6 import { holds, weakEtag } from "../etag.ts";
7 import { apiError, unauthorized } from "../respond.ts";
78 import type { Router } from "../router.ts";
89
910 export function flagsRoutes(router: Router<Handler>): void {
@@ -11,6 +12,13 @@ export function flagsRoutes(router: Router<Handler>): void {
1112 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
1213 if (!ctx.auth.user) return unauthorized();
1314 const out = readFlags(ctx.auth.user);
14 return out.ok ? json({ flags: out.flags }) : apiError("forbidden", 403, out.message);
15 if (!out.ok) return apiError("forbidden", 403, out.message);
16 // The tag is of the exact bytes sent, reached only past the access check: a flag turned on or off
17 // changes the body, and so the tag (#9214).
18 const text = JSON.stringify({ flags: out.flags });
19 const etag = weakEtag(text);
20 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
21 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
22 return new Response(text, { headers: { "content-type": "application/json", ...headers } });
1523 });
1624 }
test/flags-etag.test.ts added +111
@@ -0,0 +1,111 @@
1 /** GET /api/v1/flags carries a weak ETag and answers a repeat with 304 until a flag changes (#9214). */
2
3 import { afterAll, beforeAll, expect, test } from "bun:test";
4 import { mkdtemp, rm } from "node:fs/promises";
5 import { tmpdir } from "node:os";
6 import { join } from "node:path";
7 import { createApp, type App } from "../src/app.ts";
8 import { makeFlags } from "../src/core/flags.ts";
9 import { createToken } from "../src/core/tokens.ts";
10 import { createUser, getUserByName } from "../src/core/users.ts";
11 import { weakEtag } from "../src/http/etag.ts";
12 import { serve, type Servers } from "../src/http/server.ts";
13 import { useFlagRegistry } from "../src/web/pages/flags.ts";
14 import { quietLogs } from "./helpers/globals.ts";
15
16 quietLogs();
17
18 const PASS = "a-long-flags-passphrase";
19 const reg = makeFlags({ alpha: { why: "shows the new board", default: false, issue: 5417 } });
20
21 let dataDir = "";
22 let app: App;
23 let servers: Servers;
24 const tokenOf = (name: string) => createToken(app.db, getUserByName(app.db, name)!.id, "t", "ro").token;
25 const get = (token: string | null, headers: Record<string, string> = {}) =>
26 fetch(`${servers.url}/api/v1/flags`, { headers: { ...(token ? { authorization: `Bearer ${token}` } : {}), ...headers } });
27
28 beforeAll(async () => {
29 dataDir = await mkdtemp(join(tmpdir(), "forge-flags-etag-"));
30 app = createApp({ dataDir, port: 0 });
31 servers = await serve(app);
32 for (const [username, isAdmin] of [["alice", true], ["bob", false]] as const) {
33 const made = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@x.test`, password: PASS, isAdmin });
34 if (!made.ok) throw new Error(username);
35 }
36 useFlagRegistry(reg);
37 }, 30_000);
38
39 afterAll(async () => {
40 useFlagRegistry(undefined);
41 servers.stop();
42 app.close();
43 await rm(dataDir, { recursive: true, force: true });
44 });
45
46 test("200 carries a weak tag over the body, private no-cache and vary; the body is unchanged", async () => {
47 const res = await get(tokenOf("alice"));
48 expect(res.status).toBe(200);
49 const text = await res.text();
50 expect(text).toBe(JSON.stringify({ flags: [{ name: "alpha", why: "shows the new board", default: false, issue: 5417 }] }));
51 expect(res.headers.get("content-type")).toBe("application/json");
52 expect(res.headers.get("etag")).toBe(weakEtag(text));
53 expect(res.headers.get("cache-control")).toBe("private, no-cache");
54 expect(res.headers.get("vary")).toContain("authorization, cookie");
55 });
56
57 test("a repeat holding the tag is a bodiless 304 with the same headers and the floor", async () => {
58 const token = tokenOf("alice");
59 const first = await get(token);
60 const tag = first.headers.get("etag")!;
61 await first.text();
62 const again = await get(token, { "if-none-match": tag });
63 expect(again.status).toBe(304);
64 expect(again.headers.get("etag")).toBe(tag);
65 expect(again.headers.get("cache-control")).toBe("private, no-cache");
66 expect(again.headers.get("vary")).toContain("authorization, cookie");
67 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
68 expect(again.headers.get("content-security-policy")).not.toBeNull();
69 expect(again.headers.get("referrer-policy")).not.toBeNull();
70 expect(await again.text()).toBe("");
71 });
72
73 test("turning a flag on, then off, moves the tag each time", async () => {
74 const token = tokenOf("alice");
75 const alice = getUserByName(app.db, "alice")!;
76 const tagNow = async () => {
77 const r = await get(token);
78 await r.text();
79 return r.headers.get("etag")!;
80 };
81 const before = await tagNow();
82 expect(reg.set(app.db, "alpha", { forge: true }, alice.id)).toBe(true);
83 const on = await get(token, { "if-none-match": before });
84 expect(on.status).toBe(200);
85 expect(((await on.json()) as any).flags[0].value).toEqual({ forge: true });
86 const onTag = on.headers.get("etag")!;
87 expect(onTag).not.toBe(before);
88 expect(reg.set(app.db, "alpha", { forge: false }, alice.id)).toBe(true);
89 const off = await get(token, { "if-none-match": onTag });
90 expect(off.status).toBe(200);
91 await off.text();
92 expect(off.headers.get("etag")).not.toBe(onTag);
93 reg.set(app.db, "alpha", undefined, alice.id);
94 });
95
96 test("no session is 401 and a refused viewer 403, with no ETag even holding *", async () => {
97 const anon = await get(null, { "if-none-match": "*" });
98 expect(anon.status).toBe(401);
99 expect(anon.headers.get("etag")).toBeNull();
100 expect(((await anon.json()) as any).error).toBeDefined();
101 const bob = await get(tokenOf("bob"), { "if-none-match": "*" });
102 expect(bob.status).toBe(403);
103 expect(bob.headers.get("etag")).toBeNull();
104 expect(JSON.stringify(await bob.json())).toContain("Flags are set by a forge admin.");
105 // A tag an admin was given buys a refused viewer nothing.
106 const adm = await get(tokenOf("alice"));
107 await adm.text();
108 const replay = await get(tokenOf("bob"), { "if-none-match": adm.headers.get("etag")! });
109 expect(replay.status).toBe(403);
110 expect(replay.headers.get("etag")).toBeNull();
111 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs