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 770774a4bf..7d82d4a7fd 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 @@ -31,7 +31,10 @@ import org.springframework.context.EnvironmentAware; import org.springframework.core.env.Environment; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; @@ -54,6 +57,8 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { private RelaxedPropertyResolver propertyResolver; + private RelaxedPropertyResolver roleResolver; + private long lastAccess = 0; private Health cached; @@ -80,6 +85,8 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { public void setEnvironment(Environment environment) { this.propertyResolver = new RelaxedPropertyResolver(environment, "endpoints.health."); + this.roleResolver = new RelaxedPropertyResolver(environment, + "management.security."); } /** @@ -177,8 +184,22 @@ public class HealthMvcEndpoint implements MvcEndpoint, EnvironmentAware { } private boolean isSecure(Principal principal) { - return (principal != null - && !principal.getClass().getName().contains("Anonymous")); + if (principal == null || principal.getClass().getName().contains("Anonymous")) { + return false; + } + if (!ClassUtils.isPresent("org.springframework.security.core.Authentication", + null) || !(principal instanceof Authentication)) { + return false; + } + String role = this.roleResolver.getProperty("role", "ROLE_ADMIN"); + Authentication authentication = (Authentication) principal; + for (GrantedAuthority authority : authentication.getAuthorities()) { + String name = authority.getAuthority(); + if (role.equals(name) || ("ROLE_" + role).equals(name)) { + return true; + } + } + return false; } private boolean isUnrestricted() { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/AdhocTestSuite.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/AdhocTestSuite.java index ec1ddaf846..03959ca005 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/AdhocTestSuite.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/AdhocTestSuite.java @@ -20,8 +20,8 @@ import org.junit.Ignore; import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; -import org.springframework.boot.actuate.autoconfigure.MetricRepositoryAutoConfigurationTests; -import org.springframework.boot.actuate.autoconfigure.PublicMetricsAutoConfigurationTests; +import org.springframework.boot.actuate.endpoint.FlywayEndpointTests; +import org.springframework.boot.actuate.health.DataSourceHealthIndicatorTests; /** * A test suite for probing weird ordering problems in the tests. @@ -29,8 +29,7 @@ import org.springframework.boot.actuate.autoconfigure.PublicMetricsAutoConfigura * @author Dave Syer */ @RunWith(Suite.class) -@SuiteClasses({ PublicMetricsAutoConfigurationTests.class, - MetricRepositoryAutoConfigurationTests.class }) +@SuiteClasses({ DataSourceHealthIndicatorTests.class, FlywayEndpointTests.class }) @Ignore public class AdhocTestSuite { 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 f2bd062518..33cdf087da 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 @@ -63,6 +63,10 @@ public class HealthMvcEndpointTests { "user", "password", AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_USER")); + private UsernamePasswordAuthenticationToken admin = new UsernamePasswordAuthenticationToken( + "user", "password", + AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_ADMIN")); + @Before public void init() { this.endpoint = mock(HealthEndpoint.class); @@ -94,10 +98,10 @@ public class HealthMvcEndpointTests { @Test @SuppressWarnings("unchecked") public void customMapping() { - given(this.endpoint.invoke()).willReturn( - new Health.Builder().status("OK").build()); - this.mvc.setStatusMapping(Collections.singletonMap("OK", - HttpStatus.INTERNAL_SERVER_ERROR)); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().status("OK").build()); + this.mvc.setStatusMapping( + Collections.singletonMap("OK", HttpStatus.INTERNAL_SERVER_ERROR)); Object result = this.mvc.invoke(null); assertTrue(result instanceof ResponseEntity); ResponseEntity response = (ResponseEntity) result; @@ -108,8 +112,8 @@ public class HealthMvcEndpointTests { @Test @SuppressWarnings("unchecked") public void customMappingWithRelaxedName() { - given(this.endpoint.invoke()).willReturn( - new Health.Builder().outOfService().build()); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().outOfService().build()); this.mvc.setStatusMapping(Collections.singletonMap("out-of-service", HttpStatus.INTERNAL_SERVER_ERROR)); Object result = this.mvc.invoke(null); @@ -120,23 +124,33 @@ public class HealthMvcEndpointTests { } @Test - public void secure() { - given(this.endpoint.invoke()).willReturn( - new Health.Builder().up().withDetail("foo", "bar").build()); + public void secureEvenWhenNotSensitive() { + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); given(this.endpoint.isSensitive()).willReturn(false); - Object result = this.mvc.invoke(this.user); + Object result = this.mvc.invoke(this.admin); assertTrue(result instanceof Health); assertTrue(((Health) result).getStatus() == Status.UP); assertEquals("bar", ((Health) result).getDetails().get("foo")); } + @Test + public void secureNonAdmin() { + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(this.user); + assertTrue(result instanceof Health); + assertTrue(((Health) result).getStatus() == Status.UP); + assertNull(((Health) result).getDetails().get("foo")); + } + @Test public void healthIsCached() { given(this.endpoint.getTimeToLive()).willReturn(10000L); 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); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(this.admin); assertTrue(result instanceof Health); Health health = (Health) result; assertTrue(health.getStatus() == Status.UP); @@ -156,8 +170,8 @@ public class HealthMvcEndpointTests { 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()); + 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); @@ -167,8 +181,8 @@ public class HealthMvcEndpointTests { @Test public void unsensitiveAnonymousAccessRestricted() { this.environment.getPropertySources().addLast(NON_SENSITIVE); - given(this.endpoint.invoke()).willReturn( - new Health.Builder().up().withDetail("foo", "bar").build()); + 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); @@ -180,8 +194,8 @@ public class HealthMvcEndpointTests { 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()); + 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); @@ -191,8 +205,8 @@ public class HealthMvcEndpointTests { @Test public void noCachingWhenTimeToLiveIsZero() { given(this.endpoint.getTimeToLive()).willReturn(0L); - given(this.endpoint.invoke()).willReturn( - new Health.Builder().up().withDetail("foo", "bar").build()); + 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); @@ -207,8 +221,8 @@ public class HealthMvcEndpointTests { public void newValueIsReturnedOnceTtlExpires() throws InterruptedException { given(this.endpoint.getTimeToLive()).willReturn(50L); given(this.endpoint.isSensitive()).willReturn(false); - given(this.endpoint.invoke()).willReturn( - new Health.Builder().up().withDetail("foo", "bar").build()); + 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); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java index 57a620ff70..2a15a3bbd5 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java @@ -182,8 +182,8 @@ public class MvcEndpointIntegrationTests { EnvironmentTestUtils.addEnvironment(this.context, "spring.jackson.serialization.indent-output:true"); MockMvc mockMvc = createMockMvc(); - mockMvc.perform(get("/beans")).andExpect( - content().string(startsWith("{" + LINE_SEPARATOR))); + mockMvc.perform(get("/beans")) + .andExpect(content().string(startsWith("{" + LINE_SEPARATOR))); } private MockMvc createMockMvc() { @@ -205,8 +205,8 @@ public class MvcEndpointIntegrationTests { } @ImportAutoConfiguration({ JacksonAutoConfiguration.class, - HttpMessageConvertersAutoConfiguration.class, - EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class, + HttpMessageConvertersAutoConfiguration.class, EndpointAutoConfiguration.class, + EndpointWebMvcAutoConfiguration.class, ManagementServerPropertiesAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, WebMvcAutoConfiguration.class }) static class DefaultConfiguration { @@ -224,8 +224,8 @@ public class MvcEndpointIntegrationTests { @ImportAutoConfiguration({ HypermediaAutoConfiguration.class, RepositoryRestMvcAutoConfiguration.class, JacksonAutoConfiguration.class, - HttpMessageConvertersAutoConfiguration.class, - EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class, + HttpMessageConvertersAutoConfiguration.class, EndpointAutoConfiguration.class, + EndpointWebMvcAutoConfiguration.class, ManagementServerPropertiesAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, WebMvcAutoConfiguration.class }) static class SpringDataRestConfiguration {