Check authorities of user in HealthMvcEndpoint

We need to be a bit cautious about whether Spring Security is on
the classpath or not, but if it is we can test for the admin role
(as specified in `management.security.role`).

Fixes gh-4060

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
+		if (principal == null || principal.getClass().getName().contains("Anonymous")) {
-				&& !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() {

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.endpoint.FlywayEndpointTests;
-import org.springframework.boot.actuate.autoconfigure.PublicMetricsAutoConfigurationTests;
+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,
+@SuiteClasses({ DataSourceHealthIndicatorTests.class, FlywayEndpointTests.class })
-		MetricRepositoryAutoConfigurationTests.class })
 @Ignore
 public class AdhocTestSuite {
 

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(
+		given(this.endpoint.invoke())
-				new Health.Builder().status("OK").build());
+				.willReturn(new Health.Builder().status("OK").build());
-		this.mvc.setStatusMapping(Collections.singletonMap("OK",
+		this.mvc.setStatusMapping(
-				HttpStatus.INTERNAL_SERVER_ERROR));
+				Collections.singletonMap("OK", HttpStatus.INTERNAL_SERVER_ERROR));
 		Object result = this.mvc.invoke(null);
 		assertTrue(result instanceof ResponseEntity);
 		ResponseEntity<Health> response = (ResponseEntity<Health>) result;
@@ -108,8 +112,8 @@ public class HealthMvcEndpointTests {
 	@Test
 	@SuppressWarnings("unchecked")
 	public void customMappingWithRelaxedName() {
-		given(this.endpoint.invoke()).willReturn(
+		given(this.endpoint.invoke())
-				new Health.Builder().outOfService().build());
+				.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() {
+	public void secureEvenWhenNotSensitive() {
-		given(this.endpoint.invoke()).willReturn(
+		given(this.endpoint.invoke())
-				new Health.Builder().up().withDetail("foo", "bar").build());
+				.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(
+		given(this.endpoint.invoke())
-				new Health.Builder().up().withDetail("foo", "bar").build());
+				.willReturn(new Health.Builder().up().withDetail("foo", "bar").build());
-		Object result = this.mvc.invoke(this.user);
+		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(
+		given(this.endpoint.invoke())
-				new Health.Builder().up().withDetail("foo", "bar").build());
+				.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(
+		given(this.endpoint.invoke())
-				new Health.Builder().up().withDetail("foo", "bar").build());
+				.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(
+		given(this.endpoint.invoke())
-				new Health.Builder().up().withDetail("foo", "bar").build());
+				.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(
+		given(this.endpoint.invoke())
-				new Health.Builder().up().withDetail("foo", "bar").build());
+				.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(
+		given(this.endpoint.invoke())
-				new Health.Builder().up().withDetail("foo", "bar").build());
+				.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);

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(
+		mockMvc.perform(get("/beans"))
-				content().string(startsWith("{" + LINE_SEPARATOR)));
+				.andExpect(content().string(startsWith("{" + LINE_SEPARATOR)));
 	}
 
 	private MockMvc createMockMvc() {
@@ -205,8 +205,8 @@ public class MvcEndpointIntegrationTests {
 	}
 
 	@ImportAutoConfiguration({ JacksonAutoConfiguration.class,
-			HttpMessageConvertersAutoConfiguration.class,
+			HttpMessageConvertersAutoConfiguration.class, EndpointAutoConfiguration.class,
-			EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.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,
+			HttpMessageConvertersAutoConfiguration.class, EndpointAutoConfiguration.class,
-			EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class,
+			EndpointWebMvcAutoConfiguration.class,
 			ManagementServerPropertiesAutoConfiguration.class,
 			PropertyPlaceholderAutoConfiguration.class, WebMvcAutoConfiguration.class })
 	static class SpringDataRestConfiguration {