Polish CookieServerCsrfTokenRepository

- Only do work if subscribed to
- use test naming conventions
- Refactor tests to avoid extracting
  - Uses String for member names which are not type safe
  - Uses long argument list which makes assertions difficult to read

Issue: gh-5083

web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java

@@ -21,8 +21,6 @@ import java.util.UUID;
 
 import org.springframework.http.HttpCookie;
 import org.springframework.http.ResponseCookie;
-import org.springframework.http.server.PathContainer;
-import org.springframework.http.server.RequestPath;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -70,30 +68,30 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep
 
 	@Override
 	public Mono<Void> saveToken(ServerWebExchange exchange, CsrfToken token) {
-		Optional<String> tokenValue = Optional.ofNullable(token).map(CsrfToken::getToken);
+		return Mono.fromRunnable(() -> {
+			Optional<String> tokenValue = Optional.ofNullable(token).map(CsrfToken::getToken);
 
-		ResponseCookie cookie = ResponseCookie.from(this.cookieName, tokenValue.orElse(""))
+			ResponseCookie cookie = ResponseCookie.from(this.cookieName, tokenValue.orElse(""))
-			.domain(this.cookieDomain)
+					.domain(this.cookieDomain)
-			.httpOnly(this.cookieHttpOnly)
+					.httpOnly(this.cookieHttpOnly)
-			.maxAge(tokenValue.map(val -> -1).orElse(0))
+					.maxAge(tokenValue.map(val -> -1).orElse(0))
-			.path(Optional.ofNullable(this.cookiePath).orElseGet(() -> getRequestContext(exchange.getRequest())))
+					.path(Optional.ofNullable(this.cookiePath).orElseGet(() -> getRequestContext(exchange.getRequest())))
-			.secure(Optional.ofNullable(exchange.getRequest().getSslInfo()).map(sslInfo -> true).orElse(false))
+					.secure(Optional.ofNullable(exchange.getRequest().getSslInfo()).map(sslInfo -> true).orElse(false))
-			.build();
+					.build();
 
-		exchange.getResponse().addCookie(cookie);
+			exchange.getResponse().addCookie(cookie);
-
+		});
-		return Mono.empty();
 	}
 
 	@Override
 	public Mono<CsrfToken> loadToken(ServerWebExchange exchange) {
-		Optional<CsrfToken> token = Optional.ofNullable(exchange.getRequest())
+		return Mono.fromCallable(() -> {
-			.map(ServerHttpRequest::getCookies)
+			HttpCookie csrfCookie = exchange.getRequest().getCookies().getFirst(this.cookieName);
-			.map(cookiesMap -> cookiesMap.getFirst(this.cookieName))
+			if (csrfCookie == null) {
-			.map(HttpCookie::getValue)
+				return null;
-			.map(this::createCsrfToken);
+			}
-
+			return createCsrfToken(csrfCookie.getValue());
-		return Mono.justOrEmpty(token);
+		});
 	}
 
 	/**

web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java

@@ -19,236 +19,208 @@ package org.springframework.security.web.server.csrf;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.time.Duration;
-import java.util.function.Supplier;
 
 import org.junit.Test;
-
+import org.springframework.http.HttpCookie;
 import org.springframework.http.ResponseCookie;
 import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
 import org.springframework.mock.web.server.MockServerWebExchange;
 
-import reactor.core.publisher.Mono;
-
 /**
  * @author Eric Deandrea
  * @since 5.1
  */
 public class CookieServerCsrfTokenRepositoryTests {
-	@Test
+	private MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
-	public void generateTokenDefault() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new);
-		Mono<CsrfToken> csrfTokenMono = csrfTokenRepository.generateToken(exchange);
 
-		assertThat(csrfTokenMono).isNotNull();
+	private CookieServerCsrfTokenRepository csrfTokenRepository = new CookieServerCsrfTokenRepository();
-		assertThat(csrfTokenMono.block())
+
-			.isNotNull()
+	private String expectedHeaderName = CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME;
-			.extracting("headerName", "parameterName")
+
-			.containsExactly(CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME, CookieServerCsrfTokenRepository.DEFAULT_CSRF_PARAMETER_NAME);
+	private String expectedParameterName = CookieServerCsrfTokenRepository.DEFAULT_CSRF_PARAMETER_NAME;
-		assertThat(csrfTokenMono.block().getToken()).isNotBlank();
+
+	private Duration expectedMaxAge = Duration.ofSeconds(-1);
+
+	private String expectedDomain = null;
+
+	private String expectedPath = "/";
+
+	private boolean expectedSecure = false;
+
+	private boolean expectedHttpOnly = true;
+
+	private String expectedCookieName = CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
+
+	private String expectedCookieValue = "csrfToken";
+
+	@Test
+	public void generateTokenWhenDefaultThenDefaults() {
+		generateTokenAndAssertExpectedValues();
 	}
 
 	@Test
-	public void generateTokenChangeHeaderName() {
+	public void generateTokenWhenCustomHeaderThenCustomHeader() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
+		setExpectedHeaderName("someHeader");
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME,
-				"someHeader",
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_PARAMETER_NAME);
-		Mono<CsrfToken> csrfTokenMono = csrfTokenRepository.generateToken(exchange);
 
-		assertThat(csrfTokenMono).isNotNull();
+		generateTokenAndAssertExpectedValues();
-		assertThat(csrfTokenMono.block())
-			.isNotNull()
-			.extracting("headerName", "parameterName")
-			.containsExactly("someHeader", CookieServerCsrfTokenRepository.DEFAULT_CSRF_PARAMETER_NAME);
-		assertThat(csrfTokenMono.block().getToken()).isNotBlank();
 	}
 
 	@Test
-	public void generateTokenChangeParameterName() {
+	public void generateTokenWhenCustomParameterThenCustomParameter() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
+		setExpectedParameterName("someParam");
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME,
-				"someParam");
-		Mono<CsrfToken> csrfTokenMono = csrfTokenRepository.generateToken(exchange);
 
-		assertThat(csrfTokenMono).isNotNull();
+		generateTokenAndAssertExpectedValues();
-		assertThat(csrfTokenMono.block())
-			.isNotNull()
-			.extracting("headerName", "parameterName")
-			.containsExactly(CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME, "someParam");
-		assertThat(csrfTokenMono.block().getToken()).isNotBlank();
 	}
 
 	@Test
-	public void generateTokenChangeHeaderAndParameterName() {
+	public void generateTokenWhenCustomHeaderAndParameterThenCustomHeaderAndParameter() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
+		setExpectedHeaderName("someHeader");
-		CookieServerCsrfTokenRepository csrfTokenRepository =
+		setExpectedParameterName("someParam");
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME,
-				"someHeader",
-				"someParam");
-		Mono<CsrfToken> csrfTokenMono = csrfTokenRepository.generateToken(exchange);
 
-		assertThat(csrfTokenMono).isNotNull();
+		generateTokenAndAssertExpectedValues();
-		assertThat(csrfTokenMono.block())
-			.isNotNull()
-			.extracting("headerName", "parameterName")
-			.containsExactly("someHeader", "someParam");
-		assertThat(csrfTokenMono.block().getToken()).isNotBlank();
 	}
 
 	@Test
-	public void saveTokenDefault() {
+	public void saveTokenWhenNoSubscriptionThenNotWritten() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
+		this.csrfTokenRepository.saveToken(this.exchange, createToken());
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new);
 
-		Mono<Void> csrfTokenMono = csrfTokenRepository.saveToken(exchange, createToken("someTokenValue"));
+		assertThat(this.exchange
-		ResponseCookie cookie = exchange
+				.getResponse()
-			.getResponse()
+				.getCookies()
-			.getCookies()
+				.getFirst(this.expectedCookieName)).isNull();
-			.getFirst(CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
-
-		assertThat(csrfTokenMono).isNotNull();
-		assertThat(cookie)
-			.isNotNull()
-			.extracting("maxAge", "domain", "path", "secure", "httpOnly", "name", "value")
-			.containsExactly(Duration.ofSeconds(-1), null, "/", false, true, "XSRF-TOKEN", "someTokenValue");
 	}
 
 	@Test
-	public void saveTokenMaxAge() {
+	public void saveTokenWhenDefaultThenDefaults() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
+		saveAndAssertExpectedValues(createToken());
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new);
-
-		Mono<Void> csrfTokenMono = csrfTokenRepository.saveToken(exchange, null);
-		ResponseCookie cookie = exchange
-			.getResponse()
-			.getCookies()
-			.getFirst(CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
-
-		assertThat(csrfTokenMono).isNotNull();
-		assertThat(cookie)
-			.isNotNull()
-			.extracting("maxAge", "domain", "path", "secure", "httpOnly", "name", "value")
-			.containsExactly(Duration.ofSeconds(0), null, "/", false, true, "XSRF-TOKEN", "");
 	}
 
 	@Test
-	public void saveTokenHttpOnly() {
+	public void saveTokenWhenNullThenDeletes() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
+		saveAndAssertExpectedValues(null);
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::withHttpOnlyFalse);
-
-		Mono<Void> csrfTokenMono = csrfTokenRepository.saveToken(exchange, createToken("someTokenValue"));
-		ResponseCookie cookie = exchange
-			.getResponse()
-			.getCookies()
-			.getFirst(CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME);
-
-		assertThat(csrfTokenMono).isNotNull();
-		assertThat(cookie)
-			.isNotNull()
-			.extracting("maxAge", "domain", "path", "secure", "httpOnly", "name", "value")
-			.containsExactly(Duration.ofSeconds(-1), null, "/", false, false, "XSRF-TOKEN", "someTokenValue");
 	}
 
 	@Test
-	public void saveTokenOverriddenViaCsrfProps() {
+	public void saveTokenWhenHttpOnlyFalseThenHttpOnlyFalse() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri"));
+		setExpectedHttpOnly(false);
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new,
-			".spring.io",  "csrfCookie", "/some/path",
-				"headerName", "paramName");
 
-		Mono<Void> csrfTokenMono =
+		saveAndAssertExpectedValues(createToken());
-			csrfTokenRepository.saveToken(exchange, createToken("headerName", "paramName", "someTokenValue"));
-		ResponseCookie cookie = exchange.getResponse().getCookies().getFirst("csrfCookie");
-
-		assertThat(csrfTokenMono).isNotNull();
-		assertThat(cookie)
-			.isNotNull()
-			.extracting("maxAge", "domain", "path", "secure", "httpOnly", "name", "value")
-			.containsExactly(Duration.ofSeconds(-1), ".spring.io", "/some/path", false, true, "csrfCookie", "someTokenValue");
 	}
 
 	@Test
-	public void loadTokenThatExists() {
+	public void saveTokenWhenCustomPropertiesThenCustomProperties() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(
+		setExpectedDomain(".spring.io");
-			MockServerHttpRequest.post("/someUri")
+		setExpectedCookieName("csrfCookie");
-				.cookie(ResponseCookie.from(CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "someTokenValue").build()));
+		setExpectedPath("/some/path");
+		setExpectedHeaderName("headerName");
+		setExpectedParameterName("paramName");
 
-		CookieServerCsrfTokenRepository csrfTokenRepository =
+		saveAndAssertExpectedValues(createToken());
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new);
-		Mono<CsrfToken> csrfTokenMono = csrfTokenRepository.loadToken(exchange);
-
-		assertThat(csrfTokenMono).isNotNull();
-		assertThat(csrfTokenMono.block())
-			.isNotNull()
-			.extracting("headerName", "parameterName", "token")
-			.containsExactly(
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_PARAMETER_NAME,
-				"someTokenValue");
 	}
 
 	@Test
-	public void loadTokenThatDoesntExists() {
+	public void loadTokenWhenCookieExistThenTokenFound() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.post("/someUri"));
+		loadAndAssertExpectedValues();
-		CookieServerCsrfTokenRepository csrfTokenRepository =
-			CookieServerCsrfTokenRepositoryFactory.createRepository(CookieServerCsrfTokenRepository::new);
-
-		Mono<CsrfToken> csrfTokenMono = csrfTokenRepository.loadToken(exchange);
-		assertThat(csrfTokenMono).isNotNull();
-		assertThat(csrfTokenMono.block()).isNull();
 	}
 
-	private static CsrfToken createToken(String tokenValue) {
+	@Test
-		return createToken(CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME,
+	public void loadTokenWhenCustomThenTokenFound() {
-			CookieServerCsrfTokenRepository.DEFAULT_CSRF_PARAMETER_NAME, tokenValue);
+		setExpectedParameterName("paramName");
+		setExpectedHeaderName("headerName");
+		setExpectedCookieName("csrfCookie");
+
+		saveAndAssertExpectedValues(createToken());
+	}
+
+	@Test
+	public void loadTokenWhenNoCookiesThenNullToken() {
+		CsrfToken csrfToken = this.csrfTokenRepository.loadToken(this.exchange).block();
+		assertThat(csrfToken).isNull();
+	}
+
+	private void setExpectedHeaderName(String expectedHeaderName) {
+		this.csrfTokenRepository.setHeaderName(expectedHeaderName);
+		this.expectedHeaderName = expectedHeaderName;
+	}
+
+	private void setExpectedParameterName(String expectedParameterName) {
+		this.csrfTokenRepository.setParameterName(expectedParameterName);
+		this.expectedParameterName = expectedParameterName;
+	}
+
+	private void setExpectedDomain(String expectedDomain) {
+		this.csrfTokenRepository.setCookieDomain(expectedDomain);
+		this.expectedDomain = expectedDomain;
+	}
+
+	private void setExpectedPath(String expectedPath) {
+		this.csrfTokenRepository.setCookiePath(expectedPath);
+		this.expectedPath = expectedPath;
+	}
+
+	private void setExpectedHttpOnly(boolean expectedHttpOnly) {
+		this.expectedHttpOnly = expectedHttpOnly;
+		this.csrfTokenRepository.setCookieHttpOnly(expectedHttpOnly);
+	}
+
+	private void setExpectedCookieName(String expectedCookieName) {
+		this.expectedCookieName = expectedCookieName;
+		this.csrfTokenRepository.setCookieName(expectedCookieName);
+	}
+
+	private void loadAndAssertExpectedValues() {
+		MockServerHttpRequest.BodyBuilder request = MockServerHttpRequest.post("/someUri")
+				.cookie(new HttpCookie(this.expectedCookieName,
+						this.expectedCookieValue));
+		this.exchange = MockServerWebExchange.from(request);
+
+		CsrfToken csrfToken = this.csrfTokenRepository.loadToken(this.exchange).block();
+
+		assertThat(csrfToken).isNotNull();
+		assertThat(csrfToken.getHeaderName()).isEqualTo(this.expectedHeaderName);
+		assertThat(csrfToken.getParameterName()).isEqualTo(this.expectedParameterName);
+		assertThat(csrfToken.getToken()).isEqualTo(this.expectedCookieValue);
+	}
+
+	private void saveAndAssertExpectedValues(CsrfToken token) {
+		if (token == null) {
+			this.expectedMaxAge = Duration.ofSeconds(0);
+			this.expectedCookieValue = "";
+		}
+
+		this.csrfTokenRepository.saveToken(this.exchange, token).block();
+
+		ResponseCookie cookie =  this.exchange
+				.getResponse()
+				.getCookies()
+				.getFirst(this.expectedCookieName);
+
+		assertThat(cookie).isNotNull();
+		assertThat(cookie.getMaxAge()).isEqualTo(this.expectedMaxAge);
+		assertThat(cookie.getDomain()).isEqualTo(this.expectedDomain);
+		assertThat(cookie.getPath()).isEqualTo(this.expectedPath);
+		assertThat(cookie.isSecure()).isEqualTo(this.expectedSecure);
+		assertThat(cookie.isHttpOnly()).isEqualTo(this.expectedHttpOnly);
+		assertThat(cookie.getName()).isEqualTo(this.expectedCookieName);
+		assertThat(cookie.getValue()).isEqualTo(this.expectedCookieValue);
+	}
+
+	private void generateTokenAndAssertExpectedValues() {
+		CsrfToken csrfToken = this.csrfTokenRepository.generateToken(this.exchange).block();
+
+		assertThat(csrfToken).isNotNull();
+		assertThat(csrfToken.getHeaderName()).isEqualTo(this.expectedHeaderName);
+		assertThat(csrfToken.getParameterName()).isEqualTo(this.expectedParameterName);
+		assertThat(csrfToken.getToken()).isNotBlank();
+	}
+
+
+	private CsrfToken createToken() {
+		return createToken(this.expectedHeaderName,
+			this.expectedParameterName, this.expectedCookieValue);
 	}
 
 	private static CsrfToken createToken(String headerName, String parameterName, String tokenValue) {
 		return new DefaultCsrfToken(headerName, parameterName, tokenValue);
 	}
-
+}
-	static final class CookieServerCsrfTokenRepositoryFactory {
-		private CookieServerCsrfTokenRepositoryFactory() {
-			super();
-		}
-
-		static CookieServerCsrfTokenRepository createRepository(Supplier<CookieServerCsrfTokenRepository> cookieServerCsrfTokenRepositorySupplier) {
-			return createRepository(cookieServerCsrfTokenRepositorySupplier,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME,
-				CookieServerCsrfTokenRepository.DEFAULT_CSRF_PARAMETER_NAME);
-		}
-
-		static CookieServerCsrfTokenRepository createRepository(
-			Supplier<CookieServerCsrfTokenRepository> cookieServerCsrfTokenRepositorySupplier,
-			String cookieName, String headerName, String parameterName) {
-
-			return createRepository(cookieServerCsrfTokenRepositorySupplier,
-				null, cookieName, null, headerName, parameterName);
-		}
-
-		static CookieServerCsrfTokenRepository createRepository(
-			Supplier<CookieServerCsrfTokenRepository> cookieServerCsrfTokenRepositorySupplier,
-			String cookieDomain, String cookieName, String cookiePath, String headerName, String parameterName) {
-
-			return cookieServerCsrfTokenRepositorySupplier.get()
-				.withCookieDomain(cookieDomain)
-				.withCookieName(cookieName)
-				.withCookiePath(cookiePath)
-				.withHeaderName(headerName)
-				.withParameterName(parameterName);
-		}
-	}
-}