Skip to content

Commit 5da78f4

Browse files
author
Steve Riesenberg
committed
Merge branch '5.8.x'
2 parents bf13a84 + ea6ce05 commit 5da78f4

File tree

3 files changed

+143
-5
lines changed

3 files changed

+143
-5
lines changed

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717
package org.springframework.security.config.annotation.web.configurers;
1818

1919
import java.net.URI;
20+
import java.util.Arrays;
21+
import java.util.List;
2022

23+
import jakarta.servlet.http.Cookie;
2124
import jakarta.servlet.http.HttpServletRequest;
2225
import jakarta.servlet.http.HttpServletResponse;
2326
import org.junit.jupiter.api.Test;
@@ -27,6 +30,7 @@
2730
import org.springframework.beans.factory.annotation.Autowired;
2831
import org.springframework.context.annotation.Bean;
2932
import org.springframework.context.annotation.Configuration;
33+
import org.springframework.http.HttpHeaders;
3034
import org.springframework.http.HttpMethod;
3135
import org.springframework.mock.web.MockHttpSession;
3236
import org.springframework.security.config.Customizer;
@@ -42,6 +46,7 @@
4246
import org.springframework.security.web.SecurityFilterChain;
4347
import org.springframework.security.web.access.AccessDeniedHandler;
4448
import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy;
49+
import org.springframework.security.web.csrf.CookieCsrfTokenRepository;
4550
import org.springframework.security.web.csrf.CsrfToken;
4651
import org.springframework.security.web.csrf.CsrfTokenRepository;
4752
import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler;
@@ -308,6 +313,7 @@ public void logoutWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws E
308313
public void loginWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws Exception {
309314
CsrfTokenRepositoryConfig.REPO = mock(CsrfTokenRepository.class);
310315
DefaultCsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
316+
given(CsrfTokenRepositoryConfig.REPO.loadToken(any())).willReturn(csrfToken);
311317
given(CsrfTokenRepositoryConfig.REPO.loadDeferredToken(any(HttpServletRequest.class),
312318
any(HttpServletResponse.class))).willReturn(new TestDeferredCsrfToken(csrfToken));
313319
this.spring.register(CsrfTokenRepositoryConfig.class, BasicController.class).autowire();
@@ -318,6 +324,7 @@ public void loginWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws Ex
318324
.param("password", "password");
319325
// @formatter:on
320326
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
327+
verify(CsrfTokenRepositoryConfig.REPO).loadToken(any(HttpServletRequest.class));
321328
verify(CsrfTokenRepositoryConfig.REPO).saveToken(isNull(), any(HttpServletRequest.class),
322329
any(HttpServletResponse.class));
323330
}
@@ -449,6 +456,7 @@ public void getLoginWhenCsrfTokenRequestAttributeHandlerSetThenRespondsWithNorma
449456
public void loginWhenCsrfTokenRequestAttributeHandlerSetAndNormalCsrfTokenThenSuccess() throws Exception {
450457
CsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
451458
CsrfTokenRepository csrfTokenRepository = mock(CsrfTokenRepository.class);
459+
given(csrfTokenRepository.loadToken(any(HttpServletRequest.class))).willReturn(csrfToken);
452460
given(csrfTokenRepository.loadDeferredToken(any(HttpServletRequest.class), any(HttpServletResponse.class)))
453461
.willReturn(new TestDeferredCsrfToken(csrfToken));
454462
CsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
@@ -462,6 +470,7 @@ public void loginWhenCsrfTokenRequestAttributeHandlerSetAndNormalCsrfTokenThenSu
462470
.param("password", "password");
463471
// @formatter:on
464472
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
473+
verify(csrfTokenRepository).loadToken(any(HttpServletRequest.class));
465474
verify(csrfTokenRepository).saveToken(isNull(), any(HttpServletRequest.class), any(HttpServletResponse.class));
466475
verify(csrfTokenRepository, times(2)).loadDeferredToken(any(HttpServletRequest.class),
467476
any(HttpServletResponse.class));
@@ -487,6 +496,7 @@ public void getLoginWhenXorCsrfTokenRequestAttributeHandlerSetThenRespondsWithMa
487496
public void loginWhenXorCsrfTokenRequestAttributeHandlerSetAndMaskedCsrfTokenThenSuccess() throws Exception {
488497
CsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
489498
CsrfTokenRepository csrfTokenRepository = mock(CsrfTokenRepository.class);
499+
given(csrfTokenRepository.loadToken(any(HttpServletRequest.class))).willReturn(csrfToken);
490500
given(csrfTokenRepository.loadDeferredToken(any(HttpServletRequest.class), any(HttpServletResponse.class)))
491501
.willReturn(new TestDeferredCsrfToken(csrfToken));
492502
CsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
@@ -503,12 +513,93 @@ public void loginWhenXorCsrfTokenRequestAttributeHandlerSetAndMaskedCsrfTokenThe
503513
.param("password", "password");
504514
// @formatter:on
505515
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
516+
verify(csrfTokenRepository).loadToken(any(HttpServletRequest.class));
506517
verify(csrfTokenRepository).saveToken(isNull(), any(HttpServletRequest.class), any(HttpServletResponse.class));
507518
verify(csrfTokenRepository, times(3)).loadDeferredToken(any(HttpServletRequest.class),
508519
any(HttpServletResponse.class));
509520
verifyNoMoreInteractions(csrfTokenRepository);
510521
}
511522

523+
@Test
524+
public void loginWhenFormLoginAndCookieCsrfTokenRepositorySetAndExistingTokenThenRemovesAndGeneratesNewToken()
525+
throws Exception {
526+
CsrfToken csrfToken = new DefaultCsrfToken("X-XSRF-TOKEN", "_csrf", "token");
527+
Cookie existingCookie = new Cookie("XSRF-TOKEN", csrfToken.getToken());
528+
CookieCsrfTokenRepository csrfTokenRepository = CookieCsrfTokenRepository.withHttpOnlyFalse();
529+
csrfTokenRepository.setCookieName(existingCookie.getName());
530+
CsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
531+
CsrfTokenRequestHandlerConfig.HANDLER = new CsrfTokenRequestAttributeHandler();
532+
this.spring.register(CsrfTokenRequestHandlerConfig.class, BasicController.class).autowire();
533+
534+
// @formatter:off
535+
MockHttpServletRequestBuilder loginRequest = post("/login")
536+
.cookie(existingCookie)
537+
.header(csrfToken.getHeaderName(), csrfToken.getToken())
538+
.param("username", "user")
539+
.param("password", "password");
540+
// @formatter:on
541+
MvcResult mvcResult = this.mvc.perform(loginRequest).andExpect(redirectedUrl("/")).andReturn();
542+
List<Cookie> cookies = Arrays.asList(mvcResult.getResponse().getCookies());
543+
cookies.removeIf((cookie) -> !cookie.getName().equalsIgnoreCase(existingCookie.getName()));
544+
assertThat(cookies).hasSize(2);
545+
assertThat(cookies.get(0).getValue()).isEmpty();
546+
assertThat(cookies.get(1).getValue()).isNotEmpty();
547+
}
548+
549+
@Test
550+
public void postWhenHttpBasicAndCookieCsrfTokenRepositorySetAndExistingTokenThenRemovesAndGeneratesNewToken()
551+
throws Exception {
552+
CsrfToken csrfToken = new DefaultCsrfToken("X-XSRF-TOKEN", "_csrf", "token");
553+
Cookie existingCookie = new Cookie("XSRF-TOKEN", csrfToken.getToken());
554+
CookieCsrfTokenRepository csrfTokenRepository = CookieCsrfTokenRepository.withHttpOnlyFalse();
555+
csrfTokenRepository.setCookieName(existingCookie.getName());
556+
HttpBasicCsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
557+
HttpBasicCsrfTokenRequestHandlerConfig.HANDLER = new CsrfTokenRequestAttributeHandler();
558+
this.spring.register(HttpBasicCsrfTokenRequestHandlerConfig.class, BasicController.class).autowire();
559+
560+
HttpHeaders headers = new HttpHeaders();
561+
headers.set(csrfToken.getHeaderName(), csrfToken.getToken());
562+
headers.setBasicAuth("user", "password");
563+
// @formatter:off
564+
MvcResult mvcResult = this.mvc.perform(post("/")
565+
.cookie(existingCookie)
566+
.headers(headers))
567+
.andExpect(status().isOk())
568+
.andReturn();
569+
// @formatter:on
570+
List<Cookie> cookies = Arrays.asList(mvcResult.getResponse().getCookies());
571+
cookies.removeIf((cookie) -> !cookie.getName().equalsIgnoreCase(existingCookie.getName()));
572+
assertThat(cookies).hasSize(2);
573+
assertThat(cookies.get(0).getValue()).isEmpty();
574+
assertThat(cookies.get(1).getValue()).isNotEmpty();
575+
}
576+
577+
@Test
578+
public void getWhenHttpBasicAndCookieCsrfTokenRepositorySetAndNoExistingCookieThenGeneratesNewToken()
579+
throws Exception {
580+
CsrfToken csrfToken = new DefaultCsrfToken("X-XSRF-TOKEN", "_csrf", "token");
581+
Cookie expectedCookie = new Cookie("XSRF-TOKEN", csrfToken.getToken());
582+
CookieCsrfTokenRepository csrfTokenRepository = CookieCsrfTokenRepository.withHttpOnlyFalse();
583+
csrfTokenRepository.setCookieName(expectedCookie.getName());
584+
HttpBasicCsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
585+
HttpBasicCsrfTokenRequestHandlerConfig.HANDLER = new CsrfTokenRequestAttributeHandler();
586+
this.spring.register(HttpBasicCsrfTokenRequestHandlerConfig.class, BasicController.class).autowire();
587+
588+
HttpHeaders headers = new HttpHeaders();
589+
headers.set(csrfToken.getHeaderName(), csrfToken.getToken());
590+
headers.setBasicAuth("user", "password");
591+
// @formatter:off
592+
MvcResult mvcResult = this.mvc.perform(get("/")
593+
.headers(headers))
594+
.andExpect(status().isOk())
595+
.andReturn();
596+
// @formatter:on
597+
List<Cookie> cookies = Arrays.asList(mvcResult.getResponse().getCookies());
598+
cookies.removeIf((cookie) -> !cookie.getName().equalsIgnoreCase(expectedCookie.getName()));
599+
assertThat(cookies).hasSize(1);
600+
assertThat(cookies.get(0).getValue()).isNotEmpty();
601+
}
602+
512603
@Configuration
513604
static class AllowHttpMethodsFirewallConfig {
514605

@@ -902,6 +993,42 @@ void configure(AuthenticationManagerBuilder auth) throws Exception {
902993

903994
}
904995

996+
@Configuration
997+
@EnableWebSecurity
998+
static class HttpBasicCsrfTokenRequestHandlerConfig {
999+
1000+
static CsrfTokenRepository REPO;
1001+
1002+
static CsrfTokenRequestHandler HANDLER;
1003+
1004+
@Bean
1005+
SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
1006+
// @formatter:off
1007+
http
1008+
.authorizeHttpRequests((authorize) -> authorize
1009+
.anyRequest().authenticated()
1010+
)
1011+
.httpBasic(Customizer.withDefaults())
1012+
.csrf((csrf) -> csrf
1013+
.csrfTokenRepository(REPO)
1014+
.csrfTokenRequestHandler(HANDLER)
1015+
);
1016+
// @formatter:on
1017+
1018+
return http.build();
1019+
}
1020+
1021+
@Autowired
1022+
void configure(AuthenticationManagerBuilder auth) throws Exception {
1023+
// @formatter:off
1024+
auth
1025+
.inMemoryAuthentication()
1026+
.withUser(PasswordEncodedUser.user());
1027+
// @formatter:on
1028+
}
1029+
1030+
}
1031+
9051032
@RestController
9061033
static class BasicController {
9071034

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
* the next request.
3333
*
3434
* @author Rob Winch
35+
* @author Steve Riesenberg
3536
* @since 3.2
3637
*/
3738
public final class CsrfAuthenticationStrategy implements SessionAuthenticationStrategy {
@@ -64,10 +65,13 @@ public void setRequestHandler(CsrfTokenRequestHandler requestHandler) {
6465
@Override
6566
public void onAuthentication(Authentication authentication, HttpServletRequest request,
6667
HttpServletResponse response) throws SessionAuthenticationException {
67-
this.tokenRepository.saveToken(null, request, response);
68-
DeferredCsrfToken deferredCsrfToken = this.tokenRepository.loadDeferredToken(request, response);
69-
this.requestHandler.handle(request, response, deferredCsrfToken::get);
70-
this.logger.debug("Replaced CSRF Token");
68+
boolean containsToken = this.tokenRepository.loadToken(request) != null;
69+
if (containsToken) {
70+
this.tokenRepository.saveToken(null, request, response);
71+
DeferredCsrfToken deferredCsrfToken = this.tokenRepository.loadDeferredToken(request, response);
72+
this.requestHandler.handle(request, response, deferredCsrfToken::get);
73+
this.logger.debug("Replaced CSRF Token");
74+
}
7175
}
7276

7377
}

‎web/src/test/java/org/springframework/security/web/csrf/CsrfAuthenticationStrategyTests.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import static org.mockito.BDDMockito.given;
3636
import static org.mockito.Mockito.mock;
3737
import static org.mockito.Mockito.never;
38+
import static org.mockito.Mockito.times;
3839
import static org.mockito.Mockito.verify;
3940
import static org.mockito.Mockito.verifyNoMoreInteractions;
4041

@@ -81,23 +82,28 @@ public void setRequestHandlerWhenNullThenIllegalStateException() {
8182

8283
@Test
8384
public void onAuthenticationWhenCustomRequestHandlerThenUsed() {
85+
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken);
8486
given(this.csrfTokenRepository.loadDeferredToken(this.request, this.response))
8587
.willReturn(new TestDeferredCsrfToken(this.existingToken, false));
8688

8789
CsrfTokenRequestHandler requestHandler = mock(CsrfTokenRequestHandler.class);
8890
this.strategy.setRequestHandler(requestHandler);
8991
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
9092
this.response);
93+
verify(this.csrfTokenRepository).loadToken(this.request);
94+
verify(this.csrfTokenRepository).loadDeferredToken(this.request, this.response);
9195
verify(requestHandler).handle(eq(this.request), eq(this.response), any());
9296
verifyNoMoreInteractions(requestHandler);
9397
}
9498

9599
@Test
96100
public void logoutRemovesCsrfTokenAndLoadsNewDeferredCsrfToken() {
101+
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken);
97102
given(this.csrfTokenRepository.loadDeferredToken(this.request, this.response))
98103
.willReturn(new TestDeferredCsrfToken(this.generatedToken, false));
99104
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
100105
this.response);
106+
verify(this.csrfTokenRepository).loadToken(this.request);
101107
verify(this.csrfTokenRepository).saveToken(null, this.request, this.response);
102108
verify(this.csrfTokenRepository).loadDeferredToken(this.request, this.response);
103109
// SEC-2404, SEC-2832
@@ -112,6 +118,7 @@ public void logoutRemovesCsrfTokenAndLoadsNewDeferredCsrfToken() {
112118
@Test
113119
public void delaySavingCsrf() {
114120
this.strategy = new CsrfAuthenticationStrategy(new LazyCsrfTokenRepository(this.csrfTokenRepository));
121+
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken, (CsrfToken) null);
115122
given(this.csrfTokenRepository.generateToken(this.request)).willReturn(this.generatedToken);
116123
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
117124
this.response);
@@ -120,7 +127,7 @@ public void delaySavingCsrf() {
120127
any(HttpServletResponse.class));
121128
CsrfToken tokenInRequest = (CsrfToken) this.request.getAttribute(CsrfToken.class.getName());
122129
tokenInRequest.getToken();
123-
verify(this.csrfTokenRepository).loadToken(this.request);
130+
verify(this.csrfTokenRepository, times(2)).loadToken(this.request);
124131
verify(this.csrfTokenRepository).generateToken(this.request);
125132
verify(this.csrfTokenRepository).saveToken(eq(this.generatedToken), any(HttpServletRequest.class),
126133
any(HttpServletResponse.class));

0 commit comments

Comments
 (0)