From 9b2e13aace28dfaa640d38cea908d187c574f7c3 Mon Sep 17 00:00:00 2001 From: Hatef Palizgar Date: Thu, 1 Apr 2021 20:32:01 +0300 Subject: [PATCH 1/2] 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 4641195173..3d48aa253e 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 23e24f7693..6e449f0820 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); + + } + } } From 0699fdcc8acd39704f1bfd22e331b6ab8158bc0b Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 17 May 2021 16:50:58 -0700 Subject: [PATCH 2/2] Polish 'Allow remote devtools access with Spring Security' See gh-25868 --- ...anagementWebSecurityAutoConfiguration.java | 3 +- ...mentWebSecurityAutoConfigurationTests.java | 90 ++++--------------- 2 files changed, 18 insertions(+), 75 deletions(-) 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 3d48aa253e..e32db83141 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * 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. @@ -48,6 +48,7 @@ import org.springframework.security.web.SecurityFilterChain; * of the custom security configuration. * * @author Madhura Bhave + * @author Hatef Palizgar * @since 2.1.0 */ @Configuration(proxyBeanMethods = false) 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 6e449f0820..3b6ffee813 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,19 +17,11 @@ 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; @@ -44,13 +36,8 @@ import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfi 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; @@ -71,6 +58,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link ManagementWebSecurityAutoConfiguration}. * * @author Madhura Bhave + * @author Hatef Palizgar */ class ManagementWebSecurityAutoConfigurationTests { @@ -139,7 +127,7 @@ class ManagementWebSecurityAutoConfigurationTests { @Test void backsOffIfSecurityFilterChainBeanIsPresent() { this.contextRunner.withUserConfiguration(TestSecurityFilterChainConfig.class).run((context) -> { - assertThat(context.getBeansOfType(SecurityFilterChain.class)).isNotEmpty(); + assertThat(context.getBeansOfType(SecurityFilterChain.class)).hasSize(1); assertThat(context.containsBean("testSecurityFilterChain")).isTrue(); }); } @@ -166,26 +154,19 @@ class ManagementWebSecurityAutoConfigurationTests { @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); + this.contextRunner.withUserConfiguration(TestRemoteDevToolsSecurityFilterChainConfig.class).run((context) -> { + SecurityFilterChain testSecurityFilterChain = context.getBean("testSecurityFilterChain", + SecurityFilterChain.class); + SecurityFilterChain testRemoteDevToolsSecurityFilterChain = context + .getBean("testRemoteDevToolsSecurityFilterChain", SecurityFilterChain.class); + List orderedSecurityFilterChains = context.getBeanProvider(SecurityFilterChain.class) + .orderedStream().collect(Collectors.toList()); + assertThat(orderedSecurityFilterChains).containsExactly(testRemoteDevToolsSecurityFilterChain, + testSecurityFilterChain); 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); @@ -223,6 +204,11 @@ class ManagementWebSecurityAutoConfigurationTests { .build(); } + } + + @Configuration(proxyBeanMethods = false) + static class TestRemoteDevToolsSecurityFilterChainConfig extends TestSecurityFilterChainConfig { + @Bean @Order(SecurityProperties.BASIC_AUTH_ORDER - 1) SecurityFilterChain testRemoteDevToolsSecurityFilterChain(HttpSecurity http) throws Exception { @@ -232,48 +218,4 @@ class ManagementWebSecurityAutoConfigurationTests { } - 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); - - } - - } - }