Skip to content

Commit 2ed7cff

Browse files
author
Steve Riesenberg
committed
Check for existing token before clearing
Closes gh-12236
1 parent 1919b4e commit 2ed7cff

File tree

3 files changed

+22
-5
lines changed

3 files changed

+22
-5
lines changed

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

+6
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ public void logoutWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws E
302302
public void loginWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws Exception {
303303
CsrfTokenRepositoryConfig.REPO = mock(CsrfTokenRepository.class);
304304
DefaultCsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
305+
given(CsrfTokenRepositoryConfig.REPO.loadToken(any())).willReturn(csrfToken);
305306
given(CsrfTokenRepositoryConfig.REPO.loadDeferredToken(any(HttpServletRequest.class),
306307
any(HttpServletResponse.class))).willReturn(new TestDeferredCsrfToken(csrfToken));
307308
this.spring.register(CsrfTokenRepositoryConfig.class, BasicController.class).autowire();
@@ -312,6 +313,7 @@ public void loginWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws Ex
312313
.param("password", "password");
313314
// @formatter:on
314315
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
316+
verify(CsrfTokenRepositoryConfig.REPO).loadToken(any(HttpServletRequest.class));
315317
verify(CsrfTokenRepositoryConfig.REPO).saveToken(isNull(), any(HttpServletRequest.class),
316318
any(HttpServletResponse.class));
317319
}
@@ -443,6 +445,7 @@ public void getLoginWhenCsrfTokenRequestAttributeHandlerSetThenRespondsWithNorma
443445
public void loginWhenCsrfTokenRequestAttributeHandlerSetAndNormalCsrfTokenThenSuccess() throws Exception {
444446
CsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
445447
CsrfTokenRepository csrfTokenRepository = mock(CsrfTokenRepository.class);
448+
given(csrfTokenRepository.loadToken(any(HttpServletRequest.class))).willReturn(csrfToken);
446449
given(csrfTokenRepository.loadDeferredToken(any(HttpServletRequest.class), any(HttpServletResponse.class)))
447450
.willReturn(new TestDeferredCsrfToken(csrfToken));
448451
CsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
@@ -456,6 +459,7 @@ public void loginWhenCsrfTokenRequestAttributeHandlerSetAndNormalCsrfTokenThenSu
456459
.param("password", "password");
457460
// @formatter:on
458461
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
462+
verify(csrfTokenRepository).loadToken(any(HttpServletRequest.class));
459463
verify(csrfTokenRepository).saveToken(isNull(), any(HttpServletRequest.class), any(HttpServletResponse.class));
460464
verify(csrfTokenRepository, times(2)).loadDeferredToken(any(HttpServletRequest.class),
461465
any(HttpServletResponse.class));
@@ -481,6 +485,7 @@ public void getLoginWhenXorCsrfTokenRequestAttributeHandlerSetThenRespondsWithMa
481485
public void loginWhenXorCsrfTokenRequestAttributeHandlerSetAndMaskedCsrfTokenThenSuccess() throws Exception {
482486
CsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
483487
CsrfTokenRepository csrfTokenRepository = mock(CsrfTokenRepository.class);
488+
given(csrfTokenRepository.loadToken(any(HttpServletRequest.class))).willReturn(csrfToken);
484489
given(csrfTokenRepository.loadDeferredToken(any(HttpServletRequest.class), any(HttpServletResponse.class)))
485490
.willReturn(new TestDeferredCsrfToken(csrfToken));
486491
CsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
@@ -497,6 +502,7 @@ public void loginWhenXorCsrfTokenRequestAttributeHandlerSetAndMaskedCsrfTokenThe
497502
.param("password", "password");
498503
// @formatter:on
499504
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
505+
verify(csrfTokenRepository).loadToken(any(HttpServletRequest.class));
500506
verify(csrfTokenRepository).saveToken(isNull(), any(HttpServletRequest.class), any(HttpServletResponse.class));
501507
verify(csrfTokenRepository, times(3)).loadDeferredToken(any(HttpServletRequest.class),
502508
any(HttpServletResponse.class));

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

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

7478
}

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

+8-1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import static org.mockito.BDDMockito.given;
3737
import static org.mockito.Mockito.mock;
3838
import static org.mockito.Mockito.never;
39+
import static org.mockito.Mockito.times;
3940
import static org.mockito.Mockito.verify;
4041
import static org.mockito.Mockito.verifyNoMoreInteractions;
4142

@@ -82,23 +83,28 @@ public void setRequestHandlerWhenNullThenIllegalStateException() {
8283

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

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

96100
@Test
97101
public void logoutRemovesCsrfTokenAndLoadsNewDeferredCsrfToken() {
102+
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken);
98103
given(this.csrfTokenRepository.loadDeferredToken(this.request, this.response))
99104
.willReturn(new TestDeferredCsrfToken(this.generatedToken, false));
100105
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
101106
this.response);
107+
verify(this.csrfTokenRepository).loadToken(this.request);
102108
verify(this.csrfTokenRepository).saveToken(null, this.request, this.response);
103109
verify(this.csrfTokenRepository).loadDeferredToken(this.request, this.response);
104110
// SEC-2404, SEC-2832
@@ -113,6 +119,7 @@ public void logoutRemovesCsrfTokenAndLoadsNewDeferredCsrfToken() {
113119
@Test
114120
public void delaySavingCsrf() {
115121
this.strategy = new CsrfAuthenticationStrategy(new LazyCsrfTokenRepository(this.csrfTokenRepository));
122+
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken, (CsrfToken) null);
116123
given(this.csrfTokenRepository.generateToken(this.request)).willReturn(this.generatedToken);
117124
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
118125
this.response);
@@ -121,7 +128,7 @@ public void delaySavingCsrf() {
121128
any(HttpServletResponse.class));
122129
CsrfToken tokenInRequest = (CsrfToken) this.request.getAttribute(CsrfToken.class.getName());
123130
tokenInRequest.getToken();
124-
verify(this.csrfTokenRepository).loadToken(this.request);
131+
verify(this.csrfTokenRepository, times(2)).loadToken(this.request);
125132
verify(this.csrfTokenRepository).generateToken(this.request);
126133
verify(this.csrfTokenRepository).saveToken(eq(this.generatedToken), any(HttpServletRequest.class),
127134
any(HttpServletResponse.class));

0 commit comments

Comments
 (0)