From 9b2e13aace28dfaa640d38cea908d187c574f7c3 Mon Sep 17 00:00:00 2001 From: Hatef Palizgar Date: Thu, 1 Apr 2021 20:32:01 +0300 Subject: [PATCH] Allow remote devtools access with Spring Security Update `ManagementWebSecurityAutoConfiguration` so that the `managementSecurityFilterChain` bean has an explicit order. Prior to this commit, the `managementSecurityFilterChain` would override the `securityFilterChain` in `RemoteDevtoolsSecurityConfiguration` which would prevent the remote devtools endpoint from being accessed. See gh-25868 --- ...anagementWebSecurityAutoConfiguration.java | 3 + ...mentWebSecurityAutoConfigurationTests.java | 93 ++++++++++++++++++- 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfiguration.java index 4641195173a..3d48aa253ec 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfiguration.java @@ -26,12 +26,14 @@ import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.security.ConditionalOnDefaultWebSecurity; +import org.springframework.boot.autoconfigure.security.SecurityProperties; import org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration; import org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration; import org.springframework.boot.autoconfigure.security.saml2.Saml2RelyingPartyAutoConfiguration; import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.annotation.Order; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.web.SecurityFilterChain; @@ -58,6 +60,7 @@ import org.springframework.security.web.SecurityFilterChain; public class ManagementWebSecurityAutoConfiguration { @Bean + @Order(SecurityProperties.BASIC_AUTH_ORDER) SecurityFilterChain managementSecurityFilterChain(HttpSecurity http) throws Exception { http.authorizeRequests((requests) -> { requests.requestMatchers(EndpointRequest.to(HealthEndpoint.class, InfoEndpoint.class)).permitAll(); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfigurationTests.java index 23e24f76932..6e449f0820a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/security/servlet/ManagementWebSecurityAutoConfigurationTests.java @@ -17,9 +17,19 @@ package org.springframework.boot.actuate.autoconfigure.security.servlet; import java.io.IOException; +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; +import org.springframework.beans.factory.config.BeanDefinitionHolder; import org.springframework.boot.actuate.autoconfigure.endpoint.EndpointAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.env.EnvironmentEndpointAutoConfiguration; @@ -27,14 +37,21 @@ import org.springframework.boot.actuate.autoconfigure.health.HealthContributorAu import org.springframework.boot.actuate.autoconfigure.health.HealthEndpointAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.info.InfoEndpointAutoConfiguration; import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.security.SecurityProperties; import org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration; import org.springframework.boot.autoconfigure.security.saml2.Saml2RelyingPartyAutoConfiguration; import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.assertj.AssertableWebApplicationContext; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; +import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; +import org.springframework.core.ResolvableType; +import org.springframework.core.annotation.AnnotationAttributes; +import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; @@ -45,6 +62,7 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.web.context.WebApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -121,7 +139,7 @@ class ManagementWebSecurityAutoConfigurationTests { @Test void backsOffIfSecurityFilterChainBeanIsPresent() { this.contextRunner.withUserConfiguration(TestSecurityFilterChainConfig.class).run((context) -> { - assertThat(context.getBeansOfType(SecurityFilterChain.class).size()).isEqualTo(1); + assertThat(context.getBeansOfType(SecurityFilterChain.class)).isNotEmpty(); assertThat(context.containsBean("testSecurityFilterChain")).isTrue(); }); } @@ -146,6 +164,28 @@ class ManagementWebSecurityAutoConfigurationTests { .doesNotHaveBean(MANAGEMENT_SECURITY_FILTER_CHAIN_BEAN)); } + @Test + void backOffIfRemoteDevToolsSecurityFilterChainIsPresent() { + this.contextRunner.withUserConfiguration(TestSecurityFilterChainConfig.class).run((context) -> { + List beanNames = getOrderedBeanNames(context); + + assertThat(beanNames).containsExactly("testRemoteDevToolsSecurityFilterChain", "testSecurityFilterChain"); + assertThat(context.getBeansOfType(SecurityFilterChain.class).size()).isEqualTo(2); + assertThat(context).doesNotHaveBean(ManagementWebSecurityAutoConfiguration.class); + assertThat(context.containsBean("testRemoteDevToolsSecurityFilterChain")).isTrue(); + }); + } + + @NotNull + private List getOrderedBeanNames(AssertableWebApplicationContext context) { + return Arrays.stream(context.getBeanNamesForType(SecurityFilterChain.class)) + .map((beanName) -> Optional.of(context).map(ConfigurableApplicationContext::getBeanFactory) + .map((beanFactory) -> beanFactory.getBeanDefinition(beanName)) + .map((beanDefinition) -> new BeanDefinitionHolder(beanDefinition, beanName)).orElse(null)) + .sorted(OrderAnnotatedBeanDefinitionComparator.INSTANCE).map(BeanDefinitionHolder::getBeanName) + .collect(Collectors.toList()); + } + private HttpStatus getResponseStatus(AssertableWebApplicationContext context, String path) throws IOException, javax.servlet.ServletException { FilterChainProxy filterChainProxy = context.getBean(FilterChainProxy.class); @@ -183,6 +223,57 @@ class ManagementWebSecurityAutoConfigurationTests { .build(); } + @Bean + @Order(SecurityProperties.BASIC_AUTH_ORDER - 1) + SecurityFilterChain testRemoteDevToolsSecurityFilterChain(HttpSecurity http) throws Exception { + return http.requestMatcher(new AntPathRequestMatcher("/**")).authorizeRequests().anyRequest().anonymous() + .and().csrf().disable().build(); + } + + } + + static class OrderAnnotatedBeanDefinitionComparator implements Comparator { + + static final OrderAnnotatedBeanDefinitionComparator INSTANCE = new OrderAnnotatedBeanDefinitionComparator(); + + private final Map beanNameToOrder = new ConcurrentHashMap<>(); + + @Override + public int compare(BeanDefinitionHolder beanOne, BeanDefinitionHolder beanTwo) { + return getOrder(beanOne).compareTo(getOrder(beanTwo)); + } + + private Integer getOrder(BeanDefinitionHolder bean) { + return this.beanNameToOrder.computeIfAbsent(bean.getBeanName(), + (beanName) -> Optional.of(bean).map(BeanDefinitionHolder::getBeanDefinition) + .filter(AnnotatedBeanDefinition.class::isInstance).map(AnnotatedBeanDefinition.class::cast) + .map(this::getOrderAnnotationAttributesFromFactoryMethod).map(this::getOrder) + .orElse(Ordered.LOWEST_PRECEDENCE)); + } + + private Integer getOrder(AnnotationAttributes annotationAttributes) { + return Optional.ofNullable(annotationAttributes) + .map((it) -> it.getOrDefault("value", Ordered.LOWEST_PRECEDENCE)).map(Integer.class::cast) + .orElse(Ordered.LOWEST_PRECEDENCE); + } + + private AnnotationAttributes getOrderAnnotationAttributesFromFactoryMethod( + AnnotatedBeanDefinition beanDefinition) { + return Optional.of(beanDefinition).map(AnnotatedBeanDefinition::getFactoryMethodMetadata) + .filter((methodMetadata) -> methodMetadata.isAnnotated(Order.class.getName())) + .map((methodMetadata) -> methodMetadata.getAnnotationAttributes(Order.class.getName())) + .map(AnnotationAttributes::fromMap) + .orElseGet(() -> getOrderAnnotationAttributesFromBeanClass(beanDefinition)); + } + + private AnnotationAttributes getOrderAnnotationAttributesFromBeanClass(AnnotatedBeanDefinition beanDefinition) { + return Optional.of(beanDefinition).map(AnnotatedBeanDefinition::getResolvableType) + .map(ResolvableType::resolve).filter((beanType) -> beanType.isAnnotationPresent(Order.class)) + .map((beanType) -> beanType.getAnnotation(Order.class)) + .map(AnnotationUtils::getAnnotationAttributes).map(AnnotationAttributes::fromMap).orElse(null); + + } + } }