trevato/forge

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

More
Forks1g fMapg aRuns17g rEventsFlows

GET /api/v1/repos/:owner/:name/app/notes carries an ETag and answers a repeat with 304 until a note arrives or is moderated #9293

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9244-get-api-v1-repos-owner-name-app-notes-ca → main 2 commits fork

  • Merged. scout/9244-get-api-v1-repos-owner-name-app-notes-ca was merged into main as e75705e.

Review

Sign in to review this change.
2 files changed +130 −1
src/http/api/app_notes.ts +8 −1
@@ -9,6 +9,7 @@ import { moderateNote, ownerNote, ownerNotes, unbanSender } from "../../actions/
99 import { getAppByRepo } from "../../core/apps.ts";
1010 import { loadRepo } from "../api.ts";
1111 import type { Ctx, Handler } from "../context.ts";
12 import { holds, weakEtag } from "../etag.ts";
1213 import { apiError, forbidden, json, notFound, requireScope } from "../respond.ts";
1314 import type { Router } from "../router.ts";
1415
@@ -28,7 +29,13 @@ export function appNoteRoutes(router: Router<Handler>): void {
2829 const t = target(ctx);
2930 if (t instanceof Response) return t;
3031 const seen = ownerNotes(ctx.app, t.record, t.repo, ctx.auth.user);
31 return seen === undefined ? forbidden() : json(seen);
32 if (seen === undefined) return forbidden();
33 // Only after the access check: a refusal never carries or honours a tag.
34 const text = JSON.stringify(seen);
35 const etag = weakEtag(`${ctx.auth.user?.id ?? ""}\n${text}`);
36 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
37 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
38 return json(seen, 200, headers);
3239 });
3340
3441 router.add("POST", "/api/v1/repos/:owner/:name/app/notes/:id/hide", moderation("hide"));
test/app-notes-etag.test.ts added +122
@@ -0,0 +1,122 @@
1 /** GET …/app/notes carries a weak ETag and answers a repeat with 304 until a note arrives or is moderated (#9244). */
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 { newApp } from "../src/actions/apps.ts";
8 import { AppsRuntime } from "../src/apps/runtime.ts";
9 import { setMember } from "../src/core/roles.ts";
10 import { createToken } from "../src/core/tokens.ts";
11 import { createUser, type User } from "../src/core/users.ts";
12 import { serve, type Servers } from "../src/http/server.ts";
13 import { quietLogs } from "./helpers/globals.ts";
14
15 quietLogs();
16
17 const DOMAIN = "apps.test";
18 const BASE = "/api/v1/repos/founder/notes/app/notes";
19 let dataDir: string, app: App, servers: Servers, runtime: AppsRuntime;
20 const tokens: Record<string, string> = {};
21
22 const get = (who: string, path = BASE, extra: Record<string, string> = {}) =>
23 fetch(`${servers.url}${path}`, { headers: { authorization: `Bearer ${tokens[who]}`, ...extra } });
24 const post = (who: string, path: string) =>
25 fetch(`${servers.url}${path}`, { method: "POST", headers: { authorization: `Bearer ${tokens[who]}` } });
26 let sends = 0;
27 const send = async (text: string) => {
28 const host = `notes.${DOMAIN}:${servers.port}`;
29 const res = await fetch(`${servers.url}/__forge/note`, {
30 method: "POST",
31 headers: { host, origin: `http://${host}`, "content-type": "application/json", "x-forwarded-for": `10.8.0.${++sends}` },
32 body: JSON.stringify({ text, page: `http://${host}/list`, viewport: "390x844" }),
33 });
34 expect(res.status).toBe(201);
35 };
36 const tagOf = async (who = "founder") => {
37 const r = await get(who);
38 expect(r.status).toBe(200);
39 await r.text();
40 return r.headers.get("etag")!;
41 };
42 const notes = async () => (await (await get("founder")).json()) as { notes: { id: string }[] };
43
44 beforeAll(async () => {
45 dataDir = await mkdtemp(join(tmpdir(), "forge-app-notes-etag-"));
46 app = createApp({ dataDir, port: 0, host: "forge.test" });
47 servers = await serve(app);
48 runtime = new AppsRuntime(app, { domain: DOMAIN, scheme: "http", publicPort: servers.port, forgeUrl: servers.url, concurrency: 1 });
49 await runtime.start();
50 const make = async (username: string, first = false): Promise<User> => {
51 const made = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase", adminIfFirst: first });
52 if (!made.ok) throw new Error(made.error);
53 return made.value;
54 };
55 const siteAdmin = await make("siteadmin", true);
56 const founder = await make("founder");
57 const stranger = await make("stranger");
58 setMember(app.db, founder.id, true, siteAdmin.id, new Date());
59 const made = await newApp(app, founder, { name: "notes", owner: "founder", visibility: "public" });
60 if (!made.ok) throw new Error(String(made.error));
61 tokens.founder = createToken(app.db, founder.id, "rw", "rw").token;
62 tokens.siteadmin = createToken(app.db, siteAdmin.id, "rw", "rw").token;
63 tokens.stranger = createToken(app.db, stranger.id, "rw", "rw").token;
64 }, 120_000);
65
66 afterAll(async () => {
67 await runtime?.stop();
68 servers?.stop();
69 app?.close();
70 await rm(dataDir, { recursive: true, force: true });
71 });
72
73 test("a repeat is a 304 with the same headers and no body; the tag moves with every note change", async () => {
74 await send("One");
75 await send("Two");
76 await send("Three");
77 const r = await get("founder");
78 const tag = r.headers.get("etag")!;
79 expect(tag).toMatch(/^W\//);
80 expect(r.headers.get("cache-control")).toBe("private, no-cache");
81 expect(r.headers.get("vary")).toContain("authorization, cookie");
82 await r.text();
83 const again = await get("founder", BASE, { "if-none-match": tag });
84 expect(again.status).toBe(304);
85 expect(await again.text()).toBe("");
86 expect(again.headers.get("etag")).toBe(tag);
87 expect(again.headers.get("cache-control")).toBe("private, no-cache");
88 expect(again.headers.get("vary")).toContain("authorization, cookie");
89 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
90
91 await send("Four");
92 const afterArrival = await tagOf();
93 expect(afterArrival).not.toBe(tag);
94 expect((await get("founder", BASE, { "if-none-match": tag })).status).toBe(200);
95
96 const [a, b] = (await notes()).notes;
97 expect((await post("founder", `${BASE}/${a!.id}/hide`)).status).toBe(200);
98 const afterHide = await tagOf();
99 expect(afterHide).not.toBe(afterArrival);
100 expect((await post("founder", `${BASE}/${b!.id}/ban`)).status).toBe(200);
101 const afterBan = await tagOf();
102 expect(afterBan).not.toBe(afterHide);
103 expect((await post("founder", `${BASE}/${b!.id}/unban`)).status).toBe(200);
104 expect(await tagOf()).not.toBe(afterBan);
105 });
106
107 test("two viewers never share a tag", async () => {
108 const mine = await tagOf("founder");
109 const theirs = await tagOf("siteadmin");
110 expect(theirs).not.toBe(mine);
111 expect((await get("siteadmin", BASE, { "if-none-match": mine })).status).toBe(200);
112 });
113
114 test("a refusal or a missing app carries no tag, even with If-None-Match: *", async () => {
115 const star = { "if-none-match": "*" };
116 const denied = await get("stranger", BASE, star);
117 expect(denied.status).toBe(403);
118 expect(denied.headers.get("etag")).toBeNull();
119 const missing = await get("founder", "/api/v1/repos/founder/missing/app/notes", star);
120 expect(missing.status).toBe(404);
121 expect(missing.headers.get("etag")).toBeNull();
122 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs