fix(carddav): gate Basic auth on the shared login rate limit

[Monkey7539]

Summary

CardDAV accepts the account password over HTTP Basic, but a right password is never checked against the login rate limit. Once an IP has used up its budget, a wrong guess gets 429 and the right one still gets in. So the limit tells a guesser when they have found the password instead of stopping them.

cardavAuth only reads the shared auth:<ip> bucket inside authFail, after bcrypt.compare has already said no, and only to pick 429 over 401 (carddav.js#L83-L90, #L97-L98). A right password goes straight to next() (#L107-L108), and every guess still runs a bcrypt compare, however far over budget the IP is. The only real bound is the CardDAV request limit of 500 per window per IP (#L37-L39), an in-memory counter that is per process and resets on restart. /api/auth/login takes its slot before it looks at the password (auth.js#L77-L89), so at the default settings it allows 10 guesses per 15 minutes where CardDAV allows 500.

Installs that route /carddav/ to the backend themselves are exposed today. The stock nginx config does not proxy it yet.

Changes

• cardavAuth takes a slot from the shared auth:<ip> budget before it checks the password, with the same atomic consume() the login routes use. If the budget is used up, it answers 429 with Retry-After and does not check the password or log an auth event, the same as the login limiter.
• A wrong password keeps its slot and is logged as carddav_auth_fail with a 401, as before. So does a guess at an unknown username or at an account with no password, after the existing dummy compare.
• A right password hands its slot back through a new rateLimiter.release(). A syncing client authenticates on every request, so it never uses up the budget. Neither does a 2FA account whose phone keeps sending the right password and getting 403.
• A request refused with 429 hands its slot back too, since no password was checked. Otherwise the refused requests in a burst of right passwords larger than the budget would stay counted, and enough of them would lock the IP out of CardDAV and web login without a single wrong password. In the last two seconds of a window the refused request keeps its slot instead. The counter is about to expire anyway, and a hand-back that landed after it did would take a counted guess off the next window.
• The slot is taken after the user lookup, so a database error answers 500 without counting.
• release() decrements the Redis counter and deletes the key if it had expired mid-request and came back at -1. When Redis is down it falls back to the in-memory counter, never below zero, like consume().

Two simpler versions do not work. Checking the bucket before the password without taking a slot lets a burst of concurrent guesses all pass the check. Resetting the bucket on a right password, as /api/auth/login does, would let any CardDAV sync from the same IP, including an attacker's own account, wipe the counter that web login failures also use.

Behaviour change: once an IP has used up the budget, CardDAV from that IP gets 429 even with the right password until the window ends. The budget counts wrong CardDAV passwords plus requests to the sign-in routes that share the limit. Web login from that IP is already blocked the same way, and the admin setting's description ("Failed login attempts allowed from a single IP address before it is temporarily blocked") now holds for CardDAV too. The screen-lock PIN avoids blocking a correct PIN like this by counting failures per user and signing the session out after too many (auth.js#L639-L643). CardDAV has no session to sign out, and a right password that still gets in after the budget is spent is what tells a guesser they have found it, so the check has to come before the password.

A slot is held while the password is checked, so if more CardDAV password checks are in flight from one IP at once than the budget has room for, the extra ones get 429 with a Retry-After for the rest of the window. Those refusals are not counted, so they do not lock the IP out. With the default budget and no recent failures, that takes more than 10 at once.

The limit is keyed on req.ip, like the login limiter. Since #540 the https profile sets TRUST_PROXY_HOPS=2, so behind Caddy req.ip is the client's address, and the README says to set it behind an operator's own proxy too. Where it is not set behind a second proxy, req.ip is that proxy's address and web login already shares one budget across all users there. Once /carddav/ is proxied through nginx, CardDAV on such an install would share that budget too: ten wrong passwords from one device with a stale password would stop CardDAV sync for every user until the window ends. So this is best in before /carddav/ is proxied, and that change should point at TRUST_PROXY_HOPS.

Not in this PR:

• CardDAV Basic auth still ignores SSO-only (internal_auth_disabled) and MFA-required (mfa_enforcement). Enforcing them would turn CardDAV off on those servers until app-specific passwords exist, so that goes in a separate issue.
• A successful sign-in deletes the whole auth:<ip> counter (auth.js#L86, called at #L265 and #L313), CardDAV failures included. Anyone who can sign in to their own account from the same IP can spend most of the budget guessing another account's password, sign in, and repeat. That is already true of web login. Handing back only the successful request's own slot with release() would close it, but that changes the auth routes, so it is a separate change.

Testing

New backend/src/routes/carddav.auth.test.js mounts the real CardDAV router on Express and sends PROPFIND /carddav/, the discovery request a client starts with, over fetch, like the other route tests. The real rateLimiter.js runs against an in-memory Redis stub, with a budget of 3.

• Right password after the budget is used up: three wrong passwords get 401, then the right one gets 429 with a Retry-After.
• Concurrent guesses: eight wrong passwords sent at once reach bcrypt.compare three times, and get three 401s and five 429s.
• Syncing client: ten requests with the right password all get 207.
• Burst of right passwords: six right passwords are sent at once while bcrypt.compare waits on a gate, so three hold the whole budget and three are refused. Once the gate opens, the burst gets three 207s and three 429s, and the next right password gets 207.
• End of the window: with one second left on the counter, a refused request gets 429 and stays counted.
• Right password between wrong ones: wrong, wrong, right, wrong, wrong get 401, 401, 207, 401, 429, so a right password hands back only its own slot.
• Unknown and SSO-only usernames: after two wrong passwords, a wrong password for an unknown username gets 401. The next one gets 429, and so does one for an account with no password.
• 2FA account: four requests with the right password all get 403, none 429.
• Database error: four requests while the user lookup fails get 500, and the right password still gets 207 afterwards.

rateLimiter.test.js gains three release() cases: it hands back one hit; on an expired key it does not leave a spare hit behind; and the in-memory fallback hands back one hit and never goes below zero.

Against the current code, seven of these fail. The right password gets 207 instead of 429 in the first test and in the end-of-window test, bcrypt.compare runs 8 times instead of 3, all six requests of the burst reach it, and the three release() tests fail because release does not exist. The syncing-client, right-between-wrong, unknown-username, 2FA and database-error tests pass on the current code. They pin the parts of the fix that keep sync working and the limit tight.

Breaking any one part of the fix fails at least one test:

• No release() after a right password: the syncing-client, burst, right-between-wrong and 2FA tests.
• Releasing after the 2FA check: the 2FA test.
• Taking the slot before the user lookup: the database-error test.
• Taking the slot after the unknown-user branch: the unknown-username test.
• reset() in place of release(): the right-between-wrong test (401, 401, 207, 401, 401) and three others.
• No hand-back on the refusal path: the burst test, where the next right password gets 429.
• A hand-back on the refusal path even at the end of the window: the end-of-window test.
• release() without the expired-key cleanup or the zero floor: the matching limiter test.

Backend npm run lint clean, npm run lint:plugins clean, npm test passes (2420 passed, 29 skipped without a local Postgres). The frontend is not touched.

---

Contributor License Agreement

By submitting this pull request I confirm that:

• I have read and agree to the Contributor License Agreement.
• My contribution is my own original work (or I have identified any
  third-party material and confirmed it is compatible with the CLA).
• I have the right to submit this contribution under the terms of the CLA.

🤖 Generated with Claude Code