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 <noreply@anthropic.com>
This commit is contained in:
@@ -86,9 +86,11 @@ public class UserController {
|
||||
|
||||
@PutMapping("/users/{id}")
|
||||
@RequirePermission(Permission.ADMIN_USER)
|
||||
public ResponseEntity<AppUser> adminUpdateUser(@PathVariable UUID id,
|
||||
public ResponseEntity<AppUser> 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);
|
||||
}
|
||||
|
||||
@@ -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<UserGroup> groups = new HashSet<>(groupRepository.findAllById(dto.getGroupIds()));
|
||||
user.setGroups(groups);
|
||||
Set<UUID> beforeIds = user.getGroups().stream().map(UserGroup::getId).collect(toSet());
|
||||
Set<UserGroup> beforeGroups = new HashSet<>(user.getGroups());
|
||||
Set<UserGroup> newGroups = new HashSet<>(groupRepository.findAllById(dto.getGroupIds()));
|
||||
user.setGroups(newGroups);
|
||||
Set<UUID> afterIds = newGroups.stream().map(UserGroup::getId).collect(toSet());
|
||||
if (!beforeIds.equals(afterIds)) {
|
||||
List<String> added = newGroups.stream()
|
||||
.filter(g -> !beforeIds.contains(g.getId()))
|
||||
.map(UserGroup::getName).toList();
|
||||
List<String> 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);
|
||||
|
||||
@@ -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<java.util.Map<String, Object>> 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<String, Object> payload = payloadCaptor.getValue();
|
||||
assertThat(payload).containsEntry("email", "u@example.com");
|
||||
assertThat((java.util.List<String>) payload.get("addedGroups")).containsExactly("Editors");
|
||||
assertThat((java.util.List<String>) 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
|
||||
|
||||
Reference in New Issue
Block a user