trevato/forge

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

More
Forks1g fMapg aRuns12g rEventsFlows

GET …/app/members and …/app/domains carry an ETag and answer a repeat with 304 until a member or a domain changes #9188

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9178-get-app-members-and-app-domains-carry-an → main 2 commits fork

  • Merged. scout/9178-get-app-members-and-app-domains-carry-an was merged into main as 85870fe.

Review

Sign in to review this change.
2 files changed +211 −3
src/http/api/app.ts +17 −3
@@ -15,7 +15,8 @@ import { appsRuntime } from "../../apps/runtime.ts";
1515 import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts";
1616 import type { Result } from "../../kernel/result.ts";
1717 import { appJson, appLimitResponse, loadRepo } from "../api.ts";
18 import type { Handler } from "../context.ts";
18 import type { Ctx, Handler } from "../context.ts";
19 import { holds, weakEtag } from "../etag.ts";
1920 import { apiError, body, json, notFound, requireScope, respondAccess, str, unauthorized } from "../respond.ts";
2021 import type { Router } from "../router.ts";
2122
@@ -46,6 +47,19 @@ function membersAnswer<T>(result: Result<T, AppMembersError>): Response {
4647 }
4748 }
4849
50 /**
51 * An access-checked read tagged by its own bytes (#9178): `private, no-cache`
52 * and varied on the credential, so a repeat holding the tag is a bodiless 304
53 * until the data moves. Called only after the access check passed.
54 */
55 function tagged(ctx: Ctx, data: unknown): Response {
56 const text = JSON.stringify(data);
57 const etag = weakEtag(text);
58 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
59 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
60 return new Response(text, { headers: { "content-type": "application/json", ...headers } });
61 }
62
4963 export function appRoutes(router: Router<Handler>): void {
5064 router.add("GET", "/api/v1/repos/:owner/:name/app", (ctx) => {
5165 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
@@ -119,7 +133,7 @@ export function appRoutes(router: Router<Handler>): void {
119133 const repo = loadRepo(ctx);
120134 if (!repo) return notFound();
121135 const read = timePhase(ctx.request, "members", () => readAppMembers(ctx.app, ctx.auth.user, repo));
122 return timePhase(ctx.request, "render", () => membersAnswer(read));
136 return timePhase(ctx.request, "render", () => (read.ok ? tagged(ctx, read.value) : membersAnswer(read)));
123137 });
124138
125139 router.add("PUT", "/api/v1/repos/:owner/:name/app/members", async (ctx) => {
@@ -150,7 +164,7 @@ export function appRoutes(router: Router<Handler>): void {
150164 if (!repo) return notFound();
151165 const listed = timePhase(ctx.request, "domains", () => listAppDomains(ctx.app, ctx.auth.user, repo));
152166 if (!listed.ok) return respondAccess(listed.error, 422);
153 return timePhase(ctx.request, "render", () => json({ items: listed.value, next_cursor: null }));
167 return timePhase(ctx.request, "render", () => tagged(ctx, { items: listed.value, next_cursor: null }));
154168 });
155169
156170 router.add("POST", "/api/v1/repos/:owner/:name/app/domains", async (ctx) => {
test/app-reads-etag.test.ts added +194
@@ -0,0 +1,194 @@
1 /** GET …/app/members and …/app/domains carry an ETag and answer a repeat with 304 (#9178). */
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 { createRepo } from "../src/actions/repos.ts";
7 import { createApp, type App } from "../src/app.ts";
8 import type { DomainResolver } from "../src/apps/domains.ts";
9 import { AppsRuntime } from "../src/apps/runtime.ts";
10 import { createApp as insertApp } from "../src/core/apps.ts";
11 import { createToken } from "../src/core/tokens.ts";
12 import { createUser } from "../src/core/users.ts";
13 import { serve, type Servers } from "../src/http/server.ts";
14 import { quietLogs } from "./helpers/globals.ts";
15
16 quietLogs();
17
18 const HOST = "app.example.com";
19 const MEMBERS = "/api/v1/repos/alice/notes/app/members";
20 const DOMAINS = "/api/v1/repos/alice/notes/app/domains";
21
22 /** A zone the test writes; anything unwritten does not exist. */
23 class FakeDns implements DomainResolver {
24 readonly txt = new Map<string, string[][]>();
25 resolveTxt(name: string): Promise<string[][]> {
26 const found = this.txt.get(name);
27 return found ? Promise.resolve(found) : Promise.reject(Object.assign(new Error("no data"), { code: "ENODATA" }));
28 }
29 resolveCname(): Promise<string[]> {
30 return Promise.reject(Object.assign(new Error("no data"), { code: "ENODATA" }));
31 }
32 }
33
34 let dataDir: string;
35 let app: App;
36 let servers: Servers;
37 let runtime: AppsRuntime;
38 let adminTok = "";
39 let rwTok = "";
40 let bobTok = "";
41 const dns = new FakeDns();
42
43 const call = (method: string, path: string, tok: string, headers: Record<string, string> = {}, body?: unknown) =>
44 fetch(`${servers.url}${path}`, {
45 method,
46 headers: { authorization: `Bearer ${tok}`, "content-type": "application/json", ...headers },
47 ...(body === undefined ? {} : { body: JSON.stringify(body) }),
48 });
49 const tagOf = async (path: string) => {
50 const res = await call("GET", path, adminTok);
51 expect(res.status).toBe(200);
52 await res.text();
53 return res.headers.get("etag")!;
54 };
55 const moved = async (path: string, tag: string) => {
56 const res = await call("GET", path, adminTok, { "if-none-match": tag });
57 expect(res.status).toBe(200);
58 const next = res.headers.get("etag")!;
59 expect(next).not.toBe(tag);
60 await res.text();
61 return next;
62 };
63 const held = async (path: string, tag: string) => {
64 const res = await call("GET", path, adminTok, { "if-none-match": tag });
65 expect(res.status).toBe(304);
66 await res.text();
67 };
68
69 beforeAll(async () => {
70 dataDir = await mkdtemp(join(tmpdir(), "forge-app-etag-"));
71 app = createApp({ dataDir, port: 0, host: "forge.test" });
72 servers = await serve(app);
73 runtime = new AppsRuntime(app, {
74 domain: "apps.test",
75 scheme: "http",
76 publicPort: servers.port,
77 forgeUrl: servers.url,
78 concurrency: 1,
79 dns,
80 });
81 await runtime.start();
82 const mk = async (username: string, first: boolean) => {
83 const made = await createUser(app.db, {
84 origin: "seed",
85 trusted: true,
86 username,
87 email: `${username}@forge.local`,
88 password: "a-good-passphrase",
89 adminIfFirst: first,
90 });
91 if (!made.ok) throw new Error(String(made.error));
92 return made.value;
93 };
94 const alice = await mk("alice", true);
95 const bob = await mk("bob", false);
96 await mk("carol", false);
97 const repo = await createRepo(app, alice, { name: "notes", visibility: "public", description: "" });
98 if (!repo.ok) throw new Error("repo");
99 const row = insertApp(app.db, { repoId: repo.value.id, slug: "notes", access: "private" });
100 if (!row.ok) throw new Error(row.error);
101 adminTok = createToken(app.db, alice.id, "a", "admin").token;
102 rwTok = createToken(app.db, alice.id, "rw", "rw").token;
103 bobTok = createToken(app.db, bob.id, "b", "admin").token;
104 });
105 afterAll(async () => {
106 await runtime?.stop();
107 servers?.stop();
108 app?.close();
109 if (dataDir) await rm(dataDir, { recursive: true, force: true });
110 });
111
112 for (const path of [MEMBERS, DOMAINS]) {
113 test(`${path}: weak tag, private no-cache, vary; a repeat is a bodiless 304 with the floor`, async () => {
114 const res = await call("GET", path, adminTok);
115 expect(res.status).toBe(200);
116 const tag = res.headers.get("etag")!;
117 expect(tag).toMatch(/^W\/"/);
118 expect(res.headers.get("cache-control")).toBe("private, no-cache");
119 expect(res.headers.get("vary")).toContain("authorization, cookie");
120 expect(res.headers.get("content-type")).toContain("application/json");
121 await res.text();
122 const again = await call("GET", path, adminTok, { "if-none-match": tag });
123 expect(again.status).toBe(304);
124 expect(again.headers.get("etag")).toBe(tag);
125 expect(again.headers.get("cache-control")).toBe("private, no-cache");
126 expect(again.headers.get("vary")).toContain("authorization, cookie");
127 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
128 expect(again.headers.get("referrer-policy")).not.toBeNull();
129 expect(again.headers.get("content-security-policy")).not.toBeNull();
130 expect(await again.text()).toBe("");
131 });
132 }
133
134 test("a members PUT moves the tag; a dry run does not", async () => {
135 const tag = await tagOf(MEMBERS);
136 const plan = await call("PUT", MEMBERS, adminTok, {}, { members: ["carol"] });
137 expect(plan.status).toBe(200);
138 await plan.text();
139 await held(MEMBERS, tag);
140 const done = await call("PUT", MEMBERS, adminTok, {}, { members: ["carol"], dryRun: false });
141 expect(done.status).toBe(200);
142 await done.text();
143 const next = await moved(MEMBERS, tag);
144 await held(MEMBERS, next);
145 });
146
147 test("adding, verifying and removing a domain move the domains tag", async () => {
148 let tag = await tagOf(DOMAINS);
149 const added = await call("POST", DOMAINS, adminTok, {}, { host: HOST });
150 expect(added.status).toBe(201);
151 const domain = (await added.json()) as { records: { value: string }[] };
152 tag = await moved(DOMAINS, tag);
153 await held(DOMAINS, tag);
154 const proof = domain.records.find((r) => r.value.startsWith("forge-verify="))?.value;
155 expect(proof).toBeDefined();
156 dns.txt.set(`_forge-verify.${HOST}`, [[proof!]]);
157 const checked = await call("POST", `${DOMAINS}/${HOST}/check`, adminTok);
158 expect(checked.status).toBe(200);
159 await checked.text();
160 tag = await moved(DOMAINS, tag);
161 const removed = await call("DELETE", `${DOMAINS}/${HOST}`, adminTok);
162 expect(removed.status).toBe(200);
163 await removed.text();
164 await moved(DOMAINS, tag);
165 });
166
167 test("refusals carry no tag and never a 304, even with *", async () => {
168 const cases: [string, string, number[]][] = [
169 [MEMBERS, rwTok, [200]], // members reads at ro: rw is allowed, checked below
170 [DOMAINS, rwTok, [403]],
171 [DOMAINS, bobTok, [403, 404]],
172 ["/api/v1/repos/alice/nope/app/members", adminTok, [404]],
173 ["/api/v1/repos/alice/nope/app/domains", adminTok, [404]],
174 ];
175 for (const [path, tok, statuses] of cases) {
176 const res = await call("GET", path, tok, { "if-none-match": "*" });
177 if (path === MEMBERS && tok === rwTok) {
178 expect(res.status).toBe(304); // the same person, after the access check
179 } else {
180 expect(statuses).toContain(res.status);
181 expect(res.status).not.toBe(304);
182 expect(res.headers.get("etag")).toBeNull();
183 }
184 await res.text();
185 }
186 const stranger = await call("GET", MEMBERS, bobTok, { "if-none-match": "*" });
187 expect(stranger.status).not.toBe(304);
188 expect(stranger.headers.get("etag")).toBeNull();
189 await stranger.text();
190 const anon = await fetch(`${servers.url}${MEMBERS}`, { headers: { "if-none-match": "*" } });
191 expect([401, 404]).toContain(anon.status);
192 expect(anon.headers.get("etag")).toBeNull();
193 await anon.text();
194 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs