Compare commits

..

25 Commits

Author SHA1 Message Date
Marcel
3d9e10f3e7 docs(adr-012): add enforcement note and libLoader revisit signal
Some checks failed
CI / fail2ban Regex (push) Successful in 38s
CI / Compose Bucket Idempotency (push) Successful in 56s
CI / Unit & Component Tests (push) Failing after 2m46s
CI / OCR Service Tests (push) Successful in 16s
CI / Backend Unit Tests (push) Successful in 4m12s
CI / OCR Service Tests (pull_request) Has been cancelled
CI / Backend Unit Tests (pull_request) Has been cancelled
CI / fail2ban Regex (pull_request) Has been cancelled
CI / Compose Bucket Idempotency (pull_request) Has been cancelled
CI / Unit & Component Tests (pull_request) Has been cancelled
- Explicitly states no lint rule is planned; CI guard is the backstop
  (addresses Elicit OQ-001 from PR #536 round 4)
- Adds a "when to revisit" note: extract shared DynamicImportLoader<T>
  if 3+ components adopt the libLoader pattern
  (addresses Markus Keller round-4 observation on PR #536)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 09:38:49 +02:00
Marcel
8396d82cb4 ci(coverage): document that birpc guard covers coverage run only
Adds a comment above the assertion step so a future developer diagnosing
a birpc-related failure in `npm test` knows where to find the diagnostic.

Addresses Sara Holt + Tobias Wendt round-4 observation on PR #536.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 09:37:50 +02:00
Marcel
185c754eee refactor(test): convert TextLayerMock to class syntax in PdfViewer spec
Prototype-style assignment was a vi.mock hoisting artifact from the old
version of the file. Rest of the codebase uses class syntax — aligning.

Addresses Felix Brandt round-4 suggestion on PR #536.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 09:36:47 +02:00
Marcel
0b3ce838f4 test(pdf-renderer): guard init() against repeated calls — libLoader must fire once
Adds idempotency test: calling init() twice must invoke libLoader only once.
Adds `if (pdfjsReady) return;` guard to satisfy the contract.

Addresses Felix Brandt round-4 suggestion on PR #536.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 09:25:13 +02:00
Marcel
48b7e0f3a2 ci(coverage): use grep -F for birpc guard to avoid BRE escaping
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m47s
CI / OCR Service Tests (push) Successful in 16s
CI / Backend Unit Tests (push) Successful in 4m12s
CI / fail2ban Regex (push) Successful in 38s
CI / Compose Bucket Idempotency (push) Successful in 55s
CI / Unit & Component Tests (pull_request) Failing after 2m46s
CI / OCR Service Tests (pull_request) Successful in 15s
CI / Backend Unit Tests (pull_request) Successful in 4m9s
CI / fail2ban Regex (pull_request) Successful in 38s
CI / Compose Bucket Idempotency (pull_request) Successful in 56s
-F (fixed string) matches the literal pattern [birpc] rpc is closed
without relying on BRE bracket escaping, making the intent explicit
and immune to accidental regex interpretation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 07:56:28 +02:00
Marcel
017b404f8f ci(coverage): include coverage log in artifact upload
The birpc guard step writes to /tmp/coverage-test-<run_id>.log and exits 1
when a race is detected. Without this file in the artifact, the evidence
disappears when the runner tears down — only the exit code remained visible.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 07:55:49 +02:00
Marcel
58eb6e27d3 refactor(test): remove what-comment from makeFakePdfjsLib
The name already implies it's a fake; the comment described what, not why.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 07:55:11 +02:00
Marcel
2163cd9634 test(pdf-renderer): assert init() re-throws when libLoader rejects
.catch(()=>{}) swallowed the rejection, so the test passed vacuously even
if a future refactor silently caught the error. rejects.toThrow() proves
the propagation contract holds before asserting pdfjsReady stays false.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 07:54:33 +02:00
Marcel
e418e884b5 ci(coverage): harden coverage guard step
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m47s
CI / OCR Service Tests (push) Successful in 15s
CI / Backend Unit Tests (push) Successful in 4m4s
CI / fail2ban Regex (push) Successful in 38s
CI / Compose Bucket Idempotency (push) Successful in 55s
CI / Unit & Component Tests (pull_request) Failing after 2m49s
CI / OCR Service Tests (pull_request) Successful in 16s
CI / Backend Unit Tests (pull_request) Successful in 4m9s
CI / fail2ban Regex (pull_request) Successful in 39s
CI / Compose Bucket Idempotency (pull_request) Successful in 56s
- Add explicit set -eo pipefail so npm test:coverage exit code
  propagates through the pipe (not just tee's always-0 exit)
- Scope log file to github.run_id to prevent stale-log false positives
  on retried steps sharing the same runner /tmp
- Tighten grep pattern to \[birpc\] rpc is closed to avoid matching
  unrelated log lines that happen to contain "rpc is closed"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 23:18:22 +02:00
Marcel
d8496498ea test(pdf-renderer): document libLoader rejection leaves pdfjsReady false
Regression-protection test: init() propagates the loader rejection
before pdfjsReady is set, so the renderer stays in a safe unready state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 23:17:30 +02:00
Marcel
b8d9c0e9d5 docs(pdf-viewer): comment untrack invariant on renderer init
Without untrack, a reactive libLoader prop reference change would
reinitialise the whole renderer and lose all loaded state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 23:13:41 +02:00
Marcel
e529f9f7d1 refactor(pdf-viewer): export LibLoader type and update callers
Exporting LibLoader gives the type a stable, named identity.
PdfViewer.svelte and PdfViewer.svelte.spec.ts now import it directly
instead of using Parameters<typeof createPdfRenderer>[0].

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 23:12:55 +02:00
Marcel
11c61c8a77 ci(coverage): simplify coverage step and pin shell to bash
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m47s
CI / OCR Service Tests (push) Successful in 16s
CI / Backend Unit Tests (push) Successful in 4m8s
CI / fail2ban Regex (push) Successful in 38s
CI / Compose Bucket Idempotency (push) Has been cancelled
CI / Unit & Component Tests (pull_request) Failing after 2m48s
CI / OCR Service Tests (pull_request) Successful in 15s
CI / Backend Unit Tests (pull_request) Successful in 4m9s
CI / fail2ban Regex (pull_request) Successful in 38s
CI / Compose Bucket Idempotency (pull_request) Successful in 23s
- removes unreachable `; exit ${PIPESTATUS[0]}` — already covered by pipefail (Tobias)
- adds explicit `shell: bash` to both new steps for clarity (Tobias)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 22:52:33 +02:00
Marcel
3bb940a2c2 refactor(test): move PdfViewer import to top and annotate partial fake cast
- import PdfViewer left mid-file from vi.mock hoisting — no longer needed (Sara/Felix)
- adds one-line comment explaining as unknown as cast is an intentional partial fake (Felix)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 22:49:34 +02:00
Marcel
b9e2ed4af8 docs(adr): 012 — browser-mode test mocking strategy
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m49s
CI / OCR Service Tests (push) Successful in 17s
CI / Backend Unit Tests (push) Successful in 4m13s
CI / fail2ban Regex (push) Successful in 37s
CI / Compose Bucket Idempotency (push) Successful in 56s
CI / Unit & Component Tests (pull_request) Failing after 2m47s
CI / OCR Service Tests (pull_request) Successful in 16s
CI / Backend Unit Tests (pull_request) Successful in 4m16s
CI / fail2ban Regex (pull_request) Successful in 38s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s
Documents why vi.mock(module, factory) races with birpc teardown for
dynamically-imported modules, the libLoader injection pattern used to fix
#535, and the residual exceptions ($app/*, $env/*) that are safe to keep
as vi.mock because they are resolved statically before any test runs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 21:32:12 +02:00
Marcel
67f53fcc58 ci(guard): fail unit-tests job if [birpc] rpc is closed appears in coverage run
Captures npm run test:coverage output with tee and adds an always-run step
that greps for the teardown-race fingerprint. Any future regression where a
vi.mock factory races with birpc teardown will now surface as an explicit CI
failure rather than a silent exit-1 after all tests report green (#535).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 21:31:03 +02:00
Marcel
7a4295d403 fix(pdf-viewer): replace vi.mock(pdfjs-dist) with injected libLoader prop
Removes both vi.mock('pdfjs-dist', factory) and
vi.mock('pdfjs-dist/build/pdf.worker.min.mjs?url', factory) from
PdfViewer.svelte.spec.ts — the ManualMockedModule registrations that were
racing with vitest-browser-playwright's birpc teardown channel.

PdfViewer.svelte now accepts an optional libLoader prop (typed as
Parameters<typeof createPdfRenderer>[0]) that is passed untracked to
createPdfRenderer(). Tests supply a vi.fn() fake loader directly as a prop;
production code uses the default loader that imports the real pdfjs-dist.
The birpc route handler for pdfjs-dist is never registered, so no teardown
race is possible. Fixes #535.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 21:30:15 +02:00
Marcel
49171e596d test(pdf-renderer): inject libLoader into createPdfRenderer to eliminate vi.mock factories
Adds an optional LibLoader parameter (defaults to the real pdfjs-dist dynamic
imports) and a failing test that verified the loader is called during init().
This is the first step toward removing ManualMockedModule registrations that
race with vitest-browser-playwright's birpc teardown (#535).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-11 21:25:38 +02:00
Marcel
5f3529439a fix(infra): frontend healthcheck on 127.0.0.1, not localhost
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m53s
CI / OCR Service Tests (pull_request) Successful in 17s
CI / Backend Unit Tests (pull_request) Successful in 4m33s
CI / fail2ban Regex (pull_request) Successful in 40s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m0s
CI / Unit & Component Tests (push) Failing after 2m52s
CI / OCR Service Tests (push) Successful in 18s
CI / Backend Unit Tests (push) Successful in 4m23s
CI / fail2ban Regex (push) Successful in 39s
CI / Compose Bucket Idempotency (push) Successful in 1m0s
The new alpine-based frontend production image (`node:20.19.0-alpine3.21`)
resolves `localhost` only to `::1` in /etc/hosts. SvelteKit's adapter-node
binds to 0.0.0.0 (IPv4 only), so `wget http://localhost:3000/login` from
inside the container connects to ::1 and gets "Connection refused" every
15s. Container goes unhealthy → `docker compose up --wait` fails → nightly
staging deploy fails. The app itself is fine.

Switching to 127.0.0.1 bypasses /etc/hosts and matches what Node actually
listens on.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 18:49:32 +02:00
Marcel
48c8bb8a5f fixup: address Nora's review on #520 (security blockers)
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m48s
CI / OCR Service Tests (push) Successful in 17s
CI / Backend Unit Tests (push) Successful in 4m10s
CI / fail2ban Regex (push) Successful in 38s
CI / Compose Bucket Idempotency (push) Successful in 56s
- frontend/login: derive cookie `secure` flag from request URL protocol.
  Pre-PR the cookie was only read by SSR so the flag didn't matter; now
  the cookie IS the API credential and must be Secure on HTTPS or it
  leaks a 24h Basic token on plaintext networks. Dev runs over HTTP and
  would silently lose the cookie if we hardcoded `secure: true`, so the
  flag follows `event.url.protocol === 'https:'`.

- SecurityConfig: rewrite the CSRF-disabled comment. The old
  "browsers block cross-origin custom headers" justification no longer
  holds once /api/* is authenticated via the cookie. Make the
  load-bearing dependencies explicit: SameSite=strict on the auth_token
  cookie + Spring's default CORS rejection.

- AuthTokenCookieFilter:
  - Scope to /api/* only. /actuator/health and similar must not be
    cookie-authenticated.
  - Refuse malformed percent-encoding (URLDecoder throws); forward the
    request without a promoted Authorization rather than crash.
  - Use isBlank() instead of isEmpty() per Nora.
  - Javadoc warning: getHeaderNames/getHeaders exposes the Basic
    credential; any future header-iterating logger must scrub
    Authorization before logging.

- Tests: add `passes_through_unchanged_when_request_is_outside_api_scope`
  (/actuator/health with cookie should NOT be wrapped) and
  `passes_through_unchanged_when_cookie_value_is_malformed_percent_encoding`.
  Tighten the explicit-header test to verify same-instance forwarding
  rather than just header equality.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 18:20:10 +02:00
Marcel
023810df1e fix(security): promote auth_token cookie to Authorization header for browser /api/* calls
Closes #520.

The login action stores `Basic <base64>` in an HttpOnly `auth_token`
cookie. SSR fetches from hooks.server.ts explicitly set the
Authorization header. Vite's dev proxy does the same on every
/api/* request. Caddy in production does NOT. So browser-side
fetch() and EventSource() calls reach the backend without auth,
get 401 + WWW-Authenticate: Basic, and the browser pops a native
auth dialog over the SPA.

Add AuthTokenCookieFilter (Ordered.HIGHEST_PRECEDENCE, before any
Spring Security filter) that promotes the cookie to a request
header when no explicit Authorization is present. URL-decodes the
cookie value because SvelteKit URL-encodes spaces ("Basic " ->
"Basic%20") when serializing the cookie. Works the same for REST,
SSE (/api/notifications/stream, /api/ocr/jobs/.../progress), and
any other browser-direct backend call.

5 tests in AuthTokenCookieFilterTest cover: URL-decoded promotion,
explicit-Authorization-wins precedence, no-cookies pass-through,
absent-auth-token pass-through, empty-value pass-through.

Also: add `@ActiveProfiles("test")` to ThumbnailServiceIntegrationTest,
the one remaining @SpringBootTest in the suite that wasn't annotated.
After #516 made UserDataInitializer fail-closed outside dev/test/e2e,
this test's context load was throwing. Restores green main.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 18:20:10 +02:00
Marcel
ad3b571bba fix(user): findOrCreate Administrators group instead of blind-INSERT (#518)
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m50s
CI / OCR Service Tests (pull_request) Successful in 16s
CI / Backend Unit Tests (pull_request) Failing after 4m12s
CI / fail2ban Regex (pull_request) Successful in 39s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CI / Compose Bucket Idempotency (push) Has been cancelled
Closes #518.

UserDataInitializer.initAdminUser was doing groupRepository.save(adminGroup)
unconditionally. If a previous boot had seeded the group but failed
before creating the admin user (or if the operator deleted just the
admin row to retry with a corrected APP_ADMIN_USERNAME), the next
seed attempt violated user_groups_name_key and aborted the context.

Switch to the same findByName(...).orElseGet(...) pattern initE2EData
already uses for the "Leser" group.

Tests in AdminSeedFailClosedTest:
- reuses_existing_Administrators_group_when_seeding_a_new_admin
- creates_Administrators_group_when_seeding_admin_on_a_fresh_database
Plus updated existing tests to stub groupRepository.save now that the
seed path also exercises it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 17:29:11 +02:00
Marcel
9686e304c2 fix(caddy): wrap actuator block in handle so it takes precedence over catch-all
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CI / Compose Bucket Idempotency (push) Has been cancelled
Closes #512.

The previous `(block_actuator)` snippet emitted `respond @actuator 404`
at the top level of each archive vhost. But each vhost also has a
catch-all `handle { reverse_proxy ... }` that matches /actuator/*
too. Caddy's `handle` blocks are mutually exclusive — once one matches,
the request never reaches a top-level `respond`. So /actuator/health
was being proxied to the backend, which 302s to /login.

Wrap the actuator response in its own `handle /actuator/*` block.
Caddy sorts `handle` blocks by path specificity, so /actuator/* wins
over the catch-all and the 404 is actually returned.

Verified with `caddy validate` against the caddy:2 image.

Also unblocks the nightly.yml smoke test's `/actuator/health → 404`
assertion, which has been failing since the first staging deploy.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 17:15:03 +02:00
Marcel
ea0b3050e4 fix(user): fail-closed when admin seed would use dev defaults outside dev/test/e2e
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CI / Compose Bucket Idempotency (push) Has been cancelled
Addresses Nora's review concern on #513/#516.

The previous fix only made env-vars take effect — it did NOT close the
fail-open default path. If an operator forgets APP_ADMIN_USERNAME /
APP_ADMIN_PASSWORD on first prod boot, the seeded admin is the
well-known `admin@familienarchiv.local` / `admin123` and is permanently
locked (UserDataInitializer only seeds when the row is missing).

Refuse to seed outside dev/test/e2e profiles when either credential
matches the documented default. The startup fails fast with a clear
message pointing at the env-var names and the permanence trap.

Also adds Markus/Felix/Sara's "pin the Java side" coverage: a
reflection test on the @Value placeholder catches a future rename
of `${app.admin.email:...}` back to `${app.admin.username:...}`,
which would otherwise pass the yaml-side test but silently break
the binding.

Tests:
- AdminSeedFailClosedTest pins fail-closed for non-local profiles
  and verifies the dev/test/e2e bypass.
- AdminSeedPropertyKeyTest now also asserts the @Value placeholder
  string on UserDataInitializer.adminEmail/adminPassword.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 17:12:36 +02:00
Marcel
21343cdf23 fix(user): rename yaml key username→email so admin seed reads APP_ADMIN_USERNAME
Closes #513.

UserDataInitializer reads `@Value("${app.admin.email:...}")` but
application.yaml mapped APP_ADMIN_USERNAME to `app.admin.username`.
The keys never connected — env vars APP_ADMIN_USERNAME and
APP_ADMIN_PASSWORD were silently ignored and the admin user got
seeded with the hardcoded defaults admin@familyarchive.local /
admin123.

For production this is HIGH severity: DEPLOYMENT.md §3.5 documents
the admin password as permanently locked on first deploy. The
bug locked the lock-in to dev defaults, not to whatever an operator
set in PROD_APP_ADMIN_PASSWORD.

Rename yaml key from `username:` to `email:` so the Spring property
`app.admin.email` actually exists. Keep env-var name
APP_ADMIN_USERNAME (matches the already-set Gitea secrets and
DEPLOYMENT.md §3.3). Default value updated to an email-shape.

Added AdminSeedPropertyKeyTest (Binder pattern, no Spring context):
verifies both `app.admin.email` and `app.admin.password` resolve
from the yaml. Confirmed red without the fix, green with it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-11 17:12:36 +02:00
17 changed files with 792 additions and 49 deletions

View File

@@ -43,17 +43,34 @@ jobs:
TZ: Europe/Berlin
- name: Run coverage (server + client)
run: npm run test:coverage
shell: bash
run: |
set -eo pipefail
npm run test:coverage 2>&1 | tee /tmp/coverage-test-${{ github.run_id }}.log
working-directory: frontend
env:
TZ: Europe/Berlin
# Diagnostic guard: covers the coverage run only. If `npm test` (above)
# exits 1 with a birpc error, the named pattern appears here — not there.
- name: Assert no birpc teardown race in coverage run
shell: bash
if: always()
run: |
if grep -qF "[birpc] rpc is closed" /tmp/coverage-test-${{ github.run_id }}.log 2>/dev/null; then
echo "FAIL: [birpc] rpc is closed teardown race detected in coverage run"
grep -F "[birpc] rpc is closed" /tmp/coverage-test-${{ github.run_id }}.log
exit 1
fi
- name: Upload coverage reports
if: always()
uses: actions/upload-artifact@v4
with:
name: coverage-reports
path: frontend/coverage/
path: |
frontend/coverage/
/tmp/coverage-test-${{ github.run_id }}.log
- name: Build frontend
run: npm run build

View File

@@ -0,0 +1,137 @@
package org.raddatz.familienarchiv.security;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequestWrapper;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.core.annotation.Order;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Component;
import org.springframework.web.filter.OncePerRequestFilter;
import java.io.IOException;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Enumeration;
/**
* Promotes the {@code auth_token} cookie to an {@code Authorization} header
* so that browser-side requests to {@code /api/*} authenticate the same way
* SSR fetches do.
*
* <p>The SvelteKit login action stores the full HTTP Basic header value
* ({@code "Basic <base64>"}) in an HttpOnly cookie. SSR fetches from
* {@code hooks.server.ts} read the cookie and pass it explicitly as the
* {@code Authorization} header. In the dev environment, Vite's proxy does
* the same on every {@code /api/*} request (see {@code vite.config.ts}).
* In production, Caddy proxies {@code /api/*} straight to the backend and
* does NOT translate the cookie — so client-side {@code fetch} and
* {@code EventSource} calls reach the backend without auth, get
* {@code 401 WWW-Authenticate: Basic}, and the browser pops a native dialog.
*
* <p>This filter closes that gap: if a request has an {@code auth_token}
* cookie but no explicit {@code Authorization} header, promote the cookie
* value (URL-decoded) into the header before Spring Security inspects it.
* Explicit {@code Authorization} headers are preserved unchanged.
*
* <p>See #520. Filter runs at {@code Ordered.HIGHEST_PRECEDENCE} so it
* mutates the request before any Spring Security filter sees it.
*
* <p><b>Scope:</b> only {@code /api/*} requests are touched. The
* {@code /actuator/*} block in Caddy plus the open auth/reset paths in
* {@link SecurityConfig} must NOT receive a promoted Authorization.
*
* <p><b>⚠ Log-leakage warning:</b> the wrapped request exposes the
* Authorization header via {@code getHeaderNames}/{@code getHeaders}. Any
* filter or interceptor that iterates request headers will see the live
* Basic credential. Do NOT add a request-header logger downstream of this
* filter without explicitly scrubbing the {@code Authorization} field.
*/
@Component
@Order(org.springframework.core.Ordered.HIGHEST_PRECEDENCE)
public class AuthTokenCookieFilter extends OncePerRequestFilter {
static final String COOKIE_NAME = "auth_token";
static final String SCOPE_PREFIX = "/api/";
@Override
protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse response,
FilterChain chain) throws ServletException, IOException {
// Scope: only /api/* needs cookie promotion. /actuator/health (open),
// /api/auth/forgot-password (open), /login etc. don't.
if (!request.getRequestURI().startsWith(SCOPE_PREFIX)) {
chain.doFilter(request, response);
return;
}
// An explicit Authorization header wins — this is the SSR fetch path
// (hooks.server.ts builds the header itself).
if (request.getHeader(HttpHeaders.AUTHORIZATION) != null) {
chain.doFilter(request, response);
return;
}
Cookie[] cookies = request.getCookies();
if (cookies == null) {
chain.doFilter(request, response);
return;
}
for (Cookie c : cookies) {
if (COOKIE_NAME.equals(c.getName()) && c.getValue() != null && !c.getValue().isBlank()) {
String decoded;
try {
decoded = URLDecoder.decode(c.getValue(), StandardCharsets.UTF_8);
} catch (IllegalArgumentException malformed) {
// Malformed percent-encoding — refuse to forward a bogus
// Authorization header. Spring Security will treat the
// request as unauthenticated.
chain.doFilter(request, response);
return;
}
chain.doFilter(new AuthHeaderRequest(request, decoded), response);
return;
}
}
chain.doFilter(request, response);
}
/**
* Adds (or overrides) the {@code Authorization} header on a wrapped request.
* All other headers pass through unchanged.
*/
static final class AuthHeaderRequest extends HttpServletRequestWrapper {
private final String authorization;
AuthHeaderRequest(HttpServletRequest request, String authorization) {
super(request);
this.authorization = authorization;
}
@Override
public String getHeader(String name) {
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(name)) {
return authorization;
}
return super.getHeader(name);
}
@Override
public Enumeration<String> getHeaders(String name) {
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(name)) {
return Collections.enumeration(Collections.singletonList(authorization));
}
return super.getHeaders(name);
}
@Override
public Enumeration<String> getHeaderNames() {
Enumeration<String> base = super.getHeaderNames();
java.util.Set<String> names = new java.util.LinkedHashSet<>();
while (base.hasMoreElements()) names.add(base.nextElement());
names.add(HttpHeaders.AUTHORIZATION);
return Collections.enumeration(names);
}
}
}

View File

@@ -37,12 +37,20 @@ public class SecurityConfig {
@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
http
// CSRF is intentionally disabled: every request from the SvelteKit frontend
// carries an explicit Authorization header (Basic Auth token injected by
// hooks.server.ts). Browsers block cross-origin requests from setting custom
// headers, so cross-site request forgery via a third-party page is not
// possible with this auth scheme. If the auth model ever changes to
// cookie-based sessions, CSRF protection must be re-enabled.
// CSRF is intentionally disabled. With the cookie-promotion model
// (auth_token cookie → Authorization header via AuthTokenCookieFilter,
// see #520), every authenticated request to /api/* now carries the
// credential automatically once the cookie is set. The CSRF defence
// for state-changing endpoints is therefore LOAD-BEARING on:
//
// 1. SameSite=strict on the auth_token cookie (login/+page.server.ts).
// A cross-site POST from evil.com cannot include the cookie.
// 2. CORS — Spring's default rejects cross-origin requests with
// credentials unless explicitly allowed (no allowedOrigins config).
//
// If either of those is ever weakened (e.g. cookie flipped to
// SameSite=lax, CORS allowedOrigins expanded), CSRF protection
// MUST be re-enabled here.
.csrf(csrf -> csrf.disable())
.authorizeHttpRequests(auth -> {

View File

@@ -20,6 +20,7 @@ import org.springframework.boot.CommandLineRunner;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.core.env.Environment;
import org.springframework.security.crypto.password.PasswordEncoder;
import java.time.LocalDate;
@@ -31,26 +32,51 @@ import java.util.Set;
@DependsOn("flyway")
public class UserDataInitializer {
@Value("${app.admin.email:admin@familyarchive.local}")
static final String DEFAULT_ADMIN_EMAIL = "admin@familienarchiv.local";
static final String DEFAULT_ADMIN_PASSWORD = "admin123";
@Value("${app.admin.email:" + DEFAULT_ADMIN_EMAIL + "}")
private String adminEmail;
@Value("${app.admin.password:admin123}")
@Value("${app.admin.password:" + DEFAULT_ADMIN_PASSWORD + "}")
private String adminPassword;
private final AppUserRepository userRepository;
private final UserGroupRepository groupRepository;
private final Environment environment;
@Bean
public CommandLineRunner initAdminUser(PasswordEncoder passwordEncoder) {
return args -> {
if (userRepository.findByEmail(adminEmail).isEmpty()) {
// Fail-closed in production: refuse to seed with the well-known
// defaults. Otherwise an operator who forgets APP_ADMIN_USERNAME
// / APP_ADMIN_PASSWORD locks production to admin@…/admin123 PERMANENTLY
// (UserDataInitializer only seeds when the row is missing — see #513).
// Allowed in dev/test/e2e because those run without secrets configured.
boolean isLocalProfile = environment.matchesProfiles("dev", "test", "e2e");
if (!isLocalProfile
&& (DEFAULT_ADMIN_EMAIL.equals(adminEmail)
|| DEFAULT_ADMIN_PASSWORD.equals(adminPassword))) {
throw new IllegalStateException(
"Refusing to seed admin user with default credentials outside "
+ "the dev/test/e2e profiles. Set APP_ADMIN_USERNAME and "
+ "APP_ADMIN_PASSWORD to non-default values before first boot — "
+ "this lock-in is permanent."
);
}
log.info("Kein Admin-User '{}' gefunden. Erstelle Default-Admin...", adminEmail);
UserGroup adminGroup = UserGroup.builder()
.name("Administrators")
.permissions(Set.of("ADMIN", "READ_ALL", "WRITE_ALL", "ANNOTATE_ALL", "ADMIN_USER", "ADMIN_TAG", "ADMIN_PERMISSION"))
.build();
groupRepository.save(adminGroup);
// Reuse the Administrators group if it already exists (e.g. a
// previous boot seeded the group but failed before creating
// the admin user, or the operator deleted just the user row
// to retry the seed with a new email). Blind-INSERTing would
// violate user_groups_name_key and abort the context. See #518.
UserGroup adminGroup = groupRepository.findByName("Administrators")
.orElseGet(() -> groupRepository.save(UserGroup.builder()
.name("Administrators")
.permissions(Set.of("ADMIN", "READ_ALL", "WRITE_ALL", "ANNOTATE_ALL", "ADMIN_USER", "ADMIN_TAG", "ADMIN_PERMISSION"))
.build()));
AppUser admin = AppUser.builder()
.email(adminEmail)

View File

@@ -69,7 +69,11 @@ app:
from: ${APP_MAIL_FROM:noreply@familienarchiv.local}
admin:
username: ${APP_ADMIN_USERNAME:admin}
# Key must be `email`, not `username` — UserDataInitializer reads
# `${app.admin.email:...}`. The env-var name stays APP_ADMIN_USERNAME
# to match the existing Gitea secrets and DEPLOYMENT.md §3.3.
# See #513.
email: ${APP_ADMIN_USERNAME:admin@familienarchiv.local}
password: ${APP_ADMIN_PASSWORD:admin123}
import:

View File

@@ -10,6 +10,7 @@ import org.raddatz.familienarchiv.document.DocumentStatus;
import org.raddatz.familienarchiv.document.DocumentRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.context.annotation.Import;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
@@ -41,6 +42,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* test pyramid mocks at the FileService boundary.
*/
@SpringBootTest
@ActiveProfiles("test")
@Import(PostgresContainerConfig.class)
class ThumbnailServiceIntegrationTest {

View File

@@ -0,0 +1,134 @@
package org.raddatz.familienarchiv.security;
import jakarta.servlet.FilterChain;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
/**
* The filter must turn a browser-side {@code Cookie: auth_token=Basic%20<base64>}
* into {@code Authorization: Basic <base64>} (URL-decoded) so that Spring's
* Basic-auth filter accepts it. Skips when the request already has an explicit
* {@code Authorization} header, or when no {@code auth_token} cookie is present.
*
* <p>See #520.
*/
class AuthTokenCookieFilterTest {
private final AuthTokenCookieFilter filter = new AuthTokenCookieFilter();
@Test
void promotes_url_encoded_auth_token_cookie_to_decoded_Authorization_header() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setRequestURI("/api/users/me");
req.setCookies(new Cookie("auth_token", "Basic%20YWRtaW5AZmFtaWx5YXJjaGl2ZS5sb2NhbDpzZWNyZXQ%3D"));
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);
filter.doFilter(req, res, chain);
ArgumentCaptor<HttpServletRequest> captor = ArgumentCaptor.forClass(HttpServletRequest.class);
verify(chain, times(1)).doFilter(captor.capture(), org.mockito.ArgumentMatchers.any(HttpServletResponse.class));
HttpServletRequest forwarded = captor.getValue();
assertThat(forwarded.getHeader("Authorization"))
.as("Authorization must be URL-decoded so Spring's Basic parser sees a literal space")
.isEqualTo("Basic YWRtaW5AZmFtaWx5YXJjaGl2ZS5sb2NhbDpzZWNyZXQ=");
}
@Test
void preserves_explicit_Authorization_header_and_ignores_cookie() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setRequestURI("/api/users/me");
req.addHeader("Authorization", "Basic explicit-header-wins");
req.setCookies(new Cookie("auth_token", "Basic%20cookie-would-have-promoted"));
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);
filter.doFilter(req, res, chain);
// Forwards the original request unchanged — same instance, no wrapping.
verify(chain).doFilter(req, res);
}
@Test
void passes_through_when_no_cookies_at_all() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setRequestURI("/api/users/me");
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);
filter.doFilter(req, res, chain);
verify(chain).doFilter(req, res);
}
@Test
void passes_through_when_auth_token_cookie_is_absent() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setRequestURI("/api/users/me");
req.setCookies(new Cookie("some_other_cookie", "value"));
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);
filter.doFilter(req, res, chain);
verify(chain).doFilter(req, res);
}
@Test
void passes_through_when_auth_token_cookie_is_empty() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setRequestURI("/api/users/me");
req.setCookies(new Cookie("auth_token", ""));
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);
filter.doFilter(req, res, chain);
verify(chain).doFilter(req, res);
}
@Test
void passes_through_unchanged_when_request_is_outside_api_scope() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
// /actuator/health and similar must NOT receive a promoted Authorization
// header — they have their own access rules and should never be authed
// via the cookie.
req.setRequestURI("/actuator/health");
req.setCookies(new Cookie("auth_token", "Basic%20YWR=="));
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);
filter.doFilter(req, res, chain);
// Forwards the original request unchanged — same instance, no wrapping.
verify(chain).doFilter(req, res);
}
@Test
void passes_through_unchanged_when_cookie_value_is_malformed_percent_encoding() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setRequestURI("/api/users/me");
// Lone "%" without two hex digits → URLDecoder throws → filter must
// refuse to forward a bogus Authorization header.
req.setCookies(new Cookie("auth_token", "Basic%2"));
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);
filter.doFilter(req, res, chain);
// Forwards the original request unchanged — Spring Security treats it
// as unauthenticated rather than crashing on bad input.
verify(chain).doFilter(req, res);
}
}

View File

@@ -0,0 +1,174 @@
package org.raddatz.familienarchiv.user;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.boot.CommandLineRunner;
import org.springframework.core.env.Environment;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.test.util.ReflectionTestUtils;
import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
/**
* UserDataInitializer must refuse to seed the admin user with the hardcoded
* dev defaults when running outside the {@code dev} profile.
*
* <p>Why this matters: per DEPLOYMENT.md §3.5 and ADR-011, the admin password
* is permanently locked on first deploy (UserDataInitializer only seeds when
* the row is missing). If an operator forgets to set {@code APP_ADMIN_USERNAME}
* / {@code APP_ADMIN_PASSWORD}, prod silently boots with the well-known dev
* defaults — a credential-disclosure foot-gun, not a config typo. See #513.
*/
@ExtendWith(MockitoExtension.class)
class AdminSeedFailClosedTest {
@Mock AppUserRepository userRepository;
@Mock UserGroupRepository groupRepository;
@Mock Environment environment;
@Mock PasswordEncoder passwordEncoder;
UserDataInitializer initializer;
@BeforeEach
void setUp() {
initializer = new UserDataInitializer(userRepository, groupRepository, environment);
}
@Test
void refuses_to_seed_when_email_is_default_and_profile_is_not_dev() throws Exception {
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
ReflectionTestUtils.setField(initializer, "adminEmail", UserDataInitializer.DEFAULT_ADMIN_EMAIL);
ReflectionTestUtils.setField(initializer, "adminPassword", "operator-set-this-one");
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
assertThatThrownBy(() -> runner.run())
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("default credentials")
.hasMessageContaining("permanent");
verify(userRepository, never()).save(org.mockito.ArgumentMatchers.any());
}
@Test
void refuses_to_seed_when_password_is_default_and_profile_is_not_dev() throws Exception {
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
ReflectionTestUtils.setField(initializer, "adminEmail", "admin@archiv.raddatz.cloud");
ReflectionTestUtils.setField(initializer, "adminPassword", UserDataInitializer.DEFAULT_ADMIN_PASSWORD);
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
assertThatThrownBy(() -> runner.run())
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("default credentials");
}
@Test
void allows_seed_when_both_values_are_set_and_profile_is_not_dev() throws Exception {
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
when(groupRepository.findByName("Administrators")).thenReturn(Optional.empty());
when(groupRepository.save(any(UserGroup.class))).thenAnswer(inv -> inv.getArgument(0));
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
when(passwordEncoder.encode(anyString())).thenReturn("$2a$10$stub");
ReflectionTestUtils.setField(initializer, "adminEmail", "admin@archiv.raddatz.cloud");
ReflectionTestUtils.setField(initializer, "adminPassword", "a-real-strong-password");
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
runner.run();
verify(userRepository).save(any(AppUser.class));
}
@Test
void allows_seed_with_defaults_when_profile_is_dev() throws Exception {
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
when(groupRepository.findByName("Administrators")).thenReturn(Optional.empty());
when(groupRepository.save(any(UserGroup.class))).thenAnswer(inv -> inv.getArgument(0));
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(true);
when(passwordEncoder.encode(anyString())).thenReturn("$2a$10$stub");
ReflectionTestUtils.setField(initializer, "adminEmail", UserDataInitializer.DEFAULT_ADMIN_EMAIL);
ReflectionTestUtils.setField(initializer, "adminPassword", UserDataInitializer.DEFAULT_ADMIN_PASSWORD);
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
runner.run();
verify(userRepository).save(any(AppUser.class));
}
@Test
void does_not_check_defaults_when_admin_already_exists() throws Exception {
AppUser existing = AppUser.builder()
.email("someone@example.com")
.password("$2a$10$stub")
.build();
when(userRepository.findByEmail(anyString())).thenReturn(Optional.of(existing));
ReflectionTestUtils.setField(initializer, "adminEmail", UserDataInitializer.DEFAULT_ADMIN_EMAIL);
ReflectionTestUtils.setField(initializer, "adminPassword", UserDataInitializer.DEFAULT_ADMIN_PASSWORD);
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
runner.run();
verify(userRepository, never()).save(org.mockito.ArgumentMatchers.any());
// Importantly, no IllegalStateException — re-deploys must not panic over
// historical default-seeded data they cannot retroactively fix.
}
@Test
void reuses_existing_Administrators_group_when_seeding_a_new_admin() throws Exception {
// Setup: admin user does not exist, but the Administrators group does
// (e.g. previous boot seeded the group then failed; operator deleted
// the bad user row to retry with a corrected APP_ADMIN_USERNAME). The
// re-seed must reuse the group, not blind-INSERT a duplicate. See #518.
UserGroup existingGroup = UserGroup.builder()
.name("Administrators")
.build();
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
when(groupRepository.findByName("Administrators")).thenReturn(Optional.of(existingGroup));
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
when(passwordEncoder.encode(anyString())).thenReturn("$2a$10$stub");
ReflectionTestUtils.setField(initializer, "adminEmail", "admin@archiv.raddatz.cloud");
ReflectionTestUtils.setField(initializer, "adminPassword", "a-real-strong-password");
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
runner.run();
// Group must not be re-inserted — that would violate user_groups_name_key.
verify(groupRepository, never()).save(any(UserGroup.class));
// But the admin user IS created, with the existing group attached.
org.mockito.ArgumentCaptor<AppUser> captor = org.mockito.ArgumentCaptor.forClass(AppUser.class);
verify(userRepository).save(captor.capture());
assertThat(captor.getValue().getGroups()).containsExactly(existingGroup);
}
@Test
void creates_Administrators_group_when_seeding_admin_on_a_fresh_database() throws Exception {
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
when(groupRepository.findByName("Administrators")).thenReturn(Optional.empty());
when(groupRepository.save(any(UserGroup.class))).thenAnswer(inv -> inv.getArgument(0));
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
when(passwordEncoder.encode(anyString())).thenReturn("$2a$10$stub");
ReflectionTestUtils.setField(initializer, "adminEmail", "admin@archiv.raddatz.cloud");
ReflectionTestUtils.setField(initializer, "adminPassword", "a-real-strong-password");
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
runner.run();
// Group should be inserted exactly once.
verify(groupRepository).save(any(UserGroup.class));
verify(userRepository).save(any(AppUser.class));
}
}

View File

@@ -0,0 +1,95 @@
package org.raddatz.familienarchiv.user;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.beans.factory.config.YamlPropertiesFactoryBean;
import org.springframework.boot.context.properties.bind.Binder;
import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.core.env.PropertiesPropertySource;
import org.springframework.core.io.ClassPathResource;
import java.lang.reflect.Field;
import java.util.Properties;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Pins the admin-seed property key contract. {@code UserDataInitializer} reads
* {@code @Value("${app.admin.email:...}")} and {@code @Value("${app.admin.password:...}")}.
* The yaml MUST expose those exact keys, not e.g. {@code app.admin.username}, or
* the env vars {@code APP_ADMIN_USERNAME} / {@code APP_ADMIN_PASSWORD} are
* silently ignored and the admin user gets seeded with the hardcoded defaults.
*
* <p>Discovered as a HIGH bug during the production-deploy bootstrap (#513): on
* first deploy the prod admin password is permanently locked to whatever ends
* up in the database, so a key-name mismatch would lock prod to the dev defaults
* {@code admin@familyarchive.local} / {@code admin123}.
*
* <p>No Spring context — Binder reads application.yaml directly.
*/
class AdminSeedPropertyKeyTest {
@Test
void admin_email_key_binds_from_yaml() {
Binder binder = binderFromApplicationYaml();
String email = binder.bind("app.admin.email", String.class)
.orElseThrow(() -> new AssertionError(
"app.admin.email is missing from application.yaml. "
+ "UserDataInitializer reads this exact key; if the yaml uses "
+ "a different name (e.g. 'username'), the env var "
+ "APP_ADMIN_USERNAME is silently ignored."));
assertThat(email)
.as("app.admin.email must resolve from APP_ADMIN_USERNAME or its default")
.isNotBlank();
}
@Test
void admin_password_key_binds_from_yaml() {
Binder binder = binderFromApplicationYaml();
String password = binder.bind("app.admin.password", String.class)
.orElseThrow(() -> new AssertionError(
"app.admin.password is missing from application.yaml. "
+ "UserDataInitializer reads this exact key."));
assertThat(password)
.as("app.admin.password must resolve from APP_ADMIN_PASSWORD or its default")
.isNotBlank();
}
@Test
void userDataInitializer_reads_app_admin_email_not_username() throws NoSuchFieldException {
// Pin the Java side too: a future rename of the @Value placeholder
// (e.g. back to `${app.admin.username:...}`) would silently break the
// binding while the yaml-side assertions above still pass. See #513.
Field field = UserDataInitializer.class.getDeclaredField("adminEmail");
Value annotation = field.getAnnotation(Value.class);
assertThat(annotation)
.as("UserDataInitializer.adminEmail must be @Value-annotated")
.isNotNull();
assertThat(annotation.value())
.as("UserDataInitializer must read app.admin.email — not username or any other key")
.startsWith("${app.admin.email:");
}
@Test
void userDataInitializer_reads_app_admin_password() throws NoSuchFieldException {
Field field = UserDataInitializer.class.getDeclaredField("adminPassword");
Value annotation = field.getAnnotation(Value.class);
assertThat(annotation).isNotNull();
assertThat(annotation.value())
.as("UserDataInitializer must read app.admin.password")
.startsWith("${app.admin.password:");
}
private Binder binderFromApplicationYaml() {
YamlPropertiesFactoryBean yaml = new YamlPropertiesFactoryBean();
yaml.setResources(new ClassPathResource("application.yaml"));
Properties props = yaml.getObject();
assertThat(props).as("application.yaml must be on the classpath").isNotNull();
return new Binder(ConfigurationPropertySources.from(
new PropertiesPropertySource("application", props)));
}
}

View File

@@ -224,7 +224,7 @@ services:
networks:
- archiv-net
healthcheck:
test: ["CMD-SHELL", "wget -qO- http://localhost:3000/login >/dev/null 2>&1 || exit 1"]
test: ["CMD-SHELL", "wget -qO- http://127.0.0.1:3000/login >/dev/null 2>&1 || exit 1"]
interval: 15s
timeout: 5s
retries: 10

View File

@@ -0,0 +1,90 @@
# ADR 012 — Browser-Mode Test Mocking Strategy
**Status:** Accepted
**Date:** 2026-05-11
**Issue:** [#535 — Unit & Component Tests job exits 1 from vitest-browser teardown race](https://git.raddatz.cloud/marcel/familienarchiv/issues/535)
---
## Context
Vitest browser-mode tests (the `client` project, run with `@vitest/browser-playwright` / Chromium) use a different module resolution path than Node-environment tests. When a spec calls `vi.mock('some-module', factory)`, vitest registers a `ManualMockedModule`. At runtime, every time Chromium requests that module, a playwright route handler intercepts the request and calls the Node worker over **birpc** (`resolveManualMock`) to evaluate the factory and return the module body.
This is safe for modules that are imported **statically** at spec module-eval time (e.g. `$app/navigation`, `$env/static/public`): those requests resolve before the first test runs and well before any teardown occurs.
It is **unsafe** for modules that are imported **dynamically** (e.g. inside an `async onMount`, inside a lazy-loaded chunk): Chromium may fetch the module after the worker's birpc channel has already closed, producing:
```
Error: [birpc] rpc is closed, cannot call "resolveManualMock"
ManualMockedModule.factory node_modules/@vitest/browser/dist/index.js:3221:34
```
This raises an unhandled rejection that exits the vitest process with code 1, even though every test in the run reported green.
`pdfjs-dist` and `pdfjs-dist/build/pdf.worker.min.mjs?url` are loaded via `await Promise.all([import('pdfjs-dist'), import('pdfjs-dist/build/pdf.worker.min.mjs?url')])` inside `usePdfRenderer.svelte.ts::init()`, which is called from `onMount`. These dynamic imports triggered the race.
---
## Decision
**Prefer prop injection over `vi.mock(module, factory)` for any module that is loaded dynamically in browser-mode specs.**
### The libLoader pattern (for external rendering libraries)
When a component depends on a large external library loaded via dynamic import, extract the import into an injectable loader function with a production default:
```typescript
// usePdfRenderer.svelte.ts
type LibLoader = () => Promise<readonly [typeof import('pdfjs-dist'), { default: string }]>;
const defaultLibLoader: LibLoader = () =>
Promise.all([import('pdfjs-dist'), import('pdfjs-dist/build/pdf.worker.min.mjs?url')]);
export function createPdfRenderer(libLoader: LibLoader = defaultLibLoader) { ... }
```
The component threads the loader as an optional prop:
```svelte
<!-- PdfViewer.svelte -->
let { url, ..., libLoader = undefined } = $props();
const renderer = untrack(() => createPdfRenderer(libLoader));
```
Tests supply a synchronous fake — no `vi.mock` needed:
```typescript
const fakePdfjs = { GlobalWorkerOptions: ..., getDocument: vi.fn(), TextLayer: class {} };
const fakeLoader = vi.fn().mockResolvedValue([fakePdfjs, { default: '' }] as const);
render(PdfViewer, { url: '...', libLoader: fakeLoader });
```
### The test-host pattern (for component behaviour)
For components that fetch data or call services, the `*.test-host.svelte` pattern threads the dependency as a prop rather than mocking the module. See `PersonMentionEditor.test-host.svelte` for the canonical example.
---
## Residual exceptions
The following `vi.mock(module, factory)` calls in browser specs are **acceptable** because the mocked modules are loaded statically at spec module-eval time and cannot produce a teardown race:
| Module | Why it stays as vi.mock |
|--------|------------------------|
| `$app/navigation` | SvelteKit virtual module — no DI seam |
| `$app/forms` | SvelteKit virtual module — no DI seam |
| `$app/state` | SvelteKit virtual module — no DI seam |
| `$app/stores` | SvelteKit virtual module — no DI seam |
| `$env/static/public` | Vite env virtual module — no DI seam |
These modules are resolved at static import time (before any test runs). Their `vi.mock` factories are served by birpc synchronously during module graph resolution, not after worker teardown.
---
## Consequences
- New browser-mode specs that need to stub an external library **must not** use `vi.mock(externalLib, factory)`. Add a loader/factory parameter to the underlying hook or service instead.
- The CI `unit-tests` job includes a permanent grep guard that fails the build if `rpc is closed` appears in any coverage run log. This catches regressions before they reach the acceptance criterion.
- Acceptance criterion for #535: 60 consecutive green `workflow_dispatch` CI runs against `main` after the fix is merged, with zero `rpc is closed` lines in any log.
- **Enforcement:** No automated lint rule is planned; the CI coverage guard is the regression backstop. If a lint rule is added later (e.g. an ESLint rule flagging `vi.mock` of non-virtual modules in browser-mode spec files), update this ADR.
- **When to revisit the LibLoader home:** If three or more components adopt this pattern, consider extracting a shared `$lib/types/lib-loader.ts` or a generic `DynamicImportLoader<T>` type to avoid parallel type definitions across modules.

View File

@@ -1,6 +1,6 @@
<script lang="ts">
import { onMount, setContext } from 'svelte';
import { createPdfRenderer } from '$lib/document/viewer/usePdfRenderer.svelte';
import { onMount, setContext, untrack } from 'svelte';
import { createPdfRenderer, type LibLoader } from '$lib/document/viewer/usePdfRenderer.svelte';
import PdfControls from './PdfControls.svelte';
import AnnotationLayer from '$lib/document/annotation/AnnotationLayer.svelte';
import type { Annotation } from '$lib/shared/types';
@@ -21,7 +21,8 @@ let {
onDeleteAnnotationRequest,
documentFileHash,
annotationsDimmed = false,
flashAnnotationId = null
flashAnnotationId = null,
libLoader = undefined
}: {
url: string;
documentId?: string;
@@ -35,9 +36,11 @@ let {
documentFileHash?: string | null;
annotationsDimmed?: boolean;
flashAnnotationId?: string | null;
libLoader?: LibLoader;
} = $props();
const renderer = createPdfRenderer();
// untrack: libLoader prop change must not reinitialise the renderer
const renderer = untrack(() => createPdfRenderer(libLoader));
// Canvas and text layer container refs — bound via bind:this
let canvasEl = $state<HTMLCanvasElement | null>(null);

View File

@@ -1,14 +1,18 @@
import { vi, describe, it, expect, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte';
import { page } from 'vitest/browser';
import type { LibLoader } from '$lib/document/viewer/usePdfRenderer.svelte';
import PdfViewer from './PdfViewer.svelte';
// pdfjs-dist is a rendering dependency — we mock it so unit tests don't need
// a real browser PDF engine. The interesting behaviour under test here is the
// component's own UI logic (controls, page counter), not pdfjs internals.
vi.mock('pdfjs-dist', () => {
function TextLayerMock() {}
TextLayerMock.prototype.render = () => Promise.resolve();
TextLayerMock.prototype.cancel = () => {};
afterEach(cleanup);
function makeFakePdfjsLib() {
class TextLayerMock {
render() {
return Promise.resolve();
}
cancel() {}
}
return {
GlobalWorkerOptions: { workerSrc: '' },
@@ -23,31 +27,30 @@ vi.mock('pdfjs-dist', () => {
})
}),
TextLayer: TextLayerMock
};
});
} as unknown as typeof import('pdfjs-dist');
}
vi.mock('pdfjs-dist/build/pdf.worker.min.mjs?url', () => ({ default: '' }));
import PdfViewer from './PdfViewer.svelte';
afterEach(cleanup);
function makeFakeLibLoader(): LibLoader {
const fakePdfjs = makeFakePdfjsLib();
return vi.fn().mockResolvedValue([fakePdfjs, { default: '' }] as const);
}
describe('PdfViewer', () => {
it('shows previous and next page navigation buttons', async () => {
render(PdfViewer, { url: '/api/documents/test-id/file' });
render(PdfViewer, { url: '/api/documents/test-id/file', libLoader: makeFakeLibLoader() });
await expect.element(page.getByRole('button', { name: /zurück/i })).toBeInTheDocument();
await expect.element(page.getByRole('button', { name: /weiter/i })).toBeInTheDocument();
});
it('shows zoom controls', async () => {
render(PdfViewer, { url: '/api/documents/test-id/file' });
render(PdfViewer, { url: '/api/documents/test-id/file', libLoader: makeFakeLibLoader() });
await expect.element(page.getByRole('button', { name: /vergrößern/i })).toBeInTheDocument();
await expect.element(page.getByRole('button', { name: /verkleinern/i })).toBeInTheDocument();
});
it('displays the page counter once the PDF has loaded', async () => {
render(PdfViewer, { url: '/api/documents/test-id/file' });
// Mock resolves synchronously, so "1 / 2" should appear quickly
render(PdfViewer, { url: '/api/documents/test-id/file', libLoader: makeFakeLibLoader() });
// Fake loader resolves synchronously, so "1 / 2" should appear quickly
await expect.element(page.getByText(/1\s*\/\s*2/)).toBeInTheDocument();
});
});

View File

@@ -1,4 +1,4 @@
import { describe, it, expect } from 'vitest';
import { describe, it, expect, vi } from 'vitest';
import { createPdfRenderer } from './usePdfRenderer.svelte';
// Note: init() and loadDocument() require pdfjsLib (browser module).
@@ -66,4 +66,37 @@ describe('createPdfRenderer', () => {
expect(r.error).toBeNull();
expect(r.loading).toBe(false);
});
it('calls injected libLoader during init and sets pdfjsReady', async () => {
const fakePdfjs = {
GlobalWorkerOptions: { workerSrc: '' },
getDocument: vi.fn(),
TextLayer: class {}
} as unknown as typeof import('pdfjs-dist');
const fakeLoader = vi.fn().mockResolvedValue([fakePdfjs, { default: '' }] as const);
const r = createPdfRenderer(fakeLoader);
await r.init();
expect(fakeLoader).toHaveBeenCalledOnce();
expect(r.pdfjsReady).toBe(true);
});
it('leaves pdfjsReady false when libLoader rejects', async () => {
const failingLoader = vi.fn().mockRejectedValue(new Error('load failed'));
const r = createPdfRenderer(failingLoader);
await expect(r.init()).rejects.toThrow('load failed');
expect(r.pdfjsReady).toBe(false);
});
it('init() is idempotent — libLoader called only once on repeated calls', async () => {
const fakePdfjs = {
GlobalWorkerOptions: { workerSrc: '' },
getDocument: vi.fn(),
TextLayer: class {}
} as unknown as typeof import('pdfjs-dist');
const fakeLoader = vi.fn().mockResolvedValue([fakePdfjs, { default: '' }] as const);
const r = createPdfRenderer(fakeLoader);
await r.init();
await r.init();
expect(fakeLoader).toHaveBeenCalledOnce();
});
});

View File

@@ -1,6 +1,11 @@
import type { PDFDocumentProxy, RenderTask } from 'pdfjs-dist';
export function createPdfRenderer() {
export type LibLoader = () => Promise<readonly [typeof import('pdfjs-dist'), { default: string }]>;
const defaultLibLoader: LibLoader = () =>
Promise.all([import('pdfjs-dist'), import('pdfjs-dist/build/pdf.worker.min.mjs?url')]);
export function createPdfRenderer(libLoader: LibLoader = defaultLibLoader) {
// Reactive state — exposed via getters
let currentPage = $state(1);
let totalPages = $state(0);
@@ -18,10 +23,8 @@ export function createPdfRenderer() {
let pdfjsLib: typeof import('pdfjs-dist') | null = null;
async function init(): Promise<void> {
const [lib, { default: workerUrl }] = await Promise.all([
import('pdfjs-dist'),
import('pdfjs-dist/build/pdf.worker.min.mjs?url')
]);
if (pdfjsReady) return;
const [lib, { default: workerUrl }] = await libLoader();
lib.GlobalWorkerOptions.workerSrc = workerUrl;
pdfjsLib = lib;
pdfjsReady = true;

View File

@@ -8,7 +8,7 @@ export const load: PageServerLoad = ({ url }) => {
};
export const actions = {
login: async ({ request, cookies, fetch }) => {
login: async ({ request, cookies, fetch, url }) => {
const data = await request.formData();
const email = data.get('email') as string;
const password = data.get('password') as string;
@@ -37,11 +37,17 @@ export const actions = {
return fail(500, { error: getErrorMessage('INTERNAL_ERROR') });
}
// The cookie IS the API credential — promoted to `Authorization: Basic …`
// on every browser → backend request by AuthTokenCookieFilter on the
// Spring side (see #520). It must be Secure on HTTPS or it leaks
// a 24h Basic token on plaintext networks. Dev runs over HTTP and
// would silently lose the cookie if we hardcoded secure=true.
const isHttps = url.protocol === 'https:';
cookies.set('auth_token', authHeader, {
path: '/',
httpOnly: true,
sameSite: 'strict',
secure: false, // set to true when HTTPS is available
secure: isHttps,
maxAge: 60 * 60 * 24
});
} catch (e) {

View File

@@ -31,8 +31,16 @@
# in application.yaml, /actuator/* is unreachable externally. The internal
# Prometheus scrape (future) talks to the backend directly on the docker
# network, not via Caddy.
@actuator path /actuator/*
respond @actuator 404
#
# Why a `handle` block and not a top-level `respond @matcher`: each archive
# vhost has a catch-all `handle { reverse_proxy ... }` that matches every
# path including /actuator/*, and Caddy's `handle` blocks are mutually
# exclusive. Without our own `handle /actuator/*` the catch-all wins, the
# request is proxied to the backend, and Spring Security 302s to /login
# instead of Caddy returning 404. See #512.
handle /actuator/* {
respond 404
}
}
(access_log) {