Commit 1f6ac52b authored by Dave Syer's avatar Dave Syer

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
parent c3c1d91f
...@@ -205,7 +205,7 @@ public class EndpointWebMvcAutoConfiguration implements ApplicationContextAware, ...@@ -205,7 +205,7 @@ public class EndpointWebMvcAutoConfiguration implements ApplicationContextAware,
@ConditionalOnEnabledEndpoint("health") @ConditionalOnEnabledEndpoint("health")
public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) { public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) {
Security security = this.managementServerProperties.getSecurity(); Security security = this.managementServerProperties.getSecurity();
boolean secure = (security == null || security.isEnabled()); boolean secure = (security != null && security.isEnabled());
HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate, secure); HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate, secure);
if (this.healthMvcEndpointProperties.getMapping() != null) { if (this.healthMvcEndpointProperties.getMapping() != null) {
healthMvcEndpoint.addStatusMapping(this.healthMvcEndpointProperties healthMvcEndpoint.addStatusMapping(this.healthMvcEndpointProperties
......
...@@ -165,7 +165,7 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { ...@@ -165,7 +165,7 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware {
private boolean isUnrestricted() { private boolean isUnrestricted() {
Boolean sensitive = this.propertyResolver.getProperty("sensitive", Boolean.class); Boolean sensitive = this.propertyResolver.getProperty("sensitive", Boolean.class);
return !this.secure || Boolean.FALSE.equals(sensitive); return !this.secure && !Boolean.TRUE.equals(sensitive);
} }
@Override @Override
......
...@@ -34,6 +34,7 @@ import org.springframework.security.core.authority.AuthorityUtils; ...@@ -34,6 +34,7 @@ import org.springframework.security.core.authority.AuthorityUtils;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
...@@ -139,6 +140,31 @@ public class HealthMvcEndpointTests { ...@@ -139,6 +140,31 @@ public class HealthMvcEndpointTests {
@Test @Test
public void unsecureAnonymousAccessUnrestricted() { 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); this.environment.getPropertySources().addLast(NON_SENSITIVE);
given(this.endpoint.invoke()).willReturn( given(this.endpoint.invoke()).willReturn(
new Health.Builder().up().withDetail("foo", "bar").build()); new Health.Builder().up().withDetail("foo", "bar").build());
......
...@@ -31,6 +31,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; ...@@ -31,6 +31,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.web.WebAppConfiguration; import org.springframework.test.context.web.WebAppConfiguration;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
/** /**
* Basic integration tests for service demo application. * Basic integration tests for service demo application.
...@@ -51,11 +52,22 @@ public class SampleActuatorApplicationTests { ...@@ -51,11 +52,22 @@ public class SampleActuatorApplicationTests {
public void testHome() throws Exception { public void testHome() throws Exception {
@SuppressWarnings("rawtypes") @SuppressWarnings("rawtypes")
ResponseEntity<Map> entity = new TestRestTemplate().getForEntity( ResponseEntity<Map> entity = new TestRestTemplate().getForEntity(
"http://localhost:" + port, Map.class); "http://localhost:" + this.port, Map.class);
assertEquals(HttpStatus.OK, entity.getStatusCode()); assertEquals(HttpStatus.OK, entity.getStatusCode());
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
Map<String, Object> body = entity.getBody(); Map<String, Object> body = entity.getBody();
assertEquals("Hello Phil", body.get("message")); assertEquals("Hello Phil", body.get("message"));
} }
@Test
public void testHealth() throws Exception {
@SuppressWarnings("rawtypes")
ResponseEntity<Map> entity = new TestRestTemplate().getForEntity(
"http://localhost:" + this.port + "/health", Map.class);
assertEquals(HttpStatus.OK, entity.getStatusCode());
@SuppressWarnings("unchecked")
Map<String, Object> body = entity.getBody();
assertNotNull(body.get("diskSpace"));
}
} }
...@@ -29,7 +29,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; ...@@ -29,7 +29,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.web.WebAppConfiguration; import org.springframework.test.context.web.WebAppConfiguration;
import static org.junit.Assert.assertEquals; 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}. * Tests for /health with {@code endpoints.health.sensitive=false}.
...@@ -51,7 +51,7 @@ public class NonSensitiveHealthTests { ...@@ -51,7 +51,7 @@ public class NonSensitiveHealthTests {
ResponseEntity<String> entity = new TestRestTemplate().getForEntity( ResponseEntity<String> entity = new TestRestTemplate().getForEntity(
"http://localhost:" + this.port + "/health", String.class); "http://localhost:" + this.port + "/health", String.class);
assertEquals(HttpStatus.OK, entity.getStatusCode()); assertEquals(HttpStatus.OK, entity.getStatusCode());
assertTrue("Wrong body: " + entity.getBody(), assertFalse("Wrong body: " + entity.getBody(),
entity.getBody().contains("\"hello\":1")); entity.getBody().contains("\"hello\":1"));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment