Public: anyone can read this; the crew can change it.
The device-code connect page polls on a timer: move it to a stream or a live-region swap #11024
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/10964-the-device-code-connect-page-polls-on-a → main 5 commits fork 3 threads · 3 unresolved
9 files
changed +164
−45
docs/ORIGINS.md +2 −1
| @@ -101,7 +101,8 @@ is "No such origin."). Which | ||
| 101 | 101 | way in you get is a property of the forge, not a preference: |
| 102 | 102 | |
| 103 | 103 | - **GitHub** — a device code. The forge shows a short code and a URL; you type |
| 104 | the code on whatever device has a browser, and the forge does the polling. Setting | |
| 104 | the code on whatever device has a browser, and the forge does the polling (once per | |
| 105 | flow however many tabs wait; the page is told over a stream and reloads once). Setting | |
| 105 | 106 | it up is three steps, once, for everyone on the forge: |
| 106 | 107 | 1. Create an OAuth App at . Name it |
| 107 | 108 | anything, set the homepage to your forge's URL, set the authorization |
src/http/flow_stream.ts added +87
| @@ -0,0 +1,87 @@ | ||
| 1 | /** | |
| 2 | * GET /settings/origins/flows/:id/stream: a device-code connect, waited on by | |
| 3 | * the forge instead of by the page. The provider sets the cadence and sends no | |
| 4 | * event, so the forge asks it on that cadence, here, and tells the browser once: | |
| 5 | * `flow.settled` with the status, after which the stream ends and the page | |
| 6 | * re-reads itself. The page never loops. | |
| 7 | */ | |
| 8 | ||
| 9 | import { pollOAuthConnect } from "../actions/origins.ts"; | |
| 10 | import { getFlowFor } from "../origins/oauth.ts"; | |
| 11 | import type { Ctx } from "./context.ts"; | |
| 12 | import { tryAgain } from "./respond.ts"; | |
| 13 | import { eventStream } from "./sse.ts"; | |
| 14 | import { notFoundPage } from "../web/chrome.ts"; | |
| 15 | ||
| 16 | /** A tab may wait this long; the page re-reads itself when the stream ends. */ | |
| 17 | const MAX_WAIT_MS = 15 * 60_000; | |
| 18 | /** Tabs on one flow beyond this are refused: one person has no use for more. */ | |
| 19 | const MAX_TABS_PER_FLOW = 8; | |
| 20 | ||
| 21 | type Settled = { status: string }; | |
| 22 | type Poller = { listeners: Set<(s: Settled) => void>; timer?: ReturnType<typeof setTimeout> }; | |
| 23 | ||
| 24 | /** One poller per flow, shared by every tab that waits on it: the provider is asked once per interval. */ | |
| 25 | const pollers = new Map<string, Poller>(); | |
| 26 | ||
| 27 | function subscribe(ctx: Ctx, flowId: string, firstDelaySecs: number, onSettled: (s: Settled) => void): (() => void) | null { | |
| 28 | const user = ctx.auth.user!; | |
| 29 | let poller = pollers.get(flowId); | |
| 30 | if (poller && poller.listeners.size >= MAX_TABS_PER_FLOW) return null; | |
| 31 | if (!poller) { | |
| 32 | const made: Poller = { listeners: new Set() }; | |
| 33 | poller = made; | |
| 34 | pollers.set(flowId, made); | |
| 35 | const finish = (s: Settled): void => { | |
| 36 | pollers.delete(flowId); | |
| 37 | if (made.timer) clearTimeout(made.timer); | |
| 38 | for (const listener of [...made.listeners]) listener(s); | |
| 39 | }; | |
| 40 | const step = async (): Promise<void> => { | |
| 41 | if (pollers.get(flowId) !== made) return; | |
| 42 | try { | |
| 43 | const polled = await pollOAuthConnect(ctx.app, user, flowId); | |
| 44 | if (pollers.get(flowId) !== made) return; | |
| 45 | if (!polled.ok || polled.value.status !== "pending") return finish({ status: polled.ok ? polled.value.status : "failed" }); | |
| 46 | made.timer = setTimeout(() => void step(), Math.max(1, polled.value.intervalSecs) * 1000); | |
| 47 | } catch { | |
| 48 | finish({ status: "failed" }); | |
| 49 | } | |
| 50 | }; | |
| 51 | made.timer = setTimeout(() => void step(), Math.max(1, firstDelaySecs) * 1000); | |
| 52 | } | |
| 53 | const mine = poller; | |
| 54 | mine.listeners.add(onSettled); | |
| 55 | return () => { | |
| 56 | mine.listeners.delete(onSettled); | |
| 57 | if (mine.listeners.size === 0 && pollers.get(flowId) === mine) { | |
| 58 | if (mine.timer) clearTimeout(mine.timer); | |
| 59 | pollers.delete(flowId); | |
| 60 | } | |
| 61 | }; | |
| 62 | } | |
| 63 | ||
| 64 | export function originFlowStream(ctx: Ctx): Response { | |
| 65 | const user = ctx.auth.user; | |
| 66 | if (!user) return new Response("sign in first", { status: 401 }); | |
| 67 | const flowId = ctx.params.id ?? ""; | |
| 68 | const flow = getFlowFor(ctx.app.db, user.id, flowId); | |
| 69 | if (!flow) return notFoundPage(ctx); | |
| 70 | if ((pollers.get(flowId)?.listeners.size ?? 0) >= MAX_TABS_PER_FLOW) return tryAgain("too_many_tabs", 429, "Too many tabs are waiting on this flow.", 30); | |
| 71 | ||
| 72 | return eventStream(ctx.app, ctx.request, (emit, close) => { | |
| 73 | let lifetime: ReturnType<typeof setTimeout> | undefined; | |
| 74 | const leave = subscribe(ctx, flowId, flow.intervalSecs, (settled) => { | |
| 75 | emit("flow.settled", settled); | |
| 76 | close(); | |
| 77 | }); | |
| 78 | lifetime = setTimeout(() => { | |
| 79 | emit("flow.settled", { status: "pending" }); | |
| 80 | close(); | |
| 81 | }, MAX_WAIT_MS); | |
| 82 | return () => { | |
| 83 | leave?.(); | |
| 84 | clearTimeout(lifetime); | |
| 85 | }; | |
| 86 | }); | |
| 87 | } |
src/http/settings_routes.ts +2
| @@ -128,6 +128,7 @@ import { | ||
| 128 | 128 | originTrustedHosts, |
| 129 | 129 | } from "../web/pages/origins.ts"; |
| 130 | 130 | import { originArchivedPage, originArchivedRestore } from "../web/pages/origin-archived.ts"; |
| 131 | import { originFlowStream } from "./flow_stream.ts"; | |
| 131 | 132 | import { originFlowPage, originFlowPoll } from "../web/pages/origin-flow.ts"; |
| 132 | 133 | import { originMirrorSubmit, originReposPage } from "../web/pages/origin-repos.ts"; |
| 133 | 134 | |
| @@ -309,6 +310,7 @@ export function settingsRoutes(router: Router<Handler>, knownPatterns: () => rea | ||
| 309 | 310 | router.add("POST", "/settings/origins/apps/:id/delete", originAppDelete); |
| 310 | 311 | router.add("GET", "/settings/origins/flows/:id", originFlowPage); |
| 311 | 312 | router.add("POST", "/settings/origins/flows/:id/poll", originFlowPoll); |
| 313 | router.add("GET", "/settings/origins/flows/:id/stream", originFlowStream); | |
| 312 | 314 | router.add("GET", "/settings/origins/:id/repos", originReposPage); |
| 313 | 315 | router.add("GET", "/settings/origins/:id/archived", originArchivedPage); |
| 314 | 316 | router.add("POST", "/settings/origins/:id/archived", originArchivedRestore); |
src/web/client.ts +13 −24
| @@ -62,34 +62,23 @@ const modalJs = /* js */ ` | ||
| 62 | 62 | `; |
| 63 | 63 | |
| 64 | 64 | /** |
| 65 | * The device-code page's only enhancement: submit the "check again" form on | |
| 66 | * the interval the provider asked for, and reload when the flow stops being | |
| 67 | * pending. Everything it does is something the page already offers a button | |
| 68 | * for, and the poll route answers JSON only because it was asked to — the | |
| 69 | * same POST from a plain form gets a redirect. | |
| 65 | * The device-code page's only enhancement: listen on the page's own stream | |
| 66 | * (`data-stream` on the "check again" form) and re-read the page when the | |
| 67 | * forge says the flow has left pending. The forge does the asking of the | |
| 68 | * provider (`src/http/flow_stream.ts`); this loops on nothing. Without script | |
| 69 | * the form's button and a reload still re-poll. | |
| 70 | 70 | * |
| 71 | * Shipped per page (`PageOptions.scriptJs`) rather than in the shell: a page | |
| 72 | * with no connect in flight should carry none of it. | |
| 71 | * Shipped per page (`PageOptions.scriptJs`) rather than in the shell. | |
| 73 | 72 | */ |
| 74 | 73 | export const pollJs = /* js */ ` |
| 75 | 74 | (function () { |
| 76 | var form = document.querySelector('form[data-poll]'); | |
| 77 | if (!form) return; | |
| 78 | var every = Number(form.getAttribute('data-poll') || 0); | |
| 79 | if (!every) return; | |
| 80 | var tries = 0; | |
| 81 | function ask() { | |
| 82 | if (tries++ > 200) return; | |
| 83 | fetch(form.getAttribute('action') || location.href, { | |
| 84 | method: 'POST', | |
| 85 | headers: { accept: 'application/json' }, | |
| 86 | body: new FormData(form), | |
| 87 | }).then(function (r) { return r.json(); }).then(function (out) { | |
| 88 | if (out && out.status && out.status !== 'pending') { location.reload(); return; } | |
| 89 | window.setTimeout(ask, every * 1000); | |
| 90 | }, function () { window.setTimeout(ask, every * 1000); }); | |
| 91 | } | |
| 92 | window.setTimeout(ask, every * 1000); | |
| 75 | var form = document.querySelector('form[data-stream]'); | |
| 76 | if (!form || typeof EventSource === 'undefined') return; | |
| 77 | var source = new EventSource(form.getAttribute('data-stream')); | |
| 78 | source.addEventListener('flow.settled', function () { | |
| 79 | source.close(); | |
| 80 | location.reload(); | |
| 81 | }); | |
| 93 | 82 | })(); |
| 94 | 83 | `; |
| 95 | 84 |
src/web/pages/origin-flow.ts +3 −5
| @@ -35,8 +35,8 @@ import { | ||
| 35 | 35 | * |
| 36 | 36 | * The device grant shows the code and where to type it; the code flow shows |
| 37 | 37 | * one link to the instance. Both work with no JavaScript: the page carries a |
| 38 | * "check again" form, and reloading it re-polls. With script, the same form | |
| 39 | * is submitted for you every couple of seconds. | |
| 38 | * "check again" form, and reloading it re-polls. With script, the page listens on | |
| 39 | * a stream the forge holds open and reloads when the flow settles. | |
| 40 | 40 | */ |
| 41 | 41 | export function originFlowPage(ctx: Ctx): Response { |
| 42 | 42 | const denied = accountOnly(ctx); |
| @@ -105,9 +105,7 @@ export function originFlowPage(ctx: Ctx): Response { | ||
| 105 | 105 | class="bar mt" |
| 106 | 106 | method="post" |
| 107 | 107 | action="${pollAction}" |
| 108 | data-poll="${flow.flow === "device" | |
| 109 | ? String(Math.max(1, flow.intervalSecs)) | |
| 110 | : "0"}" | |
| 108 | ${flow.flow === "device" ? html`data-stream="/settings/origins/flows/${flow.id}/stream"` : ""} | |
outdated
raw() skips escaping for an attribute. flow.id is forge-generated today, but the rule is every output escaped: build the attribute with the escaping helper used elsewhere on the page (as for pollAction) instead of raw(). |
||
| 111 | 109 | > |
| 112 | 110 | <span class="muted small" |
| 113 | 111 | >Waiting for you to authorize |
test/bring-oauth-return.test.ts +50
| @@ -88,4 +88,54 @@ describe("sign in from /bring and come back to it", () => { | ||
| 88 | 88 | expect(back.status).toBe(200); |
| 89 | 89 | expect(await back.text()).toContain(`value="${link}"`); |
| 90 | 90 | }); |
| 91 | ||
| 92 | test("the page listens on a stream; the forge polls the provider and says once when the flow settles", async () => { | |
| 93 | // A fresh account: alice is already connected to this host by the test above. | |
| 94 | const bobAuth = `Basic ${Buffer.from(`bob:${PASSWORD}`).toString("base64")}`; | |
| 95 | await fetch(`${servers.url}/api/v1/users`, { | |
| 96 | method: "POST", | |
| 97 | headers: { "content-type": "application/json" }, | |
| 98 | body: JSON.stringify({ username: "bob", email: "[email protected]", password: PASSWORD }), | |
| 99 | }); | |
| 100 | proveAll(app); | |
| 101 | const started = await fetch(`${servers.url}/settings/origins/oauth`, { | |
| 102 | method: "POST", | |
| 103 | redirect: "manual", | |
| 104 | headers: { authorization: bobAuth, "content-type": "application/x-www-form-urlencoded" }, | |
| 105 | body: new URLSearchParams({ host: fake.url, kind: "github" }), | |
| 106 | }); | |
| 107 | const flowPage = started.headers.get("location")!; | |
| 108 | const flowId = flowPage.split("/flows/")[1]!.split("?")[0]!; | |
outdated
The test only asserts the 'failed' outcome because the account was already connected, so the real path (pending then approved gives flow.settled with status done) is never exercised, and the spec 'a behaviour that would fail without it' is weakly met. Use a fresh account/host for this test, assert pending is not emitted early, then approved gives "status":"done". Also add the other-account request (404), which the comment above promises. |
||
| 109 | const shown = await (await fetch(`${servers.url}${flowPage}`, { headers: { authorization: bobAuth } })).text(); | |
| 110 | expect(shown).toContain(`data-stream="/settings/origins/flows/${flowId}/stream"`); | |
| 111 | ||
| 112 | // Signed out: no stream. | |
| 113 | expect((await fetch(`${servers.url}/settings/origins/flows/${flowId}/stream`, { redirect: "manual" })).status).toBeGreaterThanOrEqual(401); | |
| 114 | ||
| 115 | // Someone else's flow: not found, not a stream. | |
| 116 | const foreign = await fetch(`${servers.url}/settings/origins/flows/${flowId}/stream`, { headers: { authorization: auth } }); | |
| 117 | expect(foreign.status).toBe(404); | |
| 118 | expect(foreign.headers.get("content-type") ?? "").not.toContain("text/event-stream"); | |
| 119 | ||
| 120 | // Pending says nothing; once the provider approves, the stream says "done" and ends. | |
| 121 | oauth.approved = false; | |
| 122 | const res = await fetch(`${servers.url}/settings/origins/flows/${flowId}/stream`, { headers: { authorization: bobAuth }, signal: AbortSignal.timeout(20_000) }); | |
| 123 | expect(res.headers.get("content-type")).toContain("text/event-stream"); | |
| 124 | const reader = res.body!.getReader(); | |
| 125 | const decoder = new TextDecoder(); | |
| 126 | let body = ""; | |
| 127 | const drained = (async () => { | |
| 128 | for (;;) { | |
| 129 | const { done, value } = await reader.read(); | |
| 130 | if (done) return; | |
| 131 | body += decoder.decode(value); | |
| 132 | } | |
| 133 | })(); | |
| 134 | await Bun.sleep(300); | |
| 135 | expect(body).not.toContain("flow.settled"); | |
| 136 | oauth.approved = true; | |
| 137 | await drained; // the stream ends itself once the flow settles | |
| 138 | expect(body).toContain("event: flow.settled"); | |
| 139 | expect(body).toContain('"status":"done"'); | |
| 140 | }, 25_000); | |
| 91 | 141 | }); |
test/no-polling.test.ts +5 −15
| @@ -229,12 +229,6 @@ describe("no rendered page polls", () => { | ||
| 229 | 229 | * reads each page's markup and the scripts it loads. |
| 230 | 230 | */ |
| 231 | 231 | |
| 232 | /** A script that legitimately waits on something outside the page: marker in its text → why. */ | |
| 233 | const POLL_ALLOWED: Record<string, string> = { | |
| 234 | "form[data-poll]": | |
| 235 | "the device-code connect page (pollJs, src/web/client.ts) waits on a provider that sets the cadence (its `interval`), so there is no event of ours to stream; it posts the form the page already offers, stops after 200 tries, and reloads only once the flow leaves pending", | |
| 236 | }; | |
| 237 | ||
| 238 | 232 | /** The text of the balanced `(…)` or `{…}` whose opener is at `open`. Strings and comments are not understood: a bracket inside one skews it. */ |
| 239 | 233 | function balanced(src: string, open: number): string { |
| 240 | 234 | const pairs: Record<string, string> = { "(": ")", "{": "}" }; |
| @@ -282,9 +276,6 @@ export function findPolling(source: string): string[] { | ||
| 282 | 276 | return [...new Set(found)]; |
| 283 | 277 | } |
| 284 | 278 | |
| 285 | const pollReason = (source: string): string | undefined => | |
| 286 | Object.entries(POLL_ALLOWED).find(([marker]) => source.includes(marker))?.[1]; | |
| 287 | ||
| 288 | 279 | /** |
| 289 | 280 | * GET patterns the walk cannot fill from its seed, each with why: the walk is |
| 290 | 281 | * every pattern minus these, and the list is checked both ways, so a new page |
| @@ -322,6 +313,7 @@ const NOT_WALKED: Record<string, string> = { | ||
| 322 | 313 | "/settings/origins/:id/mirror/:job": ORIGIN, |
| 323 | 314 | "/settings/origins/:id/repos": ORIGIN, |
| 324 | 315 | "/settings/origins/flows/:id": ORIGIN, |
| 316 | "/settings/origins/flows/:id/stream": ORIGIN, | |
| 325 | 317 | "/settings/teams/:slug": "needs a team", |
| 326 | 318 | "/talk/:id": "needs a conversation of the signed-in person", |
| 327 | 319 | }; |
| @@ -380,8 +372,7 @@ describe("every page the router lists", () => { | ||
| 380 | 372 | for (const [name, source] of scripts) { |
| 381 | 373 | const found = findPolling(source); |
| 382 | 374 | if (!found.length) continue; |
| 383 | if (pollReason(source)) allowed.push(name); | |
| 384 | else offenders.push(`${name}: ${found.join("; ")}`); | |
| 375 | offenders.push(`${name}: ${found.join("; ")}`); | |
| 385 | 376 | } |
| 386 | 377 | return { htmlPages: htmlPages.sort(), offenders, allowed }; |
| 387 | 378 | } |
| @@ -441,9 +432,8 @@ describe("the polling detector", () => { | ||
| 441 | 432 | expect(findPolling(`setInterval(pick("fast"), 1000)`)).toEqual([]); |
| 442 | 433 | }); |
| 443 | 434 | |
| 444 | test("the device-code poll is flagged, and the one allowlist entry matches it and says why", () => { | |
| 445 | expect(findPolling(pollJs).length).toBeGreaterThan(0); | |
| 446 | expect(pollReason(pollJs)).toBeDefined(); | |
| 447 | for (const why of Object.values(POLL_ALLOWED)) expect(why.length).toBeGreaterThan(30); | |
| 435 | test("the device-code page waits on a stream, not a timer", () => { | |
| 436 | expect(findPolling(pollJs)).toEqual([]); | |
| 437 | expect(pollJs).toContain("EventSource"); | |
| 448 | 438 | }); |
| 449 | 439 | }); |
test/pages-budget.test.ts +1
| @@ -46,6 +46,7 @@ const MAX_UNFIXTURED = 144; | ||
| 46 | 46 | const NOT_PAGES = new Set([ |
| 47 | 47 | "/events", "/system/stream", "/settings/teams/stream", "/fleet/graph/live", |
| 48 | 48 | "/settings/traffic/scrapes/export", "/settings/backups/live", "/:owner/:name/stream", |
| 49 | "/settings/origins/flows/:id/stream", | |
| 49 | 50 | ]); |
| 50 | 51 | |
| 51 | 52 | /** Patterns with an id in them, and the path the fixture fills them with. */ |
test/security-floor-routes.test.ts +1
| @@ -130,6 +130,7 @@ const UNSEEDED: Record<string, string> = { | ||
| 130 | 130 | "/settings/origins/:id/mirror/:job": ORIGIN, |
| 131 | 131 | "/settings/origins/:id/repos": ORIGIN, |
| 132 | 132 | "/settings/origins/flows/:id": ORIGIN, |
| 133 | "/settings/origins/flows/:id/stream": ORIGIN, | |
| 133 | 134 | }; |
| 134 | 135 | |
| 135 | 136 | let dataDir = ""; |
pollOAuthConnect is awaited with no try/catch. If it throws (provider unreachable, DB error) step() rejects inside
void step(), nothing is emitted, the stream stays open and the timer chain dies: the page waits forever with no signal. Wrap it, emit flow.settled with status failed (or close), and stop. Also several tabs on one flow each poll the provider and each can consume the device code or trigger slow_down; keep one poller per flow (share it among subscribers), or at least cap concurrent streams per flow and per user, and bound the stream's total lifetime.