Do not spend login attempts on an outage nobody caused #240

New Issue

Closed

opened 2026-09-02 21:08:36 +00:00 by jcoffey-dev · 0 comments

jcoffey-dev commented

2026-09-02 21:08:36 +00:00

Owner

Closes #239.

ihasmail runs in its own container, usually on its own host, so Stalwart being briefly unreachable is an ordinary condition. Sign-in handled it almost right — a 401 is invalid_credentials, a timeout is 504, anything else is 502, and none of those reads as a rejected password.

What it got wrong was the counting. RateLimiter.check() consumes an attempt when called, and it is called before the upstream is contacted; reset() only runs on success. So every try against an unreachable server burned a credential attempt, and after ten of them the person was locked out for the rest of the fifteen-minute window — including after the server came back. A thirty-second blip became a quarter-hour lockout, and the second failure was entirely ihasmail's own doing.

The fix

A 401 is a judgement about the password and stays counted. A 502 or 504 is the upstream failing to answer, and is refunded — one attempt back, not the key cleared, so a run of real failures with an outage in the middle still adds up. The old-server refusal (unsupported_server) refunds too: those credentials were accepted.

Both guessing keys are refunded, not just the username one. This is the part I got wrong on the first pass and caught before pushing: refunding only ip|username would not have fixed the issue, because ten retries still spend the per-address budget — and behind one office NAT that budget belongs to the whole building, so a company-wide outage would lock out the company.

Which needs a backstop

"Not counted" must not become "unlimited". Each attempt still costs an outbound connection that may sit until UPSTREAM_TIMEOUT, and an outage is the one moment this endpoint is cheapest to abuse.

So there is a second ceiling per address — twenty times looser, never refunded. A person retrying an outage will not come near it; something hammering will. It is the reason the other two can safely be given back.

Tests

Six on the limiter: refund returns exactly one attempt, refunding every attempt never locks out, a run of real failures still adds up around a refunded one, refunding an unspent key is harmless, and reset still clears (the successful-sign-in case).

Two end to end: app.test.ts already points STALWART_URL at 127.0.0.1:1, so every sign-in in that file is the outage case — 25 consecutive attempts now stay 502/504 and never reach 429, and the message says it is not the password.

Five of these fail on main.

npm run typecheck, npm test (992 web + 134 server), npm run build pass.

Merged 2026-09-02 as coffey-labs/ihasmail@2a41a31bb9

_{Rebuilt from: git history, session transcript.}

Closes #239. ihasmail runs in its own container, usually on its own host, so Stalwart being briefly unreachable is an ordinary condition. Sign-in handled it *almost* right — a 401 is `invalid_credentials`, a timeout is 504, anything else is 502, and none of those reads as a rejected password. **What it got wrong was the counting.** `RateLimiter.check()` consumes an attempt when called, and it is called *before* the upstream is contacted; `reset()` only runs on success. So every try against an unreachable server burned a credential attempt, and after ten of them the person was locked out for the rest of the fifteen-minute window — **including after the server came back**. A thirty-second blip became a quarter-hour lockout, and the second failure was entirely ihasmail's own doing. ## The fix A **401** is a judgement about the password and stays counted. A **502 or 504** is the upstream failing to answer, and is refunded — *one attempt back*, not the key cleared, so a run of real failures with an outage in the middle still adds up. The old-server refusal (`unsupported_server`) refunds too: those credentials were accepted. **Both guessing keys are refunded, not just the username one.** This is the part I got wrong on the first pass and caught before pushing: refunding only `ip|username` would not have fixed the issue, because ten retries still spend the per-address budget — and behind one office NAT that budget belongs to the whole building, so a company-wide outage would lock out the company. ## Which needs a backstop "Not counted" must not become "unlimited". Each attempt still costs an outbound connection that may sit until `UPSTREAM_TIMEOUT`, and an outage is the one moment this endpoint is cheapest to abuse. So there is a second ceiling per address — twenty times looser, **never refunded**. A person retrying an outage will not come near it; something hammering will. It is the reason the other two can safely be given back. ## Tests **Six on the limiter**: refund returns exactly one attempt, refunding every attempt never locks out, a run of real failures still adds up around a refunded one, refunding an unspent key is harmless, and reset still clears (the successful-sign-in case). **Two end to end**: `app.test.ts` already points `STALWART_URL` at `127.0.0.1:1`, so every sign-in in that file *is* the outage case — 25 consecutive attempts now stay 502/504 and never reach 429, and the message says it is not the password. Five of these fail on `main`. `npm run typecheck`, `npm test` (992 web + 134 server), `npm run build` pass. **Merged** 2026-09-02 as coffey-labs/ihasmail@2a41a31bb9af <sub>Rebuilt from: git history, session transcript.</sub>

This repo is archived. You cannot comment on issues.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: coffey-labs/ihasmail-github-archive#240