CookieServerCsrfTokenRepository fails when cookie is null/empty

The CookieServerCsrfTokenRepository fails with an IllegalArgumentException
 when a cookie is present but the value is null or empty.

Fixes gh-5315
This commit is contained in:
Eric Deandrea 2018-05-07 13:29:16 -04:00 committed by Rob Winch
parent 9b42831c70
commit b3c5bfe4db
2 changed files with 30 additions and 9 deletions

View File

@ -87,7 +87,7 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep
public Mono<CsrfToken> loadToken(ServerWebExchange exchange) { public Mono<CsrfToken> loadToken(ServerWebExchange exchange) {
return Mono.fromCallable(() -> { return Mono.fromCallable(() -> {
HttpCookie csrfCookie = exchange.getRequest().getCookies().getFirst(this.cookieName); HttpCookie csrfCookie = exchange.getRequest().getCookies().getFirst(this.cookieName);
if (csrfCookie == null) { if ((csrfCookie == null) || !StringUtils.hasText(csrfCookie.getValue())) {
return null; return null;
} }
return createCsrfToken(csrfCookie.getValue()); return createCsrfToken(csrfCookie.getValue());
@ -123,7 +123,6 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep
/** /**
* Sets the header name * Sets the header name
* @param headerName The header name * @param headerName The header name
* @return This instance
*/ */
public void setHeaderName(String headerName) { public void setHeaderName(String headerName) {
Assert.hasLength(headerName, "headerName can't be null"); Assert.hasLength(headerName, "headerName can't be null");
@ -133,7 +132,6 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep
/** /**
* Sets the cookie path * Sets the cookie path
* @param cookiePath The cookie path * @param cookiePath The cookie path
* @return This instance
*/ */
public void setCookiePath(String cookiePath) { public void setCookiePath(String cookiePath) {
this.cookiePath = cookiePath; this.cookiePath = cookiePath;
@ -142,13 +140,11 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep
/** /**
* Sets the cookie domain * Sets the cookie domain
* @param cookieDomain The cookie domain * @param cookieDomain The cookie domain
* @return This instance
*/ */
public void setCookieDomain(String cookieDomain) { public void setCookieDomain(String cookieDomain) {
this.cookieDomain = cookieDomain; this.cookieDomain = cookieDomain;
} }
private CsrfToken createCsrfToken() { private CsrfToken createCsrfToken() {
return createCsrfToken(createNewToken()); return createCsrfToken(createNewToken());
} }

View File

@ -21,10 +21,12 @@ import static org.assertj.core.api.Assertions.assertThat;
import java.time.Duration; import java.time.Duration;
import org.junit.Test; import org.junit.Test;
import org.springframework.http.HttpCookie; import org.springframework.http.HttpCookie;
import org.springframework.http.ResponseCookie; import org.springframework.http.ResponseCookie;
import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.mock.web.server.MockServerWebExchange;
import org.springframework.util.StringUtils;
/** /**
* @author Eric Deandrea * @author Eric Deandrea
@ -138,6 +140,20 @@ public class CookieServerCsrfTokenRepositoryTests {
assertThat(csrfToken).isNull(); assertThat(csrfToken).isNull();
} }
@Test
public void loadTokenWhenCookieExistsWithNoValue() {
setExpectedCookieValue("");
loadAndAssertExpectedValues();
}
@Test
public void loadTokenWhenCookieExistsWithNullValue() {
setExpectedCookieValue(null);
loadAndAssertExpectedValues();
}
private void setExpectedHeaderName(String expectedHeaderName) { private void setExpectedHeaderName(String expectedHeaderName) {
this.csrfTokenRepository.setHeaderName(expectedHeaderName); this.csrfTokenRepository.setHeaderName(expectedHeaderName);
this.expectedHeaderName = expectedHeaderName; this.expectedHeaderName = expectedHeaderName;
@ -168,6 +184,10 @@ public class CookieServerCsrfTokenRepositoryTests {
this.csrfTokenRepository.setCookieName(expectedCookieName); this.csrfTokenRepository.setCookieName(expectedCookieName);
} }
private void setExpectedCookieValue(String expectedCookieValue) {
this.expectedCookieValue = expectedCookieValue;
}
private void loadAndAssertExpectedValues() { private void loadAndAssertExpectedValues() {
MockServerHttpRequest.BodyBuilder request = MockServerHttpRequest.post("/someUri") MockServerHttpRequest.BodyBuilder request = MockServerHttpRequest.post("/someUri")
.cookie(new HttpCookie(this.expectedCookieName, .cookie(new HttpCookie(this.expectedCookieName,
@ -176,10 +196,15 @@ public class CookieServerCsrfTokenRepositoryTests {
CsrfToken csrfToken = this.csrfTokenRepository.loadToken(this.exchange).block(); CsrfToken csrfToken = this.csrfTokenRepository.loadToken(this.exchange).block();
assertThat(csrfToken).isNotNull(); if (StringUtils.hasText(this.expectedCookieValue)) {
assertThat(csrfToken.getHeaderName()).isEqualTo(this.expectedHeaderName); assertThat(csrfToken).isNotNull();
assertThat(csrfToken.getParameterName()).isEqualTo(this.expectedParameterName); assertThat(csrfToken.getHeaderName()).isEqualTo(this.expectedHeaderName);
assertThat(csrfToken.getToken()).isEqualTo(this.expectedCookieValue); assertThat(csrfToken.getParameterName()).isEqualTo(this.expectedParameterName);
assertThat(csrfToken.getToken()).isEqualTo(this.expectedCookieValue);
}
else {
assertThat(csrfToken).isNull();
}
} }
private void saveAndAssertExpectedValues(CsrfToken token) { private void saveAndAssertExpectedValues(CsrfToken token) {