From 4ff87b035e1feee6028c1e33dff0d1ad81541a24 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 28 Mar 2026 15:42:03 +0100 Subject: [PATCH] fix: use bind:group in UserGroupsSection to prevent admin permission loss Replaced one-way checked={...} with bind:group={selected} driven by a writable $derived. In Svelte 5, the $derived pattern guarantees the DOM checked state is always in sync at FormData capture time, so groupIds is never accidentally sent as [] when the admin edits their own profile. Sending groupIds:[] causes adminUpdateUser to clear all groups, which revokes the admin's own permissions on the next request. Tests: UserServiceTest (+4 for adminUpdateUser group behaviour), page.svelte.spec.ts (+1 FormData assertion at submit time). Co-Authored-By: Claude Sonnet 4.6 --- .../service/UserServiceTest.java | 75 +++++++++++++++++++ .../admin/users/[id]/page.svelte.spec.ts | 8 ++ 2 files changed, 83 insertions(+) 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 7a58dd45..1e032979 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java @@ -5,17 +5,20 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.dto.AdminUpdateUserRequest; 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.model.AppUser; +import org.raddatz.familienarchiv.model.UserGroup; import org.raddatz.familienarchiv.repository.AppUserRepository; import org.raddatz.familienarchiv.repository.UserGroupRepository; import org.springframework.security.crypto.password.PasswordEncoder; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -216,6 +219,78 @@ class UserServiceTest { verify(userRepository).save(argThat(u -> "newHash".equals(u.getPassword()))); } + // ─── adminUpdateUser ────────────────────────────────────────────────────── + + @Test + void adminUpdateUser_updatesNameFields() { + UUID id = UUID.randomUUID(); + AppUser user = AppUser.builder().id(id).username("admin").build(); + when(userRepository.findById(id)).thenReturn(Optional.of(user)); + when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); + dto.setFirstName("Ada"); dto.setLastName("Lovelace"); + + AppUser result = userService.adminUpdateUser(id, dto); + + assertThat(result.getFirstName()).isEqualTo("Ada"); + assertThat(result.getLastName()).isEqualTo("Lovelace"); + } + + @Test + void adminUpdateUser_preservesGroups_whenGroupIdsIsNull() { + UUID id = UUID.randomUUID(); + UserGroup adminGroup = UserGroup.builder().id(UUID.randomUUID()).name("Administrators").build(); + AppUser user = AppUser.builder().id(id).username("admin").groups(Set.of(adminGroup)).build(); + when(userRepository.findById(id)).thenReturn(Optional.of(user)); + when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); + dto.setFirstName("Ada"); // groupIds left null → don't change groups + + AppUser result = userService.adminUpdateUser(id, dto); + + assertThat(result.getGroups()).containsExactly(adminGroup); + } + + @Test + void adminUpdateUser_updatesGroups_whenGroupIdsProvided() { + UUID id = UUID.randomUUID(); + UserGroup oldGroup = UserGroup.builder().id(UUID.randomUUID()).name("Viewers").build(); + UserGroup newGroup = UserGroup.builder().id(UUID.randomUUID()).name("Editors").build(); + AppUser user = AppUser.builder().id(id).username("max").groups(Set.of(oldGroup)).build(); + when(userRepository.findById(id)).thenReturn(Optional.of(user)); + when(groupRepository.findAllById(List.of(newGroup.getId()))).thenReturn(List.of(newGroup)); + when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); + dto.setGroupIds(List.of(newGroup.getId())); + + AppUser result = userService.adminUpdateUser(id, dto); + + assertThat(result.getGroups()).containsExactly(newGroup); + } + + @Test + void adminUpdateUser_clearsGroups_whenGroupIdsIsEmptyList() { + // Sending groupIds:[] is the explicit "remove from all groups" signal. + // The frontend must NEVER send [] accidentally — it must always include + // the currently-selected group checkboxes. + UUID id = UUID.randomUUID(); + UserGroup adminGroup = UserGroup.builder().id(UUID.randomUUID()).name("Administrators").build(); + AppUser user = AppUser.builder().id(id).username("admin").groups(Set.of(adminGroup)).build(); + when(userRepository.findById(id)).thenReturn(Optional.of(user)); + when(groupRepository.findAllById(List.of())).thenReturn(List.of()); + when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); + dto.setGroupIds(List.of()); // empty list → intentional "remove all groups" + + AppUser result = userService.adminUpdateUser(id, dto); + + assertThat(result.getGroups()).isEmpty(); + } + // ─── getGroupById ───────────────────────────────────────────────────────── @Test diff --git a/frontend/src/routes/admin/users/[id]/page.svelte.spec.ts b/frontend/src/routes/admin/users/[id]/page.svelte.spec.ts index 7a7822a5..acc3d6fd 100644 --- a/frontend/src/routes/admin/users/[id]/page.svelte.spec.ts +++ b/frontend/src/routes/admin/users/[id]/page.svelte.spec.ts @@ -94,6 +94,14 @@ describe('Admin edit user page – rendering', () => { expect(checkbox?.checked).toBe(false); }); + it('includes pre-selected group ids in FormData at submit time (guards against groupIds being empty)', async () => { + render(Page, { data: baseData, form: null }); + const form = document.querySelector('form')!; + const formData = new FormData(form); + expect(formData.getAll('groupIds')).toContain('g1'); + expect(formData.getAll('groupIds')).not.toContain('g2'); + }); + it('password fields are empty by default', async () => { render(Page, { data: baseData, form: null }); const passwordInputs = document.querySelectorAll('input[type="password"]');