From f398be793d91200c3723d5a8863693ef5acc4679 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Wed, 14 Aug 2024 17:23:45 -0600 Subject: [PATCH] Simplify AuthorizationAdvisorProxyFactory Configuration Closes gh-15497 --- .../AuthorizationProxyConfiguration.java | 14 ++---- .../AuthorizationAdvisorProxyFactory.java | 45 ++++++++++++++++--- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java index a4e29505a7..0467a0301a 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java @@ -17,7 +17,6 @@ package org.springframework.security.config.annotation.method.configuration; import java.util.ArrayList; -import java.util.List; import org.aopalliance.intercept.MethodInterceptor; @@ -37,13 +36,10 @@ final class AuthorizationProxyConfiguration implements AopInfrastructureBean { @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider provider, + static AuthorizationAdvisorProxyFactory authorizationProxyFactory( ObjectProvider> customizers) { - List advisors = new ArrayList<>(); - provider.forEach(advisors::add); - AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults(); + AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(new ArrayList<>()); customizers.forEach((c) -> c.customize(factory)); - factory.setAdvisors(advisors); return factory; } @@ -51,12 +47,10 @@ final class AuthorizationProxyConfiguration implements AopInfrastructureBean { @Role(BeanDefinition.ROLE_INFRASTRUCTURE) static MethodInterceptor authorizeReturnObjectMethodInterceptor(ObjectProvider provider, AuthorizationAdvisorProxyFactory authorizationProxyFactory) { + provider.forEach(authorizationProxyFactory::addAdvisor); AuthorizeReturnObjectMethodInterceptor interceptor = new AuthorizeReturnObjectMethodInterceptor( authorizationProxyFactory); - List advisors = new ArrayList<>(); - provider.forEach(advisors::add); - advisors.add(interceptor); - authorizationProxyFactory.setAdvisors(advisors); + authorizationProxyFactory.addAdvisor(interceptor); return interceptor; } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java index 42248b3da2..9876f5e836 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java @@ -41,6 +41,7 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.aop.Advisor; +import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.aop.framework.ProxyFactory; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.lang.NonNull; @@ -75,7 +76,7 @@ import org.springframework.util.ClassUtils; * @since 6.3 */ public final class AuthorizationAdvisorProxyFactory - implements AuthorizationProxyFactory, Iterable { + implements AuthorizationProxyFactory, Iterable, AopInfrastructureBean { private static final boolean isReactivePresent = ClassUtils.isPresent("reactor.core.publisher.Mono", null); @@ -90,10 +91,18 @@ public final class AuthorizationAdvisorProxyFactory private TargetVisitor visitor = DEFAULT_VISITOR; - private AuthorizationAdvisorProxyFactory(List advisors) { + /** + * Construct an {@link AuthorizationAdvisorProxyFactory} with the provided advisors. + * + *

+ * The list may be empty, in the case where advisors are added later using + * {@link #addAdvisor}. + * @param advisors the advisors to use + * @since 6.4 + */ + public AuthorizationAdvisorProxyFactory(List advisors) { this.advisors = new ArrayList<>(advisors); - this.advisors.add(new AuthorizeReturnObjectMethodInterceptor(this)); - setAdvisors(this.advisors); + AnnotationAwareOrderComparator.sort(this.advisors); } /** @@ -108,7 +117,9 @@ public final class AuthorizationAdvisorProxyFactory advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize()); advisors.add(new PreFilterAuthorizationMethodInterceptor()); advisors.add(new PostFilterAuthorizationMethodInterceptor()); - return new AuthorizationAdvisorProxyFactory(advisors); + AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors); + proxyFactory.addAdvisor(new AuthorizeReturnObjectMethodInterceptor(proxyFactory)); + return proxyFactory; } /** @@ -123,7 +134,9 @@ public final class AuthorizationAdvisorProxyFactory advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize()); advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor()); advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor()); - return new AuthorizationAdvisorProxyFactory(advisors); + AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors); + proxyFactory.addAdvisor(new AuthorizeReturnObjectMethodInterceptor(proxyFactory)); + return proxyFactory; } /** @@ -167,7 +180,9 @@ public final class AuthorizationAdvisorProxyFactory *

* All advisors are re-sorted by their advisor order. * @param advisors the advisors to add + * @deprecated Please use {@link #addAdvisor} instead */ + @Deprecated public void setAdvisors(AuthorizationAdvisor... advisors) { this.advisors = new ArrayList<>(List.of(advisors)); AnnotationAwareOrderComparator.sort(this.advisors); @@ -179,12 +194,30 @@ public final class AuthorizationAdvisorProxyFactory *

* All advisors are re-sorted by their advisor order. * @param advisors the advisors to add + * @deprecated Please use {@link #addAdvisor} instead */ + @Deprecated public void setAdvisors(Collection advisors) { this.advisors = new ArrayList<>(advisors); AnnotationAwareOrderComparator.sort(this.advisors); } + /** + * Add an advisor that should be included to each proxy created. + * + *

+ * This method sorts the advisors based on the order in + * {@link AuthorizationAdvisor#getOrder}. You can use the values in + * {@link AuthorizationInterceptorsOrder}to ensure advisors are located where you need + * them. + * @param advisor + * @since 6.4 + */ + public void addAdvisor(AuthorizationAdvisor advisor) { + this.advisors.add(advisor); + AnnotationAwareOrderComparator.sort(this.advisors); + } + /** * Use this visitor to navigate the proxy target's hierarchy. *