Polish AOP Structure

- Changed from MethodMatcher to Pointcut since authorization
annotations also can be attached to classes
- Adjusted advice to extend Before or AfterAdvice
- Adjusted advice to extend PointcutAdvisor so
that it can share its Pointcut
- Adjusted advice to extend AopInfrastructureBean to
align with old advice classes

Issue spring-projectsgh-9289

Author: jzheaux

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

@@ -17,40 +17,35 @@
 package org.springframework.security.config.annotation.method.configuration;
 
 import java.lang.annotation.Annotation;
-import java.lang.reflect.AnnotatedElement;
-import java.lang.reflect.Method;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import javax.annotation.security.DenyAll;
 import javax.annotation.security.PermitAll;
 import javax.annotation.security.RolesAllowed;
 
-import org.springframework.aop.MethodMatcher;
 import org.springframework.aop.Pointcut;
-import org.springframework.aop.support.AopUtils;
+import org.springframework.aop.support.ComposablePointcut;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
 import org.springframework.aop.support.Pointcuts;
-import org.springframework.aop.support.StaticMethodMatcher;
+import org.springframework.aop.support.annotation.AnnotationMatchingPointcut;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.ImportAware;
 import org.springframework.context.annotation.Role;
-import org.springframework.core.annotation.AnnotatedElementUtils;
 import org.springframework.core.annotation.AnnotationAttributes;
 import org.springframework.core.type.AnnotationMetadata;
 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.prepost.PostAuthorize;
+import org.springframework.security.access.prepost.PostFilter;
 import org.springframework.security.access.prepost.PreAuthorize;
+import org.springframework.security.access.prepost.PreFilter;
 import org.springframework.security.authorization.method.AuthorizationManagerMethodAfterAdvice;
 import org.springframework.security.authorization.method.AuthorizationManagerMethodBeforeAdvice;
 import org.springframework.security.authorization.method.AuthorizationMethodAfterAdvice;
@@ -72,6 +67,7 @@
  * Base {@link Configuration} for enabling Spring Security Method Security.
  *
  * @author Evgeniy Cheban
+ * @author Josh Cummings
  * @see EnableMethodSecurity
  * @since 5.5
  */
@@ -92,7 +88,9 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean
 	@Bean
 	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
 	DefaultPointcutAdvisor methodSecurityAdvisor(AuthorizationMethodInterceptor interceptor) {
-		Pointcut pointcut = Pointcuts.union(getAuthorizationMethodBeforeAdvice(), getAuthorizationMethodAfterAdvice());
+		AuthorizationMethodBeforeAdvice<?> beforeAdvice = getAuthorizationMethodBeforeAdvice();
+		AuthorizationMethodAfterAdvice<?> afterAdvice = getAuthorizationMethodAfterAdvice();
+		Pointcut pointcut = Pointcuts.union(beforeAdvice.getPointcut(), afterAdvice.getPointcut());
 		DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(pointcut, interceptor);
 		advisor.setOrder(order());
 		return advisor;
@@ -147,32 +145,34 @@ private AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> createDefaul
 	}
 
 	private PreFilterAuthorizationMethodBeforeAdvice getPreFilterAuthorizationMethodBeforeAdvice() {
-		PreFilterAuthorizationMethodBeforeAdvice preFilterBeforeAdvice = new PreFilterAuthorizationMethodBeforeAdvice();
+		Pointcut pointcut = forAnnotation(PreFilter.class);
+		PreFilterAuthorizationMethodBeforeAdvice preFilterBeforeAdvice = new PreFilterAuthorizationMethodBeforeAdvice(
+				pointcut);
 		preFilterBeforeAdvice.setExpressionHandler(getMethodSecurityExpressionHandler());
 		return preFilterBeforeAdvice;
 	}
 
 	private AuthorizationMethodBeforeAdvice<MethodAuthorizationContext> getPreAuthorizeAuthorizationMethodBeforeAdvice() {
-		MethodMatcher methodMatcher = new SecurityAnnotationsStaticMethodMatcher(PreAuthorize.class);
+		Pointcut pointcut = forAnnotation(PreAuthorize.class);
 		PreAuthorizeAuthorizationManager authorizationManager = new PreAuthorizeAuthorizationManager();
 		authorizationManager.setExpressionHandler(getMethodSecurityExpressionHandler());
-		return new AuthorizationManagerMethodBeforeAdvice<>(methodMatcher, authorizationManager);
+		return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager);
 	}
 
 	private AuthorizationManagerMethodBeforeAdvice<MethodAuthorizationContext> getSecuredAuthorizationMethodBeforeAdvice() {
-		MethodMatcher methodMatcher = new SecurityAnnotationsStaticMethodMatcher(Secured.class);
+		Pointcut pointcut = forAnnotation(Secured.class);
 		SecuredAuthorizationManager authorizationManager = new SecuredAuthorizationManager();
-		return new AuthorizationManagerMethodBeforeAdvice<>(methodMatcher, authorizationManager);
+		return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager);
 	}
 
 	private AuthorizationManagerMethodBeforeAdvice<MethodAuthorizationContext> getJsr250AuthorizationMethodBeforeAdvice() {
-		MethodMatcher methodMatcher = new SecurityAnnotationsStaticMethodMatcher(DenyAll.class, PermitAll.class,
-				RolesAllowed.class);
+		Pointcut pointcut = new ComposablePointcut(forAnnotation(DenyAll.class)).union(forAnnotation(PermitAll.class))
+				.union(forAnnotation(RolesAllowed.class));
 		Jsr250AuthorizationManager authorizationManager = new Jsr250AuthorizationManager();
 		if (this.grantedAuthorityDefaults != null) {
 			authorizationManager.setRolePrefix(this.grantedAuthorityDefaults.getRolePrefix());
 		}
-		return new AuthorizationManagerMethodBeforeAdvice<>(methodMatcher, authorizationManager);
+		return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager);
 	}
 
 	@Autowired(required = false)
@@ -196,16 +196,18 @@ private AuthorizationMethodAfterAdvice<MethodAuthorizationContext> createDefault
 	}
 
 	private PostFilterAuthorizationMethodAfterAdvice getPostFilterAuthorizationMethodAfterAdvice() {
-		PostFilterAuthorizationMethodAfterAdvice postFilterAfterAdvice = new PostFilterAuthorizationMethodAfterAdvice();
+		Pointcut pointcut = forAnnotation(PostFilter.class);
+		PostFilterAuthorizationMethodAfterAdvice postFilterAfterAdvice = new PostFilterAuthorizationMethodAfterAdvice(
+				pointcut);
 		postFilterAfterAdvice.setExpressionHandler(getMethodSecurityExpressionHandler());
 		return postFilterAfterAdvice;
 	}
 
 	private AuthorizationManagerMethodAfterAdvice<MethodAuthorizationContext> getPostAuthorizeAuthorizationMethodAfterAdvice() {
-		MethodMatcher methodMatcher = new SecurityAnnotationsStaticMethodMatcher(PostAuthorize.class);
+		Pointcut pointcut = forAnnotation(PostAuthorize.class);
 		PostAuthorizeAuthorizationManager authorizationManager = new PostAuthorizeAuthorizationManager();
 		authorizationManager.setExpressionHandler(getMethodSecurityExpressionHandler());
-		return new AuthorizationManagerMethodAfterAdvice<>(methodMatcher, authorizationManager);
+		return new AuthorizationManagerMethodAfterAdvice<>(pointcut, authorizationManager);
 	}
 
 	@Autowired(required = false)
@@ -241,27 +243,9 @@ private int order() {
 		return this.enableMethodSecurity.getNumber("order");
 	}
 
-	private static final class SecurityAnnotationsStaticMethodMatcher extends StaticMethodMatcher {
-
-		private final Set<Class<? extends Annotation>> annotationClasses;
-
-		@SafeVarargs
-		private SecurityAnnotationsStaticMethodMatcher(Class<? extends Annotation>... annotationClasses) {
-			this.annotationClasses = new HashSet<>(Arrays.asList(annotationClasses));
-		}
-
-		@Override
-		public boolean matches(Method method, Class<?> targetClass) {
-			Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
-			return hasAnnotations(specificMethod) || hasAnnotations(specificMethod.getDeclaringClass());
-		}
-
-		private boolean hasAnnotations(AnnotatedElement annotatedElement) {
-			Set<Annotation> annotations = AnnotatedElementUtils.findAllMergedAnnotations(annotatedElement,
-					this.annotationClasses);
-			return !annotations.isEmpty();
-		}
-
+	private Pointcut forAnnotation(Class<? extends Annotation> annotationClass) {
+		return Pointcuts.union(new AnnotationMatchingPointcut(annotationClass, true),
+				new AnnotationMatchingPointcut(null, annotationClass, true));
 	}
 
 }

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

@@ -25,7 +25,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import org.springframework.aop.MethodMatcher;
+import org.springframework.aop.Pointcut;
 import org.springframework.aop.support.JdkRegexpMethodPointcut;
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -353,12 +353,12 @@ static class CustomAuthorizationManagerAfterAdviceConfig {
 
 		@Bean
 		AuthorizationMethodAfterAdvice<MethodAuthorizationContext> customAfterAdvice() {
-			JdkRegexpMethodPointcut methodMatcher = new JdkRegexpMethodPointcut();
-			methodMatcher.setPattern(".*MethodSecurityServiceImpl.*securedUser");
+			JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut();
+			pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser");
 			return new AuthorizationMethodAfterAdvice<MethodAuthorizationContext>() {
 				@Override
-				public MethodMatcher getMethodMatcher() {
-					return methodMatcher;
+				public Pointcut getPointcut() {
+					return pointcut;
 				}
 
 				@Override

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

@@ -19,6 +19,7 @@
 import java.util.function.Supplier;
 
 import org.springframework.aop.MethodMatcher;
+import org.springframework.aop.Pointcut;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.core.Authentication;
@@ -31,24 +32,24 @@
  *
  * @param <T> the type of object that the authorization check is being done one.
  * @author Evgeniy Cheban
+ * @author Josh Cummings
  * @since 5.5
  */
 public final class AuthorizationManagerMethodAfterAdvice<T> implements AuthorizationMethodAfterAdvice<T> {
 
-	private final MethodMatcher methodMatcher;
+	private final Pointcut pointcut;
 
 	private final AuthorizationManager<T> authorizationManager;
 
 	/**
 	 * Creates an instance.
-	 * @param methodMatcher the {@link MethodMatcher} to use
+	 * @param pointcut the {@link Pointcut} to use
 	 * @param authorizationManager the {@link AuthorizationManager} to use
 	 */
-	public AuthorizationManagerMethodAfterAdvice(MethodMatcher methodMatcher,
-			AuthorizationManager<T> authorizationManager) {
-		Assert.notNull(methodMatcher, "methodMatcher cannot be null");
+	public AuthorizationManagerMethodAfterAdvice(Pointcut pointcut, AuthorizationManager<T> authorizationManager) {
+		Assert.notNull(pointcut, "pointcut cannot be null");
 		Assert.notNull(authorizationManager, "authorizationManager cannot be null");
-		this.methodMatcher = methodMatcher;
+		this.pointcut = pointcut;
 		this.authorizationManager = authorizationManager;
 	}
 
@@ -65,9 +66,12 @@ public Object after(Supplier<Authentication> authentication, T object, Object re
 		return returnedObject;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	@Override
-	public MethodMatcher getMethodMatcher() {
-		return this.methodMatcher;
+	public Pointcut getPointcut() {
+		return this.pointcut;
 	}
 
 }

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

@@ -19,6 +19,7 @@
 import java.util.function.Supplier;
 
 import org.springframework.aop.MethodMatcher;
+import org.springframework.aop.Pointcut;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.core.Authentication;
@@ -31,24 +32,24 @@
  *
  * @param <T> the type of object that the authorization check is being done one.
  * @author Evgeniy Cheban
+ * @author Josh Cummings
  * @since 5.5
  */
 public final class AuthorizationManagerMethodBeforeAdvice<T> implements AuthorizationMethodBeforeAdvice<T> {
 
-	private final MethodMatcher methodMatcher;
+	private final Pointcut pointcut;
 
 	private final AuthorizationManager<T> authorizationManager;
 
 	/**
 	 * Creates an instance.
-	 * @param methodMatcher the {@link MethodMatcher} to use
+	 * @param pointcut the {@link Pointcut} to use
 	 * @param authorizationManager the {@link AuthorizationManager} to use
 	 */
-	public AuthorizationManagerMethodBeforeAdvice(MethodMatcher methodMatcher,
-			AuthorizationManager<T> authorizationManager) {
-		Assert.notNull(methodMatcher, "methodMatcher cannot be null");
+	public AuthorizationManagerMethodBeforeAdvice(Pointcut pointcut, AuthorizationManager<T> authorizationManager) {
+		Assert.notNull(pointcut, "pointcut cannot be null");
 		Assert.notNull(authorizationManager, "authorizationManager cannot be null");
-		this.methodMatcher = methodMatcher;
+		this.pointcut = pointcut;
 		this.authorizationManager = authorizationManager;
 	}
 
@@ -64,9 +65,12 @@ public void before(Supplier<Authentication> authentication, T object) {
 		this.authorizationManager.verify(authentication, object);
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	@Override
-	public MethodMatcher getMethodMatcher() {
-		return this.methodMatcher;
+	public Pointcut getPointcut() {
+		return this.pointcut;
 	}
 
 }

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

@@ -18,30 +18,40 @@
 
 import java.util.function.Supplier;
 
+import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInvocation;
 
-import org.springframework.aop.ClassFilter;
-import org.springframework.aop.Pointcut;
+import org.springframework.aop.AfterAdvice;
+import org.springframework.aop.PointcutAdvisor;
+import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.security.core.Authentication;
 
 /**
- * An Authorization advice that can determine if an {@link Authentication} has access to
- * the returned object from the {@link MethodInvocation}. The {@link #getMethodMatcher()}
+ * An {@link Advice} which can determine if an {@link Authentication} has
+ * access to the returned object from the {@link MethodInvocation}. {@link #getPointcut()}
  * describes when the advice applies for the method.
  *
  * @param <T> the type of object that the authorization check is being done one.
  * @author Evgeniy Cheban
+ * @author Josh Cummings
  * @since 5.5
  */
-public interface AuthorizationMethodAfterAdvice<T> extends Pointcut {
+public interface AuthorizationMethodAfterAdvice<T> extends AfterAdvice, PointcutAdvisor, AopInfrastructureBean {
 
 	/**
-	 * Returns the default {@link ClassFilter}.
-	 * @return the {@link ClassFilter#TRUE} to use
+	 * {@inheritDoc}
 	 */
 	@Override
-	default ClassFilter getClassFilter() {
-		return ClassFilter.TRUE;
+	default boolean isPerInstance() {
+		return true;
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	default Advice getAdvice() {
+		return this;
 	}
 
 	/**

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

@@ -18,28 +18,38 @@
 
 import java.util.function.Supplier;
 
-import org.springframework.aop.ClassFilter;
-import org.springframework.aop.Pointcut;
+import org.aopalliance.aop.Advice;
+
+import org.springframework.aop.BeforeAdvice;
+import org.springframework.aop.PointcutAdvisor;
+import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.security.core.Authentication;
 
 /**
- * An advice which can determine if an {@link Authentication} has access to the {@link T}
- * object. The {@link #getMethodMatcher()} describes when the advice applies for the
- * method.
+ * An {@link Advice} which can determine if an {@link Authentication} has access to the
+ * {@link T} object. {@link #getPointcut()} describes when the advice applies.
  *
  * @param <T> the type of object that the authorization check is being done one.
  * @author Evgeniy Cheban
+ * @author Josh Cummings
  * @since 5.5
  */
-public interface AuthorizationMethodBeforeAdvice<T> extends Pointcut {
+public interface AuthorizationMethodBeforeAdvice<T> extends BeforeAdvice, PointcutAdvisor, AopInfrastructureBean {
+
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	default boolean isPerInstance() {
+		return true;
+	}
 
 	/**
-	 * Returns the default {@link ClassFilter}.
-	 * @return the {@link ClassFilter#TRUE} to use
+	 * {@inheritDoc}
 	 */
 	@Override
-	default ClassFilter getClassFilter() {
-		return ClassFilter.TRUE;
+	default Advice getAdvice() {
+		return this;
 	}
 
 	/**