diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementWebSecurityAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementWebSecurityAutoConfiguration.java index c942b2deba..09e3649b14 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementWebSecurityAutoConfiguration.java +++ b/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 { + private static class IgnoredPathsWebSecurityConfigurerAdapter + implements WebSecurityConfigurer { @Autowired(required = false) private ErrorController errorController; @@ -152,8 +152,8 @@ public class ManagementWebSecurityAutoConfiguration { List 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.emptySet()); + Collections. 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.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 matchers = new ArrayList(); - 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()]); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java index a26c73f367..e8cc76d983 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointIntegrationTests.java +++ b/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 { diff --git a/spring-boot-actuator/src/test/resources/logback.xml b/spring-boot-actuator/src/test/resources/logback.xml index dd767df9c2..30256aabf4 100644 --- a/spring-boot-actuator/src/test/resources/logback.xml +++ b/spring-boot-actuator/src/test/resources/logback.xml @@ -4,4 +4,5 @@ + \ No newline at end of file