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 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-03-28 15:42:03 +01:00
parent f568c0aeb7
commit 4ff87b035e
2 changed files with 83 additions and 0 deletions

View File

@@ -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

View File

@@ -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<HTMLInputElement>('input[type="password"]');