trevato/forge

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

More
Forks1g fMapg aRuns17g rEventsFlows

GET /api/v1/user/keys carries an ETag and answers a repeat with 304 until one of the viewer's SSH keys changes #9255

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9243-get-api-v1-user-keys-carries-an-etag-and → main 2 commits fork

  • Merged. scout/9243-get-api-v1-user-keys-carries-an-etag-and was merged into main as e4b2076.

Review

Sign in to review this change.
2 files changed +164 −1
src/http/api/user.ts +9 −1
@@ -184,7 +184,15 @@ export function userRoutes(router: Router<Handler>): void {
184184 const window = keyedWindow(ctx);
185185 if (window instanceof Response) return window;
186186 const keys = timePhase(ctx.request, "keys", () => listKeys(ctx.app.db, user.id));
187 return timePhase(ctx.request, "render", () => json(keyedPage(keys.map(publicKey), (k) => k.id, window)));
187 return timePhase(ctx.request, "render", () => {
188 // The body shows every key and its last use, so a tag over the body (and
189 // the viewer it was rendered for) moves whenever any of them does (#9243).
190 const page = keyedPage(keys.map(publicKey), (k) => k.id, window);
191 const etag = weakEtag(`${user.id}|${JSON.stringify(page)}`);
192 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
193 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
194 return json(page, 200, headers);
195 });
188196 });
189197 router.add("POST", "/api/v1/user/keys", async (ctx) => {
190198 const denied = requireScope(ctx, "rw");
test/user-keys-etag.test.ts added +155
@@ -0,0 +1,155 @@
1 /** GET /api/v1/user/keys is tagged and answers a repeat 304 until one of the viewer's SSH keys changes (#9243). */
2 import { afterAll, beforeAll, describe, 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 { addSshKey } from "../src/actions/users.ts";
8 import { deleteKey, touchKey } from "../src/core/keys.ts";
9 import { createToken } from "../src/core/tokens.ts";
10 import { createUser, type User } from "../src/core/users.ts";
11 import { serve, type Servers } from "../src/http/server.ts";
12 import { promote } from "./helpers/members.ts";
13
14 let dataDir: string;
15 let app: App;
16 let servers: Servers;
17 let alice: User;
18 let bob: User;
19 let tokens: Record<string, string>;
20 let seed = 1;
21
22 /** A fresh, well-formed ssh-ed25519 public key line. */
23 function freshKey(): string {
24 const field = (b: Uint8Array) => {
25 const out = new Uint8Array(4 + b.length);
26 new DataView(out.buffer).setUint32(0, b.length);
27 out.set(b, 4);
28 return out;
29 };
30 const algo = field(new TextEncoder().encode("ssh-ed25519"));
31 const pub = field(new Uint8Array(32).fill(seed++));
32 return `ssh-ed25519 ${Buffer.concat([algo, pub]).toString("base64")} test`;
33 }
34
35 function addKey(who: User, name: string) {
36 const r = addSshKey(app, who, { name, key: freshKey() });
37 if (!r.ok) throw new Error(String(r.error));
38 return r.value;
39 }
40
41 const get = (token: string | undefined, etag?: string) =>
42 fetch(`${servers.url}/api/v1/user/keys`, {
43 headers: {
44 ...(token === undefined ? {} : { authorization: `Bearer ${token}` }),
45 ...(etag === undefined ? {} : { "if-none-match": etag }),
46 },
47 });
48 const tagOf = async (who: string) => {
49 const res = await get(tokens[who]);
50 await res.arrayBuffer();
51 expect(res.status).toBe(200);
52 return res.headers.get("etag")!;
53 };
54
55 beforeAll(async () => {
56 dataDir = await mkdtemp(join(tmpdir(), "forge-keys-etag-"));
57 app = createApp({ dataDir, port: 0 });
58 servers = await serve(app);
59 const mk = async (username: string, first = false) => {
60 const r = await createUser(app.db, {
61 origin: "seed", trusted: true, username, allowReserved: true,
62 email: `${username}@forge.local`, password: "a-good-passphrase", adminIfFirst: first,
63 });
64 if (!r.ok) throw new Error(r.error);
65 promote(app.db, r.value);
66 return r.value;
67 };
68 await mk("root", true);
69 alice = await mk("alice");
70 bob = await mk("bob");
71 tokens = {
72 alice: createToken(app.db, alice.id, "t", "ro").token,
73 bob: createToken(app.db, bob.id, "t", "ro").token,
74 };
75 addKey(alice, "laptop");
76 });
77
78 afterAll(async () => {
79 servers.stop();
80 app.close();
81 await rm(dataDir, { recursive: true, force: true });
82 });
83
84 describe("GET /api/v1/user/keys etag", () => {
85 test("carries a weak tag, private no-cache and vary; the body is unchanged", async () => {
86 const res = await get(tokens.alice);
87 expect(res.status).toBe(200);
88 expect(res.headers.get("etag")).toMatch(/^W\//);
89 expect(res.headers.get("cache-control")).toBe("private, no-cache");
90 expect(res.headers.get("vary")).toContain("authorization, cookie");
91 const body = (await res.json()) as { items: { name: string }[]; next_cursor: string | null };
92 expect(body.items.map((k) => k.name)).toEqual(["laptop"]);
93 expect(body.next_cursor).toBeNull();
94 });
95
96 test("a repeat is a 304 with the same headers, no body and the security floor", async () => {
97 const tag = await tagOf("alice");
98 const res = await get(tokens.alice, tag);
99 expect(res.status).toBe(304);
100 expect(await res.text()).toBe("");
101 expect(res.headers.get("etag")).toBe(tag);
102 expect(res.headers.get("cache-control")).toBe("private, no-cache");
103 expect(res.headers.get("vary")).toContain("authorization, cookie");
104 expect(res.headers.get("x-content-type-options")).toBe("nosniff");
105 expect(res.headers.get("content-security-policy")).not.toBeNull();
106 expect(res.headers.get("referrer-policy")).not.toBeNull();
107 });
108
109 test("no session is a 401 with no tag, even with If-None-Match: *", async () => {
110 const res = await get(undefined, "*");
111 expect(res.status).toBe(401);
112 expect(res.headers.get("etag")).toBeNull();
113 });
114
115 test("a token that does not hold is a 401 with no tag, even with If-None-Match: *", async () => {
116 const res = await get("forge_not-a-real-token", "*");
117 expect(res.status).toBe(401);
118 expect(res.headers.get("etag")).toBeNull();
119 });
120
121 test("two viewers shown different bodies never share a tag", async () => {
122 const [a, b] = [await tagOf("alice"), await tagOf("bob")];
123 expect(a).not.toBe(b);
124 expect((await get(tokens.bob, a)).status).toBe(200);
125 });
126
127 test("adding a key changes the tag", async () => {
128 const tag = await tagOf("alice");
129 addKey(alice, "desktop");
130 expect((await get(tokens.alice, tag)).status).toBe(200);
131 });
132
133 test("removing a key changes the tag", async () => {
134 const k = addKey(alice, "spare");
135 const tag = await tagOf("alice");
136 deleteKey(app.db, alice.id, k.id);
137 expect((await get(tokens.alice, tag)).status).toBe(200);
138 });
139
140 test("a key's last use changes the tag", async () => {
141 const k = addKey(alice, "used");
142 const tag = await tagOf("alice");
143 app.db.run("UPDATE user_key SET last_used_at = '2000-01-01T00:00:00.000Z' WHERE id = ?", [k.id]);
144 expect((await get(tokens.alice, tag)).status).toBe(200);
145 const again = await tagOf("alice");
146 touchKey(app.db, k.id);
147 expect((await get(tokens.alice, again)).status).toBe(200);
148 });
149
150 test("another viewer's key does not change it", async () => {
151 const tag = await tagOf("alice");
152 addKey(bob, "bobs");
153 expect((await get(tokens.alice, tag)).status).toBe(304);
154 });
155 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs