From 3a48ce0ff139a7d578955ce63d00cb9d22a307e0 Mon Sep 17 00:00:00 2001 From: lobo Date: Fri, 25 Sep 2026 09:06:30 +0200 Subject: [PATCH] feat(auth): rate limit registration, atomic login limit 10 failed registrations per IP block it for 15 minutes (429 TOO_MANY_ATTEMPTS with Retry-After). Login and registration now take an attempt before the check, so a burst of parallel requests cannot pass the limit. --- readme.md | 4 +- .../modules/login/api/LoginController.java | 16 +-- .../login/service/LoginAttemptService.java | 40 +++----- .../modules/login/service/LoginService.java | 5 +- .../api/RegistrationController.java | 6 +- .../api/RegistrationExceptionController.java | 11 +++ .../TooManyRegistrationAttemptsException.java | 13 +++ .../service/RegistrationAttemptService.java | 30 ++++++ .../service/RegistrationService.java | 19 ++-- .../auth_service/shared/api/ClientIp.java | 23 +++++ .../shared/security/AttemptLimiter.java | 49 ++++++++++ .../service/LoginAttemptServiceTest.java | 92 ++++++++++++++--- .../login/service/LoginServiceTest.java | 11 ++- .../api/RegistrationControllerTest.java | 48 +++++++-- .../RegistrationAttemptServiceTest.java | 98 +++++++++++++++++++ .../service/RegistrationServiceTest.java | 35 +++++-- .../auth_service/shared/api/ClientIpTest.java | 28 ++++++ 17 files changed, 445 insertions(+), 83 deletions(-) create mode 100644 src/main/java/auth_service/modules/registration/exception/TooManyRegistrationAttemptsException.java create mode 100644 src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java create mode 100644 src/main/java/auth_service/shared/api/ClientIp.java create mode 100644 src/main/java/auth_service/shared/security/AttemptLimiter.java create mode 100644 src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java create mode 100644 src/test/java/auth_service/shared/api/ClientIpTest.java diff --git a/readme.md b/readme.md index fdf659b..cfbc39d 100644 --- a/readme.md +++ b/readme.md @@ -14,6 +14,9 @@ To Be Played is split into three repositories: ## Features - Registration with an invitation code, login with email and password (BCrypt). +- Brute-force protection in memory: 5 failed logins per email or 20 per IP, and 10 failed registrations per IP, + block for 15 minutes with `429 TOO_MANY_ATTEMPTS` and `Retry-After`. Unknown emails cost the same BCrypt time as a + wrong password, and every failed login gets the same error. - JWT signed with **EdDSA (Ed25519)**. Auth holds the private key, Core only the public key, so Core can verify tokens but never create them. Tokens carry a `kid` header for key rotation. - User settings: display name (unique, case-insensitive), Steam ID, language, dark mode. @@ -99,7 +102,6 @@ Runs unit, WebMvc, persistence and integration tests plus the ArchUnit rules. Ne ## Roadmap -- Rate limit `/api/auth/login` and `/api/registration/**`. - JWKS endpoint (`/.well-known/jwks.json`), so Core fetches the public key and keys rotate without a redeploy. - Short-lived access token plus rotating refresh token in an httpOnly cookie, with CSRF protection. Today the token lives 24 h in `localStorage` and cannot be revoked. diff --git a/src/main/java/auth_service/modules/login/api/LoginController.java b/src/main/java/auth_service/modules/login/api/LoginController.java index 7a3d90d..bd6f6b2 100644 --- a/src/main/java/auth_service/modules/login/api/LoginController.java +++ b/src/main/java/auth_service/modules/login/api/LoginController.java @@ -4,6 +4,7 @@ import auth_service.modules.login.api.dto.request.LoginRequest; import auth_service.modules.login.api.dto.response.LoginResponse; import auth_service.modules.login.api.mapper.ApiMapper; import auth_service.modules.login.service.LoginService; +import auth_service.shared.api.ClientIp; import io.micrometer.core.annotation.Timed; import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; @@ -35,20 +36,7 @@ public class LoginController { public ResponseEntity login(@Valid @RequestBody LoginRequest request, HttpServletRequest httpRequest) { var user = ApiMapper.toModel(request); - var token = loginService.login(user, clientIp(httpRequest)); + var token = loginService.login(user, ClientIp.of(httpRequest)); return ResponseEntity.ok(new LoginResponse("Login successful", token)); } - - /** - * Traefik appends the real client IP as the last X-Forwarded-For entry. Earlier entries come - * from the client and can be faked, so only the last one counts. - */ - private static String clientIp(HttpServletRequest request) { - String forwardedFor = request.getHeader("X-Forwarded-For"); - if (forwardedFor == null || forwardedFor.isBlank()) { - return request.getRemoteAddr(); - } - String[] hops = forwardedFor.split(","); - return hops[hops.length - 1].trim(); - } } diff --git a/src/main/java/auth_service/modules/login/service/LoginAttemptService.java b/src/main/java/auth_service/modules/login/service/LoginAttemptService.java index ad4f4c1..7c815dd 100644 --- a/src/main/java/auth_service/modules/login/service/LoginAttemptService.java +++ b/src/main/java/auth_service/modules/login/service/LoginAttemptService.java @@ -1,13 +1,11 @@ package auth_service.modules.login.service; import auth_service.modules.login.exception.TooManyLoginAttemptsException; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; +import auth_service.shared.security.AttemptLimiter; import org.springframework.stereotype.Service; import java.time.Duration; import java.util.Locale; -import java.util.concurrent.atomic.AtomicInteger; /** * Counts failed logins per email and per client IP in memory. A key is blocked after too many @@ -20,36 +18,22 @@ public class LoginAttemptService { static final int MAX_FAILURES_PER_IP = 20; static final Duration LOCK_DURATION = Duration.ofMinutes(15); - private final Cache failures = Caffeine.newBuilder() - .expireAfterWrite(LOCK_DURATION) - .maximumSize(100_000) - .build(); + private final AttemptLimiter limiter = new AttemptLimiter(LOCK_DURATION); - public void checkAllowed(String email, String clientIp) { - if (count(emailKey(email)) >= MAX_FAILURES_PER_EMAIL || count(ipKey(clientIp)) >= MAX_FAILURES_PER_IP) { + public void acquire(String email, String clientIp) { + String emailKey = emailKey(email); + if (!limiter.tryAcquire(emailKey, MAX_FAILURES_PER_EMAIL)) { + throw new TooManyLoginAttemptsException(LOCK_DURATION.toSeconds()); + } + if (!limiter.tryAcquire(ipKey(clientIp), MAX_FAILURES_PER_IP)) { + limiter.release(emailKey); throw new TooManyLoginAttemptsException(LOCK_DURATION.toSeconds()); } } - public void recordFailure(String email, String clientIp) { - increment(emailKey(email)); - increment(ipKey(clientIp)); - } - - public void recordSuccess(String email) { - failures.invalidate(emailKey(email)); - } - - private int count(String key) { - AtomicInteger counter = failures.getIfPresent(key); - return counter == null ? 0 : counter.get(); - } - - private void increment(String key) { - // put() again so expireAfterWrite restarts the lock window with every failure - AtomicInteger counter = failures.get(key, k -> new AtomicInteger()); - counter.incrementAndGet(); - failures.put(key, counter); + public void recordSuccess(String email, String clientIp) { + limiter.reset(emailKey(email)); + limiter.release(ipKey(clientIp)); } private static String emailKey(String email) { diff --git a/src/main/java/auth_service/modules/login/service/LoginService.java b/src/main/java/auth_service/modules/login/service/LoginService.java index da75dc6..8434951 100644 --- a/src/main/java/auth_service/modules/login/service/LoginService.java +++ b/src/main/java/auth_service/modules/login/service/LoginService.java @@ -39,19 +39,18 @@ public class LoginService { */ public String login(User userToLogin, String clientIp) { String email = userToLogin.getEmail(); - loginAttemptService.checkAllowed(email, clientIp); + loginAttemptService.acquire(email, clientIp); Optional foundUser = minimalUserRepository.findByEmail(email); String hash = foundUser.map(User::getPassword).orElse(dummyHash); boolean passwordMatches = passwordEncoder.matches(userToLogin.getPassword(), hash); if (foundUser.isEmpty() || !passwordMatches || !foundUser.get().isEnabled()) { - loginAttemptService.recordFailure(email, clientIp); throw new InvalidCredentialsException("Invalid email or password"); } User user = foundUser.get(); - loginAttemptService.recordSuccess(email); + loginAttemptService.recordSuccess(email, clientIp); minimalUserRepository.updateLoginDate(user.getId()); return tokenGenerator.generateToken(String.valueOf(user.getId()), user.getEmail(), user.getSteamId()); } diff --git a/src/main/java/auth_service/modules/registration/api/RegistrationController.java b/src/main/java/auth_service/modules/registration/api/RegistrationController.java index 81196da..dbacc99 100644 --- a/src/main/java/auth_service/modules/registration/api/RegistrationController.java +++ b/src/main/java/auth_service/modules/registration/api/RegistrationController.java @@ -4,7 +4,9 @@ import auth_service.modules.registration.api.dto.mapper.ApiMapper; import auth_service.modules.registration.api.dto.request.RegistrationRequest; import auth_service.modules.registration.api.dto.response.RegistrationResponse; import auth_service.modules.registration.service.RegistrationService; +import auth_service.shared.api.ClientIp; import io.micrometer.core.annotation.Timed; +import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import org.springframework.http.ResponseEntity; @@ -22,8 +24,8 @@ public class RegistrationController { @PostMapping(value = "/new", version = "1.0", produces = "application/json", consumes = "application/json") @Timed(value = "auth.registration.new.requests.time", description = "Time taken to process registration requests") - public ResponseEntity registration(@Valid @RequestBody RegistrationRequest request) { - var userId = registrationService.register(ApiMapper.mapToModel(request)); + public ResponseEntity registration(@Valid @RequestBody RegistrationRequest request, HttpServletRequest httpRequest) { + var userId = registrationService.register(ApiMapper.mapToModel(request), ClientIp.of(httpRequest)); return ResponseEntity.ok(RegistrationResponse.builder().userId(userId).build()); } } diff --git a/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java b/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java index 19f642f..be7031c 100644 --- a/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java +++ b/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java @@ -5,9 +5,11 @@ import auth_service.modules.registration.exception.EmailAlreadyInUseException; import auth_service.modules.registration.exception.InvalidInvitationCodeException; import auth_service.modules.registration.exception.DisplayNameAlreadyRegisteredException; import auth_service.modules.registration.exception.SteamIdAlreadyRegisteredException; +import auth_service.modules.registration.exception.TooManyRegistrationAttemptsException; import lombok.NonNull; import lombok.extern.slf4j.Slf4j; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -24,6 +26,15 @@ public class RegistrationExceptionController { .body(body); } + @ExceptionHandler(TooManyRegistrationAttemptsException.class) + public ResponseEntity clientError(TooManyRegistrationAttemptsException ex) { + ErrorResponse body = new ErrorResponse("TOO_MANY_ATTEMPTS", ex.getMessage()); + return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS) + .header(HttpHeaders.RETRY_AFTER, String.valueOf(ex.getRetryAfterSeconds())) + .contentType(org.springframework.http.MediaType.APPLICATION_JSON) + .body(body); + } + @ExceptionHandler(EmailAlreadyInUseException.class) public ResponseEntity clientError(EmailAlreadyInUseException ex) { var body = new ErrorResponse("EMAIL_ALREADY_IN_USE", ex.getMessage()); diff --git a/src/main/java/auth_service/modules/registration/exception/TooManyRegistrationAttemptsException.java b/src/main/java/auth_service/modules/registration/exception/TooManyRegistrationAttemptsException.java new file mode 100644 index 0000000..d774c22 --- /dev/null +++ b/src/main/java/auth_service/modules/registration/exception/TooManyRegistrationAttemptsException.java @@ -0,0 +1,13 @@ +package auth_service.modules.registration.exception; + +import lombok.Getter; + +@Getter +public class TooManyRegistrationAttemptsException extends RuntimeException { + private final long retryAfterSeconds; + + public TooManyRegistrationAttemptsException(long retryAfterSeconds) { + super("Too many failed registration attempts. Try again later."); + this.retryAfterSeconds = retryAfterSeconds; + } +} diff --git a/src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java b/src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java new file mode 100644 index 0000000..b825dd5 --- /dev/null +++ b/src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java @@ -0,0 +1,30 @@ +package auth_service.modules.registration.service; + +import auth_service.modules.registration.exception.TooManyRegistrationAttemptsException; +import auth_service.shared.security.AttemptLimiter; +import org.springframework.stereotype.Service; + +import java.time.Duration; + +/** + * Blocks a client IP after too many failed registrations. Every rejection counts, so guessing the + * invitation code and probing which emails have an account are both limited. + */ +@Service +public class RegistrationAttemptService { + + static final int MAX_FAILURES_PER_IP = 10; + static final Duration LOCK_DURATION = Duration.ofMinutes(15); + + private final AttemptLimiter limiter = new AttemptLimiter(LOCK_DURATION); + + public void acquire(String clientIp) { + if (!limiter.tryAcquire(clientIp, MAX_FAILURES_PER_IP)) { + throw new TooManyRegistrationAttemptsException(LOCK_DURATION.toSeconds()); + } + } + + public void release(String clientIp) { + limiter.release(clientIp); + } +} diff --git a/src/main/java/auth_service/modules/registration/service/RegistrationService.java b/src/main/java/auth_service/modules/registration/service/RegistrationService.java index 89fc261..87f3a0a 100644 --- a/src/main/java/auth_service/modules/registration/service/RegistrationService.java +++ b/src/main/java/auth_service/modules/registration/service/RegistrationService.java @@ -17,8 +17,20 @@ public class RegistrationService { private final CodeValidationService codeValidationService; private final RegisteredUserRepository registeredUserRepository; private final PasswordEncoder passwordEncoder; + private final RegistrationAttemptService registrationAttemptService; - public long register(Registration registration) { + public long register(Registration registration, String clientIp) { + registrationAttemptService.acquire(clientIp); + validate(registration); + registrationAttemptService.release(clientIp); + + var rawPassword = registration.getRawPassword(); + registration.setEncryptedPassword(passwordEncoder.encode(rawPassword)); + + return registeredUserRepository.save(registration); + } + + private void validate(Registration registration) { if (!codeValidationService.isCodeValid(registration.getInviteCode())) { throw new InvalidInvitationCodeException("The provided invite code is invalid or has already been used."); } @@ -34,10 +46,5 @@ public class RegistrationService { if (registration.getDisplayName() != null && registeredUserRepository.existsByDisplayName(registration.getDisplayName())) { throw new DisplayNameAlreadyRegisteredException("The display name is already in use."); } - - var rawPassword = registration.getRawPassword(); - registration.setEncryptedPassword(passwordEncoder.encode(rawPassword)); - - return registeredUserRepository.save(registration); } } diff --git a/src/main/java/auth_service/shared/api/ClientIp.java b/src/main/java/auth_service/shared/api/ClientIp.java new file mode 100644 index 0000000..4bfede3 --- /dev/null +++ b/src/main/java/auth_service/shared/api/ClientIp.java @@ -0,0 +1,23 @@ +package auth_service.shared.api; + +import jakarta.servlet.http.HttpServletRequest; + +public final class ClientIp { + + private ClientIp() { + } + + /** + * Traefik appends the real client IP as the last X-Forwarded-For entry. Earlier entries come + * from the client and can be faked, so only the last one counts. Without a usable last entry + * (no proxy in front) the connection address counts. + */ + public static String of(HttpServletRequest request) { + String forwardedFor = request.getHeader("X-Forwarded-For"); + if (forwardedFor == null) { + return request.getRemoteAddr(); + } + String lastHop = forwardedFor.substring(forwardedFor.lastIndexOf(',') + 1).trim(); + return lastHop.isEmpty() ? request.getRemoteAddr() : lastHop; + } +} diff --git a/src/main/java/auth_service/shared/security/AttemptLimiter.java b/src/main/java/auth_service/shared/security/AttemptLimiter.java new file mode 100644 index 0000000..4c709f6 --- /dev/null +++ b/src/main/java/auth_service/shared/security/AttemptLimiter.java @@ -0,0 +1,49 @@ +package auth_service.shared.security; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; + +import java.time.Duration; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Counts attempts per key in memory. Callers take an attempt before the check and give it back on + * success, so only failures stay counted. A key is forgotten lockDuration after its last counted + * attempt. A restart resets all counters. + */ +public class AttemptLimiter { + + private final Cache counters; + + public AttemptLimiter(Duration lockDuration) { + this.counters = Caffeine.newBuilder() + .expireAfterWrite(lockDuration) + .maximumSize(100_000) + .build(); + } + + /** + * Check and count are one atomic step, so parallel requests cannot pass more than max. + */ + public boolean tryAcquire(String key, int max) { + AtomicInteger counter = counters.get(key, k -> new AtomicInteger()); + if (counter.incrementAndGet() > max) { + counter.decrementAndGet(); + return false; + } + // put() again so expireAfterWrite restarts the lock window with every counted attempt + counters.put(key, counter); + return true; + } + + public void release(String key) { + AtomicInteger counter = counters.getIfPresent(key); + if (counter != null) { + counter.updateAndGet(n -> Math.max(0, n - 1)); + } + } + + public void reset(String key) { + counters.invalidate(key); + } +} diff --git a/src/test/java/auth_service/modules/login/service/LoginAttemptServiceTest.java b/src/test/java/auth_service/modules/login/service/LoginAttemptServiceTest.java index b04abf8..b91a558 100644 --- a/src/test/java/auth_service/modules/login/service/LoginAttemptServiceTest.java +++ b/src/test/java/auth_service/modules/login/service/LoginAttemptServiceTest.java @@ -5,8 +5,18 @@ import auth_service.shared.test.AbstractUnitTest; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class LoginAttemptServiceTest extends AbstractUnitTest { @@ -20,33 +30,91 @@ class LoginAttemptServiceTest extends AbstractUnitTest { @Test void blocksEmailAfterFiveFailures() { for (int i = 0; i < LoginAttemptService.MAX_FAILURES_PER_EMAIL; i++) { - attempts.checkAllowed("a@b.de", "1.1.1." + i); - attempts.recordFailure("a@b.de", "1.1.1." + i); + attempts.acquire("a@b.de", "1.1.1." + i); } - assertThrows(TooManyLoginAttemptsException.class, () -> attempts.checkAllowed("a@b.de", "9.9.9.9")); - assertThrows(TooManyLoginAttemptsException.class, () -> attempts.checkAllowed("A@B.DE", "9.9.9.9")); - assertDoesNotThrow(() -> attempts.checkAllowed("other@b.de", "9.9.9.9")); + assertThrows(TooManyLoginAttemptsException.class, () -> attempts.acquire("a@b.de", "9.9.9.9")); + assertThrows(TooManyLoginAttemptsException.class, () -> attempts.acquire("A@B.DE", "9.9.9.9")); + assertDoesNotThrow(() -> attempts.acquire("other@b.de", "9.9.9.9")); } @Test void blocksIpAfterTwentyFailures() { for (int i = 0; i < LoginAttemptService.MAX_FAILURES_PER_IP; i++) { - attempts.recordFailure("user" + i + "@b.de", "1.2.3.4"); + attempts.acquire("user" + i + "@b.de", "1.2.3.4"); } - assertThrows(TooManyLoginAttemptsException.class, () -> attempts.checkAllowed("new@b.de", "1.2.3.4")); - assertDoesNotThrow(() -> attempts.checkAllowed("new@b.de", "5.6.7.8")); + assertThrows(TooManyLoginAttemptsException.class, () -> attempts.acquire("new@b.de", "1.2.3.4")); + assertDoesNotThrow(() -> attempts.acquire("new@b.de", "5.6.7.8")); + } + + @Test + void blockedIpDoesNotUseUpAnEmailAttempt() { + for (int i = 0; i < LoginAttemptService.MAX_FAILURES_PER_IP; i++) { + attempts.acquire("user" + i + "@b.de", "1.2.3.4"); + } + for (int i = 0; i < LoginAttemptService.MAX_FAILURES_PER_EMAIL; i++) { + assertThrows(TooManyLoginAttemptsException.class, () -> attempts.acquire("victim@b.de", "1.2.3.4")); + } + + assertDoesNotThrow(() -> attempts.acquire("victim@b.de", "5.6.7.8")); } @Test void successResetsEmailCounter() { for (int i = 0; i < LoginAttemptService.MAX_FAILURES_PER_EMAIL - 1; i++) { - attempts.recordFailure("a@b.de", "1.1.1.1"); + attempts.acquire("a@b.de", "1.1.1.1"); } - attempts.recordSuccess("a@b.de"); - attempts.recordFailure("a@b.de", "1.1.1.1"); + attempts.acquire("a@b.de", "1.1.1.1"); + attempts.recordSuccess("a@b.de", "1.1.1.1"); + attempts.acquire("a@b.de", "1.1.1.1"); - assertDoesNotThrow(() -> attempts.checkAllowed("a@b.de", "1.1.1.1")); + assertDoesNotThrow(() -> attempts.acquire("a@b.de", "1.1.1.1")); + } + + @Test + void successDoesNotUseUpAnIpAttempt() { + for (int i = 0; i < LoginAttemptService.MAX_FAILURES_PER_IP * 2; i++) { + attempts.acquire("user" + i + "@b.de", "1.2.3.4"); + attempts.recordSuccess("user" + i + "@b.de", "1.2.3.4"); + } + + assertDoesNotThrow(() -> attempts.acquire("new@b.de", "1.2.3.4")); + } + + @Test + void concurrentBurstCannotExceedEmailLimit() throws Exception { + int requests = 50; + CyclicBarrier allChecked = new CyclicBarrier(requests); + AtomicInteger passed = new AtomicInteger(); + ExecutorService pool = Executors.newFixedThreadPool(requests); + List> results = new ArrayList<>(); + + try { + for (int i = 0; i < requests; i++) { + String ip = "10.0.0." + i; + results.add(pool.submit(() -> { + boolean allowed = true; + try { + attempts.acquire("a@b.de", ip); + } catch (TooManyLoginAttemptsException e) { + allowed = false; + } + allChecked.await(5, TimeUnit.SECONDS); + if (allowed) { + passed.incrementAndGet(); + } + return null; + })); + } + for (Future result : results) { + result.get(10, TimeUnit.SECONDS); + } + } finally { + pool.shutdownNow(); + } + + assertTrue(passed.get() <= LoginAttemptService.MAX_FAILURES_PER_EMAIL, + "attempts that passed the limit: " + passed.get()); } } diff --git a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java index 2d9e946..da4790d 100644 --- a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java +++ b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java @@ -76,7 +76,8 @@ class LoginServiceTest extends AbstractUnitTest { // Assert assertEquals("mock-token", result); - verify(loginAttemptService).recordSuccess("test@example.com"); + verify(loginAttemptService).acquire("test@example.com", IP); + verify(loginAttemptService).recordSuccess("test@example.com", IP); } @Test @@ -88,7 +89,8 @@ class LoginServiceTest extends AbstractUnitTest { assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP)); // bcrypt still runs, so the response time does not reveal that the email is unknown verify(passwordEncoder).matches("password", "dummy-hash"); - verify(loginAttemptService).recordFailure("test@example.com", IP); + verify(loginAttemptService).acquire("test@example.com", IP); + verify(loginAttemptService, never()).recordSuccess(any(), any()); } @Test @@ -104,7 +106,8 @@ class LoginServiceTest extends AbstractUnitTest { // Act & Assert assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP)); - verify(loginAttemptService).recordFailure("test@example.com", IP); + verify(loginAttemptService).acquire("test@example.com", IP); + verify(loginAttemptService, never()).recordSuccess(any(), any()); } @Test @@ -127,7 +130,7 @@ class LoginServiceTest extends AbstractUnitTest { @Test void login_TooManyAttempts() { // Arrange - doThrow(new TooManyLoginAttemptsException(900)).when(loginAttemptService).checkAllowed("test@example.com", IP); + doThrow(new TooManyLoginAttemptsException(900)).when(loginAttemptService).acquire("test@example.com", IP); // Act & Assert assertThrows(TooManyLoginAttemptsException.class, () -> loginService.login(user, IP)); diff --git a/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java b/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java index 58b0ba6..0795200 100644 --- a/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java +++ b/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java @@ -4,6 +4,7 @@ import auth_service.configuration.security.JwtAuthenticationFilter; import auth_service.configuration.security.JwtService; import auth_service.modules.registration.api.dto.request.RegistrationRequest; import auth_service.modules.registration.exception.EmailAlreadyInUseException; +import auth_service.modules.registration.exception.TooManyRegistrationAttemptsException; import auth_service.modules.registration.model.Registration; import auth_service.modules.registration.service.RegistrationService; import auth_service.shared.test.AbstractWebMvcTest; @@ -21,12 +22,14 @@ import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -67,7 +70,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { long expectedUserId = 42L; // Mock the service call, ignoring the exact static mapping output - when(registrationService.register(any(Registration.class))).thenReturn(expectedUserId); + when(registrationService.register(any(Registration.class), any())).thenReturn(expectedUserId); // Act & Assert mockMvc.perform(post("/api/registration/new") @@ -87,7 +90,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { .steamId("76561198000000001") .build(); - when(registrationService.register(any(Registration.class))) + when(registrationService.register(any(Registration.class), any())) .thenThrow(new EmailAlreadyInUseException("The provided email address is already in use.")); // Act & Assert @@ -108,7 +111,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { .inviteCode("code") .steamId("76561198000000001") .build(); - when(registrationService.register(any(Registration.class))).thenReturn(42L); + when(registrationService.register(any(Registration.class), any())).thenReturn(42L); // Act mockMvc.perform(post("/api/registration/new") @@ -118,7 +121,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { // Assert var captor = ArgumentCaptor.forClass(Registration.class); - verify(registrationService).register(captor.capture()); + verify(registrationService).register(captor.capture(), any()); assertEquals(76561198000000001L, captor.getValue().getSteamId()); } @@ -160,7 +163,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { .password("SuperSecret123!") .inviteCode("code") .build(); - when(registrationService.register(any(Registration.class))) + when(registrationService.register(any(Registration.class), any())) .thenThrow(new DataIntegrityViolationException("users_email_key")); // Act & Assert: the user module's 409 mapping must not apply to registration @@ -183,7 +186,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { @Test void register_PassesDisplayName() throws Exception { // Arrange - when(registrationService.register(any(Registration.class))).thenReturn(42L); + when(registrationService.register(any(Registration.class), any())).thenReturn(42L); // Act mockMvc.perform(post("/api/registration/new") @@ -194,7 +197,38 @@ class RegistrationControllerTest extends AbstractWebMvcTest { // Assert var captor = ArgumentCaptor.forClass(Registration.class); - verify(registrationService).register(captor.capture()); + verify(registrationService).register(captor.capture(), any()); assertEquals("Lobo", captor.getValue().getDisplayName()); } + + @Test + void register_UsesLastForwardedForHopAsClientIp() throws Exception { + // Arrange + when(registrationService.register(any(Registration.class), eq("203.0.113.7"))).thenReturn(42L); + + // Act & Assert + mockMvc.perform(post("/api/registration/new") + .header("X-Forwarded-For", "10.0.0.1, 203.0.113.7") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"test@example.com","password":"SuperSecret123!","inviteCode":"code"}""")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.userId").value(42L)); + } + + @Test + void register_TooManyAttempts() throws Exception { + // Arrange + when(registrationService.register(any(Registration.class), any())) + .thenThrow(new TooManyRegistrationAttemptsException(900)); + + // Act & Assert + mockMvc.perform(post("/api/registration/new") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"test@example.com","password":"SuperSecret123!","inviteCode":"code"}""")) + .andExpect(status().isTooManyRequests()) + .andExpect(header().string("Retry-After", "900")) + .andExpect(jsonPath("$.code").value("TOO_MANY_ATTEMPTS")); + } } diff --git a/src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java b/src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java new file mode 100644 index 0000000..42908f9 --- /dev/null +++ b/src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java @@ -0,0 +1,98 @@ +package auth_service.modules.registration.service; + +import auth_service.modules.registration.exception.TooManyRegistrationAttemptsException; +import auth_service.shared.test.AbstractUnitTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class RegistrationAttemptServiceTest extends AbstractUnitTest { + + private RegistrationAttemptService attempts; + + @BeforeEach + void setUp() { + attempts = new RegistrationAttemptService(); + } + + @Test + void blocksIpAfterTenFailures() { + for (int i = 0; i < RegistrationAttemptService.MAX_FAILURES_PER_IP; i++) { + attempts.acquire("1.2.3.4"); + } + + TooManyRegistrationAttemptsException blocked = + assertThrows(TooManyRegistrationAttemptsException.class, () -> attempts.acquire("1.2.3.4")); + assertEquals(RegistrationAttemptService.LOCK_DURATION.toSeconds(), blocked.getRetryAfterSeconds()); + assertDoesNotThrow(() -> attempts.acquire("5.6.7.8")); + } + + @Test + void successDoesNotUseUpAnAttempt() { + for (int i = 0; i < RegistrationAttemptService.MAX_FAILURES_PER_IP * 2; i++) { + attempts.acquire("1.2.3.4"); + attempts.release("1.2.3.4"); + } + + assertDoesNotThrow(() -> attempts.acquire("1.2.3.4")); + } + + @Test + void blockedAttemptIsNotCounted() { + for (int i = 0; i < RegistrationAttemptService.MAX_FAILURES_PER_IP; i++) { + attempts.acquire("1.2.3.4"); + } + assertThrows(TooManyRegistrationAttemptsException.class, () -> attempts.acquire("1.2.3.4")); + attempts.release("1.2.3.4"); + + assertDoesNotThrow(() -> attempts.acquire("1.2.3.4")); + } + + @Test + void concurrentBurstCannotExceedLimit() throws Exception { + int requests = 50; + CyclicBarrier allChecked = new CyclicBarrier(requests); + AtomicInteger passed = new AtomicInteger(); + ExecutorService pool = Executors.newFixedThreadPool(requests); + List> results = new ArrayList<>(); + + try { + for (int i = 0; i < requests; i++) { + results.add(pool.submit(() -> { + boolean allowed = true; + try { + attempts.acquire("1.2.3.4"); + } catch (TooManyRegistrationAttemptsException e) { + allowed = false; + } + allChecked.await(5, TimeUnit.SECONDS); + if (allowed) { + passed.incrementAndGet(); + } + return null; + })); + } + for (Future result : results) { + result.get(10, TimeUnit.SECONDS); + } + } finally { + pool.shutdownNow(); + } + + assertTrue(passed.get() <= RegistrationAttemptService.MAX_FAILURES_PER_IP, + "attempts that passed the limit: " + passed.get()); + } +} diff --git a/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java b/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java index 2b63af3..3b6702d 100644 --- a/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java +++ b/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java @@ -4,6 +4,7 @@ import auth_service.modules.registration.exception.EmailAlreadyInUseException; import auth_service.modules.registration.exception.InvalidInvitationCodeException; import auth_service.modules.registration.exception.DisplayNameAlreadyRegisteredException; import auth_service.modules.registration.exception.SteamIdAlreadyRegisteredException; +import auth_service.modules.registration.exception.TooManyRegistrationAttemptsException; import auth_service.modules.registration.model.Registration; import auth_service.modules.registration.persistence.RegisteredUserRepository; import auth_service.shared.test.AbstractUnitTest; @@ -23,6 +24,8 @@ import static org.mockito.Mockito.*; */ class RegistrationServiceTest extends AbstractUnitTest { + private static final String IP = "203.0.113.7"; + @Mock private CodeValidationService codeValidationService; @@ -32,6 +35,9 @@ class RegistrationServiceTest extends AbstractUnitTest { @Mock private PasswordEncoder passwordEncoder; + @Mock + private RegistrationAttemptService registrationAttemptService; + @InjectMocks private RegistrationService registrationService; @@ -58,13 +64,15 @@ class RegistrationServiceTest extends AbstractUnitTest { when(registeredUserRepository.save(any(Registration.class))).thenReturn(expectedUserId); // Act - long result = registrationService.register(validRegistration); + long result = registrationService.register(validRegistration, IP); // Assert assertEquals(expectedUserId, result); verify(passwordEncoder).encode("SuperSecret123!"); verify(registeredUserRepository).save(validRegistration); assertEquals("hashedPasswordXYZ", validRegistration.getEncryptedPassword()); + verify(registrationAttemptService).acquire(IP); + verify(registrationAttemptService).release(IP); } @Test @@ -73,10 +81,11 @@ class RegistrationServiceTest extends AbstractUnitTest { when(codeValidationService.isCodeValid(validRegistration.getInviteCode())).thenReturn(false); // Act & Assert - assertThrows(InvalidInvitationCodeException.class, () -> registrationService.register(validRegistration)); + assertThrows(InvalidInvitationCodeException.class, () -> registrationService.register(validRegistration, IP)); verify(registeredUserRepository, never()).existsByEmail(anyString()); verify(registeredUserRepository, never()).save(any()); + verify(registrationAttemptService, never()).release(anyString()); } @Test @@ -86,10 +95,11 @@ class RegistrationServiceTest extends AbstractUnitTest { when(registeredUserRepository.existsByEmail(validRegistration.getEmail())).thenReturn(true); // Act & Assert - assertThrows(EmailAlreadyInUseException.class, () -> registrationService.register(validRegistration)); + assertThrows(EmailAlreadyInUseException.class, () -> registrationService.register(validRegistration, IP)); verify(registeredUserRepository, never()).existsBySteamId(anyLong()); verify(registeredUserRepository, never()).save(any()); + verify(registrationAttemptService, never()).release(anyString()); } @Test @@ -100,9 +110,10 @@ class RegistrationServiceTest extends AbstractUnitTest { when(registeredUserRepository.existsBySteamId(validRegistration.getSteamId())).thenReturn(true); // Act & Assert - assertThrows(SteamIdAlreadyRegisteredException.class, () -> registrationService.register(validRegistration)); + assertThrows(SteamIdAlreadyRegisteredException.class, () -> registrationService.register(validRegistration, IP)); verify(registeredUserRepository, never()).save(any()); + verify(registrationAttemptService, never()).release(anyString()); } @Test @@ -115,7 +126,7 @@ class RegistrationServiceTest extends AbstractUnitTest { when(registeredUserRepository.save(any(Registration.class))).thenReturn(42L); // Act - long result = registrationService.register(validRegistration); + long result = registrationService.register(validRegistration, IP); // Assert assertEquals(42L, result); @@ -132,7 +143,19 @@ class RegistrationServiceTest extends AbstractUnitTest { when(registeredUserRepository.existsByDisplayName("Lobo")).thenReturn(true); // Act & Assert - assertThrows(DisplayNameAlreadyRegisteredException.class, () -> registrationService.register(validRegistration)); + assertThrows(DisplayNameAlreadyRegisteredException.class, () -> registrationService.register(validRegistration, IP)); verify(registeredUserRepository, never()).save(any()); + verify(registrationAttemptService, never()).release(anyString()); + } + + @Test + void register_BlockedIp_ChecksNothingElse() { + // Arrange + doThrow(new TooManyRegistrationAttemptsException(900)).when(registrationAttemptService).acquire(IP); + + // Act & Assert + assertThrows(TooManyRegistrationAttemptsException.class, () -> registrationService.register(validRegistration, IP)); + verifyNoInteractions(codeValidationService, registeredUserRepository); + verify(registrationAttemptService, never()).release(anyString()); } } diff --git a/src/test/java/auth_service/shared/api/ClientIpTest.java b/src/test/java/auth_service/shared/api/ClientIpTest.java new file mode 100644 index 0000000..cf3985d --- /dev/null +++ b/src/test/java/auth_service/shared/api/ClientIpTest.java @@ -0,0 +1,28 @@ +package auth_service.shared.api; + +import auth_service.shared.test.AbstractUnitTest; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ClientIpTest extends AbstractUnitTest { + + @Test + void commaOnlyForwardedForFallsBackToRemoteAddr() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRemoteAddr("198.51.100.9"); + request.addHeader("X-Forwarded-For", ","); + + assertEquals("198.51.100.9", ClientIp.of(request)); + } + + @Test + void blankLastHopFallsBackToRemoteAddr() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRemoteAddr("198.51.100.9"); + request.addHeader("X-Forwarded-For", "10.0.0.1, "); + + assertEquals("198.51.100.9", ClientIp.of(request)); + } +}