Browse Source

Remove LazyCsrfTokenRepository

Closes gh-13196
Josh Cummings 1 month ago
parent
commit
c06b1f4916

+ 0 - 1
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

+ 0 - 246
web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java

@@ -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 <code>HttpServletResponse.class.getName()</code>
-	 */
-	@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;
-				}
-			}
-		}
-
-	}
-
-}

+ 2 - 1
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);

+ 2 - 3
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);
 	}

+ 0 - 112
web/src/test/java/org/springframework/security/web/csrf/LazyCsrfTokenRepositoryTests.java

@@ -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());
-	}
-
-}