Skip to content

Commit 59ec1f6

Browse files
committed
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
1 parent 12a9f92 commit 59ec1f6

File tree

3 files changed

+14
-62
lines changed

3 files changed

+14
-62
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,22 @@ static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider
4141
ObjectProvider<Customizer<AuthorizationAdvisorProxyFactory>> customizers) {
4242
List<AuthorizationAdvisor> advisors = new ArrayList<>();
4343
provider.forEach(advisors::add);
44-
AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisors);
44+
AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults();
4545
customizers.forEach((c) -> c.customize(factory));
46+
factory.setAdvisors(advisors);
4647
return factory;
4748
}
4849

4950
@Bean
5051
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
51-
static MethodInterceptor authorizeReturnObjectMethodInterceptor(
52+
static MethodInterceptor authorizeReturnObjectMethodInterceptor(ObjectProvider<AuthorizationAdvisor> provider,
5253
AuthorizationAdvisorProxyFactory authorizationProxyFactory) {
5354
AuthorizeReturnObjectMethodInterceptor interceptor = new AuthorizeReturnObjectMethodInterceptor(
5455
authorizationProxyFactory);
55-
authorizationProxyFactory.addAdvisors(interceptor);
56+
List<AuthorizationAdvisor> advisors = new ArrayList<>();
57+
provider.forEach(advisors::add);
58+
advisors.add(interceptor);
59+
authorizationProxyFactory.setAdvisors(advisors);
5660
return interceptor;
5761
}
5862

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

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -86,28 +86,14 @@ public final class AuthorizationAdvisorProxyFactory
8686
private static final TargetVisitor DEFAULT_VISITOR_SKIP_VALUE_TYPES = TargetVisitor.of(new ClassVisitor(),
8787
new IgnoreValueTypeVisitor(), DEFAULT_VISITOR);
8888

89-
private List<AuthorizationAdvisor> advisors = new ArrayList<>();
89+
private List<AuthorizationAdvisor> advisors;
9090

9191
private TargetVisitor visitor = DEFAULT_VISITOR;
9292

93-
/**
94-
* Construct an {@link AuthorizationAdvisorProxyFactory} with the following advisors
95-
* @param advisors the list of advisors to wrap around proxied objects
96-
* @since 6.4
97-
*/
98-
public AuthorizationAdvisorProxyFactory(List<AuthorizationAdvisor> advisors) {
99-
this.advisors.addAll(advisors);
100-
AnnotationAwareOrderComparator.sort(this.advisors);
101-
}
102-
103-
/**
104-
* Construct an {@link AuthorizationAdvisorProxyFactory} with the following advisors
105-
* @param advisors the list of advisors to wrap around proxied objects
106-
* @since 6.4
107-
*/
108-
public AuthorizationAdvisorProxyFactory(AuthorizationAdvisor... advisors) {
109-
this.advisors.addAll(List.of(advisors));
110-
AnnotationAwareOrderComparator.sort(this.advisors);
93+
private AuthorizationAdvisorProxyFactory(List<AuthorizationAdvisor> advisors) {
94+
this.advisors = new ArrayList<>(advisors);
95+
this.advisors.add(new AuthorizeReturnObjectMethodInterceptor(this));
96+
setAdvisors(this.advisors);
11197
}
11298

11399
/**
@@ -122,9 +108,7 @@ public static AuthorizationAdvisorProxyFactory withDefaults() {
122108
advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize());
123109
advisors.add(new PreFilterAuthorizationMethodInterceptor());
124110
advisors.add(new PostFilterAuthorizationMethodInterceptor());
125-
AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors);
126-
proxyFactory.addAdvisors(new AuthorizeReturnObjectMethodInterceptor(proxyFactory));
127-
return proxyFactory;
111+
return new AuthorizationAdvisorProxyFactory(advisors);
128112
}
129113

130114
/**
@@ -139,9 +123,7 @@ public static AuthorizationAdvisorProxyFactory withReactiveDefaults() {
139123
advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize());
140124
advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor());
141125
advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor());
142-
AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors);
143-
proxyFactory.addAdvisors(new AuthorizeReturnObjectMethodInterceptor(proxyFactory));
144-
return proxyFactory;
126+
return new AuthorizationAdvisorProxyFactory(advisors);
145127
}
146128

147129
/**
@@ -179,21 +161,13 @@ public Object proxy(Object target) {
179161
return factory.getProxy();
180162
}
181163

182-
public void addAdvisors(AuthorizationAdvisor... advisors) {
183-
this.advisors.addAll(List.of(advisors));
184-
AnnotationAwareOrderComparator.sort(this.advisors);
185-
}
186-
187164
/**
188165
* Add advisors that should be included to each proxy created.
189166
*
190167
* <p>
191168
* All advisors are re-sorted by their advisor order.
192169
* @param advisors the advisors to add
193-
* @deprecated Either use the constructor to provide a complete set of advisors or use
194-
* {@link #addAdvisors(AuthorizationAdvisor...)} to add to the existing list
195170
*/
196-
@Deprecated
197171
public void setAdvisors(AuthorizationAdvisor... advisors) {
198172
this.advisors = new ArrayList<>(List.of(advisors));
199173
AnnotationAwareOrderComparator.sort(this.advisors);
@@ -205,10 +179,7 @@ public void setAdvisors(AuthorizationAdvisor... advisors) {
205179
* <p>
206180
* All advisors are re-sorted by their advisor order.
207181
* @param advisors the advisors to add
208-
* @deprecated Either use the constructor to provide a complete set of advisors or use
209-
* {@link #addAdvisors(AuthorizationAdvisor...)} to add to the existing list
210182
*/
211-
@Deprecated
212183
public void setAdvisors(Collection<AuthorizationAdvisor> advisors) {
213184
this.advisors = new ArrayList<>(advisors);
214185
AnnotationAwareOrderComparator.sort(this.advisors);

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -319,29 +319,6 @@ public void setAdvisorsWhenProxyThenVisits() {
319319
verify(advisor, atLeastOnce()).getPointcut();
320320
}
321321

322-
@Test
323-
public void addAdvisorsWhenProxyThenVisits() {
324-
AuthorizationAdvisor advisor = mock(AuthorizationAdvisor.class);
325-
given(advisor.getAdvice()).willReturn(advisor);
326-
given(advisor.getPointcut()).willReturn(Pointcut.TRUE);
327-
AuthorizationAdvisorProxyFactory factory = AuthorizationAdvisorProxyFactory.withDefaults();
328-
factory.addAdvisors(advisor);
329-
Flight flight = proxy(factory, this.flight);
330-
flight.getAltitude();
331-
verify(advisor, atLeastOnce()).getPointcut();
332-
}
333-
334-
@Test
335-
public void constructWhenProxyThenVisitsAdvisors() {
336-
AuthorizationAdvisor advisor = mock(AuthorizationAdvisor.class);
337-
given(advisor.getAdvice()).willReturn(advisor);
338-
given(advisor.getPointcut()).willReturn(Pointcut.TRUE);
339-
AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisor);
340-
Flight flight = proxy(factory, this.flight);
341-
flight.getAltitude();
342-
verify(advisor, atLeastOnce()).getPointcut();
343-
}
344-
345322
@Test
346323
public void setTargetVisitorThenUses() {
347324
TargetVisitor visitor = mock(TargetVisitor.class);

0 commit comments

Comments
 (0)