From 10dc72b017e813f411c987fb1c04ae820024129c Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 19 Feb 2010 00:38:24 +0000 Subject: [PATCH] SEC-1387: Support serialization of security advised beans. MethodSecurityMetadataSourceAdvisor now takes the SecurityMetadataSource bean name as an extra constructor argument and re-obtains the bean from the BeanFactory in its readObject method. Beans that are advised using should therefore now be serializable. --- ...balMethodSecurityBeanDefinitionParser.java | 1 + ...tationDrivenBeanDefinitionParserTests.java | 42 ++++++++++++++++++ .../MethodSecurityMetadataSourceAdvisor.java | 43 ++++++++++++------- .../access/annotation/BusinessService.java | 6 +-- 4 files changed, 73 insertions(+), 19 deletions(-) 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 774155ff73..7c04988414 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 @@ -324,6 +324,7 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP advisor.setSource(source); advisor.getConstructorArgumentValues().addGenericArgumentValue(interceptor.getBeanName()); advisor.getConstructorArgumentValues().addGenericArgumentValue(metadataSource); + advisor.getConstructorArgumentValues().addGenericArgumentValue(metadataSource.getBeanName()); parserContext.getRegistry().registerBeanDefinition(BeanIds.METHOD_SECURITY_METADATA_SOURCE_ADVISOR, advisor); } diff --git a/config/src/test/java/org/springframework/security/config/method/SecuredAnnotationDrivenBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/method/SecuredAnnotationDrivenBeanDefinitionParserTests.java index 6c2209a336..d9148f5ea8 100644 --- a/config/src/test/java/org/springframework/security/config/method/SecuredAnnotationDrivenBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/method/SecuredAnnotationDrivenBeanDefinitionParserTests.java @@ -1,11 +1,18 @@ package org.springframework.security.config.method; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; + import org.junit.After; import org.junit.Before; import org.junit.Test; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.annotation.BusinessService; import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; +import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.config.ConfigTestUtils; import org.springframework.security.config.util.InMemoryXmlApplicationContext; @@ -60,4 +67,39 @@ public class SecuredAnnotationDrivenBeanDefinitionParserTests { target.someAdminMethod(); } + + // SEC-1387 + @Test(expected=AuthenticationCredentialsNotFoundException.class) + public void targetIsSerializableBeforeUse() throws Exception { + BusinessService chompedTarget = (BusinessService) serializeAndDeserialize(target); + chompedTarget.someAdminMethod(); + } + + @Test(expected=AccessDeniedException.class) + public void targetIsSerializableAfterUse() throws Exception { + try { + target.someAdminMethod(); + } catch (AuthenticationCredentialsNotFoundException expected) { + } + SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("u","p","ROLE_A")); + + BusinessService chompedTarget = (BusinessService) serializeAndDeserialize(target); + chompedTarget.someAdminMethod(); + } + + private Object serializeAndDeserialize(Object o) throws IOException, ClassNotFoundException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream(baos); + oos.writeObject(o); + oos.flush(); + baos.flush(); + byte[] bytes = baos.toByteArray(); + + ByteArrayInputStream is = new ByteArrayInputStream(bytes); + ObjectInputStream ois = new ObjectInputStream(is); + Object o2 = ois.readObject(); + + return o2; + } + } diff --git a/core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityMetadataSourceAdvisor.java b/core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityMetadataSourceAdvisor.java index b534828eb2..b98c560aae 100644 --- a/core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityMetadataSourceAdvisor.java +++ b/core/src/main/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityMetadataSourceAdvisor.java @@ -15,6 +15,9 @@ package org.springframework.security.access.intercept.aopalliance; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.Serializable; import java.lang.reflect.AccessibleObject; import java.lang.reflect.Method; @@ -31,7 +34,7 @@ import org.springframework.util.Assert; /** * Advisor driven by a {@link MethodSecurityMetadataSource}, used to exclude a {@link MethodSecurityInterceptor} from - * public (ie non-secure) methods. + * public (non-secure) methods. *

* Because the AOP framework caches advice calculations, this is normally faster than just letting the * MethodSecurityInterceptor run and find out itself that it has no work to do. @@ -44,23 +47,25 @@ import org.springframework.util.Assert; * Based on Spring's TransactionAttributeSourceAdvisor. * * @author Ben Alex + * @author Luke Taylor */ public class MethodSecurityMetadataSourceAdvisor extends AbstractPointcutAdvisor implements BeanFactoryAware { //~ Instance fields ================================================================================================ - private MethodSecurityMetadataSource attributeSource; - private MethodSecurityInterceptor interceptor; - private Pointcut pointcut = new MethodSecurityMetadataSourcePointcut(); + private transient MethodSecurityMetadataSource attributeSource; + private transient MethodSecurityInterceptor interceptor; + private final Pointcut pointcut = new MethodSecurityMetadataSourcePointcut(); private BeanFactory beanFactory; private String adviceBeanName; - private final Object adviceMonitor = new Object(); + private String metadataSourceBeanName; + private final Serializable adviceMonitor = new Serializable() {}; //~ Constructors =================================================================================================== /** * @deprecated use the decoupled approach instead */ - public MethodSecurityMetadataSourceAdvisor(MethodSecurityInterceptor advice) { + MethodSecurityMetadataSourceAdvisor(MethodSecurityInterceptor advice) { Assert.notNull(advice.getSecurityMetadataSource(), "Cannot construct a MethodSecurityMetadataSourceAdvisor using a " + "MethodSecurityInterceptor that has no SecurityMetadataSource configured"); @@ -71,21 +76,22 @@ public class MethodSecurityMetadataSourceAdvisor extends AbstractPointcutAdvisor /** * Alternative constructor for situations where we want the advisor decoupled from the advice. Instead the advice * bean name should be set. This prevents eager instantiation of the interceptor - * (and hence the AuthenticationManager). See SEC-773, for example. - *

- * This is essentially the approach taken by subclasses of Spring's {@code AbstractBeanFactoryPointcutAdvisor}, - * which this class should extend in future. The original hierarchy and constructor have been retained for backwards - * compatibility. + * (and hence the AuthenticationManager). See SEC-773, for example. The metadataSourceBeanName is used rather than + * a direct reference to support serialization via a bean factory lookup. * * @param adviceBeanName name of the MethodSecurityInterceptor bean - * @param attributeSource the attribute source (should be the same as the one used on the interceptor) + * @param attributeSource the SecurityMetadataSource (should be the same as the one used on the interceptor) + * @param attributeSourceBeanName the bean name of the attributeSource (required for serialization) */ - public MethodSecurityMetadataSourceAdvisor(String adviceBeanName, MethodSecurityMetadataSource attributeSource) { + public MethodSecurityMetadataSourceAdvisor(String adviceBeanName, MethodSecurityMetadataSource attributeSource, + String attributeSourceBeanName) { Assert.notNull(adviceBeanName, "The adviceBeanName cannot be null"); Assert.notNull(attributeSource, "The attributeSource cannot be null"); + Assert.notNull(attributeSourceBeanName, "The attributeSourceBeanName cannot be null"); this.adviceBeanName = adviceBeanName; this.attributeSource = attributeSource; + this.metadataSourceBeanName = attributeSourceBeanName; } //~ Methods ======================================================================================================== @@ -99,8 +105,7 @@ public class MethodSecurityMetadataSourceAdvisor extends AbstractPointcutAdvisor if (interceptor == null) { Assert.notNull(adviceBeanName, "'adviceBeanName' must be set for use with bean factory lookup."); Assert.state(beanFactory != null, "BeanFactory must be set to resolve 'adviceBeanName'"); - interceptor = (MethodSecurityInterceptor) - beanFactory.getBean(this.adviceBeanName, MethodSecurityInterceptor.class); + interceptor = beanFactory.getBean(this.adviceBeanName, MethodSecurityInterceptor.class); } return interceptor; } @@ -110,9 +115,15 @@ public class MethodSecurityMetadataSourceAdvisor extends AbstractPointcutAdvisor this.beanFactory = beanFactory; } + private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException { + ois.defaultReadObject(); + + attributeSource = beanFactory.getBean(metadataSourceBeanName, MethodSecurityMetadataSource.class); + } + //~ Inner Classes ================================================================================================== - class MethodSecurityMetadataSourcePointcut extends StaticMethodMatcherPointcut { + class MethodSecurityMetadataSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { @SuppressWarnings("unchecked") public boolean matches(Method m, Class targetClass) { return attributeSource.getAttributes(m, targetClass) != null; diff --git a/core/src/test/java/org/springframework/security/access/annotation/BusinessService.java b/core/src/test/java/org/springframework/security/access/annotation/BusinessService.java index 29c000d3bb..8c15effe1a 100644 --- a/core/src/test/java/org/springframework/security/access/annotation/BusinessService.java +++ b/core/src/test/java/org/springframework/security/access/annotation/BusinessService.java @@ -15,19 +15,19 @@ package org.springframework.security.access.annotation; +import java.io.Serializable; import java.util.List; -import javax.annotation.security.RolesAllowed; import javax.annotation.security.PermitAll; +import javax.annotation.security.RolesAllowed; -import org.springframework.security.access.annotation.Secured; import org.springframework.security.access.prepost.PreAuthorize; /** */ @Secured({"ROLE_USER"}) @PermitAll -public interface BusinessService { +public interface BusinessService extends Serializable { //~ Methods ======================================================================================================== @Secured({"ROLE_ADMIN"})