From df8abcfae763bb87d2d3dfc6d4b9b57b68b209b5 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 8 Apr 2021 14:31:36 -0600 Subject: [PATCH] Use Interceptors instead of Advice - Interceptor is a more descriptive term for what method security is doing - This also allows the code to follow a delegate pattern that unifies both before-method and after- method authorization Issue gh-9289 --- .../MethodSecurityConfiguration.java | 136 +++++--------- .../MethodSecurityConfigurationTests.java | 64 +++++-- .../configuration/MethodSecurityService.java | 11 ++ .../MethodSecurityServiceImpl.java | 7 + .../AbstractAuthorizationManagerRegistry.java | 21 +-- .../AbstractExpressionAttributeRegistry.java | 18 +- ...izationManagerAfterMethodInterceptor.java} | 31 ++-- ...zationManagerBeforeMethodInterceptor.java} | 27 +-- .../AuthorizationMethodAfterAdvice.java | 70 -------- .../AuthorizationMethodBeforeAdvice.java | 62 ------- .../AuthorizationMethodInterceptor.java | 83 +++++---- .../AuthorizationMethodInterceptors.java | 80 +++++++++ .../method/AuthorizationMethodInvocation.java | 123 +++++++++++++ .../method/AuthorizationMethodPointcuts.java | 54 ++++++ ...egatingAuthorizationMethodAfterAdvice.java | 106 ----------- ...gatingAuthorizationMethodBeforeAdvice.java | 101 ----------- ...egatingAuthorizationMethodInterceptor.java | 89 ++++++++++ .../method/Jsr250AuthorizationManager.java | 21 ++- .../method/MethodAuthorizationContext.java | 66 ------- .../PostAuthorizeAuthorizationManager.java | 24 ++- ...FilterAuthorizationMethodInterceptor.java} | 37 ++-- .../PreAuthorizeAuthorizationManager.java | 18 +- ...FilterAuthorizationMethodInterceptor.java} | 28 ++- .../method/SecuredAuthorizationManager.java | 20 +-- ...onManagerAfterMethodInterceptorTests.java} | 18 +- ...nManagerBeforeMethodInterceptorTests.java} | 17 +- .../AuthorizationMethodPointcutsTests.java | 167 ++++++++++++++++++ ...ngAuthorizationMethodAfterAdviceTests.java | 165 ----------------- ...gAuthorizationMethodBeforeAdviceTests.java | 164 ----------------- ...gAuthorizationMethodInterceptorTests.java} | 43 +++-- .../Jsr250AuthorizationManagerTests.java | 54 +++--- ...ostAuthorizeAuthorizationManagerTests.java | 35 ++-- ...rAuthorizationMethodInterceptorTests.java} | 54 +++--- ...PreAuthorizeAuthorizationManagerTests.java | 23 ++- ...rAuthorizationMethodInterceptorTests.java} | 99 +++++------ .../SecuredAuthorizationManagerTests.java | 36 ++-- .../authorization/method-security.adoc | 82 +++++---- 37 files changed, 1010 insertions(+), 1244 deletions(-) rename core/src/main/java/org/springframework/security/authorization/method/{AuthorizationManagerMethodAfterAdvice.java => AuthorizationManagerAfterMethodInterceptor.java} (66%) rename core/src/main/java/org/springframework/security/authorization/method/{AuthorizationManagerMethodBeforeAdvice.java => AuthorizationManagerBeforeMethodInterceptor.java} (67%) delete mode 100644 core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodAfterAdvice.java delete mode 100644 core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodBeforeAdvice.java create mode 100644 core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptors.java create mode 100644 core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInvocation.java create mode 100644 core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodPointcuts.java delete mode 100644 core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdvice.java delete mode 100644 core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdvice.java create mode 100644 core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodInterceptor.java delete mode 100644 core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationContext.java rename core/src/main/java/org/springframework/security/authorization/method/{PostFilterAuthorizationMethodAfterAdvice.java => PostFilterAuthorizationMethodInterceptor.java} (75%) rename core/src/main/java/org/springframework/security/authorization/method/{PreFilterAuthorizationMethodBeforeAdvice.java => PreFilterAuthorizationMethodInterceptor.java} (84%) rename core/src/test/java/org/springframework/security/authorization/method/{AuthorizationManagerMethodAfterAdviceTests.java => AuthorizationManagerAfterMethodInterceptorTests.java} (76%) rename core/src/test/java/org/springframework/security/authorization/method/{AuthorizationManagerMethodBeforeAdviceTests.java => AuthorizationManagerBeforeMethodInterceptorTests.java} (75%) create mode 100644 core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodPointcutsTests.java delete mode 100644 core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdviceTests.java delete mode 100644 core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdviceTests.java rename core/src/test/java/org/springframework/security/authorization/method/{AuthorizationMethodInterceptorTests.java => DelegatingAuthorizationMethodInterceptorTests.java} (65%) rename core/src/test/java/org/springframework/security/authorization/method/{PostFilterAuthorizationMethodAfterAdviceTests.java => PostFilterAuthorizationMethodInterceptorTests.java} (68%) rename core/src/test/java/org/springframework/security/authorization/method/{PreFilterAuthorizationMethodBeforeAdviceTests.java => PreFilterAuthorizationMethodInterceptorTests.java} (65%) 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 bfba8dbf8c..d9af0aa132 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 @@ -16,20 +16,11 @@ package org.springframework.security.config.annotation.method.configuration; -import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.List; import java.util.Map; -import javax.annotation.security.DenyAll; -import javax.annotation.security.PermitAll; -import javax.annotation.security.RolesAllowed; - -import org.springframework.aop.Pointcut; -import org.springframework.aop.support.ComposablePointcut; import org.springframework.aop.support.DefaultPointcutAdvisor; -import org.springframework.aop.support.Pointcuts; -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; @@ -39,26 +30,16 @@ import org.springframework.context.annotation.ImportAware; import org.springframework.context.annotation.Role; 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; -import org.springframework.security.authorization.method.AuthorizationMethodBeforeAdvice; import org.springframework.security.authorization.method.AuthorizationMethodInterceptor; -import org.springframework.security.authorization.method.DelegatingAuthorizationMethodAfterAdvice; -import org.springframework.security.authorization.method.DelegatingAuthorizationMethodBeforeAdvice; +import org.springframework.security.authorization.method.AuthorizationMethodInterceptors; +import org.springframework.security.authorization.method.DelegatingAuthorizationMethodInterceptor; import org.springframework.security.authorization.method.Jsr250AuthorizationManager; -import org.springframework.security.authorization.method.MethodAuthorizationContext; import org.springframework.security.authorization.method.PostAuthorizeAuthorizationManager; -import org.springframework.security.authorization.method.PostFilterAuthorizationMethodAfterAdvice; +import org.springframework.security.authorization.method.PostFilterAuthorizationMethodInterceptor; import org.springframework.security.authorization.method.PreAuthorizeAuthorizationManager; -import org.springframework.security.authorization.method.PreFilterAuthorizationMethodBeforeAdvice; +import org.springframework.security.authorization.method.PreFilterAuthorizationMethodInterceptor; import org.springframework.security.authorization.method.SecuredAuthorizationManager; import org.springframework.security.config.core.GrantedAuthorityDefaults; import org.springframework.util.Assert; @@ -79,30 +60,19 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean private GrantedAuthorityDefaults grantedAuthorityDefaults; - private AuthorizationMethodBeforeAdvice authorizationMethodBeforeAdvice; - - private AuthorizationMethodAfterAdvice authorizationMethodAfterAdvice; + private AuthorizationMethodInterceptor interceptor; private AnnotationAttributes enableMethodSecurity; @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - DefaultPointcutAdvisor methodSecurityAdvisor(AuthorizationMethodInterceptor interceptor) { - AuthorizationMethodBeforeAdvice beforeAdvice = getAuthorizationMethodBeforeAdvice(); - AuthorizationMethodAfterAdvice afterAdvice = getAuthorizationMethodAfterAdvice(); - Pointcut pointcut = Pointcuts.union(beforeAdvice.getPointcut(), afterAdvice.getPointcut()); - DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(pointcut, interceptor); + DefaultPointcutAdvisor methodSecurityAdvisor() { + AuthorizationMethodInterceptor interceptor = getInterceptor(); + DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(interceptor.getPointcut(), interceptor); advisor.setOrder(order()); return advisor; } - @Bean - @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - AuthorizationMethodInterceptor authorizationMethodInterceptor() { - return new AuthorizationMethodInterceptor(getAuthorizationMethodBeforeAdvice(), - getAuthorizationMethodAfterAdvice()); - } - private MethodSecurityExpressionHandler getMethodSecurityExpressionHandler() { if (this.methodSecurityExpressionHandler == null) { DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler(); @@ -124,15 +94,18 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean this.grantedAuthorityDefaults = grantedAuthorityDefaults; } - private AuthorizationMethodBeforeAdvice getAuthorizationMethodBeforeAdvice() { - if (this.authorizationMethodBeforeAdvice == null) { - this.authorizationMethodBeforeAdvice = createDefaultAuthorizationMethodBeforeAdvice(); + private AuthorizationMethodInterceptor getInterceptor() { + if (this.interceptor != null) { + return this.interceptor; } - return this.authorizationMethodBeforeAdvice; + List interceptors = new ArrayList<>(); + interceptors.addAll(createDefaultAuthorizationMethodBeforeAdvice()); + interceptors.addAll(createDefaultAuthorizationMethodAfterAdvice()); + return new DelegatingAuthorizationMethodInterceptor(interceptors); } - private AuthorizationMethodBeforeAdvice createDefaultAuthorizationMethodBeforeAdvice() { - List> beforeAdvices = new ArrayList<>(); + private List createDefaultAuthorizationMethodBeforeAdvice() { + List beforeAdvices = new ArrayList<>(); beforeAdvices.add(getPreFilterAuthorizationMethodBeforeAdvice()); beforeAdvices.add(getPreAuthorizeAuthorizationMethodBeforeAdvice()); if (securedEnabled()) { @@ -141,79 +114,55 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean if (jsr250Enabled()) { beforeAdvices.add(getJsr250AuthorizationMethodBeforeAdvice()); } - return new DelegatingAuthorizationMethodBeforeAdvice<>(beforeAdvices); + return beforeAdvices; } - private PreFilterAuthorizationMethodBeforeAdvice getPreFilterAuthorizationMethodBeforeAdvice() { - Pointcut pointcut = forAnnotation(PreFilter.class); - PreFilterAuthorizationMethodBeforeAdvice preFilterBeforeAdvice = new PreFilterAuthorizationMethodBeforeAdvice( - pointcut); - preFilterBeforeAdvice.setExpressionHandler(getMethodSecurityExpressionHandler()); - return preFilterBeforeAdvice; + private PreFilterAuthorizationMethodInterceptor getPreFilterAuthorizationMethodBeforeAdvice() { + PreFilterAuthorizationMethodInterceptor interceptor = new PreFilterAuthorizationMethodInterceptor(); + interceptor.setExpressionHandler(getMethodSecurityExpressionHandler()); + return interceptor; } - private AuthorizationMethodBeforeAdvice getPreAuthorizeAuthorizationMethodBeforeAdvice() { - Pointcut pointcut = forAnnotation(PreAuthorize.class); + private AuthorizationMethodInterceptor getPreAuthorizeAuthorizationMethodBeforeAdvice() { PreAuthorizeAuthorizationManager authorizationManager = new PreAuthorizeAuthorizationManager(); authorizationManager.setExpressionHandler(getMethodSecurityExpressionHandler()); - return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager); + return AuthorizationMethodInterceptors.preAuthorize(authorizationManager); } - private AuthorizationManagerMethodBeforeAdvice getSecuredAuthorizationMethodBeforeAdvice() { - Pointcut pointcut = forAnnotation(Secured.class); - SecuredAuthorizationManager authorizationManager = new SecuredAuthorizationManager(); - return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager); + private AuthorizationMethodInterceptor getSecuredAuthorizationMethodBeforeAdvice() { + return AuthorizationMethodInterceptors.secured(new SecuredAuthorizationManager()); } - private AuthorizationManagerMethodBeforeAdvice getJsr250AuthorizationMethodBeforeAdvice() { - Pointcut pointcut = new ComposablePointcut(forAnnotation(DenyAll.class)).union(forAnnotation(PermitAll.class)) - .union(forAnnotation(RolesAllowed.class)); + private AuthorizationMethodInterceptor getJsr250AuthorizationMethodBeforeAdvice() { Jsr250AuthorizationManager authorizationManager = new Jsr250AuthorizationManager(); if (this.grantedAuthorityDefaults != null) { authorizationManager.setRolePrefix(this.grantedAuthorityDefaults.getRolePrefix()); } - return new AuthorizationManagerMethodBeforeAdvice<>(pointcut, authorizationManager); + return AuthorizationMethodInterceptors.jsr250(authorizationManager); } @Autowired(required = false) - void setAuthorizationMethodBeforeAdvice( - AuthorizationMethodBeforeAdvice authorizationMethodBeforeAdvice) { - this.authorizationMethodBeforeAdvice = authorizationMethodBeforeAdvice; + void setAuthorizationMethodInterceptor(AuthorizationMethodInterceptor interceptor) { + this.interceptor = interceptor; } - private AuthorizationMethodAfterAdvice getAuthorizationMethodAfterAdvice() { - if (this.authorizationMethodAfterAdvice == null) { - this.authorizationMethodAfterAdvice = createDefaultAuthorizationMethodAfterAdvice(); - } - return this.authorizationMethodAfterAdvice; - } - - private AuthorizationMethodAfterAdvice createDefaultAuthorizationMethodAfterAdvice() { - List> afterAdvices = new ArrayList<>(); + private List createDefaultAuthorizationMethodAfterAdvice() { + List afterAdvices = new ArrayList<>(); afterAdvices.add(getPostFilterAuthorizationMethodAfterAdvice()); afterAdvices.add(getPostAuthorizeAuthorizationMethodAfterAdvice()); - return new DelegatingAuthorizationMethodAfterAdvice<>(afterAdvices); + return afterAdvices; } - private PostFilterAuthorizationMethodAfterAdvice getPostFilterAuthorizationMethodAfterAdvice() { - Pointcut pointcut = forAnnotation(PostFilter.class); - PostFilterAuthorizationMethodAfterAdvice postFilterAfterAdvice = new PostFilterAuthorizationMethodAfterAdvice( - pointcut); - postFilterAfterAdvice.setExpressionHandler(getMethodSecurityExpressionHandler()); - return postFilterAfterAdvice; + private AuthorizationMethodInterceptor getPostFilterAuthorizationMethodAfterAdvice() { + PostFilterAuthorizationMethodInterceptor interceptor = new PostFilterAuthorizationMethodInterceptor(); + interceptor.setExpressionHandler(getMethodSecurityExpressionHandler()); + return interceptor; } - private AuthorizationManagerMethodAfterAdvice getPostAuthorizeAuthorizationMethodAfterAdvice() { - Pointcut pointcut = forAnnotation(PostAuthorize.class); + private AuthorizationMethodInterceptor getPostAuthorizeAuthorizationMethodAfterAdvice() { PostAuthorizeAuthorizationManager authorizationManager = new PostAuthorizeAuthorizationManager(); authorizationManager.setExpressionHandler(getMethodSecurityExpressionHandler()); - return new AuthorizationManagerMethodAfterAdvice<>(pointcut, authorizationManager); - } - - @Autowired(required = false) - void setAuthorizationMethodAfterAdvice( - AuthorizationMethodAfterAdvice authorizationMethodAfterAdvice) { - this.authorizationMethodAfterAdvice = authorizationMethodAfterAdvice; + return AuthorizationMethodInterceptors.postAuthorize(authorizationManager); } @Override @@ -227,7 +176,7 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean if (!securedEnabled() && !jsr250Enabled()) { return; } - Assert.isNull(this.authorizationMethodBeforeAdvice, + Assert.isNull(this.interceptor, "You have specified your own advice, meaning that the annotation attributes securedEnabled and jsr250Enabled will be ignored. Please choose one or the other."); } @@ -243,9 +192,4 @@ final class MethodSecurityConfiguration implements ImportAware, InitializingBean return this.enableMethodSecurity.getNumber("order"); } - 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 6ad75a0d02..ea8c88597c 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 @@ -18,9 +18,11 @@ package org.springframework.security.config.annotation.method.configuration; import java.io.Serializable; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.function.Supplier; +import org.aopalliance.intercept.MethodInvocation; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -38,10 +40,8 @@ import org.springframework.security.access.expression.method.DefaultMethodSecuri import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; -import org.springframework.security.authorization.method.AuthorizationManagerMethodBeforeAdvice; -import org.springframework.security.authorization.method.AuthorizationMethodAfterAdvice; -import org.springframework.security.authorization.method.AuthorizationMethodBeforeAdvice; -import org.springframework.security.authorization.method.MethodAuthorizationContext; +import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; +import org.springframework.security.authorization.method.AuthorizationMethodInterceptor; import org.springframework.security.config.test.SpringTestRule; import org.springframework.security.core.Authentication; import org.springframework.security.test.context.annotation.SecurityTestExecutionListeners; @@ -273,6 +273,43 @@ public class MethodSecurityConfigurationTests { this.businessService.rolesAllowedUser(); } + @WithMockUser(roles = { "ADMIN", "USER" }) + @Test + public void manyAnnotationsWhenMeetsConditionsThenReturnsFilteredList() throws Exception { + List names = Arrays.asList("harold", "jonathan", "pete", "bo"); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); + List filtered = this.methodSecurityService.manyAnnotations(new ArrayList<>(names)); + assertThat(filtered).hasSize(2); + assertThat(filtered).containsExactly("harold", "jonathan"); + } + + @WithMockUser + @Test + public void manyAnnotationsWhenUserThenFails() { + List names = Arrays.asList("harold", "jonathan", "pete", "bo"); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); + assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(() -> this.methodSecurityService.manyAnnotations(new ArrayList<>(names))); + } + + @WithMockUser + @Test + public void manyAnnotationsWhenShortListThenFails() { + List names = Arrays.asList("harold", "jonathan", "pete"); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); + assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(() -> this.methodSecurityService.manyAnnotations(new ArrayList<>(names))); + } + + @WithMockUser(roles = "ADMIN") + @Test + public void manyAnnotationsWhenAdminThenFails() { + List names = Arrays.asList("harold", "jonathan", "pete", "bo"); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); + assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(() -> this.methodSecurityService.manyAnnotations(new ArrayList<>(names))); + } + @Test public void configureWhenCustomAdviceAndSecureEnabledThenException() { assertThatExceptionOfType(BeanCreationException.class).isThrownBy(() -> this.spring @@ -338,12 +375,12 @@ public class MethodSecurityConfigurationTests { static class CustomAuthorizationManagerBeforeAdviceConfig { @Bean - AuthorizationMethodBeforeAdvice customBeforeAdvice() { - JdkRegexpMethodPointcut methodMatcher = new JdkRegexpMethodPointcut(); - methodMatcher.setPattern(".*MethodSecurityServiceImpl.*securedUser"); - AuthorizationManager authorizationManager = (a, + AuthorizationMethodInterceptor customBeforeAdvice() { + JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut(); + pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser"); + AuthorizationManager authorizationManager = (a, o) -> new AuthorizationDecision("bob".equals(a.get().getName())); - return new AuthorizationManagerMethodBeforeAdvice<>(methodMatcher, authorizationManager); + return new AuthorizationManagerBeforeMethodInterceptor(pointcut, authorizationManager); } } @@ -352,18 +389,18 @@ public class MethodSecurityConfigurationTests { static class CustomAuthorizationManagerAfterAdviceConfig { @Bean - AuthorizationMethodAfterAdvice customAfterAdvice() { + + AuthorizationMethodInterceptor customAfterAdvice() { JdkRegexpMethodPointcut pointcut = new JdkRegexpMethodPointcut(); pointcut.setPattern(".*MethodSecurityServiceImpl.*securedUser"); - return new AuthorizationMethodAfterAdvice() { + AuthorizationMethodInterceptor interceptor = new AuthorizationMethodInterceptor() { @Override public Pointcut getPointcut() { return pointcut; } @Override - public Object after(Supplier authentication, - MethodAuthorizationContext methodAuthorizationContext, Object returnedObject) { + public Object invoke(Supplier authentication, MethodInvocation mi) { Authentication auth = authentication.get(); if ("bob".equals(auth.getName())) { return "granted"; @@ -371,6 +408,7 @@ public class MethodSecurityConfigurationTests { throw new AccessDeniedException("Access Denied for User '" + auth.getName() + "'"); } }; + return interceptor; } } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.java index 525ce2a477..4aab407ea2 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.java @@ -16,12 +16,16 @@ package org.springframework.security.config.annotation.method.configuration; +import java.util.List; + import javax.annotation.security.DenyAll; import javax.annotation.security.PermitAll; import org.springframework.security.access.annotation.Secured; 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.core.Authentication; import org.springframework.security.core.parameters.P; @@ -69,4 +73,11 @@ public interface MethodSecurityService { @PostAuthorize("#o?.contains('grant')") String postAnnotation(@P("o") String object); + @PreFilter("filterObject.length > 3") + @PreAuthorize("hasRole('ADMIN')") + @Secured("ROLE_USER") + @PostFilter("filterObject.length > 5") + @PostAuthorize("returnObject.size > 1") + List manyAnnotations(List array); + } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.java index 94a05216bc..6bf562cfeb 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.java @@ -16,6 +16,8 @@ package org.springframework.security.config.annotation.method.configuration; +import java.util.List; + import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; @@ -86,4 +88,9 @@ public class MethodSecurityServiceImpl implements MethodSecurityService { return null; } + @Override + public List manyAnnotations(List object) { + return object; + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AbstractAuthorizationManagerRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/AbstractAuthorizationManagerRegistry.java index 9509e7a084..47dad9dee3 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AbstractAuthorizationManagerRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AbstractAuthorizationManagerRegistry.java @@ -27,28 +27,25 @@ import org.springframework.lang.NonNull; import org.springframework.security.authorization.AuthorizationManager; /** - * An abstract registry which provides an {@link AuthorizationManager} for the - * {@link MethodInvocation}. + * For internal use only, as this contract is likely to change * * @author Evgeniy Cheban - * @since 5.5 */ abstract class AbstractAuthorizationManagerRegistry { - static final AuthorizationManager NULL_MANAGER = (a, o) -> null; + static final AuthorizationManager NULL_MANAGER = (a, o) -> null; - private final Map> cachedManagers = new ConcurrentHashMap<>(); + private final Map> cachedManagers = new ConcurrentHashMap<>(); /** - * Returns an {@link AuthorizationManager} for the {@link MethodAuthorizationContext}. - * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to use + * Returns an {@link AuthorizationManager} for the + * {@link AuthorizationMethodInvocation}. + * @param methodInvocation the {@link AuthorizationMethodInvocation} to use * @return an {@link AuthorizationManager} to use */ - final AuthorizationManager getManager( - MethodAuthorizationContext methodAuthorizationContext) { - MethodInvocation methodInvocation = methodAuthorizationContext.getMethodInvocation(); + final AuthorizationManager getManager(AuthorizationMethodInvocation methodInvocation) { Method method = methodInvocation.getMethod(); - Class targetClass = methodAuthorizationContext.getTargetClass(); + Class targetClass = methodInvocation.getTargetClass(); MethodClassKey cacheKey = new MethodClassKey(method, targetClass); return this.cachedManagers.computeIfAbsent(cacheKey, (k) -> resolveManager(method, targetClass)); } @@ -61,6 +58,6 @@ abstract class AbstractAuthorizationManagerRegistry { * @return the non-null {@link AuthorizationManager} */ @NonNull - abstract AuthorizationManager resolveManager(Method method, Class targetClass); + abstract AuthorizationManager resolveManager(Method method, Class targetClass); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AbstractExpressionAttributeRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/AbstractExpressionAttributeRegistry.java index cc943f110d..0dd24407ac 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AbstractExpressionAttributeRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AbstractExpressionAttributeRegistry.java @@ -20,31 +20,27 @@ import java.lang.reflect.Method; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import org.aopalliance.intercept.MethodInvocation; - import org.springframework.core.MethodClassKey; import org.springframework.lang.NonNull; /** - * An abstract registry which provides an {@link ExpressionAttribute} for the - * {@link MethodInvocation}. + * For internal use only, as this contract is likely to change * * @author Evgeniy Cheban - * @since 5.5 */ abstract class AbstractExpressionAttributeRegistry { private final Map cachedAttributes = new ConcurrentHashMap<>(); /** - * Returns an {@link ExpressionAttribute} for the {@link MethodAuthorizationContext}. - * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to use + * Returns an {@link ExpressionAttribute} for the + * {@link AuthorizationMethodInvocation}. + * @param mi the {@link AuthorizationMethodInvocation} to use * @return the {@link ExpressionAttribute} to use */ - final T getAttribute(MethodAuthorizationContext methodAuthorizationContext) { - MethodInvocation methodInvocation = methodAuthorizationContext.getMethodInvocation(); - Method method = methodInvocation.getMethod(); - Class targetClass = methodAuthorizationContext.getTargetClass(); + final T getAttribute(AuthorizationMethodInvocation mi) { + Method method = mi.getMethod(); + Class targetClass = mi.getTargetClass(); return getAttribute(method, targetClass); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java similarity index 66% rename from core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdvice.java rename to core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java index ae347cbcc1..14861ca141 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptor.java @@ -18,7 +18,8 @@ package org.springframework.security.authorization.method; import java.util.function.Supplier; -import org.springframework.aop.MethodMatcher; +import org.aopalliance.intercept.MethodInvocation; + import org.springframework.aop.Pointcut; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authorization.AuthorizationManager; @@ -26,28 +27,27 @@ import org.springframework.security.core.Authentication; import org.springframework.util.Assert; /** - * An {@link AuthorizationMethodAfterAdvice} which can determine if an - * {@link Authentication} has access to the {@link T} object using an - * {@link AuthorizationManager} if a {@link MethodMatcher} matches. + * An {@link AuthorizationMethodInterceptor} which can determine if an + * {@link Authentication} has access to the result of an {@link MethodInvocation} using an + * {@link AuthorizationManager} * - * @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 { +public final class AuthorizationManagerAfterMethodInterceptor implements AuthorizationMethodInterceptor { private final Pointcut pointcut; - private final AfterMethodAuthorizationManager authorizationManager; + private final AfterMethodAuthorizationManager authorizationManager; /** * Creates an instance. * @param pointcut the {@link Pointcut} to use * @param authorizationManager the {@link AuthorizationManager} to use */ - public AuthorizationManagerMethodAfterAdvice(Pointcut pointcut, - AfterMethodAuthorizationManager authorizationManager) { + public AuthorizationManagerAfterMethodInterceptor(Pointcut pointcut, + AfterMethodAuthorizationManager authorizationManager) { Assert.notNull(pointcut, "pointcut cannot be null"); Assert.notNull(authorizationManager, "authorizationManager cannot be null"); this.pointcut = pointcut; @@ -55,16 +55,17 @@ public final class AuthorizationManagerMethodAfterAdvice implements Authoriza } /** - * Determine if an {@link Authentication} has access to the {@link T} object using the - * {@link AuthorizationManager}. + * Determine if an {@link Authentication} has access to the {@link MethodInvocation} + * using the {@link AuthorizationManager}. * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param object the {@link T} object to check + * @param mi the {@link MethodInvocation} to check * @throws AccessDeniedException if access is not granted */ @Override - public Object after(Supplier authentication, T context, Object object) { - this.authorizationManager.verify(authentication, context, object); - return object; + public Object invoke(Supplier authentication, MethodInvocation mi) throws Throwable { + Object result = mi.proceed(); + this.authorizationManager.verify(authentication, mi, result); + return result; } /** diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java similarity index 67% rename from core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdvice.java rename to core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java index d63bea9d22..05a25a3b58 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptor.java @@ -18,7 +18,8 @@ package org.springframework.security.authorization.method; import java.util.function.Supplier; -import org.springframework.aop.MethodMatcher; +import org.aopalliance.intercept.MethodInvocation; + import org.springframework.aop.Pointcut; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authorization.AuthorizationManager; @@ -26,27 +27,26 @@ import org.springframework.security.core.Authentication; import org.springframework.util.Assert; /** - * An {@link AuthorizationMethodBeforeAdvice} which can determine if an - * {@link Authentication} has access to the {@link T} object using an - * {@link AuthorizationManager} if a {@link MethodMatcher} matches. + * An {@link AuthorizationMethodInterceptor} which uses a {@link AuthorizationManager} to + * determine if an {@link Authentication} may invoke the given {@link MethodInvocation} * - * @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 { +public final class AuthorizationManagerBeforeMethodInterceptor implements AuthorizationMethodInterceptor { private final Pointcut pointcut; - private final AuthorizationManager authorizationManager; + private final AuthorizationManager authorizationManager; /** * Creates an instance. * @param pointcut the {@link Pointcut} to use * @param authorizationManager the {@link AuthorizationManager} to use */ - public AuthorizationManagerMethodBeforeAdvice(Pointcut pointcut, AuthorizationManager authorizationManager) { + public AuthorizationManagerBeforeMethodInterceptor(Pointcut pointcut, + AuthorizationManager authorizationManager) { Assert.notNull(pointcut, "pointcut cannot be null"); Assert.notNull(authorizationManager, "authorizationManager cannot be null"); this.pointcut = pointcut; @@ -54,15 +54,16 @@ public final class AuthorizationManagerMethodBeforeAdvice implements Authoriz } /** - * Determine if an {@link Authentication} has access to the {@link T} object using the - * configured {@link AuthorizationManager}. + * Determine if an {@link Authentication} has access to the {@link MethodInvocation} + * using the configured {@link AuthorizationManager}. * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param object the {@link T} object to check + * @param mi the {@link MethodInvocation} to check * @throws AccessDeniedException if access is not granted */ @Override - public void before(Supplier authentication, T object) { - this.authorizationManager.verify(authentication, object); + public Object invoke(Supplier authentication, MethodInvocation mi) throws Throwable { + this.authorizationManager.verify(authentication, mi); + return mi.proceed(); } /** 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 deleted file mode 100644 index 1f360cefcd..0000000000 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodAfterAdvice.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -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.AfterAdvice; -import org.springframework.aop.PointcutAdvisor; -import org.springframework.aop.framework.AopInfrastructureBean; -import org.springframework.security.core.Authentication; - -/** - * 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 AfterAdvice, PointcutAdvisor, AopInfrastructureBean { - - /** - * {@inheritDoc} - */ - @Override - default boolean isPerInstance() { - return true; - } - - /** - * {@inheritDoc} - */ - @Override - default Advice getAdvice() { - return this; - } - - /** - * Determine if an {@link Authentication} has access to a method invocation's return - * object. - * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param object the {@link T} object to check - * @param returnedObject the returned object from the method invocation to check - * @return the {@code Object} that will ultimately be returned to the caller (if an - * implementation does not wish to modify the object to be returned to the caller, the - * implementation should simply return the same object it was passed by the - * {@code returnedObject} method argument) - */ - Object after(Supplier authentication, T object, Object returnedObject); - -} 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 deleted file mode 100644 index 4b1caee6d6..0000000000 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodBeforeAdvice.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authorization.method; - -import java.util.function.Supplier; - -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 {@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 BeforeAdvice, PointcutAdvisor, AopInfrastructureBean { - - /** - * {@inheritDoc} - */ - @Override - default boolean isPerInstance() { - return true; - } - - /** - * {@inheritDoc} - */ - @Override - default Advice getAdvice() { - return this; - } - - /** - * Determine if an {@link Authentication} has access to the {@link T} object. - * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param object the {@link T} object to check - */ - void before(Supplier authentication, T object); - -} diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptor.java index 4b411b3222..ac52e1b72c 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptor.java @@ -16,65 +16,70 @@ package org.springframework.security.authorization.method; +import java.util.function.Supplier; + +import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -import org.springframework.aop.support.AopUtils; -import org.springframework.lang.NonNull; +import org.springframework.aop.PointcutAdvisor; +import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; /** - * Provides security interception of AOP Alliance based method invocations. + * A {@link MethodInterceptor} which can determine if an {@link Authentication} has access + * to the {@link MethodInvocation}. {@link #getPointcut()} describes when the interceptor + * applies. * * @author Evgeniy Cheban + * @author Josh Cummings * @since 5.5 */ -public final class AuthorizationMethodInterceptor implements MethodInterceptor { - - private final AuthorizationMethodBeforeAdvice beforeAdvice; - - private final AuthorizationMethodAfterAdvice afterAdvice; +public interface AuthorizationMethodInterceptor extends MethodInterceptor, PointcutAdvisor, AopInfrastructureBean { /** - * Creates an instance. - * @param beforeAdvice the {@link AuthorizationMethodBeforeAdvice} to use - * @param afterAdvice the {@link AuthorizationMethodAfterAdvice} to use - */ - public AuthorizationMethodInterceptor(AuthorizationMethodBeforeAdvice beforeAdvice, - AuthorizationMethodAfterAdvice afterAdvice) { - this.beforeAdvice = beforeAdvice; - this.afterAdvice = afterAdvice; - } - - /** - * Enforce security on this {@link MethodInvocation}. - * @param mi the method being invoked which requires a security decision - * @return the returned value from the {@link MethodInvocation}, possibly altered by - * the configured {@link AuthorizationMethodAfterAdvice} + * {@inheritDoc} */ @Override - public Object invoke(@NonNull MethodInvocation mi) throws Throwable { - MethodAuthorizationContext methodAuthorizationContext = getMethodAuthorizationContext(mi); - this.beforeAdvice.before(this::getAuthentication, methodAuthorizationContext); - Object returnedObject = mi.proceed(); - return this.afterAdvice.after(this::getAuthentication, methodAuthorizationContext, returnedObject); + default Advice getAdvice() { + return this; } - private MethodAuthorizationContext getMethodAuthorizationContext(MethodInvocation mi) { - Object target = mi.getThis(); - Class targetClass = (target != null) ? AopUtils.getTargetClass(target) : null; - return new MethodAuthorizationContext(mi, targetClass); + /** + * {@inheritDoc} + */ + @Override + default boolean isPerInstance() { + return true; } - private Authentication getAuthentication() { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null) { - throw new AuthenticationCredentialsNotFoundException( - "An Authentication object was not found in the SecurityContext"); - } - return authentication; + /** + * Determine if an {@link Authentication} has access to the {@link MethodInvocation} + * @param mi the {@link MethodInvocation} to intercept and potentially invoke + * @return the result of the method invocation + * @throws Throwable if the interceptor or the target object throws an exception + */ + default Object invoke(MethodInvocation mi) throws Throwable { + Supplier supplier = () -> { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null) { + throw new AuthenticationCredentialsNotFoundException( + "An Authentication object was not found in the SecurityContext"); + } + return authentication; + }; + return invoke(supplier, new AuthorizationMethodInvocation(supplier, mi)); } + /** + * Determine if an {@link Authentication} has access to the {@link MethodInvocation} + * @param authentication the {@link Supplier} of the {@link Authentication} to check + * @param mi the {@link MethodInvocation} to intercept and potentially invoke + * @return the result of the method invocation + * @throws Throwable if the interceptor or the target object throws an exception + */ + Object invoke(Supplier authentication, MethodInvocation mi) throws Throwable; + } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptors.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptors.java new file mode 100644 index 0000000000..1d963ff7bc --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptors.java @@ -0,0 +1,80 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authorization.method; + +import javax.annotation.security.DenyAll; +import javax.annotation.security.PermitAll; +import javax.annotation.security.RolesAllowed; + +import org.springframework.security.access.annotation.Secured; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PreAuthorize; + +/** + * A static factory for constructing common {@link AuthorizationMethodInterceptor}s + * + * @author Josh Cummings + * @since 5.5 + * @see PreAuthorizeAuthorizationManager + * @see PostAuthorizeAuthorizationManager + * @see SecuredAuthorizationManager + * @see Jsr250AuthorizationManager + */ +public final class AuthorizationMethodInterceptors { + + public static AuthorizationMethodInterceptor preAuthorize() { + return preAuthorize(new PreAuthorizeAuthorizationManager()); + } + + public static AuthorizationMethodInterceptor preAuthorize(PreAuthorizeAuthorizationManager manager) { + return new AuthorizationManagerBeforeMethodInterceptor( + AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class), manager); + } + + public static AuthorizationMethodInterceptor postAuthorize() { + return postAuthorize(new PostAuthorizeAuthorizationManager()); + } + + public static AuthorizationMethodInterceptor postAuthorize(PostAuthorizeAuthorizationManager manager) { + return new AuthorizationManagerAfterMethodInterceptor( + AuthorizationMethodPointcuts.forAnnotations(PostAuthorize.class), manager); + } + + public static AuthorizationMethodInterceptor secured() { + return secured(new SecuredAuthorizationManager()); + } + + public static AuthorizationMethodInterceptor secured(SecuredAuthorizationManager manager) { + return new AuthorizationManagerBeforeMethodInterceptor( + AuthorizationMethodPointcuts.forAnnotations(Secured.class), manager); + } + + public static AuthorizationMethodInterceptor jsr250() { + return jsr250(new Jsr250AuthorizationManager()); + } + + public static AuthorizationMethodInterceptor jsr250(Jsr250AuthorizationManager manager) { + return new AuthorizationManagerBeforeMethodInterceptor( + AuthorizationMethodPointcuts.forAnnotations(DenyAll.class, PermitAll.class, RolesAllowed.class), + manager); + } + + private AuthorizationMethodInterceptors() { + + } + +} diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInvocation.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInvocation.java new file mode 100644 index 0000000000..d3976b406f --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodInvocation.java @@ -0,0 +1,123 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authorization.method; + +import java.lang.reflect.AccessibleObject; +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.List; +import java.util.function.Supplier; + +import org.aopalliance.intercept.MethodInvocation; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.AopUtils; +import org.springframework.core.log.LogMessage; +import org.springframework.security.core.Authentication; + +/** + * @author Josh Cummings + */ +class AuthorizationMethodInvocation implements MethodInvocation { + + private final Log logger = LogFactory.getLog(getClass()); + + private final Supplier authentication; + + private final MethodInvocation methodInvocation; + + private final Class targetClass; + + private final List interceptors; + + private final int size; + + private int currentPosition = 0; + + AuthorizationMethodInvocation(Supplier authentication, MethodInvocation methodInvocation) { + this(authentication, methodInvocation, Collections.emptyList()); + } + + AuthorizationMethodInvocation(Supplier authentication, MethodInvocation methodInvocation, + List interceptors) { + this.authentication = authentication; + this.methodInvocation = methodInvocation; + this.interceptors = interceptors; + Object target = methodInvocation.getThis(); + this.targetClass = (target != null) ? AopUtils.getTargetClass(target) : null; + this.size = interceptors.size(); + } + + @Override + public Method getMethod() { + return this.methodInvocation.getMethod(); + } + + @Override + public Object[] getArguments() { + return this.methodInvocation.getArguments(); + } + + /** + * Return the target class. + * @return the target class + */ + Class getTargetClass() { + return this.targetClass; + } + + @Override + public Object proceed() throws Throwable { + if (this.currentPosition == this.size) { + if (this.logger.isDebugEnabled()) { + this.logger.debug(LogMessage.of(() -> "Pre-Authorized " + this.methodInvocation.getMethod())); + } + return this.methodInvocation.proceed(); + } + AuthorizationMethodInterceptor interceptor = this.interceptors.get(this.currentPosition); + this.currentPosition++; + Pointcut pointcut = interceptor.getPointcut(); + if (!pointcut.getClassFilter().matches(getTargetClass())) { + return proceed(); + } + if (!pointcut.getMethodMatcher().matches(getMethod(), getTargetClass())) { + return proceed(); + } + if (this.logger.isTraceEnabled()) { + this.logger.trace(LogMessage.format("Applying %s (%d/%d)", interceptor.getClass().getSimpleName(), + this.currentPosition, this.size)); + } + Object result = interceptor.invoke(this.authentication, this); + if (this.logger.isDebugEnabled()) { + this.logger.debug(LogMessage.of(() -> "Post-Authorized " + this.methodInvocation.getMethod())); + } + return result; + } + + @Override + public Object getThis() { + return this.methodInvocation.getThis(); + } + + @Override + public AccessibleObject getStaticPart() { + return this.methodInvocation.getStaticPart(); + } + +} diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodPointcuts.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodPointcuts.java new file mode 100644 index 0000000000..e764d95d83 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationMethodPointcuts.java @@ -0,0 +1,54 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authorization.method; + +import java.lang.annotation.Annotation; + +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.ComposablePointcut; +import org.springframework.aop.support.Pointcuts; +import org.springframework.aop.support.annotation.AnnotationMatchingPointcut; + +/** + * @author Josh Cummings + */ +final class AuthorizationMethodPointcuts { + + @SafeVarargs + static Pointcut forAnnotations(Class... annotations) { + ComposablePointcut pointcut = null; + for (Class annotation : annotations) { + if (pointcut == null) { + pointcut = new ComposablePointcut(classOrMethod(annotation)); + } + else { + pointcut.union(classOrMethod(annotation)); + } + } + return pointcut; + } + + private static Pointcut classOrMethod(Class annotation) { + return Pointcuts.union(new AnnotationMatchingPointcut(null, annotation, true), + new AnnotationMatchingPointcut(annotation, true)); + } + + private AuthorizationMethodPointcuts() { + + } + +} 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 deleted file mode 100644 index 62acbc1657..0000000000 --- a/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdvice.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authorization.method; - -import java.util.List; -import java.util.function.Supplier; - -import org.aopalliance.intercept.MethodInvocation; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - -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 - * {@link AuthorizationMethodAfterAdvice}s and returns the result (possibly modified) from - * the {@link MethodInvocation}. - * - * @author Evgeniy Cheban - * @author Josh Cummings - * @since 5.5 - */ -public final class DelegatingAuthorizationMethodAfterAdvice implements AuthorizationMethodAfterAdvice { - - private final Log logger = LogFactory.getLog(getClass()); - - private final Pointcut pointcut; - - private final List> delegates; - - /** - * Creates an instance. - * @param delegates the {@link AuthorizationMethodAfterAdvice}s to use - */ - 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 Pointcut getPointcut() { - return this.pointcut; - } - - /** - * Delegate to a series of {@link AuthorizationMethodAfterAdvice}s, each of which may - * replace the {@code returnedObject} with its own - * - * Advices may be of type {@link AuthorizationManagerMethodAfterAdvice} in which case, - * they will throw an - * {@link org.springframework.security.access.AccessDeniedException} in the event that - * they deny access to the {@code returnedObject}. - * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param object the {@link MethodAuthorizationContext} to check - * @param returnedObject the returned object from the original method invocation - * @throws org.springframework.security.access.AccessDeniedException if any delegate - * advices deny access - */ - @Override - public Object after(Supplier authentication, T object, Object returnedObject) { - if (this.logger.isTraceEnabled()) { - this.logger.trace(LogMessage.format("Post Authorizing %s from %s", returnedObject, object)); - } - Object result = returnedObject; - for (AuthorizationMethodAfterAdvice delegate : this.delegates) { - if (this.logger.isTraceEnabled()) { - this.logger.trace( - LogMessage.format("Checking authorization on %s from %s using %s", result, object, delegate)); - } - result = delegate.after(authentication, object, result); - } - return result; - } - -} 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 deleted file mode 100644 index df8705aaa4..0000000000 --- a/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdvice.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authorization.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.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 - * {@link AuthorizationMethodBeforeAdvice} and grants access if all - * {@link AuthorizationMethodBeforeAdvice}s granted or abstained. Denies access only if - * one of the {@link AuthorizationMethodBeforeAdvice}s denied. - * - * @author Evgeniy Cheban - * @author Josh Cummings - * @since 5.5 - */ -public final class DelegatingAuthorizationMethodBeforeAdvice implements AuthorizationMethodBeforeAdvice { - - private final Log logger = LogFactory.getLog(getClass()); - - private final Pointcut pointcut; - - private final List> delegates; - - /** - * Creates an instance. - * @param delegates the {@link AuthorizationMethodBeforeAdvice}s to use - */ - 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 Pointcut getPointcut() { - return this.pointcut; - } - - /** - * Delegate to a series of {@link AuthorizationMethodBeforeAdvice}s - * - * Advices may be of type {@link AuthorizationManagerMethodBeforeAdvice} in which - * case, they will throw an - * {@link org.springframework.security.access.AccessDeniedException} in the event that - * they deny access. - * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param object the {@link MethodAuthorizationContext} to check - * @throws org.springframework.security.access.AccessDeniedException if any delegate - * advices deny access - */ - @Override - public void before(Supplier authentication, T object) { - if (this.logger.isTraceEnabled()) { - this.logger.trace(LogMessage.format("Pre Authorizing %s", object)); - } - for (AuthorizationMethodBeforeAdvice delegate : this.delegates) { - if (this.logger.isTraceEnabled()) { - this.logger.trace(LogMessage.format("Checking authorization on %s using %s", object, delegate)); - } - delegate.before(authentication, object); - } - } - -} diff --git a/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodInterceptor.java new file mode 100644 index 0000000000..e00e8a9922 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodInterceptor.java @@ -0,0 +1,89 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authorization.method; + +import java.util.Arrays; +import java.util.List; +import java.util.function.Supplier; + +import org.aopalliance.intercept.MethodInvocation; + +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.ComposablePointcut; +import org.springframework.security.core.Authentication; + +/** + * Provides security interception of AOP Alliance based method invocations. + * + * Delegates to a collection of {@link AuthorizationMethodInterceptor}s + * + * @author Evgeniy Cheban + * @author Josh Cummings + * @since 5.5 + */ +public final class DelegatingAuthorizationMethodInterceptor implements AuthorizationMethodInterceptor { + + private final List interceptors; + + private final Pointcut pointcut; + + /** + * Creates an instance using the provided parameters + * @param interceptors the delegate {@link AuthorizationMethodInterceptor}s to use + */ + public DelegatingAuthorizationMethodInterceptor(AuthorizationMethodInterceptor... interceptors) { + this(Arrays.asList(interceptors)); + } + + /** + * Creates an instance using the provided parameters + * @param interceptors the delegate {@link AuthorizationMethodInterceptor}s to use + */ + public DelegatingAuthorizationMethodInterceptor(List interceptors) { + ComposablePointcut pointcut = null; + for (AuthorizationMethodInterceptor interceptor : interceptors) { + if (pointcut == null) { + pointcut = new ComposablePointcut(interceptor.getPointcut()); + } + else { + pointcut.union(interceptor.getPointcut()); + } + } + this.pointcut = pointcut; + this.interceptors = interceptors; + } + + /** + * Enforce security on this {@link MethodInvocation}. + * @param mi the method being invoked which requires a security decision + * @return the returned value from the {@link MethodInvocation}, possibly altered by + * the configured {@link AuthorizationMethodInterceptor}s + */ + @Override + public Object invoke(Supplier authentication, MethodInvocation mi) throws Throwable { + return new AuthorizationMethodInvocation(authentication, mi, this.interceptors).proceed(); + } + + /** + * {@inheritDoc} + */ + @Override + public Pointcut getPointcut() { + return this.pointcut; + } + +} diff --git a/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java index 06cc404a18..c099e4d70f 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/method/Jsr250AuthorizationManager.java @@ -39,14 +39,14 @@ import org.springframework.security.core.Authentication; import org.springframework.util.Assert; /** - * An {@link AuthorizationManager} which can determine if an {@link Authentication} has - * access to the {@link MethodInvocation} by evaluating if the {@link Authentication} + * An {@link AuthorizationManager} which can determine if an {@link Authentication} may + * invoke the {@link MethodInvocation} by evaluating if the {@link Authentication} * contains a specified authority from the JSR-250 security annotations. * * @author Evgeniy Cheban * @since 5.5 */ -public final class Jsr250AuthorizationManager implements AuthorizationManager { +public final class Jsr250AuthorizationManager implements AuthorizationManager { private static final Set> JSR250_ANNOTATIONS = new HashSet<>(); @@ -72,25 +72,24 @@ public final class Jsr250AuthorizationManager implements AuthorizationManager authentication, - MethodAuthorizationContext methodAuthorizationContext) { - AuthorizationManager delegate = this.registry - .getManager(methodAuthorizationContext); - return delegate.check(authentication, methodAuthorizationContext); + public AuthorizationDecision check(Supplier authentication, MethodInvocation methodInvocation) { + AuthorizationManager delegate = this.registry + .getManager((AuthorizationMethodInvocation) methodInvocation); + return delegate.check(authentication, methodInvocation); } private final class Jsr250AuthorizationManagerRegistry extends AbstractAuthorizationManagerRegistry { @NonNull @Override - AuthorizationManager resolveManager(Method method, Class targetClass) { + AuthorizationManager resolveManager(Method method, Class targetClass) { for (Annotation annotation : findJsr250Annotations(method, targetClass)) { if (annotation instanceof DenyAll) { return (a, o) -> new AuthorizationDecision(false); diff --git a/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationContext.java b/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationContext.java deleted file mode 100644 index 5b361f2dd5..0000000000 --- a/core/src/main/java/org/springframework/security/authorization/method/MethodAuthorizationContext.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authorization.method; - -import org.aopalliance.intercept.MethodInvocation; - -/** - * An authorization context which is holds the {@link MethodInvocation} and the target - * class - * - * @author Evgeniy Cheban - * @since 5.5 - */ -public final class MethodAuthorizationContext { - - private final MethodInvocation methodInvocation; - - private final Class targetClass; - - /** - * Creates an instance. - * @param methodInvocation the {@link MethodInvocation} to use - * @param targetClass the target class to use - */ - public MethodAuthorizationContext(MethodInvocation methodInvocation, Class targetClass) { - this.methodInvocation = methodInvocation; - this.targetClass = targetClass; - } - - /** - * Return the {@link MethodInvocation}. - * @return the {@link MethodInvocation} - */ - public MethodInvocation getMethodInvocation() { - return this.methodInvocation; - } - - /** - * Return the target class. - * @return the target class - */ - public Class getTargetClass() { - return this.targetClass; - } - - @Override - public String toString() { - return "MethodAuthorizationContext[methodInvocation=" + this.methodInvocation + ", targetClass=" - + this.targetClass + ']'; - } - -} diff --git a/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManager.java index 7fddfa900d..9e486642b8 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManager.java @@ -36,22 +36,21 @@ import org.springframework.security.core.Authentication; import org.springframework.util.Assert; /** - * An {@link AuthorizationManager} which can determine if an {@link Authentication} has - * access to the {@link MethodInvocation} by evaluating an expression from the - * {@link PostAuthorize} annotation. + * An {@link AuthorizationManager} which can determine if an {@link Authentication} may + * return the result from an invoked {@link MethodInvocation} by evaluating an expression + * from the {@link PostAuthorize} annotation. * * @author Evgeniy Cheban * @since 5.5 */ -public final class PostAuthorizeAuthorizationManager - implements AfterMethodAuthorizationManager { +public final class PostAuthorizeAuthorizationManager implements AfterMethodAuthorizationManager { private final PostAuthorizeExpressionAttributeRegistry registry = new PostAuthorizeExpressionAttributeRegistry(); private MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); /** - * Sets the {@link MethodSecurityExpressionHandler}. + * Use this the {@link MethodSecurityExpressionHandler}. * @param expressionHandler the {@link MethodSecurityExpressionHandler} to use */ public void setExpressionHandler(MethodSecurityExpressionHandler expressionHandler) { @@ -62,22 +61,21 @@ public final class PostAuthorizeAuthorizationManager /** * Determine if an {@link Authentication} has access to the returned object by * evaluating the {@link PostAuthorize} annotation that the - * {@link MethodAuthorizationContext} specifies. + * {@link AuthorizationMethodInvocation} specifies. * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check + * @param mi the {@link AuthorizationMethodInvocation} to check * @param returnedObject the returned object to check * @return an {@link AuthorizationDecision} or {@code null} if the * {@link PostAuthorize} annotation is not present */ @Override - public AuthorizationDecision check(Supplier authentication, - MethodAuthorizationContext methodAuthorizationContext, Object returnedObject) { - ExpressionAttribute attribute = this.registry.getAttribute(methodAuthorizationContext); + public AuthorizationDecision check(Supplier authentication, MethodInvocation mi, + Object returnedObject) { + ExpressionAttribute attribute = this.registry.getAttribute((AuthorizationMethodInvocation) mi); if (attribute == ExpressionAttribute.NULL_ATTRIBUTE) { return null; } - EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), - methodAuthorizationContext.getMethodInvocation()); + EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), mi); this.expressionHandler.setReturnObject(returnedObject, ctx); boolean granted = ExpressionUtils.evaluateAsBoolean(attribute.getExpression(), ctx); return new AuthorizationDecision(granted); diff --git a/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptor.java similarity index 75% rename from core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdvice.java rename to core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptor.java index e21c1172da..40ec2b29f4 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptor.java @@ -34,16 +34,15 @@ import org.springframework.security.core.Authentication; import org.springframework.util.Assert; /** - * An {@link AuthorizationMethodAfterAdvice} which filters a returnedObject - * from the {@link MethodInvocation} by evaluating an expression from the - * {@link PostFilter} annotation. + * An {@link AuthorizationMethodInterceptor} which filters a {@code returnedObject} from + * the {@link MethodInvocation} by evaluating an expression from the {@link PostFilter} + * annotation. * * @author Evgeniy Cheban * @author Josh Cummings * @since 5.5 */ -public final class PostFilterAuthorizationMethodAfterAdvice - implements AuthorizationMethodAfterAdvice { +public final class PostFilterAuthorizationMethodInterceptor implements AuthorizationMethodInterceptor { private final PostFilterExpressionAttributeRegistry registry = new PostFilterExpressionAttributeRegistry(); @@ -52,16 +51,15 @@ public final class PostFilterAuthorizationMethodAfterAdvice private MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); /** - * Create a {@link PostFilterAuthorizationMethodAfterAdvice} using the provided + * Creates a {@link PostFilterAuthorizationMethodInterceptor} using the provided * parameters - * @param pointcut the {@link Pointcut} for when this advice applies */ - public PostFilterAuthorizationMethodAfterAdvice(Pointcut pointcut) { - this.pointcut = pointcut; + public PostFilterAuthorizationMethodInterceptor() { + this.pointcut = AuthorizationMethodPointcuts.forAnnotations(PostFilter.class); } /** - * Sets the {@link MethodSecurityExpressionHandler}. + * Use this {@link MethodSecurityExpressionHandler}. * @param expressionHandler the {@link MethodSecurityExpressionHandler} to use */ public void setExpressionHandler(MethodSecurityExpressionHandler expressionHandler) { @@ -79,24 +77,19 @@ public final class PostFilterAuthorizationMethodAfterAdvice /** * Filter a {@code returnedObject} using the {@link PostFilter} annotation that the - * {@link MethodAuthorizationContext} specifies. + * {@link AuthorizationMethodInvocation} specifies. * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check - * check + * @param mi the {@link AuthorizationMethodInvocation} to check check * @return filtered {@code returnedObject} */ @Override - public Object after(Supplier authentication, MethodAuthorizationContext methodAuthorizationContext, - Object returnedObject) { - if (returnedObject == null) { - return null; - } - ExpressionAttribute attribute = this.registry.getAttribute(methodAuthorizationContext); + public Object invoke(Supplier authentication, MethodInvocation mi) throws Throwable { + Object returnedObject = mi.proceed(); + ExpressionAttribute attribute = this.registry.getAttribute((AuthorizationMethodInvocation) mi); if (attribute == ExpressionAttribute.NULL_ATTRIBUTE) { return returnedObject; } - EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), - methodAuthorizationContext.getMethodInvocation()); + EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), mi); return this.expressionHandler.filter(returnedObject, attribute.getExpression(), ctx); } @@ -111,7 +104,7 @@ public final class PostFilterAuthorizationMethodAfterAdvice if (postFilter == null) { return ExpressionAttribute.NULL_ATTRIBUTE; } - Expression postFilterExpression = PostFilterAuthorizationMethodAfterAdvice.this.expressionHandler + Expression postFilterExpression = PostFilterAuthorizationMethodInterceptor.this.expressionHandler .getExpressionParser().parseExpression(postFilter.value()); return new ExpressionAttribute(postFilterExpression); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManager.java index 70d3f44853..9f7fc77cb2 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManager.java @@ -36,14 +36,14 @@ import org.springframework.security.core.Authentication; import org.springframework.util.Assert; /** - * An {@link AuthorizationManager} which can determine if an {@link Authentication} has - * access to the {@link MethodInvocation} by evaluating an expression from the + * An {@link AuthorizationManager} which can determine if an {@link Authentication} may + * invoke the {@link MethodInvocation} by evaluating an expression from the * {@link PreAuthorize} annotation. * * @author Evgeniy Cheban * @since 5.5 */ -public final class PreAuthorizeAuthorizationManager implements AuthorizationManager { +public final class PreAuthorizeAuthorizationManager implements AuthorizationManager { private final PreAuthorizeExpressionAttributeRegistry registry = new PreAuthorizeExpressionAttributeRegistry(); @@ -61,21 +61,19 @@ public final class PreAuthorizeAuthorizationManager implements AuthorizationMana /** * Determine if an {@link Authentication} has access to a method by evaluating an * expression from the {@link PreAuthorize} annotation that the - * {@link MethodAuthorizationContext} specifies. + * {@link AuthorizationMethodInvocation} specifies. * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check + * @param mi the {@link AuthorizationMethodInvocation} to check * @return an {@link AuthorizationDecision} or {@code null} if the * {@link PreAuthorize} annotation is not present */ @Override - public AuthorizationDecision check(Supplier authentication, - MethodAuthorizationContext methodAuthorizationContext) { - ExpressionAttribute attribute = this.registry.getAttribute(methodAuthorizationContext); + public AuthorizationDecision check(Supplier authentication, MethodInvocation mi) { + ExpressionAttribute attribute = this.registry.getAttribute((AuthorizationMethodInvocation) mi); if (attribute == ExpressionAttribute.NULL_ATTRIBUTE) { return null; } - EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), - methodAuthorizationContext.getMethodInvocation()); + EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), mi); boolean granted = ExpressionUtils.evaluateAsBoolean(attribute.getExpression(), ctx); return new AuthorizationDecision(granted); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdvice.java b/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptor.java similarity index 84% rename from core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdvice.java rename to core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptor.java index 306bd78d5b..0e56e4d47f 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdvice.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptor.java @@ -35,15 +35,14 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** - * An {@link AuthorizationMethodBeforeAdvice} which filters a method argument by - * evaluating an expression from the {@link PreFilter} annotation. + * An {@link AuthorizationMethodInterceptor} which filters a method argument by evaluating + * an expression from the {@link PreFilter} annotation. * * @author Evgeniy Cheban * @author Josh Cummings * @since 5.5 */ -public final class PreFilterAuthorizationMethodBeforeAdvice - implements AuthorizationMethodBeforeAdvice { +public final class PreFilterAuthorizationMethodInterceptor implements AuthorizationMethodInterceptor { private final PreFilterExpressionAttributeRegistry registry = new PreFilterExpressionAttributeRegistry(); @@ -52,12 +51,11 @@ public final class PreFilterAuthorizationMethodBeforeAdvice private MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); /** - * Creates a {@link PreFilterAuthorizationMethodBeforeAdvice} using the provided + * Creates a {@link PreFilterAuthorizationMethodInterceptor} using the provided * parameters - * @param pointcut the {@link Pointcut} for when this advice applies */ - public PreFilterAuthorizationMethodBeforeAdvice(Pointcut pointcut) { - this.pointcut = pointcut; + public PreFilterAuthorizationMethodInterceptor() { + this.pointcut = AuthorizationMethodPointcuts.forAnnotations(PreFilter.class); } /** @@ -79,20 +77,20 @@ public final class PreFilterAuthorizationMethodBeforeAdvice /** * Filter the method argument specified in the {@link PreFilter} annotation that - * {@link MethodAuthorizationContext} specifies. + * {@link AuthorizationMethodInvocation} specifies. * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check + * @param mi the {@link AuthorizationMethodInvocation} to check */ @Override - public void before(Supplier authentication, MethodAuthorizationContext methodAuthorizationContext) { - PreFilterExpressionAttribute attribute = this.registry.getAttribute(methodAuthorizationContext); + public Object invoke(Supplier authentication, MethodInvocation mi) throws Throwable { + PreFilterExpressionAttribute attribute = this.registry.getAttribute((AuthorizationMethodInvocation) mi); if (attribute == PreFilterExpressionAttribute.NULL_ATTRIBUTE) { - return; + return mi.proceed(); } - MethodInvocation mi = methodAuthorizationContext.getMethodInvocation(); EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), mi); Object filterTarget = findFilterTarget(attribute.filterTarget, ctx, mi); this.expressionHandler.filter(filterTarget, attribute.getExpression(), ctx); + return mi.proceed(); } private Object findFilterTarget(String filterTargetName, EvaluationContext ctx, MethodInvocation methodInvocation) { @@ -126,7 +124,7 @@ public final class PreFilterAuthorizationMethodBeforeAdvice if (preFilter == null) { return PreFilterExpressionAttribute.NULL_ATTRIBUTE; } - Expression preFilterExpression = PreFilterAuthorizationMethodBeforeAdvice.this.expressionHandler + Expression preFilterExpression = PreFilterAuthorizationMethodInterceptor.this.expressionHandler .getExpressionParser().parseExpression(preFilter.value()); return new PreFilterExpressionAttribute(preFilterExpression, preFilter.filterTarget()); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java index 400912e7be..b05529d6f9 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/method/SecuredAuthorizationManager.java @@ -31,38 +31,36 @@ import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; /** - * An {@link AuthorizationManager} which can determine if an {@link Authentication} has - * access to the {@link MethodInvocation} by evaluating if the {@link Authentication} + * An {@link AuthorizationManager} which can determine if an {@link Authentication} may + * invoke the {@link MethodInvocation} by evaluating if the {@link Authentication} * contains a specified authority from the Spring Security's {@link Secured} annotation. * * @author Evgeniy Cheban * @since 5.5 */ -public final class SecuredAuthorizationManager implements AuthorizationManager { +public final class SecuredAuthorizationManager implements AuthorizationManager { private final SecuredAuthorizationManagerRegistry registry = new SecuredAuthorizationManagerRegistry(); /** * Determine if an {@link Authentication} has access to a method by evaluating the - * {@link Secured} annotation that {@link MethodAuthorizationContext} specifies. + * {@link Secured} annotation that {@link AuthorizationMethodInvocation} specifies. * @param authentication the {@link Supplier} of the {@link Authentication} to check - * @param methodAuthorizationContext the {@link MethodAuthorizationContext} to check + * @param mi the {@link AuthorizationMethodInvocation} to check * @return an {@link AuthorizationDecision} or null if the {@link Secured} annotation * is not present */ @Override - public AuthorizationDecision check(Supplier authentication, - MethodAuthorizationContext methodAuthorizationContext) { - AuthorizationManager delegate = this.registry - .getManager(methodAuthorizationContext); - return delegate.check(authentication, methodAuthorizationContext); + public AuthorizationDecision check(Supplier authentication, MethodInvocation mi) { + AuthorizationManager delegate = this.registry.getManager((AuthorizationMethodInvocation) mi); + return delegate.check(authentication, mi); } private static final class SecuredAuthorizationManagerRegistry extends AbstractAuthorizationManagerRegistry { @NonNull @Override - AuthorizationManager resolveManager(Method method, Class targetClass) { + AuthorizationManager resolveManager(Method method, Class targetClass) { Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass); Secured secured = findSecuredAnnotation(specificMethod); return (secured != null) ? AuthorityAuthorizationManager.hasAnyAuthority(secured.value()) : NULL_MANAGER; diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java similarity index 76% rename from core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdviceTests.java rename to core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java index 8764cfb1ba..0c0519a3c3 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodAfterAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerAfterMethodInterceptorTests.java @@ -27,42 +27,44 @@ import org.springframework.security.core.Authentication; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; /** - * Tests for {@link AuthorizationManagerMethodAfterAdvice}. + * Tests for {@link AuthorizationManagerAfterMethodInterceptor}. * * @author Evgeniy Cheban */ -public class AuthorizationManagerMethodAfterAdviceTests { +public class AuthorizationManagerAfterMethodInterceptorTests { @Test public void instantiateWhenMethodMatcherNullThenException() { AfterMethodAuthorizationManager mockAuthorizationManager = mock( AfterMethodAuthorizationManager.class); assertThatIllegalArgumentException() - .isThrownBy(() -> new AuthorizationManagerMethodAfterAdvice<>(null, mockAuthorizationManager)) + .isThrownBy(() -> new AuthorizationManagerAfterMethodInterceptor(null, mockAuthorizationManager)) .withMessage("pointcut cannot be null"); } @Test public void instantiateWhenAuthorizationManagerNullThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new AuthorizationManagerMethodAfterAdvice<>(mock(Pointcut.class), null)) + .isThrownBy(() -> new AuthorizationManagerAfterMethodInterceptor(mock(Pointcut.class), null)) .withMessage("authorizationManager cannot be null"); } @Test - public void beforeWhenMockAuthorizationManagerThenVerifyAndReturnedObject() { + public void beforeWhenMockAuthorizationManagerThenVerifyAndReturnedObject() throws Throwable { Supplier authentication = TestAuthentication::authenticatedUser; MethodInvocation mockMethodInvocation = mock(MethodInvocation.class); Object returnedObject = new Object(); + given(mockMethodInvocation.proceed()).willReturn(returnedObject); AfterMethodAuthorizationManager mockAuthorizationManager = mock( AfterMethodAuthorizationManager.class); - AuthorizationManagerMethodAfterAdvice advice = new AuthorizationManagerMethodAfterAdvice<>( - mock(Pointcut.class), mockAuthorizationManager); - Object result = advice.after(authentication, mockMethodInvocation, returnedObject); + AuthorizationManagerAfterMethodInterceptor advice = new AuthorizationManagerAfterMethodInterceptor( + Pointcut.TRUE, mockAuthorizationManager); + Object result = advice.invoke(authentication, mockMethodInvocation); assertThat(result).isEqualTo(returnedObject); verify(mockAuthorizationManager).verify(authentication, mockMethodInvocation, returnedObject); } diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java similarity index 75% rename from core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdviceTests.java rename to core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java index 3f4d5ed2f2..f205d75d43 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerMethodBeforeAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeMethodInterceptorTests.java @@ -31,34 +31,35 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; /** - * Tests for {@link AuthorizationManagerMethodBeforeAdvice}. + * Tests for {@link AuthorizationManagerBeforeMethodInterceptor}. * * @author Evgeniy Cheban */ -public class AuthorizationManagerMethodBeforeAdviceTests { +public class AuthorizationManagerBeforeMethodInterceptorTests { @Test public void instantiateWhenMethodMatcherNullThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new AuthorizationManagerMethodBeforeAdvice<>(null, mock(AuthorizationManager.class))) + .isThrownBy( + () -> new AuthorizationManagerBeforeMethodInterceptor(null, mock(AuthorizationManager.class))) .withMessage("pointcut cannot be null"); } @Test public void instantiateWhenAuthorizationManagerNullThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new AuthorizationManagerMethodBeforeAdvice<>(mock(Pointcut.class), null)) + .isThrownBy(() -> new AuthorizationManagerBeforeMethodInterceptor(mock(Pointcut.class), null)) .withMessage("authorizationManager cannot be null"); } @Test - public void beforeWhenMockAuthorizationManagerThenVerify() { + public void beforeWhenMockAuthorizationManagerThenVerify() throws Throwable { Supplier authentication = TestAuthentication::authenticatedUser; MethodInvocation mockMethodInvocation = mock(MethodInvocation.class); AuthorizationManager mockAuthorizationManager = mock(AuthorizationManager.class); - AuthorizationManagerMethodBeforeAdvice advice = new AuthorizationManagerMethodBeforeAdvice<>( - mock(Pointcut.class), mockAuthorizationManager); - advice.before(authentication, mockMethodInvocation); + AuthorizationManagerBeforeMethodInterceptor advice = new AuthorizationManagerBeforeMethodInterceptor( + Pointcut.TRUE, mockAuthorizationManager); + advice.invoke(authentication, mockMethodInvocation); verify(mockAuthorizationManager).verify(authentication, mockMethodInvocation); } diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodPointcutsTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodPointcutsTests.java new file mode 100644 index 0000000000..b2bb8de7b3 --- /dev/null +++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodPointcutsTests.java @@ -0,0 +1,167 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authorization.method; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.junit.Test; + +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.AopUtils; +import org.springframework.security.access.prepost.PreAuthorize; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link AuthorizationMethodPointcuts} + */ +public class AuthorizationMethodPointcutsTests { + + @Test + public void forAnnotationsWhenAnnotationThenClassBasedAnnotationPointcut() { + Pointcut preAuthorize = AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class); + assertThat(AopUtils.canApply(preAuthorize, ClassController.class)).isTrue(); + assertThat(AopUtils.canApply(preAuthorize, NoController.class)).isFalse(); + } + + @Test + public void forAnnotationsWhenAnnotationThenMethodBasedAnnotationPointcut() { + Pointcut preAuthorize = AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class); + assertThat(AopUtils.canApply(preAuthorize, MethodController.class)).isTrue(); + } + + @Test + public void forAnnotationsWhenAnnotationThenClassInheritancePointcut() { + Pointcut preAuthorize = AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class); + assertThat(AopUtils.canApply(preAuthorize, InterfacedClassController.class)).isTrue(); + } + + @Test + public void forAnnotationsWhenAnnotationThenMethodInheritancePointcut() { + Pointcut preAuthorize = AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class); + assertThat(AopUtils.canApply(preAuthorize, InterfacedMethodController.class)).isTrue(); + } + + @Test + public void forAnnotationsWhenAnnotationThenAnnotationClassInheritancePointcut() { + Pointcut preAuthorize = AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class); + assertThat(AopUtils.canApply(preAuthorize, InterfacedAnnotationClassController.class)).isTrue(); + } + + @Test + public void forAnnotationsWhenAnnotationThenAnnotationMethodInheritancePointcut() { + Pointcut preAuthorize = AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class); + assertThat(AopUtils.canApply(preAuthorize, InterfacedAnnotationMethodController.class)).isTrue(); + } + + @PreAuthorize("hasAuthority('APP')") + public static class ClassController { + + String methodOne(String paramOne) { + return "value"; + } + + } + + public static class MethodController { + + @PreAuthorize("hasAuthority('APP')") + String methodOne(String paramOne) { + return "value"; + } + + } + + public static class NoController { + + String methodOne(String paramOne) { + return "value"; + } + + } + + @PreAuthorize("hasAuthority('APP')") + public interface ClassControllerInterface { + + String methodOne(String paramOne); + + } + + public static class InterfacedClassController implements ClassControllerInterface { + + public String methodOne(String paramOne) { + return "value"; + } + + } + + public interface MethodControllerInterface { + + @PreAuthorize("hasAuthority('APP')") + String methodOne(String paramOne); + + } + + public static class InterfacedMethodController implements MethodControllerInterface { + + public String methodOne(String paramOne) { + return "value"; + } + + } + + @Target({ ElementType.METHOD, ElementType.TYPE }) + @Retention(RetentionPolicy.RUNTIME) + @PreAuthorize("hasAuthority('APP')") + @interface MyAnnotation { + + } + + @MyAnnotation + public interface ClassAnnotationControllerInterface { + + String methodOne(String paramOne); + + } + + public static class InterfacedAnnotationClassController implements ClassAnnotationControllerInterface { + + public String methodOne(String paramOne) { + return "value"; + } + + } + + public interface MethodAnnotationControllerInterface { + + @MyAnnotation + String methodOne(String paramOne); + + } + + public static class InterfacedAnnotationMethodController implements MethodAnnotationControllerInterface { + + public String methodOne(String paramOne) { + return "value"; + } + + } + +} 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 deleted file mode 100644 index 8560cbeef2..0000000000 --- a/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodAfterAdviceTests.java +++ /dev/null @@ -1,165 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authorization.method; - -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.List; -import java.util.function.Supplier; - -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.intercept.method.MockMethodInvocation; -import org.springframework.security.authentication.TestAuthentication; -import org.springframework.security.core.Authentication; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * Tests for {@link DelegatingAuthorizationMethodAfterAdvice}. - * - * @author Evgeniy Cheban - */ -public class DelegatingAuthorizationMethodAfterAdviceTests { - - @Test - public void methodMatcherWhenNoneMatchesThenNotMatches() throws Exception { - List> delegates = new ArrayList<>(); - delegates.add(new AuthorizationMethodAfterAdvice() { - @Override - public Object after(Supplier authentication, MethodAuthorizationContext object, - Object returnedObject) { - return returnedObject; - } - - @Override - public Pointcut getPointcut() { - return new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }; - } - }); - delegates.add(new AuthorizationMethodAfterAdvice() { - @Override - public Object after(Supplier authentication, MethodAuthorizationContext object, - Object returnedObject) { - return returnedObject; - } - - @Override - public Pointcut getPointcut() { - return new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }; - } - }); - DelegatingAuthorizationMethodAfterAdvice advice = new DelegatingAuthorizationMethodAfterAdvice(delegates); - MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); - assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); - } - - @Test - public void methodMatcherWhenAnyMatchesThenMatches() throws Exception { - List> delegates = new ArrayList<>(); - delegates.add(new AuthorizationMethodAfterAdvice() { - @Override - public Object after(Supplier authentication, MethodAuthorizationContext object, - Object returnedObject) { - return returnedObject; - } - - @Override - public Pointcut getPointcut() { - return new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }; - } - }); - delegates.add(new AuthorizationMethodAfterAdvice() { - @Override - public Object after(Supplier authentication, MethodAuthorizationContext object, - Object returnedObject) { - return returnedObject; - } - - @Override - public Pointcut getPointcut() { - return Pointcut.TRUE; - } - }); - DelegatingAuthorizationMethodAfterAdvice advice = new DelegatingAuthorizationMethodAfterAdvice(delegates); - MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); - assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isTrue(); - } - - @Test - public void checkWhenDelegatingAdviceModifiesReturnedObjectThenModifiedReturnedObject() throws Exception { - List> delegates = new ArrayList<>(); - delegates.add(new AuthorizationMethodAfterAdvice() { - @Override - public Object after(Supplier authentication, MethodAuthorizationContext object, - Object returnedObject) { - return returnedObject + "b"; - } - - @Override - public Pointcut getPointcut() { - return Pointcut.TRUE; - } - }); - delegates.add(new AuthorizationMethodAfterAdvice() { - @Override - public Object after(Supplier authentication, MethodAuthorizationContext object, - Object returnedObject) { - return returnedObject + "c"; - } - - @Override - public Pointcut getPointcut() { - return Pointcut.TRUE; - } - }); - MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, - "doSomething"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - DelegatingAuthorizationMethodAfterAdvice advice = new DelegatingAuthorizationMethodAfterAdvice(delegates); - Object result = advice.after(TestAuthentication::authenticatedUser, methodAuthorizationContext, "a"); - assertThat(result).isEqualTo("abc"); - } - - public static class TestClass { - - public String doSomething() { - return null; - } - - } - -} 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 deleted file mode 100644 index e88635aff7..0000000000 --- a/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodBeforeAdviceTests.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authorization.method; - -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.function.Supplier; - -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.AccessDeniedException; -import org.springframework.security.access.intercept.method.MockMethodInvocation; -import org.springframework.security.authentication.TestAuthentication; -import org.springframework.security.authorization.AuthorizationDecision; -import org.springframework.security.core.Authentication; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; - -/** - * Tests for {@link DelegatingAuthorizationMethodBeforeAdvice}. - * - * @author Evgeniy Cheban - */ -public class DelegatingAuthorizationMethodBeforeAdviceTests { - - @Test - public void methodMatcherWhenNoneMatchesThenNotMatches() throws Exception { - List> delegates = new ArrayList<>(); - delegates.add(new AuthorizationMethodBeforeAdvice() { - @Override - public Pointcut getPointcut() { - return new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }; - } - - @Override - public void before(Supplier authentication, MethodAuthorizationContext object) { - } - }); - delegates.add(new AuthorizationMethodBeforeAdvice() { - @Override - public Pointcut getPointcut() { - return new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }; - } - - @Override - public void before(Supplier authentication, MethodAuthorizationContext object) { - } - }); - DelegatingAuthorizationMethodBeforeAdvice advice = new DelegatingAuthorizationMethodBeforeAdvice(delegates); - MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); - assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); - } - - @Test - public void methodMatcherWhenAnyMatchesThenMatches() throws Exception { - List> delegates = new ArrayList<>(); - delegates.add(new AuthorizationMethodBeforeAdvice() { - @Override - public Pointcut getPointcut() { - return new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }; - } - - @Override - public void before(Supplier authentication, MethodAuthorizationContext object) { - } - }); - delegates.add(new AuthorizationMethodBeforeAdvice() { - @Override - public Pointcut getPointcut() { - return Pointcut.TRUE; - } - - @Override - public void before(Supplier authentication, MethodAuthorizationContext object) { - } - }); - DelegatingAuthorizationMethodBeforeAdvice advice = new DelegatingAuthorizationMethodBeforeAdvice(delegates); - 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<>(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"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext); - } - - @Test - public void checkWhenAnyDeniesThenAccessDeniedException() throws Exception { - List> delegates = new ArrayList<>(); - 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, - "doSomething"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - assertThatExceptionOfType(AccessDeniedException.class) - .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) - .withMessage("Access Denied"); - } - - @Test - public void checkWhenDelegatesEmptyThenFails() { - assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> new DelegatingAuthorizationMethodBeforeAdvice(Collections.emptyList())); - } - - public static class TestClass { - - public void doSomething() { - - } - - } - -} diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptorTests.java b/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodInterceptorTests.java similarity index 65% rename from core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptorTests.java rename to core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodInterceptorTests.java index 387424e66d..6ee0d066b0 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationMethodInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/DelegatingAuthorizationMethodInterceptorTests.java @@ -16,8 +16,10 @@ package org.springframework.security.authorization.method; +import java.util.Arrays; import java.util.function.Supplier; +import org.aopalliance.intercept.MethodInvocation; import org.junit.After; import org.junit.Test; @@ -32,18 +34,17 @@ import org.springframework.security.core.context.SecurityContextImpl; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; /** - * Tests for {@link AuthorizationMethodInterceptor}. + * Tests for {@link DelegatingAuthorizationMethodInterceptor}. * * @author Evgeniy Cheban */ -public class AuthorizationMethodInterceptorTests { +public class DelegatingAuthorizationMethodInterceptorTests { @After public void tearDown() { @@ -56,41 +57,39 @@ public class AuthorizationMethodInterceptorTests { SecurityContextHolder.setContext(new SecurityContextImpl(authentication)); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingString"); - AuthorizationMethodBeforeAdvice mockBeforeAdvice = mock( - AuthorizationMethodBeforeAdvice.class); - AuthorizationMethodAfterAdvice mockAfterAdvice = mock( - AuthorizationMethodAfterAdvice.class); - given(mockAfterAdvice.after(any(), any(MethodAuthorizationContext.class), eq(null))).willReturn("abc"); - AuthorizationMethodInterceptor interceptor = new AuthorizationMethodInterceptor(mockBeforeAdvice, - mockAfterAdvice); - Object result = interceptor.invoke(mockMethodInvocation); + AuthorizationMethodInterceptor interceptor = mock(AuthorizationMethodInterceptor.class); + given(interceptor.getPointcut()).willReturn(Pointcut.TRUE); + given(interceptor.invoke(any(), any(AuthorizationMethodInvocation.class))).willReturn("abc"); + DelegatingAuthorizationMethodInterceptor chain = new DelegatingAuthorizationMethodInterceptor( + Arrays.asList(interceptor)); + Object result = chain.invoke(mockMethodInvocation); assertThat(result).isEqualTo("abc"); - verify(mockAfterAdvice).after(any(), any(MethodAuthorizationContext.class), eq(null)); + verify(interceptor).invoke(any(), any(AuthorizationMethodInvocation.class)); } @Test - public void invokeWhenNotAuthenticatedThenAuthenticationCredentialsNotFoundException() throws Exception { + public void invokeWhenNotAuthenticatedThenAuthenticationCredentialsNotFoundException() throws Throwable { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingString"); - AuthorizationMethodBeforeAdvice beforeAdvice = new AuthorizationMethodBeforeAdvice() { + AuthorizationMethodInterceptor first = new AuthorizationMethodInterceptor() { @Override public Pointcut getPointcut() { return Pointcut.TRUE; } @Override - public void before(Supplier authentication, - MethodAuthorizationContext methodAuthorizationContext) { - authentication.get(); + public Object invoke(Supplier authentication, MethodInvocation mi) { + return authentication.get(); } }; - AuthorizationMethodAfterAdvice mockAfterAdvice = mock( - AuthorizationMethodAfterAdvice.class); - AuthorizationMethodInterceptor interceptor = new AuthorizationMethodInterceptor(beforeAdvice, mockAfterAdvice); + AuthorizationMethodInterceptor second = mock(AuthorizationMethodInterceptor.class); + given(second.getPointcut()).willReturn(Pointcut.TRUE); + DelegatingAuthorizationMethodInterceptor interceptor = new DelegatingAuthorizationMethodInterceptor( + Arrays.asList(first, second)); assertThatExceptionOfType(AuthenticationCredentialsNotFoundException.class) .isThrownBy(() -> interceptor.invoke(mockMethodInvocation)) .withMessage("An Authentication object was not found in the SecurityContext"); - verifyNoInteractions(mockAfterAdvice); + verify(second, times(0)).invoke(any(), any()); } public static class TestClass { diff --git a/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java index f4414f5bdd..0b9c63e708 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/Jsr250AuthorizationManagerTests.java @@ -16,6 +16,7 @@ package org.springframework.security.authorization.method; +import java.util.Collections; import java.util.function.Supplier; import javax.annotation.security.DenyAll; @@ -62,64 +63,59 @@ public class Jsr250AuthorizationManagerTests { @Test public void checkDoSomethingWhenNoJsr250AnnotationsThenNullDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomething"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNull(); } @Test public void checkPermitAllRolesAllowedAdminWhenRoleUserThenGrantedDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "permitAllRolesAllowedAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @Test public void checkDenyAllRolesAllowedAdminWhenRoleAdminThenDeniedDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "denyAllRolesAllowedAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedAdmin, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedAdmin, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isFalse(); } @Test public void checkRolesAllowedUserOrAdminWhenRoleUserThenGrantedDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "rolesAllowedUserOrAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @Test public void checkRolesAllowedUserOrAdminWhenRoleAdminThenGrantedDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "rolesAllowedUserOrAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedAdmin, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedAdmin, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @@ -128,12 +124,12 @@ public class Jsr250AuthorizationManagerTests { public void checkRolesAllowedUserOrAdminWhenRoleAnonymousThenDeniedDecision() throws Exception { Supplier authentication = () -> new TestingAuthenticationToken("user", "password", "ROLE_ANONYMOUS"); - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "rolesAllowedUserOrAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); Jsr250AuthorizationManager manager = new Jsr250AuthorizationManager(); - AuthorizationDecision decision = manager.check(authentication, methodAuthorizationContext); + AuthorizationDecision decision = manager.check(authentication, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isFalse(); } diff --git a/core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java index c9e589fbfe..4726304056 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/PostAuthorizeAuthorizationManagerTests.java @@ -58,11 +58,10 @@ public class PostAuthorizeAuthorizationManagerTests { public void checkDoSomethingWhenNoPostAuthorizeAnnotationThenNullDecision() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomething", new Class[] {}, new Object[] {}); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext, null); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation, null); assertThat(decision).isNull(); } @@ -70,11 +69,10 @@ public class PostAuthorizeAuthorizationManagerTests { public void checkDoSomethingStringWhenArgIsGrantThenGrantedDecision() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingString", new Class[] { String.class }, new Object[] { "grant" }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext, null); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation, null); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @@ -83,11 +81,10 @@ public class PostAuthorizeAuthorizationManagerTests { public void checkDoSomethingStringWhenArgIsNotGrantThenDeniedDecision() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingString", new Class[] { String.class }, new Object[] { "deny" }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext, null); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation, null); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isFalse(); } @@ -97,11 +94,10 @@ public class PostAuthorizeAuthorizationManagerTests { List list = Arrays.asList("grant", "deny"); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingList", new Class[] { List.class }, new Object[] { list }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext, list); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation, list); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @@ -111,11 +107,10 @@ public class PostAuthorizeAuthorizationManagerTests { List list = Collections.singletonList("deny"); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingList", new Class[] { List.class }, new Object[] { list }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext, list); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation, list); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isFalse(); } diff --git a/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptorTests.java similarity index 68% rename from core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdviceTests.java rename to core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptorTests.java index 209dfc9f0e..a792b5d1a5 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodAfterAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/PostFilterAuthorizationMethodInterceptorTests.java @@ -16,14 +16,12 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.Method; +import java.util.Collections; 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; @@ -34,43 +32,38 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** - * Tests for {@link PostFilterAuthorizationMethodAfterAdvice}. + * Tests for {@link PostFilterAuthorizationMethodInterceptor}. * * @author Evgeniy Cheban */ -public class PostFilterAuthorizationMethodAfterAdviceTests { +public class PostFilterAuthorizationMethodInterceptorTests { @Test public void setExpressionHandlerWhenNotNullThenSetsExpressionHandler() { MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); + PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor(); advice.setExpressionHandler(expressionHandler); assertThat(advice).extracting("expressionHandler").isEqualTo(expressionHandler); } @Test public void setExpressionHandlerWhenNullThenException() { - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); + PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor(); assertThatIllegalArgumentException().isThrownBy(() -> advice.setExpressionHandler(null)) .withMessage("expressionHandler cannot be null"); } @Test public void methodMatcherWhenMethodHasNotPostFilterAnnotationThenNotMatches() throws Exception { - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice( - new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }); + PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor(); MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); - assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); + assertThat(methodMatcher.matches(NoPostFilterClass.class.getMethod("doSomething"), NoPostFilterClass.class)) + .isFalse(); } @Test public void methodMatcherWhenMethodHasPostFilterAnnotationThenMatches() throws Exception { - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); + PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor(); MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat( methodMatcher.matches(TestClass.class.getMethod("doSomethingArray", String[].class), TestClass.class)) @@ -78,24 +71,25 @@ public class PostFilterAuthorizationMethodAfterAdviceTests { } @Test - public void afterWhenArrayNotNullThenFilteredArray() throws Exception { + public void afterWhenArrayNotNullThenFilteredArray() throws Throwable { String[] array = { "john", "bob" }; MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, - "doSomethingArrayClassLevel", new Class[] { String[].class }, new Object[] { array }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PostFilterAuthorizationMethodAfterAdvice advice = new PostFilterAuthorizationMethodAfterAdvice(Pointcut.TRUE); - Object result = advice.after(TestAuthentication::authenticatedUser, methodAuthorizationContext, array); + "doSomethingArrayClassLevel", new Class[] { String[].class }, new Object[] { array }) { + @Override + public Object proceed() { + return array; + } + }; + PostFilterAuthorizationMethodInterceptor advice = new PostFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); + Object result = advice.invoke(TestAuthentication::authenticatedUser, methodInvocation); assertThat(result).asInstanceOf(InstanceOfAssertFactories.array(String[].class)).containsOnly("john"); } @PostFilter("filterObject == 'john'") public static class TestClass { - public void doSomething() { - - } - @PostFilter("filterObject == 'john'") public String[] doSomethingArray(String[] array) { return array; @@ -107,4 +101,12 @@ public class PostFilterAuthorizationMethodAfterAdviceTests { } + public static class NoPostFilterClass { + + public void doSomething() { + + } + + } + } diff --git a/core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java index cbc3b73ba5..b38c48c6bf 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeAuthorizationManagerTests.java @@ -16,6 +16,8 @@ package org.springframework.security.authorization.method; +import java.util.Collections; + import org.junit.Test; import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; @@ -54,11 +56,10 @@ public class PreAuthorizeAuthorizationManagerTests { public void checkDoSomethingWhenNoPostAuthorizeAnnotationThenNullDecision() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomething", new Class[] {}, new Object[] {}); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNull(); } @@ -66,11 +67,10 @@ public class PreAuthorizeAuthorizationManagerTests { public void checkDoSomethingStringWhenArgIsGrantThenGrantedDecision() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingString", new Class[] { String.class }, new Object[] { "grant" }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @@ -79,11 +79,10 @@ public class PreAuthorizeAuthorizationManagerTests { public void checkDoSomethingStringWhenArgIsNotGrantThenDeniedDecision() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingString", new Class[] { String.class }, new Object[] { "deny" }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isFalse(); } diff --git a/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdviceTests.java b/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptorTests.java similarity index 65% rename from core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdviceTests.java rename to core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptorTests.java index 9504ce5d6a..b9d54d8974 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodBeforeAdviceTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/PreFilterAuthorizationMethodInterceptorTests.java @@ -16,15 +16,13 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collections; 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; @@ -36,43 +34,38 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** - * Tests for {@link PreFilterAuthorizationMethodBeforeAdvice}. + * Tests for {@link PreFilterAuthorizationMethodInterceptor}. * * @author Evgeniy Cheban */ -public class PreFilterAuthorizationMethodBeforeAdviceTests { +public class PreFilterAuthorizationMethodInterceptorTests { @Test public void setExpressionHandlerWhenNotNullThenSetsExpressionHandler() { MethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); advice.setExpressionHandler(expressionHandler); assertThat(advice).extracting("expressionHandler").isEqualTo(expressionHandler); } @Test public void setExpressionHandlerWhenNullThenException() { - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); assertThatIllegalArgumentException().isThrownBy(() -> advice.setExpressionHandler(null)) .withMessage("expressionHandler cannot be null"); } @Test public void methodMatcherWhenMethodHasNotPreFilterAnnotationThenNotMatches() throws Exception { - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice( - new StaticMethodMatcherPointcut() { - @Override - public boolean matches(Method method, Class targetClass) { - return false; - } - }); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); - assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomething"), TestClass.class)).isFalse(); + assertThat(methodMatcher.matches(NoPreFilterClass.class.getMethod("doSomething"), NoPreFilterClass.class)) + .isFalse(); } @Test public void methodMatcherWhenMethodHasPreFilterAnnotationThenMatches() throws Exception { - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); MethodMatcher methodMatcher = advice.getPointcut().getMethodMatcher(); assertThat(methodMatcher.matches(TestClass.class.getMethod("doSomethingListFilterTargetMatch", List.class), TestClass.class)).isTrue(); @@ -82,12 +75,11 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { public void findFilterTargetWhenNameProvidedAndNotMatchThenException() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingListFilterTargetNotMatch", new Class[] { List.class }, new Object[] { new ArrayList<>() }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); assertThatIllegalArgumentException() - .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) - .withMessage( + .isThrownBy(() -> advice.invoke(TestAuthentication::authenticatedUser, methodInvocation)).withMessage( "Filter target was null, or no argument with name 'filterTargetNotMatch' found in method."); } @@ -95,25 +87,25 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { public void findFilterTargetWhenNameProvidedAndMatchAndNullThenException() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingListFilterTargetMatch", new Class[] { List.class }, new Object[] { null }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); assertThatIllegalArgumentException() - .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) + .isThrownBy(() -> advice.invoke(TestAuthentication::authenticatedUser, methodInvocation)) .withMessage("Filter target was null, or no argument with name 'list' found in method."); } @Test - public void findFilterTargetWhenNameProvidedAndMatchAndNotNullThenFiltersList() throws Exception { + public void findFilterTargetWhenNameProvidedAndMatchAndNotNullThenFiltersList() throws Throwable { List list = new ArrayList<>(); list.add("john"); list.add("bob"); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingListFilterTargetMatch", new Class[] { List.class }, new Object[] { list }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); - advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); + advice.invoke(TestAuthentication::authenticatedUser, methodInvocation); assertThat(list).hasSize(1); assertThat(list.get(0)).isEqualTo("john"); } @@ -122,25 +114,25 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { public void findFilterTargetWhenNameNotProvidedAndSingleArgListNullThenException() throws Exception { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingListFilterTargetNotProvided", new Class[] { List.class }, new Object[] { null }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); assertThatIllegalArgumentException() - .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) + .isThrownBy(() -> advice.invoke(TestAuthentication::authenticatedUser, methodInvocation)) .withMessage("Filter target was null. Make sure you passing the correct value in the method argument."); } @Test - public void findFilterTargetWhenNameNotProvidedAndSingleArgListThenFiltersList() throws Exception { + public void findFilterTargetWhenNameNotProvidedAndSingleArgListThenFiltersList() throws Throwable { List list = new ArrayList<>(); list.add("john"); list.add("bob"); MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingListFilterTargetNotProvided", new Class[] { List.class }, new Object[] { list }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); - advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); + advice.invoke(TestAuthentication::authenticatedUser, methodInvocation); assertThat(list).hasSize(1); assertThat(list.get(0)).isEqualTo("john"); } @@ -150,12 +142,11 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingArrayFilterTargetNotProvided", new Class[] { String[].class }, new Object[] { new String[] {} }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); assertThatIllegalStateException() - .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) - .withMessage( + .isThrownBy(() -> advice.invoke(TestAuthentication::authenticatedUser, methodInvocation)).withMessage( "Pre-filtering on array types is not supported. Using a Collection will solve this problem."); } @@ -164,21 +155,17 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomethingTwoArgsFilterTargetNotProvided", new Class[] { String.class, List.class }, new Object[] { "", new ArrayList<>() }); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(mockMethodInvocation, - TestClass.class); - PreFilterAuthorizationMethodBeforeAdvice advice = new PreFilterAuthorizationMethodBeforeAdvice(Pointcut.TRUE); + PreFilterAuthorizationMethodInterceptor advice = new PreFilterAuthorizationMethodInterceptor(); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); assertThatIllegalStateException() - .isThrownBy(() -> advice.before(TestAuthentication::authenticatedUser, methodAuthorizationContext)) + .isThrownBy(() -> advice.invoke(TestAuthentication::authenticatedUser, methodInvocation)) .withMessage("Unable to determine the method argument for filtering. Specify the filter target."); } @PreFilter("filterObject == 'john'") public static class TestClass { - public void doSomething() { - - } - @PreFilter(value = "filterObject == 'john'", filterTarget = "filterTargetNotMatch") public List doSomethingListFilterTargetNotMatch(List list) { return list; @@ -205,4 +192,12 @@ public class PreFilterAuthorizationMethodBeforeAdviceTests { } + public static class NoPreFilterClass { + + public void doSomething() { + + } + + } + } diff --git a/core/src/test/java/org/springframework/security/authorization/method/SecuredAuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/method/SecuredAuthorizationManagerTests.java index 15ce6ffb06..076fdf473a 100644 --- a/core/src/test/java/org/springframework/security/authorization/method/SecuredAuthorizationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authorization/method/SecuredAuthorizationManagerTests.java @@ -16,6 +16,7 @@ package org.springframework.security.authorization.method; +import java.util.Collections; import java.util.function.Supplier; import org.junit.Test; @@ -38,38 +39,35 @@ public class SecuredAuthorizationManagerTests { @Test public void checkDoSomethingWhenNoSecuredAnnotationThenNullDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "doSomething"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); SecuredAuthorizationManager manager = new SecuredAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNull(); } @Test public void checkSecuredUserOrAdminWhenRoleUserThenGrantedDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "securedUserOrAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedUser, mockMethodInvocation, Collections.emptyList()); SecuredAuthorizationManager manager = new SecuredAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @Test public void checkSecuredUserOrAdminWhenRoleAdminThenGrantedDecision() throws Exception { - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "securedUserOrAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation( + TestAuthentication::authenticatedAdmin, mockMethodInvocation, Collections.emptyList()); SecuredAuthorizationManager manager = new SecuredAuthorizationManager(); - AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedAdmin, - methodAuthorizationContext); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedAdmin, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isTrue(); } @@ -78,12 +76,12 @@ public class SecuredAuthorizationManagerTests { public void checkSecuredUserOrAdminWhenRoleAnonymousThenDeniedDecision() throws Exception { Supplier authentication = () -> new TestingAuthenticationToken("user", "password", "ROLE_ANONYMOUS"); - MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, + MockMethodInvocation mockMethodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class, "securedUserOrAdmin"); - MethodAuthorizationContext methodAuthorizationContext = new MethodAuthorizationContext(methodInvocation, - TestClass.class); + AuthorizationMethodInvocation methodInvocation = new AuthorizationMethodInvocation(authentication, + mockMethodInvocation, Collections.emptyList()); SecuredAuthorizationManager manager = new SecuredAuthorizationManager(); - AuthorizationDecision decision = manager.check(authentication, methodAuthorizationContext); + AuthorizationDecision decision = manager.check(authentication, methodInvocation); assertThat(decision).isNotNull(); assertThat(decision.isGranted()).isFalse(); } diff --git a/docs/manual/src/docs/asciidoc/_includes/servlet/authorization/method-security.adoc b/docs/manual/src/docs/asciidoc/_includes/servlet/authorization/method-security.adoc index fea12cf33f..7c1d4304b6 100644 --- a/docs/manual/src/docs/asciidoc/_includes/servlet/authorization/method-security.adoc +++ b/docs/manual/src/docs/asciidoc/_includes/servlet/authorization/method-security.adoc @@ -25,7 +25,7 @@ public class MethodSecurityConfig { Adding an annotation to a method (on a class or interface) would then limit the access to that method accordingly. Spring Security's native annotatino support defines a set of attributes for the method. -These will be passed to the `AuthorizationMethodInterceptor` for it to make the actual decision: +These will be passed to the `DefaultAuthorizationMethodInterceptorChain` for it to make the actual decision: [source,java] ---- @@ -100,41 +100,59 @@ If that authorization denies access, the method is not invoked and an `AccessDen After-method authorization is performed after the method is invoked, but before the method returns to the caller. If that authorization denies access, the value is not returned and an `AccessDeniedException` is thrown -You can customize before-method authorization by publishing your own `AuthorizationMethodBeforeAdvice` bean, which includes your custom authorization manager as well as the `Pointcut` that describes when your manager should be used. - -For example, you may want to apply a default authorization rule to all methods in your service layer. -To do this, you'll supply the pointcut as well as the rule, like so: +To recreate what Spring Security does by default, you would publish the following bean: [source,java] ---- @Bean -public AuthorizationMethodBeforeAdvice authorizationMethodBeforeAdvice() { - JdkRegexpMethodPointcut pattern = new JdkRegexpMethodPointcut(); - pattern.setPattern("org.mycompany.myapp.service.*"); - AuthorizationManager rule = AuthorityAuthorizationManager.isAuthenticated(); - return new AuthorizationManagerMethodBeforeAdvice(pattern, rule); +public List methodSecurity() { + return new DelegatingAuthorizationMethodInterceptor( + new PreFilterAuthorizationMethodInterceptor(), // before-method + AuthorizationMethodInterceptors.preAuthorize(), // before-method + new PostFilterAuthorizationMethodInterceptor(), // after-method + AuthorizationMethodInterceptors.postAuthorize() // after-method + ); } ---- -This will replace any default before advice that Spring Security provides. -To use your custom rule as well as Spring Security's `@PreAuthorize` authorization support, you can do: +[NOTE] +Keep in mind that publishing a list of `AuthorizationMethodInterceptor`s will completely replace any Spring Security defaults. + +Interceptors are invoked in the order that they are declared. + +You may want to only support `@PreAuthorize` in your application, in which case you can do the following: [source,java] ---- @Bean -public AuthorizationMethodBeforeAdvice authorizationMethodBeforeAdvice() { - JdkRegexpMethodPointcut pattern = new JdkRegexpMethodPointcut(); - pattern.setPattern("org.mycompany.myapp.service.*"); - AuthorizationManager rule = AuthorityAuthorizationManager.isAuthenticated(); - AuthorizationMethodBeforeAdvice custom = new AuthorizationManagerMethodBeforeAdvice(pattern, rule); - AuthorizationMethodBeforeAdvice pre = new AuthorizationMethodBeforeAdvice( - AuthorizationMethodPointcuts.forAnnotations(PreAuthorize.class), - new PreAuthorizeAuthorizationManager()); - return new DelegatingAuthorizationManagerBeforeAdvice(custom, pre); +public AuthorizationMethodInterceptor methodSecurity() { + return AuthorizationMethodInterceptors.preAuthorize(); } ---- -The same can be done for after-method authorization. +Or, you may have a custom before-method `AuthorizationManager` that you want to add to the list. + +In this case, you will need to tell Spring Security both the `AuthorizationManager` and to which methods and classes your authorization manager applies. + +Spring Security integrates with Spring AOP to achieve this. +Thus, you can configure Spring Security to support `@PreAuthorize`, `@PostAuthorize`, and your own `AuthorizationManager` like so: + +[source,java] +---- +@Bean +public AuthorizationMethodInterceptor methodSecurity() { + JdkRegexpMethodPointcut pattern = new JdkRegexpMethodPointcut(); + pattern.setPattern("org.mycompany.myapp.service.*"); + AuthorizationManager rule = AuthorityAuthorizationManager.isAuthenticated(); + return new DelegatingAuthorizationMethodInterceptor( + AuthorizationMethodInterceptors.preAuthorize(), + new AuthorizationManagerBeforeMethodInterceptor(pattern, rule), + AuthorizationMethodInterceptors.postAuthorize() + ); +} +---- + +The same can be done for after-method authorization and `AfterMethodAuthorizationManager`. After-method authorization is generally concerned with analysing the return value to verify access. For example, you might have a method that confirms that the account requested actually belongs to the logged-in user like so: @@ -149,22 +167,20 @@ public interface BankService { } ---- -You can supply your own `AuthorizationMethodAfterAdvice` to customize how access to the return value is evaluated. +You can supply your own `AuthorizationMethodInterceptor` to customize how access to the return value is evaluated. -For example, you can give special access to a given role in your system, like so: +For example, instead of embedding a great deal of logic into the `@PostAuthorize` SpEL expression, you may want to wire your own `@Bean`. +In that case, you can configure it like so: [source,java] ---- @Bean -public AuthorizationMethodAfterAdvice authorizationMethodAfterAdvice() { - JdkRegexpMethodPointcut pattern = new JdkRegexpMethodPointcut(); - pattern.setPattern("org.mycompany.myapp.service.*"); - AuthorizationManager rule = AuthorityAuthorizationManager.hasRole("TELLER"); - AuthorizationMethodBeforeAdvice custom = new AuthorizationManagerMethodBeforeAdvice(pattern, rule); - AuthorizationMethodBeforeAdvice post = new AuthorizationManagerMethodBeforeAdvice( - AuthorizationMethodPointcuts.forAnnotations(PostAuthorize.class), - new PostAuthorizeAuthorizationManager()); - return new DelegatingAuthorizationManagerBeforeAdvice(custom, post); +public AuthorizationMethodInterceptor methodSecurity + (AfterMethodAuthorizationManager rules) { + AnnotationMethodMatcher pattern = new AnnotationMethodMatcher(MySecurityAnnotation.class); + return new DelegatingAuthorizationMethodInterceptor( + AuthorizationMethodInterceptors.preAuthorize(), + new AuthorizationManagerAfterMethodInterceptor(pattern, rules)); } ----