Skip to content

Commit ae72f13

Browse files
committed
Sort Default Advisors and Added Advisors
This commit ensures that the default advisors and added advisors are sorted in the event that this component is not being published as a Spring bean. Issue gh-16819
1 parent 5b7baee commit ae72f13

File tree

2 files changed

+40
-6
lines changed

2 files changed

+40
-6
lines changed

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.SortedSet;
3535
import java.util.TreeMap;
3636
import java.util.TreeSet;
37+
import java.util.concurrent.atomic.AtomicBoolean;
3738
import java.util.function.Supplier;
3839
import java.util.stream.Stream;
3940

@@ -91,6 +92,8 @@ public final class AuthorizationAdvisorProxyFactory
9192

9293
private TargetVisitor visitor = DEFAULT_VISITOR;
9394

95+
private final AtomicBoolean sorted = new AtomicBoolean(false);
96+
9497
private AuthorizationAdvisorProxyFactory(List<AuthorizationAdvisor> advisors) {
9598
this.advisors = new ArrayList<>(advisors);
9699
this.advisors.add(new AuthorizeReturnObjectMethodInterceptor(this));
@@ -109,7 +112,9 @@ public static AuthorizationAdvisorProxyFactory withDefaults() {
109112
advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize());
110113
advisors.add(new PreFilterAuthorizationMethodInterceptor());
111114
advisors.add(new PostFilterAuthorizationMethodInterceptor());
112-
return new AuthorizationAdvisorProxyFactory(advisors);
115+
AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisors);
116+
AnnotationAwareOrderComparator.sort(factory.advisors);
117+
return factory;
113118
}
114119

115120
/**
@@ -124,7 +129,9 @@ public static AuthorizationAdvisorProxyFactory withReactiveDefaults() {
124129
advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize());
125130
advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor());
126131
advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor());
127-
return new AuthorizationAdvisorProxyFactory(advisors);
132+
AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisors);
133+
AnnotationAwareOrderComparator.sort(factory.advisors);
134+
return factory;
128135
}
129136

130137
@Override
@@ -160,9 +167,9 @@ public Object proxy(Object target) {
160167
return proxied;
161168
}
162169
ProxyFactory factory = new ProxyFactory(target);
163-
for (Advisor advisor : this.advisors) {
164-
factory.addAdvisors(advisor);
165-
}
170+
List<Advisor> advisors = new ArrayList<>(this.advisors);
171+
AnnotationAwareOrderComparator.sort(advisors);
172+
factory.addAdvisors(advisors);
166173
factory.setProxyTargetClass(!Modifier.isFinal(target.getClass().getModifiers()));
167174
return factory.getProxy();
168175
}

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -38,6 +38,7 @@
3838
import org.junit.jupiter.api.Test;
3939

4040
import org.springframework.aop.Pointcut;
41+
import org.springframework.core.annotation.AnnotationAwareOrderComparator;
4142
import org.springframework.security.access.AccessDeniedException;
4243
import org.springframework.security.access.prepost.PreAuthorize;
4344
import org.springframework.security.authentication.TestAuthentication;
@@ -336,6 +337,32 @@ public void setTargetVisitorIgnoreValueTypesThenIgnores() {
336337
assertThat(factory.proxy(35)).isEqualTo(35);
337338
}
338339

340+
// gh-16819
341+
@Test
342+
void advisorsWhenWithDefaultsThenAreSorted() {
343+
AuthorizationAdvisorProxyFactory proxyFactory = AuthorizationAdvisorProxyFactory.withDefaults();
344+
AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE;
345+
AuthorizationAdvisor previous = null;
346+
for (AuthorizationAdvisor advisor : proxyFactory) {
347+
boolean ordered = previous == null || comparator.compare(previous, advisor) < 0;
348+
assertThat(ordered).isTrue();
349+
previous = advisor;
350+
}
351+
}
352+
353+
// gh-16819
354+
@Test
355+
void advisorsWhenWithReactiveDefaultsThenAreSorted() {
356+
AuthorizationAdvisorProxyFactory proxyFactory = AuthorizationAdvisorProxyFactory.withReactiveDefaults();
357+
AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE;
358+
AuthorizationAdvisor previous = null;
359+
for (AuthorizationAdvisor advisor : proxyFactory) {
360+
boolean ordered = previous == null || comparator.compare(previous, advisor) < 0;
361+
assertThat(ordered).isTrue();
362+
previous = advisor;
363+
}
364+
}
365+
339366
private Authentication authenticated(String user, String... authorities) {
340367
return TestAuthentication.authenticated(TestAuthentication.withUsername(user).authorities(authorities).build());
341368
}

0 commit comments

Comments
 (0)