Polish AuthorizationAdvisorProxyFactory advisor configuration

Closes gh-15497

Author: jzheaux

config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java

@@ -41,22 +41,18 @@ static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider
 			ObjectProvider<Customizer<AuthorizationAdvisorProxyFactory>> customizers) {
 		List<AuthorizationAdvisor> advisors = new ArrayList<>();
 		provider.forEach(advisors::add);
-		AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults();
+		AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisors);
 		customizers.forEach((c) -> c.customize(factory));
-		factory.setAdvisors(advisors);
 		return factory;
 	}
 
 	@Bean
 	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
-	static MethodInterceptor authorizeReturnObjectMethodInterceptor(ObjectProvider<AuthorizationAdvisor> provider,
+	static MethodInterceptor authorizeReturnObjectMethodInterceptor(
 			AuthorizationAdvisorProxyFactory authorizationProxyFactory) {
 		AuthorizeReturnObjectMethodInterceptor interceptor = new AuthorizeReturnObjectMethodInterceptor(
 				authorizationProxyFactory);
-		List<AuthorizationAdvisor> advisors = new ArrayList<>();
-		provider.forEach(advisors::add);
-		advisors.add(interceptor);
-		authorizationProxyFactory.setAdvisors(advisors);
+		authorizationProxyFactory.addAdvisors(interceptor);
 		return interceptor;
 	}
 

core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java

@@ -86,14 +86,28 @@ public final class AuthorizationAdvisorProxyFactory
 	private static final TargetVisitor DEFAULT_VISITOR_SKIP_VALUE_TYPES = TargetVisitor.of(new ClassVisitor(),
 			new IgnoreValueTypeVisitor(), DEFAULT_VISITOR);
 
-	private List<AuthorizationAdvisor> advisors;
+	private List<AuthorizationAdvisor> advisors = new ArrayList<>();
 
 	private TargetVisitor visitor = DEFAULT_VISITOR;
 
-	private AuthorizationAdvisorProxyFactory(List<AuthorizationAdvisor> advisors) {
-		this.advisors = new ArrayList<>(advisors);
-		this.advisors.add(new AuthorizeReturnObjectMethodInterceptor(this));
-		setAdvisors(this.advisors);
+	/**
+	 * Construct an {@link AuthorizationAdvisorProxyFactory} with the following advisors
+	 * @param advisors the list of advisors to wrap around proxied objects
+	 * @since 6.4
+	 */
+	public AuthorizationAdvisorProxyFactory(List<AuthorizationAdvisor> advisors) {
+		this.advisors.addAll(advisors);
+		AnnotationAwareOrderComparator.sort(this.advisors);
+	}
+
+	/**
+	 * Construct an {@link AuthorizationAdvisorProxyFactory} with the following advisors
+	 * @param advisors the list of advisors to wrap around proxied objects
+	 * @since 6.4
+	 */
+	public AuthorizationAdvisorProxyFactory(AuthorizationAdvisor... advisors) {
+		this.advisors.addAll(List.of(advisors));
+		AnnotationAwareOrderComparator.sort(this.advisors);
 	}
 
 	/**
@@ -108,7 +122,9 @@ public static AuthorizationAdvisorProxyFactory withDefaults() {
 		advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize());
 		advisors.add(new PreFilterAuthorizationMethodInterceptor());
 		advisors.add(new PostFilterAuthorizationMethodInterceptor());
-		return new AuthorizationAdvisorProxyFactory(advisors);
+		AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors);
+		proxyFactory.addAdvisors(new AuthorizeReturnObjectMethodInterceptor(proxyFactory));
+		return proxyFactory;
 	}
 
 	/**
@@ -123,7 +139,9 @@ public static AuthorizationAdvisorProxyFactory withReactiveDefaults() {
 		advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize());
 		advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor());
 		advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor());
-		return new AuthorizationAdvisorProxyFactory(advisors);
+		AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors);
+		proxyFactory.addAdvisors(new AuthorizeReturnObjectMethodInterceptor(proxyFactory));
+		return proxyFactory;
 	}
 
 	/**
@@ -161,13 +179,21 @@ public Object proxy(Object target) {
 		return factory.getProxy();
 	}
 
+	public void addAdvisors(AuthorizationAdvisor... advisors) {
+		this.advisors.addAll(List.of(advisors));
+		AnnotationAwareOrderComparator.sort(this.advisors);
+	}
+
 	/**
 	 * Add advisors that should be included to each proxy created.
 	 *
 	 * <p>
 	 * All advisors are re-sorted by their advisor order.
 	 * @param advisors the advisors to add
+	 * @deprecated Either use the constructor to provide a complete set of advisors or use
+	 * {@link #addAdvisors(AuthorizationAdvisor...)} to add to the existing list
 	 */
+	@Deprecated
 	public void setAdvisors(AuthorizationAdvisor... advisors) {
 		this.advisors = new ArrayList<>(List.of(advisors));
 		AnnotationAwareOrderComparator.sort(this.advisors);
@@ -179,7 +205,10 @@ public void setAdvisors(AuthorizationAdvisor... advisors) {
 	 * <p>
 	 * All advisors are re-sorted by their advisor order.
 	 * @param advisors the advisors to add
+	 * @deprecated Either use the constructor to provide a complete set of advisors or use
+	 * {@link #addAdvisors(AuthorizationAdvisor...)} to add to the existing list
 	 */
+	@Deprecated
 	public void setAdvisors(Collection<AuthorizationAdvisor> advisors) {
 		this.advisors = new ArrayList<>(advisors);
 		AnnotationAwareOrderComparator.sort(this.advisors);

core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java

@@ -319,6 +319,29 @@ public void setAdvisorsWhenProxyThenVisits() {
 		verify(advisor, atLeastOnce()).getPointcut();
 	}
 
+	@Test
+	public void addAdvisorsWhenProxyThenVisits() {
+		AuthorizationAdvisor advisor = mock(AuthorizationAdvisor.class);
+		given(advisor.getAdvice()).willReturn(advisor);
+		given(advisor.getPointcut()).willReturn(Pointcut.TRUE);
+		AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults();
+		factory.addAdvisors(advisor);
+		Flight flight = proxy(factory, this.flight);
+		flight.getAltitude();
+		verify(advisor, atLeastOnce()).getPointcut();
+	}
+
+	@Test
+	public void constructWhenProxyThenVisitsAdvisors() {
+		AuthorizationAdvisor advisor = mock(AuthorizationAdvisor.class);
+		given(advisor.getAdvice()).willReturn(advisor);
+		given(advisor.getPointcut()).willReturn(Pointcut.TRUE);
+		AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisor);
+		Flight flight = proxy(factory, this.flight);
+		flight.getAltitude();
+		verify(advisor, atLeastOnce()).getPointcut();
+	}
+
 	@Test
 	public void setTargetVisitorThenUses() {
 		TargetVisitor visitor = mock(TargetVisitor.class);