diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointAutoConfiguration.java index 88a7113ab2..d0960dd5f5 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointAutoConfiguration.java @@ -78,9 +78,6 @@ public class EndpointAutoConfiguration { @Autowired private InfoPropertiesConfiguration properties; - @Autowired(required = false) - private ManagementServerProperties management; - @Autowired(required = false) private HealthAggregator healthAggregator = new OrderedHealthAggregator(); @@ -105,20 +102,7 @@ public class EndpointAutoConfiguration { @Bean @ConditionalOnMissingBean public HealthEndpoint healthEndpoint() { - HealthEndpoint endpoint = new HealthEndpoint(this.healthAggregator, - this.healthIndicators); - endpoint.setSensitive(isHealthEndpointSensitive()); - return endpoint; - } - - /** - * The default health endpoint sensitivity depends on whether all the endpoints by - * default are secure or not. User can always override with - * {@literal endpoints.health.sensitive}. - */ - private boolean isHealthEndpointSensitive() { - return (this.management != null) && (this.management.getSecurity() != null) - && this.management.getSecurity().isEnabled(); + return new HealthEndpoint(this.healthAggregator, this.healthIndicators); } @Bean diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java index 6491fae7a9..89b16589b7 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java @@ -33,6 +33,7 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.actuate.autoconfigure.ManagementServerProperties.Security; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.EnvironmentEndpoint; import org.springframework.boot.actuate.endpoint.HealthEndpoint; @@ -163,7 +164,9 @@ public class EndpointWebMvcAutoConfiguration implements ApplicationContextAware, @ConditionalOnBean(HealthEndpoint.class) @ConditionalOnProperty(prefix = "endpoints.health", name = "enabled", matchIfMissing = true) public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) { - HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate); + Security security = this.managementServerProperties.getSecurity(); + boolean secure = (security == null || security.isEnabled()); + HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate, secure); if (this.healthMvcEndpointProperties.getMapping() != null) { healthMvcEndpoint.addStatusMapping(this.healthMvcEndpointProperties .getMapping()); diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfiguration.java index b7d17a3527..b6319c82c4 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfiguration.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.Set; import javax.annotation.PostConstruct; -import javax.servlet.http.HttpServletRequest; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.Endpoint; @@ -62,7 +61,6 @@ import org.springframework.security.config.annotation.web.configuration.WebSecur import org.springframework.security.config.annotation.web.configurers.ExpressionUrlAuthorizationConfigurer; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint; -import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.StringUtils; /** @@ -256,23 +254,9 @@ public class ManagementSecurityAutoConfiguration { String[] endpointPaths, ExpressionUrlAuthorizationConfigurer.ExpressionInterceptUrlRegistry requests) { requests.antMatchers(endpointPaths).permitAll(); - if (this.endpointHandlerMapping != null) { - requests.requestMatchers(new PrincipalHandlerRequestMatcher()) - .permitAll(); - } requests.anyRequest().hasRole(this.management.getSecurity().getRole()); } - private final class PrincipalHandlerRequestMatcher implements RequestMatcher { - - @Override - public boolean matches(HttpServletRequest request) { - return ManagementWebSecurityConfigurerAdapter.this.endpointHandlerMapping - .isPrincipalHandler(request); - } - - } - } private static String[] getEndpointPaths(EndpointHandlerMapping endpointHandlerMapping) { diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EndpointHandlerMapping.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EndpointHandlerMapping.java index c3bdfc0fc8..5257d5e2db 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EndpointHandlerMapping.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EndpointHandlerMapping.java @@ -17,22 +17,17 @@ package org.springframework.boot.actuate.endpoint.mvc; import java.lang.reflect.Method; -import java.security.Principal; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; -import javax.servlet.http.HttpServletRequest; - import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import org.springframework.web.method.HandlerMethod; -import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition; import org.springframework.web.servlet.mvc.method.RequestMappingInfo; @@ -46,8 +41,8 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandl *

* One of the aims of the mapping is to support endpoints that work as HTTP endpoints but * can still provide useful service interfaces when there is no HTTP server (and no Spring - * MVC on the classpath). Note that any endpoints having method signaturess will break in - * a non-servlet environment. + * MVC on the classpath). Note that any endpoints having method signatures will break in a + * non-servlet environment. * * @author Phillip Webb * @author Christian Dupuis @@ -62,8 +57,6 @@ public class EndpointHandlerMapping extends RequestMappingHandlerMapping impleme private boolean disabled = false; - private Set principalHandlers = new HashSet(); - /** * Create a new {@link EndpointHandlerMapping} instance. All {@link Endpoint}s will be * detected from the {@link ApplicationContext}. @@ -102,9 +95,6 @@ public class EndpointHandlerMapping extends RequestMappingHandlerMapping impleme return; } String[] patterns = getPatterns(handler, mapping); - if (handlesPrincipal(method)) { - this.principalHandlers.add(new HandlerMethod(handler, method)); - } super.registerHandlerMethod(handler, method, withNewPatterns(mapping, patterns)); } @@ -132,15 +122,6 @@ public class EndpointHandlerMapping extends RequestMappingHandlerMapping impleme return ""; } - private boolean handlesPrincipal(Method method) { - for (Class type : method.getParameterTypes()) { - if (Principal.class.equals(type)) { - return true; - } - } - return false; - } - private RequestMappingInfo withNewPatterns(RequestMappingInfo mapping, String[] patternStrings) { PatternsRequestCondition patterns = new PatternsRequestCondition(patternStrings); @@ -150,23 +131,6 @@ public class EndpointHandlerMapping extends RequestMappingHandlerMapping impleme mapping.getCustomCondition()); } - /** - * Returns {@code true} if the given request is mapped to an endpoint and to a method - * that includes a {@link Principal} argument. - * @param request the http request - * @return {@code true} if the request is - */ - public boolean isPrincipalHandler(HttpServletRequest request) { - try { - HandlerExecutionChain handlerChain = getHandler(request); - Object handler = (handlerChain == null ? null : handlerChain.getHandler()); - return (handler != null && this.principalHandlers.contains(handler)); - } - catch (Exception ex) { - return false; - } - } - /** * @param prefix the prefix to set */ diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java index 2158673cb1..4ce1e18430 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java @@ -25,6 +25,9 @@ import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.HealthEndpoint; import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.Status; +import org.springframework.boot.bind.RelaxedPropertyResolver; +import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.Environment; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.util.Assert; @@ -37,20 +40,31 @@ import org.springframework.web.bind.annotation.ResponseBody; * @author Christian Dupuis * @author Dave Syer * @author Andy Wilkinson + * @author Phillip Webb * @since 1.1.0 */ -public class HealthMvcEndpoint implements MvcEndpoint { +public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { + + private final HealthEndpoint delegate; + + private final boolean secure; private Map statusMapping = new HashMap(); - private HealthEndpoint delegate; + private RelaxedPropertyResolver propertyResolver; private long lastAccess = 0; private Health cached; public HealthMvcEndpoint(HealthEndpoint delegate) { + this(delegate, true); + } + + public HealthMvcEndpoint(HealthEndpoint delegate, boolean secure) { + Assert.notNull(delegate, "Delegate must not be null"); this.delegate = delegate; + this.secure = secure; setupDefaultStatusMapping(); } @@ -59,6 +73,12 @@ public class HealthMvcEndpoint implements MvcEndpoint { addStatusMapping(Status.OUT_OF_SERVICE, HttpStatus.SERVICE_UNAVAILABLE); } + @Override + public void setEnvironment(Environment environment) { + this.propertyResolver = new RelaxedPropertyResolver(environment, + "endpoints.health."); + } + /** * Set specific status mappings. * @param statusMapping a map of status code to {@link HttpStatus} @@ -108,46 +128,42 @@ public class HealthMvcEndpoint implements MvcEndpoint { "message", "This endpoint is disabled"), HttpStatus.NOT_FOUND); } Health health = getHealth(principal); - Status status = health.getStatus(); - if (this.statusMapping.containsKey(status.getCode())) { - return new ResponseEntity(health, this.statusMapping.get(status - .getCode())); + HttpStatus status = this.statusMapping.get(health.getStatus().getCode()); + if (status != null) { + return new ResponseEntity(health, status); } return health; } private Health getHealth(Principal principal) { - Health health = (useCachedValue(principal) ? this.cached : (Health) this.delegate - .invoke()); - // Not too worried about concurrent access here, the worst that can happen is the - // odd extra call to delegate.invoke() - this.cached = health; - if (!secure(principal) && this.delegate.isSensitive()) { - // If not secure we only expose the status - health = Health.status(health.getStatus()).build(); + long accessTime = System.currentTimeMillis(); + if (isCacheStale(accessTime) || isSecure(principal) || isUnrestricted()) { + this.lastAccess = accessTime; + this.cached = this.delegate.invoke(); } - return health; + if (isSecure(principal) || isUnrestricted()) { + return this.cached; + } + return Health.status(this.cached.getStatus()).build(); } - private boolean secure(Principal principal) { + private boolean isCacheStale(long accessTime) { + if (this.cached == null) { + return true; + } + return (accessTime - this.lastAccess) > this.delegate.getTimeToLive(); + } + + private boolean isUnrestricted() { + Boolean sensitive = this.propertyResolver.getProperty("sensitive", Boolean.class); + return !this.secure || Boolean.FALSE.equals(sensitive); + } + + private boolean isSecure(Principal principal) { return (principal != null && !principal.getClass().getName() .contains("Anonymous")); } - private boolean useCachedValue(Principal principal) { - long accessTime = System.currentTimeMillis(); - if (cacheIsStale(accessTime) || secure(principal) || !this.delegate.isSensitive()) { - this.lastAccess = accessTime; - return false; - } - return this.cached != null; - } - - private boolean cacheIsStale(long accessTime) { - return this.cached == null - || (accessTime - this.lastAccess) > this.delegate.getTimeToLive(); - } - @Override public String getPath() { return "/" + this.delegate.getId(); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java index 49173bfaa4..28a703a037 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java @@ -23,8 +23,11 @@ import org.junit.Test; import org.springframework.boot.actuate.endpoint.HealthEndpoint; import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.Status; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.PropertySource; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.mock.env.MockEnvironment; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.authority.AuthorityUtils; @@ -44,10 +47,16 @@ import static org.mockito.Mockito.mock; */ public class HealthMvcEndpointTests { + private static final PropertySource NON_SENSITIVE = new MapPropertySource("test", + Collections. singletonMap("endpoints.health.sensitive", + "false"));; + private HealthEndpoint endpoint = null; private HealthMvcEndpoint mvc = null; + private MockEnvironment environment; + private UsernamePasswordAuthenticationToken user = new UsernamePasswordAuthenticationToken( "user", "password", AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_USER")); @@ -57,6 +66,8 @@ public class HealthMvcEndpointTests { this.endpoint = mock(HealthEndpoint.class); given(this.endpoint.isEnabled()).willReturn(true); this.mvc = new HealthMvcEndpoint(this.endpoint); + this.environment = new MockEnvironment(); + this.mvc.setEnvironment(this.environment); } @Test @@ -143,9 +154,9 @@ public class HealthMvcEndpointTests { @Test public void unsecureAnonymousAccessUnrestricted() { + this.environment.getPropertySources().addLast(NON_SENSITIVE); given(this.endpoint.invoke()).willReturn( new Health.Builder().up().withDetail("foo", "bar").build()); - given(this.endpoint.isSensitive()).willReturn(false); Object result = this.mvc.invoke(null); assertTrue(result instanceof Health); assertTrue(((Health) result).getStatus() == Status.UP); @@ -154,8 +165,8 @@ public class HealthMvcEndpointTests { @Test public void unsecureIsNotCachedWhenAnonymousAccessIsUnrestricted() { + this.environment.getPropertySources().addLast(NON_SENSITIVE); given(this.endpoint.getTimeToLive()).willReturn(10000L); - given(this.endpoint.isSensitive()).willReturn(false); given(this.endpoint.invoke()).willReturn( new Health.Builder().up().withDetail("foo", "bar").build()); Object result = this.mvc.invoke(null); diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java index 58eb4eae46..bdfc5c096c 100644 --- a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java @@ -20,7 +20,9 @@ import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.security.SecurityProperties; import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.TestRestTemplate; @@ -47,6 +49,9 @@ import static org.junit.Assert.assertTrue; @ActiveProfiles("endpoints") public class EndpointsPropertiesSampleActuatorApplicationTests { + @Autowired + private SecurityProperties security; + @Value("${local.server.port}") private int port; @@ -64,8 +69,9 @@ public class EndpointsPropertiesSampleActuatorApplicationTests { @Test public void testCustomContextPath() throws Exception { - ResponseEntity entity = new TestRestTemplate().getForEntity( - "http://localhost:" + this.port + "/admin/health", String.class); + ResponseEntity entity = new TestRestTemplate("user", getPassword()) + .getForEntity("http://localhost:" + this.port + "/admin/health", + String.class); assertEquals(HttpStatus.OK, entity.getStatusCode()); assertTrue("Wrong body: " + entity.getBody(), entity.getBody().contains("\"status\":\"UP\"")); @@ -73,4 +79,9 @@ public class EndpointsPropertiesSampleActuatorApplicationTests { assertTrue("Wrong body: " + entity.getBody(), entity.getBody().contains("\"hello\":\"world\"")); } + + private String getPassword() { + return this.security.getUser().getPassword(); + } + } diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/NonSensitiveHealthTests.java b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/NonSensitiveHealthTests.java new file mode 100644 index 0000000000..90c6bf90d1 --- /dev/null +++ b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/NonSensitiveHealthTests.java @@ -0,0 +1,58 @@ +/* + * Copyright 2012-2014 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 + * + * http://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 sample.actuator; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.test.IntegrationTest; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.boot.test.TestRestTemplate; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.WebAppConfiguration; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Tests for /health with {@code endpoints.health.sensitive=false}. + * + * @author Phillip Webb + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = SampleActuatorApplication.class) +@WebAppConfiguration +@IntegrationTest({ "server.port=0", "endpoints.health.sensitive=false" }) +@DirtiesContext +public class NonSensitiveHealthTests { + + @Value("${local.server.port}") + private int port; + + @Test + public void testSecureHealth() throws Exception { + ResponseEntity entity = new TestRestTemplate().getForEntity( + "http://localhost:" + this.port + "/health", String.class); + assertEquals(HttpStatus.OK, entity.getStatusCode()); + assertTrue("Wrong body: " + entity.getBody(), + entity.getBody().contains("\"hello\":1")); + } + +}