- Add load() unit tests for admin/users/[id] (permission gate, 404, success)
- Rename .test.ts → .spec.ts for consistency with rest of suite
- Add @Schema(requiredMode=REQUIRED) to InviteListItem.shareableUrl
- Add client-side allowlist for invite status query param
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Change importSingleDocument return type from boolean to Optional<String>
so callers in processRows receive the skip reason on every non-success path.
S3 upload failures now surface as "S3_UPLOAD_FAILED" and already-imported
documents as "ALREADY_EXISTS" in the skippedFiles list shown in the admin UI.
- Add two new tests: runImportAsync_addsS3UploadFailed_toSkippedFiles and
runImportAsync_addsAlreadyExists_toSkippedFiles; update
importSingleDocument_skips_whenDocumentAlreadyUploadedNotPlaceholder and
the S3-failure test to assert on the Optional return value.
- Add i18n keys for S3_UPLOAD_FAILED and ALREADY_EXISTS in de/en/es messages.
- Svelte ImportStatusCard: add aria-hidden="true" to SVG chevron, wrap
conditional warning section in aria-live="polite" div, add max-h-64
overflow-y-auto to skipped-files <ul> to cap height on large batches.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add comment to openFileStream() explaining package-private visibility
is intentional (Mockito spy seam for IOException test)
- Key {#each} skippedFiles by filename instead of array index
- Add test: skipped section hidden when state is FAILED
- Add test: reasonLabel returns raw code for unknown reason strings
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add @Schema(requiredMode = REQUIRED) to SkippedFile and ImportStatus
record components so TypeScript codegen produces non-optional fields
when generate:api is next run
- Extract openFileStream(File) as package-private method so the
IOException path can be tested deterministically without relying on
OS-level file permissions (which are bypassed when running as root)
- Replace assumeTrue-based IOException test with Mockito spy that stubs
openFileStream — test now runs in CI unconditionally (45 tests, 0 skipped)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- remove duplicate List import in AdminControllerTest
- derive skipped() from skippedFiles.size() — drop redundant int field
- use machine codes for SkippedFile.reason (INVALID_PDF_SIGNATURE, FILE_READ_ERROR)
- map reason codes to i18n strings in ImportStatusCard (de/en/es)
- replace raw amber Tailwind classes with warning semantic token
- fix <summary> accessibility: replace list-none with rotating chevron SVG
- replace <p> with <span> inside <summary> (phrasing content rule)
- extract setupOneValidOneFakeImport() helper — remove 3x copy-paste
- add lenient mock to short-file test for defensive coverage
- add IOException path test for isPdfMagicBytes
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reads first 4 bytes of each candidate file before upload; rejects any
file whose header does not match %PDF (0x25 0x50 0x44 0x46). Skipped
files are counted and collected in ImportStatus.skippedFiles so
operators can see what was rejected without querying Loki.
Breaking: ImportStatus record gains skipped + skippedFiles fields.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add @BatchSize(50) fallback comments on findBySenderId / findByReceiversId
- Replace silent size() discard in getRecentActivity test with assertThat isNotEmpty()
- Add ADR-022 reference comment above @JsonIgnoreProperties on Person and Tag
- Document within-open-transaction limitation in DocumentLazyLoadingTest Javadoc
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Refactor DocumentLazyLoadingTest: pull value assertions (assertThat) out
of assertThatCode lambdas so failures surface as AssertionError rather
than "unexpected exception: AssertionError" (review item 1)
- Add @EntityGraph("Document.full") to findBySenderId, findByReceiversId,
findConversation, and findSinglePersonCorrespondence — all return full
Documents to the controller for JSON serialization (review item 2)
- Add "// Callers access only ..." comments to un-graphed methods where no
lazy associations are touched: findByTags_Id, findByStatus,
findByMetadataCompleteFalse(Sort), findByMetadataCompleteFalse(Pageable)
- Remove "what" inline comments from @Transactional(readOnly=true)
on getRecentActivity and getDocumentById — the why is in ADR-022 (item 4)
- Add named-graph coupling consequence to ADR-022: Document.java and
DocumentRepository.java graph name strings must stay in sync (item 5)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Hibernate throws AnnotationException at startup when @BatchSize is placed
on a @ManyToOne field. @BatchSize is only valid on collections (@OneToMany,
@ManyToMany, @ElementCollection). The N+1 for sender is already covered by
the @EntityGraph overrides on DocumentRepository.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These annotations deviate from the project convention (read methods are
normally unannotated). The comment explains that the session must stay
open for callers to access lazy-loaded collections post-return, preventing
future developers from removing the annotation as a cleanup.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Consistent with the @BatchSize already on receivers and tags. Any lazy
code path not covered by an entity graph will batch-load these associations
instead of issuing one query per document.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
getRecentActivity calls findAll(Pageable) — the JpaRepository overload
not covered by the existing Specification variants. Without this override,
sender is loaded N+1 per document. Now applies Document.list graph so
sender and tags are fetched eagerly for every findAll(Pageable) call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- getDocumentById: add @Transactional(readOnly=true) — calls
tagService.resolveEffectiveColors(doc.getTags()) which requires an open
session after the LAZY switch
- getRecentActivity: add @Transactional(readOnly=true) — callers may access
tags/receivers on the returned list; keeps session open for @BatchSize fetches
- updateDocumentTags: add @Transactional — write method was missing annotation
Also adds @JsonIgnoreProperties({"hibernateLazyInitializer","handler"}) to
Person and Tag to prevent Jackson serialization errors on uninitialized
lazy proxies.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- receivers, tags, trainingLabels: FetchType.EAGER → FetchType.LAZY
- sender: add explicit FetchType.LAZY (was implicitly lazy, now explicit)
- @NamedEntityGraph("Document.full"): sender + receivers + tags
- @NamedEntityGraph("Document.list"): sender + tags
- DocumentRepository.findById overridden with @EntityGraph("Document.full")
- DocumentRepository.findAll(Specification, Pageable) overridden with
@EntityGraph("Document.list")
- DocumentRepository.findAll(Specification) overridden with
@EntityGraph("Document.list") for RECEIVER/SENDER sort paths
- @BatchSize(50) on receivers and tags as fallback for any list path
that does not go through an @EntityGraph method
Fixes issue #467.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- frontend/hooks.server.ts: replace request.url.includes('/api/') with
new URL(request.url).pathname.startsWith('/api/') so a page named
/my-api/something cannot accidentally match the API gate
- DomainException: add optional retryAfterSeconds field and a new
tooManyRequests() factory overload that carries the value
- LoginRateLimiter: pass windowMinutes * 60 as retryAfterSeconds when
throwing TOO_MANY_LOGIN_ATTEMPTS (RFC 6585 §4 SHOULD)
- GlobalExceptionHandler: emit Retry-After header when retryAfterSeconds
is set on a DomainException
- RateLimitInterceptor: emit Retry-After: 60 on 429 responses (1-min
window matches the existing MAX_REQUESTS_PER_MINUTE logic)
- LoginRateLimiterTest: assert retryAfterSeconds equals window duration
- RateLimitInterceptorTest: assert Retry-After header is set on 429
- JdbcSessionRevocationAdapterIntegrationTest: new @SpringBootTest +
Testcontainers test verifying revokeAll deletes all spring_session rows
and revokeOther leaves the current session intact
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Integration test:
- Adds post_without_csrf_token_returns_403_CSRF_TOKEN_MISSING to
AuthSessionIntegrationTest, verifying CSRF is active end-to-end (not just
in @WebMvcTest slices).
SessionRevocationConfig (new):
- Replaces fragile @ConditionalOnBean/@ConditionalOnMissingBean on @Service
beans with a single @Configuration @Bean method that accepts
JdbcIndexedSessionRepository as @Autowired(required=false). Spring
resolves the optional parameter reliably after auto-configuration fires,
choosing JdbcSessionRevocationAdapter when available and
NoOpSessionRevocationAdapter otherwise.
- JdbcSessionRevocationAdapter and NoOpSessionRevocationAdapter are now
plain implementation classes (no @Service/@Conditional annotations).
Addresses Sara Concern 2 from PR #617 review.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract SessionRevocationPort interface with JdbcSessionRevocationAdapter
(@ConditionalOnBean) and NoOpSessionRevocationAdapter (@ConditionalOnMissingBean).
AuthService now uses @RequiredArgsConstructor with final fields for both
LoginRateLimiter and SessionRevocationPort, removing all null guards.
AuthServiceTest drops ReflectionTestUtils.setField and uses @Mock on the port.
Fixes Felix's blocker: @Autowired(required=false) field injection in AuthService.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The circular-dependency that originally forced @AllArgsConstructor was
removed when changePassword orchestration moved into the controller.
No cycle now exists between UserController, UserService, AuthService,
or AuditService — final fields and constructor injection are safe again.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Case variants of the same address (e.g. User@EXAMPLE.COM vs user@example.com)
now share a single Bucket4j bucket, preventing a trivial bypass of per-email
limits via mixed-case submissions.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces per-invocation new ObjectMapper() in the accessDeniedHandler
lambda with a static field (avoids repeated allocation). ObjectMapper
cannot be injected in SecurityConfig because @WebMvcTest slices exclude
JacksonAutoConfiguration; the static instance is safe since the response
only serialises fixed String keys.
Also corrects the ADR cross-reference in the CSRF comment from ADR-020
(Spring Session JDBC) to ADR-022 (CSRF + session revocation).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses Felix (blocker 1): the old implementation consumed from both buckets
before checking either result, silently eroding the per-email quota when only the
per-IP limit was blocking. The fix checks ipEmail first, then IP; on IP failure it
refunds the ipEmail token so legitimate users behind a shared IP are not penalised.
Also adds two new test cases:
- different_email_from_same_ip_not_blocked_by_sibling_email_exhaustion (Sara)
- ip_exhaustion_does_not_consume_ipEmail_tokens_for_blocked_attempts (red → green)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses Nora (blocker 1) and Felix (suggestion): both revocation methods
now return 0 immediately when sessionRepository is unavailable (non-web
test contexts where JdbcHttpSessionAutoConfiguration does not fire).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LoginRateLimiter uses two Caffeine LoadingCaches of Bucket4j buckets —
one keyed on IP:email (10 attempts/15 min) and one on IP alone (20/15 min
backstop). Exceeding either throws DomainException(TOO_MANY_LOGIN_ATTEMPTS)
and emits LOGIN_RATE_LIMITED audit. Successful login invalidates both
buckets via invalidateOnSuccess. Buckets expire after windowMinutes of
inactivity (no clock advance needed — Caffeine handles eviction).
AuthService integrates it as an optional @Autowired field so non-web
test contexts still work without a Caffeine dependency.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After updating the user password during a reset flow, calls
authService.revokeAllSessions(email) to invalidate every active session
for the account — prevents an attacker with a stolen session from
retaining access after the owner resets their password.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
changePassword now calls authService.revokeOtherSessions() after the
password is updated and emits a LOGOUT audit with reason=password_change.
POST /api/users/{id}/force-logout (ADMIN_USER permission) revokes all
sessions for the target user and emits ADMIN_FORCE_LOGOUT audit. Returns
{"revokedCount": N} with 200.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Uses JdbcIndexedSessionRepository (optional field — null-safe in non-web
test contexts) to delete all sessions for a principal except the current
one (revokeOtherSessions) or all sessions unconditionally (revokeAllSessions).
Both methods return the count of deleted sessions for audit payloads.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Re-enables Spring Security's CSRF filter (was disabled with a TODO comment).
Uses CookieCsrfTokenRepository so the frontend can read the XSRF-TOKEN
cookie and send it as X-XSRF-TOKEN on state-mutating requests.
Returns CSRF_TOKEN_MISSING error code on 403 instead of generic FORBIDDEN.
Updates all WebMvcTest classes to include .with(csrf()) on POST/PUT/PATCH/
DELETE/multipart requests, and fixes integration tests to supply the
XSRF-TOKEN cookie + header directly (lazy generation in Spring Security 7).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pure-comment change: spell out that resolveClientIp's leftmost-X-Forwarded-For
strategy is safe only because Caddy strips client-supplied XFF before
forwarding. Future readers swapping the ingress have a tripwire. Addresses PR
#612 / Nora concern (XFF trust documentation).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reorder AuthSessionController.logout so HttpSession.invalidate runs before
AuthService.logout, and wrap the audit call in try/catch so an exception (e.g.
the user was deleted between login and logout, making the audit-time
findByEmail throw) cannot leave the session row alive in spring_session.
The user's intent — "log me out" — is honoured even when audit fails.
Addresses PR #612 / Nora B2.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Inject Spring Security's SessionAuthenticationStrategy
(ChangeSessionIdAuthenticationStrategy) into AuthSessionController and invoke
onAuthentication at the credential boundary. The strategy calls
HttpServletRequest.changeSessionId() to invalidate any pre-auth session ID an
attacker may have planted and mint a fresh ID before the SecurityContext is
attached. Addresses PR #612 / Nora B1.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Also switches pom.xml to spring-boot-starter-session-jdbc (Spring Boot 4.x
split the session auto-config into a separate starter; spring-session-jdbc
alone does not register JdbcSessionAutoConfiguration).
Adds SpringSessionConfig#cookieSerializer bean to configure fa_session name
and SameSite=Strict (spring.session.cookie.* properties are no longer
supported by the Boot 4.x auto-configuration layer).
Cleans up application.yaml / application-dev.yaml: removes store-type: jdbc
and the unsupported cookie.* keys.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract XxeSafeXmlParser with all 6 OWASP-recommended features
(disallow-doctype-decl, external-general-entities, external-parameter-entities,
load-external-dtd, XInclude, expandEntityReferences). Make readOds()
package-private; add failing-then-passing regression test and valid-ODS guard test.
POI 5.5.0 does not mitigate this: the vulnerable parser is a custom
DocumentBuilderFactory call in readOds(), not inside POI's internal ODS reader.
The hardening is defence-in-depth, not redundant with POI defaults.
Closes#528
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add @Order(1) managementFilterChain scoped to /actuator/** with explicit
401 entry point, blocking all non-public actuator paths without the
form-login redirect that the main chain uses for browser clients.
- Split single combined test into two focused assertions
(prometheus_endpoint_returns_200_without_credentials,
prometheus_endpoint_returns_jvm_metrics).
- Add negative regression test: actuator_metrics_requires_authentication
verifies that /actuator/metrics returns 401 without credentials.
Addresses reviewer concerns from @sara (missing negative test, split
assertions) and @nora (dedicated management security layer).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four Spring Boot 4.0-specific issues prevented /actuator/prometheus from working:
1. spring-boot-starter-micrometer-metrics missing — Spring Boot 4.0 splits
Micrometer metrics export (including the Prometheus scrape endpoint) out of
spring-boot-starter-actuator into its own starter. Added dependency.
2. management.prometheus.metrics.export.enabled not set — Spring Boot 4.0
defaults metrics export to false (opt-in). Added the property to
application.yaml.
3. SecurityConfig did not permit /actuator/prometheus — Spring Boot 4.0
with Jetty serves the management port (8081) via the same security filter
chain as the main port (8080). The previous commit's exclusion of
ManagementWebSecurityAutoConfiguration was a no-op (that class no longer
exists in Spring Boot 4.0); removed it and added the correct permitAll()
rule. Updated the architecture comment in application.yaml to reflect the
true filter-chain behaviour.
4. Reverted invalid FamilienarchivApplication.java change from the prior
commit (ManagementWebSecurityAutoConfiguration import compiled against a
class that does not exist in the Spring Boot 4.0 BOM).
Also adds ActuatorPrometheusIT — an integration test that asserts the
/actuator/prometheus endpoint returns 200 with jvm_memory_used_bytes without
credentials, serving as regression protection against future Spring Boot
upgrades silently breaking metrics collection.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three root causes confirmed via live server investigation (issue #604):
1. ManagementWebSecurityAutoConfiguration applied HTTP Basic auth to the
management port (8081), causing Prometheus to receive 401 HTML responses
instead of metrics. Excluded the auto-config — the Docker network
(archiv-net) provides the security boundary for this internal port.
2. promtail-config.yml had no `job` relabel rule. Grafana's Loki dashboards
query {job="$app"} which matched nothing; logs were in Loki under
compose_service but invisible to every dashboard panel.
3. prometheus.yml had a stale comment claiming the spring-boot target would
be DOWN until micrometer-registry-prometheus was added — it has been
present in pom.xml for some time.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sentry-spring-boot-starter-jakarta 8.5.0 does not support Spring Boot 4.0 —
it logs an "Incompatible Spring Boot Version" warning and its SentryAutoConfiguration
crashes SF7 bean-name generation. sentry-spring-boot-4 (added in 8.21.0) is the
dedicated Spring Boot 4 module with a fixed auto-configuration class.
- Replace sentry-spring-boot-starter-jakarta:8.5.0 with sentry-spring-boot-4:8.41.0
- Delete SentryConfig.java — workaround no longer needed, auto-config handles init
- Remove spring.autoconfigure.exclude from application.yaml + application-test.yaml
- Delete SentryConfigTest.java — tested the deleted workaround class
- Update ApplicationContextTest: assert Sentry.isEnabled() is false when no DSN set
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SentryAutoConfiguration$HubConfiguration$SentrySpanRestClientConfiguration is a triply-
nested @Configuration class conditionally loaded when RestClient is on the classpath
(always true on Spring Framework 7). Spring Framework 7's bean name generator fails
on such deeply-nested @Import-ed classes, crashing every @SpringBootTest context.
Replace the broken auto-configuration with a minimal SentryConfig bean that calls
Sentry.init() with the same properties (DSN, environment, sample rate, PII guard,
DomainException filter). Unexpected 5xx exceptions are forwarded to Sentry via
Sentry.captureException() in GlobalExceptionHandler.handleGeneric().
Also add management.server.port=0 to application-test.yaml to eliminate TIME_WAIT
conflicts from @DirtiesContext restarts on the fixed management port 8081 (see #593).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Spring Framework 7 prohibits constructor injection cycles. InviteService
already injects UserService, so UserService cannot inject InviteService
for the deleteGroup guard — repository injection is the correct workaround.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Client-submitted duplicate UUIDs were causing a false GROUP_NOT_FOUND:
size(deduplicated_db_result)==1 != size(submitted)==2. Deduplicate input
with HashSet before calling findGroupsByIds so the size comparison is
always against unique IDs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds GROUP_HAS_ACTIVE_INVITES error code and guards UserService.deleteGroup()
with a 409 conflict when any active (non-revoked, non-expired, non-exhausted)
invite token still holds the group UUID.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- @JsonIgnore on ImportStatus.message — stops internal directory paths and
raw exception text leaking through the admin import-status endpoint (CWE-209)
- Add importStatus_messageField_notPresentInApiResponse test (red/green verified)
- Add importStatus_returns401/403 auth boundary tests — documents and guards
the @RequirePermission(ADMIN) protection against configuration drift
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a statusCode field (IMPORT_IDLE / IMPORT_RUNNING / IMPORT_DONE /
IMPORT_FAILED_NO_SPREADSHEET / IMPORT_FAILED_INTERNAL) to ImportStatus.
The frontend will map these codes to localized strings via Paraglide
instead of rendering the backend's German message verbatim.
NoSpreadsheetException distinguishes a missing spreadsheet from other
I/O failures so the frontend can show a specific error without raw text.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The hardcoded `static final String IMPORT_DIR = "/import"` was the only
non-`@Value` configurable input in MassImportService — every column
index next to it is wired through `app.import.col.*`. Lifts the
contract from infrastructure (compose bind mount) into application
config (`app.import.dir`), with `/import` as the default so the existing
bind-mount path keeps working.
Addresses review feedback from Markus and Felix on #526.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- 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>
