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 2b0ecde..7a3d90d 100644 --- a/src/main/java/auth_service/modules/login/api/LoginController.java +++ b/src/main/java/auth_service/modules/login/api/LoginController.java @@ -5,6 +5,8 @@ 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 io.micrometer.core.annotation.Timed; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.PostMapping; @@ -30,10 +32,23 @@ public class LoginController { */ @PostMapping("/login") @Timed(value = "auth.login.requests.time", description = "Time taken to process login requests") - public ResponseEntity login(@RequestBody LoginRequest request) { + public ResponseEntity login(@Valid @RequestBody LoginRequest request, HttpServletRequest httpRequest) { var user = ApiMapper.toModel(request); - var token = loginService.login(user); + var token = loginService.login(user, clientIp(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/api/LoginExceptionController.java b/src/main/java/auth_service/modules/login/api/LoginExceptionController.java index edd0d5f..b2574e3 100644 --- a/src/main/java/auth_service/modules/login/api/LoginExceptionController.java +++ b/src/main/java/auth_service/modules/login/api/LoginExceptionController.java @@ -2,10 +2,11 @@ package auth_service.modules.login.api; import auth_service.shared.api.dto.response.ErrorResponse; import auth_service.modules.login.exception.InvalidCredentialsException; -import auth_service.modules.login.exception.UserNotFoundException; +import auth_service.modules.login.exception.TooManyLoginAttemptsException; 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; @@ -28,10 +29,13 @@ public class LoginExceptionController { /* ######## CLIENT ERRORS (400-499) */ - @ExceptionHandler(UserNotFoundException.class) - public ResponseEntity clientError(UserNotFoundException ex) { - var body = new ErrorResponse("USER_NOT_FOUND", ex.getMessage()); - return response(body, HttpStatus.NOT_ACCEPTABLE); + @ExceptionHandler(TooManyLoginAttemptsException.class) + public ResponseEntity clientError(TooManyLoginAttemptsException ex) { + var 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(InvalidCredentialsException.class) diff --git a/src/main/java/auth_service/modules/login/exception/TooManyLoginAttemptsException.java b/src/main/java/auth_service/modules/login/exception/TooManyLoginAttemptsException.java new file mode 100644 index 0000000..fd7a537 --- /dev/null +++ b/src/main/java/auth_service/modules/login/exception/TooManyLoginAttemptsException.java @@ -0,0 +1,13 @@ +package auth_service.modules.login.exception; + +import lombok.Getter; + +@Getter +public class TooManyLoginAttemptsException extends RuntimeException { + private final long retryAfterSeconds; + + public TooManyLoginAttemptsException(long retryAfterSeconds) { + super("Too many failed login attempts. Try again later."); + this.retryAfterSeconds = retryAfterSeconds; + } +} diff --git a/src/main/java/auth_service/modules/login/exception/UserNotFoundException.java b/src/main/java/auth_service/modules/login/exception/UserNotFoundException.java deleted file mode 100644 index 39a4569..0000000 --- a/src/main/java/auth_service/modules/login/exception/UserNotFoundException.java +++ /dev/null @@ -1,7 +0,0 @@ -package auth_service.modules.login.exception; - -public class UserNotFoundException extends RuntimeException { - public UserNotFoundException(String message) { - super(message); - } -} diff --git a/src/main/java/auth_service/modules/login/model/User.java b/src/main/java/auth_service/modules/login/model/User.java index 1a358e8..dc4e245 100644 --- a/src/main/java/auth_service/modules/login/model/User.java +++ b/src/main/java/auth_service/modules/login/model/User.java @@ -10,4 +10,5 @@ public class User { private String email; private String password; private Long steamId; + private boolean enabled; } diff --git a/src/main/java/auth_service/modules/login/persistence/mapper/EntityMapper.java b/src/main/java/auth_service/modules/login/persistence/mapper/EntityMapper.java index ca5b001..737b80b 100644 --- a/src/main/java/auth_service/modules/login/persistence/mapper/EntityMapper.java +++ b/src/main/java/auth_service/modules/login/persistence/mapper/EntityMapper.java @@ -16,6 +16,7 @@ public abstract class EntityMapper { model.setEmail(entity.getEmail()); model.setPassword(entity.getPassword()); model.setSteamId(entity.getSteamId()); + model.setEnabled(entity.isEnabled()); } return model; diff --git a/src/main/java/auth_service/modules/login/service/LoginAttemptService.java b/src/main/java/auth_service/modules/login/service/LoginAttemptService.java new file mode 100644 index 0000000..ad4f4c1 --- /dev/null +++ b/src/main/java/auth_service/modules/login/service/LoginAttemptService.java @@ -0,0 +1,62 @@ +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 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 + * failures until LOCK_DURATION has passed since its last failure. A restart resets all counters. + */ +@Service +public class LoginAttemptService { + + static final int MAX_FAILURES_PER_EMAIL = 5; + 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(); + + public void checkAllowed(String email, String clientIp) { + if (count(emailKey(email)) >= MAX_FAILURES_PER_EMAIL || count(ipKey(clientIp)) >= MAX_FAILURES_PER_IP) { + 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); + } + + private static String emailKey(String email) { + return "email:" + (email == null ? "" : email.trim().toLowerCase(Locale.ROOT)); + } + + private static String ipKey(String clientIp) { + return "ip:" + clientIp; + } +} 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 56185b4..da75dc6 100644 --- a/src/main/java/auth_service/modules/login/service/LoginService.java +++ b/src/main/java/auth_service/modules/login/service/LoginService.java @@ -2,35 +2,57 @@ package auth_service.modules.login.service; import auth_service.shared.security.TokenGenerator; import auth_service.modules.login.exception.InvalidCredentialsException; -import auth_service.modules.login.exception.UserNotFoundException; import auth_service.modules.login.model.User; import auth_service.modules.login.persistence.MinimalUserRepository; -import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; +import java.util.Optional; +import java.util.UUID; + @Service -@RequiredArgsConstructor @Slf4j public class LoginService { private final MinimalUserRepository minimalUserRepository; private final PasswordEncoder passwordEncoder; private final TokenGenerator tokenGenerator; + private final LoginAttemptService loginAttemptService; + + // Checked against for unknown emails, so they cost the same bcrypt time as a wrong password. + private final String dummyHash; + + public LoginService(MinimalUserRepository minimalUserRepository, PasswordEncoder passwordEncoder, + TokenGenerator tokenGenerator, LoginAttemptService loginAttemptService) { + this.minimalUserRepository = minimalUserRepository; + this.passwordEncoder = passwordEncoder; + this.tokenGenerator = tokenGenerator; + this.loginAttemptService = loginAttemptService; + this.dummyHash = passwordEncoder.encode(UUID.randomUUID().toString()); + } /** - * Authenticates a user and returns a token. + * Authenticates a user and returns a token. Unknown email, wrong password and disabled + * account all fail with the same InvalidCredentialsException. * * @return The generated token */ - public String login(User userToLogin) { - var foundUser = minimalUserRepository.findByEmail(userToLogin.getEmail()) - .orElseThrow(() -> new UserNotFoundException("User not found with given email")); - var loginSuccessful = passwordEncoder.matches(userToLogin.getPassword(), foundUser.getPassword()); - if (!loginSuccessful) { - throw new InvalidCredentialsException("Invalid credentials"); + public String login(User userToLogin, String clientIp) { + String email = userToLogin.getEmail(); + loginAttemptService.checkAllowed(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"); } - minimalUserRepository.updateLoginDate(foundUser.getId()); - return tokenGenerator.generateToken(String.valueOf(foundUser.getId()), foundUser.getEmail(), foundUser.getSteamId()); + + User user = foundUser.get(); + loginAttemptService.recordSuccess(email); + minimalUserRepository.updateLoginDate(user.getId()); + return tokenGenerator.generateToken(String.valueOf(user.getId()), user.getEmail(), user.getSteamId()); } } diff --git a/src/test/java/auth_service/modules/login/api/LoginControllerTest.java b/src/test/java/auth_service/modules/login/api/LoginControllerTest.java index 84a8a4c..52ca72b 100644 --- a/src/test/java/auth_service/modules/login/api/LoginControllerTest.java +++ b/src/test/java/auth_service/modules/login/api/LoginControllerTest.java @@ -3,6 +3,7 @@ package auth_service.modules.login.api; import auth_service.configuration.security.JwtAuthenticationFilter; import auth_service.modules.login.api.dto.request.LoginRequest; import auth_service.modules.login.exception.InvalidCredentialsException; +import auth_service.modules.login.exception.TooManyLoginAttemptsException; import auth_service.modules.login.model.User; import auth_service.modules.login.service.LoginService; import auth_service.shared.test.AbstractWebMvcTest; @@ -17,8 +18,11 @@ 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.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; @@ -46,7 +50,7 @@ class LoginControllerTest extends AbstractWebMvcTest { void login_Success() throws Exception { // Arrange var request = new LoginRequest("test@example.com", "password"); - when(loginService.login(any(User.class))).thenReturn("mock-token"); + when(loginService.login(any(User.class), eq("127.0.0.1"))).thenReturn("mock-token"); // Act & Assert mockMvc.perform(post("/api/auth/login") @@ -61,12 +65,53 @@ class LoginControllerTest extends AbstractWebMvcTest { void login_Failure() throws Exception { // Arrange var request = new LoginRequest("test@example.com", "wrong-password"); - when(loginService.login(any(User.class))).thenThrow(new InvalidCredentialsException("Invalid credentials")); + when(loginService.login(any(User.class), any())).thenThrow(new InvalidCredentialsException("Invalid email or password")); // Act & Assert mockMvc.perform(post("/api/auth/login") .contentType(MediaType.APPLICATION_JSON) .content(loginRequestJson.write(request).getJson())) - .andExpect(status().isUnauthorized()); + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value("INVALID_CREDENTIALS")); + } + + @Test + void login_UsesClientIpAddedByTraefik() throws Exception { + // Arrange + var request = new LoginRequest("test@example.com", "password"); + when(loginService.login(any(User.class), eq("203.0.113.7"))).thenReturn("mock-token"); + + // Act & Assert + mockMvc.perform(post("/api/auth/login") + .header("X-Forwarded-For", "10.0.0.1, 203.0.113.7") + .contentType(MediaType.APPLICATION_JSON) + .content(loginRequestJson.write(request).getJson())) + .andExpect(status().isOk()); + } + + @Test + void login_TooManyAttempts() throws Exception { + // Arrange + var request = new LoginRequest("test@example.com", "password"); + when(loginService.login(any(User.class), any())).thenThrow(new TooManyLoginAttemptsException(900)); + + // Act & Assert + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content(loginRequestJson.write(request).getJson())) + .andExpect(status().isTooManyRequests()) + .andExpect(header().string("Retry-After", "900")) + .andExpect(jsonPath("$.code").value("TOO_MANY_ATTEMPTS")); + } + + @Test + void login_MissingPassword() throws Exception { + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"test@example.com"}""")) + .andExpect(status().isBadRequest()); + + verifyNoInteractions(loginService); } } diff --git a/src/test/java/auth_service/modules/login/service/LoginAttemptServiceTest.java b/src/test/java/auth_service/modules/login/service/LoginAttemptServiceTest.java new file mode 100644 index 0000000..b04abf8 --- /dev/null +++ b/src/test/java/auth_service/modules/login/service/LoginAttemptServiceTest.java @@ -0,0 +1,52 @@ +package auth_service.modules.login.service; + +import auth_service.modules.login.exception.TooManyLoginAttemptsException; +import auth_service.shared.test.AbstractUnitTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class LoginAttemptServiceTest extends AbstractUnitTest { + + private LoginAttemptService attempts; + + @BeforeEach + void setUp() { + attempts = new LoginAttemptService(); + } + + @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); + } + + 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")); + } + + @Test + void blocksIpAfterTwentyFailures() { + for (int i = 0; i < LoginAttemptService.MAX_FAILURES_PER_IP; i++) { + attempts.recordFailure("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")); + } + + @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.recordSuccess("a@b.de"); + attempts.recordFailure("a@b.de", "1.1.1.1"); + + assertDoesNotThrow(() -> attempts.checkAllowed("a@b.de", "1.1.1.1")); + } +} 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 8926686..2d9e946 100644 --- a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java +++ b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java @@ -2,13 +2,12 @@ package auth_service.modules.login.service; import auth_service.shared.security.TokenGenerator; import auth_service.modules.login.exception.InvalidCredentialsException; -import auth_service.modules.login.exception.UserNotFoundException; +import auth_service.modules.login.exception.TooManyLoginAttemptsException; import auth_service.modules.login.model.User; import auth_service.modules.login.persistence.MinimalUserRepository; import auth_service.shared.test.AbstractUnitTest; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.springframework.security.crypto.password.PasswordEncoder; @@ -16,6 +15,14 @@ import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -23,6 +30,8 @@ import static org.mockito.Mockito.when; */ class LoginServiceTest extends AbstractUnitTest { + private static final String IP = "203.0.113.7"; + @Mock private MinimalUserRepository minimalUserRepository; @@ -32,13 +41,17 @@ class LoginServiceTest extends AbstractUnitTest { @Mock private TokenGenerator tokenGenerator; - @InjectMocks + @Mock + private LoginAttemptService loginAttemptService; + private LoginService loginService; private User user; @BeforeEach void setUp() { + lenient().when(passwordEncoder.encode(anyString())).thenReturn("dummy-hash"); + loginService = new LoginService(minimalUserRepository, passwordEncoder, tokenGenerator, loginAttemptService); user = User.builder() .email("test@example.com") .password("password") @@ -52,38 +65,73 @@ class LoginServiceTest extends AbstractUnitTest { .id(815L) .email("test@example.com") .password("encodedPassword") + .enabled(true) .build(); when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); when(tokenGenerator.generateToken("815", "test@example.com", null)).thenReturn("mock-token"); // Act - var result = loginService.login(user); + var result = loginService.login(user, IP); // Assert assertEquals("mock-token", result); + verify(loginAttemptService).recordSuccess("test@example.com"); } @Test - void login_UserNotFound() { + void login_UnknownEmail_SameErrorAsWrongPassword() { // Arrange when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.empty()); // Act & Assert - assertThrows(UserNotFoundException.class, () -> loginService.login(user)); + 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); } @Test - void login_InvalidCredentials() { + void login_WrongPassword() { // Arrange var dbUser = User.builder() .email("test@example.com") .password("encodedPassword") + .enabled(true) .build(); when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(false); // Act & Assert - assertThrows(InvalidCredentialsException.class, () -> loginService.login(user)); + assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP)); + verify(loginAttemptService).recordFailure("test@example.com", IP); + } + + @Test + void login_DisabledUser() { + // Arrange + var dbUser = User.builder() + .id(815L) + .email("test@example.com") + .password("encodedPassword") + .enabled(false) + .build(); + when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); + when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); + + // Act & Assert + assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP)); + verify(tokenGenerator, never()).generateToken(any(), any(), any()); + } + + @Test + void login_TooManyAttempts() { + // Arrange + doThrow(new TooManyLoginAttemptsException(900)).when(loginAttemptService).checkAllowed("test@example.com", IP); + + // Act & Assert + assertThrows(TooManyLoginAttemptsException.class, () -> loginService.login(user, IP)); + verifyNoInteractions(minimalUserRepository); + verify(passwordEncoder, never()).matches(any(), eq("encodedPassword")); } }