From af31d2217949e2a9ddd2794eee487d3dd0a43955 Mon Sep 17 00:00:00 2001 From: Randy Dean Date: Thu, 9 Jul 2026 20:38:58 -0400 Subject: [PATCH] Auth: enable CSRF protection (SPA double-submit pattern) CSRF was disabled on the SameSite=Lax + JSON-only rationale, but with a JDBC-backed session cookie the token machinery is the defense-in-depth norm (and security tooling rightly flags csrf.disable()). - CookieCsrfTokenRepository writes a JS-readable XSRF-TOKEN cookie on every response via SpaCsrfTokenRequestHandler (the Spring-documented SPA handler: BREACH-safe masking, raw header resolution) - apiFetch echoes the cookie as X-XSRF-TOKEN on state-changing requests - Pre-auth endpoints (request-link, verify) are exempt: their only credential is in the body and a first-time visitor has no CSRF cookie yet; logout and everything else require the token - SecurityWiringTest: token-less POST -> 403, logout with token -> 200 Co-Authored-By: Claude Fable 5 --- docs/auth-feature.md | 7 +++- js/src/lib/api/client.ts | 27 +++++++++++- .../auth/security/SecurityConfig.java | 17 ++++---- .../security/SpaCsrfTokenRequestHandler.java | 41 +++++++++++++++++++ .../auth/security/SecurityWiringTest.java | 12 +++++- 5 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 src/main/java/org/patinanetwork/patchats/auth/security/SpaCsrfTokenRequestHandler.java diff --git a/docs/auth-feature.md b/docs/auth-feature.md index ebe6562..8d364c1 100644 --- a/docs/auth-feature.md +++ b/docs/auth-feature.md @@ -56,8 +56,11 @@ js/src/features/auth/ shell accounts; 401 in the envelope when signed out. The frontend `RequireAuth` guard sends signed-out visitors to `/login` and incomplete profiles to `/sign-up`. -Why CSRF protection is off, and why that is safe here, is documented on `SecurityConfig` — keep that -javadoc current if the cookie or CORS posture ever changes. +**CSRF.** Double-submit protection (the Spring-documented SPA pattern): every response sets a +JS-readable `XSRF-TOKEN` cookie, and `apiFetch` echoes it back as an `X-XSRF-TOKEN` header on +state-changing requests. The two pre-auth endpoints (`request-link`, `verify`) are exempt — their +only credential travels in the body, and a first-time visitor has no CSRF cookie yet. Details and +rationale live in `SecurityConfig`'s javadoc. ## Manual test walkthrough (dev) diff --git a/js/src/lib/api/client.ts b/js/src/lib/api/client.ts index 667cbb4..f98b324 100644 --- a/js/src/lib/api/client.ts +++ b/js/src/lib/api/client.ts @@ -6,6 +6,11 @@ * so hooks receive typed payloads and errors carry the server's message. * Auth rides on an httpOnly session cookie, hence `credentials: "same-origin"` * (the SPA and API share an origin — the Vite proxy provides that in dev). + * + * CSRF: the backend sets a JS-readable `XSRF-TOKEN` cookie on every response; + * state-changing requests must echo it back as an `X-XSRF-TOKEN` header + * (double-submit pattern). The pre-auth endpoints (request-link, verify) are + * exempt server-side, so a missing cookie on first visit is fine. */ export class ApiError extends Error { @@ -24,14 +29,34 @@ interface ApiEnvelope { payload?: T; } +function readCookie(name: string): string | undefined { + return document.cookie + .split("; ") + .find((row) => row.startsWith(`${name}=`)) + ?.slice(name.length + 1); +} + +function csrfHeader(method: string): Record { + if (method === "GET" || method === "HEAD") { + return {}; + } + const token = readCookie("XSRF-TOKEN"); + return token ? { "X-XSRF-TOKEN": decodeURIComponent(token) } : {}; +} + export async function apiFetch( path: string, init?: RequestInit, ): Promise { + const method = (init?.method ?? "GET").toUpperCase(); const response = await fetch(`/api${path}`, { credentials: "same-origin", ...init, - headers: { "Content-Type": "application/json", ...init?.headers }, + headers: { + "Content-Type": "application/json", + ...csrfHeader(method), + ...init?.headers, + }, }); const envelope = (await response.json().catch(() => undefined)) as diff --git a/src/main/java/org/patinanetwork/patchats/auth/security/SecurityConfig.java b/src/main/java/org/patinanetwork/patchats/auth/security/SecurityConfig.java index de2c8f6..9050ac5 100644 --- a/src/main/java/org/patinanetwork/patchats/auth/security/SecurityConfig.java +++ b/src/main/java/org/patinanetwork/patchats/auth/security/SecurityConfig.java @@ -14,6 +14,7 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.security.web.context.SecurityContextRepository; +import org.springframework.security.web.csrf.CookieCsrfTokenRepository; import org.springframework.session.web.http.DefaultCookieSerializer; /** @@ -26,12 +27,12 @@ * client-supplied session id, so session fixation is prevented by construction (verify additionally rotates any * pre-existing session id). * - *

Why CSRF protection is disabled. Three layers make the classic attack a no-op here: the session cookie is - * {@code SameSite=Lax}, so browsers do not attach it to cross-site POSTs; every state-changing endpoint only consumes - * {@code application/json} request bodies, which cross-site forms cannot produce and cross-origin {@code fetch} cannot - * send without a CORS preflight (and no CORS mappings exist — the SPA is served same-origin, via the Vite proxy in - * dev); and the one anonymous cookie-consuming POST, logout, is idempotent and harmless. Revisit if the cookie ever - * needs {@code SameSite=None}. + *

CSRF. Double-submit protection via the Spring-documented SPA pattern: {@code CookieCsrfTokenRepository} + * writes a JS-readable {@code XSRF-TOKEN} cookie on every response (see {@link SpaCsrfTokenRequestHandler}), and the + * frontend echoes it back as an {@code X-XSRF-TOKEN} header on state-changing requests. This is defense-in-depth on top + * of the {@code SameSite=Lax} session cookie and the JSON-only request bodies. Two pre-auth endpoints are exempt: + * {@code request-link} and {@code verify} carry their only credential in the body (no session to ride), and a visitor + * landing directly on the login or verify page has not received the CSRF cookie yet. */ @Configuration @EnableConfigurationProperties({AuthProperties.class, SessionProperties.class}) @@ -96,7 +97,9 @@ SecurityFilterChain devSecurityFilterChain(final HttpSecurity http) throws Excep } private HttpSecurity common(final HttpSecurity http) throws Exception { - return http.csrf(AbstractHttpConfigurer::disable) + return http.csrf(csrf -> csrf.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()) + .csrfTokenRequestHandler(new SpaCsrfTokenRequestHandler()) + .ignoringRequestMatchers("/api/auth/request-link", "/api/auth/verify")) .requestCache(AbstractHttpConfigurer::disable) .logout(AbstractHttpConfigurer::disable) .securityContext(context -> context.securityContextRepository(securityContextRepository())) diff --git a/src/main/java/org/patinanetwork/patchats/auth/security/SpaCsrfTokenRequestHandler.java b/src/main/java/org/patinanetwork/patchats/auth/security/SpaCsrfTokenRequestHandler.java new file mode 100644 index 0000000..8b40b30 --- /dev/null +++ b/src/main/java/org/patinanetwork/patchats/auth/security/SpaCsrfTokenRequestHandler.java @@ -0,0 +1,41 @@ +package org.patinanetwork.patchats.auth.security; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.util.function.Supplier; +import org.springframework.security.web.csrf.CsrfToken; +import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler; +import org.springframework.security.web.csrf.CsrfTokenRequestHandler; +import org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler; +import org.springframework.util.StringUtils; + +/** + * The CSRF request handler for single-page apps, straight from the Spring Security reference. Two jobs: + * + *

    + *
  • {@link #handle} applies BREACH protection (XOR masking) to server-rendered token values and resolves the + * deferred token on every request, which makes {@code CookieCsrfTokenRepository} write the readable + * {@code XSRF-TOKEN} cookie the SPA echoes back. + *
  • {@link #resolveCsrfTokenValue} accepts the raw (unmasked) value when it arrives via the {@code X-XSRF-TOKEN} + * header — the SPA copies the cookie verbatim — while still unmasking values submitted as request parameters. + *
+ */ +final class SpaCsrfTokenRequestHandler implements CsrfTokenRequestHandler { + + private final CsrfTokenRequestHandler plain = new CsrfTokenRequestAttributeHandler(); + private final CsrfTokenRequestHandler xor = new XorCsrfTokenRequestAttributeHandler(); + + @Override + public void handle( + final HttpServletRequest request, final HttpServletResponse response, final Supplier csrfToken) { + this.xor.handle(request, response, csrfToken); + // Resolve the deferred token so the repository writes the XSRF-TOKEN cookie on this response. + csrfToken.get(); + } + + @Override + public String resolveCsrfTokenValue(final HttpServletRequest request, final CsrfToken csrfToken) { + final String headerValue = request.getHeader(csrfToken.getHeaderName()); + return (StringUtils.hasText(headerValue) ? this.plain : this.xor).resolveCsrfTokenValue(request, csrfToken); + } +} diff --git a/src/test/java/org/patinanetwork/patchats/auth/security/SecurityWiringTest.java b/src/test/java/org/patinanetwork/patchats/auth/security/SecurityWiringTest.java index 757e53a..d0d5222 100644 --- a/src/test/java/org/patinanetwork/patchats/auth/security/SecurityWiringTest.java +++ b/src/test/java/org/patinanetwork/patchats/auth/security/SecurityWiringTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -63,10 +64,17 @@ void requestLinkIsReachableAnonymously() throws Exception { void emailEndpointStaysAdminOnlyAndFailsClosed() throws Exception { mockMvc.perform(post("/api/email/send") .contentType(MediaType.APPLICATION_JSON) - .content("{}")) + .content("{}") + .with(csrf())) .andExpect(status().isUnauthorized()); } + @Test + void stateChangingPostWithoutCsrfTokenIsForbidden() throws Exception { + // Logout is not in the CSRF-exempt set: it consumes the session cookie, so it needs the token. + mockMvc.perform(post("/api/auth/logout")).andExpect(status().isForbidden()); + } + @Test void verifyEstablishesASessionThatAuthenticatesLaterRequests() throws Exception { final MemberAccount member = new MemberAccount(UUID.randomUUID(), "ann@example.com", null, null); @@ -100,7 +108,7 @@ void logoutInvalidatesTheSession() throws Exception { final MockHttpSession session = (MockHttpSession) login.getRequest().getSession(false); assertNotNull(session); - mockMvc.perform(post("/api/auth/logout").session(session)).andExpect(status().isOk()); + mockMvc.perform(post("/api/auth/logout").session(session).with(csrf())).andExpect(status().isOk()); assertTrue(session.isInvalid()); } }