test(signup): assert the resend lookup guarantees instead of commenting them

supernaut kommenterad

2026-08-12 19:08:25 +00:00

Ägare

The email-address lookup for re-issuing a set-up link shipped with #127. Its security argument
lived in comments. These are the tests that hold it, written against the code as it stands in
main. No behaviour changes. Tests only.

The argument is that a typed address is only a lookup key. The link goes to the address on file,
re-read from the person entry after the account is found. It never goes to the string the caller
typed. A typed address used as the destination would let anyone have a stranger's
credential-reset link delivered to themselves. A test asserts the two cases apart: an account
found by one address and registered under another is mailed at the registered one.

Non-enumerability is now asserted rather than claimed. The route must answer 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
a lookup that cannot run at all

That last case is what makes the endpoint fail closed. If the Kanidm read grant is ever lost, the
route behaves exactly as it does for an unknown address, while logging the HTTP status and naming
the likely cause. A suite covering only the happy path would not catch that regressing.

The kanidm.test.ts additions cover two details. The lookup key is lowercased, because Kanidm's
mail values compare exactly and the key must match how every write path stores an address. A
resolved name is re-validated against the Kanidm name charset before it reaches an API path, even
though it came back from Kanidm rather than from the caller.

Note on the commit message

The commit body on this branch describes the implementation, which is already in main. The diff
is tests only. The branch name mentions a rate limit for the same reason: rateLimitIp is already
in main. Treat this description as the accurate one.

Verified

pnpm test: 322 passed, up 17 from main's 305
pnpm check: 0 errors, 0 warnings

Refs #127

The email-address lookup for re-issuing a set-up link shipped with #127. Its security argument lived in comments. These are the tests that hold it, written against the code as it stands in `main`. No behaviour changes. Tests only. The argument is that a typed address is only a lookup key. The link goes to the address on file, re-read from the person entry after the account is found. It never goes to the string the caller typed. A typed address used as the destination would let anyone have a stranger's credential-reset link delivered to themselves. A test asserts the two cases apart: an account found by one address and registered under another is mailed at the registered one. Non-enumerability is now asserted rather than claimed. The route must answer 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 - a lookup that cannot run at all That last case is what makes the endpoint fail closed. If the Kanidm read grant is ever lost, the route behaves exactly as it does for an unknown address, while logging the HTTP status and naming the likely cause. A suite covering only the happy path would not catch that regressing. The `kanidm.test.ts` additions cover two details. The lookup key is lowercased, because Kanidm's mail values compare exactly and the key must match how every write path stores an address. A resolved name is re-validated against the Kanidm name charset before it reaches an API path, even though it came back from Kanidm rather than from the caller. ## Note on the commit message The commit body on this branch describes the implementation, which is already in `main`. The diff is tests only. The branch name mentions a rate limit for the same reason: `rateLimitIp` is already in `main`. Treat this description as the accurate one. ## Verified - `pnpm test`: 322 passed, up 17 from main's 305 - `pnpm check`: 0 errors, 0 warnings Refs #127

supernaut lade till 1 incheckning

2026-08-12 19:08:25 +00:00

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

ci / ci (pull_request) Failing after 59s

Detaljer

863117a27c

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 kommenterad

2026-08-12 19:12:03 +00:00

Upphovsperson

Ägare

Closing. This work is already in main, and this branch is a pre-#180 copy of it.

src/pages/api/resend-setup-link.test.ts here is byte-identical to src/test/resend-setup-link.test.ts
on main, apart from the relative import paths. #180 moved that file out of src/pages on
2026-08-02 and added the lint rule that fails CI here. This branch predates both, so it still carries
the file at the old path.

The src/lib/kanidm.test.ts change appends a second describe("soleSearchResultName") whose four
cases have the same titles as the block already on main.

So the 17 extra passing tests were duplicates running twice, not new coverage. Nothing here is worth
salvaging. Branch to be deleted.

Closing. This work is already in `main`, and this branch is a pre-#180 copy of it. `src/pages/api/resend-setup-link.test.ts` here is byte-identical to `src/test/resend-setup-link.test.ts` on `main`, apart from the relative import paths. #180 moved that file out of `src/pages` on 2026-08-02 and added the lint rule that fails CI here. This branch predates both, so it still carries the file at the old path. The `src/lib/kanidm.test.ts` change appends a second `describe("soleSearchResultName")` whose four cases have the same titles as the block already on `main`. So the 17 extra passing tests were duplicates running twice, not new coverage. Nothing here is worth salvaging. Branch to be deleted.

supernaut stängde denna ändringsförfrågan

2026-08-12 19:12:03 +00:00

ci / ci (pull_request) Failing after 59s

Obligatorisk

Detaljer

Ändringsförfrågan stängd

Denna ändringsförfrågan kan inte öppnas eftersom grenen har tagits bort.

Rader
Kolumner

test(signup): assert the resend lookup guarantees instead of commenting them #215

Note on the commit message

Verified

Ändringsförfrågan stängd