diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index cbc8f6d1..ffc932e7 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -117,12 +117,17 @@ public class GeschichteService { * *

Returns a {@link GeschichteSummary} projection — never carries items, preventing * LazyInitializationException on the non-transactional list path. + * + *

Security: {@code null} status always resolves to PUBLISHED — even for blog writers. + * Only an explicit {@code DRAFT} request scopes the query to the caller's own drafts. + * This prevents CWE-639: a blog writer passing {@code null} must not see all authors' drafts. */ public List list(GeschichteStatus status, List personIds, UUID documentId, int limit) { - GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED; + boolean isDraftRequest = currentUserHasBlogWrite() && status == GeschichteStatus.DRAFT; + GeschichteStatus effective = isDraftRequest ? GeschichteStatus.DRAFT : GeschichteStatus.PUBLISHED; int safeLimit = limit <= 0 ? DEFAULT_LIMIT : Math.min(limit, MAX_LIMIT); - UUID authorId = effective == GeschichteStatus.DRAFT ? currentUser().getId() : null; + UUID authorId = isDraftRequest ? currentUser().getId() : null; // When personIds is empty, personCount=0 short-circuits the IN() predicate. // Pass a sentinel UUID to avoid invalid empty IN() SQL while the predicate is skipped. diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java index 2e87c50c..61962436 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.geschichte; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -35,6 +36,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -228,21 +230,7 @@ class GeschichteServiceTest { geschichteService.list(null, List.of(), null, 50); - verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any()); - } - - @Test - void list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible() { - authenticateAs(writer, Permission.BLOG_WRITE); - GeschichteSummary s1 = mock(GeschichteSummary.class); - GeschichteSummary s2 = mock(GeschichteSummary.class); - when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) - .thenReturn(List.of(s1, s2)); - - List out = geschichteService.list(null, List.of(), null, 50); - - assertThat(out).hasSize(2); - verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any()); + verify(geschichteRepository).findSummaries(eq(GeschichteStatus.PUBLISHED), isNull(), any(), anyLong(), any()); } @Test @@ -308,6 +296,33 @@ class GeschichteServiceTest { assertThat(out).hasSizeLessThanOrEqualTo(200); } + @Test + @DisplayName("security: null status for blog writer returns PUBLISHED, never leaks drafts") + void list_with_blog_writer_and_null_status_returns_PUBLISHED_not_all_drafts() { + authenticateAs(writer, Permission.BLOG_WRITE); + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) + .thenReturn(List.of()); + + geschichteService.list(null, List.of(), null, 50); + + verify(geschichteRepository).findSummaries( + eq(GeschichteStatus.PUBLISHED), isNull(), any(), anyLong(), any()); + } + + @Test + @DisplayName("security: DRAFT status scopes to current user only") + void list_with_DRAFT_status_scopes_to_current_user_not_all_authors() { + authenticateAs(writer, Permission.BLOG_WRITE); + when(userService.findByEmail(writer.getEmail())).thenReturn(writer); + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) + .thenReturn(List.of()); + + geschichteService.list(GeschichteStatus.DRAFT, List.of(), null, 50); + + verify(geschichteRepository).findSummaries( + eq(GeschichteStatus.DRAFT), eq(writer.getId()), any(), anyLong(), any()); + } + // ─── create ────────────────────────────────────────────────────────────── @Test diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 961597fe..bfc2d084 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1042,6 +1042,10 @@ "geschichten_filter_document_chip": "Gefiltert nach Brief:", "geschichten_filter_remove_document_chip": "Brief {title} aus Filter entfernen", "geschichten_empty_for_document": "Noch keine Geschichten zu diesem Brief", + "geschichten_published_heading": "Veröffentlicht", + "geschichten_drafts_heading": "Entwürfe", + "geschichten_draft_badge": "Entwurf", + "geschichten_drafts_unfiltered_caption": "(alle Entwürfe)", "geschichten_back_to_index": "Zurück zu Geschichten", "geschichten_published_on": "veröffentlicht am {date}", "journey_compiled_on": "zusammengestellt am {date}", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 7d957e6a..2ec576aa 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1042,6 +1042,10 @@ "geschichten_filter_document_chip": "Filtered by letter:", "geschichten_filter_remove_document_chip": "Remove letter {title} from filter", "geschichten_empty_for_document": "No stories reference this letter yet", + "geschichten_published_heading": "Published", + "geschichten_drafts_heading": "Drafts", + "geschichten_draft_badge": "Draft", + "geschichten_drafts_unfiltered_caption": "(all drafts)", "geschichten_back_to_index": "Back to stories", "geschichten_published_on": "published on {date}", "journey_compiled_on": "compiled on {date}", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 6ad45fcc..0bcd27ac 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1042,6 +1042,10 @@ "geschichten_filter_document_chip": "Filtrado por carta:", "geschichten_filter_remove_document_chip": "Quitar la carta {title} del filtro", "geschichten_empty_for_document": "Aún no hay historias sobre esta carta", + "geschichten_published_heading": "Publicadas", + "geschichten_drafts_heading": "Borradores", + "geschichten_draft_badge": "Borrador", + "geschichten_drafts_unfiltered_caption": "(todos los borradores)", "geschichten_back_to_index": "Volver a Historias", "geschichten_published_on": "publicada el {date}", "journey_compiled_on": "recopilada el {date}", diff --git a/frontend/src/lib/geschichte/GeschichteListRow.svelte b/frontend/src/lib/geschichte/GeschichteListRow.svelte index 0138443b..f64a0cd1 100644 --- a/frontend/src/lib/geschichte/GeschichteListRow.svelte +++ b/frontend/src/lib/geschichte/GeschichteListRow.svelte @@ -7,12 +7,13 @@ import type { components } from '$lib/generated/api'; type GeschichteRow = Pick< components['schemas']['GeschichteSummary'], - 'id' | 'title' | 'body' | 'type' | 'author' | 'publishedAt' + 'id' | 'title' | 'body' | 'type' | 'status' | 'author' | 'publishedAt' >; let { geschichte }: { geschichte: GeschichteRow } = $props(); const isJourney = $derived(geschichte.type === 'JOURNEY'); +const isDraft = $derived(geschichte.status === 'DRAFT'); const publishedAt = $derived(formatPublishedAt(geschichte.publishedAt, 'short')); @@ -44,12 +45,20 @@ const authorName = $derived(formatAuthorName(geschichte.author)); {m.journey_badge_list()} {/if} + {#if isDraft} + + {m.geschichten_draft_badge()} + + {/if}

-
+
{authorName} + {#if isDraft} + + {m.geschichten_draft_badge()} + + {/if} {#if publishedAt} {publishedAt} {/if} diff --git a/frontend/src/lib/geschichte/GeschichteListRow.svelte.spec.ts b/frontend/src/lib/geschichte/GeschichteListRow.svelte.spec.ts index 52babd9c..4c430716 100644 --- a/frontend/src/lib/geschichte/GeschichteListRow.svelte.spec.ts +++ b/frontend/src/lib/geschichte/GeschichteListRow.svelte.spec.ts @@ -91,4 +91,34 @@ describe('GeschichteListRow', () => { render(GeschichteListRow, { props: { geschichte: baseRow() } }); expect(document.body.textContent).toContain('Anna Schmidt'); }); + + it('shows no draft badge for PUBLISHED stories', async () => { + render(GeschichteListRow, { props: { geschichte: baseRow({ status: 'PUBLISHED' }) } }); + expect(document.querySelector('[data-testid="draft-badge"]')).toBeNull(); + expect(document.querySelector('[data-testid="draft-badge-mobile"]')).toBeNull(); + }); + + it('shows desktop draft badge for DRAFT stories', async () => { + render(GeschichteListRow, { props: { geschichte: baseRow({ status: 'DRAFT' }) } }); + const badge = document.querySelector('[data-testid="draft-badge"]'); + expect(badge).not.toBeNull(); + }); + + it('shows mobile draft badge for DRAFT stories', async () => { + render(GeschichteListRow, { props: { geschichte: baseRow({ status: 'DRAFT' }) } }); + const badge = document.querySelector('[data-testid="draft-badge-mobile"]'); + expect(badge).not.toBeNull(); + }); + + it('draft badge is a plain ', async () => { + render(GeschichteListRow, { props: { geschichte: baseRow({ status: 'DRAFT' }) } }); + const badge = document.querySelector('[data-testid="draft-badge"]'); + expect(badge?.tagName.toLowerCase()).toBe('span'); + }); + + it('draft badge uses text-xs label size', async () => { + render(GeschichteListRow, { props: { geschichte: baseRow({ status: 'DRAFT' }) } }); + const badge = document.querySelector('[data-testid="draft-badge"]'); + expect(badge!.className).toContain('text-xs'); + }); }); diff --git a/frontend/src/lib/shared/server/settled.test.ts b/frontend/src/lib/shared/server/settled.test.ts new file mode 100644 index 00000000..26ca88ec --- /dev/null +++ b/frontend/src/lib/shared/server/settled.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest'; +import { settled } from './settled'; + +describe('settled', () => { + it('returns the data for a fulfilled ok response', () => { + const res: PromiseSettledResult = { + status: 'fulfilled', + value: { response: { ok: true } as Response, data: [{ id: '1' }] } + }; + expect(settled<{ id: string }[]>(res)).toEqual([{ id: '1' }]); + }); + + it('returns null for a fulfilled non-ok response', () => { + const res: PromiseSettledResult = { + status: 'fulfilled', + value: { response: { ok: false, status: 403 } as Response, data: undefined } + }; + expect(settled(res)).toBeNull(); + }); + + it('returns null for a rejected result', () => { + const res: PromiseSettledResult = { + status: 'rejected', + reason: new Error('network error') + }; + expect(settled(res)).toBeNull(); + }); + + it('returns null for undefined input', () => { + expect(settled(undefined)).toBeNull(); + }); + + it('returns null for a fulfilled null value (Promise.resolve(null) placeholder slot)', () => { + const res: PromiseSettledResult = { status: 'fulfilled', value: null }; + expect(settled(res)).toBeNull(); + }); +}); diff --git a/frontend/src/lib/shared/server/settled.ts b/frontend/src/lib/shared/server/settled.ts new file mode 100644 index 00000000..41dde16d --- /dev/null +++ b/frontend/src/lib/shared/server/settled.ts @@ -0,0 +1,5 @@ +export function settled(res: PromiseSettledResult | undefined): T | null { + if (res?.status !== 'fulfilled') return null; + const v = res.value as { response: Response; data: unknown } | null; + return v?.response?.ok ? ((v.data as T) ?? null) : null; +} diff --git a/frontend/src/routes/+page.server.ts b/frontend/src/routes/+page.server.ts index f1aea7d8..efcbeb18 100644 --- a/frontend/src/routes/+page.server.ts +++ b/frontend/src/routes/+page.server.ts @@ -1,6 +1,7 @@ import { redirect } from '@sveltejs/kit'; import { createApiClient } from '$lib/shared/api.server'; import type { components } from '$lib/generated/api'; +import { settled } from '$lib/shared/server/settled'; type StatsDTO = components['schemas']['StatsDTO']; type TranscriptionQueueItemDTO = components['schemas']['TranscriptionQueueItemDTO']; @@ -14,12 +15,6 @@ type DocumentListItem = components['schemas']['DocumentListItem']; type GeschichteSummary = components['schemas']['GeschichteSummary']; type TagTreeNodeDTO = components['schemas']['TagTreeNodeDTO']; -function settled(res: PromiseSettledResult | undefined): T | null { - if (res?.status !== 'fulfilled') return null; - const v = res.value as { response: Response; data: unknown }; - return v.response.ok ? ((v.data as T) ?? null) : null; -} - export async function load({ fetch, parent }) { const { canWrite, canAnnotate, canBlogWrite } = await parent(); // READ_ALL without WRITE_ALL or ANNOTATE_ALL — see ADR-007. diff --git a/frontend/src/routes/geschichten/+page.server.ts b/frontend/src/routes/geschichten/+page.server.ts index 76828641..d89a14b7 100644 --- a/frontend/src/routes/geschichten/+page.server.ts +++ b/frontend/src/routes/geschichten/+page.server.ts @@ -1,20 +1,23 @@ import { error } from '@sveltejs/kit'; import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { getErrorMessage } from '$lib/shared/errors'; +import { settled } from '$lib/shared/server/settled'; import type { components } from '$lib/generated/api'; import type { PageServerLoad } from './$types'; type Person = components['schemas']['Person']; +type GeschichteSummary = components['schemas']['GeschichteSummary']; const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; -export const load: PageServerLoad = async ({ url, fetch }) => { +export const load: PageServerLoad = async ({ url, fetch, parent }) => { + const { canBlogWrite } = await parent(); const api = createApiClient(fetch); const personIds = url.searchParams.getAll('personId'); const rawDocumentId = url.searchParams.get('documentId'); const documentId = rawDocumentId && UUID_PATTERN.test(rawDocumentId) ? rawDocumentId : null; - const [listResult, docResult, ...personResults] = await Promise.all([ + const [listSettled, draftsSettled, docSettled, ...personResults] = await Promise.allSettled([ api.GET('/api/geschichten', { params: { query: { @@ -24,20 +27,32 @@ export const load: PageServerLoad = async ({ url, fetch }) => { } } }), + canBlogWrite + ? api.GET('/api/geschichten', { params: { query: { status: 'DRAFT' } } }) + : Promise.resolve(null), documentId ? api.GET('/api/documents/{id}', { params: { path: { id: documentId } } }) : Promise.resolve(null), ...personIds.map((id) => api.GET('/api/persons/{id}', { params: { path: { id } } })) ]); - if (!listResult.response.ok) { - throw error(listResult.response.status, getErrorMessage(extractErrorCode(listResult.error))); + const listResult = listSettled.status === 'fulfilled' ? listSettled.value : null; + if (!listResult?.response.ok) { + throw error( + listResult?.response.status ?? 500, + getErrorMessage(extractErrorCode(listResult?.error)) + ); } - const personFilters = personResults - .filter((r) => r && r.response.ok && r.data) - .map((r) => r!.data!) as Person[]; + const drafts: GeschichteSummary[] = canBlogWrite + ? (settled(draftsSettled) ?? []) + : []; + const personFilters = personResults + .filter((r) => r.status === 'fulfilled' && r.value?.response.ok && r.value?.data) + .map((r) => (r as PromiseFulfilledResult<{ response: Response; data: Person }>).value.data); + + const docResult = docSettled.status === 'fulfilled' ? docSettled.value : null; let documentFilter: { id: string; title: string | null } | null = null; if (documentId) { if (docResult && docResult.response.ok && docResult.data) { @@ -53,6 +68,7 @@ export const load: PageServerLoad = async ({ url, fetch }) => { return { geschichten: listResult.data ?? [], + drafts, personFilters, documentFilter }; diff --git a/frontend/src/routes/geschichten/+page.svelte b/frontend/src/routes/geschichten/+page.svelte index d14f950e..b1b43cb9 100644 --- a/frontend/src/routes/geschichten/+page.svelte +++ b/frontend/src/routes/geschichten/+page.svelte @@ -133,7 +133,32 @@ function removeDocument() {
{/if} - + + {#if data.drafts.length > 0} +
+

+ {m.geschichten_drafts_heading()} + {m.geschichten_drafts_unfiltered_caption()} +

+
    + {#each data.drafts as g (g.id)} +
  • + +
    • + {/each} +
+
+ {/if} + + + {#if data.drafts.length > 0} + +

+ {m.geschichten_published_heading()} +

+ {/if} {#if data.geschichten.length === 0}
{emptyMessage} diff --git a/frontend/src/routes/geschichten/page.server.test.ts b/frontend/src/routes/geschichten/page.server.test.ts index c9ce8b94..fd532424 100644 --- a/frontend/src/routes/geschichten/page.server.test.ts +++ b/frontend/src/routes/geschichten/page.server.test.ts @@ -24,11 +24,18 @@ function makeUrl(params: Record = {}) { return url; } -function callLoad(url: URL) { +function callLoad(url: URL, parentData: { canBlogWrite?: boolean } = {}) { return load({ url, request: new Request('http://localhost/geschichten'), - fetch: vi.fn() as unknown as typeof fetch + fetch: vi.fn() as unknown as typeof fetch, + parent: vi.fn().mockResolvedValue({ + canBlogWrite: false, + canWrite: false, + canAnnotate: false, + user: null, + ...parentData + }) }); } @@ -191,3 +198,60 @@ describe('geschichten page load — documentFilter title resolution', () => { ); }); }); + +// ─── draft fetch ────────────────────────────────────────────────────────────── + +describe('geschichten page load — draft fetch', () => { + it('makes a second API call for DRAFT stories when canBlogWrite is true', async () => { + const mockGet = mockApi(); + + await callLoad(makeUrl(), { canBlogWrite: true }); + + const draftCall = mockGet.mock.calls.find( + ([, opts]: [string, { params?: { query?: { status?: string } } }]) => + opts?.params?.query?.status === 'DRAFT' + ); + expect(draftCall).toBeDefined(); + }); + + it('does NOT make a DRAFT API call when canBlogWrite is false', async () => { + const mockGet = mockApi(); + + await callLoad(makeUrl(), { canBlogWrite: false }); + + const draftCall = mockGet.mock.calls.find( + ([, opts]: [string, { params?: { query?: { status?: string } } }]) => + opts?.params?.query?.status === 'DRAFT' + ); + expect(draftCall).toBeUndefined(); + }); + + it('returns empty drafts array when the DRAFT fetch rejects', async () => { + const mockGet = vi + .fn() + .mockResolvedValueOnce({ response: { ok: true, status: 200 }, data: [] }) + .mockRejectedValueOnce(new Error('network error')); + vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< + typeof createApiClient + >); + + const result = await callLoad(makeUrl(), { canBlogWrite: true }); + + expect(result.drafts).toEqual([]); + }); + + it('returns drafts in page data when canBlogWrite is true', async () => { + const draft = { id: 'draft-1', title: 'My Draft', status: 'DRAFT' }; + const mockGet = vi + .fn() + .mockResolvedValueOnce({ response: { ok: true, status: 200 }, data: [] }) + .mockResolvedValueOnce({ response: { ok: true, status: 200 }, data: [draft] }); + vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< + typeof createApiClient + >); + + const result = await callLoad(makeUrl(), { canBlogWrite: true }); + + expect(result.drafts).toEqual([draft]); + }); +}); diff --git a/frontend/src/routes/geschichten/page.svelte.spec.ts b/frontend/src/routes/geschichten/page.svelte.spec.ts index 410b1678..059eb589 100644 --- a/frontend/src/routes/geschichten/page.svelte.spec.ts +++ b/frontend/src/routes/geschichten/page.svelte.spec.ts @@ -26,6 +26,7 @@ function person(id: string, displayName: string) { function makeData(overrides: Partial = {}): PageData { return { geschichten: [], + drafts: [], personFilters: [], documentFilter: null, canBlogWrite: false, @@ -349,3 +350,62 @@ describe('geschichten page — multi-person filter chips', () => { expect(addEl.className).toContain('h-11'); }); }); + +// ─── Entwürfe section ───────────────────────────────────────────────────────── + +describe('geschichten page — Entwürfe section', () => { + const draft = () => + ({ + id: 'draft-1', + title: 'Mein Entwurf', + body: '

test

', + type: 'STORY', + status: 'DRAFT', + author: { firstName: 'Max', lastName: 'Muster' }, + publishedAt: null + }) as unknown as PageData['geschichten'][0]; + + it('Entwürfe section is hidden when drafts array is empty', async () => { + render(Page, { data: makeData({ drafts: [] }) }); + const heading = Array.from(document.querySelectorAll('h2')).find( + (h) => h.textContent?.includes('Entwürfe') || h.textContent?.includes('Drafts') + ); + expect(heading).toBeUndefined(); + }); + + it('Entwürfe section is visible when drafts are present', async () => { + render(Page, { data: makeData({ drafts: [draft()] as PageData['geschichten'] }) }); + const heading = Array.from(document.querySelectorAll('h2')).find( + (h) => h.textContent?.includes('Entwürfe') || h.textContent?.includes('Drafts') + ); + expect(heading).not.toBeUndefined(); + }); + + it('renders a row for each draft story', async () => { + render(Page, { data: makeData({ drafts: [draft()] as PageData['geschichten'] }) }); + const link = document.querySelector('a[href="/geschichten/draft-1"]'); + expect(link).not.toBeNull(); + }); + + it('draft row shows the draft badge', async () => { + render(Page, { data: makeData({ drafts: [draft()] as PageData['geschichten'] }) }); + const badge = document.querySelector('[data-testid="draft-badge"]'); + expect(badge).not.toBeNull(); + }); + + it('Veröffentlicht heading is present when the Entwürfe section is visible', async () => { + render(Page, { data: makeData({ drafts: [draft()] as PageData['geschichten'] }) }); + const heading = Array.from(document.querySelectorAll('h2')).find( + (h) => h.textContent?.includes('Veröffentlicht') || h.textContent?.includes('Published') + ); + expect(heading).not.toBeUndefined(); + }); + + it('Veröffentlicht heading is absent when there are no drafts', async () => { + render(Page, { data: makeData({ drafts: [] }) }); + const heading = Array.from(document.querySelectorAll('h2')).find( + (h) => h.textContent?.includes('Veröffentlicht') || h.textContent?.includes('Published') + ); + expect(heading).toBeUndefined(); + }); +});