From 38eeae216604cca7bca730847a8bf45fe85392e8 Mon Sep 17 00:00:00 2001
From: Madhura Bhave <mbhave@pivotal.io>
Date: Tue, 13 Dec 2016 00:27:40 -0800
Subject: [PATCH] Send error with message from Endpoint MVC security

Update `MvcEndpointSecurityInterceptor` to that it sends an error in the
same way as Spring Security. Prior to this commit the `ErrorController`
would not handle endpoint security errors.

Fixes gh-7605
Closes gh-7634
---
 .../mvc/MvcEndpointSecurityInterceptor.java   | 18 +++++++++----
 .../MvcEndpointSecurityInterceptorTests.java  | 27 ++++++++++++++++---
 2 files changed, 36 insertions(+), 9 deletions(-)

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..7800dd5e0c 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
@@ -59,17 +59,25 @@ 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());
+			StringBuilder message = new StringBuilder();
+			for (String role : this.roles) {
+				message.append(role).append(" ");
+			}
+			response.sendError(HttpStatus.FORBIDDEN.value(),
+					"Access is denied. User must have one of the these roles: "
+							+ message.toString().trim());
 		}
 		else {
-			response.setStatus(HttpStatus.UNAUTHORIZED.value());
+			response.sendError(HttpStatus.UNAUTHORIZED.value(),
+					"Full authentication is required to access this resource. "
+							+ "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..bc8f347763 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,24 @@
 
 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.Test;
 
 import org.springframework.boot.actuate.endpoint.AbstractEndpoint;
+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}.
@@ -47,7 +52,7 @@ public class MvcEndpointSecurityInterceptorTests {
 
 	private MockHttpServletRequest request;
 
-	private MockHttpServletResponse response;
+	private HttpServletResponse response;
 
 	private MockServletContext servletContext;
 
@@ -62,7 +67,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 +92,25 @@ 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. "
+						+ "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<Object> {