Hone the security rules for actuator endpoints

Takes into account the fact that the new /actuator endpoint sometimes
loses its path (it is "" relative to a non-empty management context path).

Fixes gh-4059

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementWebSecurityAutoConfiguration.java

@@ -101,8 +101,8 @@ public class ManagementWebSecurityAutoConfiguration {
 	}
 
 	@Configuration
-	protected static class ManagementSecurityPropertiesConfiguration implements
-			SecurityPrerequisite {
+	protected static class ManagementSecurityPropertiesConfiguration
+			implements SecurityPrerequisite {
 
 		@Autowired(required = false)
 		private SecurityProperties security;
@@ -122,8 +122,8 @@ public class ManagementWebSecurityAutoConfiguration {
 
 	// Get the ignored paths in early
 	@Order(SecurityProperties.IGNORED_ORDER + 1)
-	private static class IgnoredPathsWebSecurityConfigurerAdapter implements
-			WebSecurityConfigurer<WebSecurity> {
+	private static class IgnoredPathsWebSecurityConfigurerAdapter
+			implements WebSecurityConfigurer<WebSecurity> {
 
 		@Autowired(required = false)
 		private ErrorController errorController;
@@ -152,8 +152,8 @@ public class ManagementWebSecurityAutoConfiguration {
 			List<String> ignored = SpringBootWebSecurityConfiguration
 					.getIgnored(this.security);
 			if (!this.management.getSecurity().isEnabled()) {
-				ignored.addAll(Arrays.asList(EndpointPaths
-						.get(this.endpointHandlerMapping)));
+				ignored.addAll(
+						Arrays.asList(EndpointPaths.get(this.endpointHandlerMapping)));
 			}
 			if (ignored.contains("none")) {
 				ignored.remove("none");
@@ -192,12 +192,13 @@ public class ManagementWebSecurityAutoConfiguration {
 		@Override
 		public ConditionOutcome getMatchOutcome(ConditionContext context,
 				AnnotatedTypeMetadata metadata) {
-			String managementEnabled = context.getEnvironment().getProperty(
-					"management.security.enabled", "true");
-			String basicEnabled = context.getEnvironment().getProperty(
-					"security.basic.enabled", "true");
-			return new ConditionOutcome("true".equalsIgnoreCase(managementEnabled)
-					&& !"true".equalsIgnoreCase(basicEnabled),
+			String managementEnabled = context.getEnvironment()
+					.getProperty("management.security.enabled", "true");
+			String basicEnabled = context.getEnvironment()
+					.getProperty("security.basic.enabled", "true");
+			return new ConditionOutcome(
+					"true".equalsIgnoreCase(managementEnabled)
+							&& !"true".equalsIgnoreCase(basicEnabled),
 					"Management security enabled and basic disabled");
 		}
 
@@ -207,8 +208,8 @@ public class ManagementWebSecurityAutoConfiguration {
 	@ConditionalOnMissingBean({ ManagementWebSecurityConfigurerAdapter.class })
 	@ConditionalOnProperty(prefix = "management.security", name = "enabled", matchIfMissing = true)
 	@Order(ManagementServerProperties.BASIC_AUTH_ORDER)
-	protected static class ManagementWebSecurityConfigurerAdapter extends
-			WebSecurityConfigurerAdapter {
+	protected static class ManagementWebSecurityConfigurerAdapter
+			extends WebSecurityConfigurerAdapter {
 
 		@Autowired
 		private SecurityProperties security;
@@ -234,14 +235,14 @@ public class ManagementWebSecurityAutoConfiguration {
 			if (this.endpointHandlerMapping == null) {
 				ApplicationContext context = (this.contextResolver == null ? null
 						: this.contextResolver.getApplicationContext());
-				if (context != null
-						&& context.getBeanNamesForType(EndpointHandlerMapping.class).length > 0) {
+				if (context != null && context
+						.getBeanNamesForType(EndpointHandlerMapping.class).length > 0) {
 					this.endpointHandlerMapping = context
 							.getBean(EndpointHandlerMapping.class);
 				}
 				if (this.endpointHandlerMapping == null) {
 					this.endpointHandlerMapping = new EndpointHandlerMapping(
-							Collections.<MvcEndpoint>emptySet());
+							Collections.<MvcEndpoint> emptySet());
 				}
 			}
 		}
@@ -257,9 +258,10 @@ public class ManagementWebSecurityAutoConfiguration {
 				}
 				AuthenticationEntryPoint entryPoint = entryPoint();
 				http.exceptionHandling().authenticationEntryPoint(entryPoint);
+				// Match all the requests for actuator endpoints ...
 				http.requestMatcher(matcher);
-				configureAuthorizeRequests(new EndpointPathRequestMatcher(false),
-						http.authorizeRequests());
+				// ... but permitAll() for the non-sensitive ones
+				configurePermittedRequests(http.authorizeRequests());
 				http.httpBasic().authenticationEntryPoint(entryPoint);
 				// No cookies for management endpoints by default
 				http.csrf().disable();
@@ -280,7 +282,9 @@ public class ManagementWebSecurityAutoConfiguration {
 						this.server.getPath(path) + "/**");
 				return matcher;
 			}
-			return new EndpointPathRequestMatcher();
+			// Match everything, including the sensitive and non-sensitive paths
+			return new EndpointPathRequestMatcher(
+					EndpointPaths.get(this.endpointHandlerMapping));
 		}
 
 		private AuthenticationEntryPoint entryPoint() {
@@ -289,25 +293,23 @@ public class ManagementWebSecurityAutoConfiguration {
 			return entryPoint;
 		}
 
-		private void configureAuthorizeRequests(
-				RequestMatcher permitAllMatcher,
+		private void configurePermittedRequests(
 				ExpressionUrlAuthorizationConfigurer<HttpSecurity>.ExpressionInterceptUrlRegistry requests) {
-			requests.requestMatchers(permitAllMatcher).permitAll();
+			// Permit access to the non-sensitive endpoints
+			requests.requestMatchers(new EndpointPathRequestMatcher(
+					EndpointPaths.get(this.endpointHandlerMapping, false))).permitAll();
+			// Restrict the rest to the configured role
 			requests.anyRequest().hasRole(this.management.getSecurity().getRole());
 		}
 
 		private final class EndpointPathRequestMatcher implements RequestMatcher {
 
-			private boolean sensitive;
-
 			private RequestMatcher delegate;
 
-			EndpointPathRequestMatcher() {
-				this(true);
-			}
+			private String[] paths;
 
-			EndpointPathRequestMatcher(boolean sensitive) {
-				this.sensitive = sensitive;
+			EndpointPathRequestMatcher(String[] paths) {
+				this.paths = paths;
 			}
 
 			@Override
@@ -323,33 +325,41 @@ public class ManagementWebSecurityAutoConfiguration {
 			private RequestMatcher createDelegate() {
 				ServerProperties server = ManagementWebSecurityConfigurerAdapter.this.server;
 				List<RequestMatcher> matchers = new ArrayList<RequestMatcher>();
-				for (String path : getPaths()) {
+				for (String path : this.paths) {
 					matchers.add(new AntPathRequestMatcher(server.getPath(path)));
 				}
 				return (matchers.isEmpty() ? AnyRequestMatcher.INSTANCE
 						: new OrRequestMatcher(matchers));
 			}
 
-			private String[] getPaths() {
-				EndpointHandlerMapping endpointHandlerMapping = ManagementWebSecurityConfigurerAdapter.this.endpointHandlerMapping;
-				if (this.sensitive) {
-					return EndpointPaths.get(endpointHandlerMapping);
-				}
-				return EndpointPaths.get(endpointHandlerMapping, false);
-			}
-
 		}
 
 	}
 
+	/**
+	 * Helper class for extracting lists of paths from the EndpointHandlerMapping.
+	 */
 	private static class EndpointPaths {
 
+		/**
+		 * Get all the paths (sensitive and unsensitive).
+		 *
+		 * @param endpointHandlerMapping the mapping
+		 * @return all paths
+		 */
 		public static String[] get(EndpointHandlerMapping endpointHandlerMapping) {
 			String[] insecure = get(endpointHandlerMapping, false);
 			String[] secure = get(endpointHandlerMapping, true);
 			return StringUtils.mergeStringArrays(insecure, secure);
 		}
 
+		/**
+		 * Get all the paths that are either sensitive or unsensitive.
+		 *
+		 * @param endpointHandlerMapping the mapping
+		 * @param secure flag to say if we want the secure ones
+		 * @return the relevant paths
+		 */
 		public static String[] get(EndpointHandlerMapping endpointHandlerMapping,
 				boolean secure) {
 			if (endpointHandlerMapping == null) {
@@ -362,14 +372,14 @@ public class ManagementWebSecurityAutoConfiguration {
 					String path = endpointHandlerMapping.getPath(endpoint.getPath());
 					paths.add(path);
 					if (!path.equals("")) {
-						// Ensure that nested paths are secured
-						paths.add(path + "/**");
-						// Add Spring MVC-generated additional paths
-						paths.add(path + ".*");
-					}
-					else {
-						paths.add("/");
+						if (secure) {
+							// Ensure that nested paths are secured
+							paths.add(path + "/**");
+							// Add Spring MVC-generated additional paths
+							paths.add(path + ".*");
+						}
 					}
+					paths.add(path + "/");
 				}
 			}
 			return paths.toArray(new String[paths.size()]);

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java

@@ -16,6 +16,13 @@
 
 package org.springframework.boot.actuate.endpoint.mvc;
 
+import static org.hamcrest.Matchers.startsWith;
+import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
+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;
+
+import org.junit.After;
 import org.junit.Test;
 import org.springframework.boot.actuate.autoconfigure.EndpointAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.EndpointWebMvcAutoConfiguration;
@@ -32,18 +39,14 @@ import org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration;
 import org.springframework.boot.test.EnvironmentTestUtils;
 import org.springframework.context.annotation.Import;
 import org.springframework.mock.web.MockServletContext;
+import org.springframework.security.authentication.TestingAuthenticationToken;
+import org.springframework.security.test.context.TestSecurityContextHolder;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.test.web.servlet.setup.MockMvcConfigurer;
 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
 
-import static org.hamcrest.Matchers.startsWith;
-import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
-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;
-
 /**
  * Integration tests for the Actuator's MVC endpoints.
  *
@@ -55,6 +58,11 @@ public class MvcEndpointIntegrationTests {
 
 	private AnnotationConfigWebApplicationContext context;
 
+	@After
+	public void close() {
+		TestSecurityContextHolder.clearContext();
+	}
+
 	@Test
 	public void defaultJsonResponseIsNotIndented() throws Exception {
 		this.context = new AnnotationConfigWebApplicationContext();
@@ -81,13 +89,80 @@ public class MvcEndpointIntegrationTests {
 	}
 
 	@Test
-	public void endpointsAreSecureByDefault() throws Exception {
+	public void nonSensitiveEndpointsAreNotSecureByDefault() throws Exception {
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.register(SecureConfiguration.class);
+		MockMvc mockMvc = createSecureMockMvc();
+		mockMvc.perform(get("/info")).andExpect(status().isOk());
+		mockMvc.perform(get("/actuator")).andExpect(status().isOk());
+	}
+
+	@Test
+	public void nonSensitiveEndpointsAreNotSecureByDefaultWithCustomContextPath()
+			throws Exception {
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.register(SecureConfiguration.class);
+		EnvironmentTestUtils.addEnvironment(this.context,
+				"management.context-path:/management");
+		MockMvc mockMvc = createSecureMockMvc();
+		mockMvc.perform(get("/management/info")).andExpect(status().isOk());
+		mockMvc.perform(get("/management/")).andExpect(status().isOk());
+	}
+
+	@Test
+	public void sensitiveEndpointsAreSecureByDefault() throws Exception {
 		this.context = new AnnotationConfigWebApplicationContext();
 		this.context.register(SecureConfiguration.class);
 		MockMvc mockMvc = createSecureMockMvc();
 		mockMvc.perform(get("/beans")).andExpect(status().isUnauthorized());
 	}
 
+	@Test
+	public void sensitiveEndpointsAreSecureByDefaultWithCustomContextPath()
+			throws Exception {
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.register(SecureConfiguration.class);
+		EnvironmentTestUtils.addEnvironment(this.context,
+				"management.context-path:/management");
+		MockMvc mockMvc = createSecureMockMvc();
+		mockMvc.perform(get("/management/beans")).andExpect(status().isUnauthorized());
+	}
+
+	@Test
+	public void sensitiveEndpointsAreSecureWithNonAdminRoleWithCustomContextPath()
+			throws Exception {
+		TestSecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "N/A", "ROLE_USER"));
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.register(SecureConfiguration.class);
+		EnvironmentTestUtils.addEnvironment(this.context,
+				"management.context-path:/management");
+		MockMvc mockMvc = createSecureMockMvc();
+		mockMvc.perform(get("/management/beans")).andExpect(status().isForbidden());
+	}
+
+	@Test
+	public void sensitiveEndpointsAreSecureWithAdminRoleWithCustomContextPath()
+			throws Exception {
+		TestSecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "N/A", "ROLE_ADMIN"));
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.register(SecureConfiguration.class);
+		EnvironmentTestUtils.addEnvironment(this.context,
+				"management.context-path:/management");
+		MockMvc mockMvc = createSecureMockMvc();
+		mockMvc.perform(get("/management/beans")).andExpect(status().isOk());
+	}
+
+	@Test
+	public void endpointSecurityCanBeDisabledWithCustomContextPath() throws Exception {
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.register(SecureConfiguration.class);
+		EnvironmentTestUtils.addEnvironment(this.context,
+				"management.context-path:/management",
+				"management.security.enabled:false");
+		MockMvc mockMvc = createSecureMockMvc();
+		mockMvc.perform(get("/management/beans")).andExpect(status().isOk());
+	}
+
 	@Test
 	public void endpointSecurityCanBeDisabled() throws Exception {
 		this.context = new AnnotationConfigWebApplicationContext();
@@ -104,8 +179,8 @@ public class MvcEndpointIntegrationTests {
 		EnvironmentTestUtils.addEnvironment(this.context,
 				"spring.jackson.serialization.indent-output:true");
 		MockMvc mockMvc = createMockMvc();
-		mockMvc.perform(get("/beans")).andExpect(
-				content().string(startsWith("{" + LINE_SEPARATOR)));
+		mockMvc.perform(get("/beans"))
+				.andExpect(content().string(startsWith("{" + LINE_SEPARATOR)));
 	}
 
 	private MockMvc createMockMvc() {
@@ -127,8 +202,8 @@ public class MvcEndpointIntegrationTests {
 	}
 
 	@ImportAutoConfiguration({ JacksonAutoConfiguration.class,
-			HttpMessageConvertersAutoConfiguration.class,
-			EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class,
+			HttpMessageConvertersAutoConfiguration.class, EndpointAutoConfiguration.class,
+			EndpointWebMvcAutoConfiguration.class,
 			ManagementServerPropertiesAutoConfiguration.class,
 			PropertyPlaceholderAutoConfiguration.class, WebMvcAutoConfiguration.class })
 	static class DefaultConfiguration {
@@ -146,8 +221,8 @@ public class MvcEndpointIntegrationTests {
 
 	@ImportAutoConfiguration({ HypermediaAutoConfiguration.class,
 			RepositoryRestMvcAutoConfiguration.class, JacksonAutoConfiguration.class,
-			HttpMessageConvertersAutoConfiguration.class,
-			EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class,
+			HttpMessageConvertersAutoConfiguration.class, EndpointAutoConfiguration.class,
+			EndpointWebMvcAutoConfiguration.class,
 			ManagementServerPropertiesAutoConfiguration.class,
 			PropertyPlaceholderAutoConfiguration.class, WebMvcAutoConfiguration.class })
 	static class SpringDataRestConfiguration {

spring-boot-actuator/src/test/resources/logback.xml

@@ -4,4 +4,5 @@
 	<root level="INFO">
 		<appender-ref ref="CONSOLE" />
 	</root>
+	<logger name="org.springframework.security" level="DEBUG"/>
 </configuration>