Commit e5e1f24d authored by Madhura Bhave's avatar Madhura Bhave

Revert "Skip MvcSecurityInterceptor if Spring Security present"

Instead of entirely skipping the interceptor, we will be additionally
checking for authorities.
parent ad5cb8a3
......@@ -54,7 +54,6 @@ import org.springframework.context.annotation.ConditionContext;
import org.springframework.context.annotation.Conditional;
import org.springframework.core.env.Environment;
import org.springframework.core.type.AnnotatedTypeMetadata;
import org.springframework.util.ClassUtils;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.cors.CorsConfiguration;
......@@ -103,29 +102,16 @@ public class EndpointWebMvcManagementContextConfiguration {
EndpointHandlerMapping mapping = new EndpointHandlerMapping(endpoints,
corsConfiguration);
mapping.setPrefix(this.managementServerProperties.getContextPath());
if (isSecurityInterceptorRequired()) {
MvcEndpointSecurityInterceptor securityInterceptor = new MvcEndpointSecurityInterceptor(
this.managementServerProperties.getSecurity().getRoles());
mapping.setSecurityInterceptor(securityInterceptor);
}
MvcEndpointSecurityInterceptor securityInterceptor = new MvcEndpointSecurityInterceptor(
this.managementServerProperties.getSecurity().isEnabled(),
this.managementServerProperties.getSecurity().getRoles());
mapping.setSecurityInterceptor(securityInterceptor);
for (EndpointHandlerMappingCustomizer customizer : this.mappingCustomizers) {
customizer.customize(mapping);
}
return mapping;
}
private boolean isSecurityInterceptorRequired() {
return this.managementServerProperties.getSecurity().isEnabled()
&& !isSpringSecurityAvailable();
}
private boolean isSpringSecurityAvailable() {
return ClassUtils.isPresent(
"org.springframework.security.config.annotation.web.WebSecurityConfigurer",
getClass().getClassLoader());
}
private CorsConfiguration getCorsConfiguration(EndpointCorsProperties properties) {
if (CollectionUtils.isEmpty(properties.getAllowedOrigins())) {
return null;
......
......@@ -257,10 +257,8 @@ public class ManagementWebSecurityAutoConfiguration {
private void configurePermittedRequests(
ExpressionUrlAuthorizationConfigurer<HttpSecurity>.ExpressionInterceptUrlRegistry requests) {
List<String> roles = this.management.getSecurity().getRoles();
requests.requestMatchers(new LazyEndpointPathRequestMatcher(
this.contextResolver, EndpointPaths.SENSITIVE))
.hasAnyRole(roles.toArray(new String[roles.size()]));
this.contextResolver, EndpointPaths.SENSITIVE)).authenticated();
// Permit access to the non-sensitive endpoints
requests.requestMatchers(new LazyEndpointPathRequestMatcher(
this.contextResolver, EndpointPaths.NON_SENSITIVE)).permitAll();
......
......@@ -43,18 +43,21 @@ 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(List<String> roles) {
public MvcEndpointSecurityInterceptor(boolean secure, List<String> roles) {
this.secure = secure;
this.roles = roles;
}
@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response,
Object handler) throws Exception {
if (CorsUtils.isPreFlightRequest(request)) {
if (CorsUtils.isPreFlightRequest(request) || !this.secure) {
return true;
}
HandlerMethod handlerMethod = (HandlerMethod) handler;
......
......@@ -16,6 +16,8 @@
package org.springframework.boot.actuate.autoconfigure;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
......@@ -28,6 +30,7 @@ import org.springframework.boot.autoconfigure.security.SecurityAutoConfiguration
import org.springframework.boot.autoconfigure.web.HttpMessageConvertersAutoConfiguration;
import org.springframework.boot.autoconfigure.web.WebClientAutoConfiguration;
import org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration;
import org.springframework.boot.test.util.EnvironmentTestUtils;
import org.springframework.mock.web.MockServletContext;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
......@@ -55,8 +58,6 @@ public class EndpointWebMvcManagementContextConfigurationTests {
PropertyPlaceholderAutoConfiguration.class,
WebClientAutoConfiguration.class,
EndpointWebMvcManagementContextConfiguration.class);
this.context.refresh();
}
@After
......@@ -67,13 +68,25 @@ public class EndpointWebMvcManagementContextConfigurationTests {
}
@Test
public void endpointHandlerMappingShouldNotHaveSecurityInterceptor() throws Exception {
public void endpointHandlerMapping() throws Exception {
EnvironmentTestUtils.addEnvironment(this.context,
"management.security.enabled=false",
"management.security.roles=my-role,your-role");
this.context.refresh();
EndpointHandlerMapping mapping = this.context.getBean("endpointHandlerMapping",
EndpointHandlerMapping.class);
assertThat(mapping.getPrefix()).isEmpty();
MvcEndpointSecurityInterceptor securityInterceptor = (MvcEndpointSecurityInterceptor) ReflectionTestUtils
.getField(mapping, "securityInterceptor");
assertThat(securityInterceptor).isNull();
Object secure = ReflectionTestUtils.getField(securityInterceptor, "secure");
List<String> roles = getRoles(securityInterceptor);
assertThat(secure).isEqualTo(false);
assertThat(roles).containsExactly("my-role", "your-role");
}
@SuppressWarnings("unchecked")
private List<String> getRoles(MvcEndpointSecurityInterceptor securityInterceptor) {
return (List<String>) ReflectionTestUtils.getField(securityInterceptor, "roles");
}
}
......@@ -17,6 +17,7 @@
package org.springframework.boot.actuate.autoconfigure;
import java.util.Collection;
import java.util.Collections;
import org.hamcrest.Matchers;
import org.junit.After;
......@@ -25,6 +26,7 @@ import org.junit.Test;
import org.springframework.boot.actuate.endpoint.mvc.EndpointHandlerMapping;
import org.springframework.boot.actuate.endpoint.mvc.JolokiaMvcEndpoint;
import org.springframework.boot.actuate.endpoint.mvc.MvcEndpoint;
import org.springframework.boot.actuate.endpoint.mvc.MvcEndpointSecurityInterceptor;
import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
import org.springframework.boot.autoconfigure.web.HttpMessageConvertersAutoConfiguration;
import org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration;
......@@ -142,6 +144,8 @@ public class JolokiaAutoConfigurationTests {
public EndpointHandlerMapping endpointHandlerMapping(
Collection<? extends MvcEndpoint> endpoints) {
EndpointHandlerMapping mapping = new EndpointHandlerMapping(endpoints);
mapping.setSecurityInterceptor(new MvcEndpointSecurityInterceptor(false,
Collections.<String>emptyList()));
return mapping;
}
......
......@@ -55,7 +55,6 @@ import org.springframework.test.web.servlet.ResultMatcher;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.util.Base64Utils;
import org.springframework.util.StringUtils;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
......@@ -255,57 +254,6 @@ public class ManagementWebSecurityAutoConfigurationTests {
.andExpect(status().isUnauthorized());
}
@Test
public void sensitiveEndpointWithRightRoleShouldReturnOk() throws Exception {
this.context = new AnnotationConfigWebApplicationContext();
this.context.setServletContext(new MockServletContext());
this.context.register(AuthenticationConfig.class, WebConfiguration.class);
EnvironmentTestUtils.addEnvironment(this.context, "management.security.roles:USER");
this.context.refresh();
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context)
.apply(springSecurity())
.build();
String authorizationHeader = Base64Utils.encodeToString("user:password".getBytes());
mockMvc //
.perform(get("/env").header("authorization", "Basic " + authorizationHeader))
.andExpect(status().isOk());
}
@Test
public void sensitiveEndpointWithRightRoleShouldReturnForbidden() throws Exception {
this.context = new AnnotationConfigWebApplicationContext();
this.context.setServletContext(new MockServletContext());
this.context.register(AuthenticationConfig.class, WebConfiguration.class);
this.context.refresh();
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context)
.apply(springSecurity())
.build();
String authorizationHeader = Base64Utils.encodeToString("user:password".getBytes());
mockMvc //
.perform(get("/env").header("authorization", "Basic " + authorizationHeader))
.andExpect(status().isForbidden());
}
@Test
public void nonSensitiveEndpointShouldAlwaysReturnOk() throws Exception {
this.context = new AnnotationConfigWebApplicationContext();
this.context.setServletContext(new MockServletContext());
this.context.register(WebConfiguration.class);
this.context.refresh();
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context)
.apply(springSecurity())
.build();
mockMvc //
.perform(get("/health"))
.andExpect(status().isOk());
}
private ResultMatcher springAuthenticateRealmHeader() {
return MockMvcResultMatchers.header().string("www-authenticate",
Matchers.containsString("realm=\"Spring\""));
......
/*
* Copyright 2012-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.actuate.autoconfigure;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.boot.actuate.endpoint.mvc.EndpointHandlerMapping;
import org.springframework.boot.actuate.endpoint.mvc.MvcEndpointSecurityInterceptor;
import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration;
import org.springframework.boot.autoconfigure.web.HttpMessageConvertersAutoConfiguration;
import org.springframework.boot.autoconfigure.web.WebClientAutoConfiguration;
import org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration;
import org.springframework.boot.junit.runner.classpath.ClassPathExclusions;
import org.springframework.boot.junit.runner.classpath.ModifiedClassPathRunner;
import org.springframework.boot.test.util.EnvironmentTestUtils;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.test.util.ReflectionTestUtils;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Tests for {@link EndpointWebMvcManagementContextConfiguration} when Spring Security is not available.
*
* @author Madhura Bhave
*/
@RunWith(ModifiedClassPathRunner.class)
@ClassPathExclusions("spring-security-*.jar")
public class NoSpringSecurityEndpointWebMvcManagementContextConfigurationTests {
private AnnotationConfigApplicationContext context;
@Before
public void setup() {
this.context = new AnnotationConfigApplicationContext();
this.context.register(WebMvcAutoConfiguration.class, JacksonAutoConfiguration.class,
HttpMessageConvertersAutoConfiguration.class,
EndpointAutoConfiguration.class, EndpointWebMvcAutoConfiguration.class,
ManagementServerPropertiesAutoConfiguration.class,
PropertyPlaceholderAutoConfiguration.class,
WebClientAutoConfiguration.class,
EndpointWebMvcManagementContextConfiguration.class);
}
@After
public void close() {
if (this.context != null) {
this.context.close();
}
}
@Test
public void endpointHandlerMappingWhenSecurityEnabledShouldHaveSecurityInterceptor() throws Exception {
EnvironmentTestUtils.addEnvironment(this.context,
"management.security.enabled=true",
"management.security.roles=my-role,your-role");
this.context.refresh();
EndpointHandlerMapping mapping = this.context.getBean("endpointHandlerMapping",
EndpointHandlerMapping.class);
MvcEndpointSecurityInterceptor securityInterceptor = (MvcEndpointSecurityInterceptor) ReflectionTestUtils
.getField(mapping, "securityInterceptor");
List<String> roles = getRoles(securityInterceptor);
assertThat(roles).containsExactly("my-role", "your-role");
}
@Test
public void endpointHandlerMappingWhenSecurityDisabledShouldHaveSecurityInterceptor() throws Exception {
EnvironmentTestUtils.addEnvironment(this.context,
"management.security.enabled=false",
"management.security.roles=my-role,your-role");
this.context.refresh();
EndpointHandlerMapping mapping = this.context.getBean("endpointHandlerMapping",
EndpointHandlerMapping.class);
MvcEndpointSecurityInterceptor securityInterceptor = (MvcEndpointSecurityInterceptor) ReflectionTestUtils
.getField(mapping, "securityInterceptor");
assertThat(securityInterceptor).isNull();
}
@SuppressWarnings("unchecked")
private List<String> getRoles(MvcEndpointSecurityInterceptor securityInterceptor) {
return (List<String>) ReflectionTestUtils.getField(securityInterceptor, "roles");
}
}
......@@ -66,7 +66,7 @@ public class MvcEndpointSecurityInterceptorTests {
@Before
public void setup() throws Exception {
this.roles = Arrays.asList("SUPER_HERO");
this.securityInterceptor = new MvcEndpointSecurityInterceptor(this.roles);
this.securityInterceptor = new MvcEndpointSecurityInterceptor(true, this.roles);
this.endpoint = new TestEndpoint("a");
this.mvcEndpoint = new TestMvcEndpoint(this.endpoint);
this.handlerMethod = new HandlerMethod(this.mvcEndpoint, "invoke");
......@@ -75,6 +75,13 @@ public class MvcEndpointSecurityInterceptorTests {
this.response = mock(HttpServletResponse.class);
}
@Test
public void securityDisabledShouldAllowAccess() throws Exception {
this.securityInterceptor = new MvcEndpointSecurityInterceptor(false, this.roles);
assertThat(this.securityInterceptor.preHandle(this.request, this.response,
this.handlerMethod)).isTrue();
}
@Test
public void endpointNotSensitiveShouldAllowAccess() throws Exception {
this.endpoint.setSensitive(false);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment