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
This commit is contained in:
Dave Syer
2014-11-25 16:30:10 +00:00
parent 337e9bd013
commit 3bb598a421
7 changed files with 108 additions and 23 deletions

View File

@@ -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());

View File

@@ -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.
*/

View File

@@ -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;
}

View File

@@ -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");
}
}
}

View File

@@ -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);