From 529bd6cf96d4f0821f7b31d768385aec80b4d284 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:10:53 +0100 Subject: [PATCH 1/6] fix(auth): add a shared store for MFA challenge attempts (#689) The challenge needs exact per-token and per-account counters that hold across instances. AllowRate counts in wall-clock buckets, so a five-minute challenge token can straddle two of them and get up to eight tries. The store counts from the first failure, sets locks once (so the lock is audited and mailed once), and falls back to a per-instance memory store when Redis errors instead of failing open. The spec for the issue is committed alongside. Signed-off-by: alex-dembele --- .../internal/application/auth/mfa_limits.go | 36 ++++ .../infrastructure/authmfa/attempts.go | 192 +++++++++++++++++ .../infrastructure/authmfa/attempts_test.go | 182 ++++++++++++++++ docs/689_MFA_CHALLENGE_LIMITS.md | 196 ++++++++++++++++++ 4 files changed, 606 insertions(+) create mode 100644 backend/internal/application/auth/mfa_limits.go create mode 100644 backend/internal/infrastructure/authmfa/attempts.go create mode 100644 backend/internal/infrastructure/authmfa/attempts_test.go create mode 100644 docs/689_MFA_CHALLENGE_LIMITS.md diff --git a/backend/internal/application/auth/mfa_limits.go b/backend/internal/application/auth/mfa_limits.go new file mode 100644 index 00000000..55debfe9 --- /dev/null +++ b/backend/internal/application/auth/mfa_limits.go @@ -0,0 +1,36 @@ +// 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" + "time" +) + +// MFAAttemptStore holds the counters and locks that bound guessing at the MFA +// challenge (#689). It must be shared across instances: a per-instance counter +// lets an attacker multiply the budget by the number of replicas. +type MFAAttemptStore interface { + // Increment adds one to key and returns the new count. The key expires ttl + // after its first increment; later increments do not move that deadline. + Increment(ctx context.Context, key string, ttl time.Duration) (int64, error) + // Count returns the current value of key, 0 when it does not exist. + Count(ctx context.Context, key string) (int64, error) + // Lock sets key for ttl unless it is already set, and reports whether this + // call set it. Only the call that sets the lock audits it and mails the + // owner, so a burst of refused attempts produces one notice, not fifty. + Lock(ctx context.Context, key string, ttl time.Duration) (bool, error) + // LockedFor returns how long the lock on key still holds, 0 when unlocked. + LockedFor(ctx context.Context, key string) (time.Duration, error) + // Delete removes key. Deleting a missing key is not an error. + Delete(ctx context.Context, key string) error +} + +// TokenRevoker blacklists a token's JTI until the token would have expired +// anyway. *pkg/auth.TokenBlacklistManager satisfies it. +type TokenRevoker interface { + BlacklistJTI(ctx context.Context, jti string, ttl time.Duration) error +} diff --git a/backend/internal/infrastructure/authmfa/attempts.go b/backend/internal/infrastructure/authmfa/attempts.go new file mode 100644 index 00000000..7026a668 --- /dev/null +++ b/backend/internal/infrastructure/authmfa/attempts.go @@ -0,0 +1,192 @@ +// 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 authmfa + +import ( + "context" + "strconv" + "sync" + "time" +) + +// attemptRedis is the slice of the Redis wrapper the store needs. Declared here +// so this package does not import the wrapper and tests can drive failures. +type attemptRedis interface { + Incr(ctx context.Context, key string) (int64, error) + Expire(ctx context.Context, key string, ttl time.Duration) error + Get(ctx context.Context, key string) (string, error) + SetNX(ctx context.Context, key, value string, ttl time.Duration) (bool, error) + Del(ctx context.Context, keys ...string) error +} + +// AttemptStore implements application/auth.MFAAttemptStore on Redis, so the +// MFA challenge counters hold across every instance (#689). +// +// When Redis errors, each operation falls back to a per-instance memory store. +// The limits then hold per instance instead of globally. That is weaker, but it +// never fails open, which is what an outage must not do to a brute-force guard. +type AttemptStore struct { + redis attemptRedis + fallback *MemoryAttemptStore +} + +// NewAttemptStore wraps a Redis client. +func NewAttemptStore(redis attemptRedis) *AttemptStore { + return &AttemptStore{redis: redis, fallback: NewMemoryAttemptStore()} +} + +const attemptKeyPrefix = "mfa-attempts:" + +func (s *AttemptStore) Increment(ctx context.Context, key string, ttl time.Duration) (int64, error) { + k := attemptKeyPrefix + key + n, err := s.redis.Incr(ctx, k) + if err != nil { + return s.fallback.Increment(ctx, key, ttl) + } + if n == 1 { + // Only the first increment sets the deadline, so the window is fixed + // from the first failure. A key left without a TTL by a failed Expire + // would count forever; drop it instead and let the memory store count. + if err := s.redis.Expire(ctx, k, ttl); err != nil { + _ = s.redis.Del(ctx, k) + return s.fallback.Increment(ctx, key, ttl) + } + } + return n, nil +} + +func (s *AttemptStore) Count(ctx context.Context, key string) (int64, error) { + v, err := s.redis.Get(ctx, attemptKeyPrefix+key) + if err != nil { + return s.fallback.Count(ctx, key) + } + if v == "" { + // Redis may have been down when the failures were counted. + return s.fallback.Count(ctx, key) + } + n, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return 0, nil + } + return n, nil +} + +// Lock stores the unlock time as the value, so LockedFor can answer without a +// TTL command the wrapper does not expose. +func (s *AttemptStore) Lock(ctx context.Context, key string, ttl time.Duration) (bool, error) { + until := time.Now().Add(ttl).UnixNano() + set, err := s.redis.SetNX(ctx, attemptKeyPrefix+key, strconv.FormatInt(until, 10), ttl) + if err != nil { + return s.fallback.Lock(ctx, key, ttl) + } + return set, nil +} + +func (s *AttemptStore) LockedFor(ctx context.Context, key string) (time.Duration, error) { + v, err := s.redis.Get(ctx, attemptKeyPrefix+key) + if err != nil || v == "" { + return s.fallback.LockedFor(ctx, key) + } + until, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return 0, nil + } + return remaining(time.Unix(0, until)), nil +} + +func (s *AttemptStore) Delete(ctx context.Context, key string) error { + _ = s.fallback.Delete(ctx, key) + // If Redis is down the copy there survives, but it expires on its own. + _ = s.redis.Del(ctx, attemptKeyPrefix+key) + return nil +} + +func remaining(until time.Time) time.Duration { + d := time.Until(until) + if d <= 0 { + return 0 + } + // Round up so a caller told "retry in 0s" is never told that too early. + return d.Truncate(time.Second) + time.Second +} + +// MemoryAttemptStore is the per-instance store. It backs AttemptStore when +// Redis is unreachable and serves tests. +type MemoryAttemptStore struct { + mu sync.Mutex + entries map[string]memoryEntry + now func() time.Time +} + +type memoryEntry struct { + count int64 + expires time.Time +} + +// NewMemoryAttemptStore returns an empty store. +func NewMemoryAttemptStore() *MemoryAttemptStore { + return &MemoryAttemptStore{entries: map[string]memoryEntry{}, now: time.Now} +} + +// live returns the entry for key if it has not expired. Caller holds mu. +func (m *MemoryAttemptStore) live(key string) (memoryEntry, bool) { + e, ok := m.entries[key] + if !ok { + return memoryEntry{}, false + } + if !m.now().Before(e.expires) { + delete(m.entries, key) + return memoryEntry{}, false + } + return e, true +} + +func (m *MemoryAttemptStore) Increment(_ context.Context, key string, ttl time.Duration) (int64, error) { + m.mu.Lock() + defer m.mu.Unlock() + e, ok := m.live(key) + if !ok { + e = memoryEntry{expires: m.now().Add(ttl)} + } + e.count++ + m.entries[key] = e + return e.count, nil +} + +func (m *MemoryAttemptStore) Count(_ context.Context, key string) (int64, error) { + m.mu.Lock() + defer m.mu.Unlock() + e, _ := m.live(key) + return e.count, nil +} + +func (m *MemoryAttemptStore) Lock(_ context.Context, key string, ttl time.Duration) (bool, error) { + m.mu.Lock() + defer m.mu.Unlock() + if _, ok := m.live(key); ok { + return false, nil + } + m.entries[key] = memoryEntry{count: 1, expires: m.now().Add(ttl)} + return true, nil +} + +func (m *MemoryAttemptStore) LockedFor(_ context.Context, key string) (time.Duration, error) { + m.mu.Lock() + defer m.mu.Unlock() + e, ok := m.live(key) + if !ok { + return 0, nil + } + d := e.expires.Sub(m.now()) + return d.Truncate(time.Second) + time.Second, nil +} + +func (m *MemoryAttemptStore) Delete(_ context.Context, key string) error { + m.mu.Lock() + defer m.mu.Unlock() + delete(m.entries, key) + return nil +} diff --git a/backend/internal/infrastructure/authmfa/attempts_test.go b/backend/internal/infrastructure/authmfa/attempts_test.go new file mode 100644 index 00000000..51e94e68 --- /dev/null +++ b/backend/internal/infrastructure/authmfa/attempts_test.go @@ -0,0 +1,182 @@ +// 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 authmfa + +import ( + "context" + "errors" + "strconv" + "sync" + "testing" + "time" + + appauth "github.com/opendefender/openrisk/internal/application/auth" +) + +var ( + _ appauth.MFAAttemptStore = (*AttemptStore)(nil) + _ appauth.MFAAttemptStore = (*MemoryAttemptStore)(nil) +) + +// fakeRedis is a minimal in-process Redis with switchable failure. +type fakeRedis struct { + mu sync.Mutex + vals map[string]string + ttls map[string]time.Duration + down bool + expires int +} + +func newFakeRedis() *fakeRedis { + return &fakeRedis{vals: map[string]string{}, ttls: map[string]time.Duration{}} +} + +var errDown = errors.New("redis down") + +func (f *fakeRedis) Incr(_ context.Context, key string) (int64, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.down { + return 0, errDown + } + n, _ := strconv.ParseInt(f.vals[key], 10, 64) + n++ + f.vals[key] = strconv.FormatInt(n, 10) + return n, nil +} + +func (f *fakeRedis) Expire(_ context.Context, key string, ttl time.Duration) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.down { + return errDown + } + f.expires++ + f.ttls[key] = ttl + return nil +} + +func (f *fakeRedis) Get(_ context.Context, key string) (string, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.down { + return "", errDown + } + return f.vals[key], nil +} + +func (f *fakeRedis) SetNX(_ context.Context, key, value string, ttl time.Duration) (bool, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.down { + return false, errDown + } + if _, ok := f.vals[key]; ok { + return false, nil + } + f.vals[key] = value + f.ttls[key] = ttl + return true, nil +} + +func (f *fakeRedis) Del(_ context.Context, keys ...string) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.down { + return errDown + } + for _, k := range keys { + delete(f.vals, k) + delete(f.ttls, k) + } + return nil +} + +func TestAttemptStore_CountsAndSetsTheWindowOnce(t *testing.T) { + ctx := context.Background() + r := newFakeRedis() + s := NewAttemptStore(r) + + for want := int64(1); want <= 3; want++ { + n, err := s.Increment(ctx, "k", time.Minute) + if err != nil || n != want { + t.Fatalf("increment %d: got %d, %v", want, n, err) + } + } + if r.expires != 1 { + t.Fatalf("the window must be set on the first failure only, got %d Expire calls", r.expires) + } + if got, _ := s.Count(ctx, "k"); got != 3 { + t.Fatalf("count: got %d, want 3", got) + } + _ = s.Delete(ctx, "k") + if got, _ := s.Count(ctx, "k"); got != 0 { + t.Fatalf("count after delete: got %d, want 0", got) + } +} + +func TestAttemptStore_LockIsSetOnceAndReportsWhatRemains(t *testing.T) { + ctx := context.Background() + s := NewAttemptStore(newFakeRedis()) + + set, err := s.Lock(ctx, "lock", 15*time.Minute) + if err != nil || !set { + t.Fatalf("first lock: set=%v err=%v", set, err) + } + if set, _ := s.Lock(ctx, "lock", 15*time.Minute); set { + t.Fatal("a second Lock while the first holds must report false") + } + d, _ := s.LockedFor(ctx, "lock") + if d <= 14*time.Minute || d > 15*time.Minute+time.Second { + t.Fatalf("LockedFor: got %v, want about 15m", d) + } + if d, _ := s.LockedFor(ctx, "other"); d != 0 { + t.Fatalf("an absent lock must read 0, got %v", d) + } +} + +func TestAttemptStore_FallsBackToMemoryWhenRedisIsDown(t *testing.T) { + ctx := context.Background() + r := newFakeRedis() + r.down = true + s := NewAttemptStore(r) + + for range 5 { + if _, err := s.Increment(ctx, "k", time.Minute); err != nil { + t.Fatalf("increment must not fail when Redis is down: %v", err) + } + } + if got, _ := s.Count(ctx, "k"); got != 5 { + t.Fatalf("fallback must keep counting, got %d", got) + } + if set, _ := s.Lock(ctx, "lock", time.Minute); !set { + t.Fatal("fallback must lock") + } + if d, _ := s.LockedFor(ctx, "lock"); d == 0 { + t.Fatal("fallback lock must hold") + } +} + +func TestMemoryAttemptStore_ExpiresEntries(t *testing.T) { + ctx := context.Background() + m := NewMemoryAttemptStore() + now := time.Now() + m.now = func() time.Time { return now } + + _, _ = m.Increment(ctx, "k", time.Minute) + _, _ = m.Lock(ctx, "lock", time.Minute) + + now = now.Add(time.Minute) + if got, _ := m.Count(ctx, "k"); got != 0 { + t.Fatalf("count after the window: got %d, want 0", got) + } + if d, _ := m.LockedFor(ctx, "lock"); d != 0 { + t.Fatalf("lock after its ttl: got %v, want 0", d) + } + if n, _ := m.Increment(ctx, "k", time.Minute); n != 1 { + t.Fatalf("a new window starts at 1, got %d", n) + } +} diff --git a/docs/689_MFA_CHALLENGE_LIMITS.md b/docs/689_MFA_CHALLENGE_LIMITS.md new file mode 100644 index 00000000..468eb04b --- /dev/null +++ b/docs/689_MFA_CHALLENGE_LIMITS.md @@ -0,0 +1,196 @@ +# Spec — #689 MFA challenge limits and logout revocation + +Status: approved by owner 2026-10-02 (limits kept · lock email yes · frontend copy in a follow-up issue) · Branch: `689-securityauth-mfa-challenge-codes-can-be-guessed-without-limit-no-attempt-counter-no-rate-limit` · Milestone: trust-v1 + +## Objective + +Someone who holds a user's password can currently submit TOTP or backup codes to +`POST /api/v1/auth/mfa/challenge` without limit: no attempt counter, no rate +limit, and a challenge token reusable for 5 minutes. A second factor that can be +guessed is not a second factor. Separately, `POST /auth/logout` leaves the access +token valid for up to 15 minutes. + +Success means: + +- a challenge token dies after 5 wrong codes; +- an account stops accepting challenges for a bounded time after repeated failures, whatever the token or source IP; +- the route has its own per-IP bucket; +- backup codes count the same as TOTP codes; +- a logged-out access token is refused on the next request. + +## Current state (read 2026-10-02) + +| Piece | File | Fact | +|---|---|---| +| Route | `backend/cmd/server/main.go:898` | `MFATokenMiddleware` only, no limiter | +| Handler | `backend/internal/handler/auth/mfa_handler.go` `Challenge` | audits each failure, nothing else | +| Use case | `backend/internal/application/auth/mfa_usecase.go` `ChallengeMFAUseCase.Execute` | TOTP, then bcrypt over unused backup codes; returns `ErrValidation` on a miss | +| Challenge token | `backend/internal/auth/token.go` | `MFAChallengeTTL = 5m`, carries a JTI; middleware sets `c.Locals("jti")` and `c.Locals("user")` (claims, with `ExpiresAt`) | +| Blacklist | `backend/pkg/auth/blacklist.go` | `BlacklistJTI(ctx, jti, ttl)`; read by every token middleware; fail-open if Redis is down | +| Shared counters | `backend/internal/middleware/ratelimit.go`, `backend/internal/infrastructure/redis/client.go` | `AllowRate` is a **fixed, wall-clock-aligned** window. A 5-minute token can straddle two buckets, so it cannot give an exact "5 per token" | +| Precedent | #754, `WithDisableAttemptLimit` | per-account budget on `/auth/mfa/disable` via the shared store | +| Logout | `backend/internal/handler/auth/handler.go:480`, `application/auth/logout.go` | revokes the refresh token and clears cookies; the access token is never looked at | + +## Design + +### 1. Per-token limit (AC 1) + +- Key `mfa:challenge:token:`. It counts **failed** codes only and expires with the token's remaining lifetime. +- Before verifying, the use case checks the count. At 5 or more it refuses without touching TOTP or bcrypt. +- The 5th failure blacklists the JTI for `claims.RemainingTTL()`. From then on the middleware itself answers `401 TOKEN_REVOKED`. +- The 5th failure answers `401 {"code":"MFA_CHALLENGE_EXHAUSTED"}`, so the screen can say "sign in again". +- The counter check is what enforces the limit. The blacklist is a second barrier. So a Redis outage, where the blacklist fails open, still leaves the per-instance counter refusing the token. + +### 2. Per-account limit (AC 2) + +- Key `mfa:challenge:user:`. It counts failures across all tokens and IPs, over a 15-minute window. +- At **10 failures** the lock key `mfa:challenge:lock:` is set for **15 minutes**. + - While the lock is set, every challenge for that user answers `429 {"code":"MFA_LOCKED"}` with `Retry-After`. The code is not evaluated. + - The lock is not extended by attempts made while it holds, so it is bounded. +- Setting the lock is audited once, as `mfa_verify` with success=false and reason `mfa_locked`. Reusing the existing action avoids a migration. +- Setting the lock also emails the account owner, once per lock, in their locale (FR/EN). The email says that someone passed their password and then failed the second factor repeatedly, and it recommends a password reset. It reuses the async `securityMailer` (`internal/infrastructure/authmail`) through a new `MFALockedMailer { SendMFALocked(ctx, to, fullName, locale string) error }` port, on the model of `MFADisabledMailer`. A mail failure never changes the response. +- A successful challenge clears the user's failure counter. It does not clear an active lock. +- The key is the user, not (user, tenant). The lock covers the identity across organisations, which is the stricter choice. No DB query is added, so the tenant-filter rule is not engaged. + +### 3. Per-IP limit (AC 3) + +The existing `middleware.RateLimit` runs over `authLimiterStore`, wrapped in `handlers.PrefixedRateLimitBackend{Prefix: "mfa-challenge"}`: + +- **20 requests per 5 minutes per IP**, its own bucket, separate from login's; +- mounted **before** `MFATokenMiddleware`, so a flood never reaches token validation. + +This is the pattern #688 asks for. #688 itself (login, sign-up and reset buckets) is not touched. + +### 4. Backup codes (AC 4) + +Already satisfied structurally: one `Execute` call is one attempt, whether the input matched as TOTP, as a backup code, or as neither. The tests prove it explicitly. + +### 5. Logout revokes the access token (AC 5) + +- `Logout` reads the access token from `Authorization: Bearer` or the `or_access` cookie, the same order as `extractAccessToken`. +- It validates the token's signature and expiry with the server's RSA keys. An invalid or expired token is ignored: the response is still 200 and the cookies are still cleared, as today. +- It then blacklists the JTI for the token's remaining TTL. +- This happens whether or not a refresh token was supplied. +- The revocation lives in `LogoutUseCase`, through a `TokenRevoker` port. The handler only extracts the JTI and expiry. + +### Layering + +- **`application/auth` port:** + `MFAAttemptStore { Increment(ctx, key, ttl) (int64, error); Exists(ctx, key) (bool, error); Set(ctx, key, ttl) error; Delete(ctx, key) error }` + It sits next to `TokenRevoker { BlacklistJTI(ctx, jti, ttl) error }`, which `*authpkg.TokenBlacklistManager` already satisfies. +- **`infrastructure/authmfa/attempts.go`:** a Redis implementation (`INCR`, plus `EXPIRE` on the first increment) with an in-memory fallback, mirroring `RedisRateLimitStore`. A Redis outage degrades to per-instance limits and never fails open. +- **`ChallengeMFAUseCase.WithAttemptLimits(store, revoker)`:** + - `ChallengeMFAInput` gains `ChallengeJTI` and `ChallengeExpiresAt`; + - new sentinel errors `ErrMFAChallengeExhausted` and `ErrMFAChallengeLocked` (the latter carries the retry duration), following the `ErrMFADisable*` precedent; + - unwired means today's behaviour, so the existing tests are unaffected. +- **Handler:** `Challenge` maps the two errors to 401 and 429. `Logout` gains `WithAccessTokenRevocation(rsaKeys)`. +- **`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/infrastructure/authmfa/ -run 'MFAChallenge|Logout' -race -v +``` +Live check: throwaway pg + redis on spare ports, run the server from the repo root (live-boot recipe), then curl through login → challenge. + +## Testing strategy + +**Use case** (`application/auth/mfa_challenge_limits_test.go`), with a fake store and a fake revoker: + +- `TestMFAChallenge_TokenDiesAfterFiveFailures` +- `TestMFAChallenge_UserBackoffAcrossTokens` +- `TestMFAChallenge_BackupCodesCounted` +- `TestMFAChallenge_Success` +- `TestMFAChallenge_NotFound` +- `TestMFAChallenge_Unauthorized` (a locked account is refused even with the right code) +- `TestMFAChallenge_SuccessClearsFailureCount` +- `TestMFAChallenge_LockSendsOneEmail` (one email per lock, not per refused attempt) + +**Store** (`infrastructure/authmfa/attempts_test.go`): counting, expiry, and fallback when Redis errors. + +**Handler e2e** (`handler/auth/mfa_challenge_e2e_test.go`), through real middleware with a real RSA key and an in-memory blacklist: + +- the 6th request with the same token gets 401 `TOKEN_REVOKED`; +- a lock gets 429 with `Retry-After`; +- the IP limit gets 429 before token validation. + +**Logout:** + +- `TestLogout_AccessTokenRejectedAfterLogout`: log in, call logout with the bearer token, then `/auth/me` with the same token gets 401 `TOKEN_REVOKED`; +- a cookie-only logout gets the same result. + +**Live:** criteria 1, 2 and 5 against a local instance with an enrolled account. The output goes in the issue comment. + +## Boundaries + +- **Always:** + - typed errors; + - no code and no token in logs or audit, only reason codes; + - the full `go test ./...` before the PR (route changes trip the authz registry tests). +- **Ask first:** thresholds other than those below, a new audit action (that means a migration). +- **Never:** + - touch the login, sign-up or reset buckets (#688); + - change the TOTP tolerance window; + - make the blacklist fail-closed globally. That is a design change to auth and goes to escalation. + +## Success criteria + +1. A 6th call with a token that has had 5 wrong codes answers 401, and the JTI is in the blacklist. +2. 10 failures over two or more tokens lock the account. During the lock a correct code answers 429 with `Retry-After`, the lock audit row exists, and after the lock expires a correct code succeeds. +3. 21 challenge requests from one IP within 5 minutes: the 21st answers 429, and `/auth/login` from that IP is still allowed. +4. Wrong backup codes increment the same counters as wrong TOTP codes. +5. After logout, the old access token answers 401 on `/auth/me`. +6. The four named tests, plus Success/NotFound/Unauthorized, pass under `-race`. The full build, vet and test run is green. + +## Known limits (stated, not hidden) + +- **Deliberate lockout:** an attacker who already holds the password can re-trigger the 15-minute lock every 15 minutes. That denies the owner, but grants the attacker nothing. The remedy is a password reset, which kills the attacker's first factor. The owner is emailed on each lock, so they learn the password is known. +- **Redis down:** + - the limits become per-instance; + - the JTI blacklist fails open, which is existing behaviour, so a logged-out access token stays valid until it expires; + - the per-token counter still kills challenge tokens on the instance that counted them. + +## Decisions (owner, 2026-10-02) + +1. Limits kept: 5 per token · 10 failures / 15 min → 15-min lock per account · 20 per 5 min per IP. +2. Email the owner when a lock is set: yes. +3. Frontend copy for `MFA_CHALLENGE_EXHAUSTED` and `MFA_LOCKED`: a separate follow-up issue. This PR stays backend. + +## Plan — tasks in dependency order + +Each task ends green on its own `go test` and is one commit (`fix(auth): … (#689)`). + +- [ ] **T1 — Attempt store.** Adds the `MFAAttemptStore` port in `application/auth`, and `infrastructure/authmfa/attempts.go`: Redis (`INCR` + `EXPIRE` on the first increment, `SET` with TTL, `EXISTS`, `DEL`) with an in-memory fallback. + - Accept: counts, expires, falls back when Redis errors. + - Verify: `go test ./internal/infrastructure/authmfa/ -race` + - Files: `application/auth/mfa_limits.go` (new), `infrastructure/authmfa/attempts.go`, `attempts_test.go` + +- [ ] **T2 — Per-token and per-account limits in the use case.** + - `WithAttemptLimits(store, revoker)`; input `ChallengeJTI` and `ChallengeExpiresAt`; `ErrMFAChallengeExhausted` and `ErrMFAChallengeLocked{RetryAfter}`. + - Checks the lock and the token count before verifying. Counts failures, blacklists the token at 5, locks at 10, clears the user counter on success. + - Accept: success criteria 1, 2 and 4 at the use-case level. + - Verify: `go test ./internal/application/auth/ -run MFAChallenge -race -v` + - Files: `mfa_usecase.go`, `mfa_limits.go`, `mfa_challenge_limits_test.go` + +- [ ] **T3 — Lock email.** `MFALockedMailer` port, sent once when the lock is set; FR/EN template in `authmail`. + - Accept: one email per lock, not per refused attempt; a mail failure does not change the result. + - Verify: `go test ./internal/application/auth/ ./internal/infrastructure/authmail/ -race` + - Files: `mfa_limits.go`, `authmail/` mailer and its test + +- [ ] **T4 — Handler and route.** `Challenge` passes the JTI and expiry and maps the errors (401 `MFA_CHALLENGE_EXHAUSTED`, 429 `MFA_LOCKED` with `Retry-After`). In `main.go`: wiring plus the per-IP `RateLimit` with the `mfa-challenge` prefix, mounted before `MFATokenMiddleware`. + - Accept: success criteria 1, 2 and 3 over HTTP. + - Verify: `go test ./internal/handler/auth/ -run MFAChallenge -race -v` + - Files: `mfa_handler.go`, `main.go`, `mfa_challenge_e2e_test.go` + +- [ ] **T5 — Logout revokes the access token.** A `TokenRevoker` in `LogoutUseCase` (input `AccessJTI` and `AccessExpiresAt`); the handler extracts and validates the bearer or cookie token; wiring in `main.go`. + - Accept: success criterion 5, for both bearer and cookie. + - Verify: `go test ./internal/application/auth/ ./internal/handler/auth/ -run Logout -race -v` + - Files: `logout.go`, `handler.go`, `main.go`, tests + +- [ ] **T6 — Full gate and live proof.** + - Run `go build ./... && go vet ./... && go test ./... -race`. + - Live boot with an enrolled account: criteria 1, 2 and 5 by curl, plus the lock email in the mail log. + - Open the frontend follow-up issue, post the progress comment, open the PR with `Closes #689`, set `status:in-review`. + +Risks: route registry and authz ratchet tests may trip on the new middleware order (T4). That is why the full `go test ./...` runs before the PR. From 7fbc47df546cf299e6a0cb4e6aab79accdeda063 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:18:30 +0100 Subject: [PATCH 2/6] fix(auth): limit MFA challenge attempts per token and per account (#689) The challenge checked a TOTP code, then the backup codes, and never counted a miss, so a stolen password plus enough requests was a login. Each attempt is now reserved against the challenge token and the account before the code is read, which keeps parallel requests from sharing one slot. A token gets five attempts and is blacklisted when the fifth fails. An account that fails ten times in fifteen minutes, over any tokens, is locked for a fixed fifteen minutes and the owner is told by mail. Backup codes are the same attempt as TOTP codes. If the store cannot answer, the challenge is refused rather than run unchecked. Signed-off-by: alex-dembele --- .../auth/mfa_challenge_limits_test.go | 323 ++++++++++++++++++ .../internal/application/auth/mfa_limits.go | 213 ++++++++++++ .../internal/application/auth/mfa_usecase.go | 42 ++- 3 files changed, 576 insertions(+), 2 deletions(-) create mode 100644 backend/internal/application/auth/mfa_challenge_limits_test.go diff --git a/backend/internal/application/auth/mfa_challenge_limits_test.go b/backend/internal/application/auth/mfa_challenge_limits_test.go new file mode 100644 index 00000000..e86a6fd1 --- /dev/null +++ b/backend/internal/application/auth/mfa_challenge_limits_test.go @@ -0,0 +1,323 @@ +// 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" + "errors" + "fmt" + "sync" + "testing" + "time" + + "github.com/google/uuid" + "github.com/pquerna/otp/totp" + "golang.org/x/crypto/bcrypt" + + "github.com/opendefender/openrisk/internal/domain" + "github.com/opendefender/openrisk/internal/infrastructure/authmfa" + "github.com/opendefender/openrisk/pkg/crypto" + "github.com/opendefender/openrisk/pkg/otp" +) + +// challengeFixture is one enrolled account behind a challenge use case with +// the #689 limits wired to in-memory fakes. +type challengeFixture struct { + uc *ChallengeMFAUseCase + userID uuid.UUID + tenantID uuid.UUID + secret string + backup string + store *authmfa.MemoryAttemptStore + revoker *fakeJTIRevoker + mailer *fakeLockMailer +} + +type fakeJTIRevoker struct { + mu sync.Mutex + jtis map[string]time.Duration +} + +func (r *fakeJTIRevoker) BlacklistJTI(_ context.Context, jti string, ttl time.Duration) error { + r.mu.Lock() + defer r.mu.Unlock() + r.jtis[jti] = ttl + return nil +} + +func (r *fakeJTIRevoker) revoked(jti string) bool { + r.mu.Lock() + defer r.mu.Unlock() + _, ok := r.jtis[jti] + return ok +} + +type fakeLockMailer struct { + mu sync.Mutex + sent []string +} + +func (m *fakeLockMailer) SendMFALocked(_ context.Context, to, _, locale string) error { + m.mu.Lock() + defer m.mu.Unlock() + m.sent = append(m.sent, to+"|"+locale) + return nil +} + +func (m *fakeLockMailer) count() int { + m.mu.Lock() + defer m.mu.Unlock() + return len(m.sent) +} + +type fakeLockUsers struct{ user *domain.User } + +func (f fakeLockUsers) GetByID(_ context.Context, id uuid.UUID) (*domain.User, error) { + if f.user != nil && f.user.ID == id { + return f.user, nil + } + return nil, nil +} + +func newChallengeFixture(t *testing.T) *challengeFixture { + t.Helper() + key := make([]byte, 32) + repo := NewMockMFARepository() + f := &challengeFixture{ + userID: uuid.New(), + tenantID: uuid.New(), + store: authmfa.NewMemoryAttemptStore(), + revoker: &fakeJTIRevoker{jtis: map[string]time.Duration{}}, + mailer: &fakeLockMailer{}, + backup: "K7M2Q9XT", + } + + secret, err := otp.GenerateTOTPSecret() + if err != nil { + t.Fatal(err) + } + f.secret = secret + enc, err := crypto.EncryptAES256GCM(secret, key) + if err != nil { + t.Fatal(err) + } + _ = repo.CreateMFASecret(context.Background(), &domain.MFASecret{ + ID: uuid.New(), UserID: f.userID, TenantID: f.tenantID, SecretEncrypted: enc, IsVerified: true, + }) + hash, _ := bcrypt.GenerateFromPassword([]byte(f.backup), bcrypt.MinCost) + _ = repo.SaveBackupCodes(context.Background(), []*domain.MFABackupCode{ + {ID: uuid.New(), UserID: f.userID, TenantID: f.tenantID, CodeHash: string(hash)}, + }) + + user := &domain.User{ID: f.userID, Email: "awa.diallo@example.test", FullName: "Awa Diallo", Locale: "en"} + f.uc = NewChallengeMFAUseCase(repo, key). + WithAttemptLimits(f.store, f.revoker). + WithLockNotice(fakeLockUsers{user: user}, f.mailer) + return f +} + +func (f *challengeFixture) try(jti, code string) error { + _, err := f.uc.Execute(context.Background(), ChallengeMFAInput{ + UserID: f.userID, TenantID: f.tenantID, Code: code, + ChallengeJTI: jti, ChallengeExpiresAt: time.Now().Add(5 * time.Minute), + }) + return err +} + +func (f *challengeFixture) goodCode(t *testing.T) string { + t.Helper() + code, err := totp.GenerateCode(f.secret, time.Now()) + if err != nil { + t.Fatal(err) + } + return code +} + +// badCode is a six-digit code the authenticator would not accept right now. +func (f *challengeFixture) badCode(t *testing.T, seed int) string { + t.Helper() + for i := seed; ; i++ { + c := fmt.Sprintf("%06d", (i*7919+13)%1000000) + if !otp.VerifyTOTP(f.secret, c) { + return c + } + } +} + +func isInvalidCode(err error) bool { + var appErr *domain.AppError + return errors.As(err, &appErr) && errors.Is(appErr.Err, domain.ErrValidation) +} + +func TestMFAChallenge_Success(t *testing.T) { + f := newChallengeFixture(t) + if err := f.try("jti-1", f.goodCode(t)); err != nil { + t.Fatalf("a correct code must pass: %v", err) + } +} + +func TestMFAChallenge_NotFound(t *testing.T) { + f := newChallengeFixture(t) + _, err := f.uc.Execute(context.Background(), ChallengeMFAInput{ + UserID: uuid.New(), TenantID: f.tenantID, Code: "123456", ChallengeJTI: "jti-x", + }) + var appErr *domain.AppError + if !errors.As(err, &appErr) || !errors.Is(appErr.Err, domain.ErrNotFound) { + t.Fatalf("an account without a factor must be NotFound, got %v", err) + } +} + +// A locked account is refused even with the right code: the lock is checked +// before the code is. +func TestMFAChallenge_Unauthorized(t *testing.T) { + f := newChallengeFixture(t) + if _, err := f.store.Lock(context.Background(), userLockKey(f.userID), time.Minute); err != nil { + t.Fatal(err) + } + var locked *MFAChallengeLockedError + if err := f.try("jti-1", f.goodCode(t)); !errors.As(err, &locked) { + t.Fatalf("a locked account must refuse a correct code, got %v", err) + } + if locked.JustLocked { + t.Fatal("an existing lock must not read as newly set") + } +} + +func TestMFAChallenge_TokenDiesAfterFiveFailures(t *testing.T) { + f := newChallengeFixture(t) + for i := 1; i < MFAChallengeMaxAttemptsPerToken; i++ { + if err := f.try("jti-1", f.badCode(t, i)); !isInvalidCode(err) { + t.Fatalf("failure %d: want invalid code, got %v", i, err) + } + } + if f.revoker.revoked("jti-1") { + t.Fatal("the token must survive four failures") + } + + if err := f.try("jti-1", f.badCode(t, 99)); !errors.Is(err, ErrMFAChallengeExhausted) { + t.Fatalf("fifth failure: want ErrMFAChallengeExhausted, got %v", err) + } + if !f.revoker.revoked("jti-1") { + t.Fatal("the fifth failure must blacklist the token") + } + + // Even with the blacklist failing open, the counter refuses the token, + // and it refuses it before looking at the code. + if err := f.try("jti-1", f.goodCode(t)); !errors.Is(err, ErrMFAChallengeExhausted) { + t.Fatalf("a spent token must refuse a correct code, got %v", err) + } + // A fresh login gets a fresh token. + if err := f.try("jti-2", f.goodCode(t)); err != nil { + t.Fatalf("a new token must work: %v", err) + } +} + +func TestMFAChallenge_UserBackoffAcrossTokens(t *testing.T) { + f := newChallengeFixture(t) + // Nine failures over three tokens: 4 + 4 + 1. No token is spent, the + // account is one failure short of the lock. + tokens := []string{"a", "a", "a", "a", "b", "b", "b", "b", "c"} + for i, jti := range tokens { + if err := f.try(jti, f.badCode(t, i)); !isInvalidCode(err) { + t.Fatalf("failure %d: want invalid code, got %v", i+1, err) + } + } + + var locked *MFAChallengeLockedError + if err := f.try("c", f.badCode(t, 50)); !errors.As(err, &locked) { + t.Fatalf("the tenth failure must lock the account, got %v", err) + } + if !locked.JustLocked || locked.RetryAfter != MFAChallengeLockDuration { + t.Fatalf("lock: JustLocked=%v RetryAfter=%v", locked.JustLocked, locked.RetryAfter) + } + + // Another token, another address: still locked, with what remains. + if err := f.try("d", f.goodCode(t)); !errors.As(err, &locked) { + t.Fatalf("a correct code on a new token must be refused during the lock, got %v", err) + } + if locked.JustLocked || locked.RetryAfter <= 0 || locked.RetryAfter > MFAChallengeLockDuration+time.Second { + t.Fatalf("refusal under lock: JustLocked=%v RetryAfter=%v", locked.JustLocked, locked.RetryAfter) + } +} + +func TestMFAChallenge_BackupCodesCounted(t *testing.T) { + f := newChallengeFixture(t) + // Wrong backup-shaped codes spend the token exactly like wrong TOTP codes. + wrong := []string{"AAAAAAAA", "BBBBBBBB", "CCCCCCCC", "DDDDDDDD"} + for _, c := range wrong { + if err := f.try("jti-1", c); !isInvalidCode(err) { + t.Fatalf("%s: want invalid code, got %v", c, err) + } + } + if err := f.try("jti-1", "EEEEEEEE"); !errors.Is(err, ErrMFAChallengeExhausted) { + t.Fatalf("fifth wrong backup code: want ErrMFAChallengeExhausted, got %v", err) + } + // The real backup code cannot rescue a spent token. + if err := f.try("jti-1", f.backup); !errors.Is(err, ErrMFAChallengeExhausted) { + t.Fatalf("a valid backup code on a spent token: got %v", err) + } + // And the five backup failures count towards the account too. + if n, _ := f.store.Count(context.Background(), userFailuresKey(f.userID)); n < 5 { + t.Fatalf("account failures: got %d, want at least 5", n) + } + // On a fresh token the real backup code still works. + if err := f.try("jti-2", f.backup); err != nil { + t.Fatalf("a valid backup code on a fresh token: %v", err) + } +} + +func TestMFAChallenge_SuccessClearsFailureCount(t *testing.T) { + f := newChallengeFixture(t) + for i := range 4 { + _ = f.try("a", f.badCode(t, i)) + } + for i := range 4 { + _ = f.try("b", f.badCode(t, 10+i)) + } + if err := f.try("c", f.goodCode(t)); err != nil { + t.Fatalf("correct code: %v", err) + } + // Eight earlier failures are forgiven: eight more do not lock. + for i := range 4 { + _ = f.try("d", f.badCode(t, 20+i)) + } + for i := range 4 { + if err := f.try("e", f.badCode(t, 30+i)); !isInvalidCode(err) { + t.Fatalf("after a success the count restarts, got %v", err) + } + } +} + +func TestMFAChallenge_LockSendsOneEmail(t *testing.T) { + f := newChallengeFixture(t) + for i := range MFAChallengeMaxFailuresPerUser { + _ = f.try(fmt.Sprintf("t%d", i/4), f.badCode(t, i)) + } + for i := range 20 { + _ = f.try(fmt.Sprintf("x%d", i), f.goodCode(t)) + } + if got := f.mailer.count(); got != 1 { + t.Fatalf("one lock must send one email, got %d", got) + } + if f.mailer.sent[0] != "awa.diallo@example.test|en" { + t.Fatalf("email sent to %q", f.mailer.sent[0]) + } +} + +type brokenStore struct{ *authmfa.MemoryAttemptStore } + +func (brokenStore) LockedFor(context.Context, string) (time.Duration, error) { + return 0, errors.New("store down") +} + +// If the limiter cannot answer, the challenge must not go ahead unchecked. +func TestMFAChallenge_StoreErrorFailsClosed(t *testing.T) { + f := newChallengeFixture(t) + f.uc.WithAttemptLimits(brokenStore{authmfa.NewMemoryAttemptStore()}, f.revoker) + if err := f.try("jti-1", f.goodCode(t)); err == nil { + t.Fatal("a store error must refuse the attempt") + } +} diff --git a/backend/internal/application/auth/mfa_limits.go b/backend/internal/application/auth/mfa_limits.go index 55debfe9..cd1cc5b0 100644 --- a/backend/internal/application/auth/mfa_limits.go +++ b/backend/internal/application/auth/mfa_limits.go @@ -7,9 +7,61 @@ package auth import ( "context" + "errors" + "fmt" "time" + + "github.com/google/uuid" + + coreauth "github.com/opendefender/openrisk/internal/auth" + "github.com/opendefender/openrisk/internal/domain" ) +// Limits on the MFA challenge (#689). A 6-digit code with a ±1 step tolerance +// leaves about 3 valid values in 10^6, so the budget has to be counted in +// single digits per token and in tens per account, not per minute. +const ( + // MFAChallengeMaxAttemptsPerToken is how many codes one MFA_REQUIRED token + // may try. The token is blacklisted when the last one fails. + MFAChallengeMaxAttemptsPerToken = 5 + // MFAChallengeMaxFailuresPerUser is how many failures an account takes, + // across every token and source address, before it is locked. + MFAChallengeMaxFailuresPerUser = 10 + // MFAChallengeFailureWindow is counted from the first failure. + MFAChallengeFailureWindow = 15 * time.Minute + // MFAChallengeLockDuration is fixed: attempts during the lock do not + // extend it, so a lock can never outlast this. + MFAChallengeLockDuration = 15 * time.Minute +) + +// ErrMFAChallengeExhausted means this challenge token has used all its +// attempts. The caller must sign in again with the password. +var ErrMFAChallengeExhausted = errors.New("this sign-in attempt has no codes left") + +// MFAChallengeLockedError means the account refuses challenges for a while. +type MFAChallengeLockedError struct { + RetryAfter time.Duration + // JustLocked is true only for the request that set the lock, so the + // caller audits a lock once rather than once per refused attempt. + JustLocked bool +} + +func (e *MFAChallengeLockedError) Error() string { + return "two-factor sign-in is locked for this account" +} + +// MFALockedMailer tells the owner that their account was locked after repeated +// second-factor failures, which means someone else holds their password. +type MFALockedMailer interface { + SendMFALocked(ctx context.Context, to, fullName, locale string) error +} + +// MFALockUserLookup finds the address to warn. Satisfied by +// *repository.GormUserRepository. +type MFALockUserLookup interface { + GetByID(ctx context.Context, id uuid.UUID) (*domain.User, error) +} + // MFAAttemptStore holds the counters and locks that bound guessing at the MFA // challenge (#689). It must be shared across instances: a per-instance counter // lets an attacker multiply the budget by the number of replicas. @@ -34,3 +86,164 @@ type MFAAttemptStore interface { type TokenRevoker interface { BlacklistJTI(ctx context.Context, jti string, ttl time.Duration) error } + +// WithAttemptLimits turns the limits on. Without it the challenge behaves as it +// did before #689; main.go always wires it. +func (uc *ChallengeMFAUseCase) WithAttemptLimits(store MFAAttemptStore, revoker TokenRevoker) *ChallengeMFAUseCase { + if uc.limits == nil { + uc.limits = &challengeLimits{} + } + uc.limits.store = store + uc.limits.revoker = revoker + return uc +} + +// WithLockNotice mails the owner when their account gets locked. Optional. +func (uc *ChallengeMFAUseCase) WithLockNotice(users MFALockUserLookup, mailer MFALockedMailer) *ChallengeMFAUseCase { + if uc.limits == nil { + uc.limits = &challengeLimits{} + } + uc.limits.users = users + uc.limits.mailer = mailer + return uc +} + +type challengeLimits struct { + store MFAAttemptStore + revoker TokenRevoker + users MFALockUserLookup + mailer MFALockedMailer +} + +// The account keys are per user, not per (user, tenant): the lock protects the +// identity, whichever organisation the login targets. +func tokenAttemptsKey(jti string) string { return "challenge:token:" + jti } +func userFailuresKey(userID uuid.UUID) string { return "challenge:user:" + userID.String() } +func userLockKey(userID uuid.UUID) string { return "challenge:lock:" + userID.String() } + +// reserve refuses a locked account or a spent token, then counts this attempt +// against both before the code is looked at. +func (l *challengeLimits) reserve(ctx context.Context, input ChallengeMFAInput) error { + if l.store == nil { + return nil + } + locked, err := l.store.LockedFor(ctx, userLockKey(input.UserID)) + if err != nil { + return fmt.Errorf("auth.ChallengeMFA: read lock: %w", err) + } + if locked > 0 { + return &MFAChallengeLockedError{RetryAfter: locked} + } + + if input.ChallengeJTI != "" { + n, err := l.store.Increment(ctx, tokenAttemptsKey(input.ChallengeJTI), tokenRemaining(input)) + if err != nil { + return fmt.Errorf("auth.ChallengeMFA: count token attempt: %w", err) + } + if n > MFAChallengeMaxAttemptsPerToken { + l.revoke(ctx, input) + return ErrMFAChallengeExhausted + } + } + + n, err := l.store.Increment(ctx, userFailuresKey(input.UserID), MFAChallengeFailureWindow) + if err != nil { + return fmt.Errorf("auth.ChallengeMFA: count account attempt: %w", err) + } + if n > MFAChallengeMaxFailuresPerUser { + // Only reachable when parallel requests raced past the lock check. + return l.lock(ctx, input.UserID) + } + return nil +} + +// recordFailure acts on the counts reserve already raised: the account lock +// first, because it outranks the token, then the token. +func (l *challengeLimits) recordFailure(ctx context.Context, input ChallengeMFAInput) error { + if l.store == nil { + return nil + } + failures, err := l.store.Count(ctx, userFailuresKey(input.UserID)) + if err != nil { + return fmt.Errorf("auth.ChallengeMFA: read account failures: %w", err) + } + if failures >= MFAChallengeMaxFailuresPerUser { + l.revoke(ctx, input) + return l.lock(ctx, input.UserID) + } + + if input.ChallengeJTI == "" { + return nil + } + attempts, err := l.store.Count(ctx, tokenAttemptsKey(input.ChallengeJTI)) + if err != nil { + return fmt.Errorf("auth.ChallengeMFA: read token attempts: %w", err) + } + if attempts >= MFAChallengeMaxAttemptsPerToken { + l.revoke(ctx, input) + return ErrMFAChallengeExhausted + } + return nil +} + +// recordSuccess forgives earlier mistypes. An active lock is not touched: a +// success cannot happen under one, reserve refuses first. +func (l *challengeLimits) recordSuccess(ctx context.Context, input ChallengeMFAInput) { + if l.store == nil { + return + } + _ = l.store.Delete(ctx, userFailuresKey(input.UserID)) +} + +func (l *challengeLimits) lock(ctx context.Context, userID uuid.UUID) error { + set, err := l.store.Lock(ctx, userLockKey(userID), MFAChallengeLockDuration) + if err != nil { + return fmt.Errorf("auth.ChallengeMFA: set lock: %w", err) + } + // A fresh window starts when the lock ends. + _ = l.store.Delete(ctx, userFailuresKey(userID)) + + retry := MFAChallengeLockDuration + if !set { + if d, err := l.store.LockedFor(ctx, userLockKey(userID)); err == nil && d > 0 { + retry = d + } + } else { + l.notify(ctx, userID) + } + return &MFAChallengeLockedError{RetryAfter: retry, JustLocked: set} +} + +// revoke kills the challenge token. The counter already refuses it; the +// blacklist makes the middleware refuse it too, before any code is read. +func (l *challengeLimits) revoke(ctx context.Context, input ChallengeMFAInput) { + if l.revoker == nil || input.ChallengeJTI == "" { + return + } + _ = l.revoker.BlacklistJTI(ctx, input.ChallengeJTI, tokenRemaining(input)) +} + +func (l *challengeLimits) notify(ctx context.Context, userID uuid.UUID) { + if l.mailer == nil || l.users == nil { + return + } + user, err := l.users.GetByID(ctx, userID) + if err != nil || user == nil || user.Email == "" { + return + } + _ = l.mailer.SendMFALocked(ctx, user.Email, user.FullName, normaliseLocale(user.Locale)) +} + +// tokenRemaining is how long the challenge token has left, which is how long +// its counter and its blacklist entry need to live. A token without a known +// expiry gets the full challenge lifetime. +func tokenRemaining(input ChallengeMFAInput) time.Duration { + if input.ChallengeExpiresAt.IsZero() { + return coreauth.MFAChallengeTTL + } + d := time.Until(input.ChallengeExpiresAt) + if d < time.Second { + return time.Second + } + return d +} diff --git a/backend/internal/application/auth/mfa_usecase.go b/backend/internal/application/auth/mfa_usecase.go index c863f2d0..1a7465ba 100644 --- a/backend/internal/application/auth/mfa_usecase.go +++ b/backend/internal/application/auth/mfa_usecase.go @@ -401,6 +401,10 @@ type ChallengeMFAInput struct { UserID uuid.UUID TenantID uuid.UUID Code string // TOTP code OR backup code + // ChallengeJTI and ChallengeExpiresAt identify the MFA_REQUIRED token the + // request came with. Attempts are counted against it (#689). + ChallengeJTI string + ChallengeExpiresAt time.Time } // ChallengeMFAOutput represents MFA challenge response @@ -413,6 +417,7 @@ type ChallengeMFAOutput struct { type ChallengeMFAUseCase struct { mfaRepo repository.MFARepository encKey []byte + limits *challengeLimits } // NewChallengeMFAUseCase creates a new challenge MFA use case @@ -423,7 +428,11 @@ func NewChallengeMFAUseCase(mfaRepo repository.MFARepository, encKey []byte) *Ch } } -// Execute verifies TOTP or backup code during login +// Execute verifies TOTP or backup code during login. +// +// With limits wired, every attempt is reserved against the challenge token and +// the account before the code is checked, so parallel requests cannot all slip +// under the limit together. A TOTP code and a backup code are the same attempt. func (uc *ChallengeMFAUseCase) Execute(ctx context.Context, input ChallengeMFAInput) (*ChallengeMFAOutput, error) { if input.UserID == uuid.Nil || input.TenantID == uuid.Nil { return nil, domain.NewValidationError("user_id and tenant_id required") @@ -432,6 +441,35 @@ func (uc *ChallengeMFAUseCase) Execute(ctx context.Context, input ChallengeMFAIn return nil, domain.NewValidationError("code required") } + if uc.limits != nil { + if err := uc.limits.reserve(ctx, input); err != nil { + return nil, err + } + } + + out, err := uc.verify(ctx, input) + if errors.Is(err, errMFACodeMismatch) { + if uc.limits != nil { + if limitErr := uc.limits.recordFailure(ctx, input); limitErr != nil { + return nil, limitErr + } + } + return nil, domain.NewValidationError("invalid MFA code") + } + if err != nil { + return nil, err + } + + if uc.limits != nil { + uc.limits.recordSuccess(ctx, input) + } + return out, nil +} + +// errMFACodeMismatch is the one outcome that counts as a failed guess. +var errMFACodeMismatch = errors.New("mfa code mismatch") + +func (uc *ChallengeMFAUseCase) verify(ctx context.Context, input ChallengeMFAInput) (*ChallengeMFAOutput, error) { // Get MFA secret mfaSecret, err := uc.mfaRepo.GetMFASecret(ctx, input.UserID, input.TenantID) if err != nil { @@ -480,5 +518,5 @@ func (uc *ChallengeMFAUseCase) Execute(ctx context.Context, input ChallengeMFAIn } } - return nil, domain.NewValidationError("invalid MFA code") + return nil, errMFACodeMismatch } From 15342236c1bfdc4abdd4cb9812724623fe3a64d5 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:19:18 +0100 Subject: [PATCH 3/6] fix(auth): mail the owner when MFA sign-in gets locked (#689) A lock means someone got past the password and then guessed codes. The owner is the only one who can fix that, by resetting the password, so they get a notice in their language the moment the lock is set. Sent through the async security mailer, so a slow SMTP server never delays the challenge response. Signed-off-by: alex-dembele --- .../internal/infrastructure/authmail/async.go | 7 +++ .../authmail/mfa_locked_mailer_test.go | 46 +++++++++++++++++++ .../infrastructure/authmail/reset_mailer.go | 37 +++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 backend/internal/infrastructure/authmail/mfa_locked_mailer_test.go diff --git a/backend/internal/infrastructure/authmail/async.go b/backend/internal/infrastructure/authmail/async.go index 97a3a926..e363886e 100644 --- a/backend/internal/infrastructure/authmail/async.go +++ b/backend/internal/infrastructure/authmail/async.go @@ -34,6 +34,7 @@ type ResetMailerLike interface { SendNewSignInAlert(ctx context.Context, to, fullName, ip, userAgent string, when time.Time, locale string) error SendPasswordChanged(ctx context.Context, to, fullName, locale string) error SendMFADisabled(ctx context.Context, to, fullName, locale string) error + SendMFALocked(ctx context.Context, to, fullName, locale string) error } // NewAsync wraps a mailer so every send returns immediately. @@ -65,6 +66,12 @@ func (a *Async) SendMFADisabled(_ context.Context, to, fullName, locale string) return nil } +// SendMFALocked queues the second-factor lock notice (#689). +func (a *Async) SendMFALocked(_ context.Context, to, fullName, locale string) error { + a.dispatch(func(ctx context.Context) { _ = a.inner.SendMFALocked(ctx, to, fullName, locale) }) + return nil +} + // SendNewSignInAlert queues the new-device notice. func (a *Async) SendNewSignInAlert(_ context.Context, to, fullName, ip, userAgent string, when time.Time, locale string) error { a.dispatch(func(ctx context.Context) { diff --git a/backend/internal/infrastructure/authmail/mfa_locked_mailer_test.go b/backend/internal/infrastructure/authmail/mfa_locked_mailer_test.go new file mode 100644 index 00000000..fde886b0 --- /dev/null +++ b/backend/internal/infrastructure/authmail/mfa_locked_mailer_test.go @@ -0,0 +1,46 @@ +// 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 authmail + +import ( + "context" + "strings" + "testing" +) + +func TestSendMFALocked_SaysWhatHappenedInBothLanguages(t *testing.T) { + cases := []struct { + locale, subject, mustSay string + }{ + {"en", "two-factor sign-in", "reset your password"}, + {"fr", "connexion à deux facteurs", "réinitialisez votre mot de passe"}, + } + for _, tc := range cases { + t.Run(tc.locale, func(t *testing.T) { + s := &capturingSender{} + if err := New(s).SendMFALocked(context.Background(), "awa@example.test", "Awa Diallo", tc.locale); err != nil { + t.Fatal(err) + } + if s.to != "awa@example.test" { + t.Fatalf("sent to %q", s.to) + } + if !strings.Contains(s.subject, tc.subject) { + t.Fatalf("subject %q lacks %q", s.subject, tc.subject) + } + for _, want := range []string{"Awa Diallo", "15 minutes", tc.mustSay} { + if !strings.Contains(s.body, want) { + t.Fatalf("body lacks %q", want) + } + } + }) + } +} + +func TestSendMFALocked_NoSenderIsANoOp(t *testing.T) { + if err := New(nil).SendMFALocked(context.Background(), "a@example.test", "", "en"); err != nil { + t.Fatalf("an unwired mailer must not fail the challenge: %v", err) + } +} diff --git a/backend/internal/infrastructure/authmail/reset_mailer.go b/backend/internal/infrastructure/authmail/reset_mailer.go index ce4eff9a..e793e81f 100644 --- a/backend/internal/infrastructure/authmail/reset_mailer.go +++ b/backend/internal/infrastructure/authmail/reset_mailer.go @@ -83,6 +83,17 @@ func (m *Mailer) SendMFADisabled(ctx context.Context, to, fullName, locale strin return m.sender.SendEmail(ctx, to, c.subject, m.render(c)) } +// SendMFALocked tells the owner that two-factor sign-in was locked after +// repeated wrong codes (#689). Reaching the code step takes the password, so +// the message says so plainly: the remedy is a password reset. +func (m *Mailer) SendMFALocked(ctx context.Context, to, fullName, locale string) error { + if m == nil || m.sender == nil { + return nil + } + c := mfaLockedCopy(locale, displayName(fullName, locale)) + return m.sender.SendEmail(ctx, to, c.subject, m.render(c)) +} + // SendNewSignInAlert warns about a sign-in from an unrecognised device. func (m *Mailer) SendNewSignInAlert(ctx context.Context, to, fullName, ip, userAgent string, when time.Time, locale string) error { if m == nil || m.sender == nil { @@ -206,6 +217,32 @@ func mfaDisabledCopy(locale, name string) copyBlock { } } +func mfaLockedCopy(locale, name string) copyBlock { + minutes := 15 // mirrors application/auth.MFAChallengeLockDuration + if locale == "en" { + return copyBlock{ + subject: "Too many wrong codes: two-factor sign-in to your OpenRisk account is paused", + heading: "Two-factor sign-in is paused", + paragraphs: []string{ + fmt.Sprintf("Hello %s,", name), + fmt.Sprintf("Someone entered your password correctly, then a wrong two-factor code too many times. To stop them guessing, OpenRisk refuses codes for this account for %d minutes. Your account was not opened.", minutes), + "Getting to the code step takes your password. If these attempts were not yours, someone else knows it.", + }, + footnote: "If this wasn't you, reset your password now and tell your OpenRisk administrator. If it was you, wait a few minutes and sign in again.", + } + } + return copyBlock{ + subject: "Trop de codes erronés : la connexion à deux facteurs de votre compte OpenRisk est suspendue", + heading: "La connexion à deux facteurs est suspendue", + paragraphs: []string{ + fmt.Sprintf("Bonjour %s,", name), + fmt.Sprintf("Quelqu'un a saisi votre mot de passe correctement, puis un code de double authentification erroné trop de fois. Pour l'empêcher de deviner, OpenRisk refuse les codes pour ce compte pendant %d minutes. Votre compte n'a pas été ouvert.", minutes), + "Pour arriver à l'étape du code, il faut votre mot de passe. Si ces tentatives ne viennent pas de vous, quelqu'un d'autre le connaît.", + }, + footnote: "Si ce n'était pas vous, réinitialisez votre mot de passe dès maintenant et prévenez votre administrateur OpenRisk. Si c'était vous, patientez quelques minutes et reconnectez-vous.", + } +} + func newSignInCopy(locale, name, ip, userAgent string, when time.Time) copyBlock { stamp := when.UTC().Format("2006-01-02 15:04 UTC") if locale == "en" { From c8fd842b3da41879e69dc01a37ee345624b4229d Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:23:41 +0100 Subject: [PATCH 4/6] fix(auth): enforce MFA challenge limits over HTTP (#689) The handler passes the challenge token's JTI and expiry to the use case and answers each refusal with a code the sign-in screen can act on: MFA_CHALLENGE_EXHAUSTED (401, sign in again) and MFA_LOCKED (429, with Retry-After). A lock is audited once, when it is set. Store and decryption errors now answer 500 without their text instead of a 400 that echoed it. The route gets its own per-IP limit, 20 per 5 minutes in a prefixed bucket, mounted before the token middleware so a flood never reaches token parsing and never spends the login budget. Signed-off-by: alex-dembele --- backend/cmd/server/main.go | 21 +- .../handler/auth/mfa_challenge_e2e_test.go | 249 ++++++++++++++++++ backend/internal/handler/auth/mfa_handler.go | 69 ++++- 3 files changed, 331 insertions(+), 8 deletions(-) create mode 100644 backend/internal/handler/auth/mfa_challenge_e2e_test.go diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index a04853e5..38e78fbd 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -699,7 +699,12 @@ func main() { WithTOTPKey(mfaKey[:]). RequireMFAForRoles(mfaRequiredRoles, mfaRequiredBusinessRoles). WithMailer(securityMailer) - challengeMFAUseCase := auth.NewChallengeMFAUseCase(mfaRepo, mfaKey[:]) + // #689 — five codes per challenge token, ten failures per account in + // fifteen minutes before a fixed fifteen-minute lock, counted in Redis so + // the budget holds across instances. The owner is mailed when a lock is set. + challengeMFAUseCase := auth.NewChallengeMFAUseCase(mfaRepo, mfaKey[:]). + WithAttemptLimits(authmfa.NewAttemptStore(redisClientInstance), tokenBlacklistManager). + WithLockNotice(userRepo, securityMailer) // OR26-03 — one resolver answers "must this member enrol now?" for /auth/me // and for the request-time guard, so the banner and the enforcement can never // disagree. Cached per (user, tenant) for a minute; enrolment, disabling and @@ -895,7 +900,19 @@ func main() { // on `api` BEFORE the Protected group: MFATokenMiddleware validates the special // token itself and rejects full/absent tokens. On a valid code the handler // mints the real access+refresh pair. - api.Post("/auth/mfa/challenge", middleware.MFATokenMiddleware(rsaKeys, jtiBlacklistChecker), mfaHandler.Challenge) + // + // #689 — the per-IP limit runs first, in its own bucket: a flood is refused + // before any token is parsed, and it never spends the login budget. The + // per-token and per-account limits live in the use case. + mfaChallengeRateLimit := middleware.RateLimit(middleware.RateLimitConfig{ + MaxRequests: 20, + WindowSize: 5 * time.Minute, + Store: handlers.PrefixedRateLimitBackend{ + Prefix: "mfa-challenge-ip:", + Inner: authLimiterStore, + }, + }) + api.Post("/auth/mfa/challenge", mfaChallengeRateLimit, middleware.MFATokenMiddleware(rsaKeys, jtiBlacklistChecker), mfaHandler.Challenge) // Scanner AGENT endpoints (register/stream/push) are mounted on `app` HERE — // deliberately BEFORE the /api/v1 user-token middleware below — so they are diff --git a/backend/internal/handler/auth/mfa_challenge_e2e_test.go b/backend/internal/handler/auth/mfa_challenge_e2e_test.go new file mode 100644 index 00000000..916956b1 --- /dev/null +++ b/backend/internal/handler/auth/mfa_challenge_e2e_test.go @@ -0,0 +1,249 @@ +// 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 + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/google/uuid" + "github.com/pquerna/otp/totp" + "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/middleware" + authpkg "github.com/opendefender/openrisk/pkg/auth" + "github.com/opendefender/openrisk/pkg/crypto" + "github.com/opendefender/openrisk/pkg/otp" +) + +// --------------------------------------------------------------------------- +// #689 — the MFA challenge through the real HTTP stack: per-IP limiter, the +// MFA_REQUIRED token middleware with a JTI blacklist, handler, use case and +// GORM repository, mounted in the order main.go uses. +// --------------------------------------------------------------------------- + +const challengePath = "/api/v1/auth/mfa/challenge" + +// memoryBlacklist stands in for Redis behind authpkg.TokenBlacklistManager. +type memoryBlacklist struct { + mu sync.Mutex + keys map[string]string +} + +func (m *memoryBlacklist) Set(_ context.Context, key, value string, _ time.Duration) error { + m.mu.Lock() + defer m.mu.Unlock() + m.keys[key] = value + return nil +} + +func (m *memoryBlacklist) Get(_ context.Context, key string) (string, error) { + m.mu.Lock() + defer m.mu.Unlock() + return m.keys[key], nil +} + +func (m *memoryBlacklist) Exists(_ context.Context, key string) (bool, error) { + m.mu.Lock() + defer m.mu.Unlock() + _, ok := m.keys[key] + return ok, nil +} + +// challengeStack adds the challenge (and logout) routes to a fixture's +// database and keys, on a fresh app. +type challengeStack struct { + *deferredFixture + ipLimit int +} + +func newChallengeStack(t *testing.T, ipLimit int) *challengeStack { + t.Helper() + f := newDeferredFixture(t) + userRepo := repository.NewGormUserRepository(f.db) + mfaRepo := repository.NewGormMFARepository(f.db) + tokens := coreauth.NewTokenManager(f.db, f.keys) + // What main.go's resolver derives from the default membership, reduced to + // the one account these tests sign in with. + tokens.SetSessionResolver(func(_ context.Context, userID uuid.UUID) (*coreauth.SessionClaims, error) { + return &coreauth.SessionClaims{ + TenantID: f.tenantA, + OrgRoles: map[uuid.UUID]string{f.tenantA: string(domain.RoleUser)}, + }, nil + }) + orgRoles, businessRoles := domain.DefaultMFAPrivilegeRoles() + + blacklist := authpkg.NewTokenBlacklistManager(&memoryBlacklist{keys: map[string]string{}}) + checker := blacklist.CheckJTIBlacklist(context.Background()) + + loginUC := appauth.NewLoginUseCase(userRepo, tokens, deferredHasher{}). + WithMFA(mfaRepo). + RequireMFAForRoles(orgRoles, businessRoles). + WithMFAPolicies(f.policyRepo). + WithClock(func() time.Time { return f.now }) + logoutUC := appauth.NewLogoutUseCase(tokens) + h := authhandler.NewHandler(loginUC, nil, nil, logoutUC, deferredHasher{}, nil). + WithUserLookup(userRepo). + WithMFAStatus(f.resolver) + + challengeUC := appauth.NewChallengeMFAUseCase(mfaRepo, deferredTOTPKey). + WithAttemptLimits(authmfa.NewMemoryAttemptStore(), blacklist) + mfaHandler := authhandler.NewMFAHandler(nil, nil, nil, challengeUC, tokens, userRepo, nil) + + app := fiber.New() + api := app.Group("/api/v1") + api.Post("/auth/login", h.Login) + api.Post("/auth/logout", h.Logout) + ipLimiter := middleware.RateLimit(middleware.RateLimitConfig{ + MaxRequests: ipLimit, + WindowSize: 5 * time.Minute, + Store: middleware.NewRateLimitStore(), + }) + api.Post("/auth/mfa/challenge", ipLimiter, middleware.MFATokenMiddleware(f.keys, checker), mfaHandler.Challenge) + protected := api.Use(middleware.Protected(f.keys, checker)) + protected.Get("/auth/me", h.Me) + + f.app = app + return &challengeStack{deferredFixture: f, ipLimit: ipLimit} +} + +// enrolWithSecret gives a member a working authenticator and returns its seed. +func (s *challengeStack) enrolWithSecret(t *testing.T, m *domain.OrganizationMember) string { + t.Helper() + plain, err := otp.GenerateTOTPSecret() + require.NoError(t, err) + enc, err := crypto.EncryptAES256GCM(plain, deferredTOTPKey) + require.NoError(t, err) + require.NoError(t, s.db.Create(&domain.MFASecret{ + ID: uuid.New(), UserID: m.UserID, TenantID: m.OrganizationID, SecretEncrypted: enc, IsVerified: true, + }).Error) + return plain +} + +// mfaToken signs in with the password and returns the MFA_REQUIRED token. +func (s *challengeStack) mfaToken(t *testing.T, email string) string { + t.Helper() + body, _ := s.login(t, email) + tok, _ := body["mfa_token"].(string) + require.NotEmpty(t, tok, "the login must stop at the code step: %v", body) + return tok +} + +func wrongCode(t *testing.T, plain string, seed int) string { + t.Helper() + for i := seed; ; i++ { + c := fmt.Sprintf("%06d", (i*104729+7)%1000000) + if !otp.VerifyTOTP(plain, c) { + return c + } + } +} + +func liveCode(t *testing.T, plain string) string { + t.Helper() + c, err := totp.GenerateCode(plain, time.Now()) + require.NoError(t, err) + return c +} + +func TestMFAChallengeE2E_CorrectCodeOpensASession(t *testing.T) { + s := newChallengeStack(t, 100) + plain := s.enrolWithSecret(t, s.memberA) + + status, body := s.do(t, http.MethodPost, challengePath, s.mfaToken(t, s.memberA.User.Email), jsonBody{"code": liveCode(t, plain)}) + require.Equal(t, http.StatusOK, status, "%v", body) + assert.NotNil(t, body["token_pair"]) +} + +func TestMFAChallengeE2E_TokenIsRevokedAfterFiveWrongCodes(t *testing.T) { + s := newChallengeStack(t, 100) + plain := s.enrolWithSecret(t, s.memberA) + tok := s.mfaToken(t, s.memberA.User.Email) + + for i := 1; i <= 4; i++ { + status, _ := s.do(t, http.MethodPost, challengePath, tok, jsonBody{"code": wrongCode(t, plain, i)}) + require.Equal(t, http.StatusBadRequest, status, "wrong code %d", i) + } + status, body := s.do(t, http.MethodPost, challengePath, tok, jsonBody{"code": wrongCode(t, plain, 40)}) + require.Equal(t, http.StatusUnauthorized, status) + assert.Equal(t, "MFA_CHALLENGE_EXHAUSTED", body["code"]) + + // Sixth attempt, with the right code: the middleware refuses the token + // before the handler reads anything. + status, body = s.do(t, http.MethodPost, challengePath, tok, jsonBody{"code": liveCode(t, plain)}) + require.Equal(t, http.StatusUnauthorized, status) + assert.Equal(t, "TOKEN_REVOKED", body["code"]) +} + +func TestMFAChallengeE2E_AccountLocksAcrossTokensWithRetryAfter(t *testing.T) { + s := newChallengeStack(t, 100) + plain := s.enrolWithSecret(t, s.memberA) + + n := 0 + for range 2 { + tok := s.mfaToken(t, s.memberA.User.Email) + for range 4 { + n++ + status, _ := s.do(t, http.MethodPost, challengePath, tok, jsonBody{"code": wrongCode(t, plain, n)}) + require.Equal(t, http.StatusBadRequest, status, "failure %d", n) + } + } + tok := s.mfaToken(t, s.memberA.User.Email) + status, _ := s.do(t, http.MethodPost, challengePath, tok, jsonBody{"code": wrongCode(t, plain, 90)}) + require.Equal(t, http.StatusBadRequest, status, "failure 9") + + resp, body := s.raw(t, tok, wrongCode(t, plain, 91)) + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode, "the tenth failure locks: %v", body) + assert.Equal(t, "MFA_LOCKED", body["code"]) + assert.Equal(t, "900", resp.Header.Get("Retry-After")) + + // A fresh login and the right code: still refused while the lock holds. + resp, body = s.raw(t, s.mfaToken(t, s.memberA.User.Email), liveCode(t, plain)) + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode) + assert.Equal(t, "MFA_LOCKED", body["code"]) + assert.NotEmpty(t, resp.Header.Get("Retry-After")) +} + +func TestMFAChallengeE2E_IPLimitRunsBeforeTheToken(t *testing.T) { + s := newChallengeStack(t, 3) + for i := range 3 { + status, _ := s.do(t, http.MethodPost, challengePath, "not-a-token", jsonBody{"code": "123456"}) + require.Equal(t, http.StatusUnauthorized, status, "request %d reaches the token check", i+1) + } + status, _ := s.do(t, http.MethodPost, challengePath, "not-a-token", jsonBody{"code": "123456"}) + assert.Equal(t, http.StatusTooManyRequests, status, "the fourth is refused before the token is parsed") +} + +// raw posts a code and keeps the response headers. +func (s *challengeStack) raw(t *testing.T, token, code string) (*http.Response, jsonBody) { + t.Helper() + raw, err := json.Marshal(jsonBody{"code": code}) + require.NoError(t, err) + req := httptest.NewRequest(http.MethodPost, challengePath, strings.NewReader(string(raw))) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + resp, err := s.app.Test(req, -1) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + out := jsonBody{} + _ = json.NewDecoder(resp.Body).Decode(&out) + return resp, out +} diff --git a/backend/internal/handler/auth/mfa_handler.go b/backend/internal/handler/auth/mfa_handler.go index 9f371eb4..b01719e6 100644 --- a/backend/internal/handler/auth/mfa_handler.go +++ b/backend/internal/handler/auth/mfa_handler.go @@ -7,6 +7,7 @@ package auth import ( "errors" + "strconv" "time" "github.com/gofiber/fiber/v2" @@ -17,6 +18,7 @@ import ( "github.com/opendefender/openrisk/internal/domain" "github.com/opendefender/openrisk/internal/infrastructure/repository" "github.com/opendefender/openrisk/internal/middleware" + authpkg "github.com/opendefender/openrisk/pkg/auth" ) // MFAHandler exposes the /auth/mfa/* endpoints: setup + verify (enrollment, under @@ -296,12 +298,17 @@ func (h *MFAHandler) Challenge(c *fiber.Ctx) error { return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"error": "invalid request body"}) } - if _, err := h.challenge.Execute(c.UserContext(), appauth.ChallengeMFAInput{UserID: userID, TenantID: tenantID, Code: req.Code}); err != nil { - if h.audit != nil { - reason := "invalid MFA code" - _ = h.audit.LogFiber(c, &userID, &tenantID, coreauth.AuditActionMfaVerify, false, &reason) - } - return mapAuthError(c, err) + input := appauth.ChallengeMFAInput{UserID: userID, TenantID: tenantID, Code: req.Code} + // The token's own identity, so attempts are counted against it (#689). + if jti, ok := c.Locals("jti").(string); ok { + input.ChallengeJTI = jti + } + if claims, ok := c.Locals("user").(*authpkg.Claims); ok && claims.ExpiresAt != nil { + input.ChallengeExpiresAt = claims.ExpiresAt.Time + } + + if _, err := h.challenge.Execute(c.UserContext(), input); err != nil { + return h.challengeFailed(c, userID, tenantID, err) } fp := req.DeviceFingerprint @@ -336,6 +343,56 @@ func (h *MFAHandler) Challenge(c *fiber.Ctx) error { return c.JSON(LoginResponse{TokenPair: pair, CSRFToken: csrfToken}) } +// challengeFailed answers a refused challenge and audits it. The codes let the +// sign-in screen tell "wrong code" from "start again" from "wait". +func (h *MFAHandler) challengeFailed(c *fiber.Ctx, userID, tenantID uuid.UUID, err error) error { + locale := resolveLocale(c, "") + audit := func(reason string) { + if h.audit != nil { + _ = h.audit.LogFiber(c, &userID, &tenantID, coreauth.AuditActionMfaVerify, false, &reason) + } + } + + var locked *appauth.MFAChallengeLockedError + var appErr *domain.AppError + switch { + case errors.As(err, &locked): + // Audited once, when the lock is set. The refusals that follow are the + // lock working, and would drown the event that matters. + if locked.JustLocked { + audit("mfa_locked") + } + seconds := int(locked.RetryAfter.Round(time.Second) / time.Second) + if seconds < 1 { + seconds = 1 + } + c.Set(fiber.HeaderRetryAfter, strconv.Itoa(seconds)) + return c.Status(fiber.StatusTooManyRequests).JSON(fiber.Map{ + "code": "MFA_LOCKED", + "retry_after": seconds, + "error": pick(locale, + "Trop de codes erronés. La connexion à deux facteurs est suspendue pour ce compte, réessayez plus tard.", + "Too many wrong codes. Two-factor sign-in is paused for this account, try again later."), + }) + case errors.Is(err, appauth.ErrMFAChallengeExhausted): + audit("mfa_challenge_exhausted") + return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{ + "code": "MFA_CHALLENGE_EXHAUSTED", + "error": pick(locale, + "Trop de codes erronés pour cette connexion. Reconnectez-vous avec votre mot de passe.", + "Too many wrong codes for this sign-in. Sign in again with your password."), + }) + case errors.As(err, &appErr): + audit("invalid MFA code") + return mapAuthError(c, err) + default: + // A store or decryption failure: nothing the caller can fix, and its + // text is not theirs to read. + audit("internal") + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": genericFailure(locale)}) + } +} + // mapAuthError maps typed domain errors to HTTP status codes. func mapAuthError(c *fiber.Ctx, err error) error { if appErr, ok := err.(*domain.AppError); ok { From 90eb811dbb8e47538eb7145753fbce9a44bc28b9 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:25:55 +0100 Subject: [PATCH 5/6] fix(auth): revoke the access token on logout (#689) Logout deleted the refresh token and cleared the cookies, but the access token kept working for up to fifteen minutes. Whoever had copied it kept the session. The handler now reads the access token from the Authorization header or the session cookie, verifies its signature, and the use case blacklists its JTI for the time it has left. The access token is revoked before the refresh token, and both are always attempted, so a stale refresh token or a Redis outage on one does not spare the other. A forged token is ignored rather than trusted. Signed-off-by: alex-dembele --- backend/cmd/server/main.go | 7 +- backend/internal/application/auth/logout.go | 53 +++++++-- .../internal/application/auth/logout_test.go | 107 ++++++++++++++++++ backend/internal/handler/auth/handler.go | 55 ++++++++- .../internal/handler/auth/logout_e2e_test.go | 70 ++++++++++++ .../handler/auth/mfa_challenge_e2e_test.go | 5 +- 6 files changed, 278 insertions(+), 19 deletions(-) create mode 100644 backend/internal/application/auth/logout_test.go create mode 100644 backend/internal/handler/auth/logout_e2e_test.go diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index 38e78fbd..ac7714e1 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -687,7 +687,9 @@ func main() { // Organization + owner + root membership in one transaction (#687). WithAccounts(repository.NewGormRegistrationRepository(database.DB)) refreshUseCase := auth.NewRefreshTokenUseCase(tokenManager) - logoutUseCase := auth.NewLogoutUseCase(tokenManager) + // #689 — logout also blacklists the access token it was called with, so the + // session ends now instead of at the end of the access TTL. + logoutUseCase := auth.NewLogoutUseCase(tokenManager).WithAccessTokenRevocation(tokenBlacklistManager) // MFA use cases + handler. setupMFAUseCase := auth.NewSetupMFAUseCase(mfaRepo, mfaKey[:]) @@ -764,7 +766,8 @@ func main() { // The same repository resolveSessionForOrg reads at token mint time, so // /auth/me's business role and the token's permissions come from one // membership row and cannot disagree (#338). - WithMemberLookup(userRepo) + WithMemberLookup(userRepo). + WithAccessTokenRevocation(rsaKeys) // OAuth identity resolution: known link → verified-email link → provision. // No provisioner is wired, so an identity with no OpenRisk account is refused diff --git a/backend/internal/application/auth/logout.go b/backend/internal/application/auth/logout.go index d84c9fae..b22147ff 100644 --- a/backend/internal/application/auth/logout.go +++ b/backend/internal/application/auth/logout.go @@ -7,39 +7,70 @@ package auth import ( "context" + "errors" "fmt" - - "github.com/opendefender/openrisk/internal/auth" + "time" ) // LogoutInput represents the input for user logout type LogoutInput struct { RefreshToken string + // AccessJTI and AccessExpiresAt identify the access token the request came + // with, already verified by the caller. Revoking it is what makes logout + // end the session now rather than when the token expires (#689). + AccessJTI string + AccessExpiresAt time.Time +} + +// RefreshTokenRevoker deletes a refresh token. Satisfied by *auth.TokenManager. +type RefreshTokenRevoker interface { + RevokeRefreshToken(ctx context.Context, refreshTokenValue string) error } // LogoutUseCase handles user logout type LogoutUseCase struct { - tokenManager *auth.TokenManager + tokenManager RefreshTokenRevoker + accessTokens TokenRevoker } // NewLogoutUseCase creates a new logout use case -func NewLogoutUseCase(tokenManager *auth.TokenManager) *LogoutUseCase { +func NewLogoutUseCase(tokenManager RefreshTokenRevoker) *LogoutUseCase { return &LogoutUseCase{ tokenManager: tokenManager, } } -// Execute performs user logout +// WithAccessTokenRevocation blacklists the access token on logout. Without it +// the access token stays valid until it expires, up to AccessTokenTTL. +func (uc *LogoutUseCase) WithAccessTokenRevocation(r TokenRevoker) *LogoutUseCase { + uc.accessTokens = r + return uc +} + +// Execute performs user logout. +// +// The access token is revoked first: a refresh token that is already gone (a +// second logout, a rotated cookie) must not leave the access token working. +// Both revocations are always attempted, so a Redis outage on the first never +// spares the refresh token. func (uc *LogoutUseCase) Execute(ctx context.Context, input LogoutInput) error { - // Validate input - if input.RefreshToken == "" { + if input.RefreshToken == "" && input.AccessJTI == "" { return fmt.Errorf("refresh token is required") } - // Revoke the refresh token - if err := uc.tokenManager.RevokeRefreshToken(ctx, input.RefreshToken); err != nil { - return fmt.Errorf("failed to revoke refresh token: %w", err) + var accessErr error + if input.AccessJTI != "" && uc.accessTokens != nil { + if ttl := time.Until(input.AccessExpiresAt); ttl > 0 { + if err := uc.accessTokens.BlacklistJTI(ctx, input.AccessJTI, ttl); err != nil { + accessErr = fmt.Errorf("failed to revoke access token: %w", err) + } + } } - return nil + if input.RefreshToken != "" { + if err := uc.tokenManager.RevokeRefreshToken(ctx, input.RefreshToken); err != nil { + return errors.Join(fmt.Errorf("failed to revoke refresh token: %w", err), accessErr) + } + } + return accessErr } diff --git a/backend/internal/application/auth/logout_test.go b/backend/internal/application/auth/logout_test.go new file mode 100644 index 00000000..5df5590c --- /dev/null +++ b/backend/internal/application/auth/logout_test.go @@ -0,0 +1,107 @@ +// 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" + "errors" + "testing" + "time" +) + +type fakeRefreshRevoker struct { + known map[string]bool + revoked []string +} + +func (f *fakeRefreshRevoker) RevokeRefreshToken(_ context.Context, v string) error { + if !f.known[v] { + return errors.New("refresh token not found") + } + f.revoked = append(f.revoked, v) + return nil +} + +type failingJTIRevoker struct{ calls int } + +func (f *failingJTIRevoker) BlacklistJTI(context.Context, string, time.Duration) error { + f.calls++ + return errors.New("redis down") +} + +func newLogoutFixture() (*LogoutUseCase, *fakeRefreshRevoker, *fakeJTIRevoker) { + refresh := &fakeRefreshRevoker{known: map[string]bool{"rt-1": true}} + access := &fakeJTIRevoker{jtis: map[string]time.Duration{}} + return NewLogoutUseCase(refresh).WithAccessTokenRevocation(access), refresh, access +} + +func TestLogout_Success(t *testing.T) { + uc, refresh, access := newLogoutFixture() + err := uc.Execute(context.Background(), LogoutInput{ + RefreshToken: "rt-1", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(10 * time.Minute), + }) + if err != nil { + t.Fatal(err) + } + if len(refresh.revoked) != 1 { + t.Fatal("the refresh token must be revoked") + } + ttl, ok := access.jtis["jti-1"] + if !ok || ttl <= 9*time.Minute || ttl > 10*time.Minute { + t.Fatalf("the access token must be blacklisted for what it has left, got %v %v", ok, ttl) + } +} + +// A refresh token that is already gone still lets the access token die. +func TestLogout_NotFound(t *testing.T) { + uc, _, access := newLogoutFixture() + err := uc.Execute(context.Background(), LogoutInput{ + RefreshToken: "rt-unknown", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(time.Minute), + }) + if err == nil { + t.Fatal("an unknown refresh token must be reported") + } + if !access.revoked("jti-1") { + t.Fatal("the access token must be revoked even when the refresh token is unknown") + } +} + +func TestLogout_Unauthorized(t *testing.T) { + uc, refresh, access := newLogoutFixture() + if err := uc.Execute(context.Background(), LogoutInput{}); err == nil { + t.Fatal("a logout with no credential must fail") + } + if len(refresh.revoked) != 0 || len(access.jtis) != 0 { + t.Fatal("nothing may be revoked without a credential") + } +} + +func TestLogout_ExpiredAccessTokenNeedsNoBlacklist(t *testing.T) { + uc, _, access := newLogoutFixture() + _ = uc.Execute(context.Background(), LogoutInput{ + RefreshToken: "rt-1", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(-time.Second), + }) + if access.revoked("jti-1") { + t.Fatal("an expired token is already dead; blacklisting it wastes a key") + } +} + +// A Redis outage on the blacklist must not spare the refresh token. +func TestLogout_BlacklistOutageStillRevokesRefreshToken(t *testing.T) { + refresh := &fakeRefreshRevoker{known: map[string]bool{"rt-1": true}} + broken := &failingJTIRevoker{} + uc := NewLogoutUseCase(refresh).WithAccessTokenRevocation(broken) + + err := uc.Execute(context.Background(), LogoutInput{ + RefreshToken: "rt-1", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(time.Minute), + }) + if err == nil { + t.Fatal("the outage must be reported") + } + if broken.calls != 1 || len(refresh.revoked) != 1 { + t.Fatalf("both revocations must be attempted: blacklist=%d refresh=%d", broken.calls, len(refresh.revoked)) + } +} diff --git a/backend/internal/handler/auth/handler.go b/backend/internal/handler/auth/handler.go index 6989452e..aa5b788d 100644 --- a/backend/internal/handler/auth/handler.go +++ b/backend/internal/handler/auth/handler.go @@ -8,6 +8,7 @@ package auth import ( "context" "errors" + "strings" "github.com/gofiber/fiber/v2" "github.com/google/uuid" @@ -15,6 +16,7 @@ import ( coreauth "github.com/opendefender/openrisk/internal/auth" "github.com/opendefender/openrisk/internal/domain" "github.com/opendefender/openrisk/internal/middleware" + authpkg "github.com/opendefender/openrisk/pkg/auth" ) // Handler handles authentication endpoints @@ -39,6 +41,17 @@ type Handler struct { // CURRENT business role. Optional; without it the field is omitted rather // than guessed (#338). memberLookup OrganizationMemberReader + // accessTokenKeys verifies the access token presented at logout so its + // JTI can be revoked (#689). Optional; without it logout ends only the + // refresh token, and the access token lives out its TTL. + accessTokenKeys *authpkg.RSAKeys +} + +// WithAccessTokenRevocation lets logout verify and revoke the access token it +// was called with. The use case must be wired with a revoker too. +func (h *Handler) WithAccessTokenRevocation(keys *authpkg.RSAKeys) *Handler { + h.accessTokenKeys = keys + return h } // UserByIDReader resolves a user by id for /auth/me. @@ -490,13 +503,19 @@ func (h *Handler) Logout(c *fiber.Ctx) error { // cookies behind is worse than one that reports an error. middleware.ClearSessionCookies(c) - if req.RefreshToken == "" { + input := auth.LogoutInput{RefreshToken: req.RefreshToken} + if claims := h.presentedAccessToken(c); claims != nil { + input.AccessJTI = claims.JTI + if claims.ExpiresAt != nil { + input.AccessExpiresAt = claims.ExpiresAt.Time + } + } + + if input.RefreshToken == "" && input.AccessJTI == "" { return c.JSON(fiber.Map{"message": "Logged out"}) } - err := h.logoutUseCase.Execute(c.UserContext(), auth.LogoutInput{ - RefreshToken: req.RefreshToken, - }) + err := h.logoutUseCase.Execute(c.UserContext(), input) if err != nil { reason := "logout failed" @@ -512,6 +531,34 @@ func (h *Handler) Logout(c *fiber.Ctx) error { }) } +// presentedAccessToken returns the verified claims of the access token sent +// with the request, from the Authorization header or the session cookie, the +// same order the auth middleware reads them. A missing, forged or expired +// token yields nil: logout still clears the cookies, there is just nothing +// left to revoke. A valid signature is required, so nobody can blacklist a +// JTI they do not hold. +func (h *Handler) presentedAccessToken(c *fiber.Ctx) *authpkg.Claims { + if h.accessTokenKeys == nil { + return nil + } + token := "" + if header := c.Get("Authorization"); header != "" { + if parts := strings.Split(header, " "); len(parts) == 2 && parts[0] == "Bearer" { + token = parts[1] + } + } else { + token = c.Cookies(middleware.AccessTokenCookie) + } + if token == "" { + return nil + } + claims, err := authpkg.ValidateAccessToken(h.accessTokenKeys, token, nil) + if err != nil || claims.JTI == "" { + return nil + } + return claims +} + // Me godoc // @Summary Get current user profile // @Description Get profile of currently authenticated user diff --git a/backend/internal/handler/auth/logout_e2e_test.go b/backend/internal/handler/auth/logout_e2e_test.go new file mode 100644 index 00000000..164a7b43 --- /dev/null +++ b/backend/internal/handler/auth/logout_e2e_test.go @@ -0,0 +1,70 @@ +// 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 + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/opendefender/openrisk/internal/middleware" +) + +// #689 — logout ends the access token too, not just the refresh token. + +func TestLogout_AccessTokenRejectedAfterLogout(t *testing.T) { + s := newChallengeStack(t, 100) + _, access := s.login(t, s.memberA.User.Email) + require.NotEmpty(t, access) + + status, _ := s.do(t, http.MethodGet, "/api/v1/auth/me", access, nil) + require.Equal(t, http.StatusOK, status, "the token works before logout") + + status, body := s.do(t, http.MethodPost, "/api/v1/auth/logout", access, nil) + require.Equal(t, http.StatusOK, status, "%v", body) + + status, body = s.do(t, http.MethodGet, "/api/v1/auth/me", access, nil) + assert.Equal(t, http.StatusUnauthorized, status) + assert.Equal(t, "TOKEN_REVOKED", body["code"]) +} + +// A browser holds the token in the session cookie, not in a header. +func TestLogout_CookieSessionAccessTokenRejectedAfterLogout(t *testing.T) { + s := newChallengeStack(t, 100) + _, access := s.login(t, s.memberA.User.Email) + require.NotEmpty(t, access) + + withCookie := func(method, path string) *http.Response { + req := httptest.NewRequest(method, path, strings.NewReader("")) + req.AddCookie(&http.Cookie{Name: middleware.AccessTokenCookie, Value: access}) + resp, err := s.app.Test(req, -1) + require.NoError(t, err) + _ = resp.Body.Close() + return resp + } + + require.Equal(t, http.StatusOK, withCookie(http.MethodGet, "/api/v1/auth/me").StatusCode) + require.Equal(t, http.StatusOK, withCookie(http.MethodPost, "/api/v1/auth/logout").StatusCode) + assert.Equal(t, http.StatusUnauthorized, withCookie(http.MethodGet, "/api/v1/auth/me").StatusCode) +} + +// A forged token at logout revokes nothing and still answers 200: the caller +// loses their cookies either way. +func TestLogout_ForgedTokenIsIgnored(t *testing.T) { + s := newChallengeStack(t, 100) + _, access := s.login(t, s.memberA.User.Email) + + forged := access[:len(access)-4] + "AAAA" + status, _ := s.do(t, http.MethodPost, "/api/v1/auth/logout", forged, nil) + assert.Equal(t, http.StatusOK, status) + + status, _ = s.do(t, http.MethodGet, "/api/v1/auth/me", access, nil) + assert.Equal(t, http.StatusOK, status, "the real token was not revoked by a forgery") +} diff --git a/backend/internal/handler/auth/mfa_challenge_e2e_test.go b/backend/internal/handler/auth/mfa_challenge_e2e_test.go index 916956b1..1cf11fae 100644 --- a/backend/internal/handler/auth/mfa_challenge_e2e_test.go +++ b/backend/internal/handler/auth/mfa_challenge_e2e_test.go @@ -99,10 +99,11 @@ func newChallengeStack(t *testing.T, ipLimit int) *challengeStack { RequireMFAForRoles(orgRoles, businessRoles). WithMFAPolicies(f.policyRepo). WithClock(func() time.Time { return f.now }) - logoutUC := appauth.NewLogoutUseCase(tokens) + logoutUC := appauth.NewLogoutUseCase(tokens).WithAccessTokenRevocation(blacklist) h := authhandler.NewHandler(loginUC, nil, nil, logoutUC, deferredHasher{}, nil). WithUserLookup(userRepo). - WithMFAStatus(f.resolver) + WithMFAStatus(f.resolver). + WithAccessTokenRevocation(f.keys) challengeUC := appauth.NewChallengeMFAUseCase(mfaRepo, deferredTOTPKey). WithAttemptLimits(authmfa.NewMemoryAttemptStore(), blacklist) From f4b34f32d750f25f63a465f2bd237fab16fcd216 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Fri, 2 Oct 2026 08:44:00 +0100 Subject: [PATCH 6/6] docs(auth): mark the #689 plan done (#689) Signed-off-by: alex-dembele --- docs/689_MFA_CHALLENGE_LIMITS.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/689_MFA_CHALLENGE_LIMITS.md b/docs/689_MFA_CHALLENGE_LIMITS.md index 468eb04b..23238a24 100644 --- a/docs/689_MFA_CHALLENGE_LIMITS.md +++ b/docs/689_MFA_CHALLENGE_LIMITS.md @@ -161,34 +161,34 @@ Live check: throwaway pg + redis on spare ports, run the server from the repo ro Each task ends green on its own `go test` and is one commit (`fix(auth): … (#689)`). -- [ ] **T1 — Attempt store.** Adds the `MFAAttemptStore` port in `application/auth`, and `infrastructure/authmfa/attempts.go`: Redis (`INCR` + `EXPIRE` on the first increment, `SET` with TTL, `EXISTS`, `DEL`) with an in-memory fallback. +- [x] **T1 — Attempt store.** Adds the `MFAAttemptStore` port in `application/auth`, and `infrastructure/authmfa/attempts.go`: Redis (`INCR` + `EXPIRE` on the first increment, `SET` with TTL, `EXISTS`, `DEL`) with an in-memory fallback. - Accept: counts, expires, falls back when Redis errors. - Verify: `go test ./internal/infrastructure/authmfa/ -race` - Files: `application/auth/mfa_limits.go` (new), `infrastructure/authmfa/attempts.go`, `attempts_test.go` -- [ ] **T2 — Per-token and per-account limits in the use case.** +- [x] **T2 — Per-token and per-account limits in the use case.** - `WithAttemptLimits(store, revoker)`; input `ChallengeJTI` and `ChallengeExpiresAt`; `ErrMFAChallengeExhausted` and `ErrMFAChallengeLocked{RetryAfter}`. - Checks the lock and the token count before verifying. Counts failures, blacklists the token at 5, locks at 10, clears the user counter on success. - Accept: success criteria 1, 2 and 4 at the use-case level. - Verify: `go test ./internal/application/auth/ -run MFAChallenge -race -v` - Files: `mfa_usecase.go`, `mfa_limits.go`, `mfa_challenge_limits_test.go` -- [ ] **T3 — Lock email.** `MFALockedMailer` port, sent once when the lock is set; FR/EN template in `authmail`. +- [x] **T3 — Lock email.** `MFALockedMailer` port, sent once when the lock is set; FR/EN template in `authmail`. - Accept: one email per lock, not per refused attempt; a mail failure does not change the result. - Verify: `go test ./internal/application/auth/ ./internal/infrastructure/authmail/ -race` - Files: `mfa_limits.go`, `authmail/` mailer and its test -- [ ] **T4 — Handler and route.** `Challenge` passes the JTI and expiry and maps the errors (401 `MFA_CHALLENGE_EXHAUSTED`, 429 `MFA_LOCKED` with `Retry-After`). In `main.go`: wiring plus the per-IP `RateLimit` with the `mfa-challenge` prefix, mounted before `MFATokenMiddleware`. +- [x] **T4 — Handler and route.** `Challenge` passes the JTI and expiry and maps the errors (401 `MFA_CHALLENGE_EXHAUSTED`, 429 `MFA_LOCKED` with `Retry-After`). In `main.go`: wiring plus the per-IP `RateLimit` with the `mfa-challenge` prefix, mounted before `MFATokenMiddleware`. - Accept: success criteria 1, 2 and 3 over HTTP. - Verify: `go test ./internal/handler/auth/ -run MFAChallenge -race -v` - Files: `mfa_handler.go`, `main.go`, `mfa_challenge_e2e_test.go` -- [ ] **T5 — Logout revokes the access token.** A `TokenRevoker` in `LogoutUseCase` (input `AccessJTI` and `AccessExpiresAt`); the handler extracts and validates the bearer or cookie token; wiring in `main.go`. +- [x] **T5 — Logout revokes the access token.** A `TokenRevoker` in `LogoutUseCase` (input `AccessJTI` and `AccessExpiresAt`); the handler extracts and validates the bearer or cookie token; wiring in `main.go`. - Accept: success criterion 5, for both bearer and cookie. - Verify: `go test ./internal/application/auth/ ./internal/handler/auth/ -run Logout -race -v` - Files: `logout.go`, `handler.go`, `main.go`, tests -- [ ] **T6 — Full gate and live proof.** +- [x] **T6 — Full gate and live proof.** - Run `go build ./... && go vet ./... && go test ./... -race`. - Live boot with an enrolled account: criteria 1, 2 and 5 by curl, plus the lock email in the mail log. - Open the frontend follow-up issue, post the progress comment, open the PR with `Closes #689`, set `status:in-review`.