From 68919eaab4333cb106553688d959a0f232344b73 Mon Sep 17 00:00:00 2001 From: lobo Date: Tue, 29 Sep 2026 13:27:31 +0200 Subject: [PATCH] fix(registration): check input and store emails in lower case --- readme.md | 3 +- .../login/api/dto/request/LoginRequest.java | 5 ++ .../MinimalUserRepositoryImpl.java | 5 +- .../login/service/LoginAttemptService.java | 4 +- .../api/dto/request/RegistrationRequest.java | 25 ++++++++- .../RegisteredUserRepositoryImpl.java | 3 +- .../user/persistence/UserRepositoryImpl.java | 5 +- .../user/persistence/mapper/EntityMapper.java | 4 +- .../shared/account/EmailAddress.java | 19 +++++++ .../db/migration/V19__Email_Lower_Case.sql | 4 ++ .../login/api/LoginControllerTest.java | 16 ++++++ .../MinimalUserRepositoryTest.java | 9 ++++ .../api/RegistrationControllerTest.java | 51 +++++++++++++++++++ .../RegisteredUserRepositoryTest.java | 15 ++++++ .../persistence/mapper/EntityMapperTest.java | 16 ++++++ 15 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 src/main/java/auth_service/shared/account/EmailAddress.java create mode 100644 src/main/resources/db/migration/V19__Email_Lower_Case.sql create mode 100644 src/test/java/auth_service/modules/user/persistence/mapper/EntityMapperTest.java diff --git a/readme.md b/readme.md index 512e337..48e865a 100644 --- a/readme.md +++ b/readme.md @@ -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. diff --git a/src/main/java/auth_service/modules/login/api/dto/request/LoginRequest.java b/src/main/java/auth_service/modules/login/api/dto/request/LoginRequest.java index d4c829d..2705855 100644 --- a/src/main/java/auth_service/modules/login/api/dto/request/LoginRequest.java +++ b/src/main/java/auth_service/modules/login/api/dto/request/LoginRequest.java @@ -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); + } } diff --git a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java index 528e4be..48623fa 100644 --- a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java +++ b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java @@ -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 typedQuery = entityManager.createQuery(userEntityCriteriaQuery); 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 7c815dd..dc3686d 100644 --- a/src/main/java/auth_service/modules/login/service/LoginAttemptService.java +++ b/src/main/java/auth_service/modules/login/service/LoginAttemptService.java @@ -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) { diff --git a/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java b/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java index cd731b8..d6ad14d 100644 --- a/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java +++ b/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java @@ -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; + } } diff --git a/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java b/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java index 0f80e9e..0720cc9 100644 --- a/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java +++ b/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java @@ -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; diff --git a/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java b/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java index d813118..dafe6a5 100644 --- a/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java +++ b/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java @@ -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 typedQuery = entityManager.createQuery(userEntityCriteriaQuery); diff --git a/src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java b/src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java index 70a5ab8..120f548 100644 --- a/src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java +++ b/src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java @@ -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(); } diff --git a/src/main/java/auth_service/shared/account/EmailAddress.java b/src/main/java/auth_service/shared/account/EmailAddress.java new file mode 100644 index 0000000..088e36d --- /dev/null +++ b/src/main/java/auth_service/shared/account/EmailAddress.java @@ -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); + } +} diff --git a/src/main/resources/db/migration/V19__Email_Lower_Case.sql b/src/main/resources/db/migration/V19__Email_Lower_Case.sql new file mode 100644 index 0000000..0012bb4 --- /dev/null +++ b/src/main/resources/db/migration/V19__Email_Lower_Case.sql @@ -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)); 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 9579ea8..b9ab2d8 100644 --- a/src/test/java/auth_service/modules/login/api/LoginControllerTest.java +++ b/src/test/java/auth_service/modules/login/api/LoginControllerTest.java @@ -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 = ArgumentCaptor.forClass(User.class); + verify(loginService).login(user.capture(), any(), any()); + assertEquals("bob@example.com", user.getValue().getEmail()); + } } diff --git a/src/test/java/auth_service/modules/login/persistence/MinimalUserRepositoryTest.java b/src/test/java/auth_service/modules/login/persistence/MinimalUserRepositoryTest.java index 043a99d..2aa6ebf 100644 --- a/src/test/java/auth_service/modules/login/persistence/MinimalUserRepositoryTest.java +++ b/src/test/java/auth_service/modules/login/persistence/MinimalUserRepositoryTest.java @@ -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()); + } } 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 fda694e..d90463e 100644 --- a/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java +++ b/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java @@ -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 = ArgumentCaptor.forClass(Registration.class); + verify(registrationService).register(registration.capture(), any()); + assertEquals("bob@example.com", registration.getValue().getEmail()); + } } diff --git a/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java b/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java index 75952d6..292abdd 100644 --- a/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java +++ b/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java @@ -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(); + }); + } } diff --git a/src/test/java/auth_service/modules/user/persistence/mapper/EntityMapperTest.java b/src/test/java/auth_service/modules/user/persistence/mapper/EntityMapperTest.java new file mode 100644 index 0000000..58c9ce9 --- /dev/null +++ b/src/test/java/auth_service/modules/user/persistence/mapper/EntityMapperTest.java @@ -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(); + } +}