Skip to content

Commit

Permalink
Ensure that class proxying is forced before AutoProxyCreator is created
Browse files Browse the repository at this point in the history
Previously, using @EnableGlobalMethodSecurity would cause the
AutoProxyCreator to be created before the AOP auto-configuration had
called AopUtils.forceAutoProxyCreatorToUseClassProxying. Forcing
auto proxy creation changes the AutoProxyCreator's bean definition
so it has no effect when attempted after the creator has been created.

This commit updates the AOP auto-configuration to use a
BeanFactoryPostProcessor to force the use of class proxying. This
ensures that the changes to the auto proxy creator's bean definition
are in place before any bean creation has been performed.

Fixes gh-25413
  • Loading branch information
wilkinsona committed Mar 31, 2021
1 parent e5fa9ad commit 7aabd8b
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 9 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2021 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.
Expand All @@ -19,11 +19,12 @@
import org.aspectj.weaver.Advice;

import org.springframework.aop.config.AopConfigUtils;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.EnableAspectJAutoProxy;

Expand Down Expand Up @@ -73,12 +74,15 @@ static class CglibAutoProxyConfiguration {
matchIfMissing = true)
static class ClassProxyingConfiguration {

ClassProxyingConfiguration(BeanFactory beanFactory) {
if (beanFactory instanceof BeanDefinitionRegistry) {
BeanDefinitionRegistry registry = (BeanDefinitionRegistry) beanFactory;
AopConfigUtils.registerAutoProxyCreatorIfNecessary(registry);
AopConfigUtils.forceAutoProxyCreatorToUseClassProxying(registry);
}
@Bean
static BeanFactoryPostProcessor forceAutoProxyCreatorToUseClassProxying() {
return (beanFactory) -> {
if (beanFactory instanceof BeanDefinitionRegistry) {
BeanDefinitionRegistry registry = (BeanDefinitionRegistry) beanFactory;
AopConfigUtils.registerAutoProxyCreatorIfNecessary(registry);
AopConfigUtils.forceAutoProxyCreatorToUseClassProxying(registry);
}
};
}

}
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2021 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.
Expand All @@ -18,16 +18,22 @@

import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.aspectj.weaver.Advice;
import org.junit.jupiter.api.Test;

import org.springframework.aop.support.AopUtils;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.FilteredClassLoader;
import org.springframework.boot.test.context.assertj.AssertableApplicationContext;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.boot.test.context.runner.ContextConsumer;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.EnableAspectJAutoProxy;
import org.springframework.context.annotation.Import;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
import org.springframework.web.bind.annotation.RequestMapping;

import static org.assertj.core.api.Assertions.assertThat;

Expand Down Expand Up @@ -74,6 +80,17 @@ void aopWithDisabledProxyTargetClass() {
@Test
void customConfigurationWithProxyTargetClassDefaultDoesNotDisableProxying() {
this.contextRunner.withUserConfiguration(CustomTestConfiguration.class).run(proxyTargetClassEnabled());

}

@Test
void whenGlobalMethodSecurityIsEnabledAndAspectJIsNotAvailableThenClassProxyingIsStillUsedByDefault() {
this.contextRunner.withClassLoader(new FilteredClassLoader(Advice.class))
.withUserConfiguration(ExampleController.class, EnableGlobalMethodSecurityConfiguration.class)
.run((context) -> {
ExampleController exampleController = context.getBean(ExampleController.class);
assertThat(AopUtils.isCglibProxy(exampleController)).isTrue();
});
}

private ContextConsumer<AssertableApplicationContext> proxyTargetClassEnabled() {
Expand Down Expand Up @@ -149,4 +166,25 @@ interface TestInterface {

}

@EnableGlobalMethodSecurity(prePostEnabled = true)
@Configuration(proxyBeanMethods = false)
static class EnableGlobalMethodSecurityConfiguration {

}

public static class ExampleController implements TestInterface {

@RequestMapping("/test")
@PreAuthorize("true")
String demo() {
return "test";
}

@Override
public void foo() {

}

}

}

0 comments on commit 7aabd8b

Please sign in to comment.