From 996ee243a3513231698958fd46dee1c8ee2943d8 Mon Sep 17 00:00:00 2001 From: onobc Date: Sat, 29 Jan 2022 10:16:48 -0600 Subject: [PATCH] Add ability to match Endpoint requests by HTTP method Update both servlet and reactive `EndpointRequest` classes with support for matching endpoint requests by HTTP method. See gh-29596 --- .../security/reactive/EndpointRequest.java | 44 +++- .../security/servlet/EndpointRequest.java | 50 ++-- .../EndpointRequestIntegrationTests.java | 237 ++++++++++++++++++ .../reactive/EndpointRequestTests.java | 21 ++ ...stractEndpointRequestIntegrationTests.java | 25 ++ .../servlet/EndpointRequestTests.java | 31 ++- .../AntPathRequestMatcherProvider.java | 7 +- .../servlet/RequestMatcherProvider.java | 15 +- 8 files changed, 397 insertions(+), 33 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestIntegrationTests.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequest.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequest.java index 16e91c6e90..a3f4a35e91 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequest.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequest.java @@ -40,6 +40,7 @@ import org.springframework.boot.security.reactive.ApplicationContextServerWebExc import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.http.HttpMethod; import org.springframework.security.web.server.util.matcher.OrServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; @@ -56,6 +57,7 @@ import org.springframework.web.server.ServerWebExchange; * * @author Madhura Bhave * @author Phillip Webb + * @author Chris Bono * @since 2.0.0 */ public final class EndpointRequest { @@ -181,11 +183,17 @@ public final class EndpointRequest { protected abstract ServerWebExchangeMatcher createDelegate(C context); protected final List getDelegateMatchers(Set paths) { - return paths.stream().map(this::getDelegateMatcher).collect(Collectors.toCollection(ArrayList::new)); + return getDelegateMatchers(paths, null); } - private PathPatternParserServerWebExchangeMatcher getDelegateMatcher(String path) { - return new PathPatternParserServerWebExchangeMatcher(path + "/**"); + protected final List getDelegateMatchers(Set paths, HttpMethod httpMethod) { + return paths.stream() + .map((path) -> getDelegateMatcher(path, httpMethod)) + .collect(Collectors.toCollection(ArrayList::new)); + } + + private PathPatternParserServerWebExchangeMatcher getDelegateMatcher(String path, HttpMethod httpMethod) { + return new PathPatternParserServerWebExchangeMatcher(path + "/**", httpMethod); } @Override @@ -258,39 +266,53 @@ public final class EndpointRequest { private final boolean includeLinks; + private final HttpMethod httpMethod; + private EndpointServerWebExchangeMatcher(boolean includeLinks) { - this(Collections.emptyList(), Collections.emptyList(), includeLinks); + this(Collections.emptyList(), Collections.emptyList(), includeLinks, null); } private EndpointServerWebExchangeMatcher(Class[] endpoints, boolean includeLinks) { - this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks); + this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null); } private EndpointServerWebExchangeMatcher(String[] endpoints, boolean includeLinks) { - this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks); + this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null); } - private EndpointServerWebExchangeMatcher(List includes, List excludes, boolean includeLinks) { + private EndpointServerWebExchangeMatcher(List includes, List excludes, boolean includeLinks, + HttpMethod httpMethod) { super(PathMappedEndpoints.class); this.includes = includes; this.excludes = excludes; this.includeLinks = includeLinks; + this.httpMethod = httpMethod; } public EndpointServerWebExchangeMatcher excluding(Class... endpoints) { List excludes = new ArrayList<>(this.excludes); excludes.addAll(Arrays.asList((Object[]) endpoints)); - return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks); + return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks, null); } public EndpointServerWebExchangeMatcher excluding(String... endpoints) { List excludes = new ArrayList<>(this.excludes); excludes.addAll(Arrays.asList((Object[]) endpoints)); - return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks); + return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks, null); } public EndpointServerWebExchangeMatcher excludingLinks() { - return new EndpointServerWebExchangeMatcher(this.includes, this.excludes, false); + return new EndpointServerWebExchangeMatcher(this.includes, this.excludes, false, null); + } + + /** + * Restricts the matcher to only consider requests with a particular http method. + * @param httpMethod the http method to include + * @return a copy of the matcher further restricted to only match requests with + * the specified http method + */ + public EndpointServerWebExchangeMatcher withHttpMethod(HttpMethod httpMethod) { + return new EndpointServerWebExchangeMatcher(this.includes, this.excludes, false, httpMethod); } @Override @@ -301,7 +323,7 @@ public final class EndpointRequest { } streamPaths(this.includes, endpoints).forEach(paths::add); streamPaths(this.excludes, endpoints).forEach(paths::remove); - List delegateMatchers = getDelegateMatchers(paths); + List delegateMatchers = getDelegateMatchers(paths, this.httpMethod); if (this.includeLinks && StringUtils.hasText(endpoints.getBasePath())) { delegateMatchers.add(new LinksServerWebExchangeMatcher()); } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java index 3a85bc8945..8dbed310c8 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java @@ -42,6 +42,7 @@ import org.springframework.boot.web.context.WebServerApplicationContext; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.http.HttpMethod; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -56,6 +57,7 @@ import org.springframework.web.context.WebApplicationContext; * * @author Madhura Bhave * @author Phillip Webb + * @author Chris Bono * @since 2.0.0 */ public final class EndpointRequest { @@ -212,16 +214,21 @@ public final class EndpointRequest { protected final List getDelegateMatchers(RequestMatcherFactory requestMatcherFactory, RequestMatcherProvider matcherProvider, Set paths) { + return getDelegateMatchers(requestMatcherFactory, matcherProvider, paths, null); + } + + protected final List getDelegateMatchers(RequestMatcherFactory requestMatcherFactory, + RequestMatcherProvider matcherProvider, Set paths, HttpMethod httpMethod) { return paths.stream() - .map((path) -> requestMatcherFactory.antPath(matcherProvider, path, "/**")) + .map((path) -> requestMatcherFactory.antPath(matcherProvider, httpMethod, path, "/**")) .collect(Collectors.toCollection(ArrayList::new)); } protected List getLinksMatchers(RequestMatcherFactory requestMatcherFactory, RequestMatcherProvider matcherProvider, String basePath) { List linksMatchers = new ArrayList<>(); - linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, basePath)); - linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, basePath, "/")); + linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, null, basePath)); + linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, null, basePath, "/")); return linksMatchers; } @@ -230,7 +237,7 @@ public final class EndpointRequest { return context.getBean(RequestMatcherProvider.class); } catch (NoSuchBeanDefinitionException ex) { - return AntPathRequestMatcher::new; + return (pattern, method) -> new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null); } } @@ -273,38 +280,52 @@ public final class EndpointRequest { private final boolean includeLinks; + private final HttpMethod httpMethod; + private EndpointRequestMatcher(boolean includeLinks) { - this(Collections.emptyList(), Collections.emptyList(), includeLinks); + this(Collections.emptyList(), Collections.emptyList(), includeLinks, null); } private EndpointRequestMatcher(Class[] endpoints, boolean includeLinks) { - this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks); + this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null); } private EndpointRequestMatcher(String[] endpoints, boolean includeLinks) { - this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks); + this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null); } - private EndpointRequestMatcher(List includes, List excludes, boolean includeLinks) { + private EndpointRequestMatcher(List includes, List excludes, boolean includeLinks, + HttpMethod httpMethod) { this.includes = includes; this.excludes = excludes; this.includeLinks = includeLinks; + this.httpMethod = httpMethod; } public EndpointRequestMatcher excluding(Class... endpoints) { List excludes = new ArrayList<>(this.excludes); excludes.addAll(Arrays.asList((Object[]) endpoints)); - return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks); + return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks, null); } public EndpointRequestMatcher excluding(String... endpoints) { List excludes = new ArrayList<>(this.excludes); excludes.addAll(Arrays.asList((Object[]) endpoints)); - return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks); + return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks, null); } public EndpointRequestMatcher excludingLinks() { - return new EndpointRequestMatcher(this.includes, this.excludes, false); + return new EndpointRequestMatcher(this.includes, this.excludes, false, null); + } + + /** + * Restricts the matcher to only consider requests with a particular http method. + * @param httpMethod the http method to include + * @return a copy of the matcher further restricted to only match requests with + * the specified http method + */ + public EndpointRequestMatcher withHttpMethod(HttpMethod httpMethod) { + return new EndpointRequestMatcher(this.includes, this.excludes, false, httpMethod); } @Override @@ -318,7 +339,8 @@ public final class EndpointRequest { } streamPaths(this.includes, endpoints).forEach(paths::add); streamPaths(this.excludes, endpoints).forEach(paths::remove); - List delegateMatchers = getDelegateMatchers(requestMatcherFactory, matcherProvider, paths); + List delegateMatchers = getDelegateMatchers(requestMatcherFactory, matcherProvider, paths, + this.httpMethod); String basePath = endpoints.getBasePath(); if (this.includeLinks && StringUtils.hasText(basePath)) { delegateMatchers.addAll(getLinksMatchers(requestMatcherFactory, matcherProvider, basePath)); @@ -426,12 +448,12 @@ public final class EndpointRequest { */ private static final class RequestMatcherFactory { - RequestMatcher antPath(RequestMatcherProvider matcherProvider, String... parts) { + RequestMatcher antPath(RequestMatcherProvider matcherProvider, HttpMethod httpMethod, String... parts) { StringBuilder pattern = new StringBuilder(); for (String part : parts) { pattern.append(part); } - return matcherProvider.getRequestMatcher(pattern.toString()); + return matcherProvider.getRequestMatcher(pattern.toString(), httpMethod); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestIntegrationTests.java new file mode 100644 index 0000000000..765bb15997 --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestIntegrationTests.java @@ -0,0 +1,237 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.autoconfigure.security.reactive; + +import java.time.Duration; +import java.util.Base64; + +import org.junit.jupiter.api.Test; + +import org.springframework.boot.actuate.autoconfigure.endpoint.EndpointAutoConfiguration; +import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAutoConfiguration; +import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointProperties; +import org.springframework.boot.actuate.autoconfigure.web.server.ManagementContextAutoConfiguration; +import org.springframework.boot.actuate.endpoint.annotation.Endpoint; +import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; +import org.springframework.boot.actuate.endpoint.annotation.WriteOperation; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration; +import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration; +import org.springframework.boot.autoconfigure.security.reactive.ReactiveSecurityAutoConfiguration; +import org.springframework.boot.autoconfigure.security.reactive.ReactiveUserDetailsServiceAutoConfiguration; +import org.springframework.boot.autoconfigure.web.reactive.HttpHandlerAutoConfiguration; +import org.springframework.boot.autoconfigure.web.reactive.WebFluxAutoConfiguration; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.test.context.assertj.AssertableReactiveWebApplicationContext; +import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner; +import org.springframework.boot.web.embedded.tomcat.TomcatReactiveWebServerFactory; +import org.springframework.boot.web.reactive.context.AnnotationConfigReactiveWebServerApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpMethod; +import org.springframework.security.config.Customizer; +import org.springframework.security.config.web.server.ServerHttpSecurity; +import org.springframework.security.config.web.server.ServerHttpSecurity.CsrfSpec; +import org.springframework.security.core.userdetails.MapReactiveUserDetailsService; +import org.springframework.security.core.userdetails.User; +import org.springframework.security.web.server.SecurityWebFilterChain; +import org.springframework.test.web.reactive.server.WebTestClient; + +/** + * Integration tests for {@link EndpointRequest}. + * + * @author Chris Bono + */ +class EndpointRequestIntegrationTests { + + @Test + void toEndpointShouldMatch() { + getContextRunner().run((context) -> { + WebTestClient webTestClient = getWebTestClient(context); + webTestClient.get().uri("/actuator/e1").exchange().expectStatus().isOk(); + }); + } + + @Test + void toEndpointPostShouldMatch() { + getContextRunner().withPropertyValues("spring.security.user.password=password").run((context) -> { + WebTestClient webTestClient = getWebTestClient(context); + webTestClient.post().uri("/actuator/e1").exchange().expectStatus().isUnauthorized(); + webTestClient.post() + .uri("/actuator/e1") + .header("Authorization", getBasicAuth()) + .exchange() + .expectStatus() + .isNoContent(); + }); + } + + @Test + void toAllEndpointsShouldMatch() { + getContextRunner().withPropertyValues("spring.security.user.password=password").run((context) -> { + WebTestClient webTestClient = getWebTestClient(context); + webTestClient.get().uri("/actuator/e2").exchange().expectStatus().isUnauthorized(); + webTestClient.get() + .uri("/actuator/e2") + .header("Authorization", getBasicAuth()) + .exchange() + .expectStatus() + .isOk(); + }); + } + + @Test + void toLinksShouldMatch() { + getContextRunner().run((context) -> { + WebTestClient webTestClient = getWebTestClient(context); + webTestClient.get().uri("/actuator").exchange().expectStatus().isOk(); + }); + } + + protected final ReactiveWebApplicationContextRunner getContextRunner() { + return createContextRunner().withPropertyValues("management.endpoints.web.exposure.include=*") + .withUserConfiguration(BaseConfiguration.class, SecurityConfiguration.class) + .withConfiguration( + AutoConfigurations.of(JacksonAutoConfiguration.class, ReactiveSecurityAutoConfiguration.class, + ReactiveUserDetailsServiceAutoConfiguration.class, EndpointAutoConfiguration.class, + WebEndpointAutoConfiguration.class, ManagementContextAutoConfiguration.class)); + + } + + protected ReactiveWebApplicationContextRunner createContextRunner() { + return new ReactiveWebApplicationContextRunner(AnnotationConfigReactiveWebServerApplicationContext::new) + .withUserConfiguration(WebEndpointConfiguration.class) + .withConfiguration(AutoConfigurations.of(HttpHandlerAutoConfiguration.class, + HttpMessageConvertersAutoConfiguration.class, WebFluxAutoConfiguration.class)); + } + + protected WebTestClient getWebTestClient(AssertableReactiveWebApplicationContext context) { + int port = context.getSourceApplicationContext(AnnotationConfigReactiveWebServerApplicationContext.class) + .getWebServer() + .getPort(); + return WebTestClient.bindToServer() + .baseUrl("http://localhost:" + port) + .responseTimeout(Duration.ofMinutes(5)) + .build(); + } + + private String getBasicAuth() { + return "Basic " + Base64.getEncoder().encodeToString("user:password".getBytes()); + } + + @Configuration(proxyBeanMethods = false) + static class BaseConfiguration { + + @Bean + TestEndpoint1 endpoint1() { + return new TestEndpoint1(); + } + + @Bean + TestEndpoint2 endpoint2() { + return new TestEndpoint2(); + } + + @Bean + TestEndpoint3 endpoint3() { + return new TestEndpoint3(); + } + + } + + @Endpoint(id = "e1") + static class TestEndpoint1 { + + @ReadOperation + Object getAll() { + return "endpoint 1"; + } + + @WriteOperation + void setAll() { + } + + } + + @Endpoint(id = "e2") + static class TestEndpoint2 { + + @ReadOperation + Object getAll() { + return "endpoint 2"; + } + + } + + @Endpoint(id = "e3") + static class TestEndpoint3 { + + @ReadOperation + Object getAll() { + return null; + } + + } + + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(WebEndpointProperties.class) + static class WebEndpointConfiguration { + + @Bean + TomcatReactiveWebServerFactory tomcat() { + return new TomcatReactiveWebServerFactory(0); + } + + } + + @Configuration(proxyBeanMethods = false) + static class SecurityConfiguration { + + @SuppressWarnings("deprecation") + @Bean + MapReactiveUserDetailsService userDetailsService() { + return new MapReactiveUserDetailsService( + User.withDefaultPasswordEncoder() + .username("user") + .password("password") + .authorities("ROLE_USER") + .build(), + User.withDefaultPasswordEncoder() + .username("admin") + .password("admin") + .authorities("ROLE_ACTUATOR", "ROLE_USER") + .build()); + } + + @Bean + SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http) { + http.authorizeExchange((exchanges) -> { + exchanges.matchers(EndpointRequest.toLinks()).permitAll(); + exchanges.matchers(EndpointRequest.to(TestEndpoint1.class).withHttpMethod(HttpMethod.POST)) + .authenticated(); + exchanges.matchers(EndpointRequest.to(TestEndpoint1.class)).permitAll(); + exchanges.matchers(EndpointRequest.toAnyEndpoint()).authenticated(); + exchanges.anyExchange().hasRole("ADMIN"); + }); + http.httpBasic(Customizer.withDefaults()); + http.csrf(CsrfSpec::disable); + return http.build(); + } + + } + +} diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestTests.java index 792ed54ca9..e4ee3bdc8f 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequestTests.java @@ -33,6 +33,7 @@ import org.springframework.boot.actuate.endpoint.web.PathMappedEndpoint; import org.springframework.boot.actuate.endpoint.web.PathMappedEndpoints; import org.springframework.boot.actuate.endpoint.web.WebServerNamespace; import org.springframework.context.support.StaticApplicationContext; +import org.springframework.http.HttpMethod; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; @@ -51,6 +52,7 @@ import static org.mockito.Mockito.mock; * * @author Madhura Bhave * @author Phillip Webb + * @author Chris Bono */ class EndpointRequestTests { @@ -62,6 +64,13 @@ class EndpointRequestTests { assertMatcher(matcher).matches("/actuator"); } + @Test + void toAnyEndpointWithHttpMethodShouldRespectRequestMethod() { + ServerWebExchangeMatcher matcher = EndpointRequest.toAnyEndpoint().withHttpMethod(HttpMethod.POST); + assertMatcher(matcher, "/actuator").matches(HttpMethod.POST, "/actuator/foo"); + assertMatcher(matcher, "/actuator").doesNotMatch(HttpMethod.GET, "/actuator/foo"); + } + @Test void toAnyEndpointShouldMatchEndpointPathWithTrailingSlash() { ServerWebExchangeMatcher matcher = EndpointRequest.toAnyEndpoint(); @@ -368,6 +377,12 @@ class EndpointRequestTests { matches(exchange); } + void matches(HttpMethod httpMethod, String path) { + ServerWebExchange exchange = webHandler() + .createExchange(MockServerHttpRequest.method(httpMethod, path).build(), new MockServerHttpResponse()); + matches(exchange); + } + private void matches(ServerWebExchange exchange) { assertThat(this.matcher.matches(exchange).block(Duration.ofSeconds(30)).isMatch()) .as("Matches " + getRequestPath(exchange)) @@ -380,6 +395,12 @@ class EndpointRequestTests { doesNotMatch(exchange); } + void doesNotMatch(HttpMethod httpMethod, String path) { + ServerWebExchange exchange = webHandler() + .createExchange(MockServerHttpRequest.method(httpMethod, path).build(), new MockServerHttpResponse()); + doesNotMatch(exchange); + } + private void doesNotMatch(ServerWebExchange exchange) { assertThat(this.matcher.matches(exchange).block(Duration.ofSeconds(30)).isMatch()) .as("Does not match " + getRequestPath(exchange)) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/AbstractEndpointRequestIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/AbstractEndpointRequestIntegrationTests.java index 6a2ca3dbb0..09b5c97400 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/AbstractEndpointRequestIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/AbstractEndpointRequestIntegrationTests.java @@ -32,6 +32,7 @@ import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAu import org.springframework.boot.actuate.autoconfigure.web.server.ManagementContextAutoConfiguration; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; +import org.springframework.boot.actuate.endpoint.annotation.WriteOperation; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration; import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; @@ -40,8 +41,10 @@ import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer; import org.springframework.security.core.userdetails.User; import org.springframework.security.provisioning.InMemoryUserDetailsManager; import org.springframework.security.web.SecurityFilterChain; @@ -53,6 +56,7 @@ import static org.springframework.security.config.Customizer.withDefaults; * Abstract base class for {@link EndpointRequest} tests. * * @author Madhura Bhave + * @author Chris Bono */ abstract class AbstractEndpointRequestIntegrationTests { @@ -64,6 +68,20 @@ abstract class AbstractEndpointRequestIntegrationTests { }); } + @Test + void toEndpointPostShouldMatch() { + getContextRunner().withPropertyValues("spring.security.user.password=password").run((context) -> { + WebTestClient webTestClient = getWebTestClient(context); + webTestClient.post().uri("/actuator/e1").exchange().expectStatus().isUnauthorized(); + webTestClient.post() + .uri("/actuator/e1") + .header("Authorization", getBasicAuth()) + .exchange() + .expectStatus() + .isNoContent(); + }); + } + @Test void toAllEndpointsShouldMatch() { getContextRunner().withPropertyValues("spring.security.user.password=password").run((context) -> { @@ -153,6 +171,10 @@ abstract class AbstractEndpointRequestIntegrationTests { return "endpoint 1"; } + @WriteOperation + void setAll() { + } + } @Endpoint(id = "e2") @@ -200,10 +222,13 @@ abstract class AbstractEndpointRequestIntegrationTests { SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { http.authorizeHttpRequests((requests) -> { requests.requestMatchers(EndpointRequest.toLinks()).permitAll(); + requests.requestMatchers(EndpointRequest.to(TestEndpoint1.class).withHttpMethod(HttpMethod.POST)) + .authenticated(); requests.requestMatchers(EndpointRequest.to(TestEndpoint1.class)).permitAll(); requests.requestMatchers(EndpointRequest.toAnyEndpoint()).authenticated(); requests.anyRequest().hasRole("ADMIN"); }); + http.csrf(CsrfConfigurer::disable); http.httpBasic(withDefaults()); return http.build(); } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequestTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequestTests.java index ac07f1d6ef..01dd43ec14 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequestTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequestTests.java @@ -37,6 +37,7 @@ import org.springframework.boot.actuate.endpoint.web.WebServerNamespace; import org.springframework.boot.autoconfigure.security.servlet.RequestMatcherProvider; import org.springframework.boot.web.context.WebServerApplicationContext; import org.springframework.boot.web.server.WebServer; +import org.springframework.http.HttpMethod; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockServletContext; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -52,6 +53,7 @@ import static org.mockito.Mockito.mock; * * @author Phillip Webb * @author Madhura Bhave + * @author Chris Bono */ class EndpointRequestTests { @@ -65,6 +67,14 @@ class EndpointRequestTests { assertMatcher(matcher, "/actuator").matches("/actuator"); } + @Test + void toAnyEndpointWithHttpMethodShouldRespectRequestMethod() { + EndpointRequest.EndpointRequestMatcher matcher = EndpointRequest.toAnyEndpoint() + .withHttpMethod(HttpMethod.POST); + assertMatcher(matcher, "/actuator").matches(HttpMethod.POST, "/actuator/foo"); + assertMatcher(matcher, "/actuator").doesNotMatch(HttpMethod.GET, "/actuator/foo"); + } + @Test void toAnyEndpointShouldMatchEndpointPathWithTrailingSlash() { RequestMatcher matcher = EndpointRequest.toAnyEndpoint(); @@ -199,7 +209,7 @@ class EndpointRequestTests { RequestMatcher matcher = EndpointRequest.toAnyEndpoint(); RequestMatcher mockRequestMatcher = (request) -> false; RequestMatcherAssert assertMatcher = assertMatcher(matcher, mockPathMappedEndpoints(""), - (pattern) -> mockRequestMatcher, null); + (pattern, method) -> mockRequestMatcher, null); assertMatcher.doesNotMatch("/foo"); assertMatcher.doesNotMatch("/bar"); } @@ -209,7 +219,7 @@ class EndpointRequestTests { RequestMatcher matcher = EndpointRequest.toLinks(); RequestMatcher mockRequestMatcher = (request) -> false; RequestMatcherAssert assertMatcher = assertMatcher(matcher, mockPathMappedEndpoints("/actuator"), - (pattern) -> mockRequestMatcher, null); + (pattern, method) -> mockRequestMatcher, null); assertMatcher.doesNotMatch("/actuator"); } @@ -390,7 +400,11 @@ class EndpointRequestTests { } void matches(String servletPath) { - matches(mockRequest(servletPath)); + matches(mockRequest(null, servletPath)); + } + + void matches(HttpMethod httpMethod, String servletPath) { + matches(mockRequest(httpMethod, servletPath)); } private void matches(HttpServletRequest request) { @@ -398,20 +412,27 @@ class EndpointRequestTests { } void doesNotMatch(String servletPath) { - doesNotMatch(mockRequest(servletPath)); + doesNotMatch(mockRequest(null, servletPath)); + } + + void doesNotMatch(HttpMethod httpMethod, String servletPath) { + doesNotMatch(mockRequest(httpMethod, servletPath)); } private void doesNotMatch(HttpServletRequest request) { assertThat(this.matcher.matches(request)).as("Does not match " + getRequestPath(request)).isFalse(); } - private MockHttpServletRequest mockRequest(String servletPath) { + private MockHttpServletRequest mockRequest(HttpMethod httpMethod, String servletPath) { MockServletContext servletContext = new MockServletContext(); servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, this.context); MockHttpServletRequest request = new MockHttpServletRequest(servletContext); if (servletPath != null) { request.setServletPath(servletPath); } + if (httpMethod != null) { + request.setMethod(httpMethod.name()); + } return request; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/AntPathRequestMatcherProvider.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/AntPathRequestMatcherProvider.java index 2f6e9c0d7e..2d7d29cde4 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/AntPathRequestMatcherProvider.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/AntPathRequestMatcherProvider.java @@ -18,6 +18,7 @@ package org.springframework.boot.autoconfigure.security.servlet; import java.util.function.Function; +import org.springframework.http.HttpMethod; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -25,6 +26,7 @@ import org.springframework.security.web.util.matcher.RequestMatcher; * {@link RequestMatcherProvider} that provides an {@link AntPathRequestMatcher}. * * @author Madhura Bhave + * @author Chris Bono * @since 2.1.8 */ public class AntPathRequestMatcherProvider implements RequestMatcherProvider { @@ -36,8 +38,9 @@ public class AntPathRequestMatcherProvider implements RequestMatcherProvider { } @Override - public RequestMatcher getRequestMatcher(String pattern) { - return new AntPathRequestMatcher(this.pathFactory.apply(pattern)); + public RequestMatcher getRequestMatcher(String pattern, HttpMethod httpMethod) { + return new AntPathRequestMatcher(this.pathFactory.apply(pattern), + (httpMethod != null) ? httpMethod.name() : null); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/RequestMatcherProvider.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/RequestMatcherProvider.java index 39c20aeb15..f9c4f89be0 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/RequestMatcherProvider.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/RequestMatcherProvider.java @@ -16,6 +16,7 @@ package org.springframework.boot.autoconfigure.security.servlet; +import org.springframework.http.HttpMethod; import org.springframework.security.web.util.matcher.RequestMatcher; /** @@ -23,6 +24,7 @@ import org.springframework.security.web.util.matcher.RequestMatcher; * Spring Security. * * @author Madhura Bhave + * @author Chris Bono * @since 2.0.5 */ @FunctionalInterface @@ -33,6 +35,17 @@ public interface RequestMatcherProvider { * @param pattern the request pattern * @return a request matcher */ - RequestMatcher getRequestMatcher(String pattern); + default RequestMatcher getRequestMatcher(String pattern) { + return getRequestMatcher(pattern, null); + } + + /** + * Return the {@link RequestMatcher} to be used for the specified pattern and http + * method. + * @param pattern the request pattern + * @param httpMethod the http method + * @return a request matcher + */ + RequestMatcher getRequestMatcher(String pattern, HttpMethod httpMethod); }