Commit dced154f authored by Eddú Meléndez's avatar Eddú Meléndez Committed by Stephane Nicoll

Fix health endpoint security

Commit b02aba4c has renamed `management.security.role` to
`management.security.roles`. Unfortunately, the `HealthMvcEndpoint`
was still looking at the old property.

This commit makes sure that the proper key is used and any custom
role is applied rather than an unconditional `ADMIN` role.

See gh-6540
parent 42291659
...@@ -17,7 +17,10 @@ ...@@ -17,7 +17,10 @@
package org.springframework.boot.actuate.endpoint.mvc; package org.springframework.boot.actuate.endpoint.mvc;
import java.security.Principal; import java.security.Principal;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.List;
import java.util.Map; import java.util.Map;
import org.springframework.boot.actuate.endpoint.HealthEndpoint; import org.springframework.boot.actuate.endpoint.HealthEndpoint;
...@@ -35,6 +38,7 @@ import org.springframework.security.core.Authentication; ...@@ -35,6 +38,7 @@ import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.GrantedAuthority;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.ClassUtils; import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.ResponseBody;
...@@ -45,6 +49,7 @@ import org.springframework.web.bind.annotation.ResponseBody; ...@@ -45,6 +49,7 @@ import org.springframework.web.bind.annotation.ResponseBody;
* @author Dave Syer * @author Dave Syer
* @author Andy Wilkinson * @author Andy Wilkinson
* @author Phillip Webb * @author Phillip Webb
* @author Eddú Meléndez
* @since 1.1.0 * @since 1.1.0
*/ */
@ConfigurationProperties(prefix = "endpoints.health") @ConfigurationProperties(prefix = "endpoints.health")
...@@ -184,11 +189,17 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter<HealthEndpoint ...@@ -184,11 +189,17 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter<HealthEndpoint
} }
if (isSpringSecurityAuthentication(principal)) { if (isSpringSecurityAuthentication(principal)) {
Authentication authentication = (Authentication) 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"))));
if (roles.isEmpty()) {
roles = Collections.singletonList("ROLE_ADMIN");
}
for (GrantedAuthority authority : authentication.getAuthorities()) { for (GrantedAuthority authority : authentication.getAuthorities()) {
String name = authority.getAuthority(); String name = authority.getAuthority();
if (role.equals(name) || ("ROLE_" + role).equals(name)) { for (String role : roles) {
return true; if (role.equals(name) || ("ROLE_" + role).equals(name)) {
return true;
}
} }
} }
} }
......
...@@ -42,6 +42,7 @@ import static org.mockito.Mockito.mock; ...@@ -42,6 +42,7 @@ import static org.mockito.Mockito.mock;
* @author Christian Dupuis * @author Christian Dupuis
* @author Dave Syer * @author Dave Syer
* @author Andy Wilkinson * @author Andy Wilkinson
* @author Eddú Meléndez
*/ */
public class HealthMvcEndpointTests { public class HealthMvcEndpointTests {
...@@ -49,6 +50,10 @@ public class HealthMvcEndpointTests { ...@@ -49,6 +50,10 @@ public class HealthMvcEndpointTests {
Collections.<String, Object>singletonMap("endpoints.health.sensitive", Collections.<String, Object>singletonMap("endpoints.health.sensitive",
"false")); "false"));
private static final PropertySource<?> SECURITY_ROLES = new MapPropertySource("test",
Collections.<String, Object>singletonMap("management.security.roles",
"HERO, USER"));
private HealthEndpoint endpoint = null; private HealthEndpoint endpoint = null;
private HealthMvcEndpoint mvc = null; private HealthMvcEndpoint mvc = null;
...@@ -140,6 +145,19 @@ public class HealthMvcEndpointTests { ...@@ -140,6 +145,19 @@ public class HealthMvcEndpointTests {
assertThat(((Health) result).getDetails().get("foo")).isNull(); assertThat(((Health) result).getDetails().get("foo")).isNull();
} }
@Test
public void secureCustomRole() {
this.mvc = new HealthMvcEndpoint(this.endpoint, false);
this.mvc.setEnvironment(this.environment);
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.user);
assertThat(result instanceof Health).isTrue();
assertThat(((Health) result).getStatus() == Status.UP).isTrue();
assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar");
}
@Test @Test
public void healthIsCached() { public void healthIsCached() {
given(this.endpoint.getTimeToLive()).willReturn(10000L); given(this.endpoint.getTimeToLive()).willReturn(10000L);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment