feat(signup): find an account by email when re-issuing, and re-key the rate limiter #173

Sammanfogat

supernaut sammanfogade 3 incheckningar från feat/resend-lookup-and-rate-limit in i main

2026-08-02 20:18:47 +00:00

supernaut kommenterad

2026-08-02 15:53:24 +00:00

Ägare

Closes #127, closes #142.

#127 — the re-issue form now accepts an email address as well as a username.

The security constraint, because getting it wrong is an account-takeover primitive: the typed
address is a lookup key only. The link is still sent to the address on file, never to one the
caller supplies. Accepting a typed address as the destination would let anyone have a stranger's
credential-reset link delivered to themselves.

Non-enumerability is verified against the real route handler (only captcha and mailer stubbed),
comparing status, every header sorted, and body across: username hit and miss, address hit and miss,
username versus address on the same account, an account with no address on file, a mail-provider
refusal, and the lookup being unavailable. All identical — 303, Location: …?status=sent, empty
body. Also tested: an account found by one address but registered under another is mailed at the
registered one.

#142 — the in-process limiter is re-keyed and re-sized against the edge.

It keyed on the full address with no IPv6 prefixing, so one user with a /64 could rotate freely while
everyone behind a shared IPv4 egress shared a single budget. Now grouped to /56, matching the edge
zone. IPv4-mapped IPv6 (::ffff:a.b.c.d, how Node reports IPv4 peers) is unwrapped first — masking
those would collapse every IPv4 client into one bucket. There is a test for that.

Budget	Before	After	Why
`signup`	5/min	10/min	the key widened from one address to a whole /56, so 5 silently became several times stricter; 10 still sits inside the edge's effective ceiling, so the inner ring engages first
`resend`	3/min	3/min	sends mail to a third party; shared-egress cost accepted explicitly
`captcha`	30/min	30/min	the edge matches redeem but not challenge, so this is the only limit on challenge minting
`email-change`	3/hr per IP	3/hr per account	an IP key was worse both ways — one bad account could rotate addresses for a fresh budget, and a shared campus shared 3/hr

All budgets now sit next to a description of the edge zone they pair with.

Verification

pnpm lint, pnpm check, pnpm lang-check, pnpm test — 252 passing. Both changes written
test-first.

Two things an operator should close

Timing is not equalised. The response is byte-identical but the work is not constant-time — an
existing account costs more round-trips than a miss. That channel pre-dates this change; no
artificial floor was added, since it delays every legitimate user and is a judgement call.
The authorisation for the lookup was argued from the running server's API document and upstream
source rather than executed, and the code fails closed. One live call with the portal's token
before merge would confirm it.

Closes #127, closes #142. **#127 — the re-issue form now accepts an email address as well as a username.** The security constraint, because getting it wrong is an account-takeover primitive: the typed address is a **lookup key only**. The link is still sent to the address on file, never to one the caller supplies. Accepting a typed address as the destination would let anyone have a stranger's credential-reset link delivered to themselves. Non-enumerability is verified against the **real route handler** (only captcha and mailer stubbed), comparing status, every header sorted, and body across: username hit and miss, address hit and miss, username versus address on the same account, an account with no address on file, a mail-provider refusal, and the lookup being unavailable. All identical — `303`, `Location: …?status=sent`, empty body. Also tested: an account found *by* one address but registered *under* another is mailed at the registered one. **#142 — the in-process limiter is re-keyed and re-sized against the edge.** It keyed on the full address with no IPv6 prefixing, so one user with a /64 could rotate freely while everyone behind a shared IPv4 egress shared a single budget. Now grouped to /56, matching the edge zone. IPv4-mapped IPv6 (`::ffff:a.b.c.d`, how Node reports IPv4 peers) is unwrapped first — masking those would collapse every IPv4 client into one bucket. There is a test for that. | Budget | Before | After | Why | | --- | --- | --- | --- | | `signup` | 5/min | **10/min** | the key widened from one address to a whole /56, so 5 silently became several times stricter; 10 still sits inside the edge's effective ceiling, so the inner ring engages first | | `resend` | 3/min | 3/min | sends mail to a third party; shared-egress cost accepted explicitly | | `captcha` | 30/min | 30/min | the edge matches redeem but not challenge, so this is the only limit on challenge minting | | `email-change` | 3/hr per IP | **3/hr per account** | an IP key was worse both ways — one bad account could rotate addresses for a fresh budget, and a shared campus shared 3/hr | All budgets now sit next to a description of the edge zone they pair with. ### Verification `pnpm lint`, `pnpm check`, `pnpm lang-check`, `pnpm test` — **252 passing**. Both changes written test-first. ### Two things an operator should close - **Timing is not equalised.** The response is byte-identical but the work is not constant-time — an existing account costs more round-trips than a miss. That channel pre-dates this change; no artificial floor was added, since it delays every legitimate user and is a judgement call. - The authorisation for the lookup was argued from the running server's API document and upstream source rather than executed, and the code fails closed. One live call with the portal's token before merge would confirm it.

supernaut lade till 2 incheckningar

2026-08-02 15:53:24 +00:00

fix(security): group the rate-limit key by IPv6 /56, and size it against the edge ed427f9edb

The in-process limiter and the Caddy `signup` zone are an inner and an outer ring
over the same endpoints, so they only work when they are keyed and sized together.
They were neither, and both halves failed silently.

Keying. This file counted the FULL client address while the edge groups IPv6 by
/56. Every subscriber is delegated at least a /64, so a caller on IPv6 could put
each request on a fresh source address and never spend an app budget at all, while
the edge went on counting them as one client. `ipBucket` now masks IPv6 to /56 and
leaves IPv4 exact, which is precisely what the edge does — the two layers now agree
about who a client is. IPv4-mapped addresses (`::ffff:a.b.c.d`, how Node reports an
IPv4 peer) are unwrapped first: masking those to /56 would collapse every IPv4
client onto one bucket, which is an outage rather than a limit.

Sizing. Broadening the key from one address to one site is a tightening for
everyone who shares an egress, so the numbers had to move with it. Sign-up goes
5 -> 10 a minute: a bucket is now a whole office NAT or a whole IPv6 site, and a
429 at the moment of conversion reads as a broken site rather than as a rate limit.
10 stays strictly inside the ~15 sign-ups a minute the edge zone already permits at
30 events, so the inner ring still engages first. Resend stays at 3 — it sends mail
to a third party, a resend is a rare deliberate act, and a false 429 costs a
sixty-second wait. Captcha stays at 30, and is the ONLY limit on challenge minting,
which the edge zone does not match at all.

The IPv4 shared-egress consequence is now a recorded decision rather than an
accident, and where a better key exists it is used: /api/email-change is behind a
session, so it keys on the account. An address key was worse there in both
directions — one malicious account could rotate addresses for a fresh budget, while
a shared campus had to share three requests an hour.

Every budget now lives in src/lib/rate-limit.ts next to a description of the edge
zone it pairs with, because nothing in this file previously mentioned the edge at
all, which is how the two came to be inverted. `rateLimitIp` is the only path that
applies the key grouping, so an endpoint added later cannot quietly reintroduce
per-address keying.

Closes #142

feat(signup): find the account by email address when re-issuing a set-up link

ci / ci (pull_request) Successful in 1m14s

Detaljer

31d647a409

The resend form asked for a username — the one thing a person who never received
their set-up email is least likely to remember, since the username is written down
in the email that never arrived.

An address is now accepted as well, but ONLY AS A LOOKUP KEY. The link still goes
to the address on file, re-read from the person entry after the account is found,
never to the string the caller typed. That distinction is the whole security
argument, and it is why the form was username-only to begin with: a typed address
used as the DESTINATION would let anyone have a stranger's credential-reset link
delivered to themselves. Used only to find the account, it grants a caller nothing
they did not already have, because the message still goes where the account already
points. A test asserts the two cases apart — an account found by one address and
registered under another is mailed at the registered one.

The API question the issue asked to settle, settled: /v1/person/_search/{id} is a
substring match on `name` alone (f_sub(Attribute::Name, ...) in Kanidm 1.10.4's v1
handlers), so it can never match an address, and GET /v1/person cannot be filtered
at all. POST /v1/raw/search is the only endpoint that can. Despite its "be warned
this can be dangerous" summary it is a READ, served from the read query server, and
the filter is built here — the caller's input appears in it only as the string value
of a single `eq` term, so an anonymous visitor cannot reshape the query. No schema
change, no stored derived personal data, and no new privilege: Kanidm's built-in
idm_acp_people_pii_read names idm_people_admins as a receiver with `mail` among its
searchable attributes, which is the same grant that already lets this service
account read `mail` off GET /v1/person/{name}.

Non-enumerability is preserved and now has tests that assert it rather than
comments that assert it. The route answers identically for: a username that exists
and one that does not, an address that exists and one that does not, a username and
the address on the same account, an account with no address on file, a refused
mail send, and a lookup that cannot run at all. That last one is what makes this
fail closed — if the read grant is ever lost the endpoint behaves exactly as it does
for an unknown address, while logging the HTTP status and naming the likely cause.

Kanidm's mail values compare exactly, so the lookup key is lowercased to match how
every write path stores an address. A resolved name is re-validated against the
Kanidm name charset before it goes anywhere near an API path, even though it came
back from Kanidm rather than from the caller.

Closes #127

supernaut lade till 1 incheckning

2026-08-02 20:10:19 +00:00

merge main into feat/resend-lookup-and-rate-limit

ci / ci (pull_request) Has started running

Detaljer

58d144affc

#172 landed identity-rules.ts as the single source of truth for username and
email validation, in the same files this branch reworks for the email lookup
and the re-keyed limiter. Six conflicts; the interesting ones were not
either-or.

resend.ts — this branch had its own KANIDM_NAME_RE and EMAIL_RE, which is
exactly the duplication #172 exists to remove. Dropped both in favour of
isKanidmName and normaliseEmail. That also fixes a latent bug: the local regex
accepted uppercase names, which Kanidm would never have matched. main's own
hunk referenced a  variable this signature no longer has, so its
reasoning was carried into the classifier rather than its code.

signup.ts — the one place where keeping both sides would have been wrong. Ours
held the validation regexes #172 deleted, theirs held the rate constants this
branch deleted. Both are dead; neither survives.

signup-form.astro — the field takes a username OR an address now, so it keeps
this branch's identifier name and 254 cap and no pattern, plus main's
data-lowercase, which is generic and correct for both kinds.

Three tests failed after resolving, and the tests were what was wrong:
 now folds to  (Kanidm matches lowercase, so they are one
account); the length bound is Kanidm's 64 rather than Forgejo's 40, because
addressing an existing account is not the sign-up intersection; and
 is now ACCEPTED, since the WHATWG rule Kanidm uses does not
require a dot — refusing it would refuse an address Kanidm would have stored.
Each is documented at the assertion.

293 tests pass, including all 13 non-enumerability assertions. lint, check
(0 errors) and lang-check clean.