diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java index be47210ff39..f6f17910740 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.cloudfoundry; +import java.security.Principal; + import javax.servlet.http.HttpServletRequest; import org.springframework.boot.actuate.endpoint.HealthEndpoint; @@ -36,7 +38,7 @@ class CloudFoundryHealthMvcEndpoint extends HealthMvcEndpoint { } @Override - protected boolean exposeHealthDetails(HttpServletRequest request) { + protected boolean exposeHealthDetails(HttpServletRequest request, Principal principal) { return true; } 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 053a5832300..8299a33c9c5 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,6 +16,7 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.security.Principal; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -33,7 +34,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.util.StringUtils; import org.springframework.web.bind.annotation.ResponseBody; @@ -133,12 +137,12 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter(health, status); @@ -160,13 +164,13 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter= getDelegate().getTimeToLive(); } - protected boolean exposeHealthDetails(HttpServletRequest request) { + protected boolean exposeHealthDetails(HttpServletRequest request, Principal principal) { if (!this.secure) { return true; } - for (String role : getRoles()) { - if (request.isUserInRole(role) || request.isUserInRole("ROLE_" + role)) { + List roles = getRoles(); + for (String role : roles) { + if (request.isUserInRole(role)) { return true; } + if (isSpringSecurityAuthentication(principal)) { + Authentication authentication = (Authentication) principal; + for (GrantedAuthority authority : authentication.getAuthorities()) { + String name = authority.getAuthority(); + if (role.equals(name)) { + return true; + } + } + } } return false; } @@ -201,4 +215,9 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter SECURITY_ROLES = new MapPropertySource("test", Collections.singletonMap("management.security.roles", - "HERO, USER")); + "HERO")); private HttpServletRequest request = new MockHttpServletRequest(); @@ -62,13 +66,11 @@ public class HealthMvcEndpointTests { private MockEnvironment environment; - private HttpServletRequest user = createAuthenticationToken("ROLE_USER"); + private HttpServletRequest defaultUser = createAuthenticationRequest("ROLE_ACTUATOR"); - private HttpServletRequest actuator = createAuthenticationToken("ROLE_ACTUATOR"); + private HttpServletRequest hero = createAuthenticationRequest("HERO"); - private HttpServletRequest hero = createAuthenticationToken("ROLE_HERO"); - - private HttpServletRequest createAuthenticationToken(String role) { + private HttpServletRequest createAuthenticationRequest(String role) { MockServletContext servletContext = new MockServletContext(); servletContext.declareRoles(role); return new MockHttpServletRequest(servletContext); @@ -86,7 +88,7 @@ public class HealthMvcEndpointTests { @Test public void up() { given(this.endpoint.invoke()).willReturn(new Health.Builder().up().build()); - Object result = this.mvc.invoke(this.request); + Object result = this.mvc.invoke(this.request, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); } @@ -95,7 +97,7 @@ public class HealthMvcEndpointTests { @Test public void down() { given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); - Object result = this.mvc.invoke(this.request); + Object result = this.mvc.invoke(this.request, null); assertThat(result instanceof ResponseEntity).isTrue(); ResponseEntity response = (ResponseEntity) result; assertThat(response.getBody().getStatus() == Status.DOWN).isTrue(); @@ -109,7 +111,7 @@ public class HealthMvcEndpointTests { .willReturn(new Health.Builder().status("OK").build()); this.mvc.setStatusMapping( Collections.singletonMap("OK", HttpStatus.INTERNAL_SERVER_ERROR)); - Object result = this.mvc.invoke(this.request); + Object result = this.mvc.invoke(this.request, null); assertThat(result instanceof ResponseEntity).isTrue(); ResponseEntity response = (ResponseEntity) result; assertThat(response.getBody().getStatus().equals(new Status("OK"))).isTrue(); @@ -123,7 +125,7 @@ public class HealthMvcEndpointTests { .willReturn(new Health.Builder().outOfService().build()); this.mvc.setStatusMapping(Collections.singletonMap("out-of-service", HttpStatus.INTERNAL_SERVER_ERROR)); - Object result = this.mvc.invoke(this.request); + Object result = this.mvc.invoke(this.request, null); assertThat(result instanceof ResponseEntity).isTrue(); ResponseEntity response = (ResponseEntity) result; assertThat(response.getBody().getStatus().equals(Status.OUT_OF_SERVICE)).isTrue(); @@ -134,7 +136,7 @@ public class HealthMvcEndpointTests { public void presenceOfRightRoleShouldExposeDetails() { given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.actuator); + Object result = this.mvc.invoke(this.defaultUser, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); @@ -145,7 +147,7 @@ public class HealthMvcEndpointTests { this.mvc = new HealthMvcEndpoint(this.endpoint, false); given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.user); + Object result = this.mvc.invoke(this.defaultUser, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); @@ -155,18 +157,32 @@ public class HealthMvcEndpointTests { public void rightRoleNotPresentShouldNotExposeDetails() { given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.user); + Object result = this.mvc.invoke(this.hero, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); assertThat(((Health) result).getDetails().get("foo")).isNull(); } + @Test + public void rightAuthorityPresentShouldExposeDetails() throws Exception { + this.environment.getPropertySources().addLast(SECURITY_ROLES); + Authentication principal = mock(Authentication.class); + Set authorities = Collections.singleton(new SimpleGrantedAuthority("HERO")); + doReturn(authorities).when(principal).getAuthorities(); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(this.defaultUser, principal); + assertThat(result instanceof Health).isTrue(); + assertThat(((Health) result).getStatus() == Status.UP).isTrue(); + assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); + } + @Test public void customRolePresentShouldExposeDetails() { 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); + Object result = this.mvc.invoke(this.hero, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); @@ -177,12 +193,25 @@ public class HealthMvcEndpointTests { 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.actuator); + Object result = this.mvc.invoke(this.defaultUser, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); assertThat(((Health) result).getDetails().get("foo")).isNull(); } + @Test + public void customRoleFromListShouldExposeDetails() { + // gh-8314 + this.mvc = new HealthMvcEndpoint(this.endpoint, true, + Arrays.asList("HERO", "USER")); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(this.hero, null); + assertThat(result instanceof Health).isTrue(); + assertThat(((Health) result).getStatus() == Status.UP).isTrue(); + assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); + } + @Test public void customRoleFromListShouldNotExposeDetailsForDefaultRole() { // gh-8314 @@ -190,10 +219,10 @@ public class HealthMvcEndpointTests { Arrays.asList("HERO", "USER")); given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.hero); + Object result = this.mvc.invoke(this.defaultUser, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); - assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); + assertThat(((Health) result).getDetails().get("foo")).isNull(); } @Test @@ -201,14 +230,14 @@ public class HealthMvcEndpointTests { given(this.endpoint.getTimeToLive()).willReturn(10000L); given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.actuator); + Object result = this.mvc.invoke(this.defaultUser, null); assertThat(result instanceof Health).isTrue(); Health health = (Health) result; assertThat(health.getStatus() == Status.UP).isTrue(); assertThat(health.getDetails()).hasSize(1); assertThat(health.getDetails().get("foo")).isEqualTo("bar"); given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); - result = this.mvc.invoke(this.request); // insecure now + result = this.mvc.invoke(this.request, null); // insecure now assertThat(result instanceof Health).isTrue(); health = (Health) result; // so the result is cached @@ -222,11 +251,11 @@ public class HealthMvcEndpointTests { given(this.endpoint.getTimeToLive()).willReturn(0L); given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.request); + Object result = this.mvc.invoke(this.request, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); - result = this.mvc.invoke(this.request); + result = this.mvc.invoke(this.request, null); @SuppressWarnings("unchecked") Health health = ((ResponseEntity) result).getBody(); assertThat(health.getStatus() == Status.DOWN).isTrue(); @@ -237,12 +266,12 @@ public class HealthMvcEndpointTests { given(this.endpoint.getTimeToLive()).willReturn(50L); given(this.endpoint.invoke()) .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); - Object result = this.mvc.invoke(this.request); + Object result = this.mvc.invoke(this.request, null); assertThat(result instanceof Health).isTrue(); assertThat(((Health) result).getStatus() == Status.UP).isTrue(); Thread.sleep(100); given(this.endpoint.invoke()).willReturn(new Health.Builder().down().build()); - result = this.mvc.invoke(this.request); + result = this.mvc.invoke(this.request, null); @SuppressWarnings("unchecked") Health health = ((ResponseEntity) result).getBody(); assertThat(health.getStatus() == Status.DOWN).isTrue(); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityHealthMvcEndpointIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityHealthMvcEndpointIntegrationTests.java index c77accdb353..d56a754cb4b 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityHealthMvcEndpointIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityHealthMvcEndpointIntegrationTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.security.Principal; + import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; @@ -35,12 +37,15 @@ import org.springframework.boot.junit.runner.classpath.ModifiedClassPathRunner; import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockServletContext; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.RequestPostProcessor; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import static org.hamcrest.CoreMatchers.containsString; +import static org.mockito.Mockito.mock; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -63,13 +68,13 @@ public class NoSpringSecurityHealthMvcEndpointIntegrationTests { } @Test - public void healthDetailNotPresent() throws Exception { + public void healthWhenRightRoleNotPresentShouldExposeHealthDetails() throws Exception { this.context = new AnnotationConfigWebApplicationContext(); this.context.setServletContext(new MockServletContext()); this.context.register(TestConfiguration.class); this.context.refresh(); MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context).build(); - mockMvc.perform(get("/health")).andExpect(status().isOk()) + mockMvc.perform(get("/health").with(getRequestPostProcessor())).andExpect(status().isOk()) .andExpect(content().string(containsString("\"status\":\"UP\""))); } @@ -87,6 +92,17 @@ public class NoSpringSecurityHealthMvcEndpointIntegrationTests { "\"status\":\"UP\",\"test\":{\"status\":\"UP\",\"hello\":\"world\"}"))); } + private RequestPostProcessor getRequestPostProcessor() { + return new RequestPostProcessor() { + @Override + public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) { + Principal principal = mock(Principal.class); + request.setUserPrincipal(principal); + return request; + } + }; + } + @ImportAutoConfiguration({ JacksonAutoConfiguration.class, HttpMessageConvertersAutoConfiguration.class, EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class,