From 4495eb63a05c02e6dd5571a978a779f7267041be Mon Sep 17 00:00:00 2001 From: donghyeon-ka Date: Sat, 25 Jul 2026 14:42:33 +0900 Subject: [PATCH] feat(ap3): add CSRF and SameSite defenses --- README.md | 2 + .../keycloakpattern/bff/BffController.java | 2 +- .../keycloakpattern/bff/CsrfController.java | 25 ++++++ .../keycloakpattern/bff/SecurityConfig.java | 9 +- .../bff/SpaCsrfTokenRequestHandler.java | 40 +++++++++ bff/src/main/resources/application.yml | 1 + bff/src/main/resources/static/app.js | 26 +++++- bff/src/main/resources/static/index.html | 2 +- .../bff/BffControllerTest.java | 27 +++++- docs/ap3-bff-boundary.md | 22 +++-- e2e/pattern3.mjs | 90 +++++++++++++++---- 11 files changed, 214 insertions(+), 32 deletions(-) create mode 100644 bff/src/main/java/com/example/keycloakpattern/bff/CsrfController.java create mode 100644 bff/src/main/java/com/example/keycloakpattern/bff/SpaCsrfTokenRequestHandler.java diff --git a/README.md b/README.md index 610c763..f430950 100644 --- a/README.md +++ b/README.md @@ -110,3 +110,5 @@ token을 붙여 Resource Server로 proxy합니다. 자세한 경계와 session 저장소 trade-off는 [`docs/ap3-bff-boundary.md`](docs/ap3-bff-boundary.md)를 참고하세요. +최종 AP3 branch는 `AP3_SESSION; HttpOnly; SameSite=Lax`와 Spring CSRF +token을 함께 사용하며, token 없는 상태 변경 요청은 403으로 거부합니다. diff --git a/bff/src/main/java/com/example/keycloakpattern/bff/BffController.java b/bff/src/main/java/com/example/keycloakpattern/bff/BffController.java index cda0462..cf813fc 100644 --- a/bff/src/main/java/com/example/keycloakpattern/bff/BffController.java +++ b/bff/src/main/java/com/example/keycloakpattern/bff/BffController.java @@ -56,7 +56,7 @@ public class BffController { response.put("refreshTokenStoredOnServer", client != null && client.getRefreshToken() != null); response.put("browserTokenCount", 0); - response.put("csrfProtectionEnabled", false); + response.put("csrfProtectionEnabled", true); return ResponseEntity.ok() .cacheControl(CacheControl.noStore()) diff --git a/bff/src/main/java/com/example/keycloakpattern/bff/CsrfController.java b/bff/src/main/java/com/example/keycloakpattern/bff/CsrfController.java new file mode 100644 index 0000000..fa9a859 --- /dev/null +++ b/bff/src/main/java/com/example/keycloakpattern/bff/CsrfController.java @@ -0,0 +1,25 @@ +package com.example.keycloakpattern.bff; + +import java.util.Map; + +import org.springframework.http.CacheControl; +import org.springframework.http.ResponseEntity; +import org.springframework.security.web.csrf.CsrfToken; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class CsrfController { + + @GetMapping("/bff/csrf") + ResponseEntity> csrf(CsrfToken csrfToken) { + return ResponseEntity.ok() + .cacheControl(CacheControl.noStore()) + .header("Pragma", "no-cache") + .body(Map.of( + "headerName", csrfToken.getHeaderName(), + "parameterName", csrfToken.getParameterName(), + "token", csrfToken.getToken() + )); + } +} diff --git a/bff/src/main/java/com/example/keycloakpattern/bff/SecurityConfig.java b/bff/src/main/java/com/example/keycloakpattern/bff/SecurityConfig.java index 9eebf1c..3d55c11 100644 --- a/bff/src/main/java/com/example/keycloakpattern/bff/SecurityConfig.java +++ b/bff/src/main/java/com/example/keycloakpattern/bff/SecurityConfig.java @@ -12,6 +12,7 @@ import org.springframework.security.oauth2.client.registration.ClientRegistratio import org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizationRequestResolver; import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestCustomizers; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.csrf.CookieCsrfTokenRepository; @Configuration public class SecurityConfig { @@ -30,8 +31,14 @@ public class SecurityConfig { OAuth2AuthorizationRequestCustomizers.withPkce() ); + CookieCsrfTokenRepository csrfTokenRepository = + CookieCsrfTokenRepository.withHttpOnlyFalse(); + csrfTokenRepository.setCookiePath("/"); + return http - .csrf(csrf -> csrf.disable()) + .csrf(csrf -> csrf + .csrfTokenRepository(csrfTokenRepository) + .csrfTokenRequestHandler(new SpaCsrfTokenRequestHandler())) .authorizeHttpRequests(authorize -> authorize .requestMatchers( "/", diff --git a/bff/src/main/java/com/example/keycloakpattern/bff/SpaCsrfTokenRequestHandler.java b/bff/src/main/java/com/example/keycloakpattern/bff/SpaCsrfTokenRequestHandler.java new file mode 100644 index 0000000..3072fc5 --- /dev/null +++ b/bff/src/main/java/com/example/keycloakpattern/bff/SpaCsrfTokenRequestHandler.java @@ -0,0 +1,40 @@ +package com.example.keycloakpattern.bff; + +import java.util.function.Supplier; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +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; + +final class SpaCsrfTokenRequestHandler implements CsrfTokenRequestHandler { + + private final CsrfTokenRequestHandler plain = + new CsrfTokenRequestAttributeHandler(); + private final CsrfTokenRequestHandler xor = + new XorCsrfTokenRequestAttributeHandler(); + + @Override + public void handle( + HttpServletRequest request, + HttpServletResponse response, + Supplier deferredCsrfToken + ) { + xor.handle(request, response, deferredCsrfToken); + } + + @Override + public String resolveCsrfTokenValue( + HttpServletRequest request, + CsrfToken csrfToken + ) { + if (StringUtils.hasText(request.getHeader(csrfToken.getHeaderName()))) { + return plain.resolveCsrfTokenValue(request, csrfToken); + } + return xor.resolveCsrfTokenValue(request, csrfToken); + } +} diff --git a/bff/src/main/resources/application.yml b/bff/src/main/resources/application.yml index 683e671..eebecf0 100644 --- a/bff/src/main/resources/application.yml +++ b/bff/src/main/resources/application.yml @@ -5,6 +5,7 @@ server: cookie: name: AP3_SESSION http-only: true + same-site: lax spring: application: diff --git a/bff/src/main/resources/static/app.js b/bff/src/main/resources/static/app.js index 9e00508..c6bf200 100644 --- a/bff/src/main/resources/static/app.js +++ b/bff/src/main/resources/static/app.js @@ -4,6 +4,14 @@ function render(value) { result.textContent = JSON.stringify(value, null, 2); } +function readCookie(name) { + const prefix = `${encodeURIComponent(name)}=`; + const value = document.cookie + .split("; ") + .find((cookie) => cookie.startsWith(prefix)); + return value ? decodeURIComponent(value.slice(prefix.length)) : null; +} + async function request(path, options = {}) { const response = await fetch(path, { ...options, @@ -30,10 +38,22 @@ document.querySelector("#call-bff").addEventListener("click", () => { void request("/bff/api/me"); }); -document.querySelector("#change-without-csrf").addEventListener("click", () => { - void request("/bff/api/preferences", { +document.querySelector("#change-with-csrf").addEventListener("click", async () => { + const csrfResponse = await fetch("/bff/csrf", { + headers: { Accept: "application/json" }, + }); + const csrf = await csrfResponse.json(); + const csrfToken = readCookie("XSRF-TOKEN"); + if (!csrfToken) { + render({ status: 500, error: "XSRF-TOKEN cookie was not created" }); + return; + } + await request("/bff/api/preferences", { method: "POST", body: new URLSearchParams({ theme: "dark" }), - headers: { "Content-Type": "application/x-www-form-urlencoded" }, + headers: { + "Content-Type": "application/x-www-form-urlencoded", + [csrf.headerName]: csrfToken, + }, }); }); diff --git a/bff/src/main/resources/static/index.html b/bff/src/main/resources/static/index.html index 36ada2b..f055b61 100644 --- a/bff/src/main/resources/static/index.html +++ b/bff/src/main/resources/static/index.html @@ -23,7 +23,7 @@ - + 

   
   
diff --git a/bff/src/test/java/com/example/keycloakpattern/bff/BffControllerTest.java b/bff/src/test/java/com/example/keycloakpattern/bff/BffControllerTest.java
index 6ad9e86..6cd928e 100644
--- a/bff/src/test/java/com/example/keycloakpattern/bff/BffControllerTest.java
+++ b/bff/src/test/java/com/example/keycloakpattern/bff/BffControllerTest.java
@@ -3,6 +3,7 @@ package com.example.keycloakpattern.bff;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.oidcLogin;
+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.header;
@@ -52,18 +53,38 @@ class BffControllerTest {
             .andExpect(jsonPath("$.accessTokenStoredOnServer").value(true))
             .andExpect(jsonPath("$.refreshTokenStoredOnServer").value(true))
             .andExpect(jsonPath("$.browserTokenCount").value(0))
-            .andExpect(jsonPath("$.csrfProtectionEnabled").value(false))
+            .andExpect(jsonPath("$.csrfProtectionEnabled").value(true))
             .andExpect(jsonPath("$.access_token").doesNotExist())
             .andExpect(jsonPath("$.refresh_token").doesNotExist());
     }
 
     @Test
-    void demonstratesStateChangeWithoutCsrfProtection() throws Exception {
+    void rejectsStateChangeWithoutCsrfToken() throws Exception {
         mockMvc.perform(post("/bff/api/preferences")
                 .param("theme", "attacker")
                 .with(oidcLogin().idToken(token -> token.subject("test-subject"))))
+            .andExpect(status().isForbidden());
+    }
+
+    @Test
+    void acceptsStateChangeWithCsrfToken() throws Exception {
+        mockMvc.perform(post("/bff/api/preferences")
+                .param("theme", "dark")
+                .with(oidcLogin().idToken(token -> token.subject("test-subject")))
+                .with(csrf()))
             .andExpect(status().isOk())
             .andExpect(jsonPath("$.updated").value(true))
-            .andExpect(jsonPath("$.theme").value("attacker"));
+            .andExpect(jsonPath("$.theme").value("dark"));
+    }
+
+    @Test
+    void exposesSpaCsrfTokenWithoutCaching() throws Exception {
+        mockMvc.perform(get("/bff/csrf").with(oidcLogin()
+                .idToken(token -> token.subject("test-subject"))))
+            .andExpect(status().isOk())
+            .andExpect(header().string("Cache-Control", "no-store"))
+            .andExpect(header().exists("Set-Cookie"))
+            .andExpect(jsonPath("$.headerName").value("X-XSRF-TOKEN"))
+            .andExpect(jsonPath("$.token").isNotEmpty());
     }
 }
diff --git a/docs/ap3-bff-boundary.md b/docs/ap3-bff-boundary.md
index 6ddba24..fb8f773 100644
--- a/docs/ap3-bff-boundary.md
+++ b/docs/ap3-bff-boundary.md
@@ -17,12 +17,20 @@ Resource Server는 `aud=keycloak-pattern-api`를 검증합니다. 브라우저
 보관합니다. BFF를 재시작하면 세션이 사라집니다. 다중 인스턴스 운영에서는
 Spring Session/Redis 같은 공유 저장소와 저장 token 암호화 정책이 필요합니다.
 
-## 방어 전 CSRF 재현
+## CSRF와 SameSite 방어
 
-이 feature 브랜치에서는 다음 CSRF 방어 feature와 비교하기 위해 CSRF를
-의도적으로 끕니다. 다른 origin의 자동 제출 form이 브라우저 cookie를
-자동으로 포함해 `/bff/api/preferences` 상태를 바꾸는 것을 E2E에서
-재현합니다.
+`feature/keycloak-bff-oauth2login-session`에서는 방어 전 비교를 위해
+CSRF를 끄고, 다른 origin의 자동 제출 form이 `/bff/api/preferences`
+상태를 바꾸는 것을 재현합니다.
 
-이 취약 상태는 `feature/keycloak-bff-csrf-samesite-defense`에서 Spring
-CSRF token과 명시적 SameSite=Lax를 적용해 차단합니다.
+`feature/keycloak-bff-csrf-samesite-defense`에서는 다음 방어를 함께
+적용합니다.
+
+- Spring synchronizer CSRF token과 `CookieCsrfTokenRepository`
+- JS가 읽는 `XSRF-TOKEN`과 요청의 `X-XSRF-TOKEN` header
+- HttpOnly `AP3_SESSION` cookie의 명시적 `SameSite=Lax`
+
+E2E는 token 없는 동일 위조 POST가 403이 되는 것, CSRF header가 있는
+정상 POST는 200인 것, cross-site POST에는 AP3 session cookie가 제외되는
+것을 각각 확인합니다. SameSite는 CSRF token을 대체하지 않는
+defense-in-depth입니다.
diff --git a/e2e/pattern3.mjs b/e2e/pattern3.mjs
index fca578a..5104b0e 100644
--- a/e2e/pattern3.mjs
+++ b/e2e/pattern3.mjs
@@ -36,7 +36,9 @@ try {
   const context = await browser.newContext();
   const page = await context.newPage();
   const browserRequests = [];
-  page.on("request", (request) => browserRequests.push(request.url()));
+  page.on("request", (request) =>
+    browserRequests.push({ method: request.method(), url: request.url() }),
+  );
 
   await page.goto("http://localhost:8083");
   const authorizationRequestPromise = page.waitForRequest((request) =>
@@ -53,10 +55,11 @@ try {
 
   await page.waitForURL(/localhost:8080/u);
   await completeKeycloakLogin(page);
-  const callbackRequest = browserRequests.find((url) =>
+  const callbackRequest = browserRequests.find(({ url }) =>
     url.startsWith("http://localhost:8083/login/oauth2/code/keycloak?"),
   );
   assert.ok(callbackRequest, "authorization response must use the BFF callback");
+  assert.equal(callbackRequest.method, "GET");
 
   const boundaryResponsePromise = page.waitForResponse((response) =>
     response.url().endsWith("/bff/token-boundary"),
@@ -68,7 +71,7 @@ try {
   assert.equal(boundary.accessTokenStoredOnServer, true);
   assert.equal(boundary.refreshTokenStoredOnServer, true);
   assert.equal(boundary.browserTokenCount, 0);
-  assert.equal(boundary.csrfProtectionEnabled, false);
+  assert.equal(boundary.csrfProtectionEnabled, true);
   assert.equal(JSON.stringify(boundary).includes("access_token"), false);
   assert.equal(JSON.stringify(boundary).includes("refresh_token"), false);
 
@@ -83,12 +86,14 @@ try {
   assert.ok(resource.audience.includes("keycloak-pattern-api"));
 
   assert.equal(
-    browserRequests.some((url) => url.startsWith("http://localhost:8081/")),
+    browserRequests.some(({ url }) =>
+      url.startsWith("http://localhost:8081/"),
+    ),
     false,
     "the browser must not bypass the BFF",
   );
   assert.equal(
-    browserRequests.some((url) =>
+    browserRequests.some(({ url }) =>
       url.includes("/protocol/openid-connect/token"),
     ),
     false,
@@ -99,6 +104,7 @@ try {
   const sessionCookie = cookies.find((cookie) => cookie.name === "AP3_SESSION");
   assert.ok(sessionCookie);
   assert.equal(sessionCookie.httpOnly, true);
+  assert.equal(sessionCookie.sameSite, "Lax");
 
   const storage = await page.evaluate(() => ({
     localStorage: Object.values(localStorage),
@@ -109,11 +115,61 @@ try {
   assert.deepEqual(storage.sessionStorage, []);
   assert.equal(storage.readableCookies.includes("AP3_SESSION"), false);
 
-  await page.goto("http://localhost:8088");
-  const forgedResponsePromise = page.waitForResponse(
+  const missingCsrfResponse = await page.evaluate(async () => {
+    const response = await fetch("/bff/api/preferences", {
+      method: "POST",
+      body: new URLSearchParams({ theme: "missing-csrf" }),
+      headers: { "Content-Type": "application/x-www-form-urlencoded" },
+    });
+    return response.status;
+  });
+  assert.equal(missingCsrfResponse, 403);
+
+  const csrfResponsePromise = page.waitForResponse((response) =>
+    response.url().endsWith("/bff/csrf"),
+  );
+  const validChangeResponsePromise = page.waitForResponse(
     (response) =>
-      response.url() === "http://localhost:8083/bff/api/preferences" &&
-      response.request().method() === "POST",
+      response.url().endsWith("/bff/api/preferences") &&
+      response.request().method() === "POST" &&
+      response.status() === 200,
+  );
+  await page.locator("#change-with-csrf").click();
+  const csrfResponse = await csrfResponsePromise;
+  const validChangeResponse = await validChangeResponsePromise;
+  assert.equal(csrfResponse.status(), 200);
+  assert.equal(validChangeResponse.status(), 200);
+  const validChange = await validChangeResponse.json();
+  assert.equal(validChange.theme, "dark");
+
+  const csrfCookies = await context.cookies("http://localhost:8083/");
+  const csrfCookie = csrfCookies.find((cookie) => cookie.name === "XSRF-TOKEN");
+  assert.ok(csrfCookie);
+  assert.equal(csrfCookie.httpOnly, false);
+
+  await page.goto("http://localhost:8088");
+  const [forgedResponse] = await Promise.all([
+    page.waitForNavigation(),
+    page.evaluate(() => {
+      const form = document.createElement("form");
+      form.method = "POST";
+      form.action = "http://localhost:8083/bff/api/preferences";
+      const input = document.createElement("input");
+      input.name = "theme";
+      input.value = "attacker";
+      form.append(input);
+      document.body.append(form);
+      form.submit();
+    }),
+  ]);
+  assert.ok(forgedResponse);
+  assert.equal(forgedResponse.status(), 403);
+
+  await page.goto("http://127.0.0.1:8088");
+  const crossSiteRequestPromise = page.waitForRequest(
+    (request) =>
+      request.url() === "http://localhost:8083/bff/api/preferences" &&
+      request.method() === "POST",
   );
   await page.evaluate(() => {
     const form = document.createElement("form");
@@ -121,19 +177,21 @@ try {
     form.action = "http://localhost:8083/bff/api/preferences";
     const input = document.createElement("input");
     input.name = "theme";
-    input.value = "attacker";
+    input.value = "cross-site-attacker";
     form.append(input);
     document.body.append(form);
     form.submit();
   });
-  const forgedResponse = await forgedResponsePromise;
-  assert.equal(forgedResponse.status(), 200);
-  const forgedResult = await forgedResponse.json();
-  assert.equal(forgedResult.updated, true);
-  assert.equal(forgedResult.theme, "attacker");
+  const crossSiteRequest = await crossSiteRequestPromise;
+  const crossSiteHeaders = await crossSiteRequest.allHeaders();
+  assert.equal(
+    (crossSiteHeaders.cookie ?? "").includes("AP3_SESSION="),
+    false,
+    "SameSite=Lax must omit the session cookie on a cross-site POST",
+  );
 
   console.log(
-    "pattern3 BFF verified: browser token 0, session-only proxy 200, pre-defense CSRF reproduced",
+    "pattern3 BFF verified: tokenless POST 403, CSRF header 200, SameSite=Lax session",
   );
 } finally {
   await browser.close();