Public: anyone can read this; the crew can change it.
The signup form goes through signupAllowed, and a throttled form signup says when to try again #2368
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/2363-the-signup-form-goes-through-signupallow → main 1 commit fork 1 thread · 1 unresolved
2 files
changed +74
−1
src/web/pages/site.ts +21 −1
| @@ -460,7 +460,7 @@ export function signinAllowed(ctx: Ctx, username: string): number | undefined { | ||
| 460 | 460 | * Sign-ups, throttled the same way, by the socket's peer (`clientAddress`, |
| 461 | 461 | * which reads a forwarding header only from a proxy the operator trusts): |
| 462 | 462 | * a source may make a few accounts a minute and the forge as a whole more. |
| 463 | * Not yet asked by any signup door. A refused call is not counted. | |
| 463 | * Asked by the signup form only. A refused call is not counted. | |
| 464 | 464 | */ |
| 465 | 465 | const signupLimits = new WeakMap<Ctx["app"], RateLimiter>(); |
| 466 | 466 | |
| @@ -559,6 +559,26 @@ export async function signupSubmit(ctx: Ctx): Promise<Response> { | ||
| 559 | 559 | if (form instanceof Response) return form; |
| 560 | 560 | const username = field(form, "username"); |
| 561 | 561 | const email = field(form, "email"); |
| 562 | const wait = signupAllowed(ctx); | |
| 563 | if (wait !== undefined) { | |
| 564 | const response = page( | |
| 565 | { | |
| 566 | ctx, | |
| 567 | title: "Sign up", | |
| 568 | body: authForm(ctx, "signup", { | |
| 569 | values: { username, email }, | |
| 570 | errors: { | |
| 571 | username: `Too many sign-ups from here; try again in ${wait === 1 ? "a second" : `${wait} seconds`}.`, | |
| 572 | }, | |
| 573 | }), | |
| 574 | turnstile: siteKey(ctx) !== undefined, | |
| 575 | ...(ctx.app.config.passkeyOnlySignup === true ? { scriptJs: passkeyJs } : {}), | |
| 576 | }, | |
| 577 | 429, | |
| 578 | ); | |
| 579 | response.headers.set("retry-after", String(wait)); | |
| 580 | return response; | |
| 581 | } | |
| 562 | 582 | const created = await signUp( |
| 563 | 583 | ctx.app, |
| 564 | 584 | { |
test/signup-throttle.test.ts added +53
| @@ -0,0 +1,53 @@ | ||
| 1 | /** | |
| 2 | * The signup form asks signupAllowed: a sixth form post in a minute from one | |
| 3 | * source gets the signup page with the wait and a 429, and no account. | |
| 4 | */ | |
| 5 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 6 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 7 | import { tmpdir } from "node:os"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { createApp, type App } from "../src/app.ts"; | |
| 10 | import { getUserByName } from "../src/core/users.ts"; | |
| 11 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 12 | import { setLimit } from "../src/settings/limits.ts"; | |
| 13 | ||
| 14 | describe("the signup form is throttled", () => { | |
| 15 | let dataDir: string; | |
| 16 | let app: App; | |
| 17 | let servers: Servers; | |
| 18 | ||
| 19 | const post = (username: string) => | |
| 20 | fetch(`${servers.url}/signup`, { | |
| 21 | method: "POST", | |
| 22 | body: new URLSearchParams({ | |
| 23 | username, | |
| 24 | email: `${username}@forge.test`, | |
| 25 | password: "a-good-passphrase", | |
| 26 | }), | |
| 27 | redirect: "manual", | |
| 28 | }); | |
| 29 | ||
| 30 | beforeAll(async () => { | |
| 31 | dataDir = await mkdtemp(join(tmpdir(), "forge-signup-throttle-")); | |
| 32 | app = createApp({ dataDir, port: 0 }); | |
| 33 | servers = await serve(app); | |
| 34 | }); | |
| 35 | afterAll(async () => { | |
| 36 | await servers.stop(); | |
| 37 | await rm(dataDir, { recursive: true, force: true }); | |
| 38 | }); | |
| 39 | ||
| 40 | test("a sixth sign-up in a minute shows the wait and makes no account", async () => { | |
| 41 | for (let i = 1; i <= 5; i++) expect((await post(`pat-${i}x`)).status).toBe(303); | |
| 42 | const sixth = await post("pat-6x"); | |
| 43 | expect(sixth.status).toBe(429); | |
| 44 | expect(Number(sixth.headers.get("retry-after"))).toBeGreaterThan(0); | |
| 45 | expect(await sixth.text()).toMatch(/Too many sign-ups from here; try again in (a second|\d+ seconds)/); | |
| 46 | expect(getUserByName(app.db, "pat-6x")).toBeUndefined(); | |
| 47 | }); | |
| 48 | ||
| 49 | test("raising signup.per_source lets the next one through", async () => { | |
| 50 | setLimit(app.db, "signup.per_source", 50, getUserByName(app.db, "pat-1x")!.id); | |
| 51 | expect((await post("pat-7x")).status).toBe(303); | |
| 52 | }); | |
| 53 | }); |
new above
offline
The wait is attached to the username field. Put it in the form-level slot (like
human) so it is not read as a username error. Also check signup_closed before taking from the limiter.