From 8dbd90ea14c2eaa0f52d3ab3ae47011ae78c965c Mon Sep 17 00:00:00 2001 From: Hatef Palizgar Date: Thu, 1 Apr 2021 20:32:01 +0300 Subject: [PATCH 1/2] fixes bug: remote devtools endpoint cannot be accessed if actuator and spring security are on the classpath --- .../servlet/ManagementWebSecurityAutoConfiguration.java | 3 +++ 1 file changed, 3 insertions(+) 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 f43c7dfcdca8..c96f8d5dedc7 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 @@ -25,12 +25,14 @@ 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; @@ -55,6 +57,7 @@ public class ManagementWebSecurityAutoConfiguration { @Bean + @Order(SecurityProperties.BASIC_AUTH_ORDER) SecurityFilterChain managementSecurityFilterChain(HttpSecurity http) throws Exception { http.authorizeRequests((requests) -> { requests.requestMatchers(EndpointRequest.to(HealthEndpoint.class)).permitAll(); From 3fe715d13882c42e189f654353f88aaa4827e292 Mon Sep 17 00:00:00 2001 From: Hatef Palizgar Date: Fri, 2 Apr 2021 18:42:18 +0300 Subject: [PATCH 2/2] add tests to ensure correct order is picked up by context --- ...mentWebSecurityAutoConfigurationTests.java | 93 ++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) 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 6dc70733ea97..df6f709b943e 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.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.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; @@ -113,7 +131,7 @@ void backOffIfCustomSecurityIsAdded() { @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(); }); } @@ -138,6 +156,28 @@ void backOffIfSaml2RelyingPartyAutoConfigurationPresent() { .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); @@ -175,6 +215,57 @@ SecurityFilterChain testSecurityFilterChain(HttpSecurity http) throws Exception .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); + + } + } }