INT-3539: Fix ChannelSecurityInterceptorBPP
JIRA: https://jira.spring.io/browse/INT-3539 Previously the `ChannelSecurityInterceptorBeanPostProcessor` eagerly loaded all beans from its `afterPropertiesSet` to retrieve the `ChannelSecurityInterceptor`s. According to the `BeanPostProcessor` nature the `afterPropertiesSet` hook isn't legitimate. It may cause some bad side-effects for other beans, which might not been initialized yet. Rework `ChannelSecurityInterceptorBeanPostProcessor` to accept `Collection<ChannelSecurityInterceptor>` in the ctor. Rework `SecurityIntegrationConfigurationInitializer` to iterate over `BeanDefinition`s to determine those of them, which are `ChannelSecurityInterceptor` or `ChannelSecurityInterceptorFactoryBean`. **Cherry-pick to 4.0.x**
This commit is contained in:
@@ -25,15 +25,10 @@ import org.springframework.aop.framework.ProxyFactory;
|
||||
import org.springframework.aop.support.AopUtils;
|
||||
import org.springframework.aop.support.DefaultPointcutAdvisor;
|
||||
import org.springframework.beans.BeansException;
|
||||
import org.springframework.beans.factory.BeanFactory;
|
||||
import org.springframework.beans.factory.BeanFactoryAware;
|
||||
import org.springframework.beans.factory.InitializingBean;
|
||||
import org.springframework.beans.factory.ListableBeanFactory;
|
||||
import org.springframework.beans.factory.config.BeanPostProcessor;
|
||||
import org.springframework.integration.security.channel.ChannelSecurityInterceptor;
|
||||
import org.springframework.integration.security.channel.ChannelSecurityMetadataSource;
|
||||
import org.springframework.messaging.MessageChannel;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
/**
|
||||
* A {@link BeanPostProcessor} that proxies {@link MessageChannel}s to apply a {@link ChannelSecurityInterceptor}.
|
||||
@@ -42,21 +37,12 @@ import org.springframework.util.Assert;
|
||||
* @author Oleg Zhurakousky
|
||||
* @author Artem Bilan
|
||||
*/
|
||||
public class ChannelSecurityInterceptorBeanPostProcessor implements BeanPostProcessor, BeanFactoryAware, InitializingBean {
|
||||
public class ChannelSecurityInterceptorBeanPostProcessor implements BeanPostProcessor {
|
||||
|
||||
private volatile Collection<ChannelSecurityInterceptor> securityInterceptors;
|
||||
private final Collection<ChannelSecurityInterceptor> securityInterceptors;
|
||||
|
||||
private ListableBeanFactory beanFactory;
|
||||
|
||||
@Override
|
||||
public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
|
||||
Assert.isInstanceOf(ListableBeanFactory.class, beanFactory);
|
||||
this.beanFactory = (ListableBeanFactory) beanFactory;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void afterPropertiesSet() throws Exception {
|
||||
this.securityInterceptors = this.beanFactory.getBeansOfType(ChannelSecurityInterceptor.class).values();
|
||||
public ChannelSecurityInterceptorBeanPostProcessor(Collection<ChannelSecurityInterceptor> securityInterceptors) {
|
||||
this.securityInterceptors = securityInterceptors;
|
||||
}
|
||||
|
||||
public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
|
||||
@@ -65,7 +51,7 @@ public class ChannelSecurityInterceptorBeanPostProcessor implements BeanPostProc
|
||||
|
||||
public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
|
||||
if (bean instanceof MessageChannel) {
|
||||
for (ChannelSecurityInterceptor securityInterceptor : securityInterceptors) {
|
||||
for (ChannelSecurityInterceptor securityInterceptor : this.securityInterceptors) {
|
||||
ChannelSecurityMetadataSource channelSecurityMetadataSource =
|
||||
(ChannelSecurityMetadataSource) securityInterceptor.obtainSecurityMetadataSource();
|
||||
if (this.shouldProxy(beanName, channelSecurityMetadataSource)) {
|
||||
@@ -92,4 +78,5 @@ public class ChannelSecurityInterceptorBeanPostProcessor implements BeanPostProc
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -16,11 +16,23 @@
|
||||
|
||||
package org.springframework.integration.security.config;
|
||||
|
||||
import java.lang.reflect.Method;
|
||||
import java.util.List;
|
||||
|
||||
import org.springframework.beans.BeansException;
|
||||
import org.springframework.beans.factory.CannotLoadBeanClassException;
|
||||
import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition;
|
||||
import org.springframework.beans.factory.config.BeanDefinition;
|
||||
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
|
||||
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
|
||||
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
|
||||
import org.springframework.beans.factory.support.RootBeanDefinition;
|
||||
import org.springframework.beans.factory.support.ManagedList;
|
||||
import org.springframework.core.type.MethodMetadata;
|
||||
import org.springframework.core.type.StandardMethodMetadata;
|
||||
import org.springframework.integration.config.IntegrationConfigurationInitializer;
|
||||
import org.springframework.integration.security.channel.ChannelSecurityInterceptor;
|
||||
import org.springframework.util.ClassUtils;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
||||
/**
|
||||
* The Integration Security infrastructure {@code beanFactory} initializer.
|
||||
@@ -30,13 +42,49 @@ import org.springframework.integration.config.IntegrationConfigurationInitialize
|
||||
*/
|
||||
public class SecurityIntegrationConfigurationInitializer implements IntegrationConfigurationInitializer {
|
||||
|
||||
private static final String CHANNEL_SECURITY_INTERCEPTOR_BPP_BEAN_NAME = ChannelSecurityInterceptorBeanPostProcessor.class.getName();
|
||||
private static final String CHANNEL_SECURITY_INTERCEPTOR_BPP_BEAN_NAME =
|
||||
ChannelSecurityInterceptorBeanPostProcessor.class.getName();
|
||||
|
||||
@Override
|
||||
public void initialize(ConfigurableListableBeanFactory beanFactory) throws BeansException {
|
||||
BeanDefinitionRegistry registry = (BeanDefinitionRegistry) beanFactory;
|
||||
if (!registry.containsBeanDefinition(CHANNEL_SECURITY_INTERCEPTOR_BPP_BEAN_NAME)) {
|
||||
registry.registerBeanDefinition(CHANNEL_SECURITY_INTERCEPTOR_BPP_BEAN_NAME, new RootBeanDefinition(ChannelSecurityInterceptorBeanPostProcessor.class));
|
||||
|
||||
List<BeanDefinition> securityInterceptors = new ManagedList<BeanDefinition>();
|
||||
|
||||
for (String beanName : registry.getBeanDefinitionNames()) {
|
||||
BeanDefinition beanDefinition = registry.getBeanDefinition(beanName);
|
||||
String beanClassName = beanDefinition.getBeanClassName();
|
||||
Class<?> clazz = null;
|
||||
if (StringUtils.hasText(beanClassName)) {
|
||||
try {
|
||||
clazz = ClassUtils.forName(beanClassName, beanFactory.getBeanClassLoader());
|
||||
}
|
||||
catch (ClassNotFoundException e) {
|
||||
throw new CannotLoadBeanClassException(this.toString(), beanName, beanClassName, e);
|
||||
}
|
||||
}
|
||||
else if (beanDefinition instanceof AnnotatedBeanDefinition
|
||||
&& beanDefinition.getSource() instanceof MethodMetadata) {
|
||||
MethodMetadata beanMethod = (MethodMetadata) beanDefinition.getSource();
|
||||
if (beanMethod instanceof StandardMethodMetadata) {
|
||||
Method method = ((StandardMethodMetadata) beanMethod).getIntrospectedMethod();
|
||||
clazz = method.getReturnType();
|
||||
}
|
||||
}
|
||||
|
||||
if (clazz != null &&
|
||||
(ChannelSecurityInterceptor.class.isAssignableFrom(clazz)
|
||||
|| ChannelSecurityInterceptorFactoryBean.class.isAssignableFrom(clazz))) {
|
||||
securityInterceptors.add(beanDefinition);
|
||||
}
|
||||
}
|
||||
|
||||
if (!securityInterceptors.isEmpty()) {
|
||||
BeanDefinition securityPostProcessorBd =
|
||||
BeanDefinitionBuilder.rootBeanDefinition(ChannelSecurityInterceptorBeanPostProcessor.class)
|
||||
.addConstructorArgValue(securityInterceptors)
|
||||
.getBeanDefinition();
|
||||
registry.registerBeanDefinition(CHANNEL_SECURITY_INTERCEPTOR_BPP_BEAN_NAME, securityPostProcessorBd);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2002-2010 the original author or authors.
|
||||
* Copyright 2002-2014 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.
|
||||
@@ -19,74 +19,55 @@ package org.springframework.integration.security.channel;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.Map;
|
||||
import java.util.Arrays;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.junit.Test;
|
||||
import org.mockito.Mockito;
|
||||
import org.mockito.invocation.InvocationOnMock;
|
||||
import org.mockito.stubbing.Answer;
|
||||
|
||||
import org.springframework.aop.support.AopUtils;
|
||||
import org.springframework.beans.factory.ListableBeanFactory;
|
||||
import org.springframework.integration.channel.QueueChannel;
|
||||
import org.springframework.integration.security.config.ChannelSecurityInterceptorBeanPostProcessor;
|
||||
import org.springframework.messaging.MessageChannel;
|
||||
|
||||
/**
|
||||
* @author Mark Fisher
|
||||
* @author Artem Bilan
|
||||
*/
|
||||
public class ChannelSecurityInterceptorBeanPostProcessorTests {
|
||||
|
||||
@Test
|
||||
public void securedChannelIsProxied() throws Exception {
|
||||
ChannelSecurityMetadataSource securityMetadataSource = new ChannelSecurityMetadataSource();
|
||||
securityMetadataSource.addPatternMapping(Pattern.compile("secured.*"), new DefaultChannelAccessPolicy("ROLE_ADMIN", null));
|
||||
securityMetadataSource.addPatternMapping(Pattern.compile("secured.*"),
|
||||
new DefaultChannelAccessPolicy("ROLE_ADMIN", null));
|
||||
|
||||
final ChannelSecurityInterceptor interceptor = new ChannelSecurityInterceptor(securityMetadataSource);
|
||||
ChannelSecurityInterceptor interceptor = new ChannelSecurityInterceptor(securityMetadataSource);
|
||||
|
||||
ListableBeanFactory beanFactory = Mockito.mock(ListableBeanFactory.class);
|
||||
Mockito.doAnswer(new Answer<Map<String, ChannelSecurityInterceptor>>() {
|
||||
|
||||
@Override
|
||||
public Map<String, ChannelSecurityInterceptor> answer(InvocationOnMock invocation) throws Throwable {
|
||||
return Collections.singletonMap("interceptor", interceptor);
|
||||
}
|
||||
}).when(beanFactory).getBeansOfType(ChannelSecurityInterceptor.class);
|
||||
|
||||
ChannelSecurityInterceptorBeanPostProcessor postProcessor = new ChannelSecurityInterceptorBeanPostProcessor();
|
||||
postProcessor.setBeanFactory(beanFactory);
|
||||
postProcessor.afterPropertiesSet();
|
||||
ChannelSecurityInterceptorBeanPostProcessor postProcessor =
|
||||
new ChannelSecurityInterceptorBeanPostProcessor(Arrays.asList(interceptor));
|
||||
|
||||
QueueChannel securedChannel = new QueueChannel();
|
||||
securedChannel.setBeanName("securedChannel");
|
||||
MessageChannel postProcessedChannel = (MessageChannel) postProcessor.postProcessAfterInitialization(securedChannel, "securedChannel");
|
||||
MessageChannel postProcessedChannel =
|
||||
(MessageChannel) postProcessor.postProcessAfterInitialization(securedChannel, "securedChannel");
|
||||
assertTrue(AopUtils.isAopProxy(postProcessedChannel));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void nonsecuredChannelIsNotProxied() throws Exception {
|
||||
ChannelSecurityMetadataSource securityMetadataSource = new ChannelSecurityMetadataSource();
|
||||
securityMetadataSource.addPatternMapping(Pattern.compile("secured.*"), new DefaultChannelAccessPolicy("ROLE_ADMIN", null));
|
||||
final ChannelSecurityInterceptor interceptor = new ChannelSecurityInterceptor(securityMetadataSource);
|
||||
securityMetadataSource.addPatternMapping(Pattern.compile("secured.*"),
|
||||
new DefaultChannelAccessPolicy("ROLE_ADMIN", null));
|
||||
|
||||
ListableBeanFactory beanFactory = Mockito.mock(ListableBeanFactory.class);
|
||||
Mockito.doAnswer(new Answer<Map<String, ChannelSecurityInterceptor>>() {
|
||||
ChannelSecurityInterceptor interceptor = new ChannelSecurityInterceptor(securityMetadataSource);
|
||||
|
||||
@Override
|
||||
public Map<String, ChannelSecurityInterceptor> answer(InvocationOnMock invocation) throws Throwable {
|
||||
return Collections.singletonMap("interceptor", interceptor);
|
||||
}
|
||||
}).when(beanFactory).getBeansOfType(ChannelSecurityInterceptor.class);
|
||||
|
||||
ChannelSecurityInterceptorBeanPostProcessor postProcessor = new ChannelSecurityInterceptorBeanPostProcessor();
|
||||
postProcessor.setBeanFactory(beanFactory);
|
||||
postProcessor.afterPropertiesSet();
|
||||
ChannelSecurityInterceptorBeanPostProcessor postProcessor =
|
||||
new ChannelSecurityInterceptorBeanPostProcessor(Arrays.asList(interceptor));
|
||||
|
||||
QueueChannel channel = new QueueChannel();
|
||||
channel.setBeanName("testChannel");
|
||||
MessageChannel postProcessedChannel = (MessageChannel) postProcessor.postProcessAfterInitialization(channel, "testChannel");
|
||||
MessageChannel postProcessedChannel =
|
||||
(MessageChannel) postProcessor.postProcessAfterInitialization(channel, "testChannel");
|
||||
assertFalse(AopUtils.isAopProxy(postProcessedChannel));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user