Skip to content

Commit 2a2051c

Browse files
author
Steve Riesenberg
committed
Default to Xor CSRF tokens in CsrfFilter
Issue gh-11960
1 parent 60aa799 commit 2a2051c

File tree

6 files changed

+96
-56
lines changed

6 files changed

+96
-56
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/DefaultFiltersTests.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,11 @@
5151
import org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter;
5252
import org.springframework.security.web.csrf.CsrfFilter;
5353
import org.springframework.security.web.csrf.CsrfToken;
54+
import org.springframework.security.web.csrf.CsrfTokenRepository;
55+
import org.springframework.security.web.csrf.CsrfTokenRequestHandler;
5456
import org.springframework.security.web.csrf.DefaultCsrfToken;
5557
import org.springframework.security.web.csrf.HttpSessionCsrfTokenRepository;
58+
import org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler;
5659
import org.springframework.security.web.header.HeaderWriterFilter;
5760
import org.springframework.security.web.savedrequest.RequestCacheAwareFilter;
5861
import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
@@ -121,8 +124,12 @@ public void defaultFiltersPermitAll() throws IOException, ServletException {
121124
MockHttpServletRequest request = new MockHttpServletRequest("POST", "");
122125
request.setServletPath("/logout");
123126
CsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "BaseSpringSpec_CSRFTOKEN");
124-
new HttpSessionCsrfTokenRepository().saveToken(csrfToken, request, response);
125-
request.setParameter(csrfToken.getParameterName(), csrfToken.getToken());
127+
CsrfTokenRepository repository = new HttpSessionCsrfTokenRepository();
128+
repository.saveToken(csrfToken, request, response);
129+
CsrfTokenRequestHandler handler = new XorCsrfTokenRequestAttributeHandler();
130+
handler.handle(request, response, () -> csrfToken);
131+
CsrfToken token = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
132+
request.setParameter(token.getParameterName(), token.getToken());
126133
this.spring.getContext().getBean("springSecurityFilterChain", Filter.class).doFilter(request, response,
127134
new MockFilterChain());
128135
assertThat(response.getRedirectedUrl()).isEqualTo("/login?logout");

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/DefaultLoginPageConfigurerTests.java

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ public void loginPageThenDefaultLoginPageIsRendered() throws Exception {
8585
String csrfAttributeName = HttpSessionCsrfTokenRepository.class.getName().concat(".CSRF_TOKEN");
8686
// @formatter:off
8787
this.mvc.perform(get("/login").sessionAttr(csrfAttributeName, csrfToken))
88-
.andExpect(content().string("<!DOCTYPE html>\n"
88+
.andExpect((result) -> {
89+
CsrfToken token = (CsrfToken) result.getRequest().getAttribute(CsrfToken.class.getName());
90+
assertThat(result.getResponse().getContentAsString()).isEqualTo("<!DOCTYPE html>\n"
8991
+ "<html lang=\"en\">\n"
9092
+ " <head>\n"
9193
+ " <meta charset=\"utf-8\">\n"
@@ -108,11 +110,12 @@ public void loginPageThenDefaultLoginPageIsRendered() throws Exception {
108110
+ " <label for=\"password\" class=\"sr-only\">Password</label>\n"
109111
+ " <input type=\"password\" id=\"password\" name=\"password\" class=\"form-control\" placeholder=\"Password\" required>\n"
110112
+ " </p>\n"
111-
+ "<input name=\"" + csrfToken.getParameterName() + "\" type=\"hidden\" value=\"" + csrfToken.getToken() + "\" />\n"
113+
+ "<input name=\"" + token.getParameterName() + "\" type=\"hidden\" value=\"" + token.getToken() + "\" />\n"
112114
+ " <button class=\"btn btn-lg btn-primary btn-block\" type=\"submit\">Sign in</button>\n"
113115
+ " </form>\n"
114116
+ "</div>\n"
115-
+ "</body></html>"));
117+
+ "</body></html>");
118+
});
116119
// @formatter:on
117120
}
118121

@@ -131,7 +134,9 @@ public void loginPageWhenErrorThenDefaultLoginPageWithError() throws Exception {
131134
// @formatter:off
132135
this.mvc.perform(get("/login?error").session((MockHttpSession) mvcResult.getRequest().getSession())
133136
.sessionAttr(csrfAttributeName, csrfToken))
134-
.andExpect(content().string("<!DOCTYPE html>\n"
137+
.andExpect((result) -> {
138+
CsrfToken token = (CsrfToken) result.getRequest().getAttribute(CsrfToken.class.getName());
139+
assertThat(result.getResponse().getContentAsString()).isEqualTo("<!DOCTYPE html>\n"
135140
+ "<html lang=\"en\">\n"
136141
+ " <head>\n"
137142
+ " <meta charset=\"utf-8\">\n"
@@ -153,11 +158,12 @@ public void loginPageWhenErrorThenDefaultLoginPageWithError() throws Exception {
153158
+ " <label for=\"password\" class=\"sr-only\">Password</label>\n"
154159
+ " <input type=\"password\" id=\"password\" name=\"password\" class=\"form-control\" placeholder=\"Password\" required>\n"
155160
+ " </p>\n"
156-
+ "<input name=\"" + csrfToken.getParameterName() + "\" type=\"hidden\" value=\"" + csrfToken.getToken() + "\" />\n"
161+
+ "<input name=\"" + token.getParameterName() + "\" type=\"hidden\" value=\"" + token.getToken() + "\" />\n"
157162
+ " <button class=\"btn btn-lg btn-primary btn-block\" type=\"submit\">Sign in</button>\n"
158163
+ " </form>\n"
159164
+ "</div>\n"
160-
+ "</body></html>"));
165+
+ "</body></html>");
166+
});
161167
// @formatter:on
162168
}
163169

@@ -180,7 +186,9 @@ public void loginPageWhenLoggedOutThenDefaultLoginPageWithLogoutMessage() throws
180186
String csrfAttributeName = HttpSessionCsrfTokenRepository.class.getName().concat(".CSRF_TOKEN");
181187
// @formatter:off
182188
this.mvc.perform(get("/login?logout").sessionAttr(csrfAttributeName, csrfToken))
183-
.andExpect(content().string("<!DOCTYPE html>\n"
189+
.andExpect((result) -> {
190+
CsrfToken token = (CsrfToken) result.getRequest().getAttribute(CsrfToken.class.getName());
191+
assertThat(result.getResponse().getContentAsString()).isEqualTo("<!DOCTYPE html>\n"
184192
+ "<html lang=\"en\">\n"
185193
+ " <head>\n"
186194
+ " <meta charset=\"utf-8\">\n"
@@ -203,11 +211,12 @@ public void loginPageWhenLoggedOutThenDefaultLoginPageWithLogoutMessage() throws
203211
+ " <label for=\"password\" class=\"sr-only\">Password</label>\n"
204212
+ " <input type=\"password\" id=\"password\" name=\"password\" class=\"form-control\" placeholder=\"Password\" required>\n"
205213
+ " </p>\n"
206-
+ "<input name=\"" + csrfToken.getParameterName() + "\" type=\"hidden\" value=\"" + csrfToken.getToken() + "\" />\n"
214+
+ "<input name=\"" + token.getParameterName() + "\" type=\"hidden\" value=\"" + token.getToken() + "\" />\n"
207215
+ " <button class=\"btn btn-lg btn-primary btn-block\" type=\"submit\">Sign in</button>\n"
208216
+ " </form>\n"
209217
+ "</div>\n"
210-
+ "</body></html>"));
218+
+ "</body></html>");
219+
});
211220
// @formatter:on
212221
}
213222

@@ -230,7 +239,9 @@ public void loginPageWhenRememberConfigureThenDefaultLoginPageWithRememberMeChec
230239
String csrfAttributeName = HttpSessionCsrfTokenRepository.class.getName().concat(".CSRF_TOKEN");
231240
// @formatter:off
232241
this.mvc.perform(get("/login").sessionAttr(csrfAttributeName, csrfToken))
233-
.andExpect(content().string("<!DOCTYPE html>\n"
242+
.andExpect((result) -> {
243+
CsrfToken token = (CsrfToken) result.getRequest().getAttribute(CsrfToken.class.getName());
244+
assertThat(result.getResponse().getContentAsString()).isEqualTo("<!DOCTYPE html>\n"
234245
+ "<html lang=\"en\">\n"
235246
+ " <head>\n"
236247
+ " <meta charset=\"utf-8\">\n"
@@ -254,11 +265,12 @@ public void loginPageWhenRememberConfigureThenDefaultLoginPageWithRememberMeChec
254265
+ " <input type=\"password\" id=\"password\" name=\"password\" class=\"form-control\" placeholder=\"Password\" required>\n"
255266
+ " </p>\n"
256267
+ "<p><input type='checkbox' name='remember-me'/> Remember me on this computer.</p>\n"
257-
+ "<input name=\"" + csrfToken.getParameterName() + "\" type=\"hidden\" value=\"" + csrfToken.getToken() + "\" />\n"
268+
+ "<input name=\"" + token.getParameterName() + "\" type=\"hidden\" value=\"" + token.getToken() + "\" />\n"
258269
+ " <button class=\"btn btn-lg btn-primary btn-block\" type=\"submit\">Sign in</button>\n"
259270
+ " </form>\n"
260271
+ "</div>\n"
261-
+ "</body></html>"));
272+
+ "</body></html>");
273+
});
262274
// @formatter:on
263275
}
264276

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerServlet31Tests.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@
3939
import org.springframework.security.web.context.HttpRequestResponseHolder;
4040
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
4141
import org.springframework.security.web.csrf.CsrfToken;
42+
import org.springframework.security.web.csrf.CsrfTokenRequestHandler;
43+
import org.springframework.security.web.csrf.DeferredCsrfToken;
4244
import org.springframework.security.web.csrf.HttpSessionCsrfTokenRepository;
45+
import org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler;
4346

4447
import static org.assertj.core.api.Assertions.assertThat;
4548

@@ -82,8 +85,10 @@ public void changeSessionIdThenPreserveParameters() throws Exception {
8285
request.setParameter("username", "user");
8386
request.setParameter("password", "password");
8487
HttpSessionCsrfTokenRepository repository = new HttpSessionCsrfTokenRepository();
85-
CsrfToken token = repository.generateToken(request);
86-
repository.saveToken(token, request, this.response);
88+
CsrfTokenRequestHandler handler = new XorCsrfTokenRequestAttributeHandler();
89+
DeferredCsrfToken deferredCsrfToken = repository.loadDeferredToken(request, this.response);
90+
handler.handle(request, this.response, deferredCsrfToken::get);
91+
CsrfToken token = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
8792
request.setParameter(token.getParameterName(), token.getToken());
8893
request.getSession().setAttribute("attribute1", "value1");
8994
loadConfig(SessionManagementDefaultSessionFixationServlet31Config.class);

‎test/src/test/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessorsCsrfTests.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@
4040
import org.springframework.security.web.FilterChainProxy;
4141
import org.springframework.security.web.SecurityFilterChain;
4242
import org.springframework.security.web.csrf.CsrfToken;
43+
import org.springframework.security.web.csrf.CsrfTokenRequestHandler;
44+
import org.springframework.security.web.csrf.DeferredCsrfToken;
4345
import org.springframework.security.web.csrf.HttpSessionCsrfTokenRepository;
46+
import org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler;
4447
import org.springframework.test.context.ContextConfiguration;
4548
import org.springframework.test.context.junit.jupiter.SpringExtension;
4649
import org.springframework.test.context.web.WebAppConfiguration;
@@ -157,9 +160,12 @@ public void csrfWhenUsedThenDoesNotImpactOriginalRepository() throws Exception {
157160
// @formatter:off
158161
this.mockMvc.perform(post("/").with(csrf()));
159162
MockHttpServletRequest request = new MockHttpServletRequest();
163+
MockHttpServletResponse response = new MockHttpServletResponse();
160164
HttpSessionCsrfTokenRepository repo = new HttpSessionCsrfTokenRepository();
161-
CsrfToken token = repo.generateToken(request);
162-
repo.saveToken(token, request, new MockHttpServletResponse());
165+
CsrfTokenRequestHandler handler = new XorCsrfTokenRequestAttributeHandler();
166+
DeferredCsrfToken deferredCsrfToken = repo.loadDeferredToken(request, response);
167+
handler.handle(request, response, deferredCsrfToken::get);
168+
CsrfToken token = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
163169
MockHttpServletRequestBuilder requestWithCsrf = post("/")
164170
.param(token.getParameterName(), token.getToken())
165171
.session((MockHttpSession) request.getSession());

‎web/src/main/java/org/springframework/security/web/csrf/CsrfFilter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public final class CsrfFilter extends OncePerRequestFilter {
8787

8888
private AccessDeniedHandler accessDeniedHandler = new AccessDeniedHandlerImpl();
8989

90-
private CsrfTokenRequestHandler requestHandler = new CsrfTokenRequestAttributeHandler();
90+
private CsrfTokenRequestHandler requestHandler = new XorCsrfTokenRequestAttributeHandler();
9191

9292
/**
9393
* Creates a new instance.

0 commit comments

Comments
 (0)