From eb8f9d4dc40ffb6048b5ad7a41d524fa5e705b5d Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 26 Apr 2026 14:54:42 +0200 Subject: [PATCH] feat(audit): emit GROUP_MEMBERSHIP_CHANGED when admin updates user groups Adds actorId param to adminUpdateUser(), captures beforeGroups before mutation, computes added/removed group names, emits logAfterCommit only when the group set actually changes. Payload contains group names, not permission strings. Co-Authored-By: Claude Sonnet 4.6 --- .../controller/UserController.java | 6 +- .../familienarchiv/service/UserService.java | 22 ++++- .../service/UserServiceTest.java | 92 ++++++++++++++++--- 3 files changed, 102 insertions(+), 18 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/UserController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/UserController.java index 19a78ea7..b3753546 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/UserController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/UserController.java @@ -86,9 +86,11 @@ public class UserController { @PutMapping("/users/{id}") @RequirePermission(Permission.ADMIN_USER) - public ResponseEntity adminUpdateUser(@PathVariable UUID id, + public ResponseEntity adminUpdateUser(Authentication authentication, + @PathVariable UUID id, @RequestBody AdminUpdateUserRequest dto) { - AppUser updated = userService.adminUpdateUser(id, dto); + AppUser actor = userService.findByEmail(authentication.getName()); + AppUser updated = userService.adminUpdateUser(actor.getId(), id, dto); updated.setPassword(null); return ResponseEntity.ok(updated); } 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 3d99ce75..251acae4 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java @@ -28,6 +28,8 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import static java.util.stream.Collectors.toSet; + @Service @RequiredArgsConstructor @Slf4j @@ -156,7 +158,7 @@ public class UserService { } @Transactional - public AppUser adminUpdateUser(UUID id, AdminUpdateUserRequest dto) { + public AppUser adminUpdateUser(UUID actorId, UUID id, AdminUpdateUserRequest dto) { AppUser user = getById(id); if (dto.getEmail() != null && !dto.getEmail().isBlank()) { @@ -181,8 +183,22 @@ public class UserService { } if (dto.getGroupIds() != null) { - Set groups = new HashSet<>(groupRepository.findAllById(dto.getGroupIds())); - user.setGroups(groups); + Set beforeIds = user.getGroups().stream().map(UserGroup::getId).collect(toSet()); + Set beforeGroups = new HashSet<>(user.getGroups()); + Set newGroups = new HashSet<>(groupRepository.findAllById(dto.getGroupIds())); + user.setGroups(newGroups); + Set afterIds = newGroups.stream().map(UserGroup::getId).collect(toSet()); + if (!beforeIds.equals(afterIds)) { + List added = newGroups.stream() + .filter(g -> !beforeIds.contains(g.getId())) + .map(UserGroup::getName).toList(); + List removed = beforeGroups.stream() + .filter(g -> !afterIds.contains(g.getId())) + .map(UserGroup::getName).toList(); + auditService.logAfterCommit(AuditKind.GROUP_MEMBERSHIP_CHANGED, actorId, null, + Map.of("userId", id.toString(), "email", user.getEmail(), + "addedGroups", added, "removedGroups", removed)); + } } return userRepository.save(user); 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 b522da90..2ff5ce27 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java @@ -233,7 +233,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setFirstName("Ada"); dto.setLastName("Lovelace"); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getFirstName()).isEqualTo("Ada"); assertThat(result.getLastName()).isEqualTo("Lovelace"); @@ -250,7 +250,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setFirstName("Ada"); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getGroups()).containsExactly(adminGroup); } @@ -268,7 +268,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setGroupIds(List.of(newGroup.getId())); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getGroups()).containsExactly(newGroup); } @@ -285,7 +285,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setGroupIds(List.of()); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getGroups()).isEmpty(); } @@ -382,7 +382,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setNewPassword("newSecret"); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getPassword()).isEqualTo("newHashed"); } @@ -397,7 +397,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setNewPassword(" "); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getPassword()).isEqualTo("original"); verify(passwordEncoder, never()).encode(any()); @@ -412,7 +412,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setEmail(" "); - assertThatThrownBy(() -> userService.adminUpdateUser(id, dto)) + assertThatThrownBy(() -> userService.adminUpdateUser(UUID.randomUUID(), id, dto)) .isInstanceOf(DomainException.class) .hasMessageContaining("blank"); } @@ -429,7 +429,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setEmail("taken@example.com"); - assertThatThrownBy(() -> userService.adminUpdateUser(id, dto)) + assertThatThrownBy(() -> userService.adminUpdateUser(UUID.randomUUID(), id, dto)) .isInstanceOf(DomainException.class) .hasMessageContaining("E-Mail"); } @@ -565,7 +565,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setContact(null); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getContact()).isNull(); } @@ -580,7 +580,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setContact(" "); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getContact()).isNull(); } @@ -595,7 +595,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setContact(" phone: 555 "); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getContact()).isEqualTo("phone: 555"); } @@ -610,7 +610,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setEmail(null); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getEmail()).isEqualTo("keep@example.com"); } @@ -626,7 +626,7 @@ class UserServiceTest { AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); dto.setEmail("me@example.com"); - AppUser result = userService.adminUpdateUser(id, dto); + AppUser result = userService.adminUpdateUser(UUID.randomUUID(), id, dto); assertThat(result.getEmail()).isEqualTo("me@example.com"); } @@ -703,6 +703,72 @@ class UserServiceTest { assertThat(result).containsExactly(g); } + // ─── audit: GROUP_MEMBERSHIP_CHANGED ───────────────────────────────────── + + @Test + void adminUpdateUser_logsGroupMembershipChanged_whenGroupSetChanges() { + UUID actorId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + UserGroup oldGroup = UserGroup.builder().id(UUID.randomUUID()).name("Viewers").permissions(Set.of("READ_ALL")).build(); + UserGroup newGroup = UserGroup.builder().id(UUID.randomUUID()).name("Editors").permissions(Set.of("WRITE_ALL")).build(); + AppUser user = AppUser.builder().id(userId).email("u@example.com").groups(Set.of(oldGroup)).build(); + when(userRepository.findById(userId)).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())); + + userService.adminUpdateUser(actorId, userId, dto); + + @SuppressWarnings("unchecked") + ArgumentCaptor> payloadCaptor = ArgumentCaptor.forClass(java.util.Map.class); + verify(auditService).logAfterCommit( + org.mockito.ArgumentMatchers.eq(AuditKind.GROUP_MEMBERSHIP_CHANGED), + org.mockito.ArgumentMatchers.eq(actorId), + org.mockito.ArgumentMatchers.isNull(), + payloadCaptor.capture()); + java.util.Map payload = payloadCaptor.getValue(); + assertThat(payload).containsEntry("email", "u@example.com"); + assertThat((java.util.List) payload.get("addedGroups")).containsExactly("Editors"); + assertThat((java.util.List) payload.get("removedGroups")).containsExactly("Viewers"); + } + + @Test + void adminUpdateUser_doesNotLogGroupMembershipChanged_whenGroupsUnchanged() { + UUID actorId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + UserGroup group = UserGroup.builder().id(UUID.randomUUID()).name("Admins").build(); + AppUser user = AppUser.builder().id(userId).email("u@example.com").groups(Set.of(group)).build(); + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + when(groupRepository.findAllById(List.of(group.getId()))).thenReturn(List.of(group)); + when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); + dto.setGroupIds(List.of(group.getId())); + + userService.adminUpdateUser(actorId, userId, dto); + + verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); + } + + @Test + void adminUpdateUser_doesNotLogGroupMembershipChanged_whenGroupIdsIsNull() { + UUID actorId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + UserGroup group = UserGroup.builder().id(UUID.randomUUID()).name("Admins").build(); + AppUser user = AppUser.builder().id(userId).email("u@example.com").groups(Set.of(group)).build(); + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + when(userRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + AdminUpdateUserRequest dto = new AdminUpdateUserRequest(); + // groupIds not set → null + + userService.adminUpdateUser(actorId, userId, dto); + + verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); + } + // ─── audit: USER_DELETED ────────────────────────────────────────────────── @Test