Skip to content

Commit 64ee4eb

Browse files
jzheauxjzheaux
committed
Allow Overriding ContentSecurityPolicy Matchers
Consistent with other DSL methods like HstsConfig#requestMatcher, the last call to requestMatcher(s) or exchangeMatcher(s) now wins instead of failing. Signed-off-by: Josh Cummings <[email protected]>
1 parent 275fc6a commit 64ee4eb

8 files changed

Lines changed: 65 additions & 55 deletions

File tree

‎config/src/main/java/org/springframework/security/config/annotation/web/configurers/HeadersConfigurer.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -953,8 +953,6 @@ public final class ContentSecurityPolicyConfig {
953953

954954
private final ContentSecurityPolicyNonceGeneratingFilter nonceGeneratingFilter = new ContentSecurityPolicyNonceGeneratingFilter();
955955

956-
private @Nullable RequestMatcher requestMatcher;
957-
958956
private ContentSecurityPolicyConfig() {
959957
}
960958

@@ -1003,16 +1001,12 @@ public ContentSecurityPolicyConfig nonceAttributeName(String nonceAttributeName)
10031001
* @param requestMatcher the {@link RequestMatcher} to use
10041002
* @return the {@link ContentSecurityPolicyConfig} for additional configuration
10051003
* @throws IllegalArgumentException if {@code requestMatcher} is null
1006-
* @throws IllegalStateException if a {@link RequestMatcher} is already configured
1007-
* by a previous call of this method or {@link #requestMatchers(String...)}
10081004
* @since 7.2
10091005
* @see #requestMatchers(String...)
10101006
*/
10111007
public ContentSecurityPolicyConfig requestMatcher(RequestMatcher requestMatcher) {
10121008
Assert.notNull(requestMatcher, "RequestMatcher cannot be null");
1013-
Assert.state(this.requestMatcher == null, "RequestMatcher(s) is already configured");
10141009
this.writer.setRequestMatcher(requestMatcher);
1015-
this.requestMatcher = requestMatcher;
10161010
return this;
10171011
}
10181012

@@ -1025,8 +1019,6 @@ public ContentSecurityPolicyConfig requestMatcher(RequestMatcher requestMatcher)
10251019
* @return the {@link ContentSecurityPolicyConfig} for additional configuration
10261020
* @throws IllegalArgumentException if any path pattern if rejected by
10271021
* {@link PathPatternRequestMatcher.Builder#matcher(String)}
1028-
* @throws IllegalStateException if a {@link RequestMatcher} is already configured
1029-
* by a previous call of this method or {@link #requestMatcher(RequestMatcher)}
10301022
* @since 7.2
10311023
* @see #requestMatcher(RequestMatcher)
10321024
*/

‎config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2846,8 +2846,6 @@ public final class ContentSecurityPolicySpec {
28462846

28472847
private static final String DEFAULT_SRC_SELF_POLICY = "default-src 'self'";
28482848

2849-
private @Nullable ServerWebExchangeMatcher exchangeMatcher;
2850-
28512849
private final ContentSecurityPolicyNonceGeneratingWebFilter nonceGeneratingFilter = new ContentSecurityPolicyNonceGeneratingWebFilter();
28522850

28532851
private ContentSecurityPolicySpec() {
@@ -2904,17 +2902,12 @@ public ContentSecurityPolicySpec nonceAttributeName(String nonceAttributeName) {
29042902
* @param matcher the {@link ServerWebExchangeMatcher} to use
29052903
* @return the {@link ContentSecurityPolicySpec} to continue configuring
29062904
* @throws IllegalArgumentException if {@code matcher} is {@code null}
2907-
* @throws IllegalStateException if a {@link ServerWebExchangeMatcher} is
2908-
* already configured by a previous call of this method or
2909-
* {@link #exchangeMatchers(String...)}
29102905
* @since 7.2
29112906
* @see #exchangeMatchers(String...)
29122907
*/
29132908
public ContentSecurityPolicySpec exchangeMatcher(ServerWebExchangeMatcher matcher) {
29142909
Assert.notNull(matcher, "Matcher must not be null");
2915-
Assert.state(this.exchangeMatcher == null, "ExchangeMatcher(s) is already configured");
29162910
HeaderSpec.this.contentSecurityPolicy.setExchangeMatcher(matcher);
2917-
this.exchangeMatcher = matcher;
29182911
return this;
29192912
}
29202913

@@ -2927,9 +2920,6 @@ public ContentSecurityPolicySpec exchangeMatcher(ServerWebExchangeMatcher matche
29272920
* @return the {@link ContentSecurityPolicySpec} to continue configuring
29282921
* @throws IllegalArgumentException if any path pattern is rejected by
29292922
* {@link PathPatternParserServerWebExchangeMatcher}
2930-
* @throws IllegalStateException if a {@link ServerWebExchangeMatcher} is
2931-
* already configured by a previous call of this method or
2932-
* {@link #exchangeMatcher(ServerWebExchangeMatcher)}
29332923
* @since 7.2
29342924
* @see #exchangeMatcher(ServerWebExchangeMatcher)
29352925
*/

‎config/src/main/kotlin/org/springframework/security/config/annotation/web/headers/ContentSecurityPolicyDsl.kt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,14 @@ class ContentSecurityPolicyDsl {
5252
* The default is to enable CSP in every response if
5353
* [org.springframework.security.config.annotation.web.HeadersDsl.contentSecurityPolicy]
5454
* is configured.
55-
* You can configure either this property or [requestMatchers], but not both.
5655
* @since 7.2
5756
* @see requestMatchers
5857
*/
5958
var requestMatcher: RequestMatcher? = null
59+
set(value) {
60+
field = value
61+
requestMatcherPatterns = null
62+
}
6063

6164
private var requestMatcherPatterns: Array<out String>? = null
6265

@@ -65,13 +68,13 @@ class ContentSecurityPolicyDsl {
6568
* The default is to write CSP header in every response if
6669
* [org.springframework.security.config.annotation.web.HeadersDsl.contentSecurityPolicy]
6770
* is configured.
68-
* You can configure either this method or [requestMatcher], but not both.
6971
* @param pathPatterns the path patterns to be matched with a
7072
* [org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher]
7173
* @since 7.2
7274
* @see requestMatcher
7375
*/
7476
fun requestMatchers(vararg pathPatterns: String) {
77+
requestMatcher = null
7578
requestMatcherPatterns = pathPatterns
7679
}
7780

‎config/src/main/kotlin/org/springframework/security/config/web/server/ServerContentSecurityPolicyDsl.kt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,25 +49,28 @@ class ServerContentSecurityPolicyDsl {
4949
* The [ServerWebExchangeMatcher] to use for determining when CSP should be applied.
5050
* The default is to enable CSP in every response if [ServerHeadersDsl.contentSecurityPolicy]
5151
* is configured.
52-
* You can configure either this property or [exchangeMatchers], but not both.
5352
* @since 7.2
5453
* @see exchangeMatchers
5554
*/
5655
var exchangeMatcher: ServerWebExchangeMatcher? = null
56+
set(value) {
57+
field = value
58+
exchangeMatcherPatterns = null
59+
}
5760

5861
private var exchangeMatcherPatterns: Array<out String>? = null
5962

6063
/**
6164
* Specify the matching path patterns for determining when CSP should be applied.
6265
* The default is to enable CSP in every response if [ServerHeadersDsl.contentSecurityPolicy]
6366
* is configured.
64-
* You can configure either this method or [exchangeMatcher], but not both.
6567
* @param pathPatterns the path patterns to be matched with a
6668
* [org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher]
6769
* @since 7.2
6870
* @see exchangeMatcher
6971
*/
7072
fun exchangeMatchers(vararg pathPatterns: String) {
73+
exchangeMatcher = null
7174
exchangeMatcherPatterns = pathPatterns
7275
}
7376

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/HeadersConfigurerTests.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
import org.springframework.web.bind.annotation.ResponseBody;
5454

5555
import static org.assertj.core.api.Assertions.assertThat;
56-
import static org.assertj.core.api.Assertions.assertThatException;
5756
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
5857
import static org.springframework.security.config.Customizer.withDefaults;
5958
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
@@ -481,12 +480,12 @@ public void configureWhenContentSecurityPolicyWithPathMatchersThenHeaderInRespon
481480
}
482481

483482
@Test
484-
public void configureWhenContentSecurityPolicyWithOverriddenMatchersThenThrows() {
485-
assertThatException()
486-
.isThrownBy(() -> this.spring.register(ContentSecurityPolicyOverriddenMatchersConfig.class).autowire())
487-
.havingRootCause()
488-
.isInstanceOf(IllegalStateException.class)
489-
.withMessage("RequestMatcher(s) is already configured");
483+
public void configureWhenContentSecurityPolicyWithOverriddenMatchersThenLastOneWins() throws Exception {
484+
this.spring.register(ContentSecurityPolicyOverriddenMatchersConfig.class).autowire();
485+
this.mvc.perform(get("/foo/bar").secure(true))
486+
.andExpect(header().string(HttpHeaders.CONTENT_SECURITY_POLICY, "default-src 'self'"));
487+
this.mvc.perform(get("/bar/foo").secure(true))
488+
.andExpect(header().doesNotExist(HttpHeaders.CONTENT_SECURITY_POLICY));
490489
}
491490

492491
@Test
@@ -1297,7 +1296,7 @@ SecurityFilterChain filterChain(HttpSecurity http) {
12971296
.contentSecurityPolicy((csp) -> csp
12981297
.policyDirectives("default-src 'self'")
12991298
.requestMatcher(AnyRequestMatcher.INSTANCE)
1300-
.requestMatchers("/**")));
1299+
.requestMatchers("/foo/**")));
13011300
// @formatter:on
13021301
return http.build();
13031302
}

‎config/src/test/java/org/springframework/security/config/web/server/HeaderSpecTests.java‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import org.springframework.web.bind.annotation.ResponseBody;
5353

5454
import static org.assertj.core.api.Assertions.assertThat;
55-
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
5655
import static org.springframework.security.config.Customizer.withDefaults;
5756

5857
/**
@@ -513,16 +512,25 @@ public void headersWhenContentSecurityPolicyEnabledWithPathMatchersThenHeaderInR
513512
}
514513

515514
@Test
516-
public void headersWhenContentSecurityPolicyWithOverriddenMatchersThenFailToConfigure() {
515+
public void headersWhenContentSecurityPolicyWithOverriddenMatchersThenLastOneWins() {
516+
String headerName = ContentSecurityPolicyServerHttpHeadersWriter.CONTENT_SECURITY_POLICY;
517517
// @formatter:off
518-
assertThatIllegalStateException()
519-
.isThrownBy(() -> this.http
520-
.headers((headers) -> headers
521-
.contentSecurityPolicy((csp) -> csp
522-
.exchangeMatcher(ServerWebExchangeMatchers.anyExchange())
523-
.exchangeMatchers("/**")
524-
.policyDirectives("default-src 'self'"))))
525-
.withMessage("ExchangeMatcher(s) is already configured");
518+
this.http.headers((headers) -> headers
519+
.contentSecurityPolicy((csp) -> csp
520+
.exchangeMatcher(ServerWebExchangeMatchers.anyExchange())
521+
.exchangeMatchers("/foo/**")
522+
.policyDirectives("default-src 'self'")));
523+
// @formatter:on
524+
WebTestClient client = WebTestClientBuilder.bindToWebFilters(this.http.build()).build();
525+
// @formatter:off
526+
client.get()
527+
.uri("https://example.com/foo/bar")
528+
.exchange()
529+
.expectHeader().valueEquals(headerName, "default-src 'self'");
530+
client.get()
531+
.uri("https://example.com/bar/foo")
532+
.exchange()
533+
.expectHeader().doesNotExist(headerName);
526534
// @formatter:on
527535
}
528536

‎config/src/test/kotlin/org/springframework/security/config/annotation/web/headers/ContentSecurityPolicyDslTests.kt‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.security.config.annotation.web.headers
1818

19-
import org.assertj.core.api.Assertions.assertThatThrownBy
2019
import org.hamcrest.Matchers.matchesPattern
2120
import org.junit.jupiter.api.Test
2221
import org.junit.jupiter.api.extension.ExtendWith
@@ -31,7 +30,7 @@ import org.springframework.security.config.annotation.web.invoke
3130
import org.springframework.security.config.test.SpringTestContext
3231
import org.springframework.security.config.test.SpringTestContextExtension
3332
import org.springframework.security.web.SecurityFilterChain
34-
import org.springframework.security.web.util.matcher.AnyRequestMatcher
33+
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher
3534
import org.springframework.security.web.util.matcher.RequestMatcher
3635
import org.springframework.test.web.servlet.MockMvc
3736
import org.springframework.test.web.servlet.get
@@ -281,11 +280,21 @@ class ContentSecurityPolicyDslTests {
281280
}
282281

283282
@Test
284-
fun `headers when content security policy configured with overridden matchers then throws`() {
285-
assertThatThrownBy {
286-
this.spring.register(ContentSecurityPolicyOverriddenMatchersConfig::class.java).autowire()
287-
}.hasRootCauseInstanceOf(IllegalStateException::class.java)
288-
.hasRootCauseMessage("RequestMatcher(s) is already configured")
283+
fun `headers when content security policy configured with overridden matchers then last one wins`() {
284+
this.spring.register(ContentSecurityPolicyOverriddenMatchersConfig::class.java).autowire()
285+
286+
this.mockMvc.get("/foo/bar") {
287+
secure = true
288+
}.andExpect {
289+
header {
290+
string(CONTENT_SECURITY_POLICY, "default-src 'self'")
291+
}
292+
}
293+
this.mockMvc.get("/bar/foo") {
294+
secure = true
295+
}.andExpect {
296+
header { doesNotExist(CONTENT_SECURITY_POLICY) }
297+
}
289298
}
290299

291300
@Configuration
@@ -298,8 +307,8 @@ class ContentSecurityPolicyDslTests {
298307
defaultsDisabled = true
299308
contentSecurityPolicy {
300309
policyDirectives = "default-src 'self'"
301-
requestMatcher = AnyRequestMatcher.INSTANCE
302310
requestMatchers("/**")
311+
requestMatcher = PathPatternRequestMatcher.pathPattern("/foo/**")
303312
}
304313
}
305314
}

‎config/src/test/kotlin/org/springframework/security/config/web/server/ServerContentSecurityPolicyDslTests.kt‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.security.config.web.server
1818

19-
import org.assertj.core.api.Assertions.assertThatThrownBy
2019
import org.junit.jupiter.api.Test
2120
import org.junit.jupiter.api.extension.ExtendWith
2221
import org.springframework.beans.factory.annotation.Autowired
@@ -258,11 +257,18 @@ class ServerContentSecurityPolicyDslTests {
258257
}
259258

260259
@Test
261-
fun `when matchers overridden then fails to configure`() {
262-
assertThatThrownBy {
263-
this.spring.register(CspPathOverriddenMatchersConfig::class.java).autowire()
264-
}.hasRootCauseInstanceOf(IllegalStateException::class.java)
265-
.hasRootCauseMessage("ExchangeMatcher(s) is already configured")
260+
fun `request when matchers overridden then last one wins`() {
261+
val headerName = ContentSecurityPolicyServerHttpHeadersWriter.CONTENT_SECURITY_POLICY
262+
this.spring.register(CspPathOverriddenMatchersConfig::class.java).autowire()
263+
264+
this.client.get()
265+
.uri("https://example.com/foo/bar")
266+
.exchange()
267+
.expectHeader().valueEquals(headerName, "default-src 'self'")
268+
this.client.get()
269+
.uri("https://example.com/bar/foo")
270+
.exchange()
271+
.expectHeader().doesNotExist(headerName)
266272
}
267273

268274
@Configuration
@@ -274,8 +280,8 @@ class ServerContentSecurityPolicyDslTests {
274280
headers {
275281
contentSecurityPolicy {
276282
policyDirectives = "default-src 'self'"
277-
exchangeMatcher = ServerWebExchangeMatchers.anyExchange()
278283
exchangeMatchers("/**")
284+
exchangeMatcher = ServerWebExchangeMatchers.pathMatchers("/foo/**")
279285
}
280286
}
281287
}

0 commit comments

Comments
 (0)