Only hide /health details if the app is actually secure

Also gives the user the option to override (by setting
endpoints.health.sensitive=false).

Fixes gh-1977 in a slightly different way

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java

@@ -69,6 +69,7 @@ import org.springframework.context.annotation.Configuration;
 import org.springframework.context.event.ContextClosedEvent;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.PropertySource;
+import org.springframework.util.ClassUtils;
 import org.springframework.web.context.WebApplicationContext;
 import org.springframework.web.filter.OncePerRequestFilter;
 import org.springframework.web.servlet.DispatcherServlet;
@@ -164,6 +165,10 @@ public class EndpointWebMvcAutoConfiguration implements ApplicationContextAware,
 	@ConditionalOnProperty(prefix = "endpoints.health", name = "enabled", matchIfMissing = true)
 	public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) {
 		HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate);
+		boolean secure = this.managementServerProperties.getSecurity().isEnabled()
+				&& ClassUtils.isPresent(
+						"org.springframework.security.core.Authentication", null);
+		delegate.setSensitive(secure);
 		if (this.healthMvcEndpointProperties.getMapping() != null) {
 			healthMvcEndpoint.addStatusMapping(this.healthMvcEndpointProperties
 					.getMapping());

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java

@@ -39,8 +39,6 @@ public class HealthEndpoint extends AbstractEndpoint<Health> {
 
 	private long timeToLive = 1000;
 
-	private boolean restrictAnonymousAccess = true;
-
 	/**
 	 * Create a new {@link HealthIndicator} instance.
 	 */
@@ -72,14 +70,6 @@ public class HealthEndpoint extends AbstractEndpoint<Health> {
 		this.timeToLive = ttl;
 	}
 
-	public boolean isRestrictAnonymousAccess() {
-		return this.restrictAnonymousAccess;
-	}
-
-	public void setRestrictAnonymousAccess(boolean restrictAnonymousAccess) {
-		this.restrictAnonymousAccess = restrictAnonymousAccess;
-	}
-
 	/**
 	 * Invoke all {@link HealthIndicator} delegates and collect their health information.
 	 */

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java

@@ -122,7 +122,7 @@ public class HealthMvcEndpoint implements MvcEndpoint {
 		// Not too worried about concurrent access here, the worst that can happen is the
 		// odd extra call to delegate.invoke()
 		this.cached = health;
-		if (this.delegate.isRestrictAnonymousAccess() && !secure(principal)) {
+		if (!secure(principal) && this.delegate.isSensitive()) {
 			// If not secure we only expose the status
 			health = Health.status(health.getStatus()).build();
 		}
@@ -135,8 +135,7 @@ public class HealthMvcEndpoint implements MvcEndpoint {
 
 	private boolean useCachedValue(Principal principal) {
 		long accessTime = System.currentTimeMillis();
-		if (cacheIsStale(accessTime) || secure(principal)
-				|| !this.delegate.isRestrictAnonymousAccess()) {
+		if (cacheIsStale(accessTime) || secure(principal) || !this.delegate.isSensitive()) {
 			this.lastAccess = accessTime;
 			return false;
 		}

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java

@@ -0,0 +1,92 @@
+/*
+ * 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 org.springframework.boot.actuate.autoconfigure;
+
+import org.junit.After;
+import org.junit.Test;
+import org.springframework.boot.actuate.endpoint.mvc.HealthMvcEndpoint;
+import org.springframework.boot.actuate.health.AbstractHealthIndicator;
+import org.springframework.boot.actuate.health.Health;
+import org.springframework.boot.actuate.health.Health.Builder;
+import org.springframework.boot.actuate.health.Status;
+import org.springframework.boot.autoconfigure.security.SecurityAutoConfiguration;
+import org.springframework.boot.test.EnvironmentTestUtils;
+import org.springframework.mock.web.MockServletContext;
+import org.springframework.stereotype.Component;
+import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Dave Syer
+ */
+public class HealthMvcEndpointAutoConfigurationTests {
+
+	private AnnotationConfigWebApplicationContext context;
+
+	@After
+	public void close() {
+		if (this.context != null) {
+			this.context.close();
+		}
+	}
+
+	@Test
+	public void testSecureByDefault() throws Exception {
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.setServletContext(new MockServletContext());
+		this.context.register(SecurityAutoConfiguration.class,
+				ManagementServerPropertiesAutoConfiguration.class,
+				EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class,
+				TestHealthIndicator.class);
+		this.context.refresh();
+		Health health = (Health) this.context.getBean(HealthMvcEndpoint.class).invoke(
+				null);
+		assertEquals(Status.UP, health.getStatus());
+		assertEquals(null, health.getDetails().get("foo"));
+	}
+
+	@Test
+	public void testNotSecured() throws Exception {
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.setServletContext(new MockServletContext());
+		this.context.register(SecurityAutoConfiguration.class,
+				ManagementServerPropertiesAutoConfiguration.class,
+				EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class,
+				TestHealthIndicator.class);
+		EnvironmentTestUtils.addEnvironment(this.context,
+				"management.security.enabled=false");
+		this.context.refresh();
+		Health health = (Health) this.context.getBean(HealthMvcEndpoint.class).invoke(
+				null);
+		assertEquals(Status.UP, health.getStatus());
+		Health map = (Health) health.getDetails().get(
+				"healthMvcEndpointAutoConfigurationTests.Test");
+		assertEquals("bar", map.getDetails().get("foo"));
+	}
+
+	@Component
+	protected static class TestHealthIndicator extends AbstractHealthIndicator {
+
+		@Override
+		protected void doHealthCheck(Builder builder) throws Exception {
+			builder.up().withDetail("foo", "bar");
+		}
+
+	}
+
+}

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java

@@ -96,7 +96,7 @@ public class HealthMvcEndpointTests {
 	public void secure() {
 		given(this.endpoint.invoke()).willReturn(
 				new Health.Builder().up().withDetail("foo", "bar").build());
-		given(this.endpoint.isRestrictAnonymousAccess()).willReturn(true);
+		given(this.endpoint.isSensitive()).willReturn(false);
 		Object result = this.mvc.invoke(this.user);
 		assertTrue(result instanceof Health);
 		assertTrue(((Health) result).getStatus() == Status.UP);
@@ -106,7 +106,7 @@ public class HealthMvcEndpointTests {
 	@Test
 	public void secureNotCached() {
 		given(this.endpoint.getTimeToLive()).willReturn(10000L);
-		given(this.endpoint.isRestrictAnonymousAccess()).willReturn(true);
+		given(this.endpoint.isSensitive()).willReturn(false);
 		given(this.endpoint.invoke()).willReturn(
 				new Health.Builder().up().withDetail("foo", "bar").build());
 		Object result = this.mvc.invoke(this.user);
@@ -122,7 +122,7 @@ public class HealthMvcEndpointTests {
 	@Test
 	public void unsecureCached() {
 		given(this.endpoint.getTimeToLive()).willReturn(10000L);
-		given(this.endpoint.isRestrictAnonymousAccess()).willReturn(true);
+		given(this.endpoint.isSensitive()).willReturn(true);
 		given(this.endpoint.invoke()).willReturn(
 				new Health.Builder().up().withDetail("foo", "bar").build());
 		Object result = this.mvc.invoke(this.user);
@@ -145,7 +145,7 @@ public class HealthMvcEndpointTests {
 	public void unsecureAnonymousAccessUnrestricted() {
 		given(this.endpoint.invoke()).willReturn(
 				new Health.Builder().up().withDetail("foo", "bar").build());
-		given(this.endpoint.isRestrictAnonymousAccess()).willReturn(false);
+		given(this.endpoint.isSensitive()).willReturn(false);
 		Object result = this.mvc.invoke(null);
 		assertTrue(result instanceof Health);
 		assertTrue(((Health) result).getStatus() == Status.UP);
@@ -155,7 +155,7 @@ public class HealthMvcEndpointTests {
 	@Test
 	public void unsecureIsNotCachedWhenAnonymousAccessIsUnrestricted() {
 		given(this.endpoint.getTimeToLive()).willReturn(10000L);
-		given(this.endpoint.isRestrictAnonymousAccess()).willReturn(false);
+		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);
@@ -171,7 +171,7 @@ public class HealthMvcEndpointTests {
 	@Test
 	public void newValueIsReturnedOnceTtlExpires() throws InterruptedException {
 		given(this.endpoint.getTimeToLive()).willReturn(50L);
-		given(this.endpoint.isRestrictAnonymousAccess()).willReturn(true);
+		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);

spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc

@@ -402,9 +402,8 @@ content into your application; rather pick only the properties that you need.
 	endpoints.env.enabled=true
 	endpoints.env.keys-to-sanitize=password,secret,key # suffix or regex
 	endpoints.health.id=health
-	endpoints.health.sensitive=false
+	endpoints.health.sensitive=true
 	endpoints.health.enabled=true
-	endpoints.health.restrict-anonymous-access=true
 	endpoints.health.time-to-live=1000
 	endpoints.info.id=info
 	endpoints.info.sensitive=false

spring-boot-docs/src/main/asciidoc/production-ready-features.adoc

@@ -413,7 +413,7 @@ If you don't want to expose endpoints over HTTP you can set the management port
 [[production-ready-health-access-restrictions]]
 === Health endpoint anonymous access restrictions
 The information exposed by the health endpoint varies depending on whether or not it's
-accessed anonymously. When accessed anonymously, any details about the server's health
+accessed anonymously. By default, when accessed anonymously, any details about the server's health
 are hidden and the endpoint will simply indicate whether or not the server is up or
 down. Furthermore, when accessed anonymously, the response is cached for a configurable
 period to prevent the endpoint being used in a denial of service attack.
@@ -421,7 +421,7 @@ The `endpoints.health.time-to-live` property is used to configure the caching pe
 milliseconds. It defaults to 1000, i.e. one second.
 
 The above-described restrictions can be disabled, thereby allowing anonymous users full
-access to the health endpoint. To do so, set `endpoints.health.restrict-anonymous-access`
+access to the health endpoint. To do so, set `endpoints.health.sensitive`
 to `false`.