Browse Source

Revert "Polish AuthorizationAdvisorProxyFactory advisor configuration"

This commit had some unintended consequences when the advisor
interceptor was published in a Spring Boot application. As such,
15497 will be reopened to investigate. In the meantime, this commit
reverts the previous change so as to allow the build to pass.

Issue gh-15497
Josh Cummings 1 năm trước cách đây
mục cha
commit
59ec1f6480

+ 7 - 3
config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java

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

+ 7 - 36
core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java

@@ -86,28 +86,14 @@ 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 = new ArrayList<>();
+	private List<AuthorizationAdvisor> advisors;
 
 	private TargetVisitor visitor = DEFAULT_VISITOR;
 
-	/**
-	 * 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);
+	private AuthorizationAdvisorProxyFactory(List<AuthorizationAdvisor> advisors) {
+		this.advisors = new ArrayList<>(advisors);
+		this.advisors.add(new AuthorizeReturnObjectMethodInterceptor(this));
+		setAdvisors(this.advisors);
 	}
 
 	/**
@@ -122,9 +108,7 @@ public final class AuthorizationAdvisorProxyFactory
 		advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize());
 		advisors.add(new PreFilterAuthorizationMethodInterceptor());
 		advisors.add(new PostFilterAuthorizationMethodInterceptor());
-		AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors);
-		proxyFactory.addAdvisors(new AuthorizeReturnObjectMethodInterceptor(proxyFactory));
-		return proxyFactory;
+		return new AuthorizationAdvisorProxyFactory(advisors);
 	}
 
 	/**
@@ -139,9 +123,7 @@ public final class AuthorizationAdvisorProxyFactory
 		advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize());
 		advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor());
 		advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor());
-		AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors);
-		proxyFactory.addAdvisors(new AuthorizeReturnObjectMethodInterceptor(proxyFactory));
-		return proxyFactory;
+		return new AuthorizationAdvisorProxyFactory(advisors);
 	}
 
 	/**
@@ -179,21 +161,13 @@ public final class AuthorizationAdvisorProxyFactory
 		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);
@@ -205,10 +179,7 @@ public final class AuthorizationAdvisorProxyFactory
 	 * <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);

+ 0 - 23
core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java

@@ -319,29 +319,6 @@ public class AuthorizationAdvisorProxyFactoryTests {
 		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);