fix(api): count sign-in failures per email and IP so strangers cannot lock a Player out #546

Merged

root merged 2 commits from fix/436-sign-in-lockout into main

2026-10-16 06:10:05 +00:00

root commented

2026-10-15 22:41:12 +00:00

(Migrated from gitlab.core-project.space)

Summary

Sign-in failures are now counted per email and IP, so five wrong passwords from a stranger no longer refuse the real Player from their own address. The per-IP limit is unchanged.

 auth.signIn
   admit(
-    signInFailuresPerEmail       key: email
+    signInFailuresPerEmailAndIp  key: "email ip"
     signInFailuresPerIp          key: ip          # unchanged, 20 / 15 min
   )

The limit is renamed, and so is its override: RATE_LIMIT_SIGN_IN_FAILURES_PER_EMAIL becomes RATE_LIMIT_SIGN_IN_FAILURES_PER_EMAIL_AND_IP. No deploy file or script sets the old name.

Evidence

Before: the new test in apps/api/test/rpc-rate-limits.integration.test.ts fails:
```
5 wrong passwords from 203.0.113.30, 5 from 203.0.113.31
owner signs in from 198.51.100.21 with the right password
  → TOO_MANY_REQUESTS { retryAfterSeconds: 900 }   ✗ expected the Account
```
After: it passes. The sixth attempt from the guessing address is still refused, even with the right password, and a burst from one address still lets only five reach the password check. The gate is green.

Merge Danger

Door: two-way

Reverting restores the old key. In-memory counters reset on deploy anyway (ADR 0041).

Blast Radius: sign-in

This trades the owner lockout for a looser cap on one email: failures spread across many addresses are now capped only at 5 per address, and the per-IP limit of 20 still applies to each address. Any environment that sets the old variable name outside the repo silently falls back to the default of 5.

Proposed rule changes

ADR 0041 says "Counters are keyed by IP and by lowercased email". Sign-in failures are now keyed by lowercased email together with IP, so failures from other addresses never refuse a caller who did not fail (#436). An amendment should record this and the distributed-guessing trade-off above. deploy/compose.preview.yaml and scripts/assign-dev-rate-limits.sh still say "per-email limits". That remains true of what they mean (those limits keep production values), but the comments could say "per-email and per-email-and-IP".

Closes #436

## Summary Sign-in failures are now counted per email **and** IP, so five wrong passwords from a stranger no longer refuse the real **Player** from their own address. The per-IP limit is unchanged. ```diff auth.signIn admit( - signInFailuresPerEmail key: email + signInFailuresPerEmailAndIp key: "email ip" signInFailuresPerIp key: ip # unchanged, 20 / 15 min ) ``` The limit is renamed, and so is its override: `RATE_LIMIT_SIGN_IN_FAILURES_PER_EMAIL` becomes `RATE_LIMIT_SIGN_IN_FAILURES_PER_EMAIL_AND_IP`. No deploy file or script sets the old name. ## Evidence - **Before:** the new test in `apps/api/test/rpc-rate-limits.integration.test.ts` fails: ```text 5 wrong passwords from 203.0.113.30, 5 from 203.0.113.31 owner signs in from 198.51.100.21 with the right password → TOO_MANY_REQUESTS { retryAfterSeconds: 900 } ✗ expected the Account ``` **After:** it passes. The sixth attempt from the guessing address is still refused, even with the right password, and a burst from one address still lets only five reach the password check. The gate is green. ## Merge Danger **Door:** two-way Reverting restores the old key. In-memory counters reset on deploy anyway (ADR 0041). **Blast Radius:** sign-in This trades the owner lockout for a looser cap on one email: failures spread across many addresses are now capped only at 5 per address, and the per-IP limit of 20 still applies to each address. Any environment that sets the old variable name outside the repo silently falls back to the default of 5. ## Proposed rule changes ADR 0041 says "Counters are keyed by IP and by lowercased email". Sign-in failures are now keyed by lowercased email together with IP, so failures from other addresses never refuse a caller who did not fail (#436). An amendment should record this and the distributed-guessing trade-off above. `deploy/compose.preview.yaml` and `scripts/assign-dev-rate-limits.sh` still say "per-email limits". That remains true of what they mean (those limits keep production values), but the comments could say "per-email and per-email-and-IP". Closes #436

project_1_bot_a8dd9262857d850eef8dca5206e61de5 commented

2026-10-15 23:01:12 +00:00

(Migrated from gitlab.core-project.space)

Screenshots

Before is production, after is this merge request's review app at b39aa75.

Desktop

Page	Before	After
Home
Local game
Room

Mobile

Page	Before	After
Home
Local game
Room

### Screenshots Before is production, after is this merge request's review app at b39aa75. #### Desktop | Page | Before | After | | --- | --- | --- | | Home | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/before/home-desktop.png" width="400"> | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/after/home-desktop.png" width="400"> | | Local game | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/before/local-desktop.png" width="400"> | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/after/local-desktop.png" width="400"> | | Room | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/before/room-desktop.png" width="400"> | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/after/room-desktop.png" width="400"> | #### Mobile | Page | Before | After | | --- | --- | --- | | Home | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/before/home-mobile.png" width="200"> | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/after/home-mobile.png" width="200"> | | Local game | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/before/local-mobile.png" width="200"> | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/after/local-mobile.png" width="200"> | | Room | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/before/room-mobile.png" width="200"> | <img src="https://gitlab.core-project.space/amaurycoudr/chess-project/-/jobs/1790/artifacts/raw/review-screenshots/after/room-mobile.png" width="200"> |

root commented

2026-10-16 06:01:10 +00:00

(Migrated from gitlab.core-project.space)

added 23 commits

9373cb6f...c842db7f - 21 commits from branch main
bca4cd91 - fix(api): count sign-in failures per email and IP so strangers cannot lock a Player out
b39aa755 - refactor(api): build the sign-in failure key in the order its limit names it

Compare with previous version

added 23 commits <ul><li>9373cb6f...c842db7f - 21 commits from branch <code>main</code></li><li>bca4cd91 - fix(api): count sign-in failures per email and IP so strangers cannot lock a Player out</li><li>b39aa755 - refactor(api): build the sign-in failure key in the order its limit names it</li></ul> [Compare with previous version](/amaurycoudr/chess-project/-/merge_requests/46/diffs?diff_id=310&start_sha=9373cb6f9399ed3817b228b1e557a47c966e5a82)

root commented

2026-10-16 06:01:42 +00:00

(Migrated from gitlab.core-project.space)

mentioned in issue #439