Skip to content

Commit 37a2812

Browse files
jzheauxjzheaux
committed
Mimic Annotation Fallback Logic
For backward compatibility, this commit changes the annotation traversal logic to match what is found in PrePostAnnotationSecurityMetadataSource. This reverts gh-13783 which is a feature that unfortunately regressess pre-existing behavior like that found in gh-15352. As such, that functionality has been removed. Issue gh-15352
1 parent 77bce14 commit 37a2812

15 files changed

+84
-192
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.function.Consumer;
2929
import java.util.function.Supplier;
3030

31+
import jakarta.annotation.security.DenyAll;
3132
import org.aopalliance.intercept.MethodInterceptor;
3233
import org.aopalliance.intercept.MethodInvocation;
3334
import org.junit.jupiter.api.Test;
@@ -50,6 +51,7 @@
5051
import org.springframework.security.access.annotation.BusinessServiceImpl;
5152
import org.springframework.security.access.annotation.ExpressionProtectedBusinessServiceImpl;
5253
import org.springframework.security.access.annotation.Jsr250BusinessServiceImpl;
54+
import org.springframework.security.access.annotation.Secured;
5355
import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
5456
import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
5557
import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
@@ -944,6 +946,13 @@ void getUserWhenNotAuthorizedThenHandlerUsesCustomAuthorizationDecision() {
944946
verify(handler, never()).handleDeniedInvocation(any(), any(Authz.AuthzResult.class));
945947
}
946948

949+
// gh-15352
950+
@Test
951+
void annotationsInChildClassesDoNotAffectSuperclasses() {
952+
this.spring.register(AbstractClassConfig.class).autowire();
953+
this.spring.getContext().getBean(ClassInheritingAbstractClassWithNoAnnotations.class).method();
954+
}
955+
947956
private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
948957
return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false);
949958
}
@@ -1480,4 +1489,29 @@ MethodAuthorizationDeniedHandler methodAuthorizationDeniedHandler() {
14801489

14811490
}
14821491

1492+
abstract static class AbstractClassWithNoAnnotations {
1493+
1494+
String method() {
1495+
return "ok";
1496+
}
1497+
1498+
}
1499+
1500+
@PreAuthorize("denyAll()")
1501+
@Secured("DENIED")
1502+
@DenyAll
1503+
static class ClassInheritingAbstractClassWithNoAnnotations extends AbstractClassWithNoAnnotations {
1504+
1505+
}
1506+
1507+
@EnableMethodSecurity(securedEnabled = true, jsr250Enabled = true)
1508+
static class AbstractClassConfig {
1509+
1510+
@Bean
1511+
ClassInheritingAbstractClassWithNoAnnotations inheriting() {
1512+
return new ClassInheritingAbstractClassWithNoAnnotations();
1513+
}
1514+
1515+
}
1516+
14831517
}

‎core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import jakarta.annotation.security.RolesAllowed;
3030
import org.aopalliance.intercept.MethodInvocation;
3131

32-
import org.springframework.aop.support.AopUtils;
3332
import org.springframework.lang.NonNull;
3433
import org.springframework.security.authorization.AuthoritiesAuthorizationManager;
3534
import org.springframework.security.authorization.AuthorizationDecision;
@@ -117,9 +116,8 @@ AuthorizationManager<MethodInvocation> resolveManager(Method method, Class<?> ta
117116
}
118117

119118
private Annotation findJsr250Annotation(Method method, Class<?> targetClass) {
120-
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
121-
Class<?> targetClassToUse = (targetClass != null) ? targetClass : specificMethod.getDeclaringClass();
122-
return this.synthesizer.synthesize(specificMethod, targetClassToUse);
119+
Class<?> targetClassToUse = (targetClass != null) ? targetClass : method.getDeclaringClass();
120+
return this.synthesizer.synthesize(method, targetClassToUse);
123121
}
124122

125123
private Set<String> getAllowedRolesWithPrefix(RolesAllowed rolesAllowed) {

‎core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import reactor.util.annotation.NonNull;
2424

25-
import org.springframework.aop.support.AopUtils;
2625
import org.springframework.context.ApplicationContext;
2726
import org.springframework.expression.Expression;
2827
import org.springframework.security.access.prepost.PostAuthorize;
@@ -56,8 +55,7 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA
5655
@NonNull
5756
@Override
5857
ExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
59-
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
60-
PostAuthorize postAuthorize = findPostAuthorizeAnnotation(specificMethod, targetClass);
58+
PostAuthorize postAuthorize = findPostAuthorizeAnnotation(method, targetClass);
6159
if (postAuthorize == null) {
6260
return ExpressionAttribute.NULL_ATTRIBUTE;
6361
}

‎core/src/main/java/org/springframework/security/authorization/method/PostFilterExpressionAttributeRegistry.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import java.lang.reflect.Method;
2020

21-
import org.springframework.aop.support.AopUtils;
2221
import org.springframework.expression.Expression;
2322
import org.springframework.lang.NonNull;
2423
import org.springframework.security.access.prepost.PostFilter;
@@ -39,8 +38,7 @@ final class PostFilterExpressionAttributeRegistry extends AbstractExpressionAttr
3938
@NonNull
4039
@Override
4140
ExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
42-
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
43-
PostFilter postFilter = findPostFilterAnnotation(specificMethod, targetClass);
41+
PostFilter postFilter = findPostFilterAnnotation(method, targetClass);
4442
if (postFilter == null) {
4543
return ExpressionAttribute.NULL_ATTRIBUTE;
4644
}

‎core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import reactor.util.annotation.NonNull;
2424

25-
import org.springframework.aop.support.AopUtils;
2625
import org.springframework.context.ApplicationContext;
2726
import org.springframework.expression.Expression;
2827
import org.springframework.security.access.prepost.PreAuthorize;
@@ -56,8 +55,7 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt
5655
@NonNull
5756
@Override
5857
ExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
59-
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
60-
PreAuthorize preAuthorize = findPreAuthorizeAnnotation(specificMethod, targetClass);
58+
PreAuthorize preAuthorize = findPreAuthorizeAnnotation(method, targetClass);
6159
if (preAuthorize == null) {
6260
return ExpressionAttribute.NULL_ATTRIBUTE;
6361
}

‎core/src/main/java/org/springframework/security/authorization/method/PreFilterExpressionAttributeRegistry.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import java.lang.reflect.Method;
2020

21-
import org.springframework.aop.support.AopUtils;
2221
import org.springframework.expression.Expression;
2322
import org.springframework.lang.NonNull;
2423
import org.springframework.security.access.prepost.PreFilter;
@@ -40,8 +39,7 @@ final class PreFilterExpressionAttributeRegistry
4039
@NonNull
4140
@Override
4241
PreFilterExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
43-
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
44-
PreFilter preFilter = findPreFilterAnnotation(specificMethod, targetClass);
42+
PreFilter preFilter = findPreFilterAnnotation(method, targetClass);
4543
if (preFilter == null) {
4644
return PreFilterExpressionAttribute.NULL_ATTRIBUTE;
4745
}

‎core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import org.aopalliance.intercept.MethodInvocation;
2828

29-
import org.springframework.aop.support.AopUtils;
3029
import org.springframework.core.MethodClassKey;
3130
import org.springframework.security.access.annotation.Secured;
3231
import org.springframework.security.authorization.AuthoritiesAuthorizationManager;
@@ -90,8 +89,7 @@ private Set<String> getAuthorities(MethodInvocation methodInvocation) {
9089
}
9190

9291
private Set<String> resolveAuthorities(Method method, Class<?> targetClass) {
93-
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
94-
Secured secured = findSecuredAnnotation(specificMethod, targetClass);
92+
Secured secured = findSecuredAnnotation(method, targetClass);
9593
return (secured != null) ? Set.of(secured.value()) : Collections.emptySet();
9694
}
9795

‎core/src/main/java/org/springframework/security/core/annotation/UniqueMergedAnnotationSynthesizer.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.springframework.core.annotation.MergedAnnotations;
3232
import org.springframework.core.annotation.RepeatableContainers;
3333
import org.springframework.util.Assert;
34+
import org.springframework.util.ClassUtils;
3435

3536
/**
3637
* A strategy for synthesizing an annotation from an {@link AnnotatedElement} that
@@ -124,11 +125,29 @@ private MergedAnnotation<A> requireUnique(AnnotatedElement element, List<MergedA
124125
}
125126

126127
private List<MergedAnnotation<A>> findMethodAnnotations(Method method, Class<?> targetClass) {
127-
List<MergedAnnotation<A>> annotations = findClosestMethodAnnotations(method, targetClass, new HashSet<>());
128+
// The method may be on an interface, but we need attributes from the target
129+
// class.
130+
// If the target class is null, the method will be unchanged.
131+
Method specificMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
132+
List<MergedAnnotation<A>> annotations = findClosestMethodAnnotations(specificMethod,
133+
specificMethod.getDeclaringClass(), new HashSet<>());
128134
if (!annotations.isEmpty()) {
129135
return annotations;
130136
}
131-
return findClosestClassAnnotations(targetClass, new HashSet<>());
137+
// Check the original (e.g. interface) method
138+
if (specificMethod != method) {
139+
annotations = findClosestMethodAnnotations(method, method.getDeclaringClass(), new HashSet<>());
140+
if (!annotations.isEmpty()) {
141+
return annotations;
142+
}
143+
}
144+
// Check the class-level (note declaringClass, not targetClass, which may not
145+
// actually implement the method)
146+
annotations = findClosestClassAnnotations(specificMethod.getDeclaringClass(), new HashSet<>());
147+
if (!annotations.isEmpty()) {
148+
return annotations;
149+
}
150+
return Collections.emptyList();
132151
}
133152

134153
private List<MergedAnnotation<A>> findClosestMethodAnnotations(Method method, Class<?> targetClass,

‎core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -215,56 +215,6 @@ public void checkInheritedAnnotationsWhenConflictingThenAnnotationConfigurationE
215215
.isThrownBy(() -> manager.check(authentication, methodInvocation));
216216
}
217217

218-
@Test
219-
public void checkRequiresUserWhenMethodsFromInheritThenApplies() throws Exception {
220-
MockMethodInvocation methodInvocation = new MockMethodInvocation(new RolesAllowedClass(),
221-
RolesAllowedClass.class, "securedUser");
222-
Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager();
223-
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
224-
assertThat(decision.isGranted()).isTrue();
225-
}
226-
227-
@Test
228-
public void checkPermitAllWhenMethodsFromInheritThenApplies() throws Exception {
229-
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PermitAllClass(), PermitAllClass.class,
230-
"securedUser");
231-
Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager();
232-
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
233-
assertThat(decision.isGranted()).isTrue();
234-
}
235-
236-
@Test
237-
public void checkDenyAllWhenMethodsFromInheritThenApplies() throws Exception {
238-
MockMethodInvocation methodInvocation = new MockMethodInvocation(new DenyAllClass(), DenyAllClass.class,
239-
"securedUser");
240-
Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager();
241-
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
242-
assertThat(decision.isGranted()).isFalse();
243-
}
244-
245-
@RolesAllowed("USER")
246-
public static class RolesAllowedClass extends ParentClass {
247-
248-
}
249-
250-
@PermitAll
251-
public static class PermitAllClass extends ParentClass {
252-
253-
}
254-
255-
@DenyAll
256-
public static class DenyAllClass extends ParentClass {
257-
258-
}
259-
260-
public static class ParentClass {
261-
262-
public void securedUser() {
263-
264-
}
265-
266-
}
267-
268218
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
269219

270220
public void doSomething() {

‎core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -156,29 +156,6 @@ public void checkInheritedAnnotationsWhenConflictingThenAnnotationConfigurationE
156156
.isThrownBy(() -> manager.check(authentication, result));
157157
}
158158

159-
@Test
160-
public void checkRequiresUserWhenMethodsFromInheritThenApplies() throws Exception {
161-
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PostAuthorizeClass(),
162-
PostAuthorizeClass.class, "securedUser");
163-
MethodInvocationResult result = new MethodInvocationResult(methodInvocation, null);
164-
PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
165-
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, result);
166-
assertThat(decision.isGranted()).isTrue();
167-
}
168-
169-
@PostAuthorize("hasRole('USER')")
170-
public static class PostAuthorizeClass extends ParentClass {
171-
172-
}
173-
174-
public static class ParentClass {
175-
176-
public void securedUser() {
177-
178-
}
179-
180-
}
181-
182159
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
183160

184161
public void doSomething() {

‎core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptorTests.java

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -161,34 +161,6 @@ public Object proceed() {
161161
SecurityContextHolder.setContextHolderStrategy(saved);
162162
}
163163

164-
@Test
165-
public void checkPostFilterWhenMethodsFromInheritThenApplies() throws Throwable {
166-
String[] array = { "john", "bob" };
167-
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PostFilterClass(), PostFilterClass.class,
168-
"inheritMethod", new Class[] { String[].class }, new Object[] { array }) {
169-
@Override
170-
public Object proceed() {
171-
return array;
172-
}
173-
};
174-
PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor();
175-
Object result = advice.invoke(methodInvocation);
176-
assertThat(result).asInstanceOf(InstanceOfAssertFactories.array(String[].class)).containsOnly("john");
177-
}
178-
179-
@PostFilter("filterObject == 'john'")
180-
public static class PostFilterClass extends ParentClass {
181-
182-
}
183-
184-
public static class ParentClass {
185-
186-
public String[] inheritMethod(String[] array) {
187-
return array;
188-
}
189-
190-
}
191-
192164
@PostFilter("filterObject == 'john'")
193165
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
194166

‎core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -137,28 +137,6 @@ public void checkTargetClassAwareWhenInterfaceLevelAnnotationsThenApplies() thro
137137
assertThat(decision.isGranted()).isTrue();
138138
}
139139

140-
@Test
141-
public void checkRequiresUserWhenMethodsFromInheritThenApplies() throws Exception {
142-
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PreAuthorizeClass(),
143-
PreAuthorizeClass.class, "securedUser");
144-
PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager();
145-
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
146-
assertThat(decision.isGranted()).isTrue();
147-
}
148-
149-
@PreAuthorize("hasRole('USER')")
150-
public static class PreAuthorizeClass extends ParentClass {
151-
152-
}
153-
154-
public static class ParentClass {
155-
156-
public void securedUser() {
157-
158-
}
159-
160-
}
161-
162140
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
163141

164142
public void doSomething() {

‎core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptorTests.java

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -215,32 +215,6 @@ public void preFilterWhenStaticSecurityContextHolderStrategyAfterConstructorThen
215215
SecurityContextHolder.setContextHolderStrategy(saved);
216216
}
217217

218-
@Test
219-
public void checkPreFilterWhenMethodsFromInheritThenApplies() throws Throwable {
220-
List<String> list = new ArrayList<>();
221-
list.add("john");
222-
list.add("bob");
223-
MockMethodInvocation invocation = new MockMethodInvocation(new PreFilterClass(), PreFilterClass.class,
224-
"inheritMethod", new Class[] { List.class }, new Object[] { list });
225-
PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor();
226-
advice.invoke(invocation);
227-
assertThat(list).hasSize(1);
228-
assertThat(list.get(0)).isEqualTo("john");
229-
}
230-
231-
@PreFilter("filterObject == 'john'")
232-
public static class PreFilterClass extends ParentClass {
233-
234-
}
235-
236-
public static class ParentClass {
237-
238-
public void inheritMethod(List<String> list) {
239-
240-
}
241-
242-
}
243-
244218
@PreFilter("filterObject == 'john'")
245219
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
246220

0 commit comments

Comments
 (0)