From 1f6ac52b961f788408a55621e3cb4ec4a345e409 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 26 Jun 2015 13:44:52 +0100 Subject: [PATCH] Change default behaviour of /health when not secured The default is now to reveal all details unless sensitive=true (instead of only revealing then if sensitive was explicitly false). The definition of "secure" also changes to something more sensible where it is only true if security is enabled. Fixes gh-2816 --- .../EndpointWebMvcAutoConfiguration.java | 2 +- .../endpoint/mvc/HealthMvcEndpoint.java | 2 +- .../endpoint/mvc/HealthMvcEndpointTests.java | 26 +++++++++++++++++++ .../log4j/SampleActuatorApplicationTests.java | 14 +++++++++- .../actuator/NonSensitiveHealthTests.java | 4 +-- 5 files changed, 43 insertions(+), 5 deletions(-) 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 5f3fae9a420..df1151c542b 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 @@ -205,7 +205,7 @@ public class EndpointWebMvcAutoConfiguration implements ApplicationContextAware, @ConditionalOnEnabledEndpoint("health") public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) { Security security = this.managementServerProperties.getSecurity(); - boolean secure = (security == null || security.isEnabled()); + boolean secure = (security != null && security.isEnabled()); HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate, secure); if (this.healthMvcEndpointProperties.getMapping() != null) { healthMvcEndpoint.addStatusMapping(this.healthMvcEndpointProperties 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 dad7422958e..7402df864b9 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 @@ -165,7 +165,7 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { private boolean isUnrestricted() { Boolean sensitive = this.propertyResolver.getProperty("sensitive", Boolean.class); - return !this.secure || Boolean.FALSE.equals(sensitive); + return !this.secure && !Boolean.TRUE.equals(sensitive); } @Override 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 05a2061b2df..2ae788f2b06 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 @@ -34,6 +34,7 @@ import org.springframework.security.core.authority.AuthorityUtils; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.BDDMockito.given; @@ -139,6 +140,31 @@ public class HealthMvcEndpointTests { @Test public void unsecureAnonymousAccessUnrestricted() { + this.mvc = new HealthMvcEndpoint(this.endpoint, false); + this.mvc.setEnvironment(this.environment); + given(this.endpoint.invoke()).willReturn( + new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(null); + assertTrue(result instanceof Health); + assertTrue(((Health) result).getStatus() == Status.UP); + assertEquals("bar", ((Health) result).getDetails().get("foo")); + } + + @Test + public void unsensitiveAnonymousAccessRestricted() { + this.environment.getPropertySources().addLast(NON_SENSITIVE); + given(this.endpoint.invoke()).willReturn( + new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(null); + assertTrue(result instanceof Health); + assertTrue(((Health) result).getStatus() == Status.UP); + assertNull(((Health) result).getDetails().get("foo")); + } + + @Test + public void unsecureUnsensitiveAnonymousAccessUnrestricted() { + this.mvc = new HealthMvcEndpoint(this.endpoint, false); + this.mvc.setEnvironment(this.environment); this.environment.getPropertySources().addLast(NON_SENSITIVE); given(this.endpoint.invoke()).willReturn( new Health.Builder().up().withDetail("foo", "bar").build()); diff --git a/spring-boot-samples/spring-boot-sample-actuator-log4j/src/test/java/sample/actuator/log4j/SampleActuatorApplicationTests.java b/spring-boot-samples/spring-boot-sample-actuator-log4j/src/test/java/sample/actuator/log4j/SampleActuatorApplicationTests.java index 1137113d763..0067af03e56 100644 --- a/spring-boot-samples/spring-boot-sample-actuator-log4j/src/test/java/sample/actuator/log4j/SampleActuatorApplicationTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator-log4j/src/test/java/sample/actuator/log4j/SampleActuatorApplicationTests.java @@ -31,6 +31,7 @@ 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.assertNotNull; /** * Basic integration tests for service demo application. @@ -51,11 +52,22 @@ public class SampleActuatorApplicationTests { public void testHome() throws Exception { @SuppressWarnings("rawtypes") ResponseEntity entity = new TestRestTemplate().getForEntity( - "http://localhost:" + port, Map.class); + "http://localhost:" + this.port, Map.class); assertEquals(HttpStatus.OK, entity.getStatusCode()); @SuppressWarnings("unchecked") Map body = entity.getBody(); assertEquals("Hello Phil", body.get("message")); } + @Test + public void testHealth() throws Exception { + @SuppressWarnings("rawtypes") + ResponseEntity entity = new TestRestTemplate().getForEntity( + "http://localhost:" + this.port + "/health", Map.class); + assertEquals(HttpStatus.OK, entity.getStatusCode()); + @SuppressWarnings("unchecked") + Map body = entity.getBody(); + assertNotNull(body.get("diskSpace")); + } + } 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 index 90c6bf90d1c..c8bcf817207 100644 --- 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 @@ -29,7 +29,7 @@ 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; +import static org.junit.Assert.assertFalse; /** * Tests for /health with {@code endpoints.health.sensitive=false}. @@ -51,7 +51,7 @@ public class NonSensitiveHealthTests { ResponseEntity entity = new TestRestTemplate().getForEntity( "http://localhost:" + this.port + "/health", String.class); assertEquals(HttpStatus.OK, entity.getStatusCode()); - assertTrue("Wrong body: " + entity.getBody(), + assertFalse("Wrong body: " + entity.getBody(), entity.getBody().contains("\"hello\":1")); }