diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java index 98b0495a5c..6f3dd3859e 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java @@ -17,25 +17,19 @@ 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; @@ -43,14 +37,15 @@ 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 @@ import org.springframework.util.Assert; * 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 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean } private PreFilterAuthorizationMethodBeforeAdvice getPreFilterAuthorizationMethodBeforeAdvice() { - PreFilterAuthorizationMethodBeforeAdvice preFilterBeforeAdvice = new PreFilterAuthorizationMethodBeforeAdvice(); + Pointcut pointcut = forAnnotation(PreFilter.class); + PreFilterAuthorizationMethodBeforeAdvice preFilterBeforeAdvice = new PreFilterAuthorizationMethodBeforeAdvice( + pointcut); preFilterBeforeAdvice.setExpressionHandler(getMethodSecurityExpressionHandler()); return preFilterBeforeAdvice; } private AuthorizationMethodBeforeAdvice 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 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 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 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean } private PostFilterAuthorizationMethodAfterAdvice getPostFilterAuthorizationMethodAfterAdvice() { - PostFilterAuthorizationMethodAfterAdvice postFilterAfterAdvice = new PostFilterAuthorizationMethodAfterAdvice(); + Pointcut pointcut = forAnnotation(PostFilter.class); + PostFilterAuthorizationMethodAfterAdvice postFilterAfterAdvice = new PostFilterAuthorizationMethodAfterAdvice( + pointcut); postFilterAfterAdvice.setExpressionHandler(getMethodSecurityExpressionHandler()); return postFilterAfterAdvice; } private AuthorizationManagerMethodAfterAdvice 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 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean return this.enableMethodSecurity.getNumber("order"); } - private static final class SecurityAnnotationsStaticMethodMatcher extends StaticMethodMatcher { - - private final Set> annotationClasses; - - @SafeVarargs - private SecurityAnnotationsStaticMethodMatcher(Class... 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 annotations = AnnotatedElementUtils.findAllMergedAnnotations(annotatedElement, - this.annotationClasses); - return !annotations.isEmpty(); - } - + private Pointcut forAnnotation(Class annotationClass) { + return Pointcuts.union(new AnnotationMatchingPointcut(annotationClass, true), + new AnnotationMatchingPointcut(null, annotationClass, true)); } } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java index 69baeaffb0..6ad75a0d02 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java @@ -25,7 +25,7 @@ import org.junit.Rule; 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 @@ public class MethodSecurityConfigurationTests { @Bean AuthorizationMethodAfterAdvice customAfterAdvice() { - JdkRegexpMethodPointcut methodMatcher = new JdkRegexpMethodPointcut(); - methodMatcher.setPattern(".*MethodSecurityServiceImpl.*securedUser"); + JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut(); + pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser"); return new AuthorizationMethodAfterAdvice() { @Override - public MethodMatcher getMethodMatcher() { - return methodMatcher; + public Pointcut getPointcut() { + return pointcut; } @Override diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdvice.java index f21115e0f5..257f3fc8eb 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdvice.java @@ -19,6 +19,7 @@ package org.springframework.security.authorization.method; 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 @@ import org.springframework.util.Assert; * * @param 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 implements AuthorizationMethodAfterAdvice { - private final MethodMatcher methodMatcher; + private final Pointcut pointcut; private final AuthorizationManager 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 authorizationManager) { - Assert.notNull(methodMatcher, "methodMatcher cannot be null"); + public AuthorizationManagerMethodAfterAdvice(Pointcut pointcut, AuthorizationManager 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 final class AuthorizationManagerMethodAfterAdvice implements Authoriza return returnedObject; } + /** + * {@inheritDoc} + */ @Override - public MethodMatcher getMethodMatcher() { - return this.methodMatcher; + public Pointcut getPointcut() { + return this.pointcut; } } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdvice.java index 9248179b43..e0c485b8c6 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdvice.java @@ -19,6 +19,7 @@ package org.springframework.security.authorization.method; 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 @@ import org.springframework.util.Assert; * * @param 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 implements AuthorizationMethodBeforeAdvice { - private final MethodMatcher methodMatcher; + private final Pointcut pointcut; private final AuthorizationManager 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 authorizationManager) { - Assert.notNull(methodMatcher, "methodMatcher cannot be null"); + public AuthorizationManagerMethodBeforeAdvice(Pointcut pointcut, AuthorizationManager 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 final class AuthorizationManagerMethodBeforeAdvice implements Authoriz this.authorizationManager.verify(authentication, object); } + /** + * {@inheritDoc} + */ @Override - public MethodMatcher getMethodMatcher() { - return this.methodMatcher; + public Pointcut getPointcut() { + return this.pointcut; } } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodAfterAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodAfterAdvice.java index f2c91876a2..705005be78 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodAfterAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodAfterAdvice.java @@ -18,30 +18,40 @@ package org.springframework.security.authorization.method; 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 the type of object that the authorization check is being done one. * @author Evgeniy Cheban + * @author Josh Cummings * @since 5.5 */ -public interface AuthorizationMethodAfterAdvice extends Pointcut { +public interface AuthorizationMethodAfterAdvice 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; } /** diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodBeforeAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodBeforeAdvice.java index 2cbc4de499..47b62c3a57 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodBeforeAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodBeforeAdvice.java @@ -18,28 +18,38 @@ package org.springframework.security.authorization.method; 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 the type of object that the authorization check is being done one. * @author Evgeniy Cheban + * @author Josh Cummings * @since 5.5 */ -public interface AuthorizationMethodBeforeAdvice extends Pointcut { +public interface AuthorizationMethodBeforeAdvice 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; } /** diff --git a/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdvice.java index 30207a4bd7..7ab8b89c1a 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdvice.java @@ -16,7 +16,6 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.Method; import java.util.List; import java.util.function.Supplier; @@ -24,10 +23,11 @@ import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.aop.MethodMatcher; -import org.springframework.aop.support.StaticMethodMatcher; +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.ComposablePointcut; import org.springframework.core.log.LogMessage; import org.springframework.security.core.Authentication; +import org.springframework.util.Assert; /** * An {@link AuthorizationMethodAfterAdvice} which delegates to specific @@ -35,6 +35,7 @@ import org.springframework.security.core.Authentication; * the {@link MethodInvocation}. * * @author Evgeniy Cheban + * @author Josh Cummings * @since 5.5 */ public final class DelegatingAuthorizationMethodAfterAdvice @@ -42,18 +43,7 @@ public final class DelegatingAuthorizationMethodAfterAdvice private final Log logger = LogFactory.getLog(getClass()); - private final MethodMatcher methodMatcher = new StaticMethodMatcher() { - @Override - public boolean matches(Method method, Class targetClass) { - for (AuthorizationMethodAfterAdvice delegate : DelegatingAuthorizationMethodAfterAdvice.this.delegates) { - MethodMatcher methodMatcher = delegate.getMethodMatcher(); - if (methodMatcher.matches(method, targetClass)) { - return true; - } - } - return false; - } - }; + private final Pointcut pointcut; private final List> delegates; @@ -63,12 +53,26 @@ public final class DelegatingAuthorizationMethodAfterAdvice */ public DelegatingAuthorizationMethodAfterAdvice( List> delegates) { + Assert.notEmpty(delegates, "delegates cannot be empty"); this.delegates = delegates; + ComposablePointcut pointcut = null; + for (AuthorizationMethodAfterAdvice advice : delegates) { + if (pointcut == null) { + pointcut = new ComposablePointcut(advice.getPointcut()); + } + else { + pointcut.union(advice.getPointcut()); + } + } + this.pointcut = pointcut; } + /** + * {@inheritDoc} + */ @Override - public MethodMatcher getMethodMatcher() { - return this.methodMatcher; + public Pointcut getPointcut() { + return this.pointcut; } /** diff --git a/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdvice.java index a34cbf7035..a2276d6e8e 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdvice.java @@ -16,17 +16,17 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.Method; import java.util.List; import java.util.function.Supplier; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.aop.MethodMatcher; -import org.springframework.aop.support.StaticMethodMatcher; +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.ComposablePointcut; import org.springframework.core.log.LogMessage; import org.springframework.security.core.Authentication; +import org.springframework.util.Assert; /** * An {@link AuthorizationMethodBeforeAdvice} which delegates to a specific @@ -35,6 +35,7 @@ import org.springframework.security.core.Authentication; * one of the {@link AuthorizationMethodBeforeAdvice}s denied. * * @author Evgeniy Cheban + * @author Josh Cummings * @since 5.5 */ public final class DelegatingAuthorizationMethodBeforeAdvice @@ -42,18 +43,7 @@ public final class DelegatingAuthorizationMethodBeforeAdvice private final Log logger = LogFactory.getLog(getClass()); - private final MethodMatcher methodMatcher = new StaticMethodMatcher() { - @Override - public boolean matches(Method method, Class targetClass) { - for (AuthorizationMethodBeforeAdvice delegate : DelegatingAuthorizationMethodBeforeAdvice.this.delegates) { - MethodMatcher methodMatcher = delegate.getMethodMatcher(); - if (methodMatcher.matches(method, targetClass)) { - return true; - } - } - return false; - } - }; + private final Pointcut pointcut; private final List> delegates; @@ -63,12 +53,26 @@ public final class DelegatingAuthorizationMethodBeforeAdvice */ public DelegatingAuthorizationMethodBeforeAdvice( List> delegates) { + Assert.notEmpty(delegates, "delegates cannot be empty"); this.delegates = delegates; + ComposablePointcut pointcut = null; + for (AuthorizationMethodBeforeAdvice advice : delegates) { + if (pointcut == null) { + pointcut = new ComposablePointcut(advice.getPointcut()); + } + else { + pointcut.union(advice.getPointcut()); + } + } + this.pointcut = pointcut; } + /** + * {@inheritDoc} + */ @Override - public MethodMatcher getMethodMatcher() { - return this.methodMatcher; + public Pointcut getPointcut() { + return this.pointcut; } /** diff --git a/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdvice.java index 6e5a2f9713..9798536ec2 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdvice.java @@ -21,9 +21,8 @@ import java.util.function.Supplier; import org.aopalliance.intercept.MethodInvocation; -import org.springframework.aop.MethodMatcher; +import org.springframework.aop.Pointcut; import org.springframework.aop.support.AopUtils; -import org.springframework.aop.support.StaticMethodMatcher; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; @@ -40,6 +39,7 @@ import org.springframework.util.Assert; * {@link PostFilter} annotation. * * @author Evgeniy Cheban + * @author Josh Cummings * @since 5.5 */ public final class PostFilterAuthorizationMethodAfterAdvice @@ -47,16 +47,19 @@ public final class PostFilterAuthorizationMethodAfterAdvice private final PostFilterExpressionAttributeRegistry registry = new PostFilterExpressionAttributeRegistry(); - private final MethodMatcher methodMatcher = new StaticMethodMatcher() { - @Override - public boolean matches(Method method, Class targetClass) { - return PostFilterAuthorizationMethodAfterAdvice.this.registry.getAttribute(method, - targetClass) != ExpressionAttribute.NULL_ATTRIBUTE; - } - }; + private final Pointcut pointcut; private MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); + /** + * Create a {@link PostFilterAuthorizationMethodAfterAdvice} using the provided + * parameters + * @param pointcut the {@link Pointcut} for when this advice applies + */ + public PostFilterAuthorizationMethodAfterAdvice(Pointcut pointcut) { + this.pointcut = pointcut; + } + /** * Sets the {@link MethodSecurityExpressionHandler}. * @param expressionHandler the {@link MethodSecurityExpressionHandler} to use @@ -66,9 +69,12 @@ public final class PostFilterAuthorizationMethodAfterAdvice this.expressionHandler = expressionHandler; } + /** + * {@inheritDoc} + */ @Override - public MethodMatcher getMethodMatcher() { - return this.methodMatcher; + public Pointcut getPointcut() { + return this.pointcut; } /** diff --git a/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdvice.java index 486ead59c9..9f266f8576 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdvice.java @@ -21,9 +21,8 @@ import java.util.function.Supplier; import org.aopalliance.intercept.MethodInvocation; -import org.springframework.aop.MethodMatcher; +import org.springframework.aop.Pointcut; import org.springframework.aop.support.AopUtils; -import org.springframework.aop.support.StaticMethodMatcher; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; @@ -40,6 +39,7 @@ import org.springframework.util.StringUtils; * evaluating an expression from the {@link PreFilter} annotation. * * @author Evgeniy Cheban + * @author Josh Cummings * @since 5.5 */ public final class PreFilterAuthorizationMethodBeforeAdvice @@ -47,16 +47,19 @@ public final class PreFilterAuthorizationMethodBeforeAdvice private final PreFilterExpressionAttributeRegistry registry = new PreFilterExpressionAttributeRegistry(); - private final MethodMatcher methodMatcher = new StaticMethodMatcher() { - @Override - public boolean matches(Method method, Class targetClass) { - return PreFilterAuthorizationMethodBeforeAdvice.this.registry.getAttribute(method, - targetClass) != PreFilterExpressionAttribute.NULL_ATTRIBUTE; - } - }; + private final Pointcut pointcut; private MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); + /** + * Create a {@link PreFilterAuthorizationMethodBeforeAdvice} using the provided + * parameters + * @param pointcut the {@link Pointcut} for when this advice applies + */ + public PreFilterAuthorizationMethodBeforeAdvice(Pointcut pointcut) { + this.pointcut = pointcut; + } + /** * Sets the {@link MethodSecurityExpressionHandler}. * @param expressionHandler the {@link MethodSecurityExpressionHandler} to use @@ -66,9 +69,12 @@ public final class PreFilterAuthorizationMethodBeforeAdvice this.expressionHandler = expressionHandler; } + /** + * {@inheritDoc} + */ @Override - public MethodMatcher getMethodMatcher() { - return this.methodMatcher; + public Pointcut getPointcut() { + return this.pointcut; } /** diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdviceTests.java index 1ce9dddb74..a407fcf85e 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdviceTests.java @@ -21,10 +21,9 @@ import java.util.function.Supplier; import org.aopalliance.intercept.MethodInvocation; import org.junit.Test; -import org.springframework.aop.MethodMatcher; +import org.springframework.aop.Pointcut; import org.springframework.security.authentication.TestAuthentication; import org.springframework.security.authorization.AuthorizationManager; -import org.springframework.security.authorization.method.AuthorizationManagerMethodAfterAdvice; import org.springframework.security.core.Authentication; import static org.assertj.core.api.Assertions.assertThat; @@ -43,13 +42,13 @@ public class AuthorizationManagerMethodAfterAdviceTests { public void instantiateWhenMethodMatcherNullThenException() { assertThatIllegalArgumentException() .isThrownBy(() -> new AuthorizationManagerMethodAfterAdvice<>(null, mock(AuthorizationManager.class))) - .withMessage("methodMatcher cannot be null"); + .withMessage("pointcut cannot be null"); } @Test public void instantiateWhenAuthorizationManagerNullThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new AuthorizationManagerMethodAfterAdvice<>(mock(MethodMatcher.class), null)) + .isThrownBy(() -> new AuthorizationManagerMethodAfterAdvice<>(mock(Pointcut.class), null)) .withMessage("authorizationManager cannot be null"); } @@ -60,7 +59,7 @@ public class AuthorizationManagerMethodAfterAdviceTests { Object returnedObject = new Object(); AuthorizationManager mockAuthorizationManager = mock(AuthorizationManager.class); AuthorizationManagerMethodAfterAdvice advice = new AuthorizationManagerMethodAfterAdvice<>( - mock(MethodMatcher.class), mockAuthorizationManager); + mock(Pointcut.class), mockAuthorizationManager); Object result = advice.after(authentication, mockMethodInvocation, returnedObject); assertThat(result).isEqualTo(returnedObject); verify(mockAuthorizationManager).verify(authentication, mockMethodInvocation); diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdviceTests.java index 5c7091429e..3f4d5ed2f2 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdviceTests.java @@ -21,10 +21,9 @@ import java.util.function.Supplier; import org.aopalliance.intercept.MethodInvocation; import org.junit.Test; -import org.springframework.aop.MethodMatcher; +import org.springframework.aop.Pointcut; import org.springframework.security.authentication.TestAuthentication; import org.springframework.security.authorization.AuthorizationManager; -import org.springframework.security.authorization.method.AuthorizationManagerMethodBeforeAdvice; import org.springframework.security.core.Authentication; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -42,13 +41,13 @@ public class AuthorizationManagerMethodBeforeAdviceTests { public void instantiateWhenMethodMatcherNullThenException() { assertThatIllegalArgumentException() .isThrownBy(() -> new AuthorizationManagerMethodBeforeAdvice<>(null, mock(AuthorizationManager.class))) - .withMessage("methodMatcher cannot be null"); + .withMessage("pointcut cannot be null"); } @Test public void instantiateWhenAuthorizationManagerNullThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new AuthorizationManagerMethodBeforeAdvice<>(mock(MethodMatcher.class), null)) + .isThrownBy(() -> new AuthorizationManagerMethodBeforeAdvice<>(mock(Pointcut.class), null)) .withMessage("authorizationManager cannot be null"); } @@ -58,7 +57,7 @@ public class AuthorizationManagerMethodBeforeAdviceTests { MethodInvocation mockMethodInvocation = mock(MethodInvocation.class); AuthorizationManager mockAuthorizationManager = mock(AuthorizationManager.class); AuthorizationManagerMethodBeforeAdvice advice = new AuthorizationManagerMethodBeforeAdvice<>( - mock(MethodMatcher.class), mockAuthorizationManager); + mock(Pointcut.class), mockAuthorizationManager); advice.before(authentication, mockMethodInvocation); verify(mockAuthorizationManager).verify(authentication, mockMethodInvocation); } diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptorTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptorTests.java index fe3b0915e9..387424e66d 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptorTests.java @@ -21,7 +21,7 @@ import java.util.function.Supplier; import org.junit.After; import org.junit.Test; -import org.springframework.aop.MethodMatcher; +import org.springframework.aop.Pointcut; import org.springframework.security.access.intercept.method.MockMethodInvocation; import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; import org.springframework.security.authentication.TestAuthentication; @@ -74,8 +74,8 @@ public class AuthorizationMethodInterceptorTests { "doSomethingString"); AuthorizationMethodBeforeAdvice beforeAdvice = new AuthorizationMethodBeforeAdvice() { @Override - public MethodMatcher getMethodMatcher() { - return MethodMatcher.TRUE; + public Pointcut getPointcut() { + return Pointcut.TRUE; } @Override diff --git a/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdviceTests.java index 343d6b21a2..8560cbeef2 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdviceTests.java @@ -24,12 +24,10 @@ import java.util.function.Supplier; import org.junit.Test; import org.springframework.aop.MethodMatcher; -import org.springframework.aop.support.StaticMethodMatcher; +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.StaticMethodMatcherPointcut; import org.springframework.security.access.intercept.method.MockMethodInvocation; import org.springframework.security.authentication.TestAuthentication; -import org.springframework.security.authorization.method.AuthorizationMethodAfterAdvice; -import org.springframework.security.authorization.method.DelegatingAuthorizationMethodAfterAdvice; -import org.springframework.security.authorization.method.MethodAuthorizationContext; import org.springframework.security.core.Authentication; import static org.assertj.core.api.Assertions.assertThat; @@ -52,8 +50,8 @@ public class DelegatingAuthorizationMethodAfterAdviceTests { } @Override - public MethodMatcher getMethodMatcher() { - return new StaticMethodMatcher() { + public Pointcut getPointcut() { + return new StaticMethodMatcherPointcut() { @Override public boolean matches(Method method, Class targetClass) { return false; @@ -69,8 +67,8 @@ public class DelegatingAuthorizationMethodAfterAdviceTests { } @Override - public MethodMatcher getMethodMatcher() { - return new StaticMethodMatcher() { + public Pointcut getPointcut() { + return new StaticMethodMatcherPointcut() { @Override public boolean matches(Method method, Class targetClass) { return false; @@ -79,7 +77,7 @@ public class DelegatingAuthorizationMethodAfterAdviceTests { } }); DelegatingAuthorizationMethodAfterAdvice advice = new DelegatingAuthorizationMethodAfterAdvice(delegates); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); } @@ -94,8 +92,8 @@ public class DelegatingAuthorizationMethodAfterAdviceTests { } @Override - public MethodMatcher getMethodMatcher() { - return new StaticMethodMatcher() { + public Pointcut getPointcut() { + return new StaticMethodMatcherPointcut() { @Override public boolean matches(Method method, Class targetClass) { return false; @@ -111,12 +109,12 @@ public class DelegatingAuthorizationMethodAfterAdviceTests { } @Override - public MethodMatcher getMethodMatcher() { - return MethodMatcher.TRUE; + public Pointcut getPointcut() { + return Pointcut.TRUE; } }); DelegatingAuthorizationMethodAfterAdvice advice = new DelegatingAuthorizationMethodAfterAdvice(delegates); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isTrue(); } @@ -131,8 +129,8 @@ public class DelegatingAuthorizationMethodAfterAdviceTests { } @Override - public MethodMatcher getMethodMatcher() { - return MethodMatcher.TRUE; + public Pointcut getPointcut() { + return Pointcut.TRUE; } }); delegates.add(new AuthorizationMethodAfterAdvice() { @@ -143,8 +141,8 @@ public class DelegatingAuthorizationMethodAfterAdviceTests { } @Override - public MethodMatcher getMethodMatcher() { - return MethodMatcher.TRUE; + public Pointcut getPointcut() { + return Pointcut.TRUE; } }); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, diff --git a/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdviceTests.java index 4bc00494b8..e88635aff7 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdviceTests.java @@ -25,7 +25,8 @@ import java.util.function.Supplier; import org.junit.Test; import org.springframework.aop.MethodMatcher; -import org.springframework.aop.support.StaticMethodMatcher; +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.StaticMethodMatcherPointcut; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.intercept.method.MockMethodInvocation; import org.springframework.security.authentication.TestAuthentication; @@ -47,8 +48,8 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { List> delegates = new ArrayList<>(); delegates.add(new AuthorizationMethodBeforeAdvice() { @Override - public MethodMatcher getMethodMatcher() { - return new StaticMethodMatcher() { + public Pointcut getPointcut() { + return new StaticMethodMatcherPointcut() { @Override public boolean matches(Method method, Class targetClass) { return false; @@ -62,8 +63,8 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { }); delegates.add(new AuthorizationMethodBeforeAdvice() { @Override - public MethodMatcher getMethodMatcher() { - return new StaticMethodMatcher() { + public Pointcut getPointcut() { + return new StaticMethodMatcherPointcut() { @Override public boolean matches(Method method, Class targetClass) { return false; @@ -76,7 +77,7 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { } }); DelegatingAuthorizationMethodBeforeAdvice advice = new DelegatingAuthorizationMethodBeforeAdvice(delegates); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); } @@ -85,8 +86,8 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { List> delegates = new ArrayList<>(); delegates.add(new AuthorizationMethodBeforeAdvice() { @Override - public MethodMatcher getMethodMatcher() { - return new StaticMethodMatcher() { + public Pointcut getPointcut() { + return new StaticMethodMatcherPointcut() { @Override public boolean matches(Method method, Class targetClass) { return false; @@ -100,8 +101,8 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { }); delegates.add(new AuthorizationMethodBeforeAdvice() { @Override - public MethodMatcher getMethodMatcher() { - return MethodMatcher.TRUE; + public Pointcut getPointcut() { + return Pointcut.TRUE; } @Override @@ -109,17 +110,17 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { } }); DelegatingAuthorizationMethodBeforeAdvice advice = new DelegatingAuthorizationMethodBeforeAdvice(delegates); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isTrue(); } @Test public void checkWhenAllGrantsOrAbstainsThenPasses() throws Exception { List> delegates = new ArrayList<>(); - delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(MethodMatcher.TRUE, (a, o) -> null)); - delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(MethodMatcher.TRUE, - (a, o) -> new AuthorizationDecision(true))); - delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(MethodMatcher.TRUE, (a, o) -> null)); + delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(Pointcut.TRUE, (a, o) -> null)); + delegates.add( + new AuthorizationManagerMethodBeforeAdvice<>(Pointcut.TRUE, (a, o) -> new AuthorizationDecision(true))); + delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(Pointcut.TRUE, (a, o) -> null)); DelegatingAuthorizationMethodBeforeAdvice advice = new DelegatingAuthorizationMethodBeforeAdvice(delegates); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomething"); @@ -131,10 +132,10 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { @Test public void checkWhenAnyDeniesThenAccessDeniedException() throws Exception { List> delegates = new ArrayList<>(); - delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(MethodMatcher.TRUE, (a, o) -> null)); - delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(MethodMatcher.TRUE, - (a, o) -> new AuthorizationDecision(true))); - delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(MethodMatcher.TRUE, + delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(Pointcut.TRUE, (a, o) -> null)); + delegates.add( + new AuthorizationManagerMethodBeforeAdvice<>(Pointcut.TRUE, (a, o) -> new AuthorizationDecision(true))); + delegates.add(new AuthorizationManagerMethodBeforeAdvice<>(Pointcut.TRUE, (a, o) -> new AuthorizationDecision(false))); DelegatingAuthorizationMethodBeforeAdvice advice = new DelegatingAuthorizationMethodBeforeAdvice(delegates); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, @@ -147,14 +148,9 @@ public class DelegatingAuthorizationMethodBeforeAdviceTests { } @Test - public void checkWhenDelegatesEmptyThenPasses() throws Exception { - DelegatingAuthorizationMethodBeforeAdvice advice = new DelegatingAuthorizationMethodBeforeAdvice( - Collections.emptyList()); - MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, - "doSomething"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext); + public void checkWhenDelegatesEmptyThenFails() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> new DelegatingAuthorizationMethodBeforeAdvice(Collections.emptyList())); } public static class TestClass { diff --git a/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdviceTests.java index d1bd6e50a5..209dfc9f0e 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdviceTests.java @@ -16,10 +16,14 @@ package org.springframework.security.authorization.method; +import java.lang.reflect.Method; + import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.Test; import org.springframework.aop.MethodMatcher; +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.StaticMethodMatcherPointcut; import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; import org.springframework.security.access.intercept.method.MockMethodInvocation; @@ -39,29 +43,35 @@ public class PostFilterAuthorizationMethodAfterAdviceTests { @Test public void setExpressionHandlerWhenNotNullThenSetsExpressionHandler() { MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(); + PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); advice.setExpressionHandler(expressionHandler); assertThat(advice).extracting("expressionHandler").isEqualTo(expressionHandler); } @Test public void setExpressionHandlerWhenNullThenException() { - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(); + PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); assertThatIllegalArgumentException().isThrownBy(() -> advice.setExpressionHandler(null)) .withMessage("expressionHandler cannot be null"); } @Test public void methodMatcherWhenMethodHasNotPostFilterAnnotationThenNotMatches() throws Exception { - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice( + new StaticMethodMatcherPointcut() { + @Override + public boolean matches(Method method, Class targetClass) { + return false; + } + }); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); } @Test public void methodMatcherWhenMethodHasPostFilterAnnotationThenMatches() throws Exception { - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat( methodMatcher.matches(TestClass.class.getMethod("doSomethingArray", String[].class), TestClass.class)) .isTrue(); @@ -71,14 +81,15 @@ public class PostFilterAuthorizationMethodAfterAdviceTests { public void afterWhenArrayNotNullThenFilteredArray() throws Exception { String[] array = { "john", "bob" }; MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, - "doSomethingArray", new Class[] { String[].class }, new Object[] { array }); + "doSomethingArrayClassLevel", new Class[] { String[].class }, new Object[] { array }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(); + PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); Object result = advice.after(TestAuthentication::authenticatedUser, methodAuthorizationContext, array); assertThat(result).asInstanceOf(InstanceOfAssertFactories.array(String[].class)).containsOnly("john"); } + @PostFilter("filterObject == 'john'") public static class TestClass { public void doSomething() { @@ -90,6 +101,10 @@ public class PostFilterAuthorizationMethodAfterAdviceTests { return array; } + public String[] doSomethingArrayClassLevel(String[] array) { + return array; + } + } } diff --git a/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdviceTests.java index 70d93e1410..9504ce5d6a 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdviceTests.java @@ -16,12 +16,15 @@ package org.springframework.security.authorization.method; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; import org.junit.Test; import org.springframework.aop.MethodMatcher; +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.StaticMethodMatcherPointcut; import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; import org.springframework.security.access.intercept.method.MockMethodInvocation; @@ -42,29 +45,35 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { @Test public void setExpressionHandlerWhenNotNullThenSetsExpressionHandler() { MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); advice.setExpressionHandler(expressionHandler); assertThat(advice).extracting("expressionHandler").isEqualTo(expressionHandler); } @Test public void setExpressionHandlerWhenNullThenException() { - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); assertThatIllegalArgumentException().isThrownBy(() -> advice.setExpressionHandler(null)) .withMessage("expressionHandler cannot be null"); } @Test public void methodMatcherWhenMethodHasNotPreFilterAnnotationThenNotMatches() throws Exception { - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice( + new StaticMethodMatcherPointcut() { + @Override + public boolean matches(Method method, Class targetClass) { + return false; + } + }); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); } @Test public void methodMatcherWhenMethodHasPreFilterAnnotationThenMatches() throws Exception { - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); - MethodMatcher methodMatcher = advice.getMethodMatcher(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomethingListFilterTargetMatch", List.class), TestClass.class)).isTrue(); } @@ -75,7 +84,7 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { "doSomethingListFilterTargetNotMatch", new Class[] { List.class }, new Object[] { new ArrayList<>() }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); assertThatIllegalArgumentException() .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) .withMessage( @@ -88,7 +97,7 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { "doSomethingListFilterTargetMatch", new Class[] { List.class }, new Object[] { null }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); assertThatIllegalArgumentException() .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) .withMessage("Filter target was null, or no argument with name 'list' found in method."); @@ -103,7 +112,7 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { "doSomethingListFilterTargetMatch", new Class[] { List.class }, new Object[] { list }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext); assertThat(list).hasSize(1); assertThat(list.get(0)).isEqualTo("john"); @@ -115,7 +124,7 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { "doSomethingListFilterTargetNotProvided", new Class[] { List.class }, new Object[] { null }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); assertThatIllegalArgumentException() .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) .withMessage("Filter target was null. Make sure you passing the correct value in the method argument."); @@ -130,7 +139,7 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { "doSomethingListFilterTargetNotProvided", new Class[] { List.class }, new Object[] { list }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext); assertThat(list).hasSize(1); assertThat(list.get(0)).isEqualTo("john"); @@ -143,7 +152,7 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { new Object[] { new String[] {} }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); assertThatIllegalStateException() .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) .withMessage( @@ -157,12 +166,13 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { new Object[] { "", new ArrayList<>() }); MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(); + PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); assertThatIllegalStateException() .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) .withMessage("Unable to determine the method argument for filtering. Specify the filter target."); } + @PreFilter("filterObject == 'john'") public static class TestClass { public void doSomething() { @@ -189,7 +199,6 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { return array; } - @PreFilter("filterObject == 'john'") public List doSomethingTwoArgsFilterTargetNotProvided(String s, List list) { return list; }