trevato/forge

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

More
Forks1g fMapg aRuns13g rEventsFlowsAPI

The device-code connect page polls on a timer: move it to a stream or a live-region swap #10971

Parked scoutcrew opened this
ConversationFiles
Closed without merging

scout/forge:scout/10964-the-device-code-connect-page-polls-on-a → main 4 commits fork

  • Closed without merging. Nothing was applied to main.

Review

Sign in to review this change.
9 files changed +131 −41
src/http/flow_stream.ts added +71
@@ -0,0 +1,71 @@
1 /**
2 * GET /settings/origins/flows/:id/stream — one device-code connect, held.
3 *
4 * The provider sets the cadence of its poll (the grant's `interval`, raised by
5 * `slow_down`), and it sends nothing to us. So the forge keeps that poll: this
6 * stream asks the provider on the flow's interval, server side, and the page
7 * hears one event, `flow`, when the flow leaves pending. The browser never
8 * loops; the stream ends with the event.
9 */
10
11 import { pollOAuthConnect } from "../actions/origins.ts";
12 import { getFlowFor } from "../origins/oauth.ts";
13 import { eventStream } from "./sse.ts";
14 import type { Ctx } from "./context.ts";
15 import { accountOnly } from "../web/pages/origins.ts";
16
17 /** Wait `ms`, or less when `signal` aborts. Resolves either way. */
18 function sleep(ms: number, signal: AbortSignal): Promise<void> {
19 return new Promise((resolve) => {
20 if (signal.aborted) return resolve();
21 const done = (): void => {
22 clearTimeout(timer);
23 signal.removeEventListener("abort", done);
24 resolve();
25 };
26 const timer = setTimeout(done, ms);
27 signal.addEventListener("abort", done);
28 });
29 }
30
31 export function originFlowStream(ctx: Ctx): Response {
32 const denied = accountOnly(ctx);
33 if (denied) return denied;
34 const user = ctx.auth.user!;
35 const flowId = ctx.params.id ?? "";
36 const first = getFlowFor(ctx.app.db, user.id, flowId);
37 if (!first) return new Response("not found", { status: 404 });
38 const { app } = ctx;
39
40 return eventStream(app, ctx.request, (emit, close) => {
41 const gone = new AbortController();
42 void (async () => {
43 let flow = first;
44 try {
45 while (!gone.signal.aborted) {
46 if (flow.status !== "pending") break;
47 await sleep(Math.max(1, flow.intervalSecs) * 1000, gone.signal);
48 if (gone.signal.aborted) return;
49 const polled = await pollOAuthConnect(app, user, flowId);
50 if (gone.signal.aborted) return;
51 if (!polled.ok) {
52 emit("flow", { status: "failed" });
53 break;
54 }
55 if (polled.value.status !== "pending") {
56 emit("flow", { status: polled.value.status });
57 break;
58 }
59 // `slow_down` lengthens the interval: read it again each round.
60 flow = getFlowFor(app.db, user.id, flowId) ?? flow;
61 }
62 if (!gone.signal.aborted && flow.status !== "pending") emit("flow", { status: flow.status });
63 } catch {
64 // A poll that threw ends the stream; the page's own poll form still works.
65 if (!gone.signal.aborted) emit("flow", { status: "failed" });
66 }
67 close();
68 })();
69 return () => gone.abort();
70 });
71 }
src/http/settings_routes.ts +2
@@ -128,6 +128,7 @@ import {
128128 originTrustedHosts,
129129 } from "../web/pages/origins.ts";
130130 import { originArchivedPage, originArchivedRestore } from "../web/pages/origin-archived.ts";
131 import { originFlowStream } from "./flow_stream.ts";
131132 import { originFlowPage, originFlowPoll } from "../web/pages/origin-flow.ts";
132133 import { originMirrorSubmit, originReposPage } from "../web/pages/origin-repos.ts";
133134
@@ -309,6 +310,7 @@ export function settingsRoutes(router: Router<Handler>, knownPatterns: () => rea
309310 router.add("POST", "/settings/origins/apps/:id/delete", originAppDelete);
310311 router.add("GET", "/settings/origins/flows/:id", originFlowPage);
311312 router.add("POST", "/settings/origins/flows/:id/poll", originFlowPoll);
313 router.add("GET", "/settings/origins/flows/:id/stream", originFlowStream);
312314 router.add("GET", "/settings/origins/:id/repos", originReposPage);
313315 router.add("GET", "/settings/origins/:id/archived", originArchivedPage);
314316 router.add("POST", "/settings/origins/:id/archived", originArchivedRestore);
src/web/client.ts +11 −23
@@ -62,34 +62,22 @@ const modalJs = /* js */ `
6262 `;
6363
6464 /**
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 * (`form[data-stream]`) and reload when the flow stops being pending. The
67 * forge holds the provider's poll server-side, so this script has no timer
68 * and asks nothing; without it the "check again" form still works.
7069 *
7170 * Shipped per page (`PageOptions.scriptJs`) rather than in the shell: a page
7271 * with no connect in flight should carry none of it.
7372 */
74 export const pollJs = /* js */ `
73 export const flowStreamJs = /* js */ `
7574 (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 url = form.getAttribute('data-stream');
78 if (!url) return;
79 var es = new EventSource(url);
80 es.addEventListener('flow', function () { es.close(); location.reload(); });
9381 })();
9482 `;
9583
src/web/pages/origin-flow.ts +4 −6
@@ -16,7 +16,7 @@ import { qrEncode, qrSvg } from "../../design/qr.ts";
1616 import { readForm } from "../form.ts";
1717 import { html, raw, type Html } from "../html.ts";
1818 import { page, redirect } from "../layout.ts";
19 import { pollJs } from "../client.ts";
19 import { flowStreamJs } from "../client.ts";
2020 import { btn, icon, row } from "../ui.ts";
2121 import {
2222 accountOnly,
@@ -36,7 +36,7 @@ import {
3636 * The device grant shows the code and where to type it; the code flow shows
3737 * one link to the instance. Both work with no JavaScript: the page carries a
3838 * "check again" form, and reloading it re-polls. With script, the same form
39 * is submitted for you every couple of seconds.
39 * hears the forge's own stream and reloads when the connect ends.
4040 */
4141 export function originFlowPage(ctx: Ctx): Response {
4242 const denied = accountOnly(ctx);
@@ -105,9 +105,7 @@ export function originFlowPage(ctx: Ctx): Response {
105105 class="bar mt"
106106 method="post"
107107 action="${pollAction}"
108 data-poll="${flow.flow === "device"
109 ? String(Math.max(1, flow.intervalSecs))
110 : "0"}"
108 data-stream="${flow.flow === "device" ? `/settings/origins/flows/${flow.id}/stream` : ""}"
111109 >
112110 <span class="muted small"
113111 >Waiting for you to authorize
@@ -136,7 +134,7 @@ export function originFlowPage(ctx: Ctx): Response {
136134 No JavaScript? Refresh this page to check again — the reload re-polls.
137135 </p>
138136 </div>`,
139 scriptJs: pollJs,
137 scriptJs: flowStreamJs,
140138 });
141139 }
142140
test/no-polling.test.ts +7 −9
@@ -16,7 +16,7 @@ import { createIssue } from "../src/core/issues.ts";
1616 import { createRepo } from "../src/core/repos.ts";
1717 import { createSession } from "../src/core/sessions.ts";
1818 import { buildRouter, serve, type Servers } from "../src/http/server.ts";
19 import { pollJs } from "../src/web/client.ts";
19 import { flowStreamJs } from "../src/web/client.ts";
2020 import { adminCookie, seedAdmin } from "./helpers/admin.ts";
2121 import { quietLogs } from "./helpers/globals.ts";
2222 import { pool } from "./helpers/sweep-clock.ts";
@@ -230,10 +230,7 @@ describe("no rendered page polls", () => {
230230 */
231231
232232 /** 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 };
233 const POLL_ALLOWED: Record<string, string> = {};
237234
238235 /** The text of the balanced `(…)` or `{…}` whose opener is at `open`. Strings and comments are not understood: a bracket inside one skews it. */
239236 function balanced(src: string, open: number): string {
@@ -322,6 +319,7 @@ const NOT_WALKED: Record<string, string> = {
322319 "/settings/origins/:id/mirror/:job": ORIGIN,
323320 "/settings/origins/:id/repos": ORIGIN,
324321 "/settings/origins/flows/:id": ORIGIN,
322 "/settings/origins/flows/:id/stream": ORIGIN + "; a stream, not a page",
325323 "/settings/teams/:slug": "needs a team",
326324 "/talk/:id": "needs a conversation of the signed-in person",
327325 };
@@ -441,9 +439,9 @@ describe("the polling detector", () => {
441439 expect(findPolling(`setInterval(pick("fast"), 1000)`)).toEqual([]);
442440 });
443441
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);
442 test("the device-code page holds no timer: it listens on a stream and the allowlist is empty", () => {
443 expect(findPolling(flowStreamJs)).toEqual([]);
444 expect(flowStreamJs).toContain("EventSource");
445 expect(Object.keys(POLL_ALLOWED)).toEqual([]);
448446 });
449447 });
test/pages-budget.test.ts +1 −1
@@ -40,7 +40,7 @@ const HEAVIER: Record<string, { statements?: number; git?: number }> = {
4040 };
4141
4242 /** Patterns with no fixture today: lower this as fixtures are added, never raise it silently. */
43 const MAX_UNFIXTURED = 144;
43 const MAX_UNFIXTURED = 145;
4444
4545 /** Streams and live fragments: no page. */
4646 const NOT_PAGES = new Set([
test/security-floor-routes.test.ts +1
@@ -130,6 +130,7 @@ const UNSEEDED: Record<string, string> = {
130130 "/settings/origins/:id/mirror/:job": ORIGIN,
131131 "/settings/origins/:id/repos": ORIGIN,
132132 "/settings/origins/flows/:id": ORIGIN,
133 "/settings/origins/flows/:id/stream": ORIGIN,
133134 };
134135
135136 let dataDir = "";
test/web-code.test.ts +2 −2
@@ -37,7 +37,7 @@ import {
3737 codeJs,
3838 editorJs,
3939 mirrorJs,
40 pollJs,
40 flowStreamJs,
4141 shellJs,
4242 } from "../src/web/client.ts";
4343 import { liveClientJs } from "../src/web/live.ts";
@@ -548,7 +548,7 @@ describe("the code browser on the web", () => {
548548 ["codeJs", codeJs],
549549 ["editorJs", editorJs],
550550 ["mirrorJs", mirrorJs],
551 ["pollJs", pollJs],
551 ["flowStreamJs", flowStreamJs],
552552 ["liveClientJs", liveClientJs],
553553 ] as const) {
554554 expect(() => new Function(source), `${name} must parse`).not.toThrow();
test/web-origins.test.ts +32
@@ -694,6 +694,38 @@ describe("origins on the web", () => {
694694 expect(landing).toContain("OAuth");
695695 });
696696
697 test("the device page holds the provider's poll: a stream says when the flow ends, and the page has no timer", async () => {
698 process.env.FORGE_GITHUB_CLIENT_ID = CLIENT_ID;
699 const started = await asUser(bob, "/settings/origins/oauth", {
700 form: { kind: "github", host: github.url },
701 });
702 const where = started.headers.get("location") ?? "";
703 expect(where).toStartWith("/settings/origins/flows/");
704 const page = await (await asUser(bob, where)).text();
705 expect(page).toContain(`data-stream="${where}/stream"`);
706 expect(page).not.toContain("data-poll");
707
708 // Someone else's stream is a 404, and so is a flow that does not exist.
709 expect((await asUser(alice, `${where}/stream`)).status).toBe(404);
710 expect((await asUser(bob, "/settings/origins/flows/nope/stream")).status).toBe(404);
711
712 ghOauth.approved = true;
713 const res = await asUser(bob, `${where}/stream`);
714 expect(res.status).toBe(200);
715 expect(res.headers.get("content-type")).toContain("text/event-stream");
716 const reader = res.body!.getReader();
717 let text = "";
718 const deadline = Date.now() + 20_000;
719 while (!text.includes("event: flow") && Date.now() < deadline) {
720 const { value, done } = await reader.read();
721 if (done) break;
722 text += new TextDecoder().decode(value);
723 }
724 await reader.cancel();
725 expect(text).toContain("event: flow");
726 expect(text).toContain('"status":"done"');
727 }, 30_000);
728
697729 /**
698730 * The flow page has to work for someone whose browser is a phone across
699731 * the room. Everything that gets it there is asserted here: the QR, the

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs