diff --git a/web/src/main/java/org/springframework/security/web/csrf/CsrfTokenRequestAttributeHandler.java b/web/src/main/java/org/springframework/security/web/csrf/CsrfTokenRequestAttributeHandler.java index 7e9a124c4d..b3e4d02ad4 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CsrfTokenRequestAttributeHandler.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CsrfTokenRequestAttributeHandler.java @@ -60,7 +60,6 @@ public class CsrfTokenRequestAttributeHandler implements CsrfTokenRequestHandler Assert.notNull(response, "response cannot be null"); Assert.notNull(deferredCsrfToken, "deferredCsrfToken cannot be null"); - request.setAttribute(HttpServletResponse.class.getName(), response); CsrfToken csrfToken = new SupplierCsrfToken(deferredCsrfToken); request.setAttribute(CsrfToken.class.getName(), csrfToken); String csrfAttrName = (this.csrfRequestAttributeName != null) ? this.csrfRequestAttributeName diff --git a/web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java deleted file mode 100644 index a8326fa2a7..0000000000 --- a/web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java +++ /dev/null @@ -1,246 +0,0 @@ -/* - * Copyright 2012-2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.csrf; - -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; - -import org.springframework.util.Assert; - -/** - * A {@link CsrfTokenRepository} that delays saving new {@link CsrfToken} until the - * attributes of the {@link CsrfToken} that were generated are accessed. - * - * @author Rob Winch - * @since 4.1 - * @deprecated Use - * {@link CsrfTokenRepository#loadDeferredToken(HttpServletRequest, HttpServletResponse)} - * which returns a {@link DeferredCsrfToken} - */ -@Deprecated -public final class LazyCsrfTokenRepository implements CsrfTokenRepository { - - /** - * The {@link HttpServletRequest} attribute name that the {@link HttpServletResponse} - * must be on. - */ - private static final String HTTP_RESPONSE_ATTR = HttpServletResponse.class.getName(); - - private final CsrfTokenRepository delegate; - - private boolean deferLoadToken; - - /** - * Creates a new instance - * @param delegate the {@link CsrfTokenRepository} to use. Cannot be null - * @throws IllegalArgumentException if delegate is null. - */ - public LazyCsrfTokenRepository(CsrfTokenRepository delegate) { - Assert.notNull(delegate, "delegate cannot be null"); - this.delegate = delegate; - } - - /** - * Determines if {@link #loadToken(HttpServletRequest)} should be lazily loaded. - * @param deferLoadToken true if should lazily load - * {@link #loadToken(HttpServletRequest)}. Default false. - */ - public void setDeferLoadToken(boolean deferLoadToken) { - this.deferLoadToken = deferLoadToken; - } - - /** - * Generates a new token - * @param request the {@link HttpServletRequest} to use. The - * {@link HttpServletRequest} must have the {@link HttpServletResponse} as an - * attribute with the name of HttpServletResponse.class.getName() - */ - @Override - public CsrfToken generateToken(HttpServletRequest request) { - return wrap(request, this.delegate.generateToken(request)); - } - - /** - * Does nothing if the {@link CsrfToken} is not null. Saving is done only when the - * {@link CsrfToken#getToken()} is accessed from - * {@link #generateToken(HttpServletRequest)}. If it is null, then the save is - * performed immediately. - */ - @Override - public void saveToken(CsrfToken token, HttpServletRequest request, HttpServletResponse response) { - if (token == null) { - this.delegate.saveToken(token, request, response); - } - } - - /** - * Delegates to the injected {@link CsrfTokenRepository} - */ - @Override - public CsrfToken loadToken(HttpServletRequest request) { - if (this.deferLoadToken) { - return new LazyLoadCsrfToken(request, this.delegate); - } - return this.delegate.loadToken(request); - } - - private CsrfToken wrap(HttpServletRequest request, CsrfToken token) { - HttpServletResponse response = getResponse(request); - return new SaveOnAccessCsrfToken(this.delegate, request, response, token); - } - - private HttpServletResponse getResponse(HttpServletRequest request) { - HttpServletResponse response = (HttpServletResponse) request.getAttribute(HTTP_RESPONSE_ATTR); - Assert.notNull(response, () -> "The HttpServletRequest attribute must contain an HttpServletResponse " - + "for the attribute " + HTTP_RESPONSE_ATTR); - return response; - } - - private final class LazyLoadCsrfToken implements CsrfToken { - - private final HttpServletRequest request; - - private final CsrfTokenRepository tokenRepository; - - private CsrfToken token; - - private LazyLoadCsrfToken(HttpServletRequest request, CsrfTokenRepository tokenRepository) { - this.request = request; - this.tokenRepository = tokenRepository; - } - - private CsrfToken getDelegate() { - if (this.token != null) { - return this.token; - } - // load from the delegate repository - this.token = LazyCsrfTokenRepository.this.delegate.loadToken(this.request); - if (this.token == null) { - // return a generated token that is lazily saved since - // LazyCsrfTokenRepository#loadToken always returns a value - this.token = generateToken(this.request); - } - return this.token; - } - - @Override - public String getHeaderName() { - return getDelegate().getHeaderName(); - } - - @Override - public String getParameterName() { - return getDelegate().getParameterName(); - } - - @Override - public String getToken() { - return getDelegate().getToken(); - } - - @Override - public String toString() { - return "LazyLoadCsrfToken{" + "token=" + this.token + '}'; - } - - } - - @SuppressWarnings("serial") - private static final class SaveOnAccessCsrfToken implements CsrfToken { - - private transient CsrfTokenRepository tokenRepository; - - private transient HttpServletRequest request; - - private transient HttpServletResponse response; - - private final CsrfToken delegate; - - SaveOnAccessCsrfToken(CsrfTokenRepository tokenRepository, HttpServletRequest request, - HttpServletResponse response, CsrfToken delegate) { - this.tokenRepository = tokenRepository; - this.request = request; - this.response = response; - this.delegate = delegate; - } - - @Override - public String getHeaderName() { - return this.delegate.getHeaderName(); - } - - @Override - public String getParameterName() { - return this.delegate.getParameterName(); - } - - @Override - public String getToken() { - saveTokenIfNecessary(); - return this.delegate.getToken(); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj == null || getClass() != obj.getClass()) { - return false; - } - SaveOnAccessCsrfToken other = (SaveOnAccessCsrfToken) obj; - if (this.delegate == null) { - if (other.delegate != null) { - return false; - } - } - else if (!this.delegate.equals(other.delegate)) { - return false; - } - return true; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((this.delegate == null) ? 0 : this.delegate.hashCode()); - return result; - } - - @Override - public String toString() { - return "SaveOnAccessCsrfToken [delegate=" + this.delegate + "]"; - } - - private void saveTokenIfNecessary() { - if (this.tokenRepository == null) { - return; - } - synchronized (this) { - if (this.tokenRepository != null) { - this.tokenRepository.saveToken(this.delegate, this.request, this.response); - this.tokenRepository = null; - this.request = null; - this.response = null; - } - } - } - - } - -} diff --git a/web/src/test/java/org/springframework/security/web/csrf/CsrfAuthenticationStrategyTests.java b/web/src/test/java/org/springframework/security/web/csrf/CsrfAuthenticationStrategyTests.java index 6c4199c1ab..c3ced0b459 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CsrfAuthenticationStrategyTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CsrfAuthenticationStrategyTests.java @@ -118,9 +118,10 @@ public class CsrfAuthenticationStrategyTests { // SEC-2872 @Test public void delaySavingCsrf() { - this.strategy = new CsrfAuthenticationStrategy(new LazyCsrfTokenRepository(this.csrfTokenRepository)); + this.strategy = new CsrfAuthenticationStrategy(this.csrfTokenRepository); given(this.csrfTokenRepository.loadToken(this.request)).willReturn(this.existingToken, (CsrfToken) null); given(this.csrfTokenRepository.generateToken(this.request)).willReturn(this.generatedToken); + given(this.csrfTokenRepository.loadDeferredToken(any(), any())).willCallRealMethod(); this.strategy.onAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"), this.request, this.response); verify(this.csrfTokenRepository).saveToken(null, this.request, this.response); diff --git a/web/src/test/java/org/springframework/security/web/csrf/CsrfFilterTests.java b/web/src/test/java/org/springframework/security/web/csrf/CsrfFilterTests.java index 818c074e61..4ee5afc62e 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CsrfFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CsrfFilterTests.java @@ -108,9 +108,10 @@ public class CsrfFilterTests { // SEC-2276 @Test public void doFilterDoesNotSaveCsrfTokenUntilAccessed() throws ServletException, IOException { - this.filter = createCsrfFilter(new LazyCsrfTokenRepository(this.tokenRepository)); + this.filter = createCsrfFilter(this.tokenRepository); given(this.requestMatcher.matches(this.request)).willReturn(false); given(this.tokenRepository.generateToken(this.request)).willReturn(this.token); + given(this.tokenRepository.loadDeferredToken(any(), any())).willCallRealMethod(); this.filter.doFilter(this.request, this.response, this.filterChain); CsrfToken attrToken = (CsrfToken) this.request.getAttribute(this.csrfAttrName); // no CsrfToken should have been saved yet @@ -278,8 +279,6 @@ public class CsrfFilterTests { assertThatCsrfToken(this.request.getAttribute(this.csrfAttrName)).isNotNull(); assertThatCsrfToken(this.request.getAttribute(CsrfToken.class.getName())).isNotNull(); assertThat(this.request.getAttribute(DeferredCsrfToken.class.getName())).isSameAs(deferredCsrfToken); - // LazyCsrfTokenRepository requires the response as an attribute - assertThat(this.request.getAttribute(HttpServletResponse.class.getName())).isEqualTo(this.response); verify(this.filterChain).doFilter(this.request, this.response); verifyNoMoreInteractions(this.deniedHandler); } diff --git a/web/src/test/java/org/springframework/security/web/csrf/LazyCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/csrf/LazyCsrfTokenRepositoryTests.java deleted file mode 100644 index e8e0114199..0000000000 --- a/web/src/test/java/org/springframework/security/web/csrf/LazyCsrfTokenRepositoryTests.java +++ /dev/null @@ -1,112 +0,0 @@ -/* - * Copyright 2012-2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.csrf; - -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyNoMoreInteractions; - -/** - * @author Rob Winch - */ -@ExtendWith(MockitoExtension.class) -public class LazyCsrfTokenRepositoryTests { - - @Mock - CsrfTokenRepository delegate; - - @Mock - HttpServletRequest request; - - @Mock - HttpServletResponse response; - - @InjectMocks - LazyCsrfTokenRepository repository; - - DefaultCsrfToken token; - - @BeforeEach - public void setup() { - this.token = new DefaultCsrfToken("header", "param", "token"); - } - - @Test - public void constructNullDelegateThrowsIllegalArgumentException() { - assertThatIllegalArgumentException().isThrownBy(() -> new LazyCsrfTokenRepository(null)); - } - - @Test - public void generateTokenNullResponseAttribute() { - assertThatIllegalArgumentException() - .isThrownBy(() -> this.repository.generateToken(mock(HttpServletRequest.class))); - } - - @Test - public void generateTokenGetTokenSavesToken() { - given(this.delegate.generateToken(this.request)).willReturn(this.token); - given(this.request.getAttribute(HttpServletResponse.class.getName())).willReturn(this.response); - CsrfToken newToken = this.repository.generateToken(this.request); - newToken.getToken(); - verify(this.delegate).saveToken(this.token, this.request, this.response); - } - - @Test - public void saveNonNullDoesNothing() { - this.repository.saveToken(this.token, this.request, this.response); - verifyNoMoreInteractions(this.delegate); - } - - @Test - public void saveNullDelegates() { - this.repository.saveToken(null, this.request, this.response); - verify(this.delegate).saveToken(null, this.request, this.response); - } - - @Test - public void loadTokenDelegates() { - given(this.delegate.loadToken(this.request)).willReturn(this.token); - CsrfToken loadToken = this.repository.loadToken(this.request); - assertThat(loadToken).isSameAs(this.token); - verify(this.delegate).loadToken(this.request); - } - - @Test - public void loadTokenWhenDeferLoadToken() { - given(this.delegate.loadToken(this.request)).willReturn(this.token); - this.repository.setDeferLoadToken(true); - CsrfToken loadToken = this.repository.loadToken(this.request); - verifyNoInteractions(this.delegate); - assertThat(loadToken.getToken()).isEqualTo(this.token.getToken()); - assertThat(loadToken.getHeaderName()).isEqualTo(this.token.getHeaderName()); - assertThat(loadToken.getParameterName()).isEqualTo(this.token.getParameterName()); - } - -}