From 05e4a1dd20af04bd64fca81b096879bebbfb5d14 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Wed, 12 Oct 2022 10:27:09 -0500 Subject: [PATCH] Cache Xor CsrfToken Closes gh-11988 --- .../XorCsrfTokenRequestAttributeHandler.java | 24 +++++++++++++++++-- ...erverCsrfTokenRequestAttributeHandler.java | 3 ++- ...CsrfTokenRequestAttributeHandlerTests.java | 9 +++++++ ...CsrfTokenRequestAttributeHandlerTests.java | 11 +++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandler.java b/web/src/main/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandler.java index 160b51d19e..e66d075486 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandler.java +++ b/web/src/main/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandler.java @@ -59,12 +59,12 @@ public final class XorCsrfTokenRequestAttributeHandler extends CsrfTokenRequestA } private Supplier deferCsrfTokenUpdate(Supplier csrfTokenSupplier) { - return () -> { + return new CachedCsrfTokenSupplier(() -> { CsrfToken csrfToken = csrfTokenSupplier.get(); Assert.state(csrfToken != null, "csrfToken supplier returned null"); String updatedToken = createXoredCsrfToken(this.secureRandom, csrfToken.getToken()); return new DefaultCsrfToken(csrfToken.getHeaderName(), csrfToken.getParameterName(), updatedToken); - }; + }); } @Override @@ -123,4 +123,24 @@ public final class XorCsrfTokenRequestAttributeHandler extends CsrfTokenRequestA return xoredCsrf; } + private static final class CachedCsrfTokenSupplier implements Supplier { + + private final Supplier delegate; + + private CsrfToken csrfToken; + + private CachedCsrfTokenSupplier(Supplier delegate) { + this.delegate = delegate; + } + + @Override + public CsrfToken get() { + if (this.csrfToken == null) { + this.csrfToken = this.delegate.get(); + } + return this.csrfToken; + } + + } + } diff --git a/web/src/main/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandler.java b/web/src/main/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandler.java index 8e1c53ece1..12edaf0420 100644 --- a/web/src/main/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandler.java +++ b/web/src/main/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandler.java @@ -53,7 +53,8 @@ public final class XorServerCsrfTokenRequestAttributeHandler extends ServerCsrfT Assert.notNull(exchange, "exchange cannot be null"); Assert.notNull(csrfToken, "csrfToken cannot be null"); Mono updatedCsrfToken = csrfToken.map((token) -> new DefaultCsrfToken(token.getHeaderName(), - token.getParameterName(), createXoredCsrfToken(this.secureRandom, token.getToken()))); + token.getParameterName(), createXoredCsrfToken(this.secureRandom, token.getToken()))) + .cast(CsrfToken.class).cache(); super.handle(exchange, updatedCsrfToken); } diff --git a/web/src/test/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandlerTests.java b/web/src/test/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandlerTests.java index 8b03382451..349e337d9f 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/XorCsrfTokenRequestAttributeHandlerTests.java @@ -148,6 +148,15 @@ public class XorCsrfTokenRequestAttributeHandlerTests { assertThat(csrfTokenAttribute.getToken()).isEqualTo(XOR_CSRF_TOKEN_VALUE); } + @Test + public void handleWhenCsrfTokenRequestedTwiceThenCached() { + this.handler.handle(this.request, this.response, () -> this.token); + + CsrfToken csrfTokenAttribute = (CsrfToken) this.request.getAttribute(CsrfToken.class.getName()); + assertThat(csrfTokenAttribute.getToken()).isNotEqualTo(this.token.getToken()); + assertThat(csrfTokenAttribute.getToken()).isEqualTo(csrfTokenAttribute.getToken()); + } + @Test public void resolveCsrfTokenValueWhenRequestIsNullThenThrowsIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.handler.resolveCsrfTokenValue(null, this.token)) diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandlerTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandlerTests.java index 43fdf2f3d7..258dccd521 100644 --- a/web/src/test/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandlerTests.java +++ b/web/src/test/java/org/springframework/security/web/server/csrf/XorServerCsrfTokenRequestAttributeHandlerTests.java @@ -110,6 +110,17 @@ public class XorServerCsrfTokenRequestAttributeHandlerTests { verify(this.secureRandom).nextBytes(anyByteArray()); } + @Test + public void handleWhenCsrfTokenRequestedTwiceThenCached() { + this.handler.handle(this.exchange, Mono.just(this.token)); + Mono csrfTokenAttribute = this.exchange.getAttribute(CsrfToken.class.getName()); + assertThat(csrfTokenAttribute).isNotNull(); + CsrfToken csrfToken1 = csrfTokenAttribute.block(); + CsrfToken csrfToken2 = csrfTokenAttribute.block(); + assertThat(csrfToken1.getToken()).isNotEqualTo(this.token.getToken()); + assertThat(csrfToken1.getToken()).isEqualTo(csrfToken2.getToken()); + } + @Test public void resolveCsrfTokenValueWhenExchangeIsNullThenThrowsIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.handler.resolveCsrfTokenValue(null, this.token))