Public: anyone can read this; the crew can change it.
The release smoke check reads a sign-in redirect as sign-in, 302 or 303 alike #2684
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/2673-the-release-smoke-check-reads-a-sign-in → main 1 commit fork
2 files
changed +63
−5
src/release/smoke.ts +23 −3
| @@ -55,10 +55,30 @@ export interface Options { | ||
| 55 | 55 | pollMs?: number; |
| 56 | 56 | /** The fetch to use; the global one unless a caller (the forge proving itself, a test) gives its own. */ |
| 57 | 57 | fetch?: typeof fetch; |
| 58 | /** Accept a redirect from `/board` as the sign-in it is, for a caller holding no token. */ | |
| 58 | /** Accept a redirect from `/board` to sign-in as the sign-in it is, for a caller holding no token. */ | |
| 59 | 59 | anonymousBoard?: boolean; |
| 60 | 60 | } |
| 61 | 61 | |
| 62 | /** The redirect statuses a sign-in answers with. */ | |
| 63 | const REDIRECTS = new Set([302, 303, 307]); | |
| 64 | ||
| 65 | /** | |
| 66 | * Is `res`, fetched from `url`, a redirect to sign in on the same host: the | |
| 67 | * forge's own `/signin` or the platform edge's `/_plat/login` (#2673)? | |
| 68 | */ | |
| 69 | export function isSignInRedirect(url: string, res: Response): boolean { | |
| 70 | if (!REDIRECTS.has(res.status)) return false; | |
| 71 | const location = res.headers.get("location"); | |
| 72 | if (!location) return false; | |
| 73 | let to: URL; | |
| 74 | try { | |
| 75 | to = new URL(location, url); | |
| 76 | } catch { | |
| 77 | return false; | |
| 78 | } | |
| 79 | return to.origin === new URL(url).origin && (to.pathname === "/signin" || to.pathname === "/_plat/login"); | |
| 80 | } | |
| 81 | ||
| 62 | 82 | /** Polls `/healthz` until it reports `version`; null when it does, else a failure. */ |
| 63 | 83 | async function waitForVersion(root: URL, version: string, waitMs: number, pollMs: number, fetch: typeof globalThis.fetch): Promise<Failure | null> { |
| 64 | 84 | const url = new URL("/healthz", root).href; |
| @@ -130,8 +150,8 @@ export async function smoke(base: string, token = "", options: Options = {}): Pr | ||
| 130 | 150 | } |
| 131 | 151 | } |
| 132 | 152 | const board = await get("/board", agent, auth); |
| 133 | const signIn = options.anonymousBoard && !token && board?.res.status === 302; | |
| 134 | if (board && board.res.status !== 200 && !signIn) fail(board.url, agent, board.res, "expected 200 (a 302 is sign-in)"); | |
| 153 | const signIn = options.anonymousBoard && !token && !!board && isSignInRedirect(board.url, board.res); | |
| 154 | if (board && board.res.status !== 200 && !signIn) fail(board.url, agent, board.res, "expected 200 (or a redirect to sign-in)"); | |
| 135 | 155 | const health = await get("/healthz", agent); |
| 136 | 156 | if (health && health.res.status !== 200) fail(health.url, agent, health.res, "expected 200"); |
| 137 | 157 | } |
test/smoke.test.ts +40 −2
| @@ -1,5 +1,5 @@ | ||
| 1 | 1 | import { afterEach, expect, test } from "bun:test"; |
| 2 | import { smoke, stylesheetHref } from "../scripts/smoke.ts"; | |
| 2 | import { isSignInRedirect, smoke, stylesheetHref } from "../scripts/smoke.ts"; | |
| 3 | 3 | |
| 4 | 4 | let server: ReturnType<typeof Bun.serve> | undefined; |
| 5 | 5 | afterEach(() => server?.stop(true)); |
| @@ -15,10 +15,12 @@ const GOOD_SW = `self.addEventListener('fetch', function (e) { | ||
| 15 | 15 | const BAD_SW = GOOD_SW.replace("catch(offline)", "catch(function () { return Response.error(); })"); |
| 16 | 16 | let sw: { body: string; type: string; status: number } = { body: GOOD_SW, type: "text/javascript", status: 200 }; |
| 17 | 17 | let version = () => "v1"; |
| 18 | let anonymousBoard = { status: 302, location: "/signin" }; | |
| 18 | 19 | |
| 19 | 20 | function stub(cssType: string, seen: string[] = []) { |
| 20 | 21 | sw = { body: GOOD_SW, type: "text/javascript", status: 200 }; |
| 21 | 22 | version = () => "v1"; |
| 23 | anonymousBoard = { status: 302, location: "/signin" }; | |
| 22 | 24 | server = Bun.serve({ |
| 23 | 25 | port: 0, |
| 24 | 26 | hostname: "127.0.0.1", |
| @@ -30,7 +32,7 @@ function stub(cssType: string, seen: string[] = []) { | ||
| 30 | 32 | if (path === "/board") |
| 31 | 33 | return req.headers.get("authorization") === "Bearer t" |
| 32 | 34 | ? new Response("board") |
| 33 | : new Response(null, { status: 302, headers: { location: "/signin" } }); | |
| 35 | : new Response(null, { status: anonymousBoard.status, headers: { location: anonymousBoard.location } }); | |
| 34 | 36 | if (path === "/healthz") return new Response("ok", { headers: { "x-forge-version": version() } }); |
| 35 | 37 | if (path === "/sw.js") return new Response(sw.body, { status: sw.status, headers: { "content-type": sw.type } }); |
| 36 | 38 | return new Response("no", { status: 404 }); |
| @@ -112,3 +114,39 @@ test("anonymousBoard: a 302 from /board passes with no token, and only then", as | ||
| 112 | 114 | expect((await smoke(base, "", {})).map((f) => f.status)).toEqual([302, 302]); |
| 113 | 115 | expect((await smoke(base, "wrong", { anonymousBoard: true })).map((f) => f.status)).toEqual([302, 302]); |
| 114 | 116 | }); |
| 117 | ||
| 118 | test("anonymousBoard: a 302, 303 or 307 to the forge's or the edge's sign-in is sign-in (#2673)", async () => { | |
| 119 | const base = stub("text/css"); | |
| 120 | for (const [status, location] of [ | |
| 121 | [302, "/signin?next=%2Fboard"], | |
| 122 | [303, "/signin?next=%2Fboard"], | |
| 123 | [303, `${base}/_plat/login?rd=%2Fboard`], | |
| 124 | [307, "/_plat/login?rd=%2Fboard"], | |
| 125 | ] as const) { | |
| 126 | anonymousBoard = { status, location }; | |
| 127 | expect(await smoke(base, "", { anonymousBoard: true })).toEqual([]); | |
| 128 | } | |
| 129 | }); | |
| 130 | ||
| 131 | test("anonymousBoard: a 303 anywhere but sign-in is still a failure", async () => { | |
| 132 | const base = stub("text/css"); | |
| 133 | for (const location of ["/elsewhere", "https://evil.example/signin", "/signin/parent"]) { | |
| 134 | anonymousBoard = { status: 303, location }; | |
| 135 | const failures = await smoke(base, "", { anonymousBoard: true }); | |
| 136 | expect(failures.map((f) => f.status)).toEqual([303, 303]); | |
| 137 | expect(failures[0]!.why).toBe("expected 200 (or a redirect to sign-in)"); | |
| 138 | } | |
| 139 | anonymousBoard = { status: 301, location: "/signin" }; | |
| 140 | expect((await smoke(base, "", { anonymousBoard: true })).map((f) => f.status)).toEqual([301, 301]); | |
| 141 | }); | |
| 142 | ||
| 143 | test("isSignInRedirect wants a redirect status and a same-origin sign-in Location", () => { | |
| 144 | const at = "https://forge.example/board"; | |
| 145 | const res = (status: number, location?: string) => | |
| 146 | new Response(null, { status, headers: location === undefined ? {} : { location } }); | |
| 147 | expect(isSignInRedirect(at, res(303, "https://forge.example/_plat/login?rd=%2Fboard"))).toBe(true); | |
| 148 | expect(isSignInRedirect(at, res(302, "/signin"))).toBe(true); | |
| 149 | expect(isSignInRedirect(at, res(303))).toBe(false); | |
| 150 | expect(isSignInRedirect(at, res(303, "https://other.example/_plat/login"))).toBe(false); | |
| 151 | expect(isSignInRedirect(at, res(200, "/signin"))).toBe(false); | |
| 152 | }); |
new above
offline