From f8f5ea634ede2888cec409365bb21e8fae84670c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 19 Apr 2026 09:03:29 +0200 Subject: [PATCH] refactor(invite): move user creation into UserService, add generateCode limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit InviteService was directly injecting AppUserRepository, UserGroupRepository, and PasswordEncoder — crossing domain boundaries that UserService owns. - Add UserService.createUser() with duplicate-email guard - Add UserService.findGroupsByIds() delegation method - InviteService now only injects UserService (not user repositories) - generateCode() now throws INTERNAL_ERROR after 10 failed attempts instead of looping indefinitely Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/service/InviteService.java | 53 ++++++------------ .../familienarchiv/service/UserService.java | 27 +++++++++ .../service/InviteServiceTest.java | 53 +++++++++--------- .../service/UserServiceTest.java | 55 +++++++++++++++++++ 4 files changed, 126 insertions(+), 62 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/InviteService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/InviteService.java index 375b4222..acbf49bc 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/InviteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/InviteService.java @@ -10,10 +10,7 @@ import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.InviteToken; import org.raddatz.familienarchiv.model.UserGroup; -import org.raddatz.familienarchiv.repository.AppUserRepository; import org.raddatz.familienarchiv.repository.InviteTokenRepository; -import org.raddatz.familienarchiv.repository.UserGroupRepository; -import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -28,19 +25,20 @@ public class InviteService { static final int MIN_PASSWORD_LENGTH = 8; private static final String CODE_ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; private static final int CODE_LENGTH = 10; + private static final int MAX_CODE_ATTEMPTS = 10; private static final SecureRandom SECURE_RANDOM = new SecureRandom(); private final InviteTokenRepository inviteTokenRepository; - private final AppUserRepository appUserRepository; - private final UserGroupRepository userGroupRepository; - private final PasswordEncoder passwordEncoder; + private final UserService userService; public String generateCode() { - String code; - do { - code = buildRandomCode(); - } while (inviteTokenRepository.findByCode(code).isPresent()); - return code; + for (int attempt = 0; attempt < MAX_CODE_ATTEMPTS; attempt++) { + String code = buildRandomCode(); + if (inviteTokenRepository.findByCode(code).isEmpty()) { + return code; + } + } + throw DomainException.internal(ErrorCode.INTERNAL_ERROR, "Failed to generate unique invite code after " + MAX_CODE_ATTEMPTS + " attempts"); } public InviteToken validateCode(String code) { @@ -54,7 +52,7 @@ public class InviteService { public InviteToken createInvite(CreateInviteRequest dto, AppUser creator) { Set groupIds = new HashSet<>(); if (dto.getGroupIds() != null && !dto.getGroupIds().isEmpty()) { - List groups = userGroupRepository.findAllById(dto.getGroupIds()); + List groups = userService.findGroupsByIds(dto.getGroupIds()); groups.forEach(g -> groupIds.add(g.getId())); } @@ -85,34 +83,19 @@ public class InviteService { "Password must be at least " + MIN_PASSWORD_LENGTH + " characters"); } - if (dto.getEmail() != null) { - appUserRepository.findByEmail(dto.getEmail()).ifPresent(existing -> { - throw DomainException.conflict(ErrorCode.EMAIL_ALREADY_IN_USE, - "Email already registered: " + dto.getEmail()); - }); - } - - Set groups = new HashSet<>(); - if (!token.getGroupIds().isEmpty()) { - groups.addAll(userGroupRepository.findAllById(token.getGroupIds())); - } - - AppUser user = AppUser.builder() - .email(dto.getEmail()) - .password(passwordEncoder.encode(dto.getPassword())) - .firstName(dto.getFirstName()) - .lastName(dto.getLastName()) - .groups(groups) - .enabled(true) - .build(); - - AppUser saved = appUserRepository.save(user); + AppUser user = userService.createUser( + dto.getEmail(), + dto.getPassword(), + dto.getFirstName(), + dto.getLastName(), + token.getGroupIds() + ); token.setUseCount(token.getUseCount() + 1); inviteTokenRepository.save(token); log.info("User {} registered via invite code {}", dto.getEmail(), dto.getCode()); - return saved; + return user; } @Transactional diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java index 406a53b7..5cdb47a5 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java @@ -66,6 +66,33 @@ public class UserService { return userRepository.save(user); } + @Transactional + public AppUser createUser(String email, String rawPassword, String firstName, String lastName, Set groupIds) { + userRepository.findByEmail(email).ifPresent(existing -> { + throw DomainException.conflict(ErrorCode.EMAIL_ALREADY_IN_USE, "Email already registered: " + email); + }); + + Set groups = new HashSet<>(); + if (groupIds != null && !groupIds.isEmpty()) { + groups.addAll(groupRepository.findAllById(groupIds)); + } + + AppUser user = AppUser.builder() + .email(email) + .password(passwordEncoder.encode(rawPassword)) + .firstName(firstName) + .lastName(lastName) + .groups(groups) + .enabled(true) + .build(); + + return userRepository.save(user); + } + + public List findGroupsByIds(Collection ids) { + return groupRepository.findAllById(ids); + } + @Transactional public void deleteUser(UUID userId) { AppUser user = userRepository.findById(userId) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/InviteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/InviteServiceTest.java index 501020f7..e8c2e4a9 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/InviteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/InviteServiceTest.java @@ -13,17 +13,10 @@ import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.InviteToken; import org.raddatz.familienarchiv.model.UserGroup; -import org.raddatz.familienarchiv.repository.AppUserRepository; import org.raddatz.familienarchiv.repository.InviteTokenRepository; -import org.raddatz.familienarchiv.repository.UserGroupRepository; -import org.springframework.security.crypto.password.PasswordEncoder; import java.time.LocalDateTime; import java.util.*; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.concurrent.atomic.AtomicInteger; import static org.assertj.core.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; @@ -33,9 +26,7 @@ import static org.mockito.Mockito.*; class InviteServiceTest { @Mock InviteTokenRepository inviteTokenRepository; - @Mock AppUserRepository appUserRepository; - @Mock UserGroupRepository userGroupRepository; - @Mock PasswordEncoder passwordEncoder; + @Mock UserService userService; @InjectMocks InviteService inviteService; private AppUser admin; @@ -64,6 +55,16 @@ class InviteServiceTest { verify(inviteTokenRepository, times(2)).findByCode(anyString()); } + @Test + void generateCode_throwsInternalError_afterMaxAttempts() { + when(inviteTokenRepository.findByCode(anyString())) + .thenReturn(Optional.of(InviteToken.builder().code("AAAAAAAAAA").build())); + assertThatThrownBy(() -> inviteService.generateCode()) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getCode()) + .isEqualTo(ErrorCode.INTERNAL_ERROR); + } + // ─── validateCode ───────────────────────────────────────────────────────── @Test @@ -145,7 +146,7 @@ class InviteServiceTest { void createInvite_assignsGroups_whenGroupIdsProvided() { UserGroup g = UserGroup.builder().id(UUID.randomUUID()).name("Familie").build(); when(inviteTokenRepository.findByCode(anyString())).thenReturn(Optional.empty()); - when(userGroupRepository.findAllById(anyList())).thenReturn(List.of(g)); + when(userService.findGroupsByIds(anyList())).thenReturn(List.of(g)); when(inviteTokenRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); CreateInviteRequest req = new CreateInviteRequest(); @@ -164,9 +165,9 @@ class InviteServiceTest { .groupIds(new HashSet<>()) .build(); when(inviteTokenRepository.findByCodeForUpdate("ABCDE12345")).thenReturn(Optional.of(token)); - when(appUserRepository.findByEmail(anyString())).thenReturn(Optional.empty()); - when(passwordEncoder.encode(anyString())).thenReturn("encoded"); - when(appUserRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + AppUser created = AppUser.builder().id(UUID.randomUUID()).email("new@test.com").build(); + when(userService.createUser(eq("new@test.com"), eq("password123"), eq("Max"), eq("Muster"), any())) + .thenReturn(created); when(inviteTokenRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); RegisterRequest req = new RegisterRequest(); @@ -176,11 +177,11 @@ class InviteServiceTest { req.setFirstName("Max"); req.setLastName("Muster"); - AppUser created = inviteService.redeemInvite(req); + AppUser result = inviteService.redeemInvite(req); - assertThat(created.getEmail()).isEqualTo("new@test.com"); + assertThat(result.getEmail()).isEqualTo("new@test.com"); assertThat(token.getUseCount()).isEqualTo(1); - verify(appUserRepository).save(any()); + verify(userService).createUser(eq("new@test.com"), eq("password123"), eq("Max"), eq("Muster"), any()); verify(inviteTokenRepository).save(token); } @@ -204,8 +205,8 @@ class InviteServiceTest { void redeemInvite_throwsConflict_whenEmailAlreadyInUse() { InviteToken token = InviteToken.builder().code("ABCDE12345").groupIds(new HashSet<>()).build(); when(inviteTokenRepository.findByCodeForUpdate("ABCDE12345")).thenReturn(Optional.of(token)); - when(appUserRepository.findByEmail("dupe@test.com")) - .thenReturn(Optional.of(AppUser.builder().email("dupe@test.com").build())); + when(userService.createUser(eq("dupe@test.com"), any(), any(), any(), any())) + .thenThrow(DomainException.conflict(ErrorCode.EMAIL_ALREADY_IN_USE, "Email already registered")); RegisterRequest req = new RegisterRequest(); req.setCode("ABCDE12345"); @@ -219,18 +220,15 @@ class InviteServiceTest { } @Test - void redeemInvite_assignsGroupsFromToken() { + void redeemInvite_passesGroupIdsFromTokenToUserService() { UUID groupId = UUID.randomUUID(); - UserGroup g = UserGroup.builder().id(groupId).name("Familie").build(); InviteToken token = InviteToken.builder() .code("ABCDE12345") .groupIds(new HashSet<>(Set.of(groupId))) .build(); when(inviteTokenRepository.findByCodeForUpdate("ABCDE12345")).thenReturn(Optional.of(token)); - when(appUserRepository.findByEmail(anyString())).thenReturn(Optional.empty()); - when(userGroupRepository.findAllById(any())).thenReturn(List.of(g)); - when(passwordEncoder.encode(anyString())).thenReturn("encoded"); - when(appUserRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + AppUser created = AppUser.builder().id(UUID.randomUUID()).email("new@test.com").build(); + when(userService.createUser(any(), any(), any(), any(), eq(Set.of(groupId)))).thenReturn(created); when(inviteTokenRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); RegisterRequest req = new RegisterRequest(); @@ -238,8 +236,9 @@ class InviteServiceTest { req.setEmail("new@test.com"); req.setPassword("password123"); - AppUser created = inviteService.redeemInvite(req); - assertThat(created.getGroups()).contains(g); + inviteService.redeemInvite(req); + + verify(userService).createUser(any(), any(), any(), any(), eq(Set.of(groupId))); } // ─── revokeInvite ───────────────────────────────────────────────────────── diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java index 42715028..15c684e4 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java @@ -10,6 +10,7 @@ import org.raddatz.familienarchiv.dto.ChangePasswordDTO; import org.raddatz.familienarchiv.dto.CreateUserRequest; import org.raddatz.familienarchiv.dto.UpdateProfileDTO; import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.UserGroup; import org.raddatz.familienarchiv.repository.AppUserRepository; @@ -644,6 +645,60 @@ class UserServiceTest { verify(groupRepository, never()).findAllById(any()); } + // ─── createUser ─────────────────────────────────────────────────────────── + + @Test + void createUser_savesNewUser_withEncodedPassword() { + when(userRepository.findByEmail("new@example.com")).thenReturn(Optional.empty()); + when(passwordEncoder.encode("secret")).thenReturn("hashed"); + AppUser saved = AppUser.builder().id(UUID.randomUUID()).email("new@example.com").build(); + when(userRepository.save(any())).thenReturn(saved); + + AppUser result = userService.createUser("new@example.com", "secret", "Max", "Muster", Set.of()); + + assertThat(result).isEqualTo(saved); + verify(passwordEncoder).encode("secret"); + verify(userRepository).save(any()); + } + + @Test + void createUser_throwsConflict_whenEmailAlreadyExists() { + AppUser existing = AppUser.builder().id(UUID.randomUUID()).email("dupe@example.com").build(); + when(userRepository.findByEmail("dupe@example.com")).thenReturn(Optional.of(existing)); + + assertThatThrownBy(() -> userService.createUser("dupe@example.com", "pass", null, null, Set.of())) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getCode()) + .isEqualTo(ErrorCode.EMAIL_ALREADY_IN_USE); + } + + @Test + void createUser_assignsGroupsFromIds() { + UUID groupId = UUID.randomUUID(); + UserGroup g = UserGroup.builder().id(groupId).name("Familie").build(); + when(userRepository.findByEmail("u@example.com")).thenReturn(Optional.empty()); + when(groupRepository.findAllById(Set.of(groupId))).thenReturn(List.of(g)); + when(passwordEncoder.encode(any())).thenReturn("hashed"); + when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + AppUser result = userService.createUser("u@example.com", "pass", null, null, Set.of(groupId)); + + assertThat(result.getGroups()).contains(g); + } + + // ─── findGroupsByIds ─────────────────────────────────────────────────────── + + @Test + void findGroupsByIds_delegatesToRepository() { + UUID id = UUID.randomUUID(); + UserGroup g = UserGroup.builder().id(id).name("Admins").build(); + when(groupRepository.findAllById(List.of(id))).thenReturn(List.of(g)); + + List result = userService.findGroupsByIds(List.of(id)); + + assertThat(result).containsExactly(g); + } + // ─── createGroup ────────────────────────────────────────────────────────── @Test