Revert gh-13783

This feature unfortunately regresses pre-existing behavior
like that found in gh-15352. As such, this functionality
has been removed.

Closes gh-15352
This commit is contained in:
Josh Cummings 2024-07-31 15:47:40 -06:00
parent f059c05c93
commit f20ae1a71c
No known key found for this signature in database
GPG Key ID: A306A51F43B8E5A5
14 changed files with 70 additions and 219 deletions

View File

@ -28,6 +28,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
import java.util.function.Supplier;
import jakarta.annotation.security.DenyAll;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.junit.jupiter.api.Test;
@ -50,6 +51,7 @@ import org.springframework.security.access.annotation.BusinessService;
import org.springframework.security.access.annotation.BusinessServiceImpl;
import org.springframework.security.access.annotation.ExpressionProtectedBusinessServiceImpl;
import org.springframework.security.access.annotation.Jsr250BusinessServiceImpl;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
@ -944,6 +946,13 @@ public class PrePostMethodSecurityConfigurationTests {
verify(handler, never()).handleDeniedInvocation(any(), any(Authz.AuthzResult.class));
}
// gh-15352
@Test
void annotationsInChildClassesDoNotAffectSuperclasses() {
this.spring.register(AbstractClassConfig.class).autowire();
this.spring.getContext().getBean(ClassInheritingAbstractClassWithNoAnnotations.class).method();
}
private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false);
}
@ -1480,4 +1489,29 @@ public class PrePostMethodSecurityConfigurationTests {
}
abstract static class AbstractClassWithNoAnnotations {
String method() {
return "ok";
}
}
@PreAuthorize("denyAll()")
@Secured("DENIED")
@DenyAll
static class ClassInheritingAbstractClassWithNoAnnotations extends AbstractClassWithNoAnnotations {
}
@EnableMethodSecurity(securedEnabled = true, jsr250Enabled = true)
static class AbstractClassConfig {
@Bean
ClassInheritingAbstractClassWithNoAnnotations inheriting() {
return new ClassInheritingAbstractClassWithNoAnnotations();
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -35,7 +35,6 @@ import org.springframework.util.Assert;
* For internal use only, as this contract is likely to change
*
* @author Evgeniy Cheban
* @author DingHao
*/
abstract class AbstractExpressionAttributeRegistry<T extends ExpressionAttribute> {
@ -100,8 +99,4 @@ abstract class AbstractExpressionAttributeRegistry<T extends ExpressionAttribute
@NonNull
abstract T resolveAttribute(Method method, Class<?> targetClass);
Class<?> targetClass(Method method, Class<?> targetClass) {
return (targetClass != null) ? targetClass : method.getDeclaringClass();
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -44,7 +44,6 @@ import org.springframework.util.Assert;
*
* @author Evgeniy Cheban
* @author Josh Cummings
* @author DingHao
* @since 5.6
*/
public final class Jsr250AuthorizationManager implements AuthorizationManager<MethodInvocation> {
@ -122,8 +121,7 @@ public final class Jsr250AuthorizationManager implements AuthorizationManager<Me
private Annotation findJsr250Annotation(Method method, Class<?> targetClass) {
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
Annotation annotation = findAnnotation(specificMethod);
return (annotation != null) ? annotation
: findAnnotation((targetClass != null) ? targetClass : specificMethod.getDeclaringClass());
return (annotation != null) ? annotation : findAnnotation(specificMethod.getDeclaringClass());
}
private Annotation findAnnotation(Method method) {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -33,7 +33,6 @@ import org.springframework.util.Assert;
* For internal use only, as this contract is likely to change.
*
* @author Evgeniy Cheban
* @author DingHao
* @since 5.8
*/
final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionAttributeRegistry<ExpressionAttribute> {
@ -50,33 +49,33 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA
@Override
ExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
PostAuthorize postAuthorize = findPostAuthorizeAnnotation(specificMethod, targetClass);
PostAuthorize postAuthorize = findPostAuthorizeAnnotation(specificMethod);
if (postAuthorize == null) {
return ExpressionAttribute.NULL_ATTRIBUTE;
}
Expression expression = getExpressionHandler().getExpressionParser().parseExpression(postAuthorize.value());
MethodAuthorizationDeniedHandler deniedHandler = resolveHandler(method, targetClass);
MethodAuthorizationDeniedHandler deniedHandler = resolveHandler(method);
return new PostAuthorizeExpressionAttribute(expression, deniedHandler);
}
private MethodAuthorizationDeniedHandler resolveHandler(Method method, Class<?> targetClass) {
private MethodAuthorizationDeniedHandler resolveHandler(Method method) {
Function<AnnotatedElement, HandleAuthorizationDenied> lookup = AuthorizationAnnotationUtils
.withDefaults(HandleAuthorizationDenied.class);
HandleAuthorizationDenied deniedHandler = lookup.apply(method);
if (deniedHandler != null) {
return this.handlerResolver.apply(deniedHandler.handlerClass());
}
deniedHandler = lookup.apply(targetClass(method, targetClass));
deniedHandler = lookup.apply(method.getDeclaringClass());
if (deniedHandler != null) {
return this.handlerResolver.apply(deniedHandler.handlerClass());
}
return this.defaultHandler;
}
private PostAuthorize findPostAuthorizeAnnotation(Method method, Class<?> targetClass) {
private PostAuthorize findPostAuthorizeAnnotation(Method method) {
Function<AnnotatedElement, PostAuthorize> lookup = findUniqueAnnotation(PostAuthorize.class);
PostAuthorize postAuthorize = lookup.apply(method);
return (postAuthorize != null) ? postAuthorize : lookup.apply(targetClass(method, targetClass));
return (postAuthorize != null) ? postAuthorize : lookup.apply(method.getDeclaringClass());
}
/**

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -29,7 +29,6 @@ import org.springframework.security.access.prepost.PostFilter;
* For internal use only, as this contract is likely to change.
*
* @author Evgeniy Cheban
* @author DingHao
* @since 5.8
*/
final class PostFilterExpressionAttributeRegistry extends AbstractExpressionAttributeRegistry<ExpressionAttribute> {
@ -38,7 +37,7 @@ final class PostFilterExpressionAttributeRegistry extends AbstractExpressionAttr
@Override
ExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
PostFilter postFilter = findPostFilterAnnotation(specificMethod, targetClass);
PostFilter postFilter = findPostFilterAnnotation(specificMethod);
if (postFilter == null) {
return ExpressionAttribute.NULL_ATTRIBUTE;
}
@ -47,10 +46,10 @@ final class PostFilterExpressionAttributeRegistry extends AbstractExpressionAttr
return new ExpressionAttribute(postFilterExpression);
}
private PostFilter findPostFilterAnnotation(Method method, Class<?> targetClass) {
private PostFilter findPostFilterAnnotation(Method method) {
Function<AnnotatedElement, PostFilter> lookup = findUniqueAnnotation(PostFilter.class);
PostFilter postFilter = lookup.apply(method);
return (postFilter != null) ? postFilter : lookup.apply(targetClass(method, targetClass));
return (postFilter != null) ? postFilter : lookup.apply(method.getDeclaringClass());
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -33,7 +33,6 @@ import org.springframework.util.Assert;
* For internal use only, as this contract is likely to change.
*
* @author Evgeniy Cheban
* @author DingHao
* @since 5.8
*/
final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAttributeRegistry<ExpressionAttribute> {
@ -50,33 +49,33 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt
@Override
ExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
PreAuthorize preAuthorize = findPreAuthorizeAnnotation(specificMethod, targetClass);
PreAuthorize preAuthorize = findPreAuthorizeAnnotation(specificMethod);
if (preAuthorize == null) {
return ExpressionAttribute.NULL_ATTRIBUTE;
}
Expression expression = getExpressionHandler().getExpressionParser().parseExpression(preAuthorize.value());
MethodAuthorizationDeniedHandler handler = resolveHandler(method, targetClass);
MethodAuthorizationDeniedHandler handler = resolveHandler(method);
return new PreAuthorizeExpressionAttribute(expression, handler);
}
private MethodAuthorizationDeniedHandler resolveHandler(Method method, Class<?> targetClass) {
private MethodAuthorizationDeniedHandler resolveHandler(Method method) {
Function<AnnotatedElement, HandleAuthorizationDenied> lookup = AuthorizationAnnotationUtils
.withDefaults(HandleAuthorizationDenied.class);
HandleAuthorizationDenied deniedHandler = lookup.apply(method);
if (deniedHandler != null) {
return this.handlerResolver.apply(deniedHandler.handlerClass());
}
deniedHandler = lookup.apply(targetClass(method, targetClass));
deniedHandler = lookup.apply(method.getDeclaringClass());
if (deniedHandler != null) {
return this.handlerResolver.apply(deniedHandler.handlerClass());
}
return this.defaultHandler;
}
private PreAuthorize findPreAuthorizeAnnotation(Method method, Class<?> targetClass) {
private PreAuthorize findPreAuthorizeAnnotation(Method method) {
Function<AnnotatedElement, PreAuthorize> lookup = findUniqueAnnotation(PreAuthorize.class);
PreAuthorize preAuthorize = lookup.apply(method);
return (preAuthorize != null) ? preAuthorize : lookup.apply(targetClass(method, targetClass));
return (preAuthorize != null) ? preAuthorize : lookup.apply(method.getDeclaringClass());
}
/**

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -29,7 +29,6 @@ import org.springframework.security.access.prepost.PreFilter;
* For internal use only, as this contract is likely to change.
*
* @author Evgeniy Cheban
* @author DingHao
* @since 5.8
*/
final class PreFilterExpressionAttributeRegistry
@ -39,7 +38,7 @@ final class PreFilterExpressionAttributeRegistry
@Override
PreFilterExpressionAttribute resolveAttribute(Method method, Class<?> targetClass) {
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
PreFilter preFilter = findPreFilterAnnotation(specificMethod, targetClass);
PreFilter preFilter = findPreFilterAnnotation(specificMethod);
if (preFilter == null) {
return PreFilterExpressionAttribute.NULL_ATTRIBUTE;
}
@ -48,10 +47,10 @@ final class PreFilterExpressionAttributeRegistry
return new PreFilterExpressionAttribute(preFilterExpression, preFilter.filterTarget());
}
private PreFilter findPreFilterAnnotation(Method method, Class<?> targetClass) {
private PreFilter findPreFilterAnnotation(Method method) {
Function<AnnotatedElement, PreFilter> lookup = findUniqueAnnotation(PreFilter.class);
PreFilter preFilter = lookup.apply(method);
return (preFilter != null) ? preFilter : lookup.apply(targetClass(method, targetClass));
return (preFilter != null) ? preFilter : lookup.apply(method.getDeclaringClass());
}
static final class PreFilterExpressionAttribute extends ExpressionAttribute {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -41,7 +41,6 @@ import org.springframework.util.Assert;
* contains a specified authority from the Spring Security's {@link Secured} annotation.
*
* @author Evgeniy Cheban
* @author DingHao
* @since 5.6
*/
public final class SecuredAuthorizationManager implements AuthorizationManager<MethodInvocation> {
@ -87,14 +86,14 @@ public final class SecuredAuthorizationManager implements AuthorizationManager<M
private Set<String> resolveAuthorities(Method method, Class<?> targetClass) {
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
Secured secured = findSecuredAnnotation(specificMethod, targetClass);
Secured secured = findSecuredAnnotation(specificMethod);
return (secured != null) ? Set.of(secured.value()) : Collections.emptySet();
}
private Secured findSecuredAnnotation(Method method, Class<?> targetClass) {
private Secured findSecuredAnnotation(Method method) {
Secured secured = AuthorizationAnnotationUtils.findUniqueAnnotation(method, Secured.class);
return (secured != null) ? secured : AuthorizationAnnotationUtils
.findUniqueAnnotation((targetClass != null) ? targetClass : method.getDeclaringClass(), Secured.class);
return (secured != null) ? secured
: AuthorizationAnnotationUtils.findUniqueAnnotation(method.getDeclaringClass(), Secured.class);
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -225,56 +225,6 @@ public class Jsr250AuthorizationManagerTests {
.isThrownBy(() -> manager.check(authentication, methodInvocation));
}
@Test
public void checkRequiresUserWhenMethodsFromInheritThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new RolesAllowedClass(),
RolesAllowedClass.class, "securedUser");
Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
assertThat(decision.isGranted()).isTrue();
}
@Test
public void checkPermitAllWhenMethodsFromInheritThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PermitAllClass(), PermitAllClass.class,
"securedUser");
Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
assertThat(decision.isGranted()).isTrue();
}
@Test
public void checkDenyAllWhenMethodsFromInheritThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new DenyAllClass(), DenyAllClass.class,
"securedUser");
Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
assertThat(decision.isGranted()).isFalse();
}
@RolesAllowed("USER")
public static class RolesAllowedClass extends ParentClass {
}
@PermitAll
public static class PermitAllClass extends ParentClass {
}
@DenyAll
public static class DenyAllClass extends ParentClass {
}
public static class ParentClass {
public void securedUser() {
}
}
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
public void doSomething() {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -167,29 +167,6 @@ public class PostAuthorizeAuthorizationManagerTests {
.isThrownBy(() -> manager.check(authentication, result));
}
@Test
public void checkRequiresUserWhenMethodsFromInheritThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PostAuthorizeClass(),
PostAuthorizeClass.class, "securedUser");
MethodInvocationResult result = new MethodInvocationResult(methodInvocation, null);
PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, result);
assertThat(decision.isGranted()).isTrue();
}
@PostAuthorize("hasRole('USER')")
public static class PostAuthorizeClass extends ParentClass {
}
public static class ParentClass {
public void securedUser() {
}
}
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
public void doSomething() {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -170,34 +170,6 @@ public class PostFilterAuthorizationMethodInterceptorTests {
SecurityContextHolder.setContextHolderStrategy(saved);
}
@Test
public void checkPostFilterWhenMethodsFromInheritThenApplies() throws Throwable {
String[] array = { "john", "bob" };
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PostFilterClass(), PostFilterClass.class,
"inheritMethod", new Class[] { String[].class }, new Object[] { array }) {
@Override
public Object proceed() {
return array;
}
};
PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor();
Object result = advice.invoke(methodInvocation);
assertThat(result).asInstanceOf(InstanceOfAssertFactories.array(String[].class)).containsOnly("john");
}
@PostFilter("filterObject == 'john'")
public static class PostFilterClass extends ParentClass {
}
public static class ParentClass {
public String[] inheritMethod(String[] array) {
return array;
}
}
@PostFilter("filterObject == 'john'")
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -147,28 +147,6 @@ public class PreAuthorizeAuthorizationManagerTests {
assertThat(decision.isGranted()).isTrue();
}
@Test
public void checkRequiresUserWhenMethodsFromInheritThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new PreAuthorizeClass(),
PreAuthorizeClass.class, "securedUser");
PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
assertThat(decision.isGranted()).isTrue();
}
@PreAuthorize("hasRole('USER')")
public static class PreAuthorizeClass extends ParentClass {
}
public static class ParentClass {
public void securedUser() {
}
}
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
public void doSomething() {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -224,32 +224,6 @@ public class PreFilterAuthorizationMethodInterceptorTests {
SecurityContextHolder.setContextHolderStrategy(saved);
}
@Test
public void checkPreFilterWhenMethodsFromInheritThenApplies() throws Throwable {
List<String> list = new ArrayList<>();
list.add("john");
list.add("bob");
MockMethodInvocation invocation = new MockMethodInvocation(new PreFilterClass(), PreFilterClass.class,
"inheritMethod", new Class[] { List.class }, new Object[] { list });
PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor();
advice.invoke(invocation);
assertThat(list).hasSize(1);
assertThat(list.get(0)).isEqualTo("john");
}
@PreFilter("filterObject == 'john'")
public static class PreFilterClass extends ParentClass {
}
public static class ParentClass {
public void inheritMethod(List<String> list) {
}
}
@PreFilter("filterObject == 'john'")
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -167,28 +167,6 @@ public class SecuredAuthorizationManagerTests {
assertThat(decision.isGranted()).isTrue();
}
@Test
public void checkRequiresUserWhenMethodsFromInheritThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new SecuredSonClass(), SecuredSonClass.class,
"securedUser");
SecuredAuthorizationManager manager = new SecuredAuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
assertThat(decision.isGranted()).isTrue();
}
@Secured("ROLE_USER")
public static class SecuredSonClass extends ParentClass {
}
public static class ParentClass {
public void securedUser() {
}
}
public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {
public void doSomething() {