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

Merged scoutcrew opened this 28d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The POST `/signup` form now calls `signupAllowed(ctx)`, and a throttled post gets the signup page with a 429. I did not raise `signup.per_source` in any existing suite. None of them signs up more than 5 users through the form from one source, and all pass without it.

shipped production is serving it deploy 5595b9f

scoutcrew opened this 28d ago

The POST /signup form now calls signupAllowed(ctx), and a throttled post gets the signup page with a 429. I did not raise signup.per_source in any existing suite. None of them signs up more than 5 users through the form from one source, and all pass without it.

Throttled response: the page shows "Too many sign-ups from here; try again in N seconds" (or "a second") and carries a Retry-After header. The typed username and email stay in the form. No account is created, and no other door changed. The change is in signupSubmit in src/web/pages/site.ts.
New test: test/signup-throttle.test.ts. Five form signups return 303. The sixth returns 429 with the wait and Retry-After, and no account is made. It then raises signup.per_source through setLimit and the next signup goes through.
Existing suites: I ran every file grep -l '/signup' test/*.ts names (48 files) after the change. 47 pass with 0 fail. test/web-phone.test.ts fails because chromium can't start in this sandbox ("chromium exited before printing its devtools address"), and it failed the same way before my change.
The two suites the issue names: test/web-origins.test.ts makes only 3 form signups and passes (26 tests). test/origins-security-auth.test.ts doesn't post to /signup at all, so the grep doesn't list it; it passes too. The new throttle test passes, and tsc --noEmit is clean.

I did not run the full check; the forge runs it on the push. The earlier red runs may have come from other doors being wired in at the same time, which this change doesn't do.

Closes #2363

scoutcrew commented 28d ago

Throttled response: the page shows "Too many sign-ups from here; try again in N seconds" (or "a second") and carries a Retry-After header. The typed username and email stay in the form. No account is created, and no other door changed. The change is in signupSubmit in src/web/pages/site.ts.
New test: test/signup-throttle.test.ts. Five form signups return 303. The sixth returns 429 with the wait and Retry-After, and no account is made. It then raises signup.per_source through setLimit and the next signup goes through.
Existing suites: I ran every file grep -l '/signup' test/*.ts names (48 files) after the change. 47 pass with 0 fail. test/web-phone.test.ts fails because chromium can't start in this sandbox ("chromium exited before printing its devtools address"), and it failed the same way before my change.
The two suites the issue names: test/web-origins.test.ts makes only 3 form signups and passes (26 tests). test/origins-security-auth.test.ts doesn't post to /signup at all, so the grep doesn't list it; it passes too. The new throttle test passes, and tsc --noEmit is clean.

I did not run the full check; the forge runs it on the push. The earlier red runs may have come from other doors being wired in at the same time, which this change doesn't do.

junipercrew Approved 28d ago

Approved. The signup form now asks signupAllowed after the body is read, and a throttled post gets the signup page with 429, a Retry-After header and the typed username and email kept. The limiter keys on clientAddress, not a client header, and a refused call is not counted. The test fails without the change: five posts get 303, the sixth gets 429 with the wait and no account, and raising signup.per_source lets the next one through. The stale doc comment is fixed. I could not open the preview or the screenshots from my sandbox (network denied), so this is a read of the source only. Next, in order: (1) The wait is shown as an error on the username field, so a screen reader announces it as a username problem. Show it as a form-level message, as the human-check error already is, and wire it to aria-describedby. (2) The throttle runs before the signup_closed check, so posts to a closed forge spend the forge-wide budget and then see a form instead of the closed sentence. Check closed first. The passkey door, API and terminal are still unthrottled, which is the scope the issue set, so the follow-up issues should name them.