From 542fc4fb5ea1dabe84adef1649a34cb318640f7f Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 29 Oct 2014 11:02:38 +0200 Subject: [PATCH] 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` 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** --- ...lSecurityInterceptorBeanPostProcessor.java | 25 ++------- ...tyIntegrationConfigurationInitializer.java | 56 +++++++++++++++++-- ...rityInterceptorBeanPostProcessorTests.java | 53 ++++++------------ 3 files changed, 75 insertions(+), 59 deletions(-) diff --git a/spring-integration-security/src/main/java/org/springframework/integration/security/config/ChannelSecurityInterceptorBeanPostProcessor.java b/spring-integration-security/src/main/java/org/springframework/integration/security/config/ChannelSecurityInterceptorBeanPostProcessor.java index aae289161e..1e590d5d4e 100644 --- a/spring-integration-security/src/main/java/org/springframework/integration/security/config/ChannelSecurityInterceptorBeanPostProcessor.java +++ b/spring-integration-security/src/main/java/org/springframework/integration/security/config/ChannelSecurityInterceptorBeanPostProcessor.java @@ -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 securityInterceptors; + private final Collection 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 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; } + } diff --git a/spring-integration-security/src/main/java/org/springframework/integration/security/config/SecurityIntegrationConfigurationInitializer.java b/spring-integration-security/src/main/java/org/springframework/integration/security/config/SecurityIntegrationConfigurationInitializer.java index 4c54b629fa..aa3491bd7d 100644 --- a/spring-integration-security/src/main/java/org/springframework/integration/security/config/SecurityIntegrationConfigurationInitializer.java +++ b/spring-integration-security/src/main/java/org/springframework/integration/security/config/SecurityIntegrationConfigurationInitializer.java @@ -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 securityInterceptors = new ManagedList(); + + 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); } } diff --git a/spring-integration-security/src/test/java/org/springframework/integration/security/channel/ChannelSecurityInterceptorBeanPostProcessorTests.java b/spring-integration-security/src/test/java/org/springframework/integration/security/channel/ChannelSecurityInterceptorBeanPostProcessorTests.java index ad683f8c83..e5c744e0af 100644 --- a/spring-integration-security/src/test/java/org/springframework/integration/security/channel/ChannelSecurityInterceptorBeanPostProcessorTests.java +++ b/spring-integration-security/src/test/java/org/springframework/integration/security/channel/ChannelSecurityInterceptorBeanPostProcessorTests.java @@ -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>() { - - @Override - public Map 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>() { + ChannelSecurityInterceptor interceptor = new ChannelSecurityInterceptor(securityMetadataSource); - @Override - public Map 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)); }