trevato/forge

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

More
Forks1g fMapg aRuns15g rEventsFlows

GET /api/v1/orgs/:org/teams and …/teams/:team/members carry an ETag and answer a repeat with 304 until a team or its members change #9171

Merged masoncrew opened this

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

ConversationFiles
Merged

mason/forge:mason/9165-get-api-v1-orgs-org-teams-and-teams-team → main 2 commits fork

  • Merged. mason/9165-get-api-v1-orgs-org-teams-and-teams-team was merged into main as ea5ed2e.

Review

Sign in to review this change.
4 files changed +154 −4
src/contract/openapi/routes/getApiV1OrgsOrgTeams.ts +1 −1
@@ -1,4 +1,4 @@
11 import { type Route, limitTo } from "../shared.ts";
22 import { LIST_LIMIT_MAX } from "../../../core/issues.ts";
33
4 export const route: Route = { method: "get", path: "/api/v1/orgs/{org}/teams", summary: "List an organization's teams; answers { items, next_cursor }, paged by `limit` and `cursor`: all for an org admin, only the caller's own teams for a member, [] for a stranger or anonymous caller", auth: false, query: { limit: limitTo(LIST_LIMIT_MAX), cursor: "string" } };
4 export const route: Route = { method: "get", path: "/api/v1/orgs/{org}/teams", summary: "List an organization's teams; answers { items, next_cursor }, paged by `limit` and `cursor`: all for an org admin, only the caller's own teams for a member, [] for a stranger or anonymous caller; the 200 carries a weak etag of the exact JSON and cache-control private, no-cache, and a repeat sending If-None-Match gets 304 with no body once access is checked", auth: false, alsoStatus: [304], query: { limit: limitTo(LIST_LIMIT_MAX), cursor: "string" } };
src/contract/openapi/routes/getApiV1OrgsOrgTeamsTeamMembers.ts +1 −1
@@ -1,4 +1,4 @@
11 import { type Route, limitTo } from "../shared.ts";
22 import { LIST_LIMIT_MAX } from "../../../core/issues.ts";
33
4 export const route: Route = { method: "get", path: "/api/v1/orgs/{org}/teams/{team}/members", summary: "List a team's members as public profiles; answers { items, next_cursor }, paged by `limit` and `cursor`; 404 when the caller may not see the team", auth: false, query: { limit: limitTo(LIST_LIMIT_MAX), cursor: "string" } };
4 export const route: Route = { method: "get", path: "/api/v1/orgs/{org}/teams/{team}/members", summary: "List a team's members as public profiles; answers { items, next_cursor }, paged by `limit` and `cursor`; 404 when the caller may not see the team; the 200 carries a weak etag of the exact JSON and cache-control private, no-cache, and a repeat sending If-None-Match gets 304 with no body once access is checked", auth: false, alsoStatus: [304], query: { limit: limitTo(LIST_LIMIT_MAX), cursor: "string" } };
src/http/api/orgs.ts +16 −2
@@ -34,6 +34,7 @@ import {
3434 str,
3535 } from "../respond.ts";
3636 import type { Router } from "../router.ts";
37 import { holds, weakEtag } from "../etag.ts";
3738
3839 /** An org by name plus the team a route names (by name, case-insensitive). */
3940 function loadTeam(ctx: Ctx): { org: User; team: Team } | { error: Response } {
@@ -53,6 +54,19 @@ function teamVisible(ctx: Ctx, org: User, team: Team): boolean {
5354 );
5455 }
5556
57 /**
58 * A reader-scoped JSON answer tagged by its exact bytes: a repeat holding the
59 * tag gets 304. Callers check access first, so a 304 confirms nothing to
60 * someone who may not read it.
61 */
62 function tagged(request: Request, data: unknown): Response {
63 const text = JSON.stringify(data);
64 const etag = weakEtag(text);
65 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
66 if (holds(request, etag)) return new Response(null, { status: 304, headers });
67 return new Response(text, { headers: { "content-type": "application/json", ...headers } });
68 }
69
5670 const permissionOf = (raw: unknown): Permission => {
5771 const p = str(raw);
5872 return p === "write" || p === "admin" ? p : "read";
@@ -93,7 +107,7 @@ export function orgRoutes(router: Router<Handler>): void {
93107 listTeamsVisibleTo(ctx.app.db, org.id, ctx.auth.user),
94108 );
95109 return timePhase(ctx.request, "render", () =>
96 json(keyedPage(teams, (t) => t.name, window)),
110 tagged(ctx.request, keyedPage(teams, (t) => t.name, window)),
97111 );
98112 });
99113
@@ -135,7 +149,7 @@ export function orgRoutes(router: Router<Handler>): void {
135149 listTeamMembers(ctx.app.db, found.team.id).map(publicProfile),
136150 );
137151 return timePhase(ctx.request, "render", () =>
138 json(keyedPage(members, (u) => u.username, window)),
152 tagged(ctx.request, keyedPage(members, (u) => u.username, window)),
139153 );
140154 });
141155
test/org-teams-etag.test.ts added +136
@@ -0,0 +1,136 @@
1 /** The org teams and team members lists carry an ETag; a repeat is a 304 until a team or its members change. */
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 { createOrg, createTeam } from "../src/core/orgs.ts";
8 import { createToken } from "../src/core/tokens.ts";
9 import { createUser } from "../src/core/users.ts";
10 import { serve, type Servers } from "../src/http/server.ts";
11
12 describe("org teams ETag", () => {
13 let dataDir: string;
14 let app: App;
15 let servers: Servers;
16 let owner: string;
17 let stranger: string;
18
19 const send = (path: string, token: string | null, method = "GET", extra: Record<string, string> = {}, payload?: unknown) =>
20 fetch(`${servers.url}${path}`, {
21 method,
22 headers: {
23 ...(token ? { authorization: `Bearer ${token}` } : {}),
24 "content-type": "application/json",
25 ...extra,
26 },
27 ...(payload === undefined ? {} : { body: JSON.stringify(payload) }),
28 });
29 const T = "/api/v1/orgs/acme/teams";
30
31 beforeAll(async () => {
32 dataDir = await mkdtemp(join(tmpdir(), "forge-org-teams-etag-"));
33 app = createApp({ dataDir, port: 0 });
34 servers = await serve(app);
35 const mk = async (username: string) => {
36 const u = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase" });
37 if (!u.ok) throw new Error(u.error);
38 return u.value;
39 };
40 const a = await mk("alice");
41 await mk("bob");
42 await mk("dave");
43 const c = await mk("carol");
44 owner = createToken(app.db, a.id, "admin", "admin").token;
45 stranger = createToken(app.db, c.id, "admin", "admin").token;
46 const org = createOrg(app.db, "acme", a.id);
47 if (!org.ok) throw new Error(org.error);
48 for (const name of ["docs", "ops"]) {
49 const team = createTeam(app.db, org.value.id, name, "write");
50 if (!team.ok) throw new Error(team.error);
51 }
52 }, 30_000);
53
54 afterAll(async () => {
55 servers.stop();
56 app.close();
57 await rm(dataDir, { recursive: true, force: true });
58 });
59
60 test("the teams list is 200 with a weak tag, then 304 with the same headers and no body", async () => {
61 const first = await send(T, owner);
62 expect(first.status).toBe(200);
63 const tag = first.headers.get("etag")!;
64 expect(tag).toMatch(/^W\/"/);
65 expect(first.headers.get("cache-control")).toBe("private, no-cache");
66 expect(first.headers.get("vary")).toContain("authorization, cookie");
67 expect(((await first.json()) as { items: unknown[] }).items.length).toBe(3);
68 const again = await send(T, owner, "GET", { "if-none-match": tag });
69 expect(again.status).toBe(304);
70 expect(again.headers.get("etag")).toBe(tag);
71 expect(again.headers.get("cache-control")).toBe("private, no-cache");
72 expect(again.headers.get("vary")).toContain("authorization, cookie");
73 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
74 expect(again.headers.get("referrer-policy")).not.toBeNull();
75 expect(await again.text()).toBe("");
76 });
77
78 test("creating and removing a team moves the teams tag", async () => {
79 const t0 = (await send(T, owner)).headers.get("etag")!;
80 expect((await send(T, owner, "POST", {}, { name: "extra" })).status).toBe(201);
81 const t1 = await send(T, owner, "GET", { "if-none-match": t0 });
82 expect(t1.status).toBe(200);
83 const tag1 = t1.headers.get("etag")!;
84 expect(tag1).not.toBe(t0);
85 expect((await send(`${T}/extra`, owner, "DELETE")).status).toBe(204);
86 const t2 = await send(T, owner, "GET", { "if-none-match": tag1 });
87 expect(t2.status).toBe(200);
88 expect(t2.headers.get("etag")).not.toBe(tag1);
89 });
90
91 test("a member added or removed moves that team's tag and not another's", async () => {
92 const docs = `${T}/docs/members`;
93 const ops = `${T}/ops/members`;
94 const d0 = (await send(docs, owner)).headers.get("etag")!;
95 const o0 = (await send(ops, owner)).headers.get("etag")!;
96 expect(d0).toMatch(/^W\//);
97 expect((await send(docs, owner, "GET", { "if-none-match": d0 })).status).toBe(304);
98 expect((await send(docs, owner, "POST", {}, { user: "bob" })).status).toBe(201);
99 const d1 = await send(docs, owner, "GET", { "if-none-match": d0 });
100 expect(d1.status).toBe(200);
101 const dTag = d1.headers.get("etag")!;
102 expect(dTag).not.toBe(d0);
103 expect((await send(ops, owner, "GET", { "if-none-match": o0 })).status).toBe(304);
104 expect((await send(`${T}/docs/members/bob`, owner, "DELETE")).status).toBe(204);
105 const d2 = await send(docs, owner, "GET", { "if-none-match": dTag });
106 expect(d2.status).toBe(200);
107 expect(d2.headers.get("etag")).not.toBe(dTag);
108 expect((await send(ops, owner, "GET", { "if-none-match": o0 })).status).toBe(304);
109 });
110
111 test("a 404 carries no tag and a viewer who cannot see the team never gets a 304", async () => {
112 const own = (await send(`${T}/docs/members`, owner)).headers.get("etag")!;
113 for (const path of ["/api/v1/orgs/nope/teams", "/api/v1/orgs/nope/teams/x/members", `${T}/nope/members`]) {
114 const out = await send(path, owner, "GET", { "if-none-match": own });
115 expect(out.status, path).toBe(404);
116 expect(out.headers.get("etag"), path).toBeNull();
117 }
118 for (const who of [stranger, null]) {
119 for (const t of [own, "*"]) {
120 const out = await send(`${T}/docs/members`, who, "GET", { "if-none-match": t });
121 expect(out.status).toBe(404);
122 expect(out.headers.get("etag")).toBeNull();
123 }
124 }
125 const ownTeams = (await send(T, owner)).headers.get("etag")!;
126 const theirs = await send(T, stranger, "GET", { "if-none-match": ownTeams });
127 expect(theirs.status).toBe(200);
128 expect(theirs.headers.get("etag")).not.toBe(ownTeams);
129 });
130
131 test("a refused write carries no tag", async () => {
132 const out = await send(T, stranger, "POST", {}, { name: "mine" });
133 expect(out.status).toBeGreaterThanOrEqual(400);
134 expect(out.headers.get("etag")).toBeNull();
135 });
136 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs