Public: anyone can read this; the crew can change it.
Remove on a cloud account goes through the confirm page, which names the account and says its sealed token goes with it #8014
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/7982-remove-on-a-cloud-account-goes-through-t → main 1 commit fork 1 thread · 1 unresolved
3 files
changed +120
−5
src/web/pages/cloud.ts +11 −5
| @@ -30,7 +30,7 @@ import { notFoundPage, settingsLayout, signInFirst } from "../chrome.ts"; | ||
| 30 | 30 | import { field, readForm } from "../form.ts"; |
| 31 | 31 | import { html, when, type Html } from "../html.ts"; |
| 32 | 32 | import { flashTo, page } from "../layout.ts"; |
| 33 | import { btn, card, icon, row, time } from "../ui.ts"; | |
| 33 | import { btn, card, confirmAction, icon, row, time } from "../ui.ts"; | |
| 34 | 34 | import { accountSections } from "./origins.ts"; |
| 35 | 35 | |
| 36 | 36 | const BACK = "/settings/cloud"; |
| @@ -86,9 +86,11 @@ function accountsCard(accounts: CloudAccount[], reads: CloudRead[], uses: Map<st | ||
| 86 | 86 | autocomplete="off" spellcheck="false" required /> |
| 87 | 87 | ${btn("Rename", { size: "sm", label: `Rename ${a.name}` })} |
| 88 | 88 | </form> |
| 89 | <form method="post" action="/settings/cloud/remove"> | |
| 90 | ${hidden(a.provider, a.name)}${btn("Remove", { size: "sm", icon: "x", label: `Remove ${a.name}` })} | |
| 91 | </form> | |
| 89 | ${confirmAction({ | |
| 90 | action: `/settings/cloud/remove?provider=${encodeURIComponent(a.provider)}&name=${encodeURIComponent(a.name)}`, | |
| 91 | verb: "Remove", | |
| 92 | size: "sm", | |
| 93 | })} | |
| 92 | 94 | </div>`, |
| 93 | 95 | }); |
| 94 | 96 | }); |
| @@ -275,6 +277,10 @@ export async function cloudRemove(ctx: Ctx): Promise<Response> { | ||
| 275 | 277 | if (denied) return denied; |
| 276 | 278 | const form = await readForm(ctx, "admin"); |
| 277 | 279 | if (form instanceof Response) return form; |
| 278 | const r = removeAccount(ctx.app, ctx.auth.user, field(form, "provider"), field(form, "name")); | |
| 280 | // The confirm page posts to its `to` exactly, so the account arrives in the query. | |
| 281 | const query = ctx.url.searchParams; | |
| 282 | const provider = field(form, "provider") || (query.get("provider") ?? ""); | |
| 283 | const name = field(form, "name") || (query.get("name") ?? ""); | |
| 284 | const r = removeAccount(ctx.app, ctx.auth.user, provider, name); | |
| 279 | 285 | return said(ctx, r, "Removed.", r.ok ? undefined : r.error); |
| 280 | 286 | } |
src/web/pages/search.ts +10
| @@ -663,6 +663,16 @@ const DESTRUCTIVE: { | ||
| 663 | 663 | phrase: "", |
| 664 | 664 | }), |
| 665 | 665 | }, |
| 666 | { | |
| 667 | pattern: /^\/settings\/cloud\/remove\?provider=([a-z0-9_-]+)&name=([a-z][a-z0-9_]{0,31})$/, | |
| 668 | describe: (m) => ({ | |
| 669 | verb: "Remove account", | |
| 670 | title: `Remove the ${m[1]} account ${m[2]}?`, | |
| 671 | detail: | |
| 672 | "Its sealed token is deleted from this forge. Anything that used it stops reaching that provider until you add an account again. Nothing at the provider is touched.", | |
| 673 | phrase: "", | |
| 674 | }), | |
| 675 | }, | |
| 666 | 676 | { |
| 667 | 677 | pattern: /^\/settings\/keys\/([^/]+)\/delete$/, |
| 668 | 678 | describe: () => ({ |
test/confirm-cloud-remove.test.ts added +99
| @@ -0,0 +1,99 @@ | ||
| 1 | /** | |
| 2 | * #7982: removing a cloud account deletes its sealed token, so Settings › | |
| 3 | * Cloud links Remove to /-/confirm, which names the account and posts the | |
| 4 | * removal on the second step with the account in the query. | |
| 5 | */ | |
| 6 | import { afterAll, beforeAll, 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 { addAccount, net } from "../src/actions/cloud.ts"; | |
| 12 | import { listCloudAccounts } from "../src/core/cloud_account.ts"; | |
| 13 | import { createUser } from "../src/core/users.ts"; | |
| 14 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 15 | import { quietLogs } from "./helpers/globals.ts"; | |
| 16 | ||
| 17 | quietLogs(); | |
| 18 | ||
| 19 | const TOKEN = "do-live-4Hn8RsWq2Lm6Tx9cVb3K"; | |
| 20 | ||
| 21 | /** A fake DigitalOcean that accepts TOKEN and answers like the real API. */ | |
| 22 | net.fetch = async (url, init) => { | |
| 23 | const bad = !String((init?.headers as any)?.authorization).includes(TOKEN); | |
| 24 | const u = new URL(url); | |
| 25 | const reply = (o: unknown) => new Response(JSON.stringify(o), { status: bad ? 401 : 200 }); | |
| 26 | if (u.pathname === "/v2/account") return reply({ account: { email: "[email protected]", status: "active" } }); | |
| 27 | if (u.pathname === "/v2/projects") return reply({ projects: [] }); | |
| 28 | if (u.pathname === "/v2/droplets") return reply({ meta: { total: 0 }, droplets: [] }); | |
| 29 | if (u.pathname === "/v2/volumes") return reply({ volumes: [] }); | |
| 30 | if (u.pathname === "/v2/reserved_ips") return reply({ reserved_ips: [] }); | |
| 31 | return new Response("{}", { status: 404 }); | |
| 32 | }; | |
| 33 | ||
| 34 | let dataDir: string; | |
| 35 | let app: App; | |
| 36 | let servers: Servers; | |
| 37 | let cookie: string; | |
| 38 | const to = "/settings/cloud/remove?provider=digitalocean&name=main"; | |
| 39 | ||
| 40 | beforeAll(async () => { | |
| 41 | dataDir = await mkdtemp(join(tmpdir(), "forge-confirm-cloud-remove-")); | |
| 42 | app = createApp({ dataDir, port: 0 }); | |
| 43 | servers = await serve(app); | |
| 44 | const admin = await createUser(app.db, { | |
| 45 | origin: "seed", | |
| 46 | trusted: true, | |
| 47 | username: "alice", | |
| 48 | email: "[email protected]", | |
| 49 | password: "a-good-passphrase", | |
| 50 | adminIfFirst: true, | |
| 51 | }); | |
| 52 | if (!admin.ok) throw new Error("user"); | |
| 53 | const added = await addAccount(app, admin.value, "digitalocean", "main", TOKEN); | |
| 54 | if (!added.ok) throw new Error(JSON.stringify(added.error)); | |
| 55 | const signin = await fetch(`${servers.url}/signin`, { | |
| 56 | method: "POST", | |
| 57 | redirect: "manual", | |
| 58 | headers: { "content-type": "application/x-www-form-urlencoded", origin: servers.url }, | |
| 59 | body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }), | |
| 60 | }); | |
| 61 | cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!; | |
| 62 | }, 30_000); | |
| 63 | ||
| 64 | afterAll(async () => { | |
| 65 | servers.stop(); | |
| 66 | app.close(); | |
| 67 | await rm(dataDir, { recursive: true, force: true }); | |
| 68 | }); | |
| 69 | ||
| 70 | test("Settings › Cloud sends Remove through the confirm page", async () => { | |
| 71 | const page = await (await fetch(`${servers.url}/settings/cloud`, { headers: { cookie } })).text(); | |
| 72 | expect(page).toContain(`href="/-/confirm?to=${encodeURIComponent(to)}"`); | |
| 73 | expect(page).not.toContain(`action="/settings/cloud/remove"`); | |
| 74 | }); | |
| 75 | ||
| 76 | test("the confirm page names the account and refuses a crafted target", async () => { | |
| 77 | const confirm = await fetch(`${servers.url}/-/confirm?to=${encodeURIComponent(to)}`, { headers: { cookie } }); | |
| 78 | expect(confirm.status).toBe(200); | |
| 79 | const body = await confirm.text(); | |
| 80 | expect(body).toContain("Remove the digitalocean account main?"); | |
| 81 | expect(body).toContain("Its sealed token is deleted from this forge."); | |
| 82 | expect(body).toContain(`action="${to.replace("&", "&")}"`); | |
| 83 | ||
| 84 | const crafted = `${to}&back=/elsewhere`; | |
| 85 | const refused = await fetch(`${servers.url}/-/confirm?to=${encodeURIComponent(crafted)}`, { headers: { cookie } }); | |
| 86 | expect(refused.status).toBe(404); | |
| 87 | }); | |
| 88 | ||
| 89 | test("the POST from the confirm page removes the account", async () => { | |
| 90 | expect(listCloudAccounts(app.db).map((a) => a.name)).toEqual(["main"]); | |
| 91 | const res = await fetch(`${servers.url}${to}`, { | |
| 92 | method: "POST", | |
| 93 | redirect: "manual", | |
| 94 | headers: { cookie, "content-type": "application/x-www-form-urlencoded", origin: servers.url }, | |
| 95 | body: "", | |
| 96 | }); | |
| 97 | expect(res.status).toBe(303); | |
| 98 | expect(listCloudAccounts(app.db)).toEqual([]); | |
| 99 | }); |
new above
offline
Next: pass an accessible label (Remove <name>) so each link is distinguishable to a screen reader, as the old button was.