From de7053644b325060736136cec61c984cc092301f Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:15:51 +0200 Subject: [PATCH 01/32] =?UTF-8?q?docs(adr):=20ADR-020=20=E2=80=94=20statef?= =?UTF-8?q?ul=20auth=20via=20Spring=20Session=20JDBC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- ...0-stateful-auth-via-spring-session-jdbc.md | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 docs/adr/020-stateful-auth-via-spring-session-jdbc.md diff --git a/docs/adr/020-stateful-auth-via-spring-session-jdbc.md b/docs/adr/020-stateful-auth-via-spring-session-jdbc.md new file mode 100644 index 00000000..78b62397 --- /dev/null +++ b/docs/adr/020-stateful-auth-via-spring-session-jdbc.md @@ -0,0 +1,94 @@ +# ADR-020 — Stateful Authentication via Spring Session JDBC + +**Date:** 2026-05-17 +**Status:** Accepted +**Issue:** #523 + +--- + +## Context + +PR #521 (closing #520) introduced `AuthTokenCookieFilter` to unblock a production deploy. +The filter promotes an `auth_token` cookie — which contains the full HTTP Basic credential +(`Basic `) — to an `Authorization` header so browser-direct `/api/*` +calls authenticate correctly behind Caddy. + +This model has three concrete problems: + +1. **Cookie = credential.** A stolen `auth_token` cookie leaks the user's password in + base64-encoded plaintext. No decode step is needed; the cookie value is directly usable + as a credential forever. +2. **No server-side revocation.** Logout deletes the local cookie but the credential + remains valid until the 24 h `Max-Age` elapses. An attacker who copied the cookie before + logout retains access. +3. **No audit signal.** There is no server-side record of login or logout events. Observability + and compliance tooling cannot reconstruct "who was logged in when". + +Additionally, Nora flagged that `url.protocol === 'https:'` in `login/+page.server.ts` is +incorrect behind Caddy: SvelteKit sees `http`, so `Secure=false` was set on the credential +cookie in production, transmitting it in cleartext from Caddy to the browser on any network +path without TLS. + +--- + +## Decision + +Replace the `auth_token` / `AuthTokenCookieFilter` model with **Spring Session JDBC**: + +- A `POST /api/auth/login` endpoint in a new `auth` package authenticates with `email + + password`, creates a server-side session record in PostgreSQL, and returns the `AppUser` + JSON in the response body. +- The response sets an **opaque** `fa_session` cookie (`HttpOnly`, `SameSite=Strict`, + `Secure` in non-dev profiles, `Max-Age=28800` — 8 h idle timeout) that contains only the + session ID, never a credential. +- A `POST /api/auth/logout` endpoint invalidates the session record immediately. Subsequent + requests carrying the same cookie return 401. +- `AuthTokenCookieFilter` is deleted in the same PR. No transitional coexistence period. +- Cookie name `fa_session` (not the default `SESSION`) minimises framework fingerprinting. + +Session storage uses the canonical `spring_session` / `spring_session_attributes` tables, +re-introduced via `V67__recreate_spring_session_tables.sql` (dropped by V2 when the +dependency was previously removed as unused). + +**Idle timeout:** 8 h (`MaxInactiveIntervalInSeconds = 28800`). No 24 h absolute cap is +implemented in Phase 1 — the 8 h idle bound contains the risk to one workday. A weekend-long +active session is acceptable given the family-archive threat model. The absolute cap and +additional revocation paths (password-change, admin force-logout) land in Phase 2 (#524). + +--- + +## Alternatives Considered + +### Stay on Basic cookie + add a server-side revocation table + +Keeps the credential-in-cookie problem. Implementing a revocation table would re-invent +Spring Session badly — we'd write bespoke session storage that already exists and is +well-tested upstream. + +### JWT (stateless) + +Opaque revocation is simpler than JWT revocation (token introspection or short-lived tokens ++ refresh). The cluster is single-node; session affinity is not a constraint. Stateless tokens +buy complexity without benefit here. JWKS infrastructure and refresh-token rotation are +unnecessary for a family archive with < 50 concurrent users. + +### Keep `auth_token` cookie but add `AuthTokenCookieFilter` improvements + +The root problem is that the cookie contains the credential. No amount of filter hardening +fixes that. Nora's P1 flag stands until the credential leaves the cookie. + +--- + +## Consequences + +- **One breaking deploy.** All existing sessions (the `auth_token` cookies) become inert + on the next request after the deploy. The SvelteKit `handleAuth` hook redirects to + `/login?reason=expired`; a banner renders. Users re-login. No data loss. +- **~2 KB per active session** in PostgreSQL (`spring_session_attributes` stores the + serialised `SecurityContext`). With < 50 family members, this is immaterial. +- **Session cleanup task** runs on the default Spring Session JDBC schedule (every 10 min). + No custom job needed. +- **Caddy / infrastructure unchanged.** `forward-headers-strategy: native` already ensures + `Secure` cookies work correctly behind the reverse proxy. +- **Dev profile:** `application-dev.yaml` sets `secure: false` on the session cookie so + local HTTP dev (port 5173 → 8080) works without TLS. -- 2.49.1 From 14542b6e3379024fa943ac7f1b4e4d9f1bf4b083 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:16:25 +0200 Subject: [PATCH 02/32] =?UTF-8?q?migration:=20V67=20=E2=80=94=20recreate?= =?UTF-8?q?=20spring=5Fsession=20tables=20(ADR-020)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-introduces tables dropped by V2. Canonical DDL from Spring Session 3.x schema-postgresql.sql. Co-Authored-By: Claude Sonnet 4.6 --- .../V67__recreate_spring_session_tables.sql | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 backend/src/main/resources/db/migration/V67__recreate_spring_session_tables.sql diff --git a/backend/src/main/resources/db/migration/V67__recreate_spring_session_tables.sql b/backend/src/main/resources/db/migration/V67__recreate_spring_session_tables.sql new file mode 100644 index 00000000..cde53a67 --- /dev/null +++ b/backend/src/main/resources/db/migration/V67__recreate_spring_session_tables.sql @@ -0,0 +1,27 @@ +-- Re-introduces the Spring Session JDBC tables that were dropped by V2 as unused. +-- DDL copied verbatim from Spring Session 3.x schema-postgresql.sql. +-- See ADR-020 and issue #523. + +CREATE TABLE spring_session ( + PRIMARY_ID CHAR(36) NOT NULL, + SESSION_ID CHAR(36) NOT NULL, + CREATION_TIME BIGINT NOT NULL, + LAST_ACCESS_TIME BIGINT NOT NULL, + MAX_INACTIVE_INTERVAL INT NOT NULL, + EXPIRY_TIME BIGINT NOT NULL, + PRINCIPAL_NAME VARCHAR(100), + CONSTRAINT spring_session_pk PRIMARY KEY (PRIMARY_ID) +); + +CREATE UNIQUE INDEX spring_session_ix1 ON spring_session (SESSION_ID); +CREATE INDEX spring_session_ix2 ON spring_session (EXPIRY_TIME); +CREATE INDEX spring_session_ix3 ON spring_session (PRINCIPAL_NAME); + +CREATE TABLE spring_session_attributes ( + SESSION_PRIMARY_ID CHAR(36) NOT NULL, + ATTRIBUTE_NAME VARCHAR(200) NOT NULL, + ATTRIBUTE_BYTES BYTEA NOT NULL, + CONSTRAINT spring_session_attributes_pk PRIMARY KEY (SESSION_PRIMARY_ID, ATTRIBUTE_NAME), + CONSTRAINT spring_session_attributes_fk FOREIGN KEY (SESSION_PRIMARY_ID) + REFERENCES spring_session (PRIMARY_ID) ON DELETE CASCADE +); -- 2.49.1 From 865c6ed796ff44ef675f4b3a94f056eb8609f881 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:17:46 +0200 Subject: [PATCH 03/32] feat(auth): add spring-session-jdbc 4.0.3 dependency Co-Authored-By: Claude Sonnet 4.6 --- backend/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/backend/pom.xml b/backend/pom.xml index d82d3ad0..f1a144d6 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -69,6 +69,11 @@ org.springframework.boot spring-boot-starter-security + + org.springframework.session + spring-session-jdbc + 4.0.3 + org.springframework.boot spring-boot-starter-webmvc -- 2.49.1 From 8c7a2741b0e98a0318f7b7cb8357e5a9c6fc49cf Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:18:28 +0200 Subject: [PATCH 04/32] feat(auth): configure Spring Session JDBC (fa_session, 8h idle, SameSite=strict) Co-Authored-By: Claude Sonnet 4.6 --- backend/src/main/resources/application-dev.yaml | 5 +++++ backend/src/main/resources/application.yaml | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/backend/src/main/resources/application-dev.yaml b/backend/src/main/resources/application-dev.yaml index 56c49e96..dd6c521d 100644 --- a/backend/src/main/resources/application-dev.yaml +++ b/backend/src/main/resources/application-dev.yaml @@ -1,6 +1,11 @@ spring: jpa: show-sql: true + session: + cookie: + # Dev runs over HTTP (port 5173 → 8080); Secure=true would prevent the + # cookie from being sent on plain HTTP. Override to false for local dev only. + secure: false springdoc: api-docs: diff --git a/backend/src/main/resources/application.yaml b/backend/src/main/resources/application.yaml index 776b2ab1..53e9b34a 100644 --- a/backend/src/main/resources/application.yaml +++ b/backend/src/main/resources/application.yaml @@ -38,6 +38,19 @@ spring: starttls: enable: true +spring: + session: + store-type: jdbc + timeout: 28800s # 8 h idle timeout (MaxInactiveIntervalInSeconds) + jdbc: + initialize-schema: never # Flyway owns schema creation (V67) + cookie: + name: fa_session + same-site: strict + http-only: true + # secure: true is the default when forward-headers-strategy detects HTTPS behind Caddy. + # application-dev.yaml overrides this to false for local HTTP dev. + server: # Behind Caddy/reverse proxy: trust X-Forwarded-{Proto,For,Host} so that # request.getScheme(), redirect URLs, and Spring Session "Secure" cookies -- 2.49.1 From 393a3c25fdeae61215231a4bc17ef4e028f2926e Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:19:01 +0200 Subject: [PATCH 05/32] feat(auth): add INVALID_CREDENTIALS + SESSION_EXPIRED error codes; LOGIN_SUCCESS/FAILED/LOGOUT audit kinds Co-Authored-By: Claude Sonnet 4.6 --- .../org/raddatz/familienarchiv/audit/AuditKind.java | 11 ++++++++++- .../raddatz/familienarchiv/exception/ErrorCode.java | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java index ef2939a0..3ceb8f39 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java @@ -35,7 +35,16 @@ public enum AuditKind { USER_DELETED, /** Payload: {@code {"userId": "uuid", "email": "addr", "addedGroups": ["Admin"], "removedGroups": []}} */ - GROUP_MEMBERSHIP_CHANGED; + GROUP_MEMBERSHIP_CHANGED, + + /** Payload: {@code {"userId": "uuid", "ip": "1.2.3.4", "ua": "Mozilla/5.0..."}} */ + LOGIN_SUCCESS, + + /** Payload: {@code {"email": "addr", "ip": "1.2.3.4", "ua": "Mozilla/5.0..."}} — password NEVER included */ + LOGIN_FAILED, + + /** Payload: {@code {"userId": "uuid", "ip": "1.2.3.4", "ua": "Mozilla/5.0..."}} */ + LOGOUT; public static final Set ROLLUP_ELIGIBLE = Set.of( TEXT_SAVED, FILE_UPLOADED, ANNOTATION_CREATED, diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index 07751700..7489bb83 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -62,6 +62,10 @@ public enum ErrorCode { UNAUTHORIZED, /** The authenticated user lacks the required permission. 403 */ FORBIDDEN, + /** The supplied email/password combination does not match any active account. 401 */ + INVALID_CREDENTIALS, + /** The session has expired or been invalidated. 401 */ + SESSION_EXPIRED, /** The password-reset token is missing, expired, or already used. 400 */ INVALID_RESET_TOKEN, -- 2.49.1 From a77b0c1221becf8ee0a4f264b03695727403037c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:21:46 +0200 Subject: [PATCH 06/32] =?UTF-8?q?feat(auth):=20AuthService=20=E2=80=94=20l?= =?UTF-8?q?ogin/logout=20with=20audit=20logging=20and=20timing-safe=20cred?= =?UTF-8?q?ential=20rejection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/auth/AuthService.java | 69 ++++++++++ .../exception/DomainException.java | 5 + .../familienarchiv/auth/AuthServiceTest.java | 129 ++++++++++++++++++ 3 files changed, 203 insertions(+) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java new file mode 100644 index 00000000..c66b6418 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java @@ -0,0 +1,69 @@ +package org.raddatz.familienarchiv.auth; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.UserService; +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; +import org.springframework.stereotype.Service; + +import java.util.Map; +import java.util.UUID; + +@Service +@RequiredArgsConstructor +@Slf4j +public class AuthService { + + private final AuthenticationManager authenticationManager; + private final UserService userService; + private final AuditService auditService; + + /** + * Validates credentials and returns the authenticated user plus the Spring Security + * Authentication object. The caller is responsible for persisting the Authentication + * to the session via SecurityContextRepository. + */ + public LoginResult login(String email, String password, String ip, String ua) { + try { + Authentication auth = authenticationManager.authenticate( + new UsernamePasswordAuthenticationToken(email, password)); + + AppUser user = userService.findByEmail(email); + auditService.log(AuditKind.LOGIN_SUCCESS, user.getId(), null, Map.of( + "userId", user.getId().toString(), + "ip", ip, + "ua", truncateUa(ua))); + return new LoginResult(user, auth); + } catch (AuthenticationException ex) { + // Audit login failure — intentionally does NOT log the attempted password. + // DaoAuthenticationProvider already runs a dummy BCrypt on unknown users to + // equalise timing between "user not found" and "wrong password" paths. + auditService.log(AuditKind.LOGIN_FAILED, null, null, Map.of( + "email", email, + "ip", ip, + "ua", truncateUa(ua))); + throw DomainException.invalidCredentials(); + } + } + + public void logout(UUID userId, String ip, String ua) { + auditService.log(AuditKind.LOGOUT, userId, null, Map.of( + "userId", userId.toString(), + "ip", ip, + "ua", truncateUa(ua))); + } + + private static String truncateUa(String ua) { + if (ua == null) return ""; + return ua.length() > 200 ? ua.substring(0, 200) : ua; + } + + public record LoginResult(AppUser user, Authentication authentication) {} +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/DomainException.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/DomainException.java index f82768c6..b911059e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/DomainException.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/DomainException.java @@ -39,6 +39,11 @@ public class DomainException extends RuntimeException { return new DomainException(ErrorCode.UNAUTHORIZED, HttpStatus.UNAUTHORIZED, message); } + public static DomainException invalidCredentials() { + return new DomainException(ErrorCode.INVALID_CREDENTIALS, HttpStatus.UNAUTHORIZED, + "Invalid email or password"); + } + public static DomainException conflict(ErrorCode code, String message) { return new DomainException(code, HttpStatus.CONFLICT, message); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java new file mode 100644 index 00000000..f394c056 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java @@ -0,0 +1,129 @@ +package org.raddatz.familienarchiv.auth; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.UserService; +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; + +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class AuthServiceTest { + + @Mock AuthenticationManager authenticationManager; + @Mock UserService userService; + @Mock AuditService auditService; + @InjectMocks AuthService authService; + + private static final String IP = "127.0.0.1"; + private static final String UA = "Mozilla/5.0 (Test)"; + + @Test + void login_returns_user_on_valid_credentials() { + UUID userId = UUID.randomUUID(); + AppUser user = AppUser.builder().id(userId).email("user@test.de").build(); + Authentication auth = new UsernamePasswordAuthenticationToken("user@test.de", null, Set.of()); + when(authenticationManager.authenticate(any())).thenReturn(auth); + when(userService.findByEmail("user@test.de")).thenReturn(user); + + AuthService.LoginResult result = authService.login("user@test.de", "pass123", IP, UA); + + assertThat(result.user()).isEqualTo(user); + assertThat(result.authentication()).isEqualTo(auth); + } + + @Test + void login_fires_LOGIN_SUCCESS_audit_on_valid_credentials() { + UUID userId = UUID.randomUUID(); + AppUser user = AppUser.builder().id(userId).email("user@test.de").build(); + Authentication auth = new UsernamePasswordAuthenticationToken("user@test.de", null, Set.of()); + when(authenticationManager.authenticate(any())).thenReturn(auth); + when(userService.findByEmail("user@test.de")).thenReturn(user); + + authService.login("user@test.de", "pass123", IP, UA); + + verify(auditService).log( + eq(AuditKind.LOGIN_SUCCESS), + eq(userId), + isNull(), + argThat(payload -> userId.toString().equals(payload.get("userId").toString()) + && IP.equals(payload.get("ip")) + && !payload.containsKey("password")) + ); + } + + @Test + void login_throws_INVALID_CREDENTIALS_on_bad_password() { + when(authenticationManager.authenticate(any())).thenThrow(new BadCredentialsException("bad")); + + assertThatThrownBy(() -> authService.login("user@test.de", "wrong", IP, UA)) + .isInstanceOf(DomainException.class) + .satisfies(ex -> assertThat(((DomainException) ex).getCode()) + .isEqualTo(ErrorCode.INVALID_CREDENTIALS)); + } + + @Test + void login_fires_LOGIN_FAILED_audit_on_bad_credentials_without_password_in_payload() { + when(authenticationManager.authenticate(any())).thenThrow(new BadCredentialsException("bad")); + + assertThatThrownBy(() -> authService.login("user@test.de", "wrong", IP, UA)) + .isInstanceOf(DomainException.class); + + verify(auditService).log( + eq(AuditKind.LOGIN_FAILED), + isNull(), + isNull(), + argThat(payload -> "user@test.de".equals(payload.get("email")) + && IP.equals(payload.get("ip")) + && !payload.containsKey("password") + && !payload.containsKey("pwd") + && !payload.containsKey("passwordAttempt")) + ); + } + + @Test + void login_treats_unknown_user_identically_to_bad_password() { + when(authenticationManager.authenticate(any())) + .thenThrow(new BadCredentialsException("unknown user hidden as bad creds")); + + assertThatThrownBy(() -> authService.login("unknown@test.de", "any", IP, UA)) + .isInstanceOf(DomainException.class) + .satisfies(ex -> assertThat(((DomainException) ex).getCode()) + .isEqualTo(ErrorCode.INVALID_CREDENTIALS)); + + verify(auditService).log(eq(AuditKind.LOGIN_FAILED), isNull(), isNull(), anyMap()); + } + + @Test + void logout_fires_LOGOUT_audit() { + UUID userId = UUID.randomUUID(); + + authService.logout(userId, IP, UA); + + verify(auditService).log( + eq(AuditKind.LOGOUT), + eq(userId), + isNull(), + argThat(payload -> userId.toString().equals(payload.get("userId").toString()) + && IP.equals(payload.get("ip"))) + ); + } +} -- 2.49.1 From e0aca0f88312cadb4c4c25f6710a14376ae215d2 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:26:49 +0200 Subject: [PATCH 07/32] =?UTF-8?q?feat(auth):=20AuthSessionController=20?= =?UTF-8?q?=E2=80=94=20POST=20/api/auth/login=20+=20/api/auth/logout=20wit?= =?UTF-8?q?h=20Spring=20Session=20JDBC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Expose AuthenticationManager bean in SecurityConfig - Permit /api/auth/login; return 401 (not 302) for unauthenticated requests - Remove httpBasic and formLogin from SecurityConfig Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/auth/AuthService.java | 7 +- .../auth/AuthSessionController.java | 73 ++++++++++++ .../familienarchiv/auth/LoginRequest.java | 3 + .../security/SecurityConfig.java | 39 +++--- backend/src/main/resources/application.yaml | 1 - .../familienarchiv/auth/AuthServiceTest.java | 7 +- .../auth/AuthSessionControllerTest.java | 111 ++++++++++++++++++ 7 files changed, 216 insertions(+), 25 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/auth/LoginRequest.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java index c66b6418..11c34d2d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthService.java @@ -53,9 +53,10 @@ public class AuthService { } } - public void logout(UUID userId, String ip, String ua) { - auditService.log(AuditKind.LOGOUT, userId, null, Map.of( - "userId", userId.toString(), + public void logout(String email, String ip, String ua) { + AppUser user = userService.findByEmail(email); + auditService.log(AuditKind.LOGOUT, user.getId(), null, Map.of( + "userId", user.getId().toString(), "ip", ip, "ua", truncateUa(ua))); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java new file mode 100644 index 00000000..8eca4877 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java @@ -0,0 +1,73 @@ +package org.raddatz.familienarchiv.auth; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; +import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.user.AppUser; +import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.web.bind.annotation.*; + +// @RequirePermission is intentionally absent: login is unauthenticated by design; +// logout requires an authenticated session (enforced by SecurityConfig), not a specific permission. +@RestController +@RequestMapping("/api/auth") +@RequiredArgsConstructor +public class AuthSessionController { + + private final AuthService authService; + + @PostMapping("/login") + public ResponseEntity login( + @RequestBody LoginRequest request, + HttpServletRequest httpRequest) { + + String ip = resolveClientIp(httpRequest); + String ua = resolveUserAgent(httpRequest); + + AuthService.LoginResult result = authService.login(request.email(), request.password(), ip, ua); + + // Establish server-side session. Spring Session JDBC intercepts getSession().setAttribute() + // and persists the record; the Set-Cookie: fa_session= is added automatically. + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(result.authentication()); + SecurityContextHolder.setContext(context); + httpRequest.getSession(true) + .setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, context); + + return ResponseEntity.ok(result.user()); + } + + @PostMapping("/logout") + public ResponseEntity logout(Authentication authentication, HttpServletRequest httpRequest) { + String ip = resolveClientIp(httpRequest); + String ua = resolveUserAgent(httpRequest); + + authService.logout(authentication.getName(), ip, ua); + + HttpSession session = httpRequest.getSession(false); + if (session != null) { + session.invalidate(); + } + SecurityContextHolder.clearContext(); + + return ResponseEntity.noContent().build(); + } + + private static String resolveClientIp(HttpServletRequest request) { + String forwarded = request.getHeader("X-Forwarded-For"); + if (forwarded != null && !forwarded.isBlank()) { + return forwarded.split(",")[0].trim(); + } + return request.getRemoteAddr(); + } + + private static String resolveUserAgent(HttpServletRequest request) { + String ua = request.getHeader("User-Agent"); + return ua != null ? ua : ""; + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/auth/LoginRequest.java b/backend/src/main/java/org/raddatz/familienarchiv/auth/LoginRequest.java new file mode 100644 index 00000000..e5478f5b --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/auth/LoginRequest.java @@ -0,0 +1,3 @@ +package org.raddatz.familienarchiv.auth; + +public record LoginRequest(String email, String password) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java index 8b1a45ac..8828e610 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java @@ -8,8 +8,9 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; import org.springframework.core.env.Environment; +import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; -import org.springframework.security.config.Customizer; +import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; @@ -37,6 +38,11 @@ public class SecurityConfig { return authProvider; } + @Bean + public AuthenticationManager authenticationManager(AuthenticationConfiguration config) throws Exception { + return config.getAuthenticationManager(); + } + @Bean @Order(1) public SecurityFilterChain managementFilterChain(HttpSecurity http) throws Exception { @@ -62,27 +68,21 @@ public class SecurityConfig { @Bean public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { http - // 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: + // CSRF is intentionally disabled. The session model relies on: + // 1. SameSite=Strict on the fa_session cookie — 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). // - // 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. + // If either of those is ever weakened, CSRF protection MUST be re-enabled. + // Re-enabling CSRF (CookieCsrfTokenRepository) is planned for Phase 2 (#524). .csrf(csrf -> csrf.disable()) .authorizeHttpRequests(auth -> { // Actuator endpoints are governed by managementFilterChain (@Order(1)) above. - // The permitAll() lines here are a belt-and-suspenders fallback in case any - // actuator path escapes that chain's securityMatcher. See docs/adr/017. auth.requestMatchers("/actuator/health", "/actuator/prometheus").permitAll(); + // Login is unauthenticated by definition + auth.requestMatchers("/api/auth/login").permitAll(); // Password reset endpoints are unauthenticated by nature auth.requestMatchers("/api/auth/forgot-password", "/api/auth/reset-password").permitAll(); // Invite-based registration endpoints are public @@ -102,9 +102,10 @@ public class SecurityConfig { // erlaubt pdf im Iframe .headers(headers -> headers .frameOptions(frameOptions -> frameOptions.sameOrigin())) - // Erlaubt Login via Browser-Popup oder REST-Header (Authorization: Basic ...) - .httpBasic(Customizer.withDefaults()) - .formLogin(form -> form.usernameParameter("email")); + // Return 401 (not 302 redirect to /login) for unauthenticated API requests. + // httpBasic and formLogin are removed — authentication is via Spring Session only. + .exceptionHandling(ex -> ex.authenticationEntryPoint( + (req, res, e) -> res.setStatus(HttpServletResponse.SC_UNAUTHORIZED))); return http.build(); } diff --git a/backend/src/main/resources/application.yaml b/backend/src/main/resources/application.yaml index 53e9b34a..f0df329b 100644 --- a/backend/src/main/resources/application.yaml +++ b/backend/src/main/resources/application.yaml @@ -38,7 +38,6 @@ spring: starttls: enable: true -spring: session: store-type: jdbc timeout: 28800s # 8 h idle timeout (MaxInactiveIntervalInSeconds) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java index f394c056..9ae0182a 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthServiceTest.java @@ -115,15 +115,18 @@ class AuthServiceTest { @Test void logout_fires_LOGOUT_audit() { UUID userId = UUID.randomUUID(); + AppUser user = AppUser.builder().id(userId).email("user@test.de").build(); + when(userService.findByEmail("user@test.de")).thenReturn(user); - authService.logout(userId, IP, UA); + authService.logout("user@test.de", IP, UA); verify(auditService).log( eq(AuditKind.LOGOUT), eq(userId), isNull(), argThat(payload -> userId.toString().equals(payload.get("userId").toString()) - && IP.equals(payload.get("ip"))) + && IP.equals(payload.get("ip")) + && !payload.containsKey("password")) ); } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java new file mode 100644 index 00000000..037f1ec8 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java @@ -0,0 +1,111 @@ +package org.raddatz.familienarchiv.auth; + +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.auth.AuthService.LoginResult; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.security.SecurityConfig; +import org.raddatz.familienarchiv.security.PermissionAspect; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.CustomUserDetailsService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration; +import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; +import org.springframework.context.annotation.Import; +import org.springframework.http.MediaType; +import org.springframework.security.core.Authentication; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; + +import java.util.UUID; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; + +@WebMvcTest(AuthSessionController.class) +@Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class}) +class AuthSessionControllerTest { + + @Autowired MockMvc mockMvc; + + @MockitoBean AuthService authService; + @MockitoBean CustomUserDetailsService customUserDetailsService; + + // ─── POST /api/auth/login ────────────────────────────────────────────────── + + @Test + void login_returns_200_with_user_on_valid_credentials() throws Exception { + UUID userId = UUID.randomUUID(); + AppUser appUser = AppUser.builder().id(userId).email("user@test.de").build(); + Authentication auth = mock(Authentication.class); + when(authService.login(anyString(), anyString(), anyString(), anyString())) + .thenReturn(new LoginResult(appUser, auth)); + + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"user@test.de\",\"password\":\"pass123\"}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.email").value("user@test.de")) + .andExpect(jsonPath("$.id").value(userId.toString())); + } + + @Test + void login_returns_401_with_INVALID_CREDENTIALS_on_bad_credentials() throws Exception { + when(authService.login(anyString(), anyString(), anyString(), anyString())) + .thenThrow(DomainException.invalidCredentials()); + + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"user@test.de\",\"password\":\"wrong\"}")) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(ErrorCode.INVALID_CREDENTIALS.name())); + } + + @Test + void login_is_public_no_session_required() throws Exception { + UUID userId = UUID.randomUUID(); + AppUser appUser = AppUser.builder().id(userId).email("pub@test.de").build(); + Authentication auth = mock(Authentication.class); + when(authService.login(anyString(), anyString(), anyString(), anyString())) + .thenReturn(new LoginResult(appUser, auth)); + + // No WithMockUser — must be reachable without an active session + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"pub@test.de\",\"password\":\"pass\"}")) + .andExpect(status().isOk()); + } + + @Test + void login_does_not_set_cookie_on_failure() throws Exception { + when(authService.login(anyString(), anyString(), anyString(), anyString())) + .thenThrow(DomainException.invalidCredentials()); + + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"user@test.de\",\"password\":\"wrong\"}")) + .andExpect(status().isUnauthorized()) + .andExpect(header().doesNotExist("Set-Cookie")); + } + + // ─── POST /api/auth/logout ───────────────────────────────────────────────── + + @Test + void logout_returns_204_when_authenticated() throws Exception { + doNothing().when(authService).logout(anyString(), anyString(), anyString()); + + mockMvc.perform(post("/api/auth/logout") + .with(user("user@test.de"))) + .andExpect(status().isNoContent()); + } + + @Test + void logout_returns_401_when_not_authenticated() throws Exception { + // No authentication at all — Spring Security must return 401 + mockMvc.perform(post("/api/auth/logout")) + .andExpect(status().isUnauthorized()); + } +} -- 2.49.1 From a6c85e3658fe949ce75054679e485b2c518022cc Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:27:53 +0200 Subject: [PATCH 08/32] feat(auth): delete AuthTokenCookieFilter and its test (ADR-020) Co-Authored-By: Claude Sonnet 4.6 --- .../security/AuthTokenCookieFilter.java | 137 ------------------ .../security/AuthTokenCookieFilterTest.java | 134 ----------------- 2 files changed, 271 deletions(-) delete mode 100644 backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java delete mode 100644 backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java b/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java deleted file mode 100644 index d382f872..00000000 --- a/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java +++ /dev/null @@ -1,137 +0,0 @@ -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. - * - *

The SvelteKit login action stores the full HTTP Basic header value - * ({@code "Basic "}) 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. - * - *

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. - * - *

See #520. Filter runs at {@code Ordered.HIGHEST_PRECEDENCE} so it - * mutates the request before any Spring Security filter sees it. - * - *

Scope: 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. - * - *

⚠ Log-leakage warning: 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 getHeaders(String name) { - if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(name)) { - return Collections.enumeration(Collections.singletonList(authorization)); - } - return super.getHeaders(name); - } - - @Override - public Enumeration getHeaderNames() { - Enumeration base = super.getHeaderNames(); - java.util.Set names = new java.util.LinkedHashSet<>(); - while (base.hasMoreElements()) names.add(base.nextElement()); - names.add(HttpHeaders.AUTHORIZATION); - return Collections.enumeration(names); - } - } -} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java b/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java deleted file mode 100644 index 18ceab49..00000000 --- a/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java +++ /dev/null @@ -1,134 +0,0 @@ -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} - * into {@code Authorization: Basic } (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. - * - *

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 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); - } -} -- 2.49.1 From 0fa330a357cc96233edb49ec7f2d64d259be65ce Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 19:50:22 +0200 Subject: [PATCH 09/32] test(auth): integration tests for full session lifecycle and idle-timeout 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 --- backend/pom.xml | 5 +- .../config/SpringSessionConfig.java | 22 +++ .../src/main/resources/application-dev.yaml | 8 +- backend/src/main/resources/application.yaml | 9 +- .../auth/AuthSessionIntegrationTest.java | 154 ++++++++++++++++++ 5 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/config/SpringSessionConfig.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionIntegrationTest.java diff --git a/backend/pom.xml b/backend/pom.xml index f1a144d6..6e9b389b 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -70,9 +70,8 @@ spring-boot-starter-security - org.springframework.session - spring-session-jdbc - 4.0.3 + org.springframework.boot + spring-boot-starter-session-jdbc org.springframework.boot diff --git a/backend/src/main/java/org/raddatz/familienarchiv/config/SpringSessionConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/config/SpringSessionConfig.java new file mode 100644 index 00000000..415903cd --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/config/SpringSessionConfig.java @@ -0,0 +1,22 @@ +package org.raddatz.familienarchiv.config; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.session.web.http.CookieSerializer; +import org.springframework.session.web.http.DefaultCookieSerializer; + +@Configuration +public class SpringSessionConfig { + + @Bean + public CookieSerializer cookieSerializer() { + DefaultCookieSerializer serializer = new DefaultCookieSerializer(); + serializer.setCookieName("fa_session"); + serializer.setSameSite("Strict"); + // cookieHttpOnly: true is the DefaultCookieSerializer default + // useSecureCookie not set: auto-detects from request.isSecure(). + // With forward-headers-strategy: native, Caddy's X-Forwarded-Proto: https + // causes isSecure() → true in production; direct HTTP in dev/tests → false. + return serializer; + } +} diff --git a/backend/src/main/resources/application-dev.yaml b/backend/src/main/resources/application-dev.yaml index dd6c521d..54e4a972 100644 --- a/backend/src/main/resources/application-dev.yaml +++ b/backend/src/main/resources/application-dev.yaml @@ -1,11 +1,9 @@ spring: jpa: show-sql: true - session: - cookie: - # Dev runs over HTTP (port 5173 → 8080); Secure=true would prevent the - # cookie from being sent on plain HTTP. Override to false for local dev only. - secure: false + # spring.session.cookie.secure is no longer a supported Boot 4.x property. + # DefaultCookieSerializer auto-detects Secure from request.isSecure(). + # Direct HTTP in dev → isSecure()=false → cookie sent without Secure attribute. springdoc: api-docs: diff --git a/backend/src/main/resources/application.yaml b/backend/src/main/resources/application.yaml index f0df329b..2a764e8e 100644 --- a/backend/src/main/resources/application.yaml +++ b/backend/src/main/resources/application.yaml @@ -39,16 +39,11 @@ spring: enable: true session: - store-type: jdbc timeout: 28800s # 8 h idle timeout (MaxInactiveIntervalInSeconds) jdbc: initialize-schema: never # Flyway owns schema creation (V67) - cookie: - name: fa_session - same-site: strict - http-only: true - # secure: true is the default when forward-headers-strategy detects HTTPS behind Caddy. - # application-dev.yaml overrides this to false for local HTTP dev. + # Cookie name, SameSite, and Secure are configured via SpringSessionConfig#cookieSerializer + # (spring.session.cookie.* is not supported in Spring Boot 4.x). server: # Behind Caddy/reverse proxy: trust X-Forwarded-{Proto,For,Host} so that diff --git a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionIntegrationTest.java new file mode 100644 index 00000000..92ff991e --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionIntegrationTest.java @@ -0,0 +1,154 @@ +package org.raddatz.familienarchiv.auth; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.AppUserRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.context.annotation.Import; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.web.client.DefaultResponseErrorHandler; +import org.springframework.web.client.RestTemplate; +import software.amazon.awssdk.services.s3.S3Client; + +import java.io.IOException; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@ActiveProfiles("test") +@Import(PostgresContainerConfig.class) +class AuthSessionIntegrationTest { + + @LocalServerPort int port; + @MockitoBean S3Client s3Client; + @Autowired AppUserRepository userRepository; + @Autowired PasswordEncoder passwordEncoder; + @Autowired JdbcTemplate jdbcTemplate; + + private RestTemplate http; + private String baseUrl; + + private static final String TEST_EMAIL = "session-it@test.de"; + private static final String TEST_PASSWORD = "pass4Session!"; + + @BeforeEach + void setUp() { + http = noThrowRestTemplate(); + baseUrl = "http://localhost:" + port; + // spring_session_attributes cascades on delete — removing the parent row is enough + jdbcTemplate.update("DELETE FROM spring_session"); + jdbcTemplate.update("DELETE FROM app_users WHERE email = ?", TEST_EMAIL); + userRepository.save(AppUser.builder() + .email(TEST_EMAIL) + .password(passwordEncoder.encode(TEST_PASSWORD)) + .build()); + } + + // ─── Task 13: full session lifecycle ────────────────────────────────────── + + @Test + void login_sets_opaque_fa_session_cookie() { + ResponseEntity response = doLogin(); + + assertThat(response.getStatusCode().value()).isEqualTo(200); + String cookie = extractFaSessionCookie(response); + assertThat(cookie).isNotBlank(); + // Opaque token — must not look like Basic-auth credentials (email:password) + assertThat(cookie).doesNotContain(":"); + } + + @Test + void session_cookie_authenticates_subsequent_request() { + String cookie = extractFaSessionCookie(doLogin()); + + ResponseEntity me = http.exchange( + baseUrl + "/api/users/me", HttpMethod.GET, + new HttpEntity<>(cookieHeaders(cookie)), String.class); + + assertThat(me.getStatusCode().value()).isEqualTo(200); + } + + @Test + void logout_invalidates_session_and_cookie_returns_401_on_reuse() { + String cookie = extractFaSessionCookie(doLogin()); + + ResponseEntity logout = http.postForEntity( + baseUrl + "/api/auth/logout", + new HttpEntity<>(cookieHeaders(cookie)), Void.class); + assertThat(logout.getStatusCode().value()).isEqualTo(204); + + ResponseEntity me = http.exchange( + baseUrl + "/api/users/me", HttpMethod.GET, + new HttpEntity<>(cookieHeaders(cookie)), String.class); + assertThat(me.getStatusCode().value()).isEqualTo(401); + } + + // ─── Task 14: idle-timeout ──────────────────────────────────────────────── + + @Test + void session_expired_by_idle_timeout_returns_401() { + String cookie = extractFaSessionCookie(doLogin()); + + // Backdate LAST_ACCESS_TIME by 9 hours so lastAccess + maxInactiveInterval(8h) < now + long nineHoursAgoMs = System.currentTimeMillis() - 9L * 3600 * 1000; + jdbcTemplate.update( + "UPDATE spring_session SET LAST_ACCESS_TIME = ?, EXPIRY_TIME = ?", + nineHoursAgoMs, nineHoursAgoMs); + + ResponseEntity me = http.exchange( + baseUrl + "/api/users/me", HttpMethod.GET, + new HttpEntity<>(cookieHeaders(cookie)), String.class); + assertThat(me.getStatusCode().value()).isEqualTo(401); + } + + // ─── helpers ───────────────────────────────────────────────────────────── + + private ResponseEntity doLogin() { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + String body = "{\"email\":\"" + TEST_EMAIL + "\",\"password\":\"" + TEST_PASSWORD + "\"}"; + return http.postForEntity(baseUrl + "/api/auth/login", + new HttpEntity<>(body, headers), String.class); + } + + private HttpHeaders cookieHeaders(String sessionId) { + HttpHeaders headers = new HttpHeaders(); + headers.set("Cookie", "fa_session=" + sessionId); + return headers; + } + + private String extractFaSessionCookie(ResponseEntity response) { + List setCookieHeader = response.getHeaders().get("Set-Cookie"); + if (setCookieHeader == null) return ""; + return setCookieHeader.stream() + .filter(c -> c.startsWith("fa_session=")) + .map(c -> c.split(";")[0].substring("fa_session=".length())) + .findFirst() + .orElse(""); + } + + private RestTemplate noThrowRestTemplate() { + RestTemplate template = new RestTemplate(); + template.setErrorHandler(new DefaultResponseErrorHandler() { + @Override + public boolean hasError(ClientHttpResponse response) throws IOException { + return false; + } + }); + return template; + } +} -- 2.49.1 From cfff59473279eb4e22074c77dd5b806ad607a991 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 20:51:06 +0200 Subject: [PATCH 10/32] feat(auth): frontend ErrorCode + i18n for INVALID_CREDENTIALS and SESSION_EXPIRED Mirrors the backend ErrorCode additions from commit 393a3c25. Adds error_session_expired_explainer for the login-page banner that will surface when ?reason=expired. Co-Authored-By: Claude Sonnet 4.6 --- frontend/messages/de.json | 3 +++ frontend/messages/en.json | 3 +++ frontend/messages/es.json | 3 +++ frontend/src/lib/shared/errors.ts | 6 ++++++ 4 files changed, 15 insertions(+) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 38067459..03d63f48 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -14,6 +14,9 @@ "error_file_too_large": "Die Datei ist zu groß (max. 50 MB).", "error_user_not_found": "Der Benutzer wurde nicht gefunden.", "error_import_already_running": "Ein Import läuft bereits. Bitte warten Sie, bis dieser abgeschlossen ist.", + "error_invalid_credentials": "E-Mail-Adresse oder Passwort ist falsch.", + "error_session_expired": "Ihre Sitzung ist abgelaufen. Bitte melden Sie sich erneut an.", + "error_session_expired_explainer": "Aus Sicherheitsgründen werden Sitzungen nach 8 Stunden Inaktivität automatisch beendet.", "error_unauthorized": "Sie sind nicht angemeldet.", "error_forbidden": "Sie haben keine Berechtigung für diese Aktion.", "error_validation_error": "Die Eingabe ist ungültig.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 2ecb565d..d52ddbf5 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -14,6 +14,9 @@ "error_file_too_large": "The file is too large (max. 50 MB).", "error_user_not_found": "User not found.", "error_import_already_running": "An import is already running. Please wait for it to finish.", + "error_invalid_credentials": "Email address or password is incorrect.", + "error_session_expired": "Your session has expired. Please sign in again.", + "error_session_expired_explainer": "For security reasons, sessions are automatically ended after 8 hours of inactivity.", "error_unauthorized": "You are not logged in.", "error_forbidden": "You do not have permission for this action.", "error_validation_error": "The input is invalid.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index a68b663d..177dcdff 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -14,6 +14,9 @@ "error_file_too_large": "El archivo es demasiado grande (máx. 50 MB).", "error_user_not_found": "Usuario no encontrado.", "error_import_already_running": "Ya hay una importación en curso. Por favor, espere a que finalice.", + "error_invalid_credentials": "El correo electrónico o la contraseña son incorrectos.", + "error_session_expired": "Su sesión ha expirado. Por favor, inicie sesión de nuevo.", + "error_session_expired_explainer": "Por razones de seguridad, las sesiones se terminan automáticamente tras 8 horas de inactividad.", "error_unauthorized": "No ha iniciado sesión.", "error_forbidden": "No tiene permiso para realizar esta acción.", "error_validation_error": "La entrada no es válida.", diff --git a/frontend/src/lib/shared/errors.ts b/frontend/src/lib/shared/errors.ts index 9703bbd9..ab79487f 100644 --- a/frontend/src/lib/shared/errors.ts +++ b/frontend/src/lib/shared/errors.ts @@ -44,6 +44,8 @@ export type ErrorCode = | 'CIRCULAR_RELATIONSHIP' | 'DUPLICATE_RELATIONSHIP' | 'GESCHICHTE_NOT_FOUND' + | 'INVALID_CREDENTIALS' + | 'SESSION_EXPIRED' | 'MISSING_CREDENTIALS' | 'UNAUTHORIZED' | 'FORBIDDEN' @@ -154,6 +156,10 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_duplicate_relationship(); case 'GESCHICHTE_NOT_FOUND': return m.error_geschichte_not_found(); + case 'INVALID_CREDENTIALS': + return m.error_invalid_credentials(); + case 'SESSION_EXPIRED': + return m.error_session_expired(); case 'MISSING_CREDENTIALS': return m.login_error_missing_credentials(); case 'UNAUTHORIZED': -- 2.49.1 From ea800e5e2a7b44251c3e16b82a84e58f9a91fc37 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 20:52:30 +0200 Subject: [PATCH 11/32] feat(auth): rewrite login action to POST /api/auth/login and forward fa_session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the Basic-credentials-in-cookie flow with the Spring Session model: 1. POST {email, password} as JSON to /api/auth/login 2. Map 401 → INVALID_CREDENTIALS (or SESSION_EXPIRED if the backend returns it) 3. Parse Set-Cookie for fa_session= and re-emit to the browser 4. Drop the legacy auth_token cookie load() now also exposes ?reason= so the page can show the session-expired banner (Task 21 wires it into the .svelte file). Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/routes/login/+page.server.ts | 101 ++++++++++++++-------- 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/frontend/src/routes/login/+page.server.ts b/frontend/src/routes/login/+page.server.ts index 50aabec5..3f00a76e 100644 --- a/frontend/src/routes/login/+page.server.ts +++ b/frontend/src/routes/login/+page.server.ts @@ -1,12 +1,29 @@ import { fail, redirect, type Actions } from '@sveltejs/kit'; import { env } from '$env/dynamic/private'; -import { getErrorMessage } from '$lib/shared/errors'; +import { getErrorMessage, type ErrorCode } from '$lib/shared/errors'; import type { PageServerLoad } from './$types'; export const load: PageServerLoad = ({ url }) => { - return { registered: url.searchParams.get('registered') === '1' }; + return { + registered: url.searchParams.get('registered') === '1', + reason: url.searchParams.get('reason') + }; }; +/** + * Extracts the fa_session cookie value from a Set-Cookie response header. + * The backend may emit attributes like `Path`, `HttpOnly`, `SameSite=Strict`, `Max-Age`, `Secure`; + * we only forward the opaque session id — the SvelteKit cookies API will rewrite + * the attributes itself. + */ +function extractFaSessionId(setCookieHeaders: string[]): string | null { + for (const header of setCookieHeaders) { + const match = header.match(/^fa_session=([^;]+)/); + if (match) return match[1]; + } + return null; +} + export const actions = { login: async ({ request, cookies, fetch, url }) => { const data = await request.formData(); @@ -17,44 +34,60 @@ export const actions = { return fail(400, { error: getErrorMessage('MISSING_CREDENTIALS') }); } - const credentials = btoa(`${email}:${password}`); - const authHeader = `Basic ${credentials}`; - - // Raw fetch is intentional here: we need to pass an explicit Authorization - // header built from the form data, not the cookie-based auth used elsewhere. + const baseUrl = env.API_INTERNAL_URL || 'http://localhost:8080'; + let response: Response; try { - const baseUrl = env.API_INTERNAL_URL || 'http://localhost:8080'; - const response = await fetch(`${baseUrl}/api/users/me`, { - method: 'GET', - headers: { Authorization: authHeader } - }); - - if (response.status === 401 || response.status === 403) { - return fail(401, { error: getErrorMessage('UNAUTHORIZED') }); - } - - if (!response.ok) { - 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: isHttps, - maxAge: 60 * 60 * 24 + response = await fetch(`${baseUrl}/api/auth/login`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, password }) }); } catch (e) { - console.error(e); + console.error('Login request failed', e); return fail(500, { error: getErrorMessage('INTERNAL_ERROR') }); } + if (response.status === 401) { + let code: ErrorCode = 'INVALID_CREDENTIALS'; + try { + const body = (await response.json()) as { code?: string }; + if (body?.code) code = body.code as ErrorCode; + } catch { + // Body not JSON — fall through to INVALID_CREDENTIALS + } + return fail(401, { error: getErrorMessage(code) }); + } + + if (!response.ok) { + return fail(response.status, { error: getErrorMessage('INTERNAL_ERROR') }); + } + + // Extract fa_session id from the Set-Cookie header and re-emit to the browser. + // Modern Node/Undici exposes getSetCookie(); fall back to a single header for older runtimes. + const setCookieHeaders = + typeof response.headers.getSetCookie === 'function' + ? response.headers.getSetCookie() + : response.headers.get('set-cookie') + ? [response.headers.get('set-cookie')!] + : []; + const sessionId = extractFaSessionId(setCookieHeaders); + if (!sessionId) { + console.error('Backend returned 200 OK on login but no fa_session cookie'); + return fail(500, { error: getErrorMessage('INTERNAL_ERROR') }); + } + + const isHttps = url.protocol === 'https:'; + cookies.set('fa_session', sessionId, { + path: '/', + httpOnly: true, + sameSite: 'strict', + secure: isHttps, + maxAge: 60 * 60 * 8 // 8h — must match backend spring.session.timeout + }); + + // Best-effort cleanup of the legacy Basic-auth cookie from older deployments. + cookies.delete('auth_token', { path: '/' }); + return redirect(303, '/'); } } satisfies Actions; -- 2.49.1 From bfdf64975c5606d0c7b61e188be7a2f405b1c472 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 20:53:20 +0200 Subject: [PATCH 12/32] feat(auth): rewrite logout action to call /api/auth/logout then clear fa_session The backend POST invalidates the spring_session row and writes the LOGOUT audit entry; the client cookie is deleted unconditionally so a network blip during logout still logs the user out locally. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/routes/logout/+page.server.ts | 26 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/frontend/src/routes/logout/+page.server.ts b/frontend/src/routes/logout/+page.server.ts index 43147f4f..6085b00f 100644 --- a/frontend/src/routes/logout/+page.server.ts +++ b/frontend/src/routes/logout/+page.server.ts @@ -1,12 +1,30 @@ import { redirect } from '@sveltejs/kit'; +import { env } from '$env/dynamic/private'; import type { Actions } from './$types'; export const actions = { - default: async ({ cookies }) => { - // Das Auth-Cookie löschen + default: async ({ cookies, fetch }) => { + const sessionId = cookies.get('fa_session'); + + // Best-effort backend logout: invalidates the server-side session row + // and writes the LOGOUT audit entry. The client cookie is deleted + // unconditionally below so a network failure here still logs the user out. + if (sessionId) { + try { + const baseUrl = env.API_INTERNAL_URL || 'http://localhost:8080'; + await fetch(`${baseUrl}/api/auth/logout`, { + method: 'POST', + headers: { Cookie: `fa_session=${sessionId}` } + }); + } catch (e) { + console.error('Backend logout failed; clearing client cookie anyway', e); + } + } + + cookies.delete('fa_session', { path: '/' }); + // Also drop the legacy Basic-auth cookie in case a stale one lingers from before the migration. cookies.delete('auth_token', { path: '/' }); - // Zur Login-Seite werfen - throw redirect(302, '/login'); + throw redirect(303, '/login'); } } satisfies Actions; -- 2.49.1 From 6193e28587a0c461ad039692d87d9a526240fdeb Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 20:54:42 +0200 Subject: [PATCH 13/32] feat(auth): hooks forward fa_session cookie instead of injecting Basic auth userGroup: GET /api/users/me with Cookie: fa_session=. On 401, drop the stale cookie and redirect to /login?reason=expired (unless already on a public path) so the user sees an explainer instead of a silent kick. handleFetch: forward fa_session as a Cookie header on every API call except the public auth endpoints. Drops the old auth_token injection. Also adds a one-off cleanup of any lingering auth_token cookie from pre-migration sessions. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/hooks.server.ts | 74 +++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/frontend/src/hooks.server.ts b/frontend/src/hooks.server.ts index 2c155dce..5fe167c1 100644 --- a/frontend/src/hooks.server.ts +++ b/frontend/src/hooks.server.ts @@ -58,21 +58,40 @@ const handleParaglide: Handle = ({ event, resolve }) => }); const userGroup: Handle = async ({ event, resolve }) => { - const auth = event.cookies.get('auth_token'); + // One-off cleanup of the legacy Basic-credentials cookie from before the Spring Session migration (#523). + if (event.cookies.get('auth_token')) { + event.cookies.delete('auth_token', { path: '/' }); + } - if (auth) { - try { - const apiUrl = env.API_INTERNAL_URL || 'http://localhost:8080'; - const response = await fetch(`${apiUrl}/api/users/me`, { - headers: { Authorization: auth } - }); - if (response.ok) { - const user = await response.json(); - event.locals.user = user; + const sessionId = event.cookies.get('fa_session'); + if (!sessionId) { + return resolve(event); + } + + try { + const apiUrl = env.API_INTERNAL_URL || 'http://localhost:8080'; + const response = await fetch(`${apiUrl}/api/users/me`, { + headers: { Cookie: `fa_session=${sessionId}` } + }); + + if (response.ok) { + event.locals.user = await response.json(); + } else if (response.status === 401) { + // Backend rejected the session (expired or invalidated). Drop the stale + // cookie and surface the reason on the login page. PUBLIC_PATHS check + // avoids a redirect loop if the user is already on /login. + event.cookies.delete('fa_session', { path: '/' }); + const isPublic = PUBLIC_PATHS.some((p) => event.url.pathname.startsWith(p)); + if (!isPublic) { + throw redirect(302, '/login?reason=expired'); } - } catch (error) { - console.error('Error fetching user in hook:', error); } + } catch (error) { + // Don't swallow SvelteKit redirects — they're thrown as objects with a `status` field. + if (error instanceof Object && 'status' in error && 'location' in error) { + throw error; + } + console.error('Error fetching user in hook:', error); } return resolve(event); @@ -83,14 +102,11 @@ export const handleFetch: HandleFetch = async ({ event, request, fetch }) => { const isApi = request.url.startsWith(apiUrl) || request.url.includes('/api/'); if (isApi) { - // If the request already carries an explicit Authorization header (e.g. the - // login action sends Basic auth), pass it through unchanged. - if (request.headers.has('Authorization')) { - return fetch(request); - } - - // Password reset endpoints are public — no auth header needed. + // Auth endpoints that establish/check their own credentials manage cookies themselves; + // don't double-inject a stale fa_session. const PUBLIC_API_PATHS = [ + '/api/auth/login', + '/api/auth/logout', '/api/auth/forgot-password', '/api/auth/reset-password', '/api/auth/invite/', @@ -100,24 +116,20 @@ export const handleFetch: HandleFetch = async ({ event, request, fetch }) => { return fetch(request); } - const token = event.cookies.get('auth_token'); - - if (!token) { + const sessionId = event.cookies.get('fa_session'); + if (!sessionId) { return new Response('Unauthorized', { status: 401 }); } - // Clone the request first to preserve the body - const clonedRequest = request.clone(); - - // Create new request with Authorization header and preserved body - const modifiedRequest = new Request(clonedRequest, { + // Clone first so the body stream is preserved on the new Request. + const cloned = request.clone(); + const modified = new Request(cloned, { headers: { - ...Object.fromEntries(clonedRequest.headers), - Authorization: token + ...Object.fromEntries(cloned.headers), + Cookie: `fa_session=${sessionId}` } }); - - return fetch(modifiedRequest); + return fetch(modified); } return fetch(request); -- 2.49.1 From d301825e502a1527fe19d72864364e203aafad89 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 20:55:30 +0200 Subject: [PATCH 14/32] feat(auth): remove auth_token cookie injection from Vite dev proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the Spring Session model the browser forwards fa_session itself — the proxy no longer needs to translate auth_token → Authorization: Basic. Co-Authored-By: Claude Sonnet 4.6 --- frontend/vite.config.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 02f2cc48..bb4113c5 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -17,19 +17,9 @@ export default defineConfig({ proxy: { '/api': { target: process.env.API_PROXY_TARGET || 'http://localhost:8080', - changeOrigin: true, - // Inject Authorization header from the auth_token cookie so that - // browser-side fetch('/api/...') calls work the same as SSR fetches - // (which go through handleFetch in hooks.server.ts). - configure: (proxy) => { - proxy.on('proxyReq', (proxyReq, req) => { - const cookies = req.headers.cookie ?? ''; - const match = cookies.match(/auth_token=([^;]+)/); - if (match) { - proxyReq.setHeader('Authorization', decodeURIComponent(match[1])); - } - }); - } + changeOrigin: true + // The browser forwards the fa_session cookie to the backend automatically; + // no header injection needed (ADR-020). } } }, -- 2.49.1 From 0bd00a3044688416b7dc187530feb938e4195515 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 20:58:30 +0200 Subject: [PATCH 15/32] feat(auth): session-expired banner + autofocus + 44px touch target on login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Amber aria-live banner when ?reason=expired (set by hooks.server.ts after the backend rejects an expired fa_session) with a one-line explainer about the 8h idle window. - autofocus on email so users returning after a session-expired kick can immediately retype credentials. - min-h-[44px] on the submit button hits the iOS HIG / WCAG 2.1 AAA touch target minimum — relevant for the reader cohort on phones. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/routes/login/+page.svelte | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/frontend/src/routes/login/+page.svelte b/frontend/src/routes/login/+page.svelte index 95607e1c..49550710 100644 --- a/frontend/src/routes/login/+page.svelte +++ b/frontend/src/routes/login/+page.svelte @@ -5,7 +5,10 @@ import AuthHeader from '../AuthHeader.svelte'; let { data, form -}: { data: { registered: boolean }; form?: { error?: string; success?: boolean } } = $props(); +}: { + data: { registered: boolean; reason?: string | null }; + form?: { error?: string; success?: boolean }; +} = $props(); @@ -38,6 +41,17 @@ let { {/if} + {#if data.reason === 'expired'} +

+

{m.error_session_expired()}

+

{m.error_session_expired_explainer()}

+
+ {/if} +

{m.login_heading()}

@@ -49,11 +63,13 @@ let { class="mb-1.5 block font-sans text-xs font-bold tracking-widest text-ink-2 uppercase" >{m.login_label_email()} + @@ -81,7 +97,7 @@ let { -- 2.49.1 From 343826009026175bccf47c46deab5f29f61e9ef7 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 20:59:44 +0200 Subject: [PATCH 16/32] docs: rewrite seq-auth-flow.puml for the Spring Session model (ADR-020) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the cookie-promotion step (auth_token → Authorization: Basic) and splits the diagram into three labelled phases: Login, Authenticated request, Logout. Adds the spring_session DB round-trip on every authenticated request and the alt branch for an expired session returning 401 → /login?reason=expired. Co-Authored-By: Claude Sonnet 4.6 --- docs/architecture/c4/seq-auth-flow.puml | 79 +++++++++++++++++-------- 1 file changed, 53 insertions(+), 26 deletions(-) diff --git a/docs/architecture/c4/seq-auth-flow.puml b/docs/architecture/c4/seq-auth-flow.puml index 63b9038d..24d57e95 100644 --- a/docs/architecture/c4/seq-auth-flow.puml +++ b/docs/architecture/c4/seq-auth-flow.puml @@ -1,15 +1,21 @@ @startuml -title Authentication Flow (behind Caddy reverse proxy) +title Authentication Flow (Spring Session JDBC, behind Caddy reverse proxy) +note over Browser, DB + Phase 1 of the auth rewrite (ADR-020 / #523). + Replaces the Basic-credentials-in-cookie model + with an opaque server-side session id (fa_session). +end note actor User participant Browser participant "Caddy (TLS termination)" as Caddy participant "Frontend (SvelteKit)" as Frontend participant "Backend (Spring Boot)" as Backend -participant PostgreSQL as DB +participant "spring_session\n(PostgreSQL)" as DB +== Login == User -> Browser: Enter email + password -Browser -> Caddy: HTTPS POST /login (form action) +Browser -> Caddy: HTTPS POST /?/login (form action) note right of Caddy Caddy terminates TLS and forwards to Frontend over HTTP with: @@ -17,33 +23,54 @@ note right of Caddy X-Forwarded-For: X-Forwarded-Host: archiv.raddatz.cloud end note -Caddy -> Frontend: HTTP POST /login\n+ X-Forwarded-Proto: https -Frontend -> Frontend: Base64 encode "email:password" -Frontend -> Backend: GET /api/users/me\nAuthorization: Basic \n+ X-Forwarded-Proto: https +Caddy -> Frontend: HTTP POST /?/login + X-Forwarded-Proto: https +Frontend -> Backend: POST /api/auth/login\n{email, password}\n+ X-Forwarded-Proto: https note right of Backend server.forward-headers-strategy: native - Jetty's ForwardedRequestCustomizer - reads X-Forwarded-Proto so - request.getScheme() returns "https". + → request.getScheme() = "https" + → Secure cookie flag set automatically. end note -Backend -> Backend: Spring Security parses Basic Auth +Backend -> Backend: AuthenticationManager\nauthenticate(email, password) Backend -> DB: SELECT user WHERE email=? DB --> Backend: AppUser + groups + permissions -Backend -> Backend: BCrypt.matches(password, hash) -Backend --> Frontend: 200 OK — UserDTO -Frontend -> Caddy: Set-Cookie: auth_token=\n(httpOnly, **Secure**, SameSite=strict, maxAge=86400) -note right of Frontend - Secure flag is set because the - request scheme observed by the - app is https (forwarded by Caddy). -end note -Caddy -> Browser: HTTPS 200 + Set-Cookie -Browser -> Caddy: HTTPS GET / (next request) -Caddy -> Frontend: HTTP GET / + X-Forwarded-Proto: https -Frontend -> Frontend: hooks.server.ts reads auth_token cookie -Frontend -> Backend: GET /api/users/me\nAuthorization: Basic -Backend --> Frontend: 200 OK — user in event.locals -Frontend --> Caddy: rendered page -Caddy --> Browser: HTTPS 200 +Backend -> Backend: BCrypt.matches(password, hash)\n(timing-safe: dummy hash on miss) +Backend -> Backend: getSession(true).setAttribute(\n SPRING_SECURITY_CONTEXT, ctx) +Backend -> DB: INSERT spring_session\n+ spring_session_attributes +Backend -> Backend: AuditService.log(LOGIN_SUCCESS,\n {userId, ip, ua}) +Backend --> Frontend: 200 OK — AppUser\nSet-Cookie: fa_session=;\n Path=/; HttpOnly; SameSite=Strict; Secure +Frontend -> Frontend: Parse Set-Cookie, re-emit fa_session\n(matches backend attrs) +Frontend --> Caddy: 303 → /\nSet-Cookie: fa_session= +Caddy --> Browser: HTTPS 303 + Set-Cookie + +== Authenticated request == +Browser -> Caddy: HTTPS GET /\nCookie: fa_session= +Caddy -> Frontend: HTTP GET / + Cookie + X-Forwarded-Proto: https +Frontend -> Frontend: hooks.server.ts reads fa_session +Frontend -> Backend: GET /api/users/me\nCookie: fa_session= +Backend -> DB: SELECT * FROM spring_session\nWHERE SESSION_ID = ? +DB --> Backend: row (or null if expired) +alt Session valid + Backend -> DB: UPDATE spring_session\nSET LAST_ACCESS_TIME = now + Backend --> Frontend: 200 OK — AppUser + Frontend --> Caddy: rendered page + Caddy --> Browser: HTTPS 200 +else Session expired (idle > 8h) or unknown + Backend --> Frontend: 401 Unauthorized + Frontend -> Frontend: hooks: delete fa_session cookie + Frontend --> Caddy: 302 → /login?reason=expired + Caddy --> Browser: HTTPS 302 +end + +== Logout == +Browser -> Caddy: HTTPS POST /logout +Caddy -> Frontend: HTTP POST /logout\nCookie: fa_session= +Frontend -> Backend: POST /api/auth/logout\nCookie: fa_session= +Backend -> Backend: session.invalidate()\nSecurityContextHolder.clearContext() +Backend -> DB: DELETE FROM spring_session\nWHERE SESSION_ID = ? +Backend -> Backend: AuditService.log(LOGOUT,\n {userId, ip, ua}) +Backend --> Frontend: 204 No Content +Frontend -> Frontend: cookies.delete('fa_session') +Frontend --> Caddy: 303 → /login +Caddy --> Browser: HTTPS 303 (cookie cleared) @enduml -- 2.49.1 From 17b29edd14ecddf23bde22a6d225fff6a97cd982 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:36:33 +0200 Subject: [PATCH 17/32] fix(auth): rotate session ID on login to prevent session fixation (CWE-384) 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 --- .../auth/AuthSessionController.java | 17 +++++++++++---- .../security/SecurityConfig.java | 10 +++++++++ .../auth/AuthSessionControllerTest.java | 21 +++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java index 8eca4877..f9ba62ad 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java @@ -9,6 +9,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.web.bind.annotation.*; @@ -20,23 +21,31 @@ import org.springframework.web.bind.annotation.*; public class AuthSessionController { private final AuthService authService; + private final SessionAuthenticationStrategy sessionAuthenticationStrategy; @PostMapping("/login") public ResponseEntity login( @RequestBody LoginRequest request, - HttpServletRequest httpRequest) { + HttpServletRequest httpRequest, + HttpServletResponse httpResponse) { String ip = resolveClientIp(httpRequest); String ua = resolveUserAgent(httpRequest); AuthService.LoginResult result = authService.login(request.email(), request.password(), ip, ua); - // Establish server-side session. Spring Session JDBC intercepts getSession().setAttribute() - // and persists the record; the Set-Cookie: fa_session= is added automatically. + // Session-fixation defense (CWE-384): rotate the session ID at the authentication + // boundary. ChangeSessionIdAuthenticationStrategy invalidates any pre-auth session ID + // an attacker may have planted and mints a fresh one before we attach the SecurityContext. + httpRequest.getSession(true); + sessionAuthenticationStrategy.onAuthentication(result.authentication(), httpRequest, httpResponse); + + // Spring Session JDBC intercepts setAttribute() and persists the record under the + // (now rotated) opaque ID; the Set-Cookie: fa_session= is added automatically. SecurityContext context = SecurityContextHolder.createEmptyContext(); context.setAuthentication(result.authentication()); SecurityContextHolder.setContext(context); - httpRequest.getSession(true) + httpRequest.getSession() .setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, context); return ResponseEntity.ok(result.user()); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java index 8828e610..80747a8f 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java @@ -17,6 +17,8 @@ import org.springframework.security.config.annotation.web.configurers.AbstractHt import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.session.ChangeSessionIdAuthenticationStrategy; +import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; @Configuration @EnableWebSecurity @@ -43,6 +45,14 @@ public class SecurityConfig { return config.getAuthenticationManager(); } + @Bean + public SessionAuthenticationStrategy sessionAuthenticationStrategy() { + // ChangeSessionIdAuthenticationStrategy rotates the session ID via the Servlet 3.1+ + // HttpServletRequest.changeSessionId() — preserves attributes, mints a fresh ID. + // Used by AuthSessionController.login to defend against session fixation (CWE-384). + return new ChangeSessionIdAuthenticationStrategy(); + } + @Bean @Order(1) public SecurityFilterChain managementFilterChain(HttpSecurity http) throws Exception { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java index 037f1ec8..d948dfab 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java @@ -14,12 +14,14 @@ import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; import org.springframework.context.annotation.Import; import org.springframework.http.MediaType; import org.springframework.security.core.Authentication; +import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; import java.util.UUID; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -33,6 +35,7 @@ class AuthSessionControllerTest { @MockitoBean AuthService authService; @MockitoBean CustomUserDetailsService customUserDetailsService; + @MockitoBean SessionAuthenticationStrategy sessionAuthenticationStrategy; // ─── POST /api/auth/login ────────────────────────────────────────────────── @@ -79,6 +82,24 @@ class AuthSessionControllerTest { .andExpect(status().isOk()); } + @Test + void login_delegates_to_SessionAuthenticationStrategy_for_fixation_protection() throws Exception { + UUID userId = UUID.randomUUID(); + AppUser appUser = AppUser.builder().id(userId).email("fix@test.de").build(); + Authentication auth = mock(Authentication.class); + when(authService.login(anyString(), anyString(), anyString(), anyString())) + .thenReturn(new LoginResult(appUser, auth)); + + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"fix@test.de\",\"password\":\"pass\"}")) + .andExpect(status().isOk()); + + // Session-fixation defense (CWE-384): the controller must hand the new + // Authentication to Spring Security's strategy, which rotates the session ID. + verify(sessionAuthenticationStrategy).onAuthentication(eq(auth), any(), any()); + } + @Test void login_does_not_set_cookie_on_failure() throws Exception { when(authService.login(anyString(), anyString(), anyString(), anyString())) -- 2.49.1 From ea656116907638af3ee0c7cf0d2c6d5c6b7897ed Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:38:57 +0200 Subject: [PATCH 18/32] fix(auth): logout invalidates session before audit (CWE-613) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../auth/AuthSessionController.java | 15 +++++++++++++-- .../auth/AuthSessionControllerTest.java | 12 ++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java index f9ba62ad..18c8b4d3 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java @@ -4,6 +4,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.raddatz.familienarchiv.user.AppUser; import org.springframework.http.ResponseEntity; import org.springframework.security.core.Authentication; @@ -18,6 +19,7 @@ import org.springframework.web.bind.annotation.*; @RestController @RequestMapping("/api/auth") @RequiredArgsConstructor +@Slf4j public class AuthSessionController { private final AuthService authService; @@ -53,17 +55,26 @@ public class AuthSessionController { @PostMapping("/logout") public ResponseEntity logout(Authentication authentication, HttpServletRequest httpRequest) { + String email = authentication.getName(); String ip = resolveClientIp(httpRequest); String ua = resolveUserAgent(httpRequest); - authService.logout(authentication.getName(), ip, ua); - + // CWE-613 defense: invalidate the session first — that is the contract the user + // is relying on when they click "Log out." Audit is best-effort and must not + // bubble up: if the user record was deleted while the session was live, the + // audit lookup throws, but the session row in spring_session must still die. HttpSession session = httpRequest.getSession(false); if (session != null) { session.invalidate(); } SecurityContextHolder.clearContext(); + try { + authService.logout(email, ip, ua); + } catch (Exception ex) { + log.warn("Audit logout failed for {}; session was already invalidated", email, ex); + } + return ResponseEntity.noContent().build(); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java index d948dfab..45b96d11 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java @@ -129,4 +129,16 @@ class AuthSessionControllerTest { mockMvc.perform(post("/api/auth/logout")) .andExpect(status().isUnauthorized()); } + + @Test + void logout_returns_204_even_when_audit_throws() throws Exception { + // CWE-613 defense: the session MUST be invalidated even if the audit lookup + // explodes (e.g. user deleted between login and logout). Audit is best-effort. + doThrow(new RuntimeException("audit DB down")) + .when(authService).logout(anyString(), anyString(), anyString()); + + mockMvc.perform(post("/api/auth/logout") + .with(user("ghost@test.de"))) + .andExpect(status().isNoContent()); + } } -- 2.49.1 From c7782d554f9fbef7ea847b5fd6e6ae4812c23a69 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:40:41 +0200 Subject: [PATCH 19/32] test(auth): login response never leaks the password field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin the @JsonProperty(WRITE_ONLY) invariant on AppUser.password. If the annotation is ever dropped — or a new field aliases the hash — the CI run that ships the regression flags it the next morning rather than waiting for a security review. Addresses PR #612 / Nora concern (regression test). Co-Authored-By: Claude Opus 4.7 --- .../auth/AuthSessionControllerTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java index 45b96d11..7ace8c45 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java @@ -100,6 +100,31 @@ class AuthSessionControllerTest { verify(sessionAuthenticationStrategy).onAuthentication(eq(auth), any(), any()); } + @Test + void login_response_body_does_not_contain_password_field() throws Exception { + // Regression guard: AppUser.password is @JsonProperty(WRITE_ONLY). If anyone + // ever drops that annotation, this assertion catches the credential leak on + // the very next CI run. + UUID userId = UUID.randomUUID(); + AppUser appUser = AppUser.builder() + .id(userId) + .email("leak@test.de") + .password("$2a$10$shouldnotappearinresponse") + .build(); + Authentication auth = mock(Authentication.class); + when(authService.login(anyString(), anyString(), anyString(), anyString())) + .thenReturn(new LoginResult(appUser, auth)); + + mockMvc.perform(post("/api/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"leak@test.de\",\"password\":\"pass\"}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.password").doesNotExist()) + .andExpect(jsonPath("$.pwd").doesNotExist()) + .andExpect(content().string(org.hamcrest.Matchers.not( + org.hamcrest.Matchers.containsString("$2a$10$shouldnotappearinresponse")))); + } + @Test void login_does_not_set_cookie_on_failure() throws Exception { when(authService.login(anyString(), anyString(), anyString(), anyString())) -- 2.49.1 From 20fe83d88962d34a3372cd0eb958ab3df8350002 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:41:39 +0200 Subject: [PATCH 20/32] docs(auth): document XFF trust-the-proxy assumption on resolveClientIp 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 --- .../familienarchiv/auth/AuthSessionController.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java index 18c8b4d3..a7f119a9 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java @@ -78,6 +78,15 @@ public class AuthSessionController { return ResponseEntity.noContent().build(); } + /** + * Resolves the client IP for audit-log purposes. + * + *

Trust model: the leftmost {@code X-Forwarded-For} value is taken at face value. + * This is correct only if the ingress (Caddy in production) strips any + * client-supplied XFF before forwarding — otherwise an attacker can pin audit-log + * IPs to whatever they want. Verify the reverse-proxy config before exposing this + * service behind a different ingress. + */ private static String resolveClientIp(HttpServletRequest request) { String forwarded = request.getHeader("X-Forwarded-For"); if (forwarded != null && !forwarded.isBlank()) { -- 2.49.1 From b607677f3063d3a208dafaa6bee611c911a8cb7f Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:43:09 +0200 Subject: [PATCH 21/32] refactor(auth): extract extractFaSessionId to \$lib/shared/cookies Move the Set-Cookie parser out of login/+page.server.ts into a shared module with its own Vitest coverage (single-header, multi-header getSetCookie path, missing-header, attribute-stripping, prefix-match-rejection). An Undici or Node upgrade that changes header shape now trips its own test instead of silently breaking login. Addresses PR #612 / Felix F2. Co-Authored-By: Claude Opus 4.7 --- frontend/src/lib/shared/cookies.spec.ts | 38 +++++++++++++++++++++++++ frontend/src/lib/shared/cookies.ts | 20 +++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 frontend/src/lib/shared/cookies.spec.ts create mode 100644 frontend/src/lib/shared/cookies.ts diff --git a/frontend/src/lib/shared/cookies.spec.ts b/frontend/src/lib/shared/cookies.spec.ts new file mode 100644 index 00000000..64bb9b2c --- /dev/null +++ b/frontend/src/lib/shared/cookies.spec.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest'; +import { extractFaSessionId } from './cookies'; + +describe('extractFaSessionId', () => { + it('extracts the opaque id from a single Set-Cookie header', () => { + const headers = ['fa_session=abc123; Path=/; HttpOnly; SameSite=Strict']; + expect(extractFaSessionId(headers)).toBe('abc123'); + }); + + it('extracts the value when multiple Set-Cookie headers are present (getSetCookie path)', () => { + const headers = [ + 'JSESSIONID=legacy; Path=/', + 'fa_session=xyz789; Path=/; Max-Age=28800; HttpOnly', + 'XSRF-TOKEN=ignored; Path=/' + ]; + expect(extractFaSessionId(headers)).toBe('xyz789'); + }); + + it('returns null when no header carries fa_session', () => { + expect(extractFaSessionId(['Other=foo; Path=/'])).toBeNull(); + }); + + it('returns null for an empty header list', () => { + expect(extractFaSessionId([])).toBeNull(); + }); + + it('strips all attributes after the first semicolon', () => { + const headers = ['fa_session=opaque-token-with.dots_and-dashes; Path=/; Secure; HttpOnly']; + expect(extractFaSessionId(headers)).toBe('opaque-token-with.dots_and-dashes'); + }); + + it('only matches a cookie whose name is exactly fa_session', () => { + // A different cookie name that happens to contain "fa_session" as a substring + // must not match — anchored to start of header. + const headers = ['xfa_session=should-not-match; Path=/']; + expect(extractFaSessionId(headers)).toBeNull(); + }); +}); diff --git a/frontend/src/lib/shared/cookies.ts b/frontend/src/lib/shared/cookies.ts new file mode 100644 index 00000000..ca71e08d --- /dev/null +++ b/frontend/src/lib/shared/cookies.ts @@ -0,0 +1,20 @@ +/** + * Extracts the fa_session cookie value from a list of Set-Cookie response headers. + * + * The backend may append attributes like `Path`, `HttpOnly`, `SameSite=Strict`, + * `Max-Age`, `Secure`; we only forward the opaque session id — the SvelteKit + * cookies API rewrites the attributes itself when re-emitting to the browser. + * + * Pass the result of `response.headers.getSetCookie()` (modern Node/Undici) or + * a single-element array containing `response.headers.get('set-cookie')` for + * older runtimes that lack `getSetCookie`. + * + * Returns `null` if no fa_session cookie is present. + */ +export function extractFaSessionId(setCookieHeaders: string[]): string | null { + for (const header of setCookieHeaders) { + const match = header.match(/^fa_session=([^;]+)/); + if (match) return match[1]; + } + return null; +} -- 2.49.1 From dd99c5dd74b1a4a4a85949708a7cc8a45735393f Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:44:30 +0200 Subject: [PATCH 22/32] refactor(auth): login action imports extractFaSessionId from \$lib/shared/cookies Drop the inline parser; reuse the now-shared helper. Pure rewire, no behaviour change. Addresses PR #612 / Felix F2. Co-Authored-By: Claude Opus 4.7 --- frontend/src/routes/login/+page.server.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/frontend/src/routes/login/+page.server.ts b/frontend/src/routes/login/+page.server.ts index 3f00a76e..244711d0 100644 --- a/frontend/src/routes/login/+page.server.ts +++ b/frontend/src/routes/login/+page.server.ts @@ -1,5 +1,6 @@ import { fail, redirect, type Actions } from '@sveltejs/kit'; import { env } from '$env/dynamic/private'; +import { extractFaSessionId } from '$lib/shared/cookies'; import { getErrorMessage, type ErrorCode } from '$lib/shared/errors'; import type { PageServerLoad } from './$types'; @@ -10,20 +11,6 @@ export const load: PageServerLoad = ({ url }) => { }; }; -/** - * Extracts the fa_session cookie value from a Set-Cookie response header. - * The backend may emit attributes like `Path`, `HttpOnly`, `SameSite=Strict`, `Max-Age`, `Secure`; - * we only forward the opaque session id — the SvelteKit cookies API will rewrite - * the attributes itself. - */ -function extractFaSessionId(setCookieHeaders: string[]): string | null { - for (const header of setCookieHeaders) { - const match = header.match(/^fa_session=([^;]+)/); - if (match) return match[1]; - } - return null; -} - export const actions = { login: async ({ request, cookies, fetch, url }) => { const data = await request.formData(); -- 2.49.1 From 9f1e2c9ff52c3c86a0cd566c9cb668de476d327c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:45:46 +0200 Subject: [PATCH 23/32] refactor(auth): hooks.server.ts re-throws redirects via isRedirect() Replace the duck-typed `status in error && location in error` check with the official SvelteKit guard. Fragile against minor-version error-shape changes becomes a one-liner against a typed helper. Addresses PR #612 / Felix F1. Co-Authored-By: Claude Opus 4.7 --- frontend/src/hooks.server.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frontend/src/hooks.server.ts b/frontend/src/hooks.server.ts index 5fe167c1..de71e2b9 100644 --- a/frontend/src/hooks.server.ts +++ b/frontend/src/hooks.server.ts @@ -1,5 +1,5 @@ import * as Sentry from '@sentry/sveltekit'; -import { redirect, type Handle, type HandleFetch } from '@sveltejs/kit'; +import { isRedirect, redirect, type Handle, type HandleFetch } from '@sveltejs/kit'; import { paraglideMiddleware } from '$lib/paraglide/server'; import { sequence } from '@sveltejs/kit/hooks'; import { env } from 'process'; @@ -87,10 +87,9 @@ const userGroup: Handle = async ({ event, resolve }) => { } } } catch (error) { - // Don't swallow SvelteKit redirects — they're thrown as objects with a `status` field. - if (error instanceof Object && 'status' in error && 'location' in error) { - throw error; - } + // Re-throw SvelteKit redirects (e.g. the /login?reason=expired throw above) + // using the official guard rather than duck-typing on the error shape. + if (isRedirect(error)) throw error; console.error('Error fetching user in hook:', error); } -- 2.49.1 From 2779502f3bae7f88614df6f27a72d754e8731352 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:48:16 +0200 Subject: [PATCH 24/32] test(auth): Vitest coverage for login action Six tests covering: load() exposes ?registered and ?reason; action returns 400 on missing email; 401 with INVALID_CREDENTIALS on backend reject; success re-emits fa_session and deletes legacy auth_token; 500 when backend omits fa_session in Set-Cookie. Closes the frontend coverage gap on the credential- handling logic that moved out of the Java side. Addresses PR #612 / Sara S1. Co-Authored-By: Claude Opus 4.7 --- frontend/src/routes/login/page.server.test.ts | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 frontend/src/routes/login/page.server.test.ts diff --git a/frontend/src/routes/login/page.server.test.ts b/frontend/src/routes/login/page.server.test.ts new file mode 100644 index 00000000..fa0878dd --- /dev/null +++ b/frontend/src/routes/login/page.server.test.ts @@ -0,0 +1,117 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('$env/dynamic/private', () => ({ + env: { API_INTERNAL_URL: 'http://backend:8080' } +})); + +import { actions, load } from './+page.server'; + +type ActionsRecord = Record unknown>; + +function makeRequest(form: Record): Request { + const fd = new FormData(); + for (const [k, v] of Object.entries(form)) fd.set(k, v); + return new Request('http://localhost/login?/login', { method: 'POST', body: fd }); +} + +function makeCookies() { + return { + set: vi.fn(), + delete: vi.fn(), + get: vi.fn() + }; +} + +function loadEvent(search: string) { + return { + url: new URL(`http://localhost/login${search}`), + request: new Request('http://localhost/login', { method: 'GET' }), + route: { id: '/login' } + } as never; +} + +describe('login load', () => { + it('exposes registered=true when ?registered=1 is present', async () => { + const result = await load(loadEvent('?registered=1')); + expect(result).toEqual({ registered: true, reason: null }); + }); + + it('exposes reason=expired when ?reason=expired is present', async () => { + const result = await load(loadEvent('?reason=expired')); + expect(result).toEqual({ registered: false, reason: 'expired' }); + }); +}); + +describe('login action', () => { + beforeEach(() => vi.restoreAllMocks()); + + it('returns 400 when email is missing', async () => { + const result = await (actions as ActionsRecord).login({ + request: makeRequest({ password: 'pw' }), + cookies: makeCookies(), + fetch: vi.fn(), + url: new URL('http://localhost/login') + } as never); + expect((result as { status: number }).status).toBe(400); + }); + + it('returns 401 with INVALID_CREDENTIALS when the backend rejects credentials', async () => { + const mockFetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ code: 'INVALID_CREDENTIALS' }), { + status: 401, + headers: { 'Content-Type': 'application/json' } + }) + ); + + const result = await (actions as ActionsRecord).login({ + request: makeRequest({ email: 'a@b.de', password: 'wrong' }), + cookies: makeCookies(), + fetch: mockFetch, + url: new URL('http://localhost/login') + } as never); + + expect((result as { status: number }).status).toBe(401); + }); + + it('re-emits fa_session and deletes legacy auth_token on success', async () => { + const mockFetch = vi.fn().mockResolvedValue( + new Response('{}', { + status: 200, + headers: { 'Set-Cookie': 'fa_session=opaque-id; Path=/; HttpOnly; SameSite=Strict' } + }) + ); + const cookies = makeCookies(); + + // redirect() throws a Redirect instance — assert via rejects. + const redirected = (actions as ActionsRecord).login({ + request: makeRequest({ email: 'a@b.de', password: 'pw' }), + cookies, + fetch: mockFetch, + url: new URL('http://localhost/login') + } as never); + + await expect(redirected).rejects.toMatchObject({ status: 303, location: '/' }); + + expect(cookies.set).toHaveBeenCalledWith( + 'fa_session', + 'opaque-id', + expect.objectContaining({ httpOnly: true, sameSite: 'strict', maxAge: 60 * 60 * 8 }) + ); + expect(cookies.delete).toHaveBeenCalledWith('auth_token', { path: '/' }); + }); + + it('returns 500 when backend response omits fa_session cookie', async () => { + const mockFetch = vi.fn().mockResolvedValue(new Response('{}', { status: 200 })); + const cookies = makeCookies(); + + const result = await (actions as ActionsRecord).login({ + request: makeRequest({ email: 'a@b.de', password: 'pw' }), + cookies, + fetch: mockFetch, + url: new URL('http://localhost/login') + } as never); + + expect((result as { status: number }).status).toBe(500); + expect(cookies.set).not.toHaveBeenCalled(); + }); +}); -- 2.49.1 From d64139d9d14d4d4ee043ef01be89261027c4ecc5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:49:06 +0200 Subject: [PATCH 25/32] test(auth): Vitest coverage for logout action Three tests: happy path POSTs to backend with the session cookie and clears both fa_session and legacy auth_token; cookies are cleared even when the backend call rejects (best-effort logout); skips the backend call when no session cookie is present. Addresses PR #612 / Sara S1. Co-Authored-By: Claude Opus 4.7 --- .../src/routes/logout/page.server.test.ts | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 frontend/src/routes/logout/page.server.test.ts diff --git a/frontend/src/routes/logout/page.server.test.ts b/frontend/src/routes/logout/page.server.test.ts new file mode 100644 index 00000000..2366bbd7 --- /dev/null +++ b/frontend/src/routes/logout/page.server.test.ts @@ -0,0 +1,63 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('$env/dynamic/private', () => ({ + env: { API_INTERNAL_URL: 'http://backend:8080' } +})); + +import { actions } from './+page.server'; + +type ActionsRecord = Record unknown>; + +function makeCookies(sessionId?: string) { + return { + get: vi.fn((name: string) => (name === 'fa_session' ? sessionId : undefined)), + set: vi.fn(), + delete: vi.fn() + }; +} + +describe('logout action', () => { + beforeEach(() => vi.restoreAllMocks()); + + it('calls backend /api/auth/logout with the session cookie and redirects to /login', async () => { + const mockFetch = vi.fn().mockResolvedValue(new Response(null, { status: 204 })); + const cookies = makeCookies('opaque-id'); + + await expect( + (actions as ActionsRecord).default({ cookies, fetch: mockFetch } as never) + ).rejects.toMatchObject({ status: 303, location: '/login' }); + + expect(mockFetch).toHaveBeenCalledWith( + 'http://backend:8080/api/auth/logout', + expect.objectContaining({ + method: 'POST', + headers: { Cookie: 'fa_session=opaque-id' } + }) + ); + expect(cookies.delete).toHaveBeenCalledWith('fa_session', { path: '/' }); + expect(cookies.delete).toHaveBeenCalledWith('auth_token', { path: '/' }); + }); + + it('clears cookies even when the backend logout call fails', async () => { + const mockFetch = vi.fn().mockRejectedValue(new Error('connection refused')); + const cookies = makeCookies('opaque-id'); + + await expect( + (actions as ActionsRecord).default({ cookies, fetch: mockFetch } as never) + ).rejects.toMatchObject({ status: 303, location: '/login' }); + + expect(cookies.delete).toHaveBeenCalledWith('fa_session', { path: '/' }); + }); + + it('skips the backend call when no session cookie is present', async () => { + const mockFetch = vi.fn(); + const cookies = makeCookies(undefined); + + await expect( + (actions as ActionsRecord).default({ cookies, fetch: mockFetch } as never) + ).rejects.toMatchObject({ status: 303, location: '/login' }); + + expect(mockFetch).not.toHaveBeenCalled(); + expect(cookies.delete).toHaveBeenCalledWith('fa_session', { path: '/' }); + }); +}); -- 2.49.1 From 1f4e8a5958db429209587af954c5bfced3729c57 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:50:41 +0200 Subject: [PATCH 26/32] test(auth): userGroup hook redirect + cookie cleanup coverage Four new tests against the composed handle (with sequence stubbed to return the head function): backend 401 on a private path redirects to /login?reason=expired; backend 401 on /login does NOT redirect (no loop); missing fa_session passes through without a backend call; 200 attaches the user to event.locals. Closes the hook-coverage gap flagged by Sara S1. Co-Authored-By: Claude Opus 4.7 --- frontend/src/hooks.server.test.ts | 96 ++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/frontend/src/hooks.server.test.ts b/frontend/src/hooks.server.test.ts index b24f2e16..37423ded 100644 --- a/frontend/src/hooks.server.test.ts +++ b/frontend/src/hooks.server.test.ts @@ -6,11 +6,22 @@ vi.mock('@sentry/sveltekit', () => ({ lastEventId: vi.fn(() => 'sentry-event-id-abc123') })); -vi.mock('@sveltejs/kit', () => ({ redirect: vi.fn() })); +class RedirectMarker { + constructor( + public status: number, + public location: string + ) {} +} + +vi.mock('@sveltejs/kit', () => ({ + redirect: vi.fn((status: number, location: string) => new RedirectMarker(status, location)), + isRedirect: (e: unknown) => e instanceof RedirectMarker +})); vi.mock('@sveltejs/kit/hooks', () => ({ sequence: vi.fn((...fns: unknown[]) => fns[0]) })); vi.mock('$lib/paraglide/server', () => ({ paraglideMiddleware: vi.fn() })); vi.mock('$lib/paraglide/runtime', () => ({ cookieName: 'locale', cookieMaxAge: 86400 })); vi.mock('$lib/shared/server/locale', () => ({ detectLocale: vi.fn(() => 'de') })); +vi.mock('process', () => ({ env: { API_INTERNAL_URL: 'http://backend:8080' } })); const makeEvent = () => ({ url: { pathname: '/documents/123' }, @@ -56,3 +67,86 @@ describe('hooks.server handleError', () => { expect(result.message).toBe('An unexpected error occurred'); }); }); + +interface UserGroupEvent { + url: URL; + cookies: { get: ReturnType; delete: ReturnType }; + locals: { user?: unknown }; + request: Request; +} + +function makeUserGroupEvent(pathname: string, sessionId?: string): UserGroupEvent { + return { + url: new URL(`http://localhost${pathname}`), + cookies: { + get: vi.fn((name: string) => (name === 'fa_session' ? sessionId : undefined)), + delete: vi.fn() + }, + locals: {}, + request: new Request(`http://localhost${pathname}`) + }; +} + +describe('hooks.server userGroup (session lookup + 401 handling)', () => { + beforeEach(() => { + vi.resetModules(); + vi.stubGlobal('fetch', vi.fn()); + }); + + it('redirects to /login?reason=expired when backend rejects the session on a non-public path', async () => { + vi.mocked(fetch).mockResolvedValue(new Response(null, { status: 401 })); + + const { handle } = await import('./hooks.server'); + const event = makeUserGroupEvent('/documents/123', 'stale-session'); + const resolve = vi.fn(); + + await expect((handle as (a: unknown) => unknown)({ event, resolve })).rejects.toMatchObject({ + status: 302, + location: '/login?reason=expired' + }); + + expect(event.cookies.delete).toHaveBeenCalledWith('fa_session', { path: '/' }); + expect(resolve).not.toHaveBeenCalled(); + }); + + it('does not redirect when backend 401 fires on a public path (no /login → /login loop)', async () => { + vi.mocked(fetch).mockResolvedValue(new Response(null, { status: 401 })); + + const { handle } = await import('./hooks.server'); + const event = makeUserGroupEvent('/login', 'stale-session'); + const resolve = vi.fn().mockResolvedValue(new Response()); + + await (handle as (a: unknown) => Promise)({ event, resolve }); + + expect(event.cookies.delete).toHaveBeenCalledWith('fa_session', { path: '/' }); + expect(resolve).toHaveBeenCalledWith(event); + }); + + it('passes through when no fa_session cookie is present', async () => { + const { handle } = await import('./hooks.server'); + const event = makeUserGroupEvent('/documents/123', undefined); + const resolve = vi.fn().mockResolvedValue(new Response()); + + await (handle as (a: unknown) => Promise)({ event, resolve }); + + expect(fetch).not.toHaveBeenCalled(); + expect(resolve).toHaveBeenCalledWith(event); + }); + + it('attaches the user to locals when backend returns 200', async () => { + vi.mocked(fetch).mockResolvedValue( + new Response(JSON.stringify({ id: 'u1', email: 'a@b.de' }), { + status: 200, + headers: { 'Content-Type': 'application/json' } + }) + ); + + const { handle } = await import('./hooks.server'); + const event = makeUserGroupEvent('/documents/123', 'valid-session'); + const resolve = vi.fn().mockResolvedValue(new Response()); + + await (handle as (a: unknown) => Promise)({ event, resolve }); + + expect((event.locals as { user: { email: string } }).user.email).toBe('a@b.de'); + }); +}); -- 2.49.1 From 4f1594390ec656002d6d5bba480aaf36f5a0a285 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:51:13 +0200 Subject: [PATCH 27/32] style(login): banner uses the text-warning semantic token Replace text-amber-900/text-amber-800 with the existing --color-warning utility from layout.css. The amber soft fill stays (matching the precedent of the green "registered" banner; a full surface-token pair is out of scope for this PR). Addresses PR #612 / Leonie L1. Co-Authored-By: Claude Opus 4.7 --- frontend/src/routes/login/+page.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/routes/login/+page.svelte b/frontend/src/routes/login/+page.svelte index 49550710..ad01e5a0 100644 --- a/frontend/src/routes/login/+page.svelte +++ b/frontend/src/routes/login/+page.svelte @@ -47,8 +47,8 @@ let { aria-live="polite" class="mb-5 rounded-sm border border-amber-200 bg-amber-50 px-4 py-3 font-sans" > -

{m.error_session_expired()}

-

{m.error_session_expired_explainer()}

+

{m.error_session_expired()}

+

{m.error_session_expired_explainer()}

{/if} -- 2.49.1 From e10090b9efc9bffe6166439961756d9a3830f439 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:51:48 +0200 Subject: [PATCH 28/32] style(login): banner has an aria-hidden warning icon Color-blind reader cohort (8% of men) on a phone in sunlight cannot rely on amber alone to parse the banner as a warning. Add a Heroicons-style exclamation-triangle SVG, aria-hidden because the heading text already conveys the meaning to assistive tech. Addresses PR #612 / Leonie L2. Co-Authored-By: Claude Opus 4.7 --- frontend/src/routes/login/+page.svelte | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/frontend/src/routes/login/+page.svelte b/frontend/src/routes/login/+page.svelte index ad01e5a0..7771b7e7 100644 --- a/frontend/src/routes/login/+page.svelte +++ b/frontend/src/routes/login/+page.svelte @@ -45,10 +45,24 @@ let {
-

{m.error_session_expired()}

-

{m.error_session_expired_explainer()}

+ +
+

{m.error_session_expired()}

+

{m.error_session_expired_explainer()}

+
{/if} -- 2.49.1 From 17d9328c6225f73074a317ecfd687cf335eca126 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:52:19 +0200 Subject: [PATCH 29/32] style(login): banner body text raised from text-xs to text-sm text-xs (12px) is below Leonie's body-copy floor for the senior reader cohort who hit /login?reason=expired on a phone in sunlight after being logged out. text-sm (14px) restores legibility without breaking the visual hierarchy with the heading. Addresses PR #612 / Leonie L3. Co-Authored-By: Claude Opus 4.7 --- frontend/src/routes/login/+page.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/routes/login/+page.svelte b/frontend/src/routes/login/+page.svelte index 7771b7e7..b2ee6b2e 100644 --- a/frontend/src/routes/login/+page.svelte +++ b/frontend/src/routes/login/+page.svelte @@ -60,8 +60,8 @@ let { />
-

{m.error_session_expired()}

-

{m.error_session_expired_explainer()}

+

{m.error_session_expired()}

+

{m.error_session_expired_explainer()}

{/if} -- 2.49.1 From 97a2dd8743e8eeb9a8bf61c5d8ccbff3083c41ad Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:53:17 +0200 Subject: [PATCH 30/32] docs(claude): add auth/ package row, drop auth-controllers from user/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #523 moved login/logout into a new auth/ package (AuthSessionController, AuthService, LoginRequest) — register the row in both CLAUDE.md trees alphabetically and strip the stale "auth controllers" line from the user/ description so the next LLM reading either file finds the right home. Addresses PR #612 / Markus M1. Co-Authored-By: Claude Opus 4.7 --- CLAUDE.md | 3 ++- backend/CLAUDE.md | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d3acc1a2..6481eb8f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,6 +77,7 @@ npm run generate:api # Regenerate TypeScript API types from OpenAPI spec ``` backend/src/main/java/org/raddatz/familienarchiv/ ├── audit/ Audit logging +├── auth/ AuthService, AuthSessionController, LoginRequest (Spring Session JDBC) ├── config/ Infrastructure config (Minio, Async, Web) ├── dashboard/ Dashboard analytics + StatsController/StatsService ├── document/ Document domain (entities, controller, service, repository, DTOs) @@ -93,7 +94,7 @@ backend/src/main/java/org/raddatz/familienarchiv/ │ └── relationship/ PersonRelationship sub-domain ├── security/ SecurityConfig, Permission, @RequirePermission, PermissionAspect ├── tag/ Tag domain -└── user/ User domain — AppUser, UserGroup, UserService, auth controllers +└── user/ User domain — AppUser, UserGroup, UserService ``` ### Layering Rules diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index 69d2f154..5a0e281a 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -24,6 +24,7 @@ Spring Boot 4.0 monolith serving the Familienarchiv REST API. Handles document m ``` src/main/java/org/raddatz/familienarchiv/ ├── audit/ # Audit logging (AuditService, AuditLogQueryService) +├── auth/ # AuthService, AuthSessionController, LoginRequest (Spring Session JDBC — ADR-020) ├── config/ # Infrastructure config (MinioConfig, AsyncConfig, WebConfig) ├── dashboard/ # Dashboard analytics + StatsController/StatsService ├── document/ # Document domain — entities, controller, service, repository, DTOs @@ -40,7 +41,7 @@ src/main/java/org/raddatz/familienarchiv/ │ └── relationship/ # PersonRelationship sub-domain ├── security/ # SecurityConfig, Permission, @RequirePermission, PermissionAspect ├── tag/ # Tag domain — Tag, TagService, TagController -└── user/ # User domain — AppUser, UserGroup, UserService, auth controllers +└── user/ # User domain — AppUser, UserGroup, UserService ``` For per-domain ownership and public surface, see each domain's `README.md`. -- 2.49.1 From e4c8535f42b66dfaa98273b06a799f8a38f519e1 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:54:07 +0200 Subject: [PATCH 31/32] docs(personas): exempt framework-owned tables from DB-diagram updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spring Session JDBC's spring_session/spring_session_attributes (introduced in V67 / ADR-020) and Flyway's own history table are framework-managed and opaque to app code — modelling them on db-orm.puml would mislead future readers into thinking they participate in domain relationships. Codify the exclusion in the doc-currency tables of architect.md and developer.md, with a pointer to "the relevant ADR" so a future exclusion still carries justification. Addresses PR #612 / Markus M2. Co-Authored-By: Claude Opus 4.7 --- .claude/personas/architect.md | 2 +- .claude/personas/developer.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/personas/architect.md b/.claude/personas/architect.md index 6c6d599c..190b391f 100644 --- a/.claude/personas/architect.md +++ b/.claude/personas/architect.md @@ -414,7 +414,7 @@ Never Kafka for teams under 10 or <100k events/day. Never gRPC inside a monolith | PR contains | Required doc update | |---|---| -| New Flyway migration adding/removing/renaming a table or column | `docs/architecture/db/db-orm.puml` and `docs/architecture/db/db-relationships.puml` | +| New Flyway migration adding/removing/renaming a table or column | `docs/architecture/db/db-orm.puml` and `docs/architecture/db/db-relationships.puml` — **except** framework-owned tables (e.g. Spring Session JDBC's `spring_session*`, Flyway's `flyway_schema_history`), which are opaque to app code; reference the relevant ADR if an exclusion is load-bearing | | New `@ManyToMany` join table or FK | Both DB diagrams | | New backend package or domain module | `CLAUDE.md` package table + matching `docs/architecture/c4/l3-backend-*.puml` | | New controller or service in an existing backend domain | Matching `docs/architecture/c4/l3-backend-*.puml` | diff --git a/.claude/personas/developer.md b/.claude/personas/developer.md index 65e14740..ea90ec5a 100644 --- a/.claude/personas/developer.md +++ b/.claude/personas/developer.md @@ -984,7 +984,7 @@ Mark with `@pytest.mark.asyncio` so pytest runs the coroutine. Without it, the t | What changed in code | Doc(s) to update | |---|---| -| New Flyway migration adds/removes/renames a table or column | `docs/architecture/db/db-orm.puml` (add/remove entity or attribute) **and** `docs/architecture/db/db-relationships.puml` (add/remove relationship line) | +| New Flyway migration adds/removes/renames a table or column | `docs/architecture/db/db-orm.puml` (add/remove entity or attribute) **and** `docs/architecture/db/db-relationships.puml` (add/remove relationship line) — **except** framework-owned tables (e.g. Spring Session JDBC's `spring_session*`, Flyway's `flyway_schema_history`), which are opaque to app code; reference the relevant ADR if an exclusion is load-bearing | | New `@ManyToMany` join table or FK relationship | Both DB diagrams above | | New backend package / domain module | `CLAUDE.md` (package structure table) **and** the matching `docs/architecture/c4/l3-backend-*.puml` diagram for that domain | | New Spring Boot controller or service in an existing domain | The matching `docs/architecture/c4/l3-backend-*.puml` for that domain | -- 2.49.1 From 9b21d6aee84570d9736e762c15f58a0dfb428436 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 17 May 2026 22:54:55 +0200 Subject: [PATCH 32/32] docs(c4): l3-security includes auth package and Spring Session JDBC Replace the stale Basic-Auth picture with the post-#523 model: AuthSessionController + AuthService (the new auth/ package), Spring Session JDBC (spring_session*, 8h idle timeout, fa_session cookie), and the ChangeSessionIdAuthenticationStrategy bean used by login to defend against session fixation. Addresses PR #612 / Markus M3. Co-Authored-By: Claude Opus 4.7 --- .../c4/l3-backend-3a-security.puml | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/architecture/c4/l3-backend-3a-security.puml b/docs/architecture/c4/l3-backend-3a-security.puml index 33e41dc9..92135266 100644 --- a/docs/architecture/c4/l3-backend-3a-security.puml +++ b/docs/architecture/c4/l3-backend-3a-security.puml @@ -7,15 +7,24 @@ Container(frontend, "Web Frontend", "SvelteKit") ContainerDb(db, "PostgreSQL", "PostgreSQL 16") System_Boundary(backend, "API Backend (Spring Boot)") { - Component(secFilter, "Security Filter Chain", "Spring Security", "Enforces authentication on all requests. Parses Basic Auth header and constructs an Authentication token; delegates credential validation to DaoAuthenticationProvider via BCrypt. Permits password-reset, invite, and register endpoints without authentication.") - Component(permAspect, "PermissionAspect", "Spring AOP", "Intercepts methods annotated with @RequirePermission. Checks user's granted authorities against the required permission. Throws 401/403 if denied.") - Component(secConf, "SecurityConfig", "Spring @Configuration", "Configures filter chain: all routes require authentication, CSRF disabled, BCrypt password encoder, DaoAuthenticationProvider with CustomUserDetailsService.") - Component(userDetails, "CustomUserDetailsService", "Spring Security UserDetailsService", "Loads AppUser by email from DB. Converts group permissions to Spring GrantedAuthority objects. Logs unknown permissions.") + Component(authCtrl, "AuthSessionController", "@RestController org.raddatz.familienarchiv.auth", "POST /api/auth/login validates credentials, rotates the session ID via SessionAuthenticationStrategy (CWE-384 defense), attaches the SecurityContext to the new session. POST /api/auth/logout invalidates the session unconditionally, then best-effort audits.") + Component(authSvc, "AuthService", "@Service org.raddatz.familienarchiv.auth", "Delegates credential validation to AuthenticationManager (DaoAuthenticationProvider — timing-equalised via dummy BCrypt on misses). Emits LOGIN_SUCCESS / LOGIN_FAILED / LOGOUT audit entries without ever logging the password attempt.") + Component(secFilter, "Security Filter Chain", "Spring Security", "Permits /api/auth/login, /api/auth/forgot-password, /api/auth/reset-password, /api/auth/invite/**, /api/auth/register; everything else requires an authenticated session. Returns 401 (not 302) on missing/expired session. CSRF is disabled pending #524.") + Component(sessionRepo, "Spring Session JDBC", "spring-boot-starter-session-jdbc", "Persists sessions in spring_session / spring_session_attributes (Flyway V67). 8-hour idle timeout. Cookie name fa_session, SameSite=Strict, HttpOnly, Secure behind Caddy. Indexes the session by Principal name for revocation in #524.") + Component(permAspect, "PermissionAspect", "Spring AOP", "Intercepts methods annotated with @RequirePermission. Checks the authenticated user's granted authorities against the required permission. Throws 401/403 if denied.") + Component(secConf, "SecurityConfig", "Spring @Configuration", "Wires the filter chain, BCryptPasswordEncoder, DaoAuthenticationProvider, AuthenticationManager, and the ChangeSessionIdAuthenticationStrategy bean used by AuthSessionController.") + Component(userDetails, "CustomUserDetailsService", "Spring Security UserDetailsService", "Loads AppUser by email from DB. Converts group permissions to Spring GrantedAuthority objects.") } -Rel(frontend, secFilter, "All requests", "HTTP / Basic Auth header") +Rel(frontend, authCtrl, "POST /api/auth/login + /logout", "HTTPS, JSON") +Rel(frontend, secFilter, "All other API calls", "HTTPS + fa_session cookie") +Rel(authCtrl, authSvc, "Validate creds + audit") +Rel(authCtrl, sessionRepo, "getSession() / invalidate()") +Rel(authSvc, userDetails, "Authenticates via AuthenticationManager") +Rel(secFilter, sessionRepo, "Resolves session by fa_session cookie") Rel(secFilter, permAspect, "Authenticated requests reach guarded service methods") Rel(secConf, userDetails, "Wires as UserDetailsService") Rel(userDetails, db, "Loads user by email", "JDBC") +Rel(sessionRepo, db, "spring_session, spring_session_attributes", "JDBC") @enduml -- 2.49.1