Refine error message from Endpoint MVC security

Update the error message to return less information to the client.
Details of how to disable security are now written to the log instead.

See gh-7605
See gh-7634
This commit is contained in:
Phillip Webb 2016-12-20 18:47:39 -08:00
parent 38eeae2166
commit c76bd2d81e
2 changed files with 35 additions and 10 deletions

View File

@ -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<String> roles;
private AtomicBoolean loggedUnauthorizedAttempt = new AtomicBoolean();
public MvcEndpointSecurityInterceptor(boolean secure, List<String> roles) {
this.secure = secure;
this.roles = roles;
@ -66,18 +76,23 @@ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter {
private void sendFailureResponse(HttpServletRequest request,
HttpServletResponse response) throws Exception {
if (request.getUserPrincipal() != null) {
StringBuilder message = new StringBuilder();
for (String role : this.roles) {
message.append(role).append(" ");
}
String roles = StringUtils.collectionToDelimitedString(this.roles, " ");
response.sendError(HttpStatus.FORBIDDEN.value(),
"Access is denied. User must have one of the these roles: "
+ message.toString().trim());
"Access is denied. User must have one of the these roles: " + roles);
}
else {
logUnauthorizedAttempt();
response.sendError(HttpStatus.UNAUTHORIZED.value(),
"Full authentication is required to access this resource. "
+ "Consider adding Spring Security or set management.security.enabled to false.");
"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.");
}
}

View File

@ -23,9 +23,11 @@ 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.MockServletContext;
@ -42,6 +44,9 @@ import static org.mockito.Mockito.verify;
*/
public class MvcEndpointSecurityInterceptorTests {
@Rule
public OutputCapture output = new OutputCapture();
private MvcEndpointSecurityInterceptor securityInterceptor;
private TestMvcEndpoint mvcEndpoint;
@ -97,8 +102,13 @@ public class MvcEndpointSecurityInterceptorTests {
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. "
+ "Consider adding Spring Security or set management.security.enabled to false.");
"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