Skip to content

Commit

Permalink
Check for existing token before clearing
Browse files Browse the repository at this point in the history
Closes gh-12236
  • Loading branch information
sjohnr committed Nov 18, 2022
1 parent 1919b4e commit 2ed7cff
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ public void logoutWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws E
public void loginWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws Exception {
CsrfTokenRepositoryConfig.REPO = mock(CsrfTokenRepository.class);
DefaultCsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
given(CsrfTokenRepositoryConfig.REPO.loadToken(any())).willReturn(csrfToken);
given(CsrfTokenRepositoryConfig.REPO.loadDeferredToken(any(HttpServletRequest.class),
any(HttpServletResponse.class))).willReturn(new TestDeferredCsrfToken(csrfToken));
this.spring.register(CsrfTokenRepositoryConfig.class, BasicController.class).autowire();
Expand All @@ -312,6 +313,7 @@ public void loginWhenCustomCsrfTokenRepositoryThenCsrfTokenIsCleared() throws Ex
.param("password", "password");
// @formatter:on
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
verify(CsrfTokenRepositoryConfig.REPO).loadToken(any(HttpServletRequest.class));
verify(CsrfTokenRepositoryConfig.REPO).saveToken(isNull(), any(HttpServletRequest.class),
any(HttpServletResponse.class));
}
Expand Down Expand Up @@ -443,6 +445,7 @@ public void getLoginWhenCsrfTokenRequestAttributeHandlerSetThenRespondsWithNorma
public void loginWhenCsrfTokenRequestAttributeHandlerSetAndNormalCsrfTokenThenSuccess() throws Exception {
CsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
CsrfTokenRepository csrfTokenRepository = mock(CsrfTokenRepository.class);
given(csrfTokenRepository.loadToken(any(HttpServletRequest.class))).willReturn(csrfToken);
given(csrfTokenRepository.loadDeferredToken(any(HttpServletRequest.class), any(HttpServletResponse.class)))
.willReturn(new TestDeferredCsrfToken(csrfToken));
CsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
Expand All @@ -456,6 +459,7 @@ public void loginWhenCsrfTokenRequestAttributeHandlerSetAndNormalCsrfTokenThenSu
.param("password", "password");
// @formatter:on
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
verify(csrfTokenRepository).loadToken(any(HttpServletRequest.class));
verify(csrfTokenRepository).saveToken(isNull(), any(HttpServletRequest.class), any(HttpServletResponse.class));
verify(csrfTokenRepository, times(2)).loadDeferredToken(any(HttpServletRequest.class),
any(HttpServletResponse.class));
Expand All @@ -481,6 +485,7 @@ public void getLoginWhenXorCsrfTokenRequestAttributeHandlerSetThenRespondsWithMa
public void loginWhenXorCsrfTokenRequestAttributeHandlerSetAndMaskedCsrfTokenThenSuccess() throws Exception {
CsrfToken csrfToken = new DefaultCsrfToken("X-CSRF-TOKEN", "_csrf", "token");
CsrfTokenRepository csrfTokenRepository = mock(CsrfTokenRepository.class);
given(csrfTokenRepository.loadToken(any(HttpServletRequest.class))).willReturn(csrfToken);
given(csrfTokenRepository.loadDeferredToken(any(HttpServletRequest.class), any(HttpServletResponse.class)))
.willReturn(new TestDeferredCsrfToken(csrfToken));
CsrfTokenRequestHandlerConfig.REPO = csrfTokenRepository;
Expand All @@ -497,6 +502,7 @@ public void loginWhenXorCsrfTokenRequestAttributeHandlerSetAndMaskedCsrfTokenThe
.param("password", "password");
// @formatter:on
this.mvc.perform(loginRequest).andExpect(redirectedUrl("/"));
verify(csrfTokenRepository).loadToken(any(HttpServletRequest.class));
verify(csrfTokenRepository).saveToken(isNull(), any(HttpServletRequest.class), any(HttpServletResponse.class));
verify(csrfTokenRepository, times(3)).loadDeferredToken(any(HttpServletRequest.class),
any(HttpServletResponse.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
* the next request.
*
* @author Rob Winch
* @author Steve Riesenberg
* @since 3.2
*/
public final class CsrfAuthenticationStrategy implements SessionAuthenticationStrategy {
Expand Down Expand Up @@ -65,10 +66,13 @@ public void setRequestHandler(CsrfTokenRequestHandler requestHandler) {
@Override
public void onAuthentication(Authentication authentication, HttpServletRequest request,
HttpServletResponse response) throws SessionAuthenticationException {
this.tokenRepository.saveToken(null, request, response);
DeferredCsrfToken deferredCsrfToken = this.tokenRepository.loadDeferredToken(request, response);
this.requestHandler.handle(request, response, deferredCsrfToken::get);
this.logger.debug("Replaced CSRF Token");
boolean containsToken = this.tokenRepository.loadToken(request) != null;
if (containsToken) {
this.tokenRepository.saveToken(null, request, response);
DeferredCsrfToken deferredCsrfToken = this.tokenRepository.loadDeferredToken(request, response);
this.requestHandler.handle(request, response, deferredCsrfToken::get);
this.logger.debug("Replaced CSRF Token");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

Expand Down Expand Up @@ -82,23 +83,28 @@ public void setRequestHandlerWhenNullThenIllegalStateException() {

@Test
public void onAuthenticationWhenCustomRequestHandlerThenUsed() {
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken);
given(this.csrfTokenRepository.loadDeferredToken(this.request, this.response))
.willReturn(new TestDeferredCsrfToken(this.existingToken, false));

CsrfTokenRequestHandler requestHandler = mock(CsrfTokenRequestHandler.class);
this.strategy.setRequestHandler(requestHandler);
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
this.response);
verify(this.csrfTokenRepository).loadToken(this.request);
verify(this.csrfTokenRepository).loadDeferredToken(this.request, this.response);
verify(requestHandler).handle(eq(this.request), eq(this.response), any());
verifyNoMoreInteractions(requestHandler);
}

@Test
public void logoutRemovesCsrfTokenAndLoadsNewDeferredCsrfToken() {
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken);
given(this.csrfTokenRepository.loadDeferredToken(this.request, this.response))
.willReturn(new TestDeferredCsrfToken(this.generatedToken, false));
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
this.response);
verify(this.csrfTokenRepository).loadToken(this.request);
verify(this.csrfTokenRepository).saveToken(null, this.request, this.response);
verify(this.csrfTokenRepository).loadDeferredToken(this.request, this.response);
// SEC-2404, SEC-2832
Expand All @@ -113,6 +119,7 @@ public void logoutRemovesCsrfTokenAndLoadsNewDeferredCsrfToken() {
@Test
public void delaySavingCsrf() {
this.strategy = new CsrfAuthenticationStrategy(new LazyCsrfTokenRepository(this.csrfTokenRepository));
given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken, (CsrfToken) null);
given(this.csrfTokenRepository.generateToken(this.request)).willReturn(this.generatedToken);
this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request,
this.response);
Expand All @@ -121,7 +128,7 @@ public void delaySavingCsrf() {
any(HttpServletResponse.class));
CsrfToken tokenInRequest = (CsrfToken) this.request.getAttribute(CsrfToken.class.getName());
tokenInRequest.getToken();
verify(this.csrfTokenRepository).loadToken(this.request);
verify(this.csrfTokenRepository, times(2)).loadToken(this.request);
verify(this.csrfTokenRepository).generateToken(this.request);
verify(this.csrfTokenRepository).saveToken(eq(this.generatedToken), any(HttpServletRequest.class),
any(HttpServletResponse.class));
Expand Down

0 comments on commit 2ed7cff

Please sign in to comment.