Skip to content

Commit

Permalink
Feature/fix configuration spring security (#674)
Browse files Browse the repository at this point in the history
* fix, add `ProblemHttpConfigurer` for using configuration with `WebSecurityConfigurerAdapter`

* fix `Found WebSecurityConfigurerAdapter as well as SecurityFilterChain. Please select just one.`

* Add logging register message

* Change BeanInstantiationException to BeanCreationException

* Fix, improve test coverage
  • Loading branch information
aafwu00 authored Aug 30, 2021
1 parent 40ed0fe commit 7c436f7
Show file tree
Hide file tree
Showing 10 changed files with 414 additions and 40 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.zalando.problem.spring.web.autoconfigure.security;

import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.context.ApplicationContext;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
import org.zalando.problem.spring.web.advice.security.SecurityProblemSupport;

@Slf4j
public class ProblemHttpConfigurer extends AbstractHttpConfigurer<ProblemHttpConfigurer, HttpSecurity> {

@Override
public void init(final HttpSecurity http) {
final ApplicationContext applicationContext = http.getSharedObject(ApplicationContext.class);
if (applicationContext == null) {
log.warn("HttpSecurity's SharedObject doesn't have ApplicationContext");
return;
}
final ObjectProvider<SecurityProblemSupport> provider = applicationContext.getBeanProvider(SecurityProblemSupport.class);
provider.ifAvailable(support -> register(http, support));
}

private void register(final HttpSecurity http, final SecurityProblemSupport support) {
try {
http.exceptionHandling().authenticationEntryPoint(support).accessDeniedHandler(support);
} catch (final Exception cause) {
throw new BeanCreationException("Fail to register HttpSecurity's exceptionHandling", cause);
}
log.info("Register HttpSecurity's exceptionHandling");
}
}
Original file line number Diff line number Diff line change
@@ -1,55 +1,44 @@
package org.zalando.problem.spring.web.autoconfigure.security;

import lombok.AllArgsConstructor;
import org.apiguardian.api.API;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.AutoConfigureAfter;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type;
import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.security.config.annotation.web.WebSecurityConfigurer;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.zalando.problem.spring.web.advice.AdviceTrait;
import org.zalando.problem.spring.web.advice.security.SecurityProblemSupport;
import org.zalando.problem.spring.web.autoconfigure.ProblemAutoConfiguration;

import static org.apiguardian.api.API.Status.INTERNAL;

@API(status = INTERNAL)
@AllArgsConstructor
@Configuration(proxyBeanMethods = false)
@ConditionalOnWebApplication
@ConditionalOnWebApplication(type = Type.SERVLET)
@ConditionalOnClass(WebSecurityConfigurer.class)
@ConditionalOnBean(WebSecurityConfiguration.class)
@Import(SecurityProblemSupport.class)
//subtract random, uncommon number to reduce chances of collision with a user-selected order
@Order(Ordered.LOWEST_PRECEDENCE - 21)
@AutoConfigureAfter(SecurityAutoConfiguration.class)
@AutoConfigureBefore(ProblemAutoConfiguration.class)
public class ProblemSecurityAutoConfiguration extends WebSecurityConfigurerAdapter {

private final SecurityProblemSupport support;

@Override
public void configure(final HttpSecurity http) throws Exception {
http.exceptionHandling()
.authenticationEntryPoint(support)
.accessDeniedHandler(support);
}
public class ProblemSecurityAutoConfiguration {

@Bean
@ConditionalOnMissingBean(AdviceTrait.class)
public AdviceTrait securityExceptionHandling() {
return new SecurityExceptionHandling();
}

@Bean
public ProblemSecurityBeanPostProcessor problemSecurityBeanPostProcessor(final ObjectProvider<SecurityProblemSupport> securityProblemSupport) {
return new ProblemSecurityBeanPostProcessor(securityProblemSupport);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.zalando.problem.spring.web.autoconfigure.security;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.util.ClassUtils;
import org.zalando.problem.spring.web.advice.security.SecurityProblemSupport;

@Slf4j
@RequiredArgsConstructor
public class ProblemSecurityBeanPostProcessor implements BeanPostProcessor {
private final ObjectProvider<SecurityProblemSupport> securityProblemSupport;

@Override
public Object postProcessAfterInitialization(final Object bean, final String beanName) throws BeansException {
if (ClassUtils.isAssignableValue(HttpSecurity.class, bean)) {
securityProblemSupport.ifAvailable(support -> register((HttpSecurity) bean, beanName, support));
}
return bean;
}

private void register(final HttpSecurity http, final String beanName, final SecurityProblemSupport support) {
try {
http.exceptionHandling().authenticationEntryPoint(support).accessDeniedHandler(support);
} catch (final Exception cause) {
throw new BeanCreationException(beanName, cause);
}
log.info("Register HttpSecurity's exceptionHandling");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ org.springframework.boot.autoconfigure.EnableAutoConfiguration=\
org.zalando.problem.spring.web.autoconfigure.ProblemJacksonWebMvcAutoConfiguration, \
org.zalando.problem.spring.web.autoconfigure.ProblemJacksonAutoConfiguration, \
org.zalando.problem.spring.web.autoconfigure.security.ProblemSecurityAutoConfiguration

org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer=\
org.zalando.problem.spring.web.autoconfigure.security.ProblemHttpConfigurer
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package org.zalando.problem.spring.web.autoconfigure.security;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.web.servlet.MockMvc;

import static org.hamcrest.Matchers.is;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

@SpringBootTest
@AutoConfigureMockMvc
@DirtiesContext
final class NoConfigurerSecurityTest {
@Autowired
private MockMvc mockMvc;

@Test
void shouldConfigureExceptionHandling() throws Exception {
mockMvc.perform(get("/not-exists-url"))
.andExpect(status().isUnauthorized())
.andExpect(jsonPath("$.status", is(401)))
.andExpect(jsonPath("$.title", is("Unauthorized")))
.andExpect(jsonPath("$.detail", is("Full authentication is required to access this resource")));
}

@SpringBootApplication
static class TestApp {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package org.zalando.problem.spring.web.autoconfigure.security;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.BeanInitializationException;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.context.ApplicationContext;
import org.springframework.core.io.support.SpringFactoriesLoader;
import org.springframework.security.config.annotation.ObjectPostProcessor;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.util.ClassUtils;
import org.zalando.problem.spring.web.advice.security.SecurityProblemSupport;

import java.util.HashMap;
import java.util.Map;
import java.util.function.Consumer;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;

class ProblemHttpConfigurerTest {

private ProblemHttpConfigurer configurer;
private Map<Class<?>, Object> sharedObjects;
private ApplicationContext applicationContext;
private ObjectPostProcessor<Object> objectPostProcessor;
private AuthenticationManagerBuilder authenticationManagerBuilder;

@SuppressWarnings("unchecked")
@BeforeEach
void setUp() {
configurer = new ProblemHttpConfigurer();
sharedObjects = new HashMap<>();
applicationContext = mock(ApplicationContext.class);
objectPostProcessor = mock(ObjectPostProcessor.class);
authenticationManagerBuilder = mock(AuthenticationManagerBuilder.class);
}

@Test
void shouldBeRegisteredSpringFactoriesLoader() {
assertThat(SpringFactoriesLoader.loadFactories(AbstractHttpConfigurer.class, ClassUtils.getDefaultClassLoader()))
.hasAtLeastOneElementOfType(ProblemHttpConfigurer.class);
}

@Test
@SuppressWarnings("unchecked")
void shouldBeConfigured() throws Exception {
sharedObjects.put(ApplicationContext.class, applicationContext);
final SecurityProblemSupport securityProblemSupport = mock(SecurityProblemSupport.class);
final ObjectProvider<SecurityProblemSupport> beanProvider = mock(ObjectProvider.class);
doReturn(beanProvider).when(applicationContext).getBeanProvider(SecurityProblemSupport.class);
doAnswer(answer -> {
final Consumer<SecurityProblemSupport> consumer = (Consumer<SecurityProblemSupport>) answer.getArguments()[0];
consumer.accept(securityProblemSupport);
return null;
}).when(beanProvider).ifAvailable(any(Consumer.class));
final HttpSecurity http = new HttpSecurity(objectPostProcessor, authenticationManagerBuilder, sharedObjects);

configurer.init(http);

assertThat(ReflectionTestUtils.getField(http.exceptionHandling(), "authenticationEntryPoint")).isEqualTo(securityProblemSupport);
assertThat(ReflectionTestUtils.getField(http.exceptionHandling(), "accessDeniedHandler")).isEqualTo(securityProblemSupport);
}


@Test
@SuppressWarnings("unchecked")
void shouldBeNotConfiguredWhenSecurityProblemSupportNotExists() throws Exception {
sharedObjects.put(ApplicationContext.class, applicationContext);
final ObjectProvider<SecurityProblemSupport> beanProvider = mock(ObjectProvider.class);
doReturn(beanProvider).when(applicationContext).getBeanProvider(SecurityProblemSupport.class);
doReturn(null).when(beanProvider).getIfAvailable();
final HttpSecurity http = new HttpSecurity(objectPostProcessor, authenticationManagerBuilder, sharedObjects);

configurer.init(http);

assertThat(ReflectionTestUtils.getField(http.exceptionHandling(), "authenticationEntryPoint")).isEqualTo(null);
assertThat(ReflectionTestUtils.getField(http.exceptionHandling(), "accessDeniedHandler")).isEqualTo(null);
}

@Test
void shouldBeNotConfiguredWhenApplicationContextNotExists() throws Exception {
final HttpSecurity http = new HttpSecurity(objectPostProcessor, authenticationManagerBuilder, sharedObjects);

configurer.init(http);

assertThat(ReflectionTestUtils.getField(http.exceptionHandling(), "authenticationEntryPoint")).isEqualTo(null);
assertThat(ReflectionTestUtils.getField(http.exceptionHandling(), "accessDeniedHandler")).isEqualTo(null);
}


@Test
@SuppressWarnings("unchecked")
void shouldBeThrowBeanCreationExceptionWhenRegisterExceptionHandling() throws Exception {
sharedObjects.put(ApplicationContext.class, applicationContext);
final SecurityProblemSupport securityProblemSupport = mock(SecurityProblemSupport.class);
final ObjectProvider<SecurityProblemSupport> beanProvider = mock(ObjectProvider.class);
doReturn(beanProvider).when(applicationContext).getBeanProvider(SecurityProblemSupport.class);
doAnswer(answer -> {
final Consumer<SecurityProblemSupport> consumer = (Consumer<SecurityProblemSupport>) answer.getArguments()[0];
consumer.accept(securityProblemSupport);
return null;
}).when(beanProvider).ifAvailable(any(Consumer.class));
final HttpSecurity http = spy(new HttpSecurity(objectPostProcessor, authenticationManagerBuilder, sharedObjects));
doThrow(new BeanInitializationException("test")).when(http).exceptionHandling();

assertThatThrownBy(() -> configurer.init(http)).isExactlyInstanceOf(BeanCreationException.class);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package org.zalando.problem.spring.web.autoconfigure.security;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration;
import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.web.servlet.HandlerExceptionResolver;
import org.zalando.problem.spring.web.advice.AdviceTrait;
import org.zalando.problem.spring.web.advice.security.SecurityProblemSupport;
import org.zalando.problem.spring.web.autoconfigure.ProblemAutoConfiguration;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

class ProblemSecurityAutoConfigurationTest {
WebApplicationContextRunner contextRunner;

@BeforeEach
void setUp() {
contextRunner = new WebApplicationContextRunner()
.withConfiguration(
AutoConfigurations.of(ProblemSecurityAutoConfiguration.class,
ProblemAutoConfiguration.class,
SecurityAutoConfiguration.class));
}

@Test
void shouldBeConfiguredDefaults() {
contextRunner.withUserConfiguration(SecurityConfiguration.class)
.run(context -> assertThat(context)
.hasSingleBean(SecurityProblemSupport.class)
.hasSingleBean(AdviceTrait.class)
.hasSingleBean(SecurityExceptionHandling.class)
.hasSingleBean(ProblemSecurityBeanPostProcessor.class));
}

@Test
void shouldBeConfiguredWithWebSecurityConfigurerAdapter() {
contextRunner.withUserConfiguration(WebSecurityConfigurerAdapterConfiguration.class)
.run(context -> assertThat(context)
.hasSingleBean(SecurityProblemSupport.class)
.hasSingleBean(AdviceTrait.class)
.hasSingleBean(SecurityExceptionHandling.class)
.hasSingleBean(ProblemSecurityBeanPostProcessor.class));
}

@Configuration(proxyBeanMethods = false)
static class SecurityConfiguration {
@Bean
HandlerExceptionResolver handlerExceptionResolver() {
return mock(HandlerExceptionResolver.class);
}
}

@Configuration(proxyBeanMethods = false)
static class WebSecurityConfigurerAdapterConfiguration {
@Bean
HandlerExceptionResolver handlerExceptionResolver() {
return mock(HandlerExceptionResolver.class);
}

@Configuration
static class SecurityConfig extends WebSecurityConfigurerAdapter {
@Override
public void configure(final HttpSecurity http) {
}
}
}
}
Loading

0 comments on commit 7c436f7

Please sign in to comment.