|
| 1 | +From e2993d93e109c1a3c9020b7ea9efb6e556751ed4 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Thomas Vitale <ThomasVitale@users.noreply.github.com> |
| 3 | +Date: Mon, 26 Apr 2021 18:13:20 +0200 |
| 4 | +Subject: [PATCH 2/3] Make Csrf cookie secure flag configurable (WebFlux) |
| 5 | + |
| 6 | +Make the XSRF-TOKEN cookie secure flag configurable in CookieServerCsrfTokenRepository. |
| 7 | + |
| 8 | +Closes gh-9678 |
| 9 | +--- |
| 10 | + .../csrf/CookieServerCsrfTokenRepository.java | 30 ++++-- |
| 11 | + .../CookieServerCsrfTokenRepositoryTests.java | 100 ++++++++++++++++-- |
| 12 | + 2 files changed, 113 insertions(+), 17 deletions(-) |
| 13 | + |
| 14 | +diff --git a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java |
| 15 | +index 5910ff3e45..bc3a20e711 100644 |
| 16 | +--- a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java |
| 17 | ++++ b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java |
| 18 | +@@ -1,5 +1,5 @@ |
| 19 | + /* |
| 20 | +- * Copyright 2002-2019 the original author or authors. |
| 21 | ++ * Copyright 2002-2021 the original author or authors. |
| 22 | + * |
| 23 | + * Licensed under the Apache License, Version 2.0 (the "License"); |
| 24 | + * you may not use this file except in compliance with the License. |
| 25 | +@@ -34,6 +34,7 @@ import org.springframework.web.server.ServerWebExchange; |
| 26 | + * AngularJS. When using with AngularJS be sure to use {@link #withHttpOnlyFalse()} . |
| 27 | + * |
| 28 | + * @author Eric Deandrea |
| 29 | ++ * @author Thomas Vitale |
| 30 | + * @since 5.1 |
| 31 | + */ |
| 32 | + public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRepository { |
| 33 | +@@ -54,6 +55,8 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep |
| 34 | + |
| 35 | + private boolean cookieHttpOnly = true; |
| 36 | + |
| 37 | ++ private Boolean secure; |
| 38 | ++ |
| 39 | + /** |
| 40 | + * Factory method to conveniently create an instance that has |
| 41 | + * {@link #setCookieHttpOnly(boolean)} set to false. |
| 42 | +@@ -75,11 +78,16 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep |
| 43 | + public Mono<Void> saveToken(ServerWebExchange exchange, CsrfToken token) { |
| 44 | + return Mono.fromRunnable(() -> { |
| 45 | + String tokenValue = (token != null) ? token.getToken() : ""; |
| 46 | +- int maxAge = !tokenValue.isEmpty() ? -1 : 0; |
| 47 | +- String path = (this.cookiePath != null) ? this.cookiePath : getRequestContext(exchange.getRequest()); |
| 48 | +- boolean secure = exchange.getRequest().getSslInfo() != null; |
| 49 | +- ResponseCookie cookie = ResponseCookie.from(this.cookieName, tokenValue).domain(this.cookieDomain) |
| 50 | +- .httpOnly(this.cookieHttpOnly).maxAge(maxAge).path(path).secure(secure).build(); |
| 51 | ++ // @formatter:off |
| 52 | ++ ResponseCookie cookie = ResponseCookie |
| 53 | ++ .from(this.cookieName, tokenValue) |
| 54 | ++ .domain(this.cookieDomain) |
| 55 | ++ .httpOnly(this.cookieHttpOnly) |
| 56 | ++ .maxAge(!tokenValue.isEmpty() ? -1 : 0) |
| 57 | ++ .path((this.cookiePath != null) ? this.cookiePath : getRequestContext(exchange.getRequest())) |
| 58 | ++ .secure((this.secure != null) ? this.secure : (exchange.getRequest().getSslInfo() != null)) |
| 59 | ++ .build(); |
| 60 | ++ // @formatter:on |
| 61 | + exchange.getResponse().addCookie(cookie); |
| 62 | + }); |
| 63 | + } |
| 64 | +@@ -146,6 +154,16 @@ public final class CookieServerCsrfTokenRepository implements ServerCsrfTokenRep |
| 65 | + this.cookieDomain = cookieDomain; |
| 66 | + } |
| 67 | + |
| 68 | ++ /** |
| 69 | ++ * Sets the cookie secure flag. If not set, the value depends on |
| 70 | ++ * {@link ServerHttpRequest#getSslInfo()}. |
| 71 | ++ * @param secure The value for the secure flag |
| 72 | ++ * @since 5.5 |
| 73 | ++ */ |
| 74 | ++ public void setSecure(boolean secure) { |
| 75 | ++ this.secure = secure; |
| 76 | ++ } |
| 77 | ++ |
| 78 | + private CsrfToken createCsrfToken() { |
| 79 | + return createCsrfToken(createNewToken()); |
| 80 | + } |
| 81 | +diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java |
| 82 | +index d16f131920..7160337053 100644 |
| 83 | +--- a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java |
| 84 | ++++ b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java |
| 85 | +@@ -1,5 +1,5 @@ |
| 86 | + /* |
| 87 | +- * Copyright 2002-2018 the original author or authors. |
| 88 | ++ * Copyright 2002-2021 the original author or authors. |
| 89 | + * |
| 90 | + * Licensed under the Apache License, Version 2.0 (the "License"); |
| 91 | + * you may not use this file except in compliance with the License. |
| 92 | +@@ -16,12 +16,15 @@ |
| 93 | + |
| 94 | + package org.springframework.security.web.server.csrf; |
| 95 | + |
| 96 | ++import java.security.cert.X509Certificate; |
| 97 | + import java.time.Duration; |
| 98 | + |
| 99 | ++import org.junit.Before; |
| 100 | + import org.junit.Test; |
| 101 | + |
| 102 | + import org.springframework.http.HttpCookie; |
| 103 | + import org.springframework.http.ResponseCookie; |
| 104 | ++import org.springframework.http.server.reactive.SslInfo; |
| 105 | + import org.springframework.mock.http.server.reactive.MockServerHttpRequest; |
| 106 | + import org.springframework.mock.web.server.MockServerWebExchange; |
| 107 | + import org.springframework.util.StringUtils; |
| 108 | +@@ -30,13 +33,14 @@ import static org.assertj.core.api.Assertions.assertThat; |
| 109 | + |
| 110 | + /** |
| 111 | + * @author Eric Deandrea |
| 112 | ++ * @author Thomas Vitale |
| 113 | + * @since 5.1 |
| 114 | + */ |
| 115 | + public class CookieServerCsrfTokenRepositoryTests { |
| 116 | + |
| 117 | +- private MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/someUri")); |
| 118 | ++ private CookieServerCsrfTokenRepository csrfTokenRepository; |
| 119 | + |
| 120 | +- private CookieServerCsrfTokenRepository csrfTokenRepository = new CookieServerCsrfTokenRepository(); |
| 121 | ++ private MockServerHttpRequest.BaseBuilder<?> request; |
| 122 | + |
| 123 | + private String expectedHeaderName = CookieServerCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME; |
| 124 | + |
| 125 | +@@ -56,6 +60,12 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 126 | + |
| 127 | + private String expectedCookieValue = "csrfToken"; |
| 128 | + |
| 129 | ++ @Before |
| 130 | ++ public void setUp() { |
| 131 | ++ this.csrfTokenRepository = new CookieServerCsrfTokenRepository(); |
| 132 | ++ this.request = MockServerHttpRequest.get("/someUri"); |
| 133 | ++ } |
| 134 | ++ |
| 135 | + @Test |
| 136 | + public void generateTokenWhenDefaultThenDefaults() { |
| 137 | + generateTokenAndAssertExpectedValues(); |
| 138 | +@@ -82,8 +92,9 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 139 | + |
| 140 | + @Test |
| 141 | + public void saveTokenWhenNoSubscriptionThenNotWritten() { |
| 142 | +- this.csrfTokenRepository.saveToken(this.exchange, createToken()); |
| 143 | +- assertThat(this.exchange.getResponse().getCookies().getFirst(this.expectedCookieName)).isNull(); |
| 144 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 145 | ++ this.csrfTokenRepository.saveToken(exchange, createToken()); |
| 146 | ++ assertThat(exchange.getResponse().getCookies().getFirst(this.expectedCookieName)).isNull(); |
| 147 | + } |
| 148 | + |
| 149 | + @Test |
| 150 | +@@ -112,6 +123,56 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 151 | + saveAndAssertExpectedValues(createToken()); |
| 152 | + } |
| 153 | + |
| 154 | ++ @Test |
| 155 | ++ public void saveTokenWhenSslInfoPresentThenSecure() { |
| 156 | ++ this.request.sslInfo(new MockSslInfo()); |
| 157 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 158 | ++ this.csrfTokenRepository.saveToken(exchange, createToken()).block(); |
| 159 | ++ ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); |
| 160 | ++ assertThat(cookie).isNotNull(); |
| 161 | ++ assertThat(cookie.isSecure()).isTrue(); |
| 162 | ++ } |
| 163 | ++ |
| 164 | ++ @Test |
| 165 | ++ public void saveTokenWhenSslInfoNullThenNotSecure() { |
| 166 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 167 | ++ this.csrfTokenRepository.saveToken(exchange, createToken()).block(); |
| 168 | ++ ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); |
| 169 | ++ assertThat(cookie).isNotNull(); |
| 170 | ++ assertThat(cookie.isSecure()).isFalse(); |
| 171 | ++ } |
| 172 | ++ |
| 173 | ++ @Test |
| 174 | ++ public void saveTokenWhenSecureFlagTrueThenSecure() { |
| 175 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 176 | ++ this.csrfTokenRepository.setSecure(true); |
| 177 | ++ this.csrfTokenRepository.saveToken(exchange, createToken()).block(); |
| 178 | ++ ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); |
| 179 | ++ assertThat(cookie).isNotNull(); |
| 180 | ++ assertThat(cookie.isSecure()).isTrue(); |
| 181 | ++ } |
| 182 | ++ |
| 183 | ++ @Test |
| 184 | ++ public void saveTokenWhenSecureFlagFalseThenNotSecure() { |
| 185 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 186 | ++ this.csrfTokenRepository.setSecure(false); |
| 187 | ++ this.csrfTokenRepository.saveToken(exchange, createToken()).block(); |
| 188 | ++ ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); |
| 189 | ++ assertThat(cookie).isNotNull(); |
| 190 | ++ assertThat(cookie.isSecure()).isFalse(); |
| 191 | ++ } |
| 192 | ++ |
| 193 | ++ @Test |
| 194 | ++ public void saveTokenWhenSecureFlagFalseAndSslInfoThenNotSecure() { |
| 195 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 196 | ++ this.request.sslInfo(new MockSslInfo()); |
| 197 | ++ this.csrfTokenRepository.setSecure(false); |
| 198 | ++ this.csrfTokenRepository.saveToken(exchange, createToken()).block(); |
| 199 | ++ ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); |
| 200 | ++ assertThat(cookie).isNotNull(); |
| 201 | ++ assertThat(cookie.isSecure()).isFalse(); |
| 202 | ++ } |
| 203 | ++ |
| 204 | + @Test |
| 205 | + public void loadTokenWhenCookieExistThenTokenFound() { |
| 206 | + loadAndAssertExpectedValues(); |
| 207 | +@@ -127,7 +188,8 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 208 | + |
| 209 | + @Test |
| 210 | + public void loadTokenWhenNoCookiesThenNullToken() { |
| 211 | +- CsrfToken csrfToken = this.csrfTokenRepository.loadToken(this.exchange).block(); |
| 212 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 213 | ++ CsrfToken csrfToken = this.csrfTokenRepository.loadToken(exchange).block(); |
| 214 | + assertThat(csrfToken).isNull(); |
| 215 | + } |
| 216 | + |
| 217 | +@@ -180,8 +242,8 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 218 | + private void loadAndAssertExpectedValues() { |
| 219 | + MockServerHttpRequest.BodyBuilder request = MockServerHttpRequest.post("/someUri") |
| 220 | + .cookie(new HttpCookie(this.expectedCookieName, this.expectedCookieValue)); |
| 221 | +- this.exchange = MockServerWebExchange.from(request); |
| 222 | +- CsrfToken csrfToken = this.csrfTokenRepository.loadToken(this.exchange).block(); |
| 223 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(request); |
| 224 | ++ CsrfToken csrfToken = this.csrfTokenRepository.loadToken(exchange).block(); |
| 225 | + if (StringUtils.hasText(this.expectedCookieValue)) { |
| 226 | + assertThat(csrfToken).isNotNull(); |
| 227 | + assertThat(csrfToken.getHeaderName()).isEqualTo(this.expectedHeaderName); |
| 228 | +@@ -198,8 +260,9 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 229 | + this.expectedMaxAge = Duration.ofSeconds(0); |
| 230 | + this.expectedCookieValue = ""; |
| 231 | + } |
| 232 | +- this.csrfTokenRepository.saveToken(this.exchange, token).block(); |
| 233 | +- ResponseCookie cookie = this.exchange.getResponse().getCookies().getFirst(this.expectedCookieName); |
| 234 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 235 | ++ this.csrfTokenRepository.saveToken(exchange, token).block(); |
| 236 | ++ ResponseCookie cookie = exchange.getResponse().getCookies().getFirst(this.expectedCookieName); |
| 237 | + assertThat(cookie).isNotNull(); |
| 238 | + assertThat(cookie.getMaxAge()).isEqualTo(this.expectedMaxAge); |
| 239 | + assertThat(cookie.getDomain()).isEqualTo(this.expectedDomain); |
| 240 | +@@ -211,7 +274,8 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 241 | + } |
| 242 | + |
| 243 | + private void generateTokenAndAssertExpectedValues() { |
| 244 | +- CsrfToken csrfToken = this.csrfTokenRepository.generateToken(this.exchange).block(); |
| 245 | ++ MockServerWebExchange exchange = MockServerWebExchange.from(this.request); |
| 246 | ++ CsrfToken csrfToken = this.csrfTokenRepository.generateToken(exchange).block(); |
| 247 | + assertThat(csrfToken).isNotNull(); |
| 248 | + assertThat(csrfToken.getHeaderName()).isEqualTo(this.expectedHeaderName); |
| 249 | + assertThat(csrfToken.getParameterName()).isEqualTo(this.expectedParameterName); |
| 250 | +@@ -226,4 +290,18 @@ public class CookieServerCsrfTokenRepositoryTests { |
| 251 | + return new DefaultCsrfToken(headerName, parameterName, tokenValue); |
| 252 | + } |
| 253 | + |
| 254 | ++ static class MockSslInfo implements SslInfo { |
| 255 | ++ |
| 256 | ++ @Override |
| 257 | ++ public String getSessionId() { |
| 258 | ++ return "sessionId"; |
| 259 | ++ } |
| 260 | ++ |
| 261 | ++ @Override |
| 262 | ++ public X509Certificate[] getPeerCertificates() { |
| 263 | ++ return new X509Certificate[] {}; |
| 264 | ++ } |
| 265 | ++ |
| 266 | ++ } |
| 267 | ++ |
| 268 | + } |
| 269 | +-- |
| 270 | +2.24.1 |
| 271 | + |
0 commit comments