Fix/failed login lockout

database/03-tables.sql

@@ -6,7 +6,8 @@ CREATE TABLE users (
     password_hash TEXT NOT NULL,
     role user_role NOT NULL DEFAULT 'USER',
     is_active BOOLEAN DEFAULT TRUE,
-    failed_login_attempts INT DEFAULT 0 CHECK (failed_login_attempts >= 0 AND failed_login_attempts <= 3),
+    failed_login_attempts INT DEFAULT 0 CONSTRAINT users_failed_login_attempts_range
+        CHECK (failed_login_attempts >= 0 AND failed_login_attempts <= 5),
     locked_until TIMESTAMP,
     email_verified BOOLEAN DEFAULT FALSE,
     two_factor_enabled BOOLEAN DEFAULT FALSE,
@@ -20,7 +21,7 @@ CREATE TABLE users (
 
 COMMENT ON TABLE users IS 'System users with 2FA authentication and lock control';
 COMMENT ON COLUMN users.password_hash IS 'BCrypt hash - NEVER store passwords in plain text';
-COMMENT ON COLUMN users.locked_until IS 'NULL = unlocked, TIMESTAMP = unlocked until that date';
+COMMENT ON COLUMN users.locked_until IS 'NULL = unlocked, TIMESTAMP = locked until that date';
 COMMENT ON COLUMN users.two_factor_secret IS 'Base32 secret for TOTP (Google authenticator)';
 
 -- Table: wallets
@@ -120,4 +121,4 @@ CREATE TABLE transaction_history (
 
 COMMENT ON TABLE transaction_history IS 'Complete history of transaction status changes';
 
-CREATE INDEX idx_transaction_history_transaction ON transaction_history(transaction_id);
+CREATE INDEX idx_transaction_history_transaction ON transaction_history(transaction_id);

database/08-migrations.sql

@@ -21,3 +21,17 @@ ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'WALLET_RESTORE';
 -- Date: 2026-10-27
 -- OWASP A07/A09: a consumed refresh token presented again is a security signal.
 ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'TOKEN_REFRESH';
+
+-- Migration: align failed-login counter with the application lockout policy
+-- Date: 2026-10-31
+-- OWASP A07: the database must accept every state used by the authoritative
+--            five-attempt policy; otherwise a valid lockout update fails.
+ALTER TABLE users
+    DROP CONSTRAINT IF EXISTS users_failed_login_attempts_check;
+
+ALTER TABLE users
+    DROP CONSTRAINT IF EXISTS users_failed_login_attempts_range;
+
+ALTER TABLE users
+    ADD CONSTRAINT users_failed_login_attempts_range
+        CHECK (failed_login_attempts >= 0 AND failed_login_attempts <= 5);

docs/IMPLEMENTATION-PLAN.md

@@ -163,7 +163,7 @@ The former public seed is no longer used and a controlled bootstrap now exists.
 
 **Current problem**
 
-The refresh endpoint issues only a new access token and keeps the same refresh token. Reuse detection and atomic rotation are not implemented.
+Resolved. The refresh endpoint now consumes the presented session under a pessimistic row lock and creates a replacement session in the same transaction.
 
 **Expected behavior**
 
@@ -184,11 +184,17 @@ The refresh endpoint issues only a new access token and keeps the same refresh t
 
 **Acceptance criteria**
 
-- [ ] A successful refresh returns two new tokens.
-- [ ] The previous refresh token fails immediately afterward.
-- [ ] Exactly one of two concurrent refresh requests with the same token succeeds.
-- [ ] Rotation and reuse failures create audit evidence without logging token values.
-- [ ] Logout revokes the current rotated session.
+- [x] A successful refresh returns two new tokens.
+- [x] The previous refresh token fails immediately afterward.
+- [x] Exactly one of two concurrent refresh requests with the same token succeeds through `PESSIMISTIC_WRITE` session locking.
+- [x] Rotation and reuse failures create audit evidence without logging token values.
+- [x] Logout revokes the current rotated session.
+
+**Verification note — 2026-10-27**
+
+- `SessionService.rotateSession()` locks the active session row, revokes it and creates its replacement in one transaction; a `jti` makes refresh JWTs unique even when issued in the same second.
+- A real Docker/PostgreSQL flow confirmed refresh rotation, rejection of the old token, and logout revocation of the rotated token.
+- `./scripts/test.sh` passed: 112 tests, 0 failures and 0 errors.
 
 ### SEC-005 — Correct failed-login lockout
 
@@ -198,7 +204,7 @@ The refresh endpoint issues only a new access token and keeps the same refresh t
 
 **Current problem**
 
-Failed login events are audited, but the authentication flow does not demonstrably call repository operations that increment attempts and set `lockedUntil`. Values also conflict: application configuration states five attempts while the database constrains the counter to three.
+Resolved in code. Failed attempts are serialized with a pessimistic user-row lock and persisted in a `REQUIRES_NEW` transaction, so rethrowing the authentication failure cannot roll the counter back. The database and configuration now both use five attempts and a 30-minute lock.
 
 **Expected behavior**
 
@@ -222,12 +228,19 @@ Failed login events are audited, but the authentication flow does not demonstrab
 
 **Acceptance criteria**
 
-- [ ] The configured number of consecutive failures locks the account.
-- [ ] PostgreSQL accepts every valid counter state under the selected policy.
-- [ ] A correct password during the lock period cannot authenticate.
-- [ ] Authentication succeeds after expiry according to the chosen unlock policy.
-- [ ] Success resets failed attempts to zero.
-- [ ] Unit and PostgreSQL-backed tests cover boundary and concurrency cases.
+- [x] The configured number of consecutive failures locks the account.
+- [x] PostgreSQL accepts every valid counter state under the selected policy through the migration and schema constraint.
+- [x] A correct password during the lock period cannot authenticate through Spring Security's account-lock status.
+- [x] Authentication succeeds after expiry through the time-based `isAccountLocked()` policy; an expired lock starts a new counter sequence and a successful login resets it to zero.
+- [x] Success resets failed attempts to zero.
+- [x] Docker/PostgreSQL concurrency verification proves that simultaneous failed attempts serialize correctly. Unit tests cover boundary behavior, unknown-email non-enumeration, locked-account state and the failure handoff.
+
+**Verification note — 2026-10-31**
+
+- `LoginAttemptService` uses `REQUIRES_NEW` and `findByEmailWithLock()` because `AuthService.login()` rethrows `BadCredentialsException`; without the independent transaction, the counter would roll back with the failed login.
+- The fifth consecutive failed attempt caps the counter at five, sets `locked_until` for 30 minutes, writes a failure audit event and writes a CRITICAL lockout event. When that lock expires, the next failure starts again at one. No raw credentials are logged.
+- `./scripts/test.sh` passed: 118 tests, 0 failures and 0 errors.
+- Docker/PostgreSQL verification: five simultaneous incorrect requests all returned `401`; the resulting row had `failed_login_attempts = 5` and an active `locked_until`; a following correct-password request returned `423`.
 
 ### SEC-006 — Reject JWT authentication for disabled or locked accounts
 

src/main/java/com/wallet/secure/auth/service/AuthService.java

@@ -50,6 +50,7 @@ public class AuthService {
     private final UserService userService;
     private final AuditService auditService;
     private final SessionService sessionService;
+    private final LoginAttemptService loginAttemptService;
 
     // --- Register
 
@@ -162,7 +163,8 @@ private String extractUserAgent(HttpServletRequest request) {
      * </p>
      * OWASP A07:
      * - BadCredentialsException -> same message as "User not found" (prevents user enumeration)
-     * - Failed attempts tracked -> account lockout after 3 failures
+     * - Failed attempts are persisted independently -> lockout after the
+     *   configured threshold (five by default), even though login itself fails
      * - LastLoginAt updated on success -> suspicious activity detection
      *
      * @param request DTO with email and password
@@ -180,38 +182,33 @@ public ApiResponse<AuthResponse> login(LoginRequest request, HttpServletRequest
                             request.getEmail(),
                             request.getPassword()));
         } catch (LockedException e) {
-            // Account is locked - log as WARNING, not CRITICAL (already handled)
+            // OWASP A09: a blocked attempt is security-relevant, but the CRITICAL
+            // event was already recorded when the threshold originally was reached.
             userRepository.findByEmail(request.getEmail()).ifPresent(user ->
                     auditService.logLoginFailure(user.getId(),
                             request.getEmail(),
                             "Account locked", extractIp(httpRequest), extractUserAgent(httpRequest)));
             throw e;
         } catch (BadCredentialsException e) {
-            // Log failure - check if brute force threshold is reached
-            userRepository.findByEmail(request.getEmail()).ifPresent(user -> {
-                auditService.logLoginFailure(user.getId(),
-                        request.getEmail(),
-                        "Bad credentials", extractIp(httpRequest), extractUserAgent(httpRequest));
-                // OWASP A07: brute force detection
-                // If >= 5 failures in 15 minutes -> CRITICAL alert
-                long recentFailures = auditService.countRecentFailedLogins(user.getId(), 15);
-                if (recentFailures >= 5) {
-                    auditService.logCriticalSecurityEvent(
-                            user.getId(),
-                            String.format("Brute force detected: %s failed logins in 15 minutes", recentFailures),
-
-                            extractIp(httpRequest), extractUserAgent(httpRequest));
-                }
-            });
+            // OWASP A07: persist the counter in an independent transaction because
+            // this method rethrows and its transaction will roll back.
+            loginAttemptService.recordFailure(request.getEmail(), ip, userAgent);
             throw e;
         }
 
         // Only reached if authentication succeeded
-        User user = userRepository.findByEmail(request.getEmail())
+        User user = userRepository.findByEmailWithLock(request.getEmail())
                 .orElseThrow(() -> new InvalidCredentialsException("Invalid credentials"));
 
-        // Reset failed attempts counter + update lastLoginAt
-        // OWASP A07: counter reset prevents permanent lockout after legitime login
+        // OWASP A07: authentication may have started just before another request
+        // reached the threshold. Recheck after obtaining the same row lock.
+        if (user.isAccountLocked()) {
+            auditService.logLoginFailure(user.getId(), user.getEmail(), "Account locked", ip, userAgent);
+            throw new LockedException("Account temporarily locked");
+        }
+
+        // A legitimate login closes the current failure sequence.
+        // OWASP A07: reset removes stale failures and clears an expired lock marker.
         user.resetFailedLoginAttempts();
         user.setLastLoginAt(Instant.now());
 
@@ -243,7 +240,7 @@ public ApiResponse<AuthResponse> login(LoginRequest request, HttpServletRequest
      * 4. Return both replacements; the old token is now invalid
      *
      * @param request DTO with refreshToken
-     * @return ApiResponse with new access token (refresh token not rotated here)
+     * @return ApiResponse with a new access token and a rotated refresh token
      */
     @Transactional
     public ApiResponse<AuthResponse> refresh(RefreshTokenRequest request, HttpServletRequest httpRequest)  {

src/main/java/com/wallet/secure/auth/service/LoginAttemptService.java

@@ -0,0 +1,70 @@
+package com.wallet.secure.auth.service;
+
+import com.wallet.secure.audit.service.AuditService;
+import com.wallet.secure.user.entity.User;
+import com.wallet.secure.user.repository.UserRepository;
+import lombok.RequiredArgsConstructor;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.time.Instant;
+
+/**
+ * Persists the security state for a failed login independently from authentication.
+ *
+ * WHY REQUIRES_NEW: AuthService rethrows BadCredentialsException, so its
+ * transaction rolls back. The counter, temporary lock and audit evidence must
+ * survive that rollback (OWASP A07/A09).
+ *
+ * WHY the row lock: simultaneous guesses for the same account must observe the
+ * latest counter instead of overwriting one another.
+ */
+@Service
+@RequiredArgsConstructor
+public class LoginAttemptService {
+
+    private final UserRepository userRepository;
+    private final AuditService auditService;
+
+    @Value("${app.security.max-login-attempts}")
+    private int maxLoginAttempts;
+
+    @Value("${app.security.lockout-duration-minutes}")
+    private long lockoutDurationMinutes;
+
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public void recordFailure(String email, String ipAddress, String userAgent) {
+        userRepository.findByEmailWithLock(email).ifPresent(user -> recordFailure(user, email, ipAddress, userAgent));
+    }
+
+    private void recordFailure(User user, String email, String ipAddress, String userAgent) {
+        // OWASP A07: a row lock makes each increment observe the latest count.
+        if (user.isAccountLocked()) {
+            auditService.logLoginFailure(user.getId(), email, "Account locked", ipAddress, userAgent);
+            return;
+        }
+
+        // The lock has expired: begin a new failure sequence. Without this reset,
+        // one wrong password after expiry would immediately lock the user again.
+        // OWASP A07: temporary lockouts must recover predictably for legitimate users.
+        if (user.getLockedUntil() != null) {
+            user.resetFailedLoginAttempts();
+        }
+
+        user.incrementFailedLoginAttempts();
+        boolean locked = user.getFailedLoginAttempts() >= maxLoginAttempts;
+        if (locked) {
+            user.setFailedLoginAttempts(maxLoginAttempts);
+            user.setLockedUntil(Instant.now().plusSeconds(lockoutDurationMinutes * 60));
+        }
+
+        auditService.logLoginFailure(user.getId(), email,
+                locked ? "Account locked after failed attempts" : "Bad credentials", ipAddress, userAgent);
+        if (locked) {
+            auditService.logCriticalSecurityEvent(user.getId(),
+                    "Account locked after " + maxLoginAttempts + " failed login attempts", ipAddress, userAgent);
+        }
+    }
+}

src/main/java/com/wallet/secure/user/entity/User.java

@@ -93,8 +93,8 @@ public class User {
 
     /**
      * Failed login counter — reset to 0 on successful login.
-     * DB constraint: CHECK (>= 0 AND <= 3).
-     * When reaches 3 → lockUntil is set.
+     * DB constraint: CHECK (>= 0 AND <= 5).
+     * When reaches the configured threshold → lockUntil is set.
      * OWASP A07: protection against brute force attacks.
      */
     @Column(name = "failed_login_attempts", nullable = false)
@@ -175,8 +175,9 @@ public class User {
     // ─── Business Methods ─────────────────────────────────────────────────────
 
     /**
-     * Checks if the account is currently locked.
-     * Returns true if lockedUntil is set and is in the future.
+     * Checks whether the temporary lock is still active.
+     * A past lockedUntil value no longer blocks authentication; LoginAttemptService
+     * clears it before starting any new failed-attempt sequence.
      */
     public boolean isAccountLocked() {
         return lockedUntil != null && Instant.now().isBefore(lockedUntil);
@@ -197,14 +198,15 @@ public boolean isFullyActive() {
 
     /**
      * Increments failed login counter.
-     * Called by AuthService on each failed login attempt.
+     * Called by LoginAttemptService while it holds the user's row lock.
      */
     public void incrementFailedLoginAttempts() {
         this.failedLoginAttempts = (this.failedLoginAttempts == null ? 0 : this.failedLoginAttempts) + 1;
     }
 
     /**
-     * Resets failed login counter after successful login.
+     * Resets the current failed-login sequence after a successful login or after
+     * an expired lock is observed before a new sequence starts.
      */
     public void resetFailedLoginAttempts() {
         this.failedLoginAttempts = 0;

src/main/java/com/wallet/secure/user/repository/UserRepository.java

@@ -7,6 +7,9 @@
 import org.springframework.data.repository.query.Param;
 import org.springframework.stereotype.Repository;
 
+import jakarta.persistence.LockModeType;
+import org.springframework.data.jpa.repository.Lock;
+
 import java.time.Instant;
 import java.util.Optional;
 import java.util.UUID;
@@ -39,6 +42,14 @@ public interface UserRepository extends JpaRepository<User, UUID> {
      */
     Optional<User> findByEmail(String email);
 
+    /**
+     * Serializes failed-login accounting for one account.
+     * OWASP A07: concurrent guesses must not overwrite each other's counters.
+     */
+    @Lock(LockModeType.PESSIMISTIC_WRITE)
+    @Query("SELECT u FROM User u WHERE u.email = :email")
+    Optional<User> findByEmailWithLock(@Param("email") String email);
+
     /**
      * Check if an email is already registered.
      * More efficient than findByEmail() — only checks existence, no data transfer.
@@ -102,18 +113,21 @@ Optional<User> findActiveUserByEmail(
      * Atomic operation — avoids race conditions on concurrent login attempts.
      * OWASP A07: brute force protection counter.
      *
-     * Used by: AuthService on each failed login attempt.
+     * Retained for future batch or administrative operations. SEC-005 uses the
+     * locked entity returned by findByEmailWithLock() so the threshold decision
+     * and update are serialized in one transaction.
      */
     @Modifying
     @Query("UPDATE User u SET u.failedLoginAttempts = u.failedLoginAttempts + 1 WHERE u.id = :id")
     void incrementFailedLoginAttempts(@Param("id") UUID id);
 
     /**
      * Lock account until a specific timestamp.
-     * Called when failedLoginAttempts reaches MAX_FAILED_ATTEMPTS (3).
+     * Retained for future batch or administrative operations. SEC-005 applies
+     * the configured threshold while holding the user's pessimistic row lock.
      * OWASP A07: temporary lockout after repeated failures.
      *
-     * Used by: AuthService after 3rd consecutive failed login.
+     * The configured threshold is five by default (application.yml).
      */
     @Modifying
     @Query("UPDATE User u SET u.lockedUntil = :until, u.failedLoginAttempts = :attempts WHERE u.id = :id")
@@ -127,7 +141,8 @@ void lockAccount(
      * Reset failed attempts and remove lock — after successful login.
      * OWASP A07: clean state after legitimate access.
      *
-     * Used by: AuthService after successful authentication.
+     * Retained for future bulk operations; the SEC-005 entity flow resets the
+     * managed user after successful authentication.
      */
     @Modifying
     @Query("UPDATE User u SET u.failedLoginAttempts = 0, u.lockedUntil = NULL WHERE u.id = :id")