diff --git a/config/config.gradle b/config/config.gradle index aeed6ce299..4e4689e758 100644 --- a/config/config.gradle +++ b/config/config.gradle @@ -33,9 +33,12 @@ dependencies { "org.springframework.ldap:spring-ldap-core:$springLdapVersion", "org.springframework:spring-expression:$springVersion", "org.springframework:spring-jdbc:$springVersion", + "org.springframework:spring-orm:$springVersion", "org.springframework:spring-tx:$springVersion", 'org.spockframework:spock-core:0.6-groovy-1.8', "org.slf4j:jcl-over-slf4j:$slf4jVersion", + "org.hibernate.javax.persistence:hibernate-jpa-2.0-api:1.0.1.Final", + "org.hibernate:hibernate-entitymanager:4.1.0.Final", powerMockDependencies testCompile('org.openid4java:openid4java-nodeps:0.9.6') { exclude group: 'com.google.code.guice', module: 'guice' diff --git a/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java index 88e5547223..6ab33386b8 100644 --- a/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java @@ -10,6 +10,8 @@ import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.aop.config.AopNamespaceUtils; +import org.springframework.aop.framework.ProxyFactoryBean; +import org.springframework.aop.target.LazyInitTargetSource; import org.springframework.beans.BeanMetadataElement; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; @@ -17,15 +19,19 @@ import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanReference; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.parsing.BeanComponentDefinition; import org.springframework.beans.factory.parsing.CompositeComponentDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; import org.springframework.beans.factory.support.ManagedList; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.security.access.ConfigAttribute; +import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.access.SecurityConfig; import org.springframework.security.access.annotation.Jsr250MethodSecurityMetadataSource; import org.springframework.security.access.annotation.Jsr250Voter; @@ -34,6 +40,7 @@ import org.springframework.security.access.expression.method.DefaultMethodSecuri import org.springframework.security.access.expression.method.ExpressionBasedAnnotationAttributeFactory; import org.springframework.security.access.expression.method.ExpressionBasedPostInvocationAdvice; import org.springframework.security.access.expression.method.ExpressionBasedPreInvocationAdvice; +import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; import org.springframework.security.access.intercept.AfterInvocationProviderManager; import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor; import org.springframework.security.access.intercept.aopalliance.MethodSecurityMetadataSourceAdvisor; @@ -139,6 +146,20 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP if (StringUtils.hasText(expressionHandlerRef)) { logger.info("Using bean '" + expressionHandlerRef + "' as method ExpressionHandler implementation"); + RootBeanDefinition lazyInitPP = new RootBeanDefinition(LazyInitBeanDefinitionRegistryPostProcessor.class); + lazyInitPP.getConstructorArgumentValues().addGenericArgumentValue(expressionHandlerRef); + pc.getReaderContext().registerWithGeneratedName(lazyInitPP); + + BeanDefinitionBuilder lazyMethodSecurityExpressionHandlerBldr = BeanDefinitionBuilder.rootBeanDefinition(LazyInitTargetSource.class); + lazyMethodSecurityExpressionHandlerBldr.addPropertyValue("targetBeanName", expressionHandlerRef); + + BeanDefinitionBuilder expressionHandlerProxyBldr = BeanDefinitionBuilder.rootBeanDefinition(ProxyFactoryBean.class); + expressionHandlerProxyBldr.addPropertyValue("targetSource", lazyMethodSecurityExpressionHandlerBldr.getBeanDefinition()); + expressionHandlerProxyBldr.addPropertyValue("proxyInterfaces", MethodSecurityExpressionHandler.class); + + expressionHandlerRef = pc.getReaderContext().generateBeanName(expressionHandlerProxyBldr.getBeanDefinition()); + + pc.registerBeanComponent(new BeanComponentDefinition(expressionHandlerProxyBldr.getBeanDefinition(), expressionHandlerRef)); } else { BeanDefinition expressionHandler = new RootBeanDefinition(DefaultMethodSecurityExpressionHandler.class); expressionHandlerRef = pc.getReaderContext().generateBeanName(expressionHandler); @@ -401,4 +422,26 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP this.beanFactory = beanFactory; } } + + /** + * Delays setting a bean of a given name to be lazyily initialized until after all the beans are registered. + * + * @author Rob Winch + * @since 3.2 + */ + private static final class LazyInitBeanDefinitionRegistryPostProcessor implements BeanDefinitionRegistryPostProcessor { + private final String beanName; + + private LazyInitBeanDefinitionRegistryPostProcessor(String beanName) { + this.beanName = beanName; + } + + public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { + BeanDefinition beanDefinition = registry.getBeanDefinition(beanName); + beanDefinition.setLazyInit(true); + } + + public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { + } + } } diff --git a/config/src/test/java/org/springframework/security/config/method/sec2136/JpaPermissionEvaluator.java b/config/src/test/java/org/springframework/security/config/method/sec2136/JpaPermissionEvaluator.java new file mode 100644 index 0000000000..c4edf57cda --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/method/sec2136/JpaPermissionEvaluator.java @@ -0,0 +1,48 @@ +/* + * Copyright 2002-2013 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.security.config.method.sec2136; + +import java.io.Serializable; + +import javax.persistence.EntityManager; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.PermissionEvaluator; +import org.springframework.security.core.Authentication; + +/** + * + * @author Rob Winch + * + */ +public class JpaPermissionEvaluator implements PermissionEvaluator { + @Autowired + private EntityManager entityManager; + + public JpaPermissionEvaluator() { + System.out.println("initializing "+this); + } + + public boolean hasPermission(Authentication authentication, + Object targetDomainObject, Object permission) { + return true; + } + + public boolean hasPermission(Authentication authentication, + Serializable targetId, String targetType, Object permission) { + return true; + } +} diff --git a/config/src/test/java/org/springframework/security/config/method/sec2136/Sec2136Tests.java b/config/src/test/java/org/springframework/security/config/method/sec2136/Sec2136Tests.java new file mode 100644 index 0000000000..95aff97909 --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/method/sec2136/Sec2136Tests.java @@ -0,0 +1,36 @@ +/* + * Copyright 2002-2013 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.security.config.method.sec2136; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * + * @author Rob Winch + * @since 3.2 + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration("sec2136.xml") +public class Sec2136Tests { + + @Test + public void configurationLoads() { + + } +} diff --git a/config/src/test/resources/org/springframework/security/config/method/sec2136/sec2136.xml b/config/src/test/resources/org/springframework/security/config/method/sec2136/sec2136.xml new file mode 100644 index 0000000000..efd213362b --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/method/sec2136/sec2136.xml @@ -0,0 +1,55 @@ + + + + + + + + + + + + + + + + + + create-drop + true + false + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedAnnotationAttributeFactory.java b/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedAnnotationAttributeFactory.java index 1617b8014c..e201374b55 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedAnnotationAttributeFactory.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/ExpressionBasedAnnotationAttributeFactory.java @@ -1,5 +1,17 @@ -/** +/* + * Copyright 2002-2013 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.security.access.expression.method; @@ -15,18 +27,22 @@ import org.springframework.security.access.prepost.PrePostInvocationAttributeFac * an expression to be evaluated at runtime. * * @author Luke Taylor + * @author Rob Winch * @since 3.0 */ public class ExpressionBasedAnnotationAttributeFactory implements PrePostInvocationAttributeFactory { - private final ExpressionParser parser; + private final Object parserLock = new Object(); + private ExpressionParser parser; + private MethodSecurityExpressionHandler handler; public ExpressionBasedAnnotationAttributeFactory(MethodSecurityExpressionHandler handler) { - parser = handler.getExpressionParser(); + this.handler = handler; } public PreInvocationAttribute createPreInvocationAttribute(String preFilterAttribute, String filterObject, String preAuthorizeAttribute) { try { - // TODO: Optimization of permitAll + // TODO: Optimization of permitAll + ExpressionParser parser = getParser(); Expression preAuthorizeExpression = preAuthorizeAttribute == null ? parser.parseExpression("permitAll") : parser.parseExpression(preAuthorizeAttribute); Expression preFilterExpression = preFilterAttribute == null ? null : parser.parseExpression(preFilterAttribute); return new PreInvocationExpressionAttribute(preFilterExpression, filterObject, preAuthorizeExpression); @@ -37,6 +53,7 @@ public class ExpressionBasedAnnotationAttributeFactory implements PrePostInvocat public PostInvocationAttribute createPostInvocationAttribute(String postFilterAttribute, String postAuthorizeAttribute) { try { + ExpressionParser parser = getParser(); Expression postAuthorizeExpression = postAuthorizeAttribute == null ? null : parser.parseExpression(postAuthorizeAttribute); Expression postFilterExpression = postFilterAttribute == null ? null : parser.parseExpression(postFilterAttribute); @@ -49,4 +66,20 @@ public class ExpressionBasedAnnotationAttributeFactory implements PrePostInvocat return null; } + + /** + * Delay the lookup of the {@link ExpressionParser} to prevent SEC-2136 + * + * @return + */ + private ExpressionParser getParser() { + if(this.parser != null) { + return this.parser; + } + synchronized(parserLock) { + this.parser = handler.getExpressionParser(); + this.handler = null; + } + return this.parser; + } }