marcel/familienarchiv
1
0
Fork 0
Code Issues 119 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix(document): record actorId on DOCUMENT_DELETED audit (#805)

Browse Source 
deleteDocument now threads the acting user through to the audit log so
DOCUMENT_DELETED records who deleted the document, matching every other
audited write path (storeDocument, updateDocument, applyBulkEditToDocument,
attachFile). Tightens the AC-7 assertion from any() to the concrete actor.

Also adds the missing ApplicationEventPublisher @Mock to DocumentServiceTest,
which the publishEvent call (added with the cascade fix) left null.

Addresses @nora (CWE-778, blocker), @felix and @sara review on PR #806.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-11 19:17:52 +02:00
parent eefc67bd81
commit c2b98e6e84
6 changed files with 20 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java
View File

@@ -168,8 +168,8 @@ public class DocumentController {
@DeleteMapping("/{id}") @DeleteMapping("/{id}")
@RequirePermission(Permission.WRITE_ALL) @RequirePermission(Permission.WRITE_ALL)
public ResponseEntity<Void> deleteDocument(@PathVariable UUID id) { public ResponseEntity<Void> deleteDocument(@PathVariable UUID id, Authentication authentication) {
documentService.deleteDocument(id); documentService.deleteDocument(id, requireUserId(authentication));
return ResponseEntity.noContent().build(); return ResponseEntity.noContent().build();
} }

4
backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java
View File

@@ -1098,13 +1098,13 @@ public class DocumentService {
} }
@Transactional @Transactional
public void deleteDocument(UUID id) { public void deleteDocument(UUID id, UUID actorId) {
if (!documentRepository.existsById(id)) { if (!documentRepository.existsById(id)) {
throw DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id); throw DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id);
} }
eventPublisher.publishEvent(new DocumentDeletingEvent(id)); eventPublisher.publishEvent(new DocumentDeletingEvent(id));
documentRepository.deleteById(id); documentRepository.deleteById(id);
auditService.logAfterCommit(AuditKind.DOCUMENT_DELETED, null, id, null); auditService.logAfterCommit(AuditKind.DOCUMENT_DELETED, actorId, id, null);
} }
@Transactional @Transactional

1
backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java
View File

@@ -402,6 +402,7 @@ class DocumentControllerTest {
@WithMockUser(authorities = "WRITE_ALL") @WithMockUser(authorities = "WRITE_ALL")
void deleteDocument_returns204_whenHasWritePermission() throws Exception { void deleteDocument_returns204_whenHasWritePermission() throws Exception {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build());
mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders
.delete("/api/documents/" + id).with(csrf())) .delete("/api/documents/" + id).with(csrf()))
.andExpect(status().isNoContent()); .andExpect(status().isNoContent());

6
backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java
View File

@@ -30,6 +30,7 @@ import org.raddatz.familienarchiv.document.DocumentRepository;
import org.raddatz.familienarchiv.filestorage.FileService; import org.raddatz.familienarchiv.filestorage.FileService;
import org.raddatz.familienarchiv.tag.TagService; import org.raddatz.familienarchiv.tag.TagService;
import org.raddatz.familienarchiv.person.PersonService; import org.raddatz.familienarchiv.person.PersonService;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.data.domain.Page; import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.PageRequest;
@@ -75,6 +76,7 @@ class DocumentServiceTest {
@Mock AuditLogQueryService auditLogQueryService; @Mock AuditLogQueryService auditLogQueryService;
@Mock TranscriptionBlockQueryService transcriptionBlockQueryService; @Mock TranscriptionBlockQueryService transcriptionBlockQueryService;
@Mock ThumbnailAsyncRunner thumbnailAsyncRunner; @Mock ThumbnailAsyncRunner thumbnailAsyncRunner;
@Mock ApplicationEventPublisher eventPublisher;
// Real factory (pure, dependency-free) so save-time title-regeneration tests exercise the // Real factory (pure, dependency-free) so save-time title-regeneration tests exercise the
// shared composition rather than a stub — the #726 single source of truth. // shared composition rather than a stub — the #726 single source of truth.
@Spy DocumentTitleFactory documentTitleFactory = new DocumentTitleFactory(); @Spy DocumentTitleFactory documentTitleFactory = new DocumentTitleFactory();
@@ -87,7 +89,7 @@ class DocumentServiceTest {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
when(documentRepository.existsById(id)).thenReturn(true); when(documentRepository.existsById(id)).thenReturn(true);
documentService.deleteDocument(id); documentService.deleteDocument(id, UUID.randomUUID());
verify(documentRepository).deleteById(id); verify(documentRepository).deleteById(id);
} }
@@ -97,7 +99,7 @@ class DocumentServiceTest {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
when(documentRepository.existsById(id)).thenReturn(false); when(documentRepository.existsById(id)).thenReturn(false);
assertThatThrownBy(() -> documentService.deleteDocument(id)) assertThatThrownBy(() -> documentService.deleteDocument(id, UUID.randomUUID()))
.isInstanceOf(DomainException.class) .isInstanceOf(DomainException.class)
.hasMessageContaining(id.toString()); .hasMessageContaining(id.toString());
verify(documentRepository, never()).deleteById(any()); verify(documentRepository, never()).deleteById(any());

18
backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java
View File

@@ -109,7 +109,7 @@ class JourneyItemDocumentDeleteTest {
JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); JourneyItem.builder().geschichte(journey).position(10).document(doc).build());
em.clear(); em.clear();
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
assertThat(journeyItemRepository.findById(item.getId())).isEmpty(); assertThat(journeyItemRepository.findById(item.getId())).isEmpty();
assertThat(docRepo.findById(doc.getId())).isEmpty(); assertThat(docRepo.findById(doc.getId())).isEmpty();
@@ -123,7 +123,7 @@ class JourneyItemDocumentDeleteTest {
JourneyItem.builder().geschichte(journey).position(10).document(doc).note("curator context").build()); JourneyItem.builder().geschichte(journey).position(10).document(doc).note("curator context").build());
em.clear(); em.clear();
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
em.clear(); em.clear();
JourneyItem surviving = journeyItemRepository.findById(item.getId()).orElseThrow(); JourneyItem surviving = journeyItemRepository.findById(item.getId()).orElseThrow();
@@ -139,7 +139,7 @@ class JourneyItemDocumentDeleteTest {
JourneyItem.builder().geschichte(journey).position(10).note("Einleitung").build()); JourneyItem.builder().geschichte(journey).position(10).note("Einleitung").build());
em.clear(); em.clear();
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
em.clear(); em.clear();
JourneyItem reloaded = journeyItemRepository.findById(noteOnly.getId()).orElseThrow(); JourneyItem reloaded = journeyItemRepository.findById(noteOnly.getId()).orElseThrow();
@@ -163,7 +163,7 @@ class JourneyItemDocumentDeleteTest {
JourneyItem.builder().geschichte(journey2).position(10).document(doc).note("Begleittext").build()); JourneyItem.builder().geschichte(journey2).position(10).document(doc).note("Begleittext").build());
em.clear(); em.clear();
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
em.clear(); em.clear();
assertThat(journeyItemRepository.findById(noteLess.getId())).isEmpty(); assertThat(journeyItemRepository.findById(noteLess.getId())).isEmpty();
@@ -183,7 +183,7 @@ class JourneyItemDocumentDeleteTest {
doThrow(new RuntimeException("simulated failure")) doThrow(new RuntimeException("simulated failure"))
.when(documentRepository).deleteById(any()); .when(documentRepository).deleteById(any());
assertThatThrownBy(() -> documentService.deleteDocument(doc.getId())) assertThatThrownBy(() -> documentService.deleteDocument(doc.getId(), writer.getId()))
.isInstanceOf(RuntimeException.class); .isInstanceOf(RuntimeException.class);
em.clear(); em.clear();
@@ -213,7 +213,7 @@ class JourneyItemDocumentDeleteTest {
whitespaceNoteItemId, journey2.getId(), 20, doc.getId(), " "); whitespaceNoteItemId, journey2.getId(), 20, doc.getId(), " ");
em.clear(); em.clear();
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
em.clear(); em.clear();
assertThat(journeyItemRepository.findById(emptyNoteItemId)).isEmpty(); assertThat(journeyItemRepository.findById(emptyNoteItemId)).isEmpty();
@@ -235,7 +235,7 @@ class JourneyItemDocumentDeleteTest {
JourneyItem.builder().geschichte(journey).position(10).note("unrelated note").build()); JourneyItem.builder().geschichte(journey).position(10).note("unrelated note").build());
em.clear(); em.clear();
documentService.deleteDocument(unlinked.getId()); documentService.deleteDocument(unlinked.getId(), writer.getId());
em.clear(); em.clear();
assertThat(docRepo.findById(unlinked.getId())).isEmpty(); assertThat(docRepo.findById(unlinked.getId())).isEmpty();
@@ -251,9 +251,9 @@ class JourneyItemDocumentDeleteTest {
JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); JourneyItem.builder().geschichte(journey).position(10).document(doc).build());
em.clear(); em.clear();
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
verify(auditService).logAfterCommit(eq(AuditKind.DOCUMENT_DELETED), any(), eq(doc.getId()), any()); verify(auditService).logAfterCommit(eq(AuditKind.DOCUMENT_DELETED), eq(writer.getId()), eq(doc.getId()), any());
verify(auditService, never()).logAfterCommit(eq(AuditKind.JOURNEY_ITEM_REMOVED), any(), any(), any()); verify(auditService, never()).logAfterCommit(eq(AuditKind.JOURNEY_ITEM_REMOVED), any(), any(), any());
} }
} }

4
backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java
View File

@@ -220,7 +220,7 @@ class JourneyItemIntegrationTest {
// Route through service so the DocumentDeletingEvent fires and the listener // Route through service so the DocumentDeletingEvent fires and the listener
// removes note-less items before ON DELETE SET NULL acts on note-carrying rows. // removes note-less items before ON DELETE SET NULL acts on note-carrying rows.
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
em.flush(); em.flush();
em.clear(); em.clear();
@@ -359,7 +359,7 @@ class JourneyItemIntegrationTest {
em.clear(); em.clear();
// Route through service so the DocumentDeletingEvent fires (V72 cascade fix). // Route through service so the DocumentDeletingEvent fires (V72 cascade fix).
documentService.deleteDocument(doc.getId()); documentService.deleteDocument(doc.getId(), writer.getId());
em.flush(); em.flush();
em.clear(); em.clear();