Merge pull request #6540 from eddumelendez:health-endpoint-visibility

* pr/6540:
  Polish contribution
  Fix health endpoint security

Author: snicoll

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java

@@ -17,7 +17,9 @@
 package org.springframework.boot.actuate.endpoint.mvc;
 
 import java.security.Principal;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.springframework.boot.actuate.endpoint.HealthEndpoint;
@@ -35,6 +37,7 @@
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
 
@@ -45,6 +48,7 @@
  * @author Dave Syer
  * @author Andy Wilkinson
  * @author Phillip Webb
+ * @author Eddú Meléndez
  * @since 1.1.0
  */
 @ConfigurationProperties(prefix = "endpoints.health")
@@ -184,11 +188,14 @@ private boolean isSecure(Principal principal) {
 		}
 		if (isSpringSecurityAuthentication(principal)) {
 			Authentication authentication = (Authentication) principal;
-			String role = this.roleResolver.getProperty("role", "ROLE_ADMIN");
+			List<String> roles = Arrays.asList(StringUtils.trimArrayElements(StringUtils
+					.commaDelimitedListToStringArray(this.roleResolver.getProperty("roles", "ROLE_ADMIN"))));
 			for (GrantedAuthority authority : authentication.getAuthorities()) {
 				String name = authority.getAuthority();
-				if (role.equals(name) || ("ROLE_" + role).equals(name)) {
-					return true;
+				for (String role : roles) {
+					if (role.equals(name) || ("ROLE_" + role).equals(name)) {
+						return true;
+					}
 				}
 			}
 		}

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java

@@ -42,26 +42,35 @@
  * @author Christian Dupuis
  * @author Dave Syer
  * @author Andy Wilkinson
+ * @author Eddú Meléndez
  */
 public class HealthMvcEndpointTests {
 
 	private static final PropertySource<?> NON_SENSITIVE = new MapPropertySource("test",
 			Collections.<String, Object>singletonMap("endpoints.health.sensitive",
 					"false"));
 
+	private static final PropertySource<?> SECURITY_ROLES = new MapPropertySource("test",
+			Collections.<String, Object>singletonMap("management.security.roles",
+					"HERO, USER"));
+
 	private HealthEndpoint endpoint = null;
 
 	private HealthMvcEndpoint mvc = null;
 
 	private MockEnvironment environment;
 
-	private UsernamePasswordAuthenticationToken user = new UsernamePasswordAuthenticationToken(
-			"user", "password",
-			AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_USER"));
+	private UsernamePasswordAuthenticationToken user = createAuthenticationToken("ROLE_USER");
+
+	private UsernamePasswordAuthenticationToken admin = createAuthenticationToken("ROLE_ADMIN");
+
+	private UsernamePasswordAuthenticationToken hero = createAuthenticationToken("ROLE_HERO");
 
-	private UsernamePasswordAuthenticationToken admin = new UsernamePasswordAuthenticationToken(
-			"user", "password",
-			AuthorityUtils.commaSeparatedStringToAuthorityList("ROLE_ADMIN"));
+	private UsernamePasswordAuthenticationToken createAuthenticationToken(String authority) {
+		return new UsernamePasswordAuthenticationToken(
+				"user", "password",
+				AuthorityUtils.commaSeparatedStringToAuthorityList(authority));
+	}
 
 	@Before
 	public void init() {
@@ -140,6 +149,28 @@ public void secureNonAdmin() {
 		assertThat(((Health) result).getDetails().get("foo")).isNull();
 	}
 
+	@Test
+	public void secureCustomRole() {
+		this.environment.getPropertySources().addLast(SECURITY_ROLES);
+		given(this.endpoint.invoke())
+				.willReturn(new Health.Builder().up().withDetail("foo", "bar").build());
+		Object result = this.mvc.invoke(this.hero);
+		assertThat(result instanceof Health).isTrue();
+		assertThat(((Health) result).getStatus() == Status.UP).isTrue();
+		assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar");
+	}
+
+	@Test
+	public void secureCustomRoleNoAccess() {
+		this.environment.getPropertySources().addLast(SECURITY_ROLES);
+		given(this.endpoint.invoke())
+				.willReturn(new Health.Builder().up().withDetail("foo", "bar").build());
+		Object result = this.mvc.invoke(this.admin);
+		assertThat(result instanceof Health).isTrue();
+		assertThat(((Health) result).getStatus() == Status.UP).isTrue();
+		assertThat(((Health) result).getDetails().get("foo")).isNull();
+	}
+
 	@Test
 	public void healthIsCached() {
 		given(this.endpoint.getTimeToLive()).willReturn(10000L);