From a3bcb2778fa56ea419cea698629cedc71508ecbc Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 8 Nov 2016 15:01:13 -0800 Subject: [PATCH 1/2] Add message to response body for Cloud Foundry security error See gh-7108 --- .../actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java | 4 ++++ .../cloudfoundry/CloudFoundrySecurityInterceptorTests.java | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java index cc3f0c5b3b..ab13a0e148 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java @@ -24,6 +24,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.boot.actuate.cloudfoundry.CloudFoundryAuthorizationException.Reason; import org.springframework.boot.actuate.endpoint.mvc.MvcEndpoint; +import org.springframework.http.MediaType; import org.springframework.util.StringUtils; import org.springframework.web.cors.CorsUtils; import org.springframework.web.method.HandlerMethod; @@ -74,6 +75,9 @@ class CloudFoundrySecurityInterceptor extends HandlerInterceptorAdapter { } catch (CloudFoundryAuthorizationException ex) { this.logger.error(ex); + response.setContentType(MediaType.APPLICATION_JSON.toString()); + response.getWriter() + .write("{\"security_error\":\"" + ex.getMessage() + "\"}"); response.setStatus(ex.getStatusCode().value()); return false; } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptorTests.java index a8e8ca51b2..7a338d210d 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptorTests.java @@ -28,6 +28,7 @@ import org.springframework.boot.actuate.endpoint.AbstractEndpoint; import org.springframework.boot.actuate.endpoint.mvc.EndpointMvcAdapter; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.util.Base64Utils; @@ -87,6 +88,9 @@ public class CloudFoundrySecurityInterceptorTests { assertThat(preHandle).isFalse(); assertThat(this.response.getStatus()) .isEqualTo(Reason.MISSING_AUTHORIZATION.getStatus().value()); + assertThat(this.response.getContentAsString()).contains("security_error"); + assertThat(this.response.getContentType()) + .isEqualTo(MediaType.APPLICATION_JSON.toString()); } @Test From af612782131fffd64d5796329bf816a7cb59c964 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 8 Nov 2016 15:04:35 -0800 Subject: [PATCH 2/2] Extend HealthMvcEndpoint for Cloud Foundry The CloudFoundryHealthMvcEndpoint does not perform additional security checks since security is handled by the interceptor. See gh-7108 --- .../actuate/cloudfoundry/AccessLevel.java | 2 +- .../CloudFoundryEndpointHandlerMapping.java | 13 ++++- .../CloudFoundryHealthMvcEndpoint.java | 43 ++++++++++++++++ .../endpoint/mvc/HealthMvcEndpoint.java | 2 +- ...oudFoundryEndpointHandlerMappingTests.java | 29 +++++++++++ .../CloudFoundryHealthMvcEndpointTests.java | 51 +++++++++++++++++++ 6 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java create mode 100644 spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java index fe556f2a17..3bd7e7e4a2 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java @@ -49,7 +49,7 @@ enum AccessLevel { /** * Returns if the access level should allow access to the specified endpoint path. - * @param endpointPath the endpoitn path + * @param endpointPath the endpoint path * @return {@code true} if access is allowed */ public boolean isAccessAllowed(String endpointPath) { diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java index 3f10756e9c..6a9555f53d 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java @@ -27,6 +27,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.mvc.AbstractEndpointHandlerMapping; import org.springframework.boot.actuate.endpoint.mvc.HalJsonMvcEndpoint; +import org.springframework.boot.actuate.endpoint.mvc.HealthMvcEndpoint; import org.springframework.boot.actuate.endpoint.mvc.MvcEndpoint; import org.springframework.boot.actuate.endpoint.mvc.NamedMvcEndpoint; import org.springframework.web.cors.CorsConfiguration; @@ -54,10 +55,20 @@ class CloudFoundryEndpointHandlerMapping protected void postProcessEndpoints(Set endpoints) { super.postProcessEndpoints(endpoints); Iterator iterator = endpoints.iterator(); + HealthMvcEndpoint healthMvcEndpoint = null; while (iterator.hasNext()) { - if (iterator.next() instanceof HalJsonMvcEndpoint) { + NamedMvcEndpoint endpoint = iterator.next(); + if (endpoint instanceof HalJsonMvcEndpoint) { iterator.remove(); } + else if (endpoint instanceof HealthMvcEndpoint) { + iterator.remove(); + healthMvcEndpoint = (HealthMvcEndpoint) endpoint; + } + } + if (healthMvcEndpoint != null) { + endpoints.add( + new CloudFoundryHealthMvcEndpoint(healthMvcEndpoint.getDelegate())); } } 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 new file mode 100644 index 0000000000..7d25eb2234 --- /dev/null +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java @@ -0,0 +1,43 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.cloudfoundry; + +import java.security.Principal; + +import org.springframework.boot.actuate.endpoint.HealthEndpoint; +import org.springframework.boot.actuate.endpoint.mvc.HealthMvcEndpoint; + +/** + * Extension of {@link HealthMvcEndpoint} for Cloud Foundry. Since security for Cloud + * Foundry actuators is already handled by the {@link CloudFoundrySecurityInterceptor}, + * this endpoint skips the additional security checks done by the regular + * {@link HealthMvcEndpoint}. + * + * @author Madhura Bhave + */ +class CloudFoundryHealthMvcEndpoint extends HealthMvcEndpoint { + + CloudFoundryHealthMvcEndpoint(HealthEndpoint delegate) { + super(delegate); + } + + @Override + protected boolean exposeHealthDetails(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 02e2e51032..374ac3d577 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 @@ -178,7 +178,7 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter= getDelegate().getTimeToLive(); } - private boolean exposeHealthDetails(Principal principal) { + protected boolean exposeHealthDetails(Principal principal) { return isSecure(principal) || isUnrestricted(); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java index 613a59b60b..a83559390e 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java @@ -22,11 +22,15 @@ import org.junit.Test; import org.mockito.Mockito; import org.springframework.boot.actuate.endpoint.AbstractEndpoint; +import org.springframework.boot.actuate.endpoint.HealthEndpoint; import org.springframework.boot.actuate.endpoint.mvc.AbstractEndpointHandlerMappingTests; import org.springframework.boot.actuate.endpoint.mvc.EndpointMvcAdapter; import org.springframework.boot.actuate.endpoint.mvc.HalJsonMvcEndpoint; +import org.springframework.boot.actuate.endpoint.mvc.HealthMvcEndpoint; import org.springframework.boot.actuate.endpoint.mvc.ManagementServletContext; import org.springframework.boot.actuate.endpoint.mvc.NamedMvcEndpoint; +import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.boot.actuate.health.OrderedHealthAggregator; import org.springframework.context.support.StaticApplicationContext; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.method.HandlerMethod; @@ -88,6 +92,23 @@ public class CloudFoundryEndpointHandlerMappingTests .isInstanceOf(CloudFoundryDiscoveryMvcEndpoint.class); } + @Test + public void registersCloudFoundryHealthEndpoint() throws Exception { + StaticApplicationContext context = new StaticApplicationContext(); + HealthEndpoint delegate = new HealthEndpoint(new OrderedHealthAggregator(), + Collections.emptyMap()); + CloudFoundryEndpointHandlerMapping handlerMapping = new CloudFoundryEndpointHandlerMapping( + Collections.singleton(new TestHealthMvcEndpoint(delegate)), null, null); + handlerMapping.setPrefix("/test"); + handlerMapping.setApplicationContext(context); + handlerMapping.afterPropertiesSet(); + HandlerExecutionChain handler = handlerMapping + .getHandler(new MockHttpServletRequest("GET", "/test/health")); + HandlerMethod handlerMethod = (HandlerMethod) handler.getHandler(); + Object handlerMethodBean = handlerMethod.getBean(); + assertThat(handlerMethodBean).isInstanceOf(CloudFoundryHealthMvcEndpoint.class); + } + private static class TestEndpoint extends AbstractEndpoint { TestEndpoint(String id) { @@ -124,4 +145,12 @@ public class CloudFoundryEndpointHandlerMappingTests } + private static class TestHealthMvcEndpoint extends HealthMvcEndpoint { + + TestHealthMvcEndpoint(HealthEndpoint delegate) { + super(delegate); + } + + } + } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java new file mode 100644 index 0000000000..d885cd706a --- /dev/null +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java @@ -0,0 +1,51 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.cloudfoundry; + +import org.junit.Test; + +import org.springframework.boot.actuate.endpoint.HealthEndpoint; +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.Status; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link CloudFoundryHealthMvcEndpoint}. + * + * @author Madhura Bhave + */ +public class CloudFoundryHealthMvcEndpointTests { + + @Test + public void cloudFoundryHealthEndpointShouldAlwaysReturnAllHealthDetails() + throws Exception { + HealthEndpoint endpoint = mock(HealthEndpoint.class); + given(endpoint.isEnabled()).willReturn(true); + CloudFoundryHealthMvcEndpoint mvc = new CloudFoundryHealthMvcEndpoint(endpoint); + given(endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + given(endpoint.isSensitive()).willReturn(false); + Object result = mvc.invoke(null); + assertThat(result instanceof Health).isTrue(); + assertThat(((Health) result).getStatus() == Status.UP).isTrue(); + assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); + } + +}