Compare commits

..

6 Commits

Author SHA1 Message Date
Marcel
1dd6e054fc test(audit): add GROUP_MEMBERSHIP_CHANGED integration test with payload assertions
Some checks failed
CI / Unit & Component Tests (push) Failing after 3m59s
CI / OCR Service Tests (push) Successful in 36s
CI / Backend Unit Tests (push) Failing after 2m57s
CI / Unit & Component Tests (pull_request) Failing after 3m0s
CI / OCR Service Tests (pull_request) Successful in 34s
CI / Backend Unit Tests (pull_request) Failing after 3m3s
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 15:53:55 +02:00
Marcel
23cff1cdd7 refactor(audit): drop @DirtiesContext, add @BeforeEach, use existsByKind in wait conditions
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 15:53:55 +02:00
Marcel
11d93919b2 refactor(audit): replace LIMIT :limit JPQL with Pageable in audit query
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 15:53:55 +02:00
Marcel
f6bcc4f72a refactor(audit): extract actorId() helper in UserController
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 15:53:55 +02:00
Marcel
f4a4436eda test(audit): add 403 permission tests for createUser, adminUpdateUser, deleteUser
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 15:53:55 +02:00
Marcel
1d3a3b3338 refactor(audit): extract groupChangePayload() from adminUpdateUser
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 15:53:55 +02:00
8 changed files with 127 additions and 35 deletions

View File

@@ -1,5 +1,7 @@
package org.raddatz.familienarchiv.audit;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
@@ -198,7 +200,5 @@ public interface AuditLogQueryRepository extends JpaRepository<AuditLog, UUID> {
""", nativeQuery = true)
List<ContributorRow> findRecentContributorsForDocuments(@Param("documentIds") List<UUID> documentIds);
@Query("SELECT a FROM AuditLog a WHERE a.kind IN :kinds ORDER BY a.happenedAt DESC LIMIT :limit")
List<AuditLog> findRecentByKinds(@Param("kinds") Collection<AuditKind> kinds,
@Param("limit") int limit);
Page<AuditLog> findByKindIn(Collection<AuditKind> kinds, Pageable pageable);
}

View File

@@ -1,6 +1,8 @@
package org.raddatz.familienarchiv.audit;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Sort;
import org.springframework.stereotype.Service;
import java.time.OffsetDateTime;
@@ -56,7 +58,8 @@ public class AuditLogQueryService {
}
public List<AuditLog> findRecentUserManagementEvents(int limit) {
return queryRepository.findRecentByKinds(Set.of(USER_CREATED, USER_DELETED, GROUP_MEMBERSHIP_CHANGED), limit);
PageRequest page = PageRequest.of(0, limit, Sort.by("happenedAt").descending());
return queryRepository.findByKindIn(Set.of(USER_CREATED, USER_DELETED, GROUP_MEMBERSHIP_CHANGED), page).getContent();
}
private Map<UUID, List<ActivityActorDTO>> toContributorMap(List<ContributorRow> rows) {

View File

@@ -5,4 +5,5 @@ import org.springframework.data.jpa.repository.JpaRepository;
import java.util.UUID;
public interface AuditLogRepository extends JpaRepository<AuditLog, UUID> {
boolean existsByKind(AuditKind kind);
}

View File

@@ -80,8 +80,7 @@ public class UserController {
@RequirePermission(Permission.ADMIN_USER)
public ResponseEntity<AppUser> createUser(Authentication authentication,
@Valid @RequestBody CreateUserRequest request) {
AppUser actor = userService.findByEmail(authentication.getName());
return ResponseEntity.ok(userService.createUserOrUpdate(actor.getId(), request));
return ResponseEntity.ok(userService.createUserOrUpdate(actorId(authentication), request));
}
@PutMapping("/users/{id}")
@@ -89,8 +88,7 @@ public class UserController {
public ResponseEntity<AppUser> adminUpdateUser(Authentication authentication,
@PathVariable UUID id,
@RequestBody AdminUpdateUserRequest dto) {
AppUser actor = userService.findByEmail(authentication.getName());
AppUser updated = userService.adminUpdateUser(actor.getId(), id, dto);
AppUser updated = userService.adminUpdateUser(actorId(authentication), id, dto);
updated.setPassword(null);
return ResponseEntity.ok(updated);
}
@@ -99,9 +97,12 @@ public class UserController {
@RequirePermission(Permission.ADMIN_USER)
public ResponseEntity<Void> deleteUser(Authentication authentication,
@PathVariable UUID id) {
AppUser actor = userService.findByEmail(authentication.getName());
userService.deleteUser(actor.getId(), id);
userService.deleteUser(actorId(authentication), id);
return ResponseEntity.ok().build();
}
private UUID actorId(Authentication auth) {
return userService.findByEmail(auth.getName()).getId();
}
}

View File

@@ -183,27 +183,27 @@ public class UserService {
}
if (dto.getGroupIds() != null) {
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));
}
Set<UserGroup> before = new HashSet<>(user.getGroups());
Set<UserGroup> after = new HashSet<>(groupRepository.findAllById(dto.getGroupIds()));
user.setGroups(after);
groupChangePayload(before, after, id, user.getEmail())
.ifPresent(payload -> auditService.logAfterCommit(AuditKind.GROUP_MEMBERSHIP_CHANGED, actorId, null, payload));
}
return userRepository.save(user);
}
private Optional<Map<String, Object>> groupChangePayload(
Set<UserGroup> before, Set<UserGroup> after, UUID userId, String email) {
Set<UUID> beforeIds = before.stream().map(UserGroup::getId).collect(toSet());
Set<UUID> afterIds = after.stream().map(UserGroup::getId).collect(toSet());
if (beforeIds.equals(afterIds)) return Optional.empty();
List<String> added = after.stream().filter(g -> !beforeIds.contains(g.getId())).map(UserGroup::getName).toList();
List<String> removed = before.stream().filter(g -> !afterIds.contains(g.getId())).map(UserGroup::getName).toList();
return Optional.of(Map.of("userId", userId.toString(), "email", email,
"addedGroups", added, "removedGroups", removed));
}
@Transactional
public void changePassword(UUID userId, ChangePasswordDTO dto) {
AppUser user = getById(userId);

View File

@@ -7,6 +7,8 @@ import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.raddatz.familienarchiv.model.AppUser;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
import java.util.Collection;
import java.util.List;
@@ -14,8 +16,8 @@ import java.util.Set;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyCollection;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
@@ -56,16 +58,17 @@ class AuditLogQueryServiceTest {
@Test
void findRecentUserManagementEvents_delegatesToRepositoryWithAllThreeKinds() {
AuditLog entry = AuditLog.builder().id(UUID.randomUUID()).kind(AuditKind.USER_CREATED).build();
when(queryRepository.findRecentByKinds(anyCollection(), eq(5))).thenReturn(List.of(entry));
when(queryRepository.findByKindIn(anyCollection(), any(Pageable.class)))
.thenReturn(new PageImpl<>(List.of(entry)));
List<AuditLog> result = auditLogQueryService.findRecentUserManagementEvents(5);
assertThat(result).containsExactly(entry);
verify(queryRepository).findRecentByKinds(
verify(queryRepository).findByKindIn(
argThat((Collection<AuditKind> kinds) ->
kinds.contains(AuditKind.USER_CREATED) &&
kinds.contains(AuditKind.USER_DELETED) &&
kinds.contains(AuditKind.GROUP_MEMBERSHIP_CHANGED)),
eq(5));
any(Pageable.class));
}
}

View File

@@ -1,21 +1,25 @@
package org.raddatz.familienarchiv.audit;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.raddatz.familienarchiv.PostgresContainerConfig;
import org.raddatz.familienarchiv.dto.AdminUpdateUserRequest;
import org.raddatz.familienarchiv.dto.CreateUserRequest;
import org.raddatz.familienarchiv.dto.GroupDTO;
import org.raddatz.familienarchiv.model.AppUser;
import org.raddatz.familienarchiv.model.UserGroup;
import org.raddatz.familienarchiv.repository.AppUserRepository;
import org.raddatz.familienarchiv.service.UserService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Import;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.bean.override.mockito.MockitoBean;
import org.springframework.transaction.support.TransactionTemplate;
import software.amazon.awssdk.services.s3.S3Client;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -25,7 +29,6 @@ import static org.awaitility.Awaitility.await;
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE)
@ActiveProfiles("test")
@Import(PostgresContainerConfig.class)
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
class UserManagementAuditIntegrationTest {
@MockitoBean S3Client s3Client;
@@ -35,6 +38,11 @@ class UserManagementAuditIntegrationTest {
@Autowired AuditLogQueryService auditLogQueryService;
@Autowired TransactionTemplate transactionTemplate;
@BeforeEach
void clearAuditLog() {
transactionTemplate.execute(status -> { auditLogRepository.deleteAll(); return null; });
}
@Test
void createAndDeleteUser_producesOrderedAuditEntries() {
// Create the actor (admin) user directly — bypasses audit logging so no FK issue
@@ -46,7 +54,7 @@ class UserManagementAuditIntegrationTest {
UUID actorId = actor.getId();
// The admin creation is logged with null actorId — clear to start with a clean slate
await().atMost(5, SECONDS).until(() -> auditLogRepository.count() > 0);
await().atMost(5, SECONDS).until(() -> auditLogRepository.existsByKind(AuditKind.USER_CREATED));
transactionTemplate.execute(status -> { auditLogRepository.deleteAll(); return null; });
// Create the target user — should emit USER_CREATED
@@ -57,7 +65,7 @@ class UserManagementAuditIntegrationTest {
userService.createUserOrUpdate(actorId, req);
return null;
});
await().atMost(5, SECONDS).until(() -> auditLogRepository.count() > 0);
await().atMost(5, SECONDS).until(() -> auditLogRepository.existsByKind(AuditKind.USER_CREATED));
// Delete the target user — should emit USER_DELETED
AppUser created = userRepository.findByEmail("audit-test@example.com").orElseThrow();
@@ -65,11 +73,58 @@ class UserManagementAuditIntegrationTest {
userService.deleteUser(actorId, created.getId());
return null;
});
await().atMost(5, SECONDS).until(() -> auditLogRepository.count() >= 2);
await().atMost(5, SECONDS).until(() -> auditLogRepository.existsByKind(AuditKind.USER_DELETED));
List<AuditLog> events = auditLogQueryService.findRecentUserManagementEvents(10);
assertThat(events).hasSize(2);
assertThat(events.get(0).getKind()).isEqualTo(AuditKind.USER_DELETED);
assertThat(events.get(1).getKind()).isEqualTo(AuditKind.USER_CREATED);
}
@Test
void updateUserGroups_producesGroupMembershipChangedEvent() {
// Create groups before creating users — required for group assignment on creation
GroupDTO groupADto = new GroupDTO(); groupADto.setName("Viewers"); groupADto.setPermissions(Set.of("READ_ALL"));
GroupDTO groupBDto = new GroupDTO(); groupBDto.setName("Editors"); groupBDto.setPermissions(Set.of("WRITE_ALL"));
UserGroup gA = transactionTemplate.execute(status -> userService.createGroup(groupADto));
UserGroup gB = transactionTemplate.execute(status -> userService.createGroup(groupBDto));
// Create actor (bootstrap — null actorId, event not relevant)
CreateUserRequest actorReq = new CreateUserRequest();
actorReq.setEmail("actor-group-test@test.example.com");
actorReq.setInitialPassword("secret");
AppUser actor = transactionTemplate.execute(status -> userService.createUserOrUpdate(null, actorReq));
await().atMost(5, SECONDS).until(() -> auditLogRepository.existsByKind(AuditKind.USER_CREATED));
transactionTemplate.execute(status -> { auditLogRepository.deleteAll(); return null; });
// Create target user pre-assigned to gA
CreateUserRequest targetReq = new CreateUserRequest();
targetReq.setEmail("target-group-test@test.example.com");
targetReq.setInitialPassword("secret");
targetReq.setGroupIds(List.of(gA.getId()));
transactionTemplate.execute(status -> userService.createUserOrUpdate(actor.getId(), targetReq));
await().atMost(5, SECONDS).until(() -> auditLogRepository.existsByKind(AuditKind.USER_CREATED));
transactionTemplate.execute(status -> { auditLogRepository.deleteAll(); return null; });
AppUser target = userRepository.findByEmail("target-group-test@test.example.com").orElseThrow();
// Change groups: Viewers → Editors
AdminUpdateUserRequest dto = new AdminUpdateUserRequest();
dto.setGroupIds(List.of(gB.getId()));
transactionTemplate.execute(status -> userService.adminUpdateUser(actor.getId(), target.getId(), dto));
await().atMost(5, SECONDS).until(() -> auditLogRepository.existsByKind(AuditKind.GROUP_MEMBERSHIP_CHANGED));
List<AuditLog> events = auditLogQueryService.findRecentUserManagementEvents(10);
assertThat(events).hasSize(1);
AuditLog event = events.get(0);
assertThat(event.getKind()).isEqualTo(AuditKind.GROUP_MEMBERSHIP_CHANGED);
assertThat(event.getPayload()).containsEntry("email", "target-group-test@test.example.com");
@SuppressWarnings("unchecked")
List<String> added = (List<String>) event.getPayload().get("addedGroups");
@SuppressWarnings("unchecked")
List<String> removed = (List<String>) event.getPayload().get("removedGroups");
assertThat(added).containsExactlyInAnyOrder("Editors");
assertThat(removed).containsExactlyInAnyOrder("Viewers");
}
}

View File

@@ -18,8 +18,10 @@ import java.util.UUID;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -104,4 +106,31 @@ class UserControllerTest {
.content("{\"email\":\"\",\"initialPassword\":\"secret123\"}"))
.andExpect(status().isBadRequest());
}
// ─── permission enforcement ───────────────────────────────────────────────
@Test
@WithMockUser(username = "reader@example.com")
void createUser_returns403_whenCallerLacksAdminUserPermission() throws Exception {
mockMvc.perform(post("/api/users")
.contentType(org.springframework.http.MediaType.APPLICATION_JSON)
.content("{\"email\":\"x@x.com\",\"initialPassword\":\"secret123\"}"))
.andExpect(status().isForbidden());
}
@Test
@WithMockUser(username = "reader@example.com")
void adminUpdateUser_returns403_whenCallerLacksAdminUserPermission() throws Exception {
mockMvc.perform(put("/api/users/" + UUID.randomUUID())
.contentType(org.springframework.http.MediaType.APPLICATION_JSON)
.content("{}"))
.andExpect(status().isForbidden());
}
@Test
@WithMockUser(username = "reader@example.com")
void deleteUser_returns403_whenCallerLacksAdminUserPermission() throws Exception {
mockMvc.perform(delete("/api/users/" + UUID.randomUUID()))
.andExpect(status().isForbidden());
}
}