Public Access
fix(registration): check input and store emails in lower case
This commit is contained in:
1 parent
ea4ae089ff
commit
68919eaab4
15 files changed
+173
-11
No files matched your search
@@ -13,7 +13,8 @@ To Be Played is split into three repositories:
|
||||
|
||||
## Features
|
||||
|
||||
- Registration with an invitation code, login with email and password (BCrypt).
|
||||
- Registration with an invitation code, login with email and password (BCrypt). Emails are stored trimmed and in
|
||||
lower case (V19, unique on `lower(email)`); a password has 8 characters and at most 72 bytes (BCrypt's limit).
|
||||
- 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.
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
package auth_service.modules.login.api.dto.request;
|
||||
|
||||
import auth_service.shared.account.EmailAddress;
|
||||
import jakarta.validation.constraints.NotBlank;
|
||||
import jakarta.validation.constraints.NotNull;
|
||||
|
||||
@@ -7,4 +8,8 @@ import jakarta.validation.constraints.NotNull;
|
||||
* DTO (Data Transfer Object) representing the incoming login request.
|
||||
*/
|
||||
public record LoginRequest(@NotNull String email, @NotNull @NotBlank String password) {
|
||||
|
||||
public LoginRequest {
|
||||
email = EmailAddress.normalize(email);
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
package auth_service.modules.login.persistence;
|
||||
|
||||
import auth_service.shared.account.EmailAddress;
|
||||
import auth_service.modules.login.model.User;
|
||||
import auth_service.modules.login.persistence.entities.MinimalUserEntity;
|
||||
import auth_service.modules.login.persistence.mapper.EntityMapper;
|
||||
@@ -25,8 +26,8 @@ public class MinimalUserRepositoryImpl implements MinimalUserRepository {
|
||||
var userEntityCriteriaQuery = criteriaBuilder.createQuery(MinimalUserEntity.class);
|
||||
var userEntity = userEntityCriteriaQuery.from(MinimalUserEntity.class);
|
||||
|
||||
// Build the query: SELECT u FROM UserEntity u WHERE u.email = :email
|
||||
userEntityCriteriaQuery.select(userEntity).where(criteriaBuilder.equal(userEntity.get("email"), email));
|
||||
userEntityCriteriaQuery.select(userEntity).where(criteriaBuilder.equal(
|
||||
userEntity.get("email"), EmailAddress.normalize(email)));
|
||||
|
||||
TypedQuery<MinimalUserEntity> typedQuery = entityManager.createQuery(userEntityCriteriaQuery);
|
||||
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
package auth_service.modules.login.service;
|
||||
|
||||
import auth_service.modules.login.exception.TooManyLoginAttemptsException;
|
||||
import auth_service.shared.account.EmailAddress;
|
||||
import auth_service.shared.security.AttemptLimiter;
|
||||
import org.springframework.stereotype.Service;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.util.Locale;
|
||||
|
||||
/**
|
||||
* Counts failed logins per email and per client IP in memory. A key is blocked after too many
|
||||
@@ -37,7 +37,7 @@ public class LoginAttemptService {
|
||||
}
|
||||
|
||||
private static String emailKey(String email) {
|
||||
return "email:" + (email == null ? "" : email.trim().toLowerCase(Locale.ROOT));
|
||||
return "email:" + (email == null ? "" : EmailAddress.normalize(email));
|
||||
}
|
||||
|
||||
private static String ipKey(String clientIp) {
|
||||
|
||||
+24
-1
@@ -1,13 +1,36 @@
|
||||
package auth_service.modules.registration.api.dto.request;
|
||||
|
||||
import auth_service.shared.api.validation.DisplayNameFormat;
|
||||
import auth_service.shared.account.EmailAddress;
|
||||
import com.fasterxml.jackson.annotation.JsonIgnore;
|
||||
import jakarta.validation.constraints.AssertTrue;
|
||||
import jakarta.validation.constraints.Email;
|
||||
import jakarta.validation.constraints.NotBlank;
|
||||
import jakarta.validation.constraints.Pattern;
|
||||
import jakarta.validation.constraints.Size;
|
||||
import lombok.Builder;
|
||||
|
||||
import java.nio.charset.StandardCharsets;
|
||||
|
||||
/**
|
||||
* No Steam ID: it comes only from "Sign in through Steam", later in the settings.
|
||||
*/
|
||||
@Builder
|
||||
public record RegistrationRequest(String email, String password, String inviteCode,
|
||||
public record RegistrationRequest(@NotBlank @Email @Size(max = EmailAddress.MAX_LENGTH) String email,
|
||||
@NotBlank @Size(min = 8) String password,
|
||||
@NotBlank String inviteCode,
|
||||
@Pattern(regexp = DisplayNameFormat.PATTERN) String displayName) {
|
||||
|
||||
public RegistrationRequest {
|
||||
email = EmailAddress.normalize(email);
|
||||
}
|
||||
|
||||
/**
|
||||
* BCrypt uses only the first 72 bytes and refuses a longer password.
|
||||
*/
|
||||
@AssertTrue(message = "The password may have at most 72 bytes")
|
||||
@JsonIgnore
|
||||
public boolean isPasswordShortEnough() {
|
||||
return password == null || password.getBytes(StandardCharsets.UTF_8).length <= 72;
|
||||
}
|
||||
}
|
||||
+2
-1
@@ -1,5 +1,6 @@
|
||||
package auth_service.modules.registration.persistence;
|
||||
|
||||
import auth_service.shared.account.EmailAddress;
|
||||
import auth_service.modules.registration.model.Registration;
|
||||
import auth_service.modules.registration.persistence.mapper.EntityMapper;
|
||||
import jakarta.persistence.EntityManager;
|
||||
@@ -18,7 +19,7 @@ public class RegisteredUserRepositoryImpl implements RegisteredUserRepository {
|
||||
public boolean existsByEmail(String email) {
|
||||
var jpql = "SELECT COUNT(u) FROM UserEntity u WHERE u.email = :email";
|
||||
var count = entityManager.createQuery(jpql, Long.class)
|
||||
.setParameter("email", email)
|
||||
.setParameter("email", EmailAddress.normalize(email))
|
||||
.getSingleResult();
|
||||
|
||||
return count != null && count > 0;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
package auth_service.modules.user.persistence;
|
||||
|
||||
import auth_service.shared.account.EmailAddress;
|
||||
import auth_service.modules.user.exception.UserNotFoundException;
|
||||
import auth_service.modules.user.model.Appearance;
|
||||
import auth_service.modules.user.model.ProfileStyle;
|
||||
@@ -36,8 +37,8 @@ public class UserRepositoryImpl implements UserRepository {
|
||||
// A LEFT JOIN ensures the user is returned even if no settings exist yet.
|
||||
userEntity.fetch("settings", jakarta.persistence.criteria.JoinType.LEFT);
|
||||
|
||||
// Build the query: SELECT u FROM UserEntity u LEFT JOIN FETCH u.settings WHERE u.email = :email
|
||||
userEntityCriteriaQuery.select(userEntity).where(criteriaBuilder.equal(userEntity.get("email"), email));
|
||||
userEntityCriteriaQuery.select(userEntity).where(criteriaBuilder.equal(
|
||||
userEntity.get("email"), EmailAddress.normalize(email)));
|
||||
|
||||
TypedQuery<UserEntity> typedQuery = entityManager.createQuery(userEntityCriteriaQuery);
|
||||
|
||||
|
||||
@@ -40,9 +40,9 @@ public abstract class EntityMapper {
|
||||
.favoritePlatform(entity.getSettings().getFavoritePlatform())
|
||||
.build();
|
||||
} else {
|
||||
// Fallback default settings for users without an entry in the database
|
||||
// No settings row. "" means no language chosen, as in a new row; the frontend keeps its own.
|
||||
settingsModel = UserSettings.builder()
|
||||
.language("en")
|
||||
.language("")
|
||||
.theme("system")
|
||||
.build();
|
||||
}
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
package auth_service.shared.account;
|
||||
|
||||
import java.util.Locale;
|
||||
|
||||
/**
|
||||
* One account per address, whatever case the user types: emails are stored and looked up trimmed and in lower case
|
||||
* (V19 has a unique index on lower(email)). Look-ups normalize too: an access token from before V19 still carries
|
||||
* the email as it was typed.
|
||||
*/
|
||||
public final class EmailAddress {
|
||||
public static final int MAX_LENGTH = 254;
|
||||
|
||||
private EmailAddress() {
|
||||
}
|
||||
|
||||
public static String normalize(String email) {
|
||||
return email == null ? null : email.strip().toLowerCase(Locale.ROOT);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,4 @@
|
||||
-- One account per address, whatever case the user types (EmailAddress). Checked on prod before: no two emails
|
||||
-- differ only in case, so the update cannot collide.
|
||||
UPDATE users SET email = lower(trim(email)) WHERE email <> lower(trim(email));
|
||||
CREATE UNIQUE INDEX users_email_lower ON users (lower(email));
|
||||
@@ -13,6 +13,7 @@ import auth_service.shared.security.SessionProperties;
|
||||
import jakarta.servlet.http.Cookie;
|
||||
import auth_service.shared.test.AbstractWebMvcTest;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.autoconfigure.json.AutoConfigureJsonTesters;
|
||||
import org.springframework.boot.test.json.JacksonTester;
|
||||
@@ -26,6 +27,7 @@ import org.springframework.test.web.servlet.MockMvc;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.verify;
|
||||
@@ -183,4 +185,18 @@ class LoginControllerTest extends AbstractWebMvcTest {
|
||||
|
||||
verify(refreshTokenService).revoke("the-refresh");
|
||||
}
|
||||
|
||||
@Test
|
||||
void login_Email_IsTrimmedAndLowerCase() throws Exception {
|
||||
when(loginService.login(any(User.class), any(), any())).thenReturn(new Session("mock-access", "mock-refresh"));
|
||||
|
||||
mockMvc.perform(post("/api/auth/login")
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.content(loginRequestJson.write(new LoginRequest(" Bob@Example.COM ", "password")).getJson()))
|
||||
.andExpect(status().isOk());
|
||||
|
||||
ArgumentCaptor<User> user = ArgumentCaptor.forClass(User.class);
|
||||
verify(loginService).login(user.capture(), any(), any());
|
||||
assertEquals("bob@example.com", user.getValue().getEmail());
|
||||
}
|
||||
}
|
||||
@@ -57,4 +57,13 @@ class MinimalUserRepositoryTest extends AbstractPersistenceTest {
|
||||
var updated = entityManager.find(MinimalUserEntity.class, entity.getId());
|
||||
assertNotNull(updated.getLastLoginDate());
|
||||
}
|
||||
|
||||
// An access token from before V19 still carries the email as it was typed.
|
||||
@Test
|
||||
void findByEmail_IgnoresCase() {
|
||||
entityManager.persist(MinimalUserEntity.builder().email("case@example.com").password("password").enabled(true).build());
|
||||
entityManager.flush();
|
||||
|
||||
assertTrue(minimalUserRepository.findByEmail("Case@Example.com").isPresent());
|
||||
}
|
||||
}
|
||||
@@ -9,6 +9,8 @@ import auth_service.modules.registration.model.Registration;
|
||||
import auth_service.modules.registration.service.RegistrationService;
|
||||
import auth_service.shared.test.AbstractWebMvcTest;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.CsvSource;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.autoconfigure.json.AutoConfigureJsonTesters;
|
||||
@@ -187,4 +189,53 @@ class RegistrationControllerTest extends AbstractWebMvcTest {
|
||||
.andExpect(header().string("Retry-After", "900"))
|
||||
.andExpect(jsonPath("$.code").value("TOO_MANY_ATTEMPTS"));
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@CsvSource(delimiter = '|', value = {
|
||||
" |SuperSecret123!|code",
|
||||
"no-at-sign|SuperSecret123!|code",
|
||||
"test@example.com| |code",
|
||||
"test@example.com|short|code",
|
||||
"test@example.com|SuperSecret123!| ",
|
||||
})
|
||||
void register_InvalidInput_Is400WithoutRegistering(String email, String password, String inviteCode) throws Exception {
|
||||
var request = RegistrationRequest.builder().email(email).password(password).inviteCode(inviteCode).build();
|
||||
|
||||
mockMvc.perform(post("/api/registration/new")
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.content(registrationRequestJson.write(request).getJson()))
|
||||
.andExpect(status().isBadRequest());
|
||||
|
||||
verifyNoInteractions(registrationService);
|
||||
}
|
||||
|
||||
// BCrypt uses only 72 bytes: a longer password would mostly be ignored, or refused with a 500.
|
||||
@Test
|
||||
void register_PasswordOver72Bytes_Is400() throws Exception {
|
||||
var request = RegistrationRequest.builder().email("test@example.com").password("é".repeat(37))
|
||||
.inviteCode("code").build();
|
||||
|
||||
mockMvc.perform(post("/api/registration/new")
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.content(registrationRequestJson.write(request).getJson()))
|
||||
.andExpect(status().isBadRequest());
|
||||
|
||||
verifyNoInteractions(registrationService);
|
||||
}
|
||||
|
||||
@Test
|
||||
void register_Email_IsTrimmedAndLowerCase() throws Exception {
|
||||
var request = RegistrationRequest.builder().email(" Bob@Example.COM ").password("SuperSecret123!")
|
||||
.inviteCode("code").build();
|
||||
when(registrationService.register(any(Registration.class), any())).thenReturn(1L);
|
||||
|
||||
mockMvc.perform(post("/api/registration/new")
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.content(registrationRequestJson.write(request).getJson()))
|
||||
.andExpect(status().isOk());
|
||||
|
||||
ArgumentCaptor<Registration> registration = ArgumentCaptor.forClass(Registration.class);
|
||||
verify(registrationService).register(registration.capture(), any());
|
||||
assertEquals("bob@example.com", registration.getValue().getEmail());
|
||||
}
|
||||
}
|
||||
+15
@@ -54,4 +54,19 @@ class RegisteredUserRepositoryTest extends AbstractPersistenceTest {
|
||||
assertNotNull(saved);
|
||||
assertEquals("save@example.com", saved.getEmail());
|
||||
}
|
||||
|
||||
// V19: a second account under the same email in other case is refused by the database too.
|
||||
@Test
|
||||
void emailsThatDifferOnlyInCase_AreOneAccount() {
|
||||
entityManager.persist(RegisteredUserEntity.builder().email("dup@example.com").password("password")
|
||||
.enabled(true).registrationDate(java.time.LocalDateTime.now()).build());
|
||||
entityManager.flush();
|
||||
|
||||
assertTrue(registeredUserRepository.existsByEmail("Dup@Example.com"));
|
||||
assertThrows(org.hibernate.exception.ConstraintViolationException.class, () -> {
|
||||
entityManager.persist(RegisteredUserEntity.builder().email("Dup@example.com").password("password")
|
||||
.enabled(true).registrationDate(java.time.LocalDateTime.now()).build());
|
||||
entityManager.flush();
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
package auth_service.modules.user.persistence.mapper;
|
||||
|
||||
import auth_service.modules.user.persistence.entities.UserEntity;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
class EntityMapperTest {
|
||||
|
||||
@Test
|
||||
void mapToModel_UserWithoutSettingsRow_HasNoLanguage() {
|
||||
UserEntity entity = UserEntity.builder().id(1L).email("a@b.c").role("USER").build();
|
||||
|
||||
assertThat(EntityMapper.mapToModel(entity).getSettings().getLanguage()).isEmpty();
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user