Skip to content

Commit

Permalink
Produce Exactly One AuthorizationAdvisor Per Annotation
Browse files Browse the repository at this point in the history
Closes gh-15592
  • Loading branch information
jzheaux committed Aug 19, 2024
1 parent 27af1df commit ae8e4d1
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,24 @@

package org.springframework.security.config.annotation.method.configuration;

import org.aopalliance.aop.Advice;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import org.springframework.aop.Advisor;
import org.springframework.aop.Pointcut;
import org.springframework.aop.PointcutAdvisor;
import org.springframework.aop.framework.AopInfrastructureBean;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.context.annotation.ImportBeanDefinitionRegistrar;
import org.springframework.core.Ordered;
import org.springframework.core.type.AnnotationMetadata;
import org.springframework.security.authorization.method.AuthorizationAdvisor;

class MethodSecurityAdvisorRegistrar implements ImportBeanDefinitionRegistrar {

Expand Down Expand Up @@ -49,9 +61,49 @@ private void registerAsAdvisor(String prefix, BeanDefinitionRegistry registry) {
if (!(definition instanceof RootBeanDefinition)) {
return;
}
RootBeanDefinition advisor = new RootBeanDefinition((RootBeanDefinition) definition);
BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(AdvisorWrapper.class);
builder.setFactoryMethod("of");
builder.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
builder.addConstructorArgReference(interceptorName);
RootBeanDefinition advisor = (RootBeanDefinition) builder.getBeanDefinition();
advisor.setTargetType(Advisor.class);
registry.registerBeanDefinition(prefix + "Advisor", advisor);
registry.registerBeanDefinition(advisorName, advisor);
}

public static final class AdvisorWrapper
implements PointcutAdvisor, MethodInterceptor, Ordered, AopInfrastructureBean {

private final AuthorizationAdvisor advisor;

private AdvisorWrapper(AuthorizationAdvisor advisor) {
this.advisor = advisor;
}

public static AdvisorWrapper of(AuthorizationAdvisor advisor) {
return new AdvisorWrapper(advisor);
}

@Override
public Advice getAdvice() {
return this.advisor.getAdvice();
}

@Override
public Pointcut getPointcut() {
return this.advisor.getPointcut();
}

@Override
public int getOrder() {
return this.advisor.getOrder();
}

@Nullable
@Override
public Object invoke(@NotNull MethodInvocation invocation) throws Throwable {
return this.advisor.invoke(invocation);
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@
import org.junit.jupiter.api.extension.ExtendWith;

import org.springframework.aop.Advisor;
import org.springframework.aop.config.AopConfigUtils;
import org.springframework.aop.support.DefaultPointcutAdvisor;
import org.springframework.aop.support.JdkRegexpMethodPointcut;
import org.springframework.beans.factory.FactoryBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor;
import org.springframework.context.annotation.AdviceMode;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand All @@ -63,6 +66,7 @@
import org.springframework.security.authorization.AuthorizationDecision;
import org.springframework.security.authorization.AuthorizationEventPublisher;
import org.springframework.security.authorization.AuthorizationManager;
import org.springframework.security.authorization.method.AuthorizationAdvisor;
import org.springframework.security.authorization.method.AuthorizationAdvisorProxyFactory;
import org.springframework.security.authorization.method.AuthorizationAdvisorProxyFactory.TargetVisitor;
import org.springframework.security.authorization.method.AuthorizationInterceptorsOrder;
Expand All @@ -82,6 +86,7 @@
import org.springframework.security.test.context.support.WithAnonymousUser;
import org.springframework.security.test.context.support.WithMockUser;
import org.springframework.security.test.context.support.WithSecurityContextTestExecutionListener;
import org.springframework.stereotype.Component;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.TestExecutionListeners;
import org.springframework.test.context.junit.jupiter.SpringExtension;
Expand Down Expand Up @@ -953,6 +958,32 @@ void annotationsInChildClassesDoNotAffectSuperclasses() {
this.spring.getContext().getBean(ClassInheritingAbstractClassWithNoAnnotations.class).method();
}

// gh-15592
@Test
void autowireWhenDefaultsThenCreatesExactlyOneAdvisorPerAnnotation() {
this.spring.register(MethodSecurityServiceConfig.class).autowire();
AuthorizationAdvisorProxyFactory proxyFactory = this.spring.getContext()
.getBean(AuthorizationAdvisorProxyFactory.class);
assertThat(proxyFactory).hasSize(5);
assertThat(this.spring.getContext().getBeanNamesForType(AuthorizationAdvisor.class)).hasSize(5)
.containsExactlyInAnyOrder("preFilterAuthorizationMethodInterceptor",
"preAuthorizeAuthorizationMethodInterceptor", "postAuthorizeAuthorizationMethodInterceptor",
"postFilterAuthorizationMethodInterceptor", "authorizeReturnObjectMethodInterceptor");
}

// gh-15592
@Test
void autowireWhenAspectJAutoProxyAndFactoryBeanThenExactlyOneAdvisorPerAnnotation() {
this.spring.register(AspectJAwareAutoProxyAndFactoryBeansConfig.class).autowire();
AuthorizationAdvisorProxyFactory proxyFactory = this.spring.getContext()
.getBean(AuthorizationAdvisorProxyFactory.class);
assertThat(proxyFactory).hasSize(5);
assertThat(this.spring.getContext().getBeanNamesForType(AuthorizationAdvisor.class)).hasSize(5)
.containsExactlyInAnyOrder("preFilterAuthorizationMethodInterceptor",
"preAuthorizeAuthorizationMethodInterceptor", "postAuthorizeAuthorizationMethodInterceptor",
"postFilterAuthorizationMethodInterceptor", "authorizeReturnObjectMethodInterceptor");
}

private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false);
}
Expand Down Expand Up @@ -1514,4 +1545,30 @@ ClassInheritingAbstractClassWithNoAnnotations inheriting() {

}

@Configuration
@EnableMethodSecurity
static class AspectJAwareAutoProxyAndFactoryBeansConfig {

@Bean
static BeanDefinitionRegistryPostProcessor beanDefinitionRegistryPostProcessor() {
return AopConfigUtils::registerAspectJAnnotationAutoProxyCreatorIfNecessary;
}

@Component
static class MyFactoryBean implements FactoryBean<Object> {

@Override
public Object getObject() throws Exception {
return new Object();
}

@Override
public Class<?> getObjectType() {
return Object.class;
}

}

}

}

0 comments on commit ae8e4d1

Please sign in to comment.