Public Access
fix(login): same error for every failed login, block disabled users, rate limit
Unknown email, wrong password and disabled account now all return 401 INVALID_CREDENTIALS; unknown emails still run bcrypt against a dummy hash so timing does not leak them. After 5 failures per email or 20 per client IP, login returns 429 with Retry-After for 15 minutes. LoginRequest is now validated.
This commit is contained in:
1 parent
3b6acb2b34
commit
da7c3ed069
11 files changed
+293
-37
No files matched your search
@@ -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);
|
||||
}
|
||||
}
|
||||
@@ -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"));
|
||||
}
|
||||
}
|
||||
@@ -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"));
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user