trevato/forge

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

More
Forks1g fMapg aRuns16g rEventsFlows

A member cannot mint an admin-scope token; ro and rw tokens stay as they are #2570

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/2425-a-member-cannot-mint-an-admin-scope-toke → main 2 commits fork 2 threads · 2 unresolved

  • Merged. scout/2425-a-member-cannot-mint-an-admin-scope-toke was merged into main as 5ba4c23.

Review

Sign in to review this change.
16 files changed +134 −113
README.md +4 −1
@@ -114,7 +114,10 @@ forge, at the sign-in limits (429 past them). The account made is in the audit
114114 as `user.created` `via: first-admin`, and the sign-in page links `/first-admin`
115115 only while the forge has no admin. Later tokens come from Settings → Tokens, or
116116 `POST /api/v1/tokens` from a signed-in session with `{"name", "scope"}`; the
117 admin adds `"adminConfirm": true` to mean a token that acts as admin.
117 admin adds `"adminConfirm": true` to mean a token that acts as admin. `ro` and
118 `rw` are open to anyone; a member asking for `admin` gets 403 ("Only an admin
119 can make a token with admin scope; ro and rw are open to you."), and the
120 Settings form offers `admin` only to an admin.
118121
119122 The same token spawns a child forge (`dryRun` is true unless `false`, so
120123 without it you get the plan and nothing is made). The child's admin comes
src/actions/users.ts +3 −1
@@ -193,9 +193,11 @@ export function createToken(
193193 name: string,
194194 scope: TokenScope,
195195 options: { expiresAt?: string } = {},
196 ): Result<{ id: string; token: string }, "unauthorized"> {
196 ): Result<{ id: string; token: string }, "unauthorized" | "forbidden"> {
197197 const user = requireUser(actor);
198198 if (!user.ok) return user;
199 // A token that acts as admin is for an admin to hold; ro and rw are anyone's.
sentinelcrew

Good: this is the one gate, and it reads the actor's stored role. Every caller (the API, the page and recovery) gets it for free.
200 if (scope === "admin" && !user.value.isAdmin) return err("forbidden");
199201 const minted = mintToken(app.db, user.value.id, name, scope, options);
200202 app.bus.append({
201203 kind: "token.created",
src/core/tokens.ts +3
@@ -13,6 +13,9 @@ import { getUserById, isActive, type User } from "./users.ts";
1313
1414 export type TokenScope = "ro" | "rw" | "admin";
1515
16 /** What a non-admin is told when they ask for a token that acts as admin. */
17 export const ADMIN_TOKEN_WORDS = "Only an admin can make a token with admin scope; ro and rw are open to you.";
18
1619 const rank: Record<TokenScope, number> = { ro: 0, rw: 1, admin: 2 };
1720
1821 export function scopeAllows(held: TokenScope, needed: TokenScope): boolean {
src/http/api.ts +3 −1
@@ -221,7 +221,7 @@ import {
221221 } from "../core/reviews.ts";
222222 import { createSession } from "../core/sessions.ts";
223223 import { mintProposal } from "../actions/proposal_tokens.ts";
224 import { listTokens } from "../core/tokens.ts";
224 import { ADMIN_TOKEN_WORDS, listTokens } from "../core/tokens.ts";
225225 import {
226226 headInNamespace,
227227 PROPOSAL_BODY_MAX,
@@ -1854,6 +1854,8 @@ export function apiRoutes(router: Router<Handler>): void {
18541854 str(input.name) || "unnamed",
18551855 scope,
18561856 );
1857 if (!minted.ok && minted.error === "forbidden")
1858 return apiError("forbidden", 403, ADMIN_TOKEN_WORDS);
18571859 if (!minted.ok) return unauthorized();
18581860 return json(minted.value, 201);
18591861 });
src/web/pages/account.ts +6 −2
@@ -31,7 +31,7 @@ import { ABILITIES, roleOf, rowOf, type Ability } from "../../core/roles.ts";
3131 import { listKeys } from "../../core/keys.ts";
3232 import { endAccountSession } from "../../actions/signins.ts";
3333 import { listSessions, type SessionInfo } from "../../core/sessions.ts";
34 import { listTokens, type TokenScope } from "../../core/tokens.ts";
34 import { ADMIN_TOKEN_WORDS, listTokens, type TokenScope } from "../../core/tokens.ts";
3535 import type { CommitEmail, User } from "../../core/users.ts";
3636 import { cookieValue, type Ctx } from "../../http/context.ts";
3737 import { sshCommand } from "../../ssh/command.ts";
@@ -539,7 +539,9 @@ function tokensView(ctx: Ctx): Html {
539539 options: [
540540 { value: "rw", label: "read + write — clone, push, open issues" },
541541 { value: "ro", label: "read only — clone and browse" },
542 { value: "admin", label: "admin — everything, including settings" },
542 ...(ctx.auth.user?.isAdmin
543 ? [{ value: "admin", label: "admin — everything, including settings" }]
544 : []),
543545 ],
544546 hint: "A token can never mint another token, whatever its scope.",
545547 })}
@@ -594,6 +596,8 @@ export async function tokenCreate(ctx: Ctx): Promise<Response> {
594596 field(form, "name").trim() || "token",
595597 chosen,
sentinelcrew outdated

This leaves the settings page for a bare plain-text page with no way back. Use flashTo(ctx, "/settings/tokens", { tone: "danger", message: ADMIN_TOKEN_WORDS }) as the aiAdmin refusal above does. Also stop offering the admin option in the form to non-admins, so the only way to reach this branch is a forged request.
596598 );
599 if (!minted.ok && minted.error === "forbidden")
600 return flashTo(ctx, "/settings/tokens", { tone: "danger", message: ADMIN_TOKEN_WORDS });
597601 if (!minted.ok) return signInFirst(ctx);
598602 // Post-redirect-get: the secret rides a one-shot flash, so a refresh shows
599603 // the list rather than minting a second token.
test/apps-api.test.ts +2 −14
@@ -17,6 +17,7 @@ import { getUserByName } from "../src/core/users.ts";
1717 import { serve, type Servers } from "../src/http/server.ts";
1818 import { startCrew, type CrewHandle } from "../src/crew/dispatch.ts";
1919 import { seedAdmin } from "./helpers/admin.ts";
20 import { tokenFor } from "./helpers/tokens.ts";
2021
2122 /** Read from an SSE body until `predicate(buffer)` holds or we time out —
2223 * never to a "done" that #284's still-live stream no longer sends. */
@@ -455,20 +456,7 @@ describe("apps, runs and the crew over the API", () => {
455456 },
456457 null,
457458 );
458 const mallory = (
459 (await (
460 await post(
461 "/api/v1/tokens",
462 {
463 username: "mallory",
464 password: "a-good-passphrase",
465 name: "admin",
466 scope: "admin",
467 },
468 null,
469 )
470 ).json()) as { token: string }
471 ).token;
459 const mallory = tokenFor(app, "mallory", "admin", "admin");
472460 expect((await api(env, { token: mallory })).status).toBe(404);
473461 expect((await put("GREETING", "x", mallory)).status).toBe(404);
474462 expect(
test/cross-repo-pull.test.ts +2 −12
@@ -7,6 +7,7 @@ import { serve, type Servers } from "../src/http/server.ts";
77 import { gitClientEnv } from "./helpers/git-client.ts";
88 import { proveAll } from "./helpers/verified.ts";
99 import { promote } from "./helpers/members.ts";
10 import { tokenFor } from "./helpers/tokens.ts";
1011
1112 const gitEnv = gitClientEnv({
1213 GIT_AUTHOR_NAME: "X",
@@ -56,18 +57,7 @@ describe("cross-repo pull requests (fork → upstream)", () => {
5657 redirect: "manual",
5758 });
5859 const cookie = (signup.headers.get("set-cookie") ?? "").split(";")[0]!;
59 const token = await fetch(`${base}/api/v1/tokens`, {
60 method: "POST",
61 headers: { "content-type": "application/json" },
62 body: JSON.stringify({ adminConfirm: true,
63 username,
64 password: "cross-repo-pass",
65 name: "c",
66 scope: "admin",
67 }),
68 })
69 .then((r) => r.json())
70 .then((j) => (j as { token: string }).token);
60 const token = tokenFor(app, username, "admin", "c");
7161 return { token, cookie };
7262 }
7363
test/everyday.test.ts +21
@@ -32,6 +32,7 @@ import { createUser, getUserByName } from "../src/core/users.ts";
3232 import { serve, type Servers } from "../src/http/server.ts";
3333 import { quietLogs } from "./helpers/globals.ts";
3434 import { passkeyFields } from "./helpers/passkey.ts";
35 import { ADMIN_TOKEN_WORDS } from "../src/core/tokens.ts";
3536
3637 quietLogs();
3738
@@ -241,6 +242,26 @@ describe("the everyday account", () => {
241242 );
242243 });
243244
245 test("the token page keeps a member on the page with the sentence for admin scope, and makes rw", async () => {
246 const kim = await signIn("kim");
247 const count = () =>
248 (
249 app.db
250 .query("SELECT COUNT(*) AS n FROM token WHERE user_id = ?1")
251 .get(getUserByName(app.db, "kim")!.id) as { n: number }
252 ).n;
253 const before = count();
254 expect(await page(kim, "/settings/tokens")).not.toContain('value="admin"');
255 const denied = await post(kim, "/settings/tokens", { name: "big", scope: "admin" });
256 expect(denied.status).toBe(303);
257 expect(new URL(denied.headers.get("location")!, servers.url).pathname).toBe("/settings/tokens");
258 const flash = (denied.headers.get("set-cookie") ?? "").split(";")[0]!;
259 expect(decodeURIComponent(flash.replace(/\+/g, " "))).toContain(ADMIN_TOKEN_WORDS);
260 expect(count()).toBe(before);
261 await post(kim, "/settings/tokens", { name: "small", scope: "rw" });
262 expect(count()).toBe(before + 1);
263 });
264
244265 test("from a page's session, the API refuses an admin's token without adminConfirm", async () => {
245266 const root = await signIn("ada");
246267 const mint = (body: Record<string, unknown>) =>
test/fork-flow.test.ts +2 −13
@@ -16,6 +16,7 @@ import {
1616 } from "./fake-origin.ts";
1717 import { proveAll } from "./helpers/verified.ts";
1818 import { promote } from "./helpers/members.ts";
19 import { tokenFor } from "./helpers/tokens.ts";
1920
2021 /**
2122 * The fork flow: grab a repository you cannot push to, branch, edit, and
@@ -100,19 +101,7 @@ describe("the fork flow", () => {
100101 password: PASSWORD,
101102 }),
102103 });
103 const token = await read<{ token: string }>(
104 await fetch(`${server.url}/api/v1/tokens`, {
105 method: "POST",
106 headers: { "content-type": "application/json" },
107 body: JSON.stringify({ adminConfirm: true,
108 username,
109 password: PASSWORD,
110 name: "t",
111 scope: "admin",
112 }),
113 }),
114 );
115 return token.token;
104 return tokenFor(server === originServers ? originApp : app, username, "admin");
116105 }
117106
118107 /** Seed a public repo on the upstream forge with one file on `main`. */
test/helpers/tokens.ts added +14
@@ -0,0 +1,14 @@
1 import type { App } from "../../src/app.ts";
2 import { createToken } from "../../src/core/tokens.ts";
3 import { getUserByName } from "../../src/core/users.ts";
4
5 /**
6 * A token of a scope for an account a suite already made, minted directly:
7 * the API refuses `admin` to a non-admin (#2425), and suites that want a
8 * member's full-account token take it here, as the operator's tools would.
9 */
10 export function tokenFor(app: App, username: string, scope: "ro" | "rw" | "admin", name = "t"): string {
11 const user = getUserByName(app.db, username);
12 if (!user) throw new Error(`no account ${username}`);
13 return createToken(app.db, user.id, name, scope).token;
14 }
test/orgs.test.ts +4 −13
@@ -26,6 +26,7 @@ import { createUser, getUserByName } from "../src/core/users.ts";
2626 import { serve, type Servers } from "../src/http/server.ts";
2727 import { gitClientEnv } from "./helpers/git-client.ts";
2828 import { seedAdmin } from "./helpers/admin.ts";
29 import { tokenFor } from "./helpers/tokens.ts";
2930
3031 const gitEnv = gitClientEnv({
3132 GIT_AUTHOR_NAME: "O",
@@ -218,19 +219,9 @@ describe("orgs over HTTP", () => {
218219 });
219220 return tokenOf(username);
220221 }
221 async function tokenOf(username: string): Promise<string> {
222 return fetch(`${base}/api/v1/tokens`, {
223 method: "POST",
224 headers: { "content-type": "application/json" },
225 body: JSON.stringify({ adminConfirm: true,
226 username,
227 password: "org-http-pass",
228 name: "c",
229 scope: "admin",
230 }),
231 })
232 .then((r) => r.json())
233 .then((j) => (j as { token: string }).token);
222 function tokenOf(username: string): string {
223 // An account that could not be made (a reserved name) has no token, as before.
224 return getUserByName(app.db, username) ? tokenFor(app, username, "admin", "c") : "";
234225 }
235226
236227 beforeAll(async () => {
test/parity.test.ts +2 −6
@@ -19,6 +19,7 @@ import { serve, type Servers } from "../src/http/server.ts";
1919 import { gitClientEnv } from "./helpers/git-client.ts";
2020 import { proveAll } from "./helpers/verified.ts";
2121 import { promote } from "./helpers/members.ts";
22 import { tokenFor } from "./helpers/tokens.ts";
2223
2324 const gitEnv = gitClientEnv({
2425 GIT_AUTHOR_NAME: "A",
@@ -98,12 +99,7 @@ describe("parity: full control through actions and the API", () => {
9899 };
99100 }
100101
101 const mint = async (user: string, scope: string) =>
102 (
103 await call("POST", "/api/v1/tokens", {
104 body: { adminConfirm: true, username: user, password: PASS, name: scope, scope },
105 })
106 ).json.token as string;
102 const mint = async (user: string, scope: "ro" | "rw" | "admin") => tokenFor(app, user, scope, scope);
107103
108104 const kinds = () =>
109105 listEvents(app.db, { limit: 1000 })
test/reviews-states.test.ts +2 −13
@@ -6,6 +6,7 @@ import { createApp, type App } from "../src/app.ts";
66 import { serve, type Servers } from "../src/http/server.ts";
77 import { listEvents } from "../src/core/events.ts";
88 import { seedAdmin } from "./helpers/admin.ts";
9 import { tokenFor } from "./helpers/tokens.ts";
910
1011 /**
1112 * Review states: the verdict that gates a merge.
@@ -56,19 +57,7 @@ describe("review states, and the merge they gate", () => {
5657 }
5758
5859 async function mintToken(username: string): Promise<void> {
59 const token = await read<{ token: string }>(
60 await fetch(`${base}/api/v1/tokens`, {
61 method: "POST",
62 headers: { "content-type": "application/json" },
63 body: JSON.stringify({ adminConfirm: true,
64 username,
65 password: PASSWORD,
66 name: "t",
67 scope: "admin",
68 }),
69 }),
70 );
71 tokens[username] = token.token;
60 tokens[username] = tokenFor(app, username, "admin");
7261 }
7362
7463 /** The pull number of a fresh branch with one file changed on it. */
test/secret-store.test.ts +2 −15
@@ -20,6 +20,7 @@ import { createUser, type User } from "../src/core/users.ts";
2020 import { open, seal } from "../src/kernel/secret.ts";
2121 import { serve, type Servers } from "../src/http/server.ts";
2222 import { quietLogs } from "./helpers/globals.ts";
23 import { tokenFor } from "./helpers/tokens.ts";
2324
2425 quietLogs();
2526
@@ -49,21 +50,7 @@ describe("forge-level secrets", () => {
4950 "content-type": "application/json",
5051 },
5152 });
52 const mint = async (username: string): Promise<string> =>
53 (
54 (await (
55 await fetch(`${servers.url}/api/v1/tokens`, {
56 method: "POST",
57 headers: { "content-type": "application/json" },
58 body: JSON.stringify({ adminConfirm: true,
59 username,
60 password: "a-good-passphrase",
61 name: "t",
62 scope: "admin",
63 }),
64 })
65 ).json()) as { token: string }
66 ).token;
53 const mint = async (username: string): Promise<string> => tokenFor(app, username, "admin");
6754
6855 beforeAll(async () => {
6956 dataDir = await mkdtemp(join(tmpdir(), "forge-secret-store-"));
test/token-admin-scope.test.ts added +61
@@ -0,0 +1,61 @@
1 /**
2 * #2425: a member cannot mint an admin-scope token (403, one plain sentence);
3 * ro and rw are unchanged, and an admin still mints admin.
4 */
5
6 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
7 import { mkdtemp, rm } from "node:fs/promises";
8 import { tmpdir } from "node:os";
9 import { join } from "node:path";
10 import { createApp, type App } from "../src/app.ts";
11 import { signUp } from "../src/actions/users.ts";
12 import { ADMIN_TOKEN_WORDS } from "../src/core/tokens.ts";
13 import { serve, type Servers } from "../src/http/server.ts";
14 import { quietLogs } from "./helpers/globals.ts";
15 import { seedAdmin } from "./helpers/admin.ts";
16 import { promote } from "./helpers/members.ts";
17 import { fixtureSlot } from "./helpers/slot.ts";
18
19 quietLogs();
20
21 const PASS = "a-long-member-passphrase";
22
23 describe("minting an admin-scope token", () => {
24 let dataDir = "";
25 let app: App;
26 let servers: Servers;
27
28 const mint = (username: string, scope: string) =>
29 fetch(`${servers.url}/api/v1/tokens`, {
30 method: "POST",
31 headers: { "content-type": "application/json" },
32 body: JSON.stringify({ username, password: PASS, name: "t", scope, adminConfirm: true }),
33 });
34
35 beforeAll(async () => {
36 dataDir = await mkdtemp(join(tmpdir(), "forge-token-admin-"));
37 app = createApp({ dataDir, port: 0, url: "https://forge.example" });
38 servers = await serve(app);
39 await seedAdmin(app, { username: "olive", email: "[email protected]", password: PASS });
40 const made = await signUp(app, { username: "bob", email: "[email protected]", password: PASS }, { slot: fixtureSlot(app) });
41 if (!made.ok) throw new Error(made.error);
42 promote(app.db, made.value);
43 });
44
45 afterAll(async () => {
46 await servers.stop();
47 await rm(dataDir, { recursive: true, force: true });
48 });
49
50 test("a member gets 403 for admin and 201 for rw and ro", async () => {
51 const denied = await mint("bob", "admin");
52 expect(denied.status).toBe(403);
53 expect(((await denied.json()) as { message: string }).message).toBe(ADMIN_TOKEN_WORDS);
54 expect((await mint("bob", "rw")).status).toBe(201);
55 expect((await mint("bob", "ro")).status).toBe(201);
56 });
57
58 test("an admin can still mint admin", async () => {
59 expect((await mint("olive", "admin")).status).toBe(201);
60 });
61 });
test/web-workflows.test.ts +3 −22
@@ -42,6 +42,7 @@ import { quietLogs } from "./helpers/globals.ts";
4242 import { newFakeState, serveFakeForge, type FakeForge } from "./fake-origin.ts";
4343 import { proveAll } from "./helpers/verified.ts";
4444 import { promote } from "./helpers/members.ts";
45 import { tokenFor } from "./helpers/tokens.ts";
4546
4647 quietLogs();
4748
@@ -140,17 +141,7 @@ describe("fork at the origin, and propose a change through it", () => {
140141 });
141142 proveAll(upApp);
142143 promote(upApp.db);
143 const made = await fetch(`${upServers.url}/api/v1/tokens`, {
144 method: "POST",
145 headers: { "content-type": "application/json" },
146 body: JSON.stringify({ adminConfirm: true,
147 username,
148 password: PASSWORD,
149 name: "t",
150 scope: "admin",
151 }),
152 });
153 return ((await made.json()) as { token: string }).token;
144 return tokenFor(upApp, username, "admin");
154145 }
155146
156147 const upstream = (
@@ -577,17 +568,7 @@ describe("verdicts, threads, and the gate they open", () => {
577568 cookies[username] = (signed.headers.get("set-cookie") ?? "").split(";")[0]!;
578569 proveAll(app);
579570 promote(app.db);
580 const token = await fetch(`${base}/api/v1/tokens`, {
581 method: "POST",
582 headers: { "content-type": "application/json" },
583 body: JSON.stringify({ adminConfirm: true,
584 username,
585 password: PASSWORD,
586 name: "t",
587 scope: "admin",
588 }),
589 });
590 tokens[username] = ((await token.json()) as { token: string }).token;
571 tokens[username] = tokenFor(app, username, "admin");
591572 }
592573
593574 /** The first line of the diff, as a review anchor form would name it. */

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs