diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java index 08d4dba5c0..e9c6d3b785 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java @@ -17,11 +17,16 @@ package org.springframework.boot.actuate.endpoint.mvc; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.http.HttpStatus; +import org.springframework.util.StringUtils; import org.springframework.web.cors.CorsUtils; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; @@ -34,10 +39,15 @@ import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; */ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter { + private static final Log logger = LogFactory + .getLog(MvcEndpointSecurityInterceptor.class); + private final boolean secure; private final List roles; + private AtomicBoolean loggedUnauthorizedAttempt = new AtomicBoolean(); + public MvcEndpointSecurityInterceptor(boolean secure, List roles) { this.secure = secure; this.roles = roles; @@ -59,17 +69,30 @@ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter { return true; } } - setFailureResponseStatus(request, response); + sendFailureResponse(request, response); return false; } - private void setFailureResponseStatus(HttpServletRequest request, - HttpServletResponse response) { + private void sendFailureResponse(HttpServletRequest request, + HttpServletResponse response) throws Exception { if (request.getUserPrincipal() != null) { - response.setStatus(HttpStatus.FORBIDDEN.value()); + String roles = StringUtils.collectionToDelimitedString(this.roles, " "); + response.sendError(HttpStatus.FORBIDDEN.value(), + "Access is denied. User must have one of the these roles: " + roles); } else { - response.setStatus(HttpStatus.UNAUTHORIZED.value()); + logUnauthorizedAttempt(); + response.sendError(HttpStatus.UNAUTHORIZED.value(), + "Full authentication is required to access this resource."); + } + } + + private void logUnauthorizedAttempt() { + if (this.loggedUnauthorizedAttempt.compareAndSet(false, true) + && logger.isInfoEnabled()) { + logger.info("Full authentication is required to access " + + "actuator endpoints. Consider adding Spring Security " + + "or set 'management.security.enabled' to false."); } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java index 8586bdd7f7..cbfa698133 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java @@ -16,19 +16,26 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.security.Principal; import java.util.Arrays; import java.util.List; +import javax.servlet.http.HttpServletResponse; + import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.springframework.boot.actuate.endpoint.AbstractEndpoint; +import org.springframework.boot.test.rule.OutputCapture; +import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockServletContext; import org.springframework.web.method.HandlerMethod; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Tests for {@link MvcEndpointSecurityInterceptor}. @@ -37,6 +44,9 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class MvcEndpointSecurityInterceptorTests { + @Rule + public OutputCapture output = new OutputCapture(); + private MvcEndpointSecurityInterceptor securityInterceptor; private TestMvcEndpoint mvcEndpoint; @@ -47,7 +57,7 @@ public class MvcEndpointSecurityInterceptorTests { private MockHttpServletRequest request; - private MockHttpServletResponse response; + private HttpServletResponse response; private MockServletContext servletContext; @@ -62,7 +72,7 @@ public class MvcEndpointSecurityInterceptorTests { this.handlerMethod = new HandlerMethod(this.mvcEndpoint, "invoke"); this.servletContext = new MockServletContext(); this.request = new MockHttpServletRequest(this.servletContext); - this.response = new MockHttpServletResponse(); + this.response = mock(HttpServletResponse.class); } @Test @@ -87,11 +97,30 @@ public class MvcEndpointSecurityInterceptorTests { } @Test - public void sensitiveEndpointIfRoleIsNotPresentShouldNotAllowAccess() + public void sensitiveEndpointIfNotAuthenticatedShouldNotAllowAccess() + throws Exception { + assertThat(this.securityInterceptor.preHandle(this.request, this.response, + this.handlerMethod)).isFalse(); + verify(this.response).sendError(HttpStatus.UNAUTHORIZED.value(), + "Full authentication is required to access this resource."); + assertThat(this.securityInterceptor.preHandle(this.request, this.response, + this.handlerMethod)).isFalse(); + assertThat(this.output.toString()) + .containsOnlyOnce("Full authentication is required to access actuator " + + "endpoints. Consider adding Spring Security or set " + + "'management.security.enabled' to false"); + } + + @Test + public void sensitiveEndpointIfRoleIsNotCorrectShouldNotAllowAccess() throws Exception { + Principal principal = mock(Principal.class); + this.request.setUserPrincipal(principal); this.servletContext.declareRoles("HERO"); assertThat(this.securityInterceptor.preHandle(this.request, this.response, this.handlerMethod)).isFalse(); + verify(this.response).sendError(HttpStatus.FORBIDDEN.value(), + "Access is denied. User must have one of the these roles: SUPER_HERO"); } private static class TestEndpoint extends AbstractEndpoint {