From 0684c57060e362e64d116f8dddb0620339be7b07 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:38:54 +0100 Subject: [PATCH 1/7] docs(auth): spec for per-purpose auth throttling and login backoff (#688) Signed-off-by: alex-dembele --- docs/688_AUTH_THROTTLE.md | 144 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 docs/688_AUTH_THROTTLE.md diff --git a/docs/688_AUTH_THROTTLE.md b/docs/688_AUTH_THROTTLE.md new file mode 100644 index 000000000..74e6fa879 --- /dev/null +++ b/docs/688_AUTH_THROTTLE.md @@ -0,0 +1,144 @@ +# Spec — #688 Auth throttle: per-purpose buckets, per-account backoff, constant-work login + +Status: approved by owner 2026-10-02 (thresholds as written · `GetByEmail` itself becomes case-insensitive) · Branch: `688-auth-throttle`, stacked on #689 · Milestone: trust-v1 + +## Objective + +The auth throttle protects the wrong thing in the wrong way: + +- one per-IP bucket (15 / 5 min) serves login, sign-up, forgot, reset **and** the + password-strength meter, so a person typing on the sign-up screen locks + themselves out of signing up and signing in, and an office behind one NAT + shares 15 requests; +- nothing limits guesses against **one account** from many IPs; +- login for an unknown address returns in ~6 ms and for a real one in ~45 ms, so + the clock tells an attacker which addresses have accounts; +- login matches the address case-sensitively, while sign-up and reset normalise. + +Success means: the meter never costs a login; a throttled person is told how long +to wait, in FR and EN; ten wrong passwords on one address from any number of IPs +pause that address for a bounded time; unknown and real addresses cost the same; +`Awa.Diallo@X.test` and `awa.diallo@x.test` are the same login. + +## Current state (read 2026-10-02) + +| Piece | File | Fact | +|---|---|---| +| Limiter | `backend/internal/middleware/ratelimit.go` `RateLimit` | key `c.IP()` only; 429 body `{"error":true,"msg":"Rate limit exceeded"}`, no `Retry-After`, no `code` | +| Shared bucket | `backend/cmd/server/main.go:849-882` | one `authRateLimit` (15/5 min) on login, register, forgot, reset, check; also on protected `/auth/password/change` (1143) and `/auth/mfa/disable` (1125) | +| Redis window | `backend/internal/infrastructure/redis/client.go` `AllowRate` | fixed wall-clock bucket `now / window`, so the time left in a bucket is computable without a Redis round trip | +| Prefix pattern | `backend/internal/handler/public_vendor_assessment_handler.go:37` | `PrefixedRateLimitBackend{Prefix, Inner}` | +| Login | `backend/internal/application/auth/login.go` `Execute` | unknown address → return before hashing; disabled → return before hashing; wrong password → Argon2id `Verify` | +| Lookup | `backend/internal/infrastructure/repository/gorm_user_repository.go:73` | `WHERE email = ?`, input not normalised by login | +| Legacy rows | #687 closing comment | register normalises since #694, but no backfill and no `lower(email)` index; mixed-case rows may still exist | +| Reset confirm | `backend/internal/application/auth/password_reset.go:259` | loads the user, writes the new hash — the place to clear a login lock | +| Audit | `backend/internal/auth/audit.go` `LogFiber` | `failure_reason` text column; no metadata field | +| Screen | `frontend/src/features/auth/AuthScreen.tsx` | login: any error → `signInFailed`; register: non-409/400 → `registerFailed` | +| Sibling | `docs/689_MFA_CHALLENGE_LIMITS.md` (approved, not implemented) | plans an `MFAAttemptStore` (Incr / Exists / Set / Delete) in `infrastructure/authmfa` — the same primitive this issue needs | + +## Assumptions (correct me now or I proceed) + +1. **Thresholds** (per IP, each its own Redis key prefix): + | Bucket | Routes | Limit | + |---|---|---| + | `auth-login:` | `/auth/login` | 15 / 5 min (unchanged) | + | `auth-register:` | `/auth/register` | 10 / 15 min | + | `auth-reset:` | `/auth/password/forgot`, `/auth/password/reset` | 15 / 5 min | + | `auth-check:` | `/auth/password/check` | 120 / 5 min | + | `auth-reauth:` | `/auth/password/change`, `/auth/mfa/disable` | 15 / 5 min (today's limit, moved off the login key) | +2. **Per-account backoff**, keyed by `sha256(normalised address)`, so unknown + addresses lock exactly like real ones (no new oracle): + 10 failed logins in 15 min → the address is refused for 15 min with + `429 {"code":"LOGIN_LOCKED","retry_after":N}` + `Retry-After`. Refused attempts + neither count nor extend the lock, so it is bounded. A successful login clears + the failure count; a successful password reset clears the count **and** the + lock. Same shape and numbers as #689's per-account MFA lock. +3. **What counts as a failure:** unknown address, disabled account, wrong + password. Not: revoked membership or missing organisation (the password was right). +4. **Audit:** every failed login row carries `failure_reason = + "invalid_credentials email_sha256="`; a refused-by-lock attempt carries + `"login_locked email_sha256="`. The hash is `domain.HashEmailForReset` + (unsalted SHA-256, the function reset already uses). No migration. +5. **Constant work:** the login use case hashes a fixed dummy password once at + construction with the live hasher (current Argon2id params) and verifies the + submitted password against it for unknown addresses. A disabled account is + verified against its real hash, then refused. +6. **Case-insensitive lookup:** login normalises with `domain.NormaliseEmail`; + `GormUserRepository.GetByEmail` becomes `LOWER(email) = ?` with + `ORDER BY (email = ?) DESC, created_at ASC`, so a legacy case-variant duplicate + resolves deterministically to the exact-spelling row, then the oldest. This + makes every `GetByEmail` caller (register, reset, invitations, OAuth link, + automation) case-insensitive — which is what #687 intended. No index here + (migration numbering waits on the TPRM stack, per #687); a seq scan on `users` + is fine at current scale, and the index is a follow-up issue. +7. **Throttle response, every `RateLimit` route:** 429 gains `Retry-After` + (seconds to the end of the fixed Redis window) and the body gains + `"code":"RATE_LIMITED","retry_after":N`. `error` and `msg` stay for existing + clients. The JSON field exists because a cross-origin SPA cannot read + `Retry-After` unless CORS exposes it. +8. **Frontend:** sign-in and sign-up show "Trop de tentatives. Réessayez dans + N minute(s)." / "Too many attempts. Try again in N minute(s)." for any 429 + (`RATE_LIMITED` or `LOGIN_LOCKED`), N = ceil(retry_after / 60), min 1. +9. **Shared store with #689:** #689 (PR #871) already ships the store this + needs: `infrastructure/authmfa.AttemptStore`, Redis with an in-memory + fallback. This branch is stacked on #689 and reuses it through a narrower + login-side port; no second store is written. + +## Design + +- **Port** `application/auth/login_limits.go`: `LoginAttemptStore { Increment; Lock; LockedFor; Delete }` — a subset of #689's `MFAAttemptStore`, satisfied by `authmfa.AttemptStore` without changes. Keys `login:fail:` and `login:lock:`. +- **Use case** `LoginUseCase.WithAttemptLimits(store)` → sentinel `ErrLoginLocked` carrying `RetryAfter` (`errors.As`). Unwired = today's behaviour; existing tests untouched. +- **Use case** `ConfirmPasswordResetUseCase.WithLoginLockClearer(store)` clears both keys after the new hash is written. +- **Middleware** `RateLimit` sets `Retry-After` and the new body fields. The prefix wrapper moves to `middleware.PrefixedBackend`; `handler.PrefixedRateLimitBackend` becomes an alias, no churn elsewhere. +- **Handler** `Login` maps `ErrLoginLocked` → 429; writes the hashed address in every failure audit row. +- **main.go**: wiring only. + +## Commands + +``` +cd backend && go build ./... && go vet ./... && go test ./... -race +cd backend && go test ./internal/application/auth/ ./internal/handler/auth/ ./internal/middleware/ -race -v -run 'Login|AuthRateLimit|RateLimit' +cd frontend && npx tsc -b --noEmit && npx eslint src/features/auth && npx vitest run && npm run build +``` +Live: throwaway pg + redis on spare ports, server from the repo root (live-boot recipe); re-run reproductions 1, 2, 4 by curl; 20-attempt timing medians; real-browser pass on sign-in and sign-up at 429. + +## Testing strategy +- **Use case** (`login_throttle_test.go`, fake store + counting hasher): `TestLogin_UnknownAddressCostsAHashComparison` (Verify called once for unknown and for disabled), `TestLogin_EmailCaseInsensitive`, `TestLogin_PerAccountBackoff` (10 failures → `ErrLoginLocked` even with the right password; unknown address locks too; lock bounded; success clears count), plus Success / NotFound / Unauthorized. +- **Reset**: `TestConfirmPasswordReset_ClearsLoginLock`. +- **Repository**: `GetByEmail` case-insensitive and deterministic on duplicates. +- **HTTP**, real middleware chain: `TestAuthRateLimit_PasswordCheckDoesNotConsumeLoginBudget`; 429 carries `Retry-After` + `code`; `LOGIN_LOCKED` over HTTP. +- **Frontend**: `authThrottle.test.tsx` — login 429 and register 429 render the wait message in FR and EN. +- **Timing**: live medians over 20 attempts, unknown vs existing, < 10 ms apart. + +## Boundaries +- **Always:** typed errors; no address, password or token in logs (hash only); full `go test ./...` before the PR. +- **Ask first:** thresholds other than the ones above; any migration (index, audit metadata column); CORS changes. +- **Never:** touch the MFA challenge route (#689); change the password hash algorithm or params; make Redis fail-closed. + +## Success criteria +1. 121 × `/auth/password/check` from one IP, then `/auth/login` with valid credentials → 200; `/auth/register` → 201. +2. Any throttled auth call → 429 with `Retry-After` and `code`; both screens show the wait in FR and EN. +3. 10 wrong passwords on one address over ≥ 2 IPs → the 11th, even correct, answers 429 `LOGIN_LOCKED`; after the lock, correct → 200; a password reset clears it immediately; every failure has an audit row with `email_sha256=`. +4. Median login time over 20 attempts, unknown vs existing address, differs by < 10 ms. +5. `Awa.Diallo@X.test` logs into `awa.diallo@x.test`. +6. The five named tests pass under `-race`; full backend and frontend gates green. + +## Known limits +- **Deliberate lockout:** anyone can pause any address for 15 min, repeatedly. Bounded per lock; the owner's way out is a password reset. Same trade-off accepted for #689. +- **Redis down:** buckets and locks become per-instance. +- **`Retry-After` in degraded mode:** computed from the Redis fixed window; the in-memory fallback slides, so the hint may be early by up to one window. +- **Audit hash:** unsalted SHA-256 of an address is dictionary-reversible for a known address list. Kept for consistency with reset; HMAC is a follow-up if wanted. +- **Legacy hashes:** accounts on a pre-upgrade hash verify at a different cost than the dummy until their next login migrates them. +- **`LogFiber` reads `X-Forwarded-For` raw** for the audit IP (spoofable). Pre-existing, out of scope; follow-up issue. + +## Plan — one commit per task, `fix(auth): … (#688)` +- [ ] **T1** Throttle response (`Retry-After`, `code`, `retry_after`) + `middleware.PrefixedBackend`. Verify `go test ./internal/middleware/ -race`. +- [ ] **T2** Five per-purpose buckets in `main.go`; `TestAuthRateLimit_PasswordCheckDoesNotConsumeLoginBudget`. +- [ ] **T3** Case-insensitive lookup (login normalises; repo lower + deterministic order); `TestLogin_EmailCaseInsensitive` + repo test. +- [ ] **T4** Constant work (dummy hash; unknown + disabled verified); `TestLogin_UnknownAddressCostsAHashComparison`. +- [ ] **T5** ~~Attempt store~~ — reused from #689. +- [ ] **T6** Per-account backoff: login lock, reset clears, handler 429 + hashed audit, wiring; `TestLogin_PerAccountBackoff`, `TestConfirmPasswordReset_ClearsLoginLock`. +- [ ] **T7** Frontend FR/EN strings + 429 on sign-in and sign-up + vitest. +- [ ] **T8** Full gates; reproductions 1, 2, 4 re-run live; browser pass; issue comment; PR `Closes #688`; follow-up issues (email index, audit IP header). + +Risk: the route registry / authz ratchet tests may trip on the new middleware (T2) — full `go test ./...` before the PR. From 70b95650d62ea5f2fbf107e3a767e594dfc704ce Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:39:35 +0100 Subject: [PATCH 2/7] fix(auth): tell a throttled caller how long to wait (#688) Signed-off-by: alex-dembele --- backend/internal/middleware/ratelimit.go | 27 +++++++- backend/internal/middleware/ratelimit_test.go | 62 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/backend/internal/middleware/ratelimit.go b/backend/internal/middleware/ratelimit.go index ad60003c1..8a2e2532c 100644 --- a/backend/internal/middleware/ratelimit.go +++ b/backend/internal/middleware/ratelimit.go @@ -8,6 +8,7 @@ package middleware import ( "context" "fmt" + "strconv" "sync" "time" @@ -152,9 +153,16 @@ func RateLimit(config RateLimitConfig) fiber.Handler { } } // Check rate limit if !config.Store.IsAllowed(key, config.MaxRequests, config.WindowSize) { + // #688: say how long to wait. The header serves HTTP clients; the + // body serves the SPA, which cannot read a header CORS does not + // expose. error/msg stay for the clients that already read them. + wait := int(retryAfter(time.Now(), config.WindowSize) / time.Second) + c.Set(fiber.HeaderRetryAfter, strconv.Itoa(wait)) return c.Status(fiber.StatusTooManyRequests).JSON(fiber.Map{ - "error": true, - "msg": "Rate limit exceeded", + "error": true, + "msg": "Rate limit exceeded", + "code": "RATE_LIMITED", + "retry_after": wait, }) } @@ -162,6 +170,21 @@ func RateLimit(config RateLimitConfig) fiber.Handler { } } +// retryAfter is the time left in the current window, rounded up to whole +// seconds and never under one. +// +// The Redis store counts in fixed, wall-clock-aligned buckets (now / window, see +// redis.Client.AllowRate), so the end of the bucket is known without asking +// Redis. The in-memory fallback slides instead, so in degraded mode this can be +// early by up to one window; a retry then is simply refused again. +func retryAfter(now time.Time, window time.Duration) time.Duration { + left := window - time.Duration(now.UnixNano()%int64(window)) + if left < time.Second { + return time.Second + } + return (left + time.Second - 1).Truncate(time.Second) +} + // AuthRateLimit creates a strict rate limiter for auth endpoints // Default: 5 requests per 15 minutes per IP func AuthRateLimit() fiber.Handler { diff --git a/backend/internal/middleware/ratelimit_test.go b/backend/internal/middleware/ratelimit_test.go index a6d840721..fbb2da220 100644 --- a/backend/internal/middleware/ratelimit_test.go +++ b/backend/internal/middleware/ratelimit_test.go @@ -7,8 +7,11 @@ package middleware import ( "context" + "encoding/json" "errors" "net/http" + "net/http/httptest" + "strconv" "testing" "time" @@ -275,3 +278,62 @@ func BenchmarkRateLimitStore_IsAllowed(b *testing.B) { store.IsAllowed("bench-key", 1000, 1*time.Minute) } } + +// #688 — a throttled caller must be told how long to wait, in a header for +// HTTP clients and in the body for a browser app that cannot read headers +// CORS does not expose. +func TestRateLimit_RefusalCarriesRetryAfterAndCode(t *testing.T) { + app := fiber.New() + window := 5 * time.Minute + app.Post("/x", RateLimit(RateLimitConfig{MaxRequests: 1, WindowSize: window, Store: NewRateLimitStore()}), + func(c *fiber.Ctx) error { return c.SendStatus(fiber.StatusNoContent) }) + + first, _ := app.Test(httptest.NewRequest(http.MethodPost, "/x", nil)) + if first.StatusCode != fiber.StatusNoContent { + t.Fatalf("first request: got %d", first.StatusCode) + } + resp, _ := app.Test(httptest.NewRequest(http.MethodPost, "/x", nil)) + if resp.StatusCode != fiber.StatusTooManyRequests { + t.Fatalf("second request: got %d, want 429", resp.StatusCode) + } + + header, err := strconv.Atoi(resp.Header.Get("Retry-After")) + if err != nil || header < 1 || header > int(window.Seconds()) { + t.Fatalf("Retry-After = %q, want 1..%d seconds", resp.Header.Get("Retry-After"), int(window.Seconds())) + } + + var body struct { + Error bool `json:"error"` + Msg string `json:"msg"` + Code string `json:"code"` + RetryAfter int `json:"retry_after"` + } + if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + t.Fatalf("decode: %v", err) + } + if body.Code != "RATE_LIMITED" || body.RetryAfter != header { + t.Fatalf("body = %+v, want code RATE_LIMITED and retry_after %d", body, header) + } + // Existing clients read these two; they stay. + if !body.Error || body.Msg != "Rate limit exceeded" { + t.Fatalf("legacy fields changed: %+v", body) + } +} + +func TestRetryAfter_IsTheRestOfTheFixedWindow(t *testing.T) { + window := 5 * time.Minute + start := time.Unix(0, 0).Add(1000 * window) // a bucket boundary + cases := []struct { + at time.Duration + want time.Duration + }{ + {0, window}, + {time.Minute, 4 * time.Minute}, + {window - 300*time.Millisecond, time.Second}, // never below one second + } + for _, tc := range cases { + if got := retryAfter(start.Add(tc.at), window); got != tc.want { + t.Errorf("at +%v: got %v, want %v", tc.at, got, tc.want) + } + } +} From 15b1fda53cb082b1e7fa46d8f78c5e36f066ede6 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:40:48 +0100 Subject: [PATCH 3/7] fix(auth): give login, sign-up, reset and the strength meter separate throttles (#688) Signed-off-by: alex-dembele --- backend/cmd/server/main.go | 35 +++---- .../public_vendor_assessment_handler.go | 19 +--- backend/internal/middleware/auth_limits.go | 79 +++++++++++++++ .../internal/middleware/auth_limits_test.go | 97 +++++++++++++++++++ 4 files changed, 193 insertions(+), 37 deletions(-) create mode 100644 backend/internal/middleware/auth_limits.go create mode 100644 backend/internal/middleware/auth_limits_test.go diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index ac7714e1b..3718df6a3 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -850,25 +850,16 @@ func main() { }) }) - // Brute-force protection on credential endpoints (5 attempts / 15 min per IP). - // Backed by Redis so the counter is shared across every instance of a - // horizontally-scaled deployment; degrades gracefully to a per-instance - // in-memory limiter if Redis is unreachable. + // Per-IP throttles on the auth routes, one bucket per purpose (#688). Backed + // by Redis so the counters are shared across every instance; degrades to a + // per-instance in-memory limiter if Redis is unreachable. Guessing against + // one account is bounded per address in the login use case, not here. authLimiterStore := middleware.NewRedisRateLimitStore(redisClientInstance) - // Per-IP throttle on auth endpoints. 5/15min locked out legitimate users (a - // couple of mistyped passwords, MFA re-auth, or shared-NAT colleagues) for a - // quarter hour (OR-BUG-008). 15/5min still blocks rapid brute force (~3/min - // sustained) without punishing normal use. Per-account lockout + captcha are - // the stronger long-term controls (tracked separately). - authRateLimit := middleware.RateLimit(middleware.RateLimitConfig{ - MaxRequests: 15, - WindowSize: 5 * time.Minute, - Store: authLimiterStore, - }) + authLimits := middleware.NewAuthLimiters(authLimiterStore) // Clean Architecture Auth Routes - api.Post("/auth/login", authRateLimit, cleanAuthHandler.Login) - api.Post("/auth/register", authRateLimit, cleanAuthHandler.Register) + api.Post("/auth/login", authLimits.Login, cleanAuthHandler.Login) + api.Post("/auth/register", authLimits.Register, cleanAuthHandler.Register) api.Post("/auth/refresh", cleanAuthHandler.RefreshToken) api.Post("/auth/logout", cleanAuthHandler.Logout) @@ -878,16 +869,16 @@ func main() { // path. /users/me is served by the profile handler (#719). // --- Password reset (public) --- - // Both legs sit behind the per-IP auth limiter on top of the per-address cap + // Both legs share the per-IP reset bucket, on top of the per-address cap // the use case enforces: the address cap stops targeting one account, the IP // limiter stops sweeping many. // // /password/check is unauthenticated by necessity — it serves the strength // meter on the registration and reset screens, where there is no session. It // discloses nothing: the caller already knows the password they typed. - api.Post("/auth/password/forgot", authRateLimit, passwordHandler.ForgotPassword) - api.Post("/auth/password/reset", authRateLimit, passwordHandler.ResetPassword) - api.Post("/auth/password/check", authRateLimit, passwordHandler.CheckPassword) + api.Post("/auth/password/forgot", authLimits.Reset, passwordHandler.ForgotPassword) + api.Post("/auth/password/reset", authLimits.Reset, passwordHandler.ResetPassword) + api.Post("/auth/password/check", authLimits.Check, passwordHandler.CheckPassword) // --- OAuth2 Routes --- api.Get("/auth/oauth2/login/:provider", handlers.OAuth2Login) @@ -1142,7 +1133,7 @@ func main() { api.Post("/auth/mfa/setup", mfaEnrollmentGuard, mfaHandler.Setup) api.Post("/auth/mfa/verify", mfaEnrollmentGuard, mfaHandler.Verify) // Throttled like /auth/password/change: the body carries a password guess. - protected.Post("/auth/mfa/disable", authRateLimit, mfaHandler.Disable) + protected.Post("/auth/mfa/disable", authLimits.Reauth, mfaHandler.Disable) // --- MFA policy (OR26-03) — "force MFA after N days" ----------------------- // Reading is open to any authenticated member: everyone subject to a deadline @@ -1160,7 +1151,7 @@ func main() { // In-session password change (#720, D-047). Acts on the session's own user; // behind the auth rate limiter so a wrong current password spends the same // budget a failed sign-in does. - protected.Post("/auth/password/change", authRateLimit, passwordHandler.ChangePassword) + protected.Post("/auth/password/change", authLimits.Reauth, passwordHandler.ChangePassword) protected.Delete("/auth/sessions/:id", sessionHandler.RevokeSession) // Organization switching — list the orgs the user may enter, and switch into diff --git a/backend/internal/handler/public_vendor_assessment_handler.go b/backend/internal/handler/public_vendor_assessment_handler.go index ee0e77fe6..9afbdae1d 100644 --- a/backend/internal/handler/public_vendor_assessment_handler.go +++ b/backend/internal/handler/public_vendor_assessment_handler.go @@ -28,21 +28,10 @@ const ( publicSubmitsPerHour = 10 ) -// PrefixedRateLimitBackend namespaces a shared counter store. -// -// middleware.RateLimit keys its counter by the raw client IP, and the Redis -// store is shared by every limiter in the process. Without a prefix, the -// questionnaire's 60/min counter and the login throttle's 15/5min counter would -// be the SAME key, each consuming the other's budget. -type PrefixedRateLimitBackend struct { - Prefix string - Inner middleware.RateLimitBackend -} - -// IsAllowed consults the inner store under the prefixed key. -func (p PrefixedRateLimitBackend) IsAllowed(key string, maxRequests int, window time.Duration) bool { - return p.Inner.IsAllowed(p.Prefix+key, maxRequests, window) -} +// PrefixedRateLimitBackend namespaces a shared counter store. It moved to +// middleware with the per-purpose auth limiters (#688); the name stays for its +// callers. +type PrefixedRateLimitBackend = middleware.PrefixedBackend // PublicVendorAssessmentHandler serves the questionnaire to a vendor contact who // holds no account (#670, ADR 0004 D4). No auth middleware runs in front of it; diff --git a/backend/internal/middleware/auth_limits.go b/backend/internal/middleware/auth_limits.go new file mode 100644 index 000000000..aa1946bf6 --- /dev/null +++ b/backend/internal/middleware/auth_limits.go @@ -0,0 +1,79 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package middleware + +import ( + "time" + + "github.com/gofiber/fiber/v2" +) + +// PrefixedBackend namespaces a shared counter store. +// +// RateLimit keys its counter by the raw client IP, and the Redis store is shared +// by every limiter in the process. Without a prefix, two limiters on the same IP +// are the SAME key, and each spends the other's budget. +type PrefixedBackend struct { + Prefix string + Inner RateLimitBackend +} + +// IsAllowed consults the inner store under the prefixed key. +func (p PrefixedBackend) IsAllowed(key string, maxRequests int, window time.Duration) bool { + return p.Inner.IsAllowed(p.Prefix+key, maxRequests, window) +} + +// Per-IP budgets on the public auth routes, one per purpose (#688). +// +// They used to be one 15/5min bucket for login, sign-up, reset and the password +// strength meter, which the sign-up screen calls as the person types: typing a +// password locked them out of signing up and signing in. Guessing against one +// account is bounded per address in the login use case, not here; these only +// stop one source from sweeping. +const ( + AuthLoginLimit = 15 + AuthLoginWindow = 5 * time.Minute + AuthRegisterLimit = 10 + AuthRegisterWindow = 15 * time.Minute + // Forgot and reset are one purpose: both legs of the same recovery. + AuthResetLimit = 15 + AuthResetWindow = 5 * time.Minute + // The strength meter is debounced but still fires several times per + // password typed. It discloses nothing the caller did not type. + AuthCheckLimit = 120 + AuthCheckWindow = 5 * time.Minute + // Re-authentication inside a session: changing the password, disabling MFA. + AuthReauthLimit = 15 + AuthReauthWindow = 5 * time.Minute +) + +// AuthLimiters are the per-purpose limiters for the auth routes. +type AuthLimiters struct { + Login fiber.Handler + Register fiber.Handler + Reset fiber.Handler + Check fiber.Handler + Reauth fiber.Handler +} + +// NewAuthLimiters builds every auth limiter over one store, each under its own +// key prefix. Pass the shared Redis store so the budgets hold across instances. +func NewAuthLimiters(store RateLimitBackend) AuthLimiters { + limit := func(prefix string, max int, window time.Duration) fiber.Handler { + return RateLimit(RateLimitConfig{ + MaxRequests: max, + WindowSize: window, + Store: PrefixedBackend{Prefix: prefix, Inner: store}, + }) + } + return AuthLimiters{ + Login: limit("auth-login:", AuthLoginLimit, AuthLoginWindow), + Register: limit("auth-register:", AuthRegisterLimit, AuthRegisterWindow), + Reset: limit("auth-reset:", AuthResetLimit, AuthResetWindow), + Check: limit("auth-check:", AuthCheckLimit, AuthCheckWindow), + Reauth: limit("auth-reauth:", AuthReauthLimit, AuthReauthWindow), + } +} diff --git a/backend/internal/middleware/auth_limits_test.go b/backend/internal/middleware/auth_limits_test.go new file mode 100644 index 000000000..e4004fc73 --- /dev/null +++ b/backend/internal/middleware/auth_limits_test.go @@ -0,0 +1,97 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gofiber/fiber/v2" +) + +// authLimitedApp mounts the five auth limiters the way main.go does, over ONE +// shared store — the production shape, where every limiter talks to the same +// Redis. +func authLimitedApp() *fiber.App { + l := NewAuthLimiters(NewRateLimitStore()) + ok := func(c *fiber.Ctx) error { return c.SendStatus(fiber.StatusOK) } + app := fiber.New() + app.Post("/auth/login", l.Login, ok) + app.Post("/auth/register", l.Register, ok) + app.Post("/auth/password/forgot", l.Reset, ok) + app.Post("/auth/password/reset", l.Reset, ok) + app.Post("/auth/password/check", l.Check, ok) + app.Post("/auth/password/change", l.Reauth, ok) + return app +} + +func post(t *testing.T, app *fiber.App, path string) int { + t.Helper() + resp, err := app.Test(httptest.NewRequest(http.MethodPost, path, nil)) + if err != nil { + t.Fatalf("%s: %v", path, err) + } + return resp.StatusCode +} + +// #688 reproduction 1: the strength meter, called while a person types, used to +// spend the budget login and sign-up share, and lock them out of both. +func TestAuthRateLimit_PasswordCheckDoesNotConsumeLoginBudget(t *testing.T) { + app := authLimitedApp() + + for i := 1; i <= AuthCheckLimit; i++ { + if got := post(t, app, "/auth/password/check"); got != fiber.StatusOK { + t.Fatalf("check %d: got %d", i, got) + } + } + if got := post(t, app, "/auth/password/check"); got != fiber.StatusTooManyRequests { + t.Fatalf("check %d: got %d, want 429", AuthCheckLimit+1, got) + } + + for _, path := range []string{"/auth/login", "/auth/register", "/auth/password/forgot", "/auth/password/change"} { + if got := post(t, app, path); got != fiber.StatusOK { + t.Errorf("%s after the check budget ran out: got %d, want 200", path, got) + } + } +} + +func TestAuthRateLimit_EachPurposeHasItsOwnBudget(t *testing.T) { + app := authLimitedApp() + + for i := 0; i < AuthLoginLimit; i++ { + post(t, app, "/auth/login") + } + if got := post(t, app, "/auth/login"); got != fiber.StatusTooManyRequests { + t.Fatalf("login past its budget: got %d, want 429", got) + } + // Login exhausted; nothing else is. + for _, path := range []string{"/auth/register", "/auth/password/forgot", "/auth/password/check", "/auth/password/change"} { + if got := post(t, app, path); got != fiber.StatusOK { + t.Errorf("%s after login ran out: got %d, want 200", path, got) + } + } + + // Forgot and reset are one purpose and share one budget. + for i := 0; i < AuthResetLimit; i++ { + post(t, app, "/auth/password/forgot") + } + if got := post(t, app, "/auth/password/reset"); got != fiber.StatusTooManyRequests { + t.Errorf("reset after forgot spent the reset budget: got %d, want 429", got) + } +} + +func TestPrefixedBackend_KeepsCountersApart(t *testing.T) { + shared := NewRateLimitStore() + a := PrefixedBackend{Prefix: "a:", Inner: shared} + b := PrefixedBackend{Prefix: "b:", Inner: shared} + if !a.IsAllowed("1.2.3.4", 1, 60e9) || a.IsAllowed("1.2.3.4", 1, 60e9) { + t.Fatal("a: want one allowed then refused") + } + if !b.IsAllowed("1.2.3.4", 1, 60e9) { + t.Fatal("b must not see a's count") + } +} From e8e4866f39d646af02037cdb35b234e67bded692 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:43:12 +0100 Subject: [PATCH 4/7] fix(auth): case-insensitive sign-in lookup and equal work for every refusal (#688) GetByEmail now matches LOWER(email) and resolves a legacy case-variant duplicate to the exact spelling, then the oldest row. Login normalises the address before the lookup. An unknown address is compared against a hash made at start-up with the live hasher, and a disabled account against its own hash, so neither answers faster than a wrong password. Signed-off-by: alex-dembele --- backend/internal/application/auth/login.go | 37 ++++-- .../application/auth/login_throttle_test.go | 118 ++++++++++++++++++ .../repository/gorm_user_repository.go | 19 ++- .../gorm_user_repository_email_test.go | 95 ++++++++++++++ 4 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 backend/internal/application/auth/login_throttle_test.go create mode 100644 backend/internal/infrastructure/repository/gorm_user_repository_email_test.go diff --git a/backend/internal/application/auth/login.go b/backend/internal/application/auth/login.go index 725545ae8..a99a90579 100644 --- a/backend/internal/application/auth/login.go +++ b/backend/internal/application/auth/login.go @@ -73,6 +73,9 @@ type LoginUseCase struct { mfaPolicies MFAPolicyReader // now is injectable so the grace arithmetic is testable without sleeping. now func() time.Time + // dummyHash is compared against when there is no account to compare with, + // so an unknown address costs what a real one does (#688). + dummyHash string } // PasswordUpgrader is the half of the hasher that knows whether a stored hash @@ -109,13 +112,26 @@ type MFAGraceAnchorWriter interface { // NewLoginUseCase creates a new login use case func NewLoginUseCase(userRepo UserRepository, tokenManager *auth.TokenManager, passwordHasher auth.PasswordHasher) *LoginUseCase { - return &LoginUseCase{ + uc := &LoginUseCase{ userRepo: userRepo, tokenManager: tokenManager, passwordHasher: passwordHasher, } + // Hashed once, with the live hasher, so it carries today's cost parameters. + // If hashing fails the comparison runs against an empty hash, which the + // Argon2id hasher refuses at full cost anyway (spendArgon2idCost). + if dummy, err := passwordHasher.Hash(loginTimingDummyPassword); err == nil { + uc.dummyHash = dummy + } else { + log.Printf("login: timing-equaliser hash unavailable: %v", err) + } + return uc } +// loginTimingDummyPassword is hashed at start-up and never matches anything a +// person can sign in with: it is compared only when there is no account. +const loginTimingDummyPassword = "openrisk-login-timing-equaliser" + // RequireMFAForRoles makes MFA mandatory for the named org roles and business // roles. // @@ -169,22 +185,27 @@ func (uc *LoginUseCase) Execute(ctx context.Context, input LoginInput) (*LoginOu return nil, domain.NewValidationError("password is required") } - // Find user by email - user, err := uc.userRepo.GetByEmail(ctx, input.Email) + // One spelling of an address, as sign-up and reset use (#688). + email := domain.NormaliseEmail(input.Email) + + user, err := uc.userRepo.GetByEmail(ctx, email) if err != nil { return nil, fmt.Errorf("authentication failed") } + + // Every refusal for bad credentials costs one password comparison (#688). + // Answering an unknown address or a disabled account before hashing made + // those replies several times faster than a wrong password, and the clock + // told an attacker which addresses held an account. if user == nil { + uc.passwordHasher.Verify(uc.dummyHash, input.Password) return nil, domain.NewValidationError("invalid credentials") } - - // Check if user is active + passwordOK := uc.passwordHasher.Verify(user.Password, input.Password) if !user.IsActive { return nil, domain.NewValidationError("account is disabled") } - - // Verify password using Argon2id (OWASP recommended) - if !uc.passwordHasher.Verify(user.Password, input.Password) { + if !passwordOK { return nil, domain.NewValidationError("invalid credentials") } diff --git a/backend/internal/application/auth/login_throttle_test.go b/backend/internal/application/auth/login_throttle_test.go new file mode 100644 index 000000000..2d03addbc --- /dev/null +++ b/backend/internal/application/auth/login_throttle_test.go @@ -0,0 +1,118 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package auth + +import ( + "context" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/opendefender/openrisk/internal/domain" +) + +// exactUsers matches the address byte for byte, as the database used to, and +// records what it was asked for. +type exactUsers struct { + *loginUsers + asked []string +} + +func (e *exactUsers) GetByEmail(_ context.Context, email string) (*domain.User, error) { + e.asked = append(e.asked, email) + if e.user != nil && e.user.Email == email { + return e.user, nil + } + return nil, nil +} + +// countingHasher records every hash Verify was asked to compare against. +type countingHasher struct { + fakeHasher + mu sync.Mutex + verified []string +} + +func (h *countingHasher) Verify(hash, plain string) bool { + h.mu.Lock() + h.verified = append(h.verified, hash) + h.mu.Unlock() + return h.fakeHasher.Verify(hash, plain) +} + +func newThrottleHarness(t *testing.T) (*LoginUseCase, *loginUsers, *countingHasher) { + t.Helper() + base, users, _, _ := newLoginHarness(t, domain.RoleUser) + hasher := &countingHasher{} + uc := NewLoginUseCase(users, base.tokenManager, hasher).WithClock(base.now) + return uc, users, hasher +} + +// #688 reproduction 4: login is reached by the normalised address. +func TestLogin_EmailCaseInsensitive(t *testing.T) { + base, users, _, _ := newLoginHarness(t, domain.RoleUser) + exact := &exactUsers{loginUsers: users} + uc := NewLoginUseCase(exact, base.tokenManager, fakeHasher{}).WithClock(base.now) + + out, err := uc.Execute(context.Background(), LoginInput{ + Email: " Admin@OpenDefender.IO ", Password: "Ancre-Vitrail7-Cobalt", + }) + require.NoError(t, err) + require.NotNil(t, out.TokenPair) + assert.Equal(t, []string{"admin@opendefender.io"}, exact.asked) +} + +// #688 reproduction 2: an unknown address used to answer before any hashing, +// and a disabled account likewise, so the response time told which addresses +// held an account. Every refusal for bad credentials now costs one comparison. +func TestLogin_UnknownAddressCostsAHashComparison(t *testing.T) { + ctx := context.Background() + + t.Run("unknown address", func(t *testing.T) { + uc, _, hasher := newThrottleHarness(t) + _, err := uc.Execute(ctx, LoginInput{Email: "nobody@opendefender.io", Password: "whatever"}) + require.Error(t, err) + require.Len(t, hasher.verified, 1, "one comparison, as for a real account") + assert.Equal(t, "hashed:"+loginTimingDummyPassword, hasher.verified[0], + "compared against a hash made with the live hasher, so it costs the same") + }) + + t.Run("disabled account", func(t *testing.T) { + uc, users, hasher := newThrottleHarness(t) + users.user.IsActive = false + _, err := uc.Execute(ctx, LoginInput{Email: users.user.Email, Password: "Ancre-Vitrail7-Cobalt"}) + require.Error(t, err, "a disabled account is refused even with the right password") + assert.Equal(t, []string{users.user.Password}, hasher.verified) + }) + + t.Run("wrong password", func(t *testing.T) { + uc, users, hasher := newThrottleHarness(t) + _, err := uc.Execute(ctx, LoginInput{Email: users.user.Email, Password: "wrong"}) + require.Error(t, err) + assert.Len(t, hasher.verified, 1) + }) +} + +func TestLogin_Success(t *testing.T) { + uc, users, _ := newThrottleHarness(t) + out, err := uc.Execute(context.Background(), LoginInput{Email: users.user.Email, Password: "Ancre-Vitrail7-Cobalt"}) + require.NoError(t, err) + assert.NotNil(t, out.TokenPair) +} + +func TestLogin_NotFound(t *testing.T) { + uc, _, _ := newThrottleHarness(t) + _, err := uc.Execute(context.Background(), LoginInput{Email: "nobody@opendefender.io", Password: "x"}) + assert.ErrorIs(t, err, domain.ErrValidation) +} + +func TestLogin_Unauthorized(t *testing.T) { + uc, users, _ := newThrottleHarness(t) + _, err := uc.Execute(context.Background(), LoginInput{Email: users.user.Email, Password: "wrong"}) + assert.ErrorIs(t, err, domain.ErrValidation) +} diff --git a/backend/internal/infrastructure/repository/gorm_user_repository.go b/backend/internal/infrastructure/repository/gorm_user_repository.go index 7e8448857..f42faae93 100644 --- a/backend/internal/infrastructure/repository/gorm_user_repository.go +++ b/backend/internal/infrastructure/repository/gorm_user_repository.go @@ -10,6 +10,7 @@ import ( "github.com/google/uuid" "gorm.io/gorm" + "gorm.io/gorm/clause" "github.com/opendefender/openrisk/internal/domain" ) @@ -72,8 +73,24 @@ func (r *GormUserRepository) EmailsByIDs(ctx context.Context, ids []uuid.UUID) ( // GetByEmail retrieves a user by email func (r *GormUserRepository) GetByEmail(ctx context.Context, email string) (*domain.User, error) { var user domain.User + // Case-insensitive (#688). Sign-up and reset normalise the address, so login + // and every other lookup must too, or the account reached depends on the + // casing typed that day. Rows written before #687 may still hold a mixed-case + // address, possibly next to a lowercase twin: the row spelled exactly as asked + // wins, then the oldest, so the same address always reaches the same account. + // + // No lower(email) index yet; a scan of users is cheap at today's sizes. + normalised := domain.NormaliseEmail(email) // Preload Role: the login response serializes it, and the frontend needs the role name for RBAC. - err := r.db.WithContext(ctx).Preload("Role").Where("email = ?", email).First(&user).Error + err := r.db.WithContext(ctx).Preload("Role"). + Where("LOWER(email) = ?", normalised). + Order(clause.OrderBy{Expression: clause.Expr{ + SQL: "CASE WHEN email = ? THEN 0 ELSE 1 END, created_at ASC, id ASC", + Vars: []interface{}{email}, + }}). + // Take, not First: First appends its own ORDER BY id and the merge + // drops the expression above. + Take(&user).Error if err != nil { if err == gorm.ErrRecordNotFound { return nil, nil diff --git a/backend/internal/infrastructure/repository/gorm_user_repository_email_test.go b/backend/internal/infrastructure/repository/gorm_user_repository_email_test.go new file mode 100644 index 000000000..156365f18 --- /dev/null +++ b/backend/internal/infrastructure/repository/gorm_user_repository_email_test.go @@ -0,0 +1,95 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package repository + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + + "github.com/opendefender/openrisk/internal/domain" +) + +func setupUserEmailRepo(t *testing.T) (*GormUserRepository, *gorm.DB) { + t.Helper() + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + require.NoError(t, err) + // Hand-written: both models carry Postgres-only defaults sqlite rejects. The + // email column is unique and case-sensitive, as in production. + for _, ddl := range []string{ + `CREATE TABLE roles (id TEXT PRIMARY KEY, name TEXT, description TEXT, + permissions TEXT, created_at DATETIME, updated_at DATETIME)`, + `CREATE TABLE users (id TEXT PRIMARY KEY, email TEXT NOT NULL UNIQUE, + username TEXT NOT NULL UNIQUE, password TEXT, full_name TEXT, bio TEXT, + phone TEXT, department TEXT, timezone TEXT, role_id TEXT, is_active BOOLEAN, + avatar_url TEXT, avatar_key TEXT, locale TEXT, date_format TEXT, + theme_mode TEXT, last_login DATETIME, tenant_id TEXT, default_org_id TEXT, + created_by_id TEXT, created_at DATETIME, updated_at DATETIME, + deleted_at DATETIME)`, + } { + require.NoError(t, db.Exec(ddl).Error) + } + return NewGormUserRepository(db), db +} + +func insertUser(t *testing.T, db *gorm.DB, email string, created time.Time) uuid.UUID { + t.Helper() + id := uuid.New() + require.NoError(t, db.Create(&domain.User{ + ID: id, Email: email, Username: id.String()[:8], Password: "x", + IsActive: true, CreatedAt: created, + }).Error) + return id +} + +// #688: login used to match the address byte for byte, while sign-up and reset +// normalised it, so the account depended on the casing typed that day. +func TestGetByEmail_CaseInsensitive(t *testing.T) { + repo, db := setupUserEmailRepo(t) + id := insertUser(t, db, "awa.diallo@e2e.test", time.Now()) + + for _, typed := range []string{"awa.diallo@e2e.test", "Awa.Diallo@E2E.test", "AWA.DIALLO@E2E.TEST"} { + u, err := repo.GetByEmail(context.Background(), typed) + require.NoError(t, err) + require.NotNil(t, u, typed) + assert.Equal(t, id, u.ID, typed) + } +} + +// Rows written before #687 may still hold a mixed-case address, and may even +// collide with a lowercase twin. The lookup must find the legacy row, and must +// pick the same row every time when there are two. +func TestGetByEmail_LegacyMixedCaseRows(t *testing.T) { + repo, db := setupUserEmailRepo(t) + ctx := context.Background() + t0 := time.Now().Add(-48 * time.Hour) + + legacy := insertUser(t, db, "Karim@Alpha.test", t0) + u, err := repo.GetByEmail(ctx, "karim@alpha.test") + require.NoError(t, err) + require.NotNil(t, u) + assert.Equal(t, legacy, u.ID, "a legacy mixed-case row is found by its normalised address") + + exact := insertUser(t, db, "karim@alpha.test", t0.Add(time.Hour)) + for i := 0; i < 3; i++ { + u, err = repo.GetByEmail(ctx, "karim@alpha.test") + require.NoError(t, err) + assert.Equal(t, exact, u.ID, "the row spelled exactly as asked wins") + } + u, err = repo.GetByEmail(ctx, "KARIM@alpha.test") + require.NoError(t, err) + assert.Equal(t, legacy, u.ID, "with no exact spelling, the oldest row wins") + + u, err = repo.GetByEmail(ctx, "nobody@alpha.test") + require.NoError(t, err) + assert.Nil(t, u) +} From 5c6b18102ed094d75fda016408182763af9165f0 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:46:05 +0100 Subject: [PATCH 5/7] fix(auth): pause sign-in on an address after ten failures from any source (#688) Failed sign-ins are counted per normalised address, hashed, in the store #689 added. The tenth failure locks the address for fifteen minutes; the lock is not extended by refused attempts and a password reset ends it. A locked address answers 429 LOGIN_LOCKED with Retry-After, and every failed sign-in is audited with the address hashed. Signed-off-by: alex-dembele --- backend/cmd/server/main.go | 9 +- backend/internal/application/auth/login.go | 12 ++ .../internal/application/auth/login_limits.go | 126 ++++++++++++++++++ .../application/auth/login_throttle_test.go | 125 +++++++++++++++++ .../application/auth/password_reset.go | 6 + backend/internal/handler/auth/handler.go | 22 ++- .../handler/auth/login_throttle_e2e_test.go | 93 +++++++++++++ 7 files changed, 390 insertions(+), 3 deletions(-) create mode 100644 backend/internal/application/auth/login_limits.go create mode 100644 backend/internal/handler/auth/login_throttle_e2e_test.go diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index 3718df6a3..47a70516e 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -677,10 +677,15 @@ func main() { // OR26-03 — the tenant's grace window. Deployment decides WHO is privileged; // each tenant decides HOW LONG they may defer (default 7 days). mfaPolicyRepo := repository.NewGormMFAPolicyRepository(database.DB) + // #688 — ten failed sign-ins on one address, from any source, pause it for + // fifteen minutes; a password reset ends the pause. Counted in Redis (the + // store #689 built) so the budget holds across instances. + loginAttemptStore := authmfa.NewAttemptStore(redisClientInstance) loginUseCase := auth.NewLoginUseCase(userRepo, tokenManager, passwordHasher). WithMFA(mfaRepo). RequireMFAForRoles(mfaRequiredRoles, mfaRequiredBusinessRoles). - WithMFAPolicies(mfaPolicyRepo) + WithMFAPolicies(mfaPolicyRepo). + WithAttemptLimits(loginAttemptStore) registerUseCase := auth.NewRegisterUseCase(userRepo, orgRepo, notificationService, passwordHasher). // Anchors t0 for the time-to-Aha histogram. WithActivation(activationRecorder). @@ -728,7 +733,7 @@ func main() { requestResetUseCase := auth.NewRequestPasswordResetUseCase(userRepo, passwordResetRepo, securityMailer) confirmResetUseCase := auth.NewConfirmPasswordResetUseCase( userRepo, passwordResetRepo, passwordHasher, passwordPolicy, tokenManager, securityMailer, - ) + ).WithLoginLockClearer(loginAttemptStore) passwordHandler := authhandler.NewPasswordHandler( requestResetUseCase, confirmResetUseCase, passwordPolicy, appBaseURL, authAudit, ).WithChangePassword(auth.NewChangePasswordUseCase( diff --git a/backend/internal/application/auth/login.go b/backend/internal/application/auth/login.go index a99a90579..4e8dd958f 100644 --- a/backend/internal/application/auth/login.go +++ b/backend/internal/application/auth/login.go @@ -76,6 +76,8 @@ type LoginUseCase struct { // dummyHash is compared against when there is no account to compare with, // so an unknown address costs what a real one does (#688). dummyHash string + // attempts bounds failed sign-ins per address (#688). Optional. + attempts LoginAttemptStore } // PasswordUpgrader is the half of the hasher that knows whether a stored hash @@ -188,6 +190,12 @@ func (uc *LoginUseCase) Execute(ctx context.Context, input LoginInput) (*LoginOu // One spelling of an address, as sign-up and reset use (#688). email := domain.NormaliseEmail(input.Email) + // A locked address is refused before the database or the hasher is asked + // anything, so the refusal costs the same whether the account exists. + if left := uc.lockedFor(ctx, email); left > 0 { + return nil, &LoginLockedError{RetryAfter: left} + } + user, err := uc.userRepo.GetByEmail(ctx, email) if err != nil { return nil, fmt.Errorf("authentication failed") @@ -199,15 +207,19 @@ func (uc *LoginUseCase) Execute(ctx context.Context, input LoginInput) (*LoginOu // told an attacker which addresses held an account. if user == nil { uc.passwordHasher.Verify(uc.dummyHash, input.Password) + uc.recordFailure(ctx, email) return nil, domain.NewValidationError("invalid credentials") } passwordOK := uc.passwordHasher.Verify(user.Password, input.Password) if !user.IsActive { + uc.recordFailure(ctx, email) return nil, domain.NewValidationError("account is disabled") } if !passwordOK { + uc.recordFailure(ctx, email) return nil, domain.NewValidationError("invalid credentials") } + uc.clearFailures(ctx, email) // The password is correct and the plaintext is in hand: if the stored hash // was written with a lower Argon2id cost than today's, rewrite it now. diff --git a/backend/internal/application/auth/login_limits.go b/backend/internal/application/auth/login_limits.go new file mode 100644 index 000000000..fa2096721 --- /dev/null +++ b/backend/internal/application/auth/login_limits.go @@ -0,0 +1,126 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package auth + +import ( + "context" + "log" + "time" + + "github.com/opendefender/openrisk/internal/domain" +) + +// Per-address limits on password sign-in (#688). The per-IP limiter on the +// route does nothing against guessing one account from many addresses; this +// does, because the key is the address being signed into. Same shape and +// numbers as the MFA challenge lock (#689). +const ( + // LoginMaxFailuresPerAddress is how many bad-credential refusals one + // address takes, from any source, before it is locked. + LoginMaxFailuresPerAddress = 10 + // LoginFailureWindow is counted from the first failure. + LoginFailureWindow = 15 * time.Minute + // LoginLockDuration is fixed: attempts during the lock neither count nor + // extend it, so a lock never outlasts this. A password reset ends it early. + LoginLockDuration = 15 * time.Minute +) + +// LoginAttemptStore holds the per-address counters and locks. It is the part of +// MFAAttemptStore login needs, so authmfa.AttemptStore serves both; it must be +// shared across instances, or each replica multiplies the budget. +type LoginAttemptStore interface { + Increment(ctx context.Context, key string, ttl time.Duration) (int64, error) + Lock(ctx context.Context, key string, ttl time.Duration) (bool, error) + LockedFor(ctx context.Context, key string) (time.Duration, error) + Delete(ctx context.Context, key string) error +} + +// LoginLockedError means the address refuses sign-in for a while. It is +// returned without the password being looked at. +type LoginLockedError struct { + RetryAfter time.Duration +} + +func (e *LoginLockedError) Error() string { + return "sign-in is paused for this address after repeated failures" +} + +// The keys carry the address hashed, never in clear. They are computed for +// every address, known or not, so a lock says nothing about whether an account +// exists. +func loginFailKey(email string) string { return "login:fail:" + domain.HashEmailForReset(email) } +func loginLockKey(email string) string { return "login:lock:" + domain.HashEmailForReset(email) } + +// WithAttemptLimits turns the per-address limits on. Without it login behaves +// as it did before #688; main.go always wires it. +func (uc *LoginUseCase) WithAttemptLimits(store LoginAttemptStore) *LoginUseCase { + uc.attempts = store + return uc +} + +// lockedFor reports how long the address stays locked, 0 when it is not. +// +// A store error lets the attempt through. The store already falls back to +// per-instance memory when Redis fails, so an error here is a bug, and turning +// it into a lockout of every account would be the worse failure. +func (uc *LoginUseCase) lockedFor(ctx context.Context, email string) time.Duration { + if uc.attempts == nil { + return 0 + } + left, err := uc.attempts.LockedFor(ctx, loginLockKey(email)) + if err != nil { + log.Printf("login: lock check failed: %v", err) + return 0 + } + return left +} + +// recordFailure counts one bad-credential refusal and locks the address when +// the count reaches the limit. The attempt that reaches it is still answered +// as a plain failure: it was evaluated. The next one is refused. +func (uc *LoginUseCase) recordFailure(ctx context.Context, email string) { + if uc.attempts == nil { + return + } + n, err := uc.attempts.Increment(ctx, loginFailKey(email), LoginFailureWindow) + if err != nil { + log.Printf("login: failure count not recorded: %v", err) + return + } + if n < LoginMaxFailuresPerAddress { + return + } + if _, err := uc.attempts.Lock(ctx, loginLockKey(email), LoginLockDuration); err != nil { + log.Printf("login: lock not set: %v", err) + return + } + // The lock carries the refusal now; a fresh count starts after it. + _ = uc.attempts.Delete(ctx, loginFailKey(email)) +} + +// clearFailures forgets the failures on an address after a success. +func (uc *LoginUseCase) clearFailures(ctx context.Context, email string) { + if uc.attempts == nil { + return + } + _ = uc.attempts.Delete(ctx, loginFailKey(email)) +} + +// WithLoginLockClearer lets a completed password reset end a sign-in lock on +// the account at once. The reset proves control of the mailbox, which is what +// the owner of an address someone else is hammering needs to get back in. +func (uc *ConfirmPasswordResetUseCase) WithLoginLockClearer(store LoginAttemptStore) *ConfirmPasswordResetUseCase { + uc.loginAttempts = store + return uc +} + +func (uc *ConfirmPasswordResetUseCase) clearLoginLock(ctx context.Context, email string) { + if uc.loginAttempts == nil { + return + } + _ = uc.loginAttempts.Delete(ctx, loginLockKey(email)) + _ = uc.loginAttempts.Delete(ctx, loginFailKey(email)) +} diff --git a/backend/internal/application/auth/login_throttle_test.go b/backend/internal/application/auth/login_throttle_test.go index 2d03addbc..9056a2d9c 100644 --- a/backend/internal/application/auth/login_throttle_test.go +++ b/backend/internal/application/auth/login_throttle_test.go @@ -7,13 +7,17 @@ package auth import ( "context" + "errors" "sync" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/opendefender/openrisk/internal/domain" + "github.com/opendefender/openrisk/internal/infrastructure/authmfa" + "github.com/opendefender/openrisk/pkg/pwpolicy" ) // exactUsers matches the address byte for byte, as the database used to, and @@ -116,3 +120,124 @@ func TestLogin_Unauthorized(t *testing.T) { _, err := uc.Execute(context.Background(), LoginInput{Email: users.user.Email, Password: "wrong"}) assert.ErrorIs(t, err, domain.ErrValidation) } + +func failLogin(t *testing.T, uc *LoginUseCase, email string) error { + t.Helper() + _, err := uc.Execute(context.Background(), LoginInput{Email: email, Password: "wrong"}) + require.Error(t, err) + return err +} + +// #688 criterion 3: wrong passwords on one address are bounded across every +// source, not per IP. +func TestLogin_PerAccountBackoff(t *testing.T) { + ctx := context.Background() + right := "Ancre-Vitrail7-Cobalt" + + t.Run("ten failures lock the address, even against the right password", func(t *testing.T) { + uc, users, hasher := newThrottleHarness(t) + uc.WithAttemptLimits(authmfa.NewMemoryAttemptStore()) + + for i := 1; i <= LoginMaxFailuresPerAddress; i++ { + err := failLogin(t, uc, users.user.Email) + var locked *LoginLockedError + require.False(t, errors.As(err, &locked), "failure %d is evaluated, not refused", i) + } + + before := len(hasher.verified) + _, err := uc.Execute(ctx, LoginInput{Email: users.user.Email, Password: right}) + var locked *LoginLockedError + require.True(t, errors.As(err, &locked), "got %v", err) + assert.InDelta(t, LoginLockDuration.Seconds(), locked.RetryAfter.Seconds(), 1) + assert.Len(t, hasher.verified, before, "a refused attempt never reaches the hasher") + }) + + t.Run("the casing of the address does not buy more attempts", func(t *testing.T) { + uc, users, _ := newThrottleHarness(t) + uc.WithAttemptLimits(authmfa.NewMemoryAttemptStore()) + for i := 0; i < LoginMaxFailuresPerAddress; i++ { + failLogin(t, uc, map[bool]string{true: "ADMIN@opendefender.io", false: "admin@OpenDefender.io"}[i%2 == 0]) + } + _, err := uc.Execute(ctx, LoginInput{Email: users.user.Email, Password: right}) + var locked *LoginLockedError + assert.True(t, errors.As(err, &locked)) + }) + + t.Run("an unknown address locks exactly like a real one", func(t *testing.T) { + uc, _, _ := newThrottleHarness(t) + uc.WithAttemptLimits(authmfa.NewMemoryAttemptStore()) + for i := 0; i < LoginMaxFailuresPerAddress; i++ { + failLogin(t, uc, "nobody@opendefender.io") + } + var locked *LoginLockedError + assert.True(t, errors.As(failLogin(t, uc, "nobody@opendefender.io"), &locked), + "otherwise a lock would tell which addresses hold an account") + }) + + t.Run("a success clears the count", func(t *testing.T) { + uc, users, _ := newThrottleHarness(t) + uc.WithAttemptLimits(authmfa.NewMemoryAttemptStore()) + for i := 0; i < LoginMaxFailuresPerAddress-1; i++ { + failLogin(t, uc, users.user.Email) + } + _, err := uc.Execute(ctx, LoginInput{Email: users.user.Email, Password: right}) + require.NoError(t, err) + for i := 0; i < LoginMaxFailuresPerAddress-1; i++ { + failLogin(t, uc, users.user.Email) + } + _, err = uc.Execute(ctx, LoginInput{Email: users.user.Email, Password: right}) + assert.NoError(t, err, "nine failures after a success are not ten") + }) + + t.Run("the lock is bounded: refused attempts do not extend it", func(t *testing.T) { + uc, users, _ := newThrottleHarness(t) + store := authmfa.NewMemoryAttemptStore() + uc.WithAttemptLimits(store) + for i := 0; i < LoginMaxFailuresPerAddress; i++ { + failLogin(t, uc, users.user.Email) + } + first, err := store.LockedFor(ctx, loginLockKey(users.user.Email)) + require.NoError(t, err) + time.Sleep(1100 * time.Millisecond) + for i := 0; i < 5; i++ { + failLogin(t, uc, users.user.Email) + } + later, err := store.LockedFor(ctx, loginLockKey(users.user.Email)) + require.NoError(t, err) + assert.Less(t, later, first, "the lock only runs down") + assert.LessOrEqual(t, first, LoginLockDuration) + }) +} + +// A password reset proves control of the mailbox, which is what the owner of a +// locked address needs to get back in at once. +func TestConfirmPasswordReset_ClearsLoginLock(t *testing.T) { + ctx := context.Background() + user := activeUser("real@opendefender.io") + user.Password = "hashed:old" + store := authmfa.NewMemoryAttemptStore() + + users, tokens := newFakeResetUsers(user), &fakeResetTokens{} + uc, _, _ := newThrottleHarness(t) + uc.userRepo = &loginUsers{user: user} + uc.WithAttemptLimits(store) + for i := 0; i < LoginMaxFailuresPerAddress; i++ { + failLogin(t, uc, "Real@OpenDefender.io") + } + locked, err := store.LockedFor(ctx, loginLockKey(user.Email)) + require.NoError(t, err) + require.Positive(t, locked) + + secret := issueToken(t, users, tokens, user.Email) + confirm := NewConfirmPasswordResetUseCase(users, tokens, fakeHasher{}, pwpolicy.New(), &fakeRevoker{}, &fakeMailer{}). + WithLoginLockClearer(store) + _, err = confirm.Execute(ctx, ConfirmPasswordResetInput{Token: secret, NewPassword: "Ancre-Vitrail7-Cobalt"}) + require.NoError(t, err) + + locked, err = store.LockedFor(ctx, loginLockKey(user.Email)) + require.NoError(t, err) + assert.Zero(t, locked) + n, err := store.Count(ctx, loginFailKey(user.Email)) + require.NoError(t, err) + assert.Zero(t, n) +} diff --git a/backend/internal/application/auth/password_reset.go b/backend/internal/application/auth/password_reset.go index 3a37b18fe..90bf921e1 100644 --- a/backend/internal/application/auth/password_reset.go +++ b/backend/internal/application/auth/password_reset.go @@ -215,6 +215,8 @@ type ConfirmPasswordResetUseCase struct { policy PasswordAssessor revoker SessionRevoker mailer ResetMailer + // loginAttempts lets a completed reset end a sign-in lock (#688). Optional. + loginAttempts LoginAttemptStore } // NewConfirmPasswordResetUseCase wires the confirm side. @@ -309,6 +311,10 @@ func (uc *ConfirmPasswordResetUseCase) Execute(ctx context.Context, input Confir // attacker requested one too, it must not survive the legitimate reset. _ = uc.tokens.InvalidateOutstandingForUser(ctx, user.ID, record.ID) + // The new password is in place: whoever was hammering the old one has + // nothing left to guess, and the owner must not wait out their lock. + uc.clearLoginLock(ctx, user.Email) + // --- End every session ---------------------------------------------------- out := &ConfirmPasswordResetOutput{} if uc.revoker != nil { diff --git a/backend/internal/handler/auth/handler.go b/backend/internal/handler/auth/handler.go index aa5b788d7..443cd13db 100644 --- a/backend/internal/handler/auth/handler.go +++ b/backend/internal/handler/auth/handler.go @@ -8,7 +8,9 @@ package auth import ( "context" "errors" + "strconv" "strings" + "time" "github.com/gofiber/fiber/v2" "github.com/google/uuid" @@ -200,7 +202,25 @@ func (h *Handler) Login(c *fiber.Ctx) error { }) if err != nil { - reason := "authentication failed" + // Every failure names the address it was against, hashed (#688): enough + // to see one address being hammered from many sources, without writing + // addresses that may not belong to any account into the audit trail. + addr := " email_sha256=" + domain.HashEmailForReset(req.Email) + + var locked *auth.LoginLockedError + if errors.As(err, &locked) { + reason := "login_locked" + addr + h.logAudit(c, nil, nil, coreauth.AuditActionLogin, false, &reason) + wait := int((locked.RetryAfter + time.Second - 1) / time.Second) + c.Set(fiber.HeaderRetryAfter, strconv.Itoa(wait)) + return c.Status(fiber.StatusTooManyRequests).JSON(fiber.Map{ + "error": "Too many failed sign-ins for this address", + "code": "LOGIN_LOCKED", + "retry_after": wait, + }) + } + + reason := "authentication failed" + addr h.logAudit(c, nil, nil, coreauth.AuditActionLogin, false, &reason) return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{ "error": "Authentication failed", diff --git a/backend/internal/handler/auth/login_throttle_e2e_test.go b/backend/internal/handler/auth/login_throttle_e2e_test.go new file mode 100644 index 000000000..731d22df4 --- /dev/null +++ b/backend/internal/handler/auth/login_throttle_e2e_test.go @@ -0,0 +1,93 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package auth_test + +// #688 over HTTP: the per-address lock through the real login route, the real +// GORM user repository, the real hasher and the audit trail. + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/gofiber/fiber/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + appauth "github.com/opendefender/openrisk/internal/application/auth" + coreauth "github.com/opendefender/openrisk/internal/auth" + "github.com/opendefender/openrisk/internal/domain" + authhandler "github.com/opendefender/openrisk/internal/handler/auth" + "github.com/opendefender/openrisk/internal/infrastructure/authmfa" + "github.com/opendefender/openrisk/internal/infrastructure/repository" + "github.com/opendefender/openrisk/internal/testsupport/sqliteschema" +) + +func TestLoginThrottle_LockedAddressAnswers429AndEveryFailureIsAudited(t *testing.T) { + hasher := coreauth.NewArgon2idPasswordHasherWithParams(coreauth.Argon2idParams{Time: 1, Memory: 1024, Threads: 1}) + stored, err := hasher.Hash(migrationPassword) + require.NoError(t, err) + f := newMigrationFixture(t, stored) + + require.NoError(t, f.db.Exec(`CREATE TABLE auth_audit_logs (id TEXT PRIMARY KEY)`).Error) + require.NoError(t, sqliteschema.Reconcile(f.db, "auth_audit_logs", &domain.AuthAuditLog{})) + audit := coreauth.NewAuditService(repository.NewGormAuthAuditLogRepository(f.db)) + + loginUC := appauth.NewLoginUseCase(repository.NewGormUserRepository(f.db), coreauth.NewTokenManager(f.db, nil), hasher). + WithAttemptLimits(authmfa.NewMemoryAttemptStore()) + f.app = newLoginApp(authhandler.NewHandler(loginUC, nil, nil, nil, hasher, audit)) + + for i := 1; i <= appauth.LoginMaxFailuresPerAddress; i++ { + status, _ := f.login(t, "wrong-"+strconv.Itoa(i)) + require.Equal(t, http.StatusUnauthorized, status, "failure %d", i) + } + + // Correct password, but the address is locked. + resp := postLogin(t, f, strings.ToUpper(f.user.Email), migrationPassword) + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode) + wait, err := strconv.Atoi(resp.Header.Get("Retry-After")) + require.NoError(t, err) + assert.InDelta(t, appauth.LoginLockDuration.Seconds(), float64(wait), 2) + var body struct { + Code string `json:"code"` + RetryAfter int `json:"retry_after"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) + assert.Equal(t, "LOGIN_LOCKED", body.Code) + assert.Equal(t, wait, body.RetryAfter) + + var reasons []string + require.NoError(t, f.db.Model(&domain.AuthAuditLog{}).Where("success = ?", false). + Order("created_at").Pluck("failure_reason", &reasons).Error) + require.Len(t, reasons, appauth.LoginMaxFailuresPerAddress+1) + hashed := "email_sha256=" + domain.HashEmailForReset(f.user.Email) + for _, r := range reasons { + assert.Contains(t, r, hashed) + assert.NotContains(t, r, f.user.Email, "never the address in clear") + } + assert.True(t, strings.HasPrefix(reasons[len(reasons)-1], "login_locked ")) +} + +func newLoginApp(h *authhandler.Handler) *fiber.App { + app := fiber.New() + app.Post("/api/v1/auth/login", h.Login) + return app +} + +func postLogin(t *testing.T, f *migrationFixture, email, password string) *http.Response { + t.Helper() + raw, err := json.Marshal(map[string]string{"email": email, "password": password}) + require.NoError(t, err) + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", strings.NewReader(string(raw))) + req.Header.Set("Content-Type", "application/json") + resp, err := f.app.Test(req, -1) + require.NoError(t, err) + t.Cleanup(func() { _ = resp.Body.Close() }) + return resp +} From 0eb306b820691c5568a5d469c6b17552e975f0e5 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:49:22 +0100 Subject: [PATCH 6/7] fix(auth): tell a throttled person how long to wait on sign-in and sign-up (#688) Signed-off-by: alex-dembele --- frontend/src/features/auth/AuthScreen.tsx | 31 +++- .../auth/__tests__/authThrottle.test.tsx | 138 ++++++++++++++++++ frontend/src/features/auth/authStrings.ts | 12 ++ 3 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 frontend/src/features/auth/__tests__/authThrottle.test.tsx diff --git a/frontend/src/features/auth/AuthScreen.tsx b/frontend/src/features/auth/AuthScreen.tsx index 39cc4dbec..07d9cc7d5 100644 --- a/frontend/src/features/auth/AuthScreen.tsx +++ b/frontend/src/features/auth/AuthScreen.tsx @@ -57,6 +57,26 @@ function isPasswordResetRequired(err: unknown): boolean { return body?.code === 'password_reset_required'; } +/** + * For a throttled call (429), the whole minutes to wait, rounded up; null when + * the server gave no figure. Undefined when the error is not a throttle. + * + * The body's retry_after is read first: a cross-origin SPA cannot see the + * Retry-After header unless CORS exposes it (#688). + */ +function throttleMinutes(err: unknown): number | null | undefined { + if (!axios.isAxiosError(err) || err.response?.status !== 429) return undefined; + const body = err.response.data as { retry_after?: unknown } | undefined; + const header = Number(err.response.headers?.['retry-after']); + const seconds = + typeof body?.retry_after === 'number' && body.retry_after > 0 + ? body.retry_after + : Number.isFinite(header) && header > 0 + ? header + : null; + return seconds === null ? null : Math.max(1, Math.ceil(seconds / 60)); +} + function LoginForm({ onRegister }: { onRegister: () => void }) { const navigate = useNavigate(); const [params, setParams] = useSearchParams(); @@ -137,9 +157,12 @@ function LoginForm({ onRegister }: { onRegister: () => void }) { } catch (err) { // The server only says password_reset_required to someone who typed the // right password; everything else keeps the one generic message. + const wait = throttleMinutes(err); if (isPasswordResetRequired(err)) { setResetRequired(true); fail(copy.signInResetRequired); + } else if (wait !== undefined) { + fail(copy.tooManyAttempts(wait)); } else { fail(copy.signInFailed); } @@ -635,9 +658,15 @@ function RegisterForm({ onLogin }: { onLogin: () => void }) { const body = axios.isAxiosError(err) ? (err.response?.data as { error?: string; code?: string } | undefined) : undefined; + const wait = throttleMinutes(err); // Only an email conflict may be told as one (#687). Any other 409 — a // username taken, say — shows what the server actually said. - if (status === 409 && body?.code && body.code !== 'EMAIL_ALREADY_REGISTERED') { + if (wait !== undefined) { + // Sign-up or the sign-in right after it was throttled (#688). Saying + // "registration failed" here sent people to retry, which only extended + // the wait. + setError(copy.tooManyAttempts(wait)); + } else if (status === 409 && body?.code && body.code !== 'EMAIL_ALREADY_REGISTERED') { setError(body.error || copy.registerFailed); } else if (status === 409) { setError(copy.registerEmailExists); diff --git a/frontend/src/features/auth/__tests__/authThrottle.test.tsx b/frontend/src/features/auth/__tests__/authThrottle.test.tsx new file mode 100644 index 000000000..3fcf708be --- /dev/null +++ b/frontend/src/features/auth/__tests__/authThrottle.test.tsx @@ -0,0 +1,138 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// +// #688: a throttled sign-in or sign-up used to read "incorrect password" or +// "registration failed", which sent people to retry and only extended the wait. +// The screen now says it was throttled and for how long, in both languages. + +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter } from 'react-router'; + +const login = vi.fn(); +const post = vi.fn(); + +vi.mock('../../../lib/api', () => ({ + api: { post: (...a: unknown[]) => post(...a), defaults: { baseURL: '' } }, +})); +vi.mock('../authService', () => ({ + challengeMFA: vi.fn(), + setupMFA: vi.fn(), + verifyMFA: vi.fn(), + requestPasswordReset: vi.fn(), +})); +vi.mock('../../../hooks/useAuthStore', () => { + const state = { + user: undefined, + login: (...a: unknown[]) => login(...a), + }; + const useAuthStore = (sel?: (s: typeof state) => unknown) => (sel ? sel(state) : state); + useAuthStore.getState = () => state; + return { useAuthStore }; +}); + +import { AuthScreen } from '../AuthScreen'; +import { useUIStore } from '../../../store/uiStore'; + +// Shaped like an AxiosError: an Error carrying isAxiosError and a response. +function throttled(data: Record, headers: Record = {}) { + return Object.assign(new Error('HTTP 429'), { + isAxiosError: true, + response: { status: 429, data, headers }, + }); +} + +async function signIn() { + render( + + + , + ); + const user = userEvent.setup(); + await user.type(screen.getByTestId('login-email'), 'awa.diallo@example.com'); + await user.type(screen.getByTestId('login-password'), 'Ancre-Vitrail7-Cobalt'); + await user.click(screen.getByTestId('login-submit')); +} + +async function signUp() { + render( + + + , + ); + const user = userEvent.setup(); + await user.type(screen.getByTestId('register-name'), 'Alix Mensah'); + await user.type(screen.getByTestId('register-email'), 'alix@example.com'); + await user.type(screen.getByTestId('register-password'), 'MotDePasse2026!'); + await user.click(screen.getByTestId('register-submit')); +} + +describe('throttled sign-in and sign-up', () => { + beforeEach(() => { + vi.clearAllMocks(); + useUIStore.setState({ lang: 'fr' }); + post.mockResolvedValue({ data: {} }); + }); + + it('sign-in locked for the address: says how long, in French', async () => { + login.mockRejectedValue(throttled({ code: 'LOGIN_LOCKED', retry_after: 900 })); + await signIn(); + expect( + await screen.findByText('Trop de tentatives. Réessayez dans 15 minutes.'), + ).toBeInTheDocument(); + expect(screen.queryByText(/mot de passe incorrect/i)).not.toBeInTheDocument(); + }); + + it('sign-in throttled per IP: says how long, in English, rounding up', async () => { + useUIStore.setState({ lang: 'en' }); + login.mockRejectedValue(throttled({ code: 'RATE_LIMITED', retry_after: 61 })); + await signIn(); + expect( + await screen.findByText('Too many attempts. Try again in 2 minutes.'), + ).toBeInTheDocument(); + }); + + it('reads the Retry-After header when the body has no figure', async () => { + login.mockRejectedValue(throttled({ error: true }, { 'retry-after': '30' })); + await signIn(); + expect( + await screen.findByText('Trop de tentatives. Réessayez dans 1 minute.'), + ).toBeInTheDocument(); + }); + + it('stays vague rather than guess when the server gives no figure', async () => { + useUIStore.setState({ lang: 'en' }); + login.mockRejectedValue(throttled({ error: true })); + await signIn(); + expect( + await screen.findByText('Too many attempts. Try again in a few minutes.'), + ).toBeInTheDocument(); + }); + + it('sign-up throttled: no longer "registration failed", in French and English', async () => { + post.mockImplementation((url: string) => + url === '/auth/register' + ? Promise.reject(throttled({ code: 'RATE_LIMITED', retry_after: 600 })) + : Promise.resolve({ data: {} }), + ); + await signUp(); + expect( + await screen.findByText('Trop de tentatives. Réessayez dans 10 minutes.'), + ).toBeInTheDocument(); + expect(screen.queryByText(/création du compte a échoué/i)).not.toBeInTheDocument(); + }); + + it('sign-up throttled in English', async () => { + useUIStore.setState({ lang: 'en' }); + post.mockImplementation((url: string) => + url === '/auth/register' + ? Promise.reject(throttled({ code: 'RATE_LIMITED', retry_after: 600 })) + : Promise.resolve({ data: {} }), + ); + await signUp(); + expect( + await screen.findByText('Too many attempts. Try again in 10 minutes.'), + ).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/features/auth/authStrings.ts b/frontend/src/features/auth/authStrings.ts index fd4e1c6ba..56419a833 100644 --- a/frontend/src/features/auth/authStrings.ts +++ b/frontend/src/features/auth/authStrings.ts @@ -224,6 +224,10 @@ export interface AuthCopy { // OAuth failures — keyed to the backend's ?error= codes. oauth: Record; oauthConflictWith: (existing: string) => string; + + // Throttled sign-in or sign-up (#688). `minutes` is null when the server did + // not say how long; the message then stays vague rather than guess. + tooManyAttempts: (minutes: number | null) => string; } /** The error codes internal/handler/oauth2_handler.go can redirect with. */ @@ -357,6 +361,10 @@ const fr: AuthCopy = { }, oauthConflictWith: (existing) => `Cette adresse se connecte déjà avec ${existing}. Utilisez ${existing} ou votre mot de passe.`, + tooManyAttempts: (minutes) => + minutes === null + ? 'Trop de tentatives. Réessayez dans quelques minutes.' + : `Trop de tentatives. Réessayez dans ${minutes} minute${minutes > 1 ? 's' : ''}.`, }; const en: AuthCopy = { @@ -467,6 +475,10 @@ const en: AuthCopy = { }, oauthConflictWith: (existing) => `This address already signs in with ${existing}. Use ${existing} or your password.`, + tooManyAttempts: (minutes) => + minutes === null + ? 'Too many attempts. Try again in a few minutes.' + : `Too many attempts. Try again in ${minutes} minute${minutes === 1 ? '' : 's'}.`, }; // Partial: a registered language without auth copy falls back in `authCopy`. From 956dfa2883a962d4bb09c9ea696f99af62f12659 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:53:20 +0100 Subject: [PATCH 7/7] docs(auth): record what changed in the #688 spec during implementation (#688) Signed-off-by: alex-dembele --- docs/688_AUTH_THROTTLE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/688_AUTH_THROTTLE.md b/docs/688_AUTH_THROTTLE.md index 74e6fa879..c9dddc6d2 100644 --- a/docs/688_AUTH_THROTTLE.md +++ b/docs/688_AUTH_THROTTLE.md @@ -89,7 +89,7 @@ pause that address for a bounded time; unknown and real addresses cost the same; - **Port** `application/auth/login_limits.go`: `LoginAttemptStore { Increment; Lock; LockedFor; Delete }` — a subset of #689's `MFAAttemptStore`, satisfied by `authmfa.AttemptStore` without changes. Keys `login:fail:` and `login:lock:`. - **Use case** `LoginUseCase.WithAttemptLimits(store)` → sentinel `ErrLoginLocked` carrying `RetryAfter` (`errors.As`). Unwired = today's behaviour; existing tests untouched. - **Use case** `ConfirmPasswordResetUseCase.WithLoginLockClearer(store)` clears both keys after the new hash is written. -- **Middleware** `RateLimit` sets `Retry-After` and the new body fields. The prefix wrapper moves to `middleware.PrefixedBackend`; `handler.PrefixedRateLimitBackend` becomes an alias, no churn elsewhere. +- **Middleware** `RateLimit` sets `Retry-After` and the new body fields. The five limiters are built by `middleware.NewAuthLimiters` so their separation is testable; the prefix wrapper moves there as `middleware.PrefixedBackend`, and `handler.PrefixedRateLimitBackend` stays as an alias. - **Handler** `Login` maps `ErrLoginLocked` → 429; writes the hashed address in every failure audit row. - **main.go**: wiring only. @@ -128,7 +128,8 @@ Live: throwaway pg + redis on spare ports, server from the repo root (live-boot - **Redis down:** buckets and locks become per-instance. - **`Retry-After` in degraded mode:** computed from the Redis fixed window; the in-memory fallback slides, so the hint may be early by up to one window. - **Audit hash:** unsalted SHA-256 of an address is dictionary-reversible for a known address list. Kept for consistency with reset; HMAC is a follow-up if wanted. -- **Legacy hashes:** accounts on a pre-upgrade hash verify at a different cost than the dummy until their next login migrates them. +- ~~Legacy hashes~~ — not a limit: the Argon2id hasher already spends full cost on any hash it cannot parse (`spendArgon2idCost`). +- **Redis keys** live under #689's `mfa-attempts:` namespace (`mfa-attempts:login:lock:`), because the store is shared. - **`LogFiber` reads `X-Forwarded-For` raw** for the audit IP (spoofable). Pre-existing, out of scope; follow-up issue. ## Plan — one commit per task, `fix(auth): … (#688)`