From d8dd09ae5468141b0291046109abeed53a0d7db0 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 11 May 2026 17:41:26 +0200 Subject: [PATCH 1/2] fix(security): promote auth_token cookie to Authorization header for browser /api/* calls Closes #520. The login action stores `Basic ` in an HttpOnly `auth_token` cookie. SSR fetches from hooks.server.ts explicitly set the Authorization header. Vite's dev proxy does the same on every /api/* request. Caddy in production does NOT. So browser-side fetch() and EventSource() calls reach the backend without auth, get 401 + WWW-Authenticate: Basic, and the browser pops a native auth dialog over the SPA. Add AuthTokenCookieFilter (Ordered.HIGHEST_PRECEDENCE, before any Spring Security filter) that promotes the cookie to a request header when no explicit Authorization is present. URL-decodes the cookie value because SvelteKit URL-encodes spaces ("Basic " -> "Basic%20") when serializing the cookie. Works the same for REST, SSE (/api/notifications/stream, /api/ocr/jobs/.../progress), and any other browser-direct backend call. 5 tests in AuthTokenCookieFilterTest cover: URL-decoded promotion, explicit-Authorization-wins precedence, no-cookies pass-through, absent-auth-token pass-through, empty-value pass-through. Also: add `@ActiveProfiles("test")` to ThumbnailServiceIntegrationTest, the one remaining @SpringBootTest in the suite that wasn't annotated. After #516 made UserDataInitializer fail-closed outside dev/test/e2e, this test's context load was throwing. Restores green main. Co-Authored-By: Claude Opus 4.7 --- .../security/AuthTokenCookieFilter.java | 109 ++++++++++++++++++ .../ThumbnailServiceIntegrationTest.java | 2 + .../security/AuthTokenCookieFilterTest.java | 97 ++++++++++++++++ 3 files changed, 208 insertions(+) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java create 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 new file mode 100644 index 00000000..1ef6c593 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java @@ -0,0 +1,109 @@ +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. + */ +@Component +@Order(org.springframework.core.Ordered.HIGHEST_PRECEDENCE) +public class AuthTokenCookieFilter extends OncePerRequestFilter { + + static final String COOKIE_NAME = "auth_token"; + + @Override + protected void doFilterInternal(HttpServletRequest request, + HttpServletResponse response, + FilterChain chain) throws ServletException, IOException { + 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().isEmpty()) { + String decoded = URLDecoder.decode(c.getValue(), StandardCharsets.UTF_8); + 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/document/ThumbnailServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceIntegrationTest.java index ce198a7c..cfd7f0f4 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceIntegrationTest.java @@ -10,6 +10,7 @@ import org.raddatz.familienarchiv.document.DocumentStatus; import org.raddatz.familienarchiv.document.DocumentRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; import org.springframework.context.annotation.Import; import org.springframework.test.context.DynamicPropertyRegistry; import org.springframework.test.context.DynamicPropertySource; @@ -41,6 +42,7 @@ import static org.assertj.core.api.Assertions.assertThat; * test pyramid mocks at the FileService boundary. */ @SpringBootTest +@ActiveProfiles("test") @Import(PostgresContainerConfig.class) class ThumbnailServiceIntegrationTest { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java b/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java new file mode 100644 index 00000000..bd301fcd --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java @@ -0,0 +1,97 @@ +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.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.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); + + ArgumentCaptor captor = ArgumentCaptor.forClass(HttpServletRequest.class); + verify(chain).doFilter(captor.capture(), org.mockito.ArgumentMatchers.any(HttpServletResponse.class)); + assertThat(captor.getValue().getHeader("Authorization")) + .isEqualTo("Basic explicit-header-wins"); + } + + @Test + void passes_through_when_no_cookies_at_all() throws Exception { + MockHttpServletRequest req = new MockHttpServletRequest(); + 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.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.setCookies(new Cookie("auth_token", "")); + MockHttpServletResponse res = new MockHttpServletResponse(); + FilterChain chain = mock(FilterChain.class); + + filter.doFilter(req, res, chain); + + verify(chain).doFilter(req, res); + } +} -- 2.49.1 From 285764fe4281aa0ae6e7821863e5755bd8722f4d Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 11 May 2026 17:49:27 +0200 Subject: [PATCH 2/2] fixup: address Nora's review on #520 (security blockers) - frontend/login: derive cookie `secure` flag from request URL protocol. Pre-PR the cookie was only read by SSR so the flag didn't matter; now the cookie IS the API credential and must be Secure on HTTPS or it leaks a 24h Basic token on plaintext networks. Dev runs over HTTP and would silently lose the cookie if we hardcoded `secure: true`, so the flag follows `event.url.protocol === 'https:'`. - SecurityConfig: rewrite the CSRF-disabled comment. The old "browsers block cross-origin custom headers" justification no longer holds once /api/* is authenticated via the cookie. Make the load-bearing dependencies explicit: SameSite=strict on the auth_token cookie + Spring's default CORS rejection. - AuthTokenCookieFilter: - Scope to /api/* only. /actuator/health and similar must not be cookie-authenticated. - Refuse malformed percent-encoding (URLDecoder throws); forward the request without a promoted Authorization rather than crash. - Use isBlank() instead of isEmpty() per Nora. - Javadoc warning: getHeaderNames/getHeaders exposes the Basic credential; any future header-iterating logger must scrub Authorization before logging. - Tests: add `passes_through_unchanged_when_request_is_outside_api_scope` (/actuator/health with cookie should NOT be wrapped) and `passes_through_unchanged_when_cookie_value_is_malformed_percent_encoding`. Tighten the explicit-header test to verify same-instance forwarding rather than just header equality. Co-Authored-By: Claude Opus 4.7 --- .../security/AuthTokenCookieFilter.java | 32 ++++++++++++- .../security/SecurityConfig.java | 20 ++++++--- .../security/AuthTokenCookieFilterTest.java | 45 +++++++++++++++++-- frontend/src/routes/login/+page.server.ts | 10 ++++- 4 files changed, 93 insertions(+), 14 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java b/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java index 1ef6c593..d382f872 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilter.java @@ -39,17 +39,36 @@ import java.util.Enumeration; * *

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; @@ -60,8 +79,17 @@ public class AuthTokenCookieFilter extends OncePerRequestFilter { return; } for (Cookie c : cookies) { - if (COOKIE_NAME.equals(c.getName()) && c.getValue() != null && !c.getValue().isEmpty()) { - String decoded = URLDecoder.decode(c.getValue(), StandardCharsets.UTF_8); + 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; } 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 86672f80..298d9fa6 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/security/SecurityConfig.java @@ -37,12 +37,20 @@ public class SecurityConfig { @Bean public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { http - // CSRF is intentionally disabled: every request from the SvelteKit frontend - // carries an explicit Authorization header (Basic Auth token injected by - // hooks.server.ts). Browsers block cross-origin requests from setting custom - // headers, so cross-site request forgery via a third-party page is not - // possible with this auth scheme. If the auth model ever changes to - // cookie-based sessions, CSRF protection must be re-enabled. + // CSRF is intentionally disabled. With the cookie-promotion model + // (auth_token cookie → Authorization header via AuthTokenCookieFilter, + // see #520), every authenticated request to /api/* now carries the + // credential automatically once the cookie is set. The CSRF defence + // for state-changing endpoints is therefore LOAD-BEARING on: + // + // 1. SameSite=strict on the auth_token cookie (login/+page.server.ts). + // A cross-site POST from evil.com cannot include the cookie. + // 2. CORS — Spring's default rejects cross-origin requests with + // credentials unless explicitly allowed (no allowedOrigins config). + // + // If either of those is ever weakened (e.g. cookie flipped to + // SameSite=lax, CORS allowedOrigins expanded), CSRF protection + // MUST be re-enabled here. .csrf(csrf -> csrf.disable()) .authorizeHttpRequests(auth -> { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java b/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java index bd301fcd..18ceab49 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/security/AuthTokenCookieFilterTest.java @@ -29,6 +29,7 @@ class AuthTokenCookieFilterTest { @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); @@ -47,6 +48,7 @@ class AuthTokenCookieFilterTest { @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(); @@ -54,15 +56,14 @@ class AuthTokenCookieFilterTest { filter.doFilter(req, res, chain); - ArgumentCaptor captor = ArgumentCaptor.forClass(HttpServletRequest.class); - verify(chain).doFilter(captor.capture(), org.mockito.ArgumentMatchers.any(HttpServletResponse.class)); - assertThat(captor.getValue().getHeader("Authorization")) - .isEqualTo("Basic explicit-header-wins"); + // 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); @@ -74,6 +75,7 @@ class AuthTokenCookieFilterTest { @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); @@ -86,6 +88,7 @@ class AuthTokenCookieFilterTest { @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); @@ -94,4 +97,38 @@ class AuthTokenCookieFilterTest { 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); + } } diff --git a/frontend/src/routes/login/+page.server.ts b/frontend/src/routes/login/+page.server.ts index d3e22098..50aabec5 100644 --- a/frontend/src/routes/login/+page.server.ts +++ b/frontend/src/routes/login/+page.server.ts @@ -8,7 +8,7 @@ export const load: PageServerLoad = ({ url }) => { }; export const actions = { - login: async ({ request, cookies, fetch }) => { + login: async ({ request, cookies, fetch, url }) => { const data = await request.formData(); const email = data.get('email') as string; const password = data.get('password') as string; @@ -37,11 +37,17 @@ export const actions = { return fail(500, { error: getErrorMessage('INTERNAL_ERROR') }); } + // The cookie IS the API credential — promoted to `Authorization: Basic …` + // on every browser → backend request by AuthTokenCookieFilter on the + // Spring side (see #520). It must be Secure on HTTPS or it leaks + // a 24h Basic token on plaintext networks. Dev runs over HTTP and + // would silently lose the cookie if we hardcoded secure=true. + const isHttps = url.protocol === 'https:'; cookies.set('auth_token', authHeader, { path: '/', httpOnly: true, sameSite: 'strict', - secure: false, // set to true when HTTPS is available + secure: isHttps, maxAge: 60 * 60 * 24 }); } catch (e) { -- 2.49.1