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 3addb3832e4b..eb87a13d32d9 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 @@ -141,11 +141,9 @@ public void init(WebSecurity builder) throws Exception { // add them back. List ignored = SpringBootWebSecurityConfiguration .getIgnored(this.security); - ignored.addAll(Arrays.asList(getEndpointPaths(this.endpointHandlerMapping, - false))); if (!this.management.getSecurity().isEnabled()) { ignored.addAll(Arrays.asList(getEndpointPaths( - this.endpointHandlerMapping, true))); + this.endpointHandlerMapping))); } if (ignored.contains("none")) { ignored.remove("none"); @@ -220,7 +218,7 @@ protected static class ManagementWebSecurityConfigurerAdapter extends protected void configure(HttpSecurity http) throws Exception { // secure endpoints - String[] paths = getEndpointPaths(this.endpointHandlerMapping, true); + String[] paths = getEndpointPaths(this.endpointHandlerMapping); if (paths.length > 0 && this.management.getSecurity().isEnabled()) { // Always protect them if present if (this.security.isRequireSsl()) { @@ -229,10 +227,12 @@ protected void configure(HttpSecurity http) throws Exception { http.exceptionHandling().authenticationEntryPoint(entryPoint()); paths = this.server.getPathsArray(paths); http.requestMatchers().antMatchers(paths); - http.authorizeRequests().anyRequest() - .hasRole(this.management.getSecurity().getRole()) // - .and().httpBasic() // - .and().anonymous().disable(); + // @formatter:off + http.authorizeRequests() + .antMatchers(this.server.getPathsArray(getEndpointPaths(this.endpointHandlerMapping, false))).access("permitAll()") + .anyRequest().hasRole(this.management.getSecurity().getRole()); + // @formatter:on + http.httpBasic(); // No cookies for management endpoints by default http.csrf().disable(); @@ -254,6 +254,12 @@ private AuthenticationEntryPoint entryPoint() { } + private static String[] getEndpointPaths(EndpointHandlerMapping endpointHandlerMapping) { + return StringUtils.mergeStringArrays( + getEndpointPaths(endpointHandlerMapping, false), + getEndpointPaths(endpointHandlerMapping, true)); + } + private static String[] getEndpointPaths( EndpointHandlerMapping endpointHandlerMapping, boolean secure) { if (endpointHandlerMapping == null) { @@ -264,13 +270,11 @@ private static String[] getEndpointPaths( List paths = new ArrayList(endpoints.size()); for (MvcEndpoint endpoint : endpoints) { if (endpoint.isSensitive() == secure) { - String path = endpointHandlerMapping.getPrefix() + endpoint.getPath(); + String path = endpointHandlerMapping.getPath(endpoint.getPath()); paths.add(path); - if (secure) { - // Add Spring MVC-generated additional paths - paths.add(path + "/"); - paths.add(path + ".*"); - } + // Add Spring MVC-generated additional paths + paths.add(path + "/"); + paths.add(path + ".*"); } } return paths.toArray(new String[paths.size()]); diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java index 13ebbe25de80..050c1a44b040 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java @@ -202,6 +202,7 @@ private Map sanitize(Map map) { * Extension to {@link JacksonAnnotationIntrospector} to suppress CGLIB generated bean * properties. */ + @SuppressWarnings("serial") private static class CglibAnnotationIntrospector extends JacksonAnnotationIntrospector { diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java index e50db2890046..de0952f28476 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java @@ -36,6 +36,22 @@ public class HealthEndpoint extends AbstractEndpoint { private final HealthIndicator healthIndicator; + private long ttl = 1000; + + /** + * Time to live for cached result. If accessed anonymously, we might need to cache the + * result of this endpoint to prevent a DOS attack. + * + * @return time to live in milliseconds (default 1000) + */ + public long getTtl() { + return ttl; + } + + public void setTtl(long ttl) { + this.ttl = ttl; + } + /** * Create a new {@link HealthIndicator} instance. */ 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 de6e4636029a..6c4584ec2fdc 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 @@ -18,7 +18,9 @@ import java.lang.reflect.Method; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.springframework.boot.actuate.endpoint.Endpoint; @@ -50,7 +52,7 @@ public class EndpointHandlerMapping extends RequestMappingHandlerMapping implements ApplicationContextAware { - private final Set endpoints; + private final Map endpoints = new HashMap(); private String prefix = ""; @@ -62,7 +64,10 @@ public class EndpointHandlerMapping extends RequestMappingHandlerMapping impleme * @param endpoints */ public EndpointHandlerMapping(Collection endpoints) { - this.endpoints = new HashSet(endpoints); + HashMap map = (HashMap) this.endpoints; + for (MvcEndpoint endpoint : endpoints) { + map.put(endpoint.getPath(), endpoint); + } // By default the static resource handler mapping is LOWEST_PRECEDENCE - 1 // and the RequestMappingHandlerMapping is 0 (we ideally want to be before both) setOrder(-100); @@ -72,7 +77,7 @@ public EndpointHandlerMapping(Collection endpoints) { public void afterPropertiesSet() { super.afterPropertiesSet(); if (!this.disabled) { - for (MvcEndpoint endpoint : this.endpoints) { + for (MvcEndpoint endpoint : this.endpoints.values()) { detectHandlerMethods(endpoint); } } @@ -146,6 +151,13 @@ public String getPrefix() { return this.prefix; } + /** + * @return the path used in mappings + */ + public String getPath(String endpoint) { + return this.prefix + endpoint; + } + /** * Sets if this mapping is disabled. */ @@ -164,7 +176,7 @@ public boolean isDisabled() { * Return the endpoints */ public Set getEndpoints() { - return this.endpoints; + return new HashSet(this.endpoints.values()); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java index 268939ec7815..28e34f0803cd 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/EnvironmentMvcEndpoint.java @@ -56,6 +56,7 @@ public void setEnvironment(Environment environment) { this.environment = environment; } + @SuppressWarnings("serial") @ResponseStatus(value = HttpStatus.NOT_FOUND, reason = "No such property") public static class NoSuchPropertyException extends RuntimeException { 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 210c9b206802..5a7971ff3146 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 @@ -16,10 +16,12 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.security.Principal; import java.util.Collections; import java.util.HashMap; import java.util.Map; +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; @@ -33,14 +35,21 @@ * Adapter to expose {@link HealthEndpoint} as an {@link MvcEndpoint}. * * @author Christian Dupuis + * @author Dave Syer * @since 1.1.0 */ -public class HealthMvcEndpoint extends EndpointMvcAdapter { +public class HealthMvcEndpoint implements MvcEndpoint { private Map statusMapping = new HashMap(); + private HealthEndpoint delegate; + + private long lastAccess = 0; + + private Health cached; + public HealthMvcEndpoint(HealthEndpoint delegate) { - super(delegate); + this.delegate = delegate; setupDefaultStatusMapping(); } @@ -91,21 +100,65 @@ public void addStatusMapping(String statusCode, HttpStatus httpStatus) { @RequestMapping @ResponseBody - @Override - public Object invoke() { - if (!this.getDelegate().isEnabled()) { - // Shouldn't happen + public Object invoke(Principal principal) { + + if (!delegate.isEnabled()) { + // Shouldn't happen because the request mapping should not be registered return new ResponseEntity>(Collections.singletonMap( "message", "This endpoint is disabled"), HttpStatus.NOT_FOUND); } - Health health = (Health) getDelegate().invoke(); + Health health = getHealth(principal); Status status = health.getStatus(); if (this.statusMapping.containsKey(status.getCode())) { return new ResponseEntity(health, this.statusMapping.get(status .getCode())); } + return health; + + } + + private Health getHealth(Principal principal) { + Health health = useCachedValue(principal) ? cached : (Health) delegate.invoke(); + // Not too worried about concurrent access here, the worst that can happen is the + // odd extra call to delegate.invoke() + cached = health; + if (!secure(principal)) { + // If not secure we only expose the status + health = Health.status(health.getStatus()).build(); + } + return health; + } + + private boolean secure(Principal principal) { + return principal != null && !principal.getClass().getName().contains("Anonymous"); + } + + private boolean useCachedValue(Principal principal) { + long currentAccess = System.currentTimeMillis(); + if (cached == null || secure(principal) + || currentAccess - lastAccess > delegate.getTtl()) { + lastAccess = currentAccess; + return false; + } + return cached != null; + } + + @Override + public String getPath() { + return "/" + this.delegate.getId(); + } + + @Override + public boolean isSensitive() { + return this.delegate.isSensitive(); + } + + @Override + @SuppressWarnings("rawtypes") + public Class getEndpointType() { + return this.delegate.getClass(); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MetricsMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MetricsMvcEndpoint.java index 496b4811bd22..eb9b951daa19 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MetricsMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MetricsMvcEndpoint.java @@ -48,6 +48,7 @@ public Object value(@PathVariable String name) { return value; } + @SuppressWarnings("serial") @ResponseStatus(value = HttpStatus.NOT_FOUND, reason = "No such metric") public static class NoSuchMetricException extends RuntimeException { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfigurationTests.java index 5874b0afdabb..1d225236b2bf 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfigurationTests.java @@ -73,8 +73,8 @@ public void testWebConfiguration() throws Exception { PropertyPlaceholderAutoConfiguration.class); this.context.refresh(); assertNotNull(this.context.getBean(AuthenticationManagerBuilder.class)); - // 6 for static resources, one for management endpoints and one for the rest - assertEquals(8, this.context.getBean(FilterChainProxy.class).getFilterChains() + // 4 for static resources, one for management endpoints and one for the rest + assertEquals(6, this.context.getBean(FilterChainProxy.class).getFilterChains() .size()); } @@ -144,7 +144,7 @@ public void testDisableBasicAuthOnApplicationPaths() throws Exception { this.context.refresh(); // Just the management endpoints (one filter) and ignores now plus the backup // filter on app endpoints - assertEquals(8, this.context.getBean(FilterChainProxy.class).getFilterChains() + assertEquals(6, this.context.getBean(FilterChainProxy.class).getFilterChains() .size()); } 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 08c7873eb438..a712c30a3aa8 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 @@ -16,6 +16,11 @@ package org.springframework.boot.actuate.endpoint.mvc; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + import java.util.Collections; import org.junit.Before; @@ -25,11 +30,8 @@ import org.springframework.boot.actuate.health.Status; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.AuthorityUtils; /** * Tests for {@link HealthMvcEndpoint}. @@ -42,6 +44,10 @@ public class HealthMvcEndpointTests { private HealthMvcEndpoint mvc = null; + private UsernamePasswordAuthenticationToken user = new UsernamePasswordAuthenticationToken( + "user", "password", + AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_USER")); + @Before public void init() { this.endpoint = mock(HealthEndpoint.class); @@ -52,7 +58,7 @@ public void init() { @Test public void up() { given(this.endpoint.invoke()).willReturn(new Health.Builder().up().build()); - Object result = this.mvc.invoke(); + Object result = this.mvc.invoke(null); assertTrue(result instanceof Health); assertTrue(((Health) result).getStatus() == Status.UP); } @@ -61,7 +67,7 @@ public void up() { @Test public void down() { given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); - Object result = this.mvc.invoke(); + Object result = this.mvc.invoke(null); assertTrue(result instanceof ResponseEntity); ResponseEntity response = (ResponseEntity) result; assertTrue(response.getBody().getStatus() == Status.DOWN); @@ -75,10 +81,51 @@ public void customMapping() { new Health.Builder().status("OK").build()); this.mvc.setStatusMapping(Collections.singletonMap("OK", HttpStatus.INTERNAL_SERVER_ERROR)); - Object result = this.mvc.invoke(); + Object result = this.mvc.invoke(null); assertTrue(result instanceof ResponseEntity); ResponseEntity response = (ResponseEntity) result; assertTrue(response.getBody().getStatus().equals(new Status("OK"))); assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); } + + @Test + public void secure() { + given(this.endpoint.invoke()).willReturn( + new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(user); + assertTrue(result instanceof Health); + assertTrue(((Health) result).getStatus() == Status.UP); + assertEquals("bar", ((Health) result).getDetails().get("foo")); + } + + @Test + public void secureNotCached() { + given(this.endpoint.getTtl()).willReturn(10000L); + given(this.endpoint.invoke()).willReturn( + new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(user); + assertTrue(result instanceof Health); + assertTrue(((Health) result).getStatus() == Status.UP); + given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); + result = this.mvc.invoke(user); + @SuppressWarnings("unchecked") + Health health = (Health) ((ResponseEntity) result).getBody(); + assertTrue(health.getStatus() == Status.DOWN); + } + + @Test + public void unsecureCached() { + given(this.endpoint.getTtl()).willReturn(10000L); + given(this.endpoint.invoke()).willReturn( + new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(user); + assertTrue(result instanceof Health); + assertTrue(((Health) result).getStatus() == Status.UP); + given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); + result = this.mvc.invoke(null); // insecure now + Health health = (Health) result; + // so the result is cached + assertTrue(health.getStatus() == Status.UP); + } + } diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/main/java/sample/actuator/SampleActuatorApplication.java b/spring-boot-samples/spring-boot-sample-actuator/src/main/java/sample/actuator/SampleActuatorApplication.java index ac8387a8412b..10f8309a370a 100644 --- a/spring-boot-samples/spring-boot-sample-actuator/src/main/java/sample/actuator/SampleActuatorApplication.java +++ b/spring-boot-samples/spring-boot-sample-actuator/src/main/java/sample/actuator/SampleActuatorApplication.java @@ -17,6 +17,8 @@ package sample.actuator; import org.springframework.boot.SpringApplication; +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.ComponentScan; @@ -26,10 +28,15 @@ @EnableAutoConfiguration @EnableConfigurationProperties @ComponentScan -public class SampleActuatorApplication { +public class SampleActuatorApplication implements HealthIndicator { public static void main(String[] args) throws Exception { SpringApplication.run(SampleActuatorApplication.class, args); } + @Override + public Health health() { + return Health.up().withDetail("hello", "world").build(); + } + } diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/main/resources/application.properties b/spring-boot-samples/spring-boot-sample-actuator/src/main/resources/application.properties index d36c067f8772..21d52efbf37f 100644 --- a/spring-boot-samples/spring-boot-sample-actuator/src/main/resources/application.properties +++ b/spring-boot-samples/spring-boot-sample-actuator/src/main/resources/application.properties @@ -1,5 +1,5 @@ logging.file: /tmp/logs/app.log -logging.level.org.springframework.security: INFO +logging.level.org.springframework.security: DEBUG management.address: 127.0.0.1 #management.port: 8181 endpoints.shutdown.enabled: true diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java index 3007733f6056..bab3aeb79344 100644 --- a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java @@ -132,6 +132,17 @@ public void testHealth() throws Exception { assertEquals(HttpStatus.OK, entity.getStatusCode()); assertTrue("Wrong body: " + entity.getBody(), entity.getBody().contains("\"status\":\"UP\"")); + assertFalse("Wrong body: " + entity.getBody(), + entity.getBody().contains("\"hello\":\"1\"")); + } + + @Test + public void testSecureHealth() throws Exception { + ResponseEntity entity = new TestRestTemplate("user", getPassword()).getForEntity( + "http://localhost:" + this.port + "/health", String.class); + assertEquals(HttpStatus.OK, entity.getStatusCode()); + assertTrue("Wrong body: " + entity.getBody(), + entity.getBody().contains("\"hello\":1")); } @Test