diff --git a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java index f80ad94d63..d978b3c8ea 100644 --- a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java +++ b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java @@ -47,6 +47,8 @@ public class PreAuthorizeAspectTests { private PrePostSecured prePostSecured = new PrePostSecured(); + private MultipleInterfaces multiple = new MultipleInterfaces(); + @BeforeEach public final void setUp() { MockitoAnnotations.initMocks(this); @@ -110,6 +112,12 @@ public class PreAuthorizeAspectTests { .isThrownBy(() -> this.secured.myObject().denyAllMethod()); } + @Test + public void multipleInterfacesPreAuthorizeAllows() { + // aspectj doesn't inherit annotations + this.multiple.securedMethod(); + } + interface SecuredInterface { @PreAuthorize("hasRole('X')") @@ -177,4 +185,19 @@ public class PreAuthorizeAspectTests { } + interface AnotherSecuredInterface { + + @PreAuthorize("hasRole('Y')") + void securedMethod(); + + } + + static class MultipleInterfaces implements SecuredInterface, AnotherSecuredInterface { + + @Override + public void securedMethod() { + } + + } + } 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 d1aa34f8e7..ba87efa9dc 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 @@ -16,12 +16,9 @@ package org.springframework.security.authorization.method; -import java.lang.annotation.Annotation; -import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Function; import org.aopalliance.intercept.MethodInvocation; @@ -43,8 +40,6 @@ abstract class AbstractExpressionAttributeRegistry resolveAttribute(method, targetClass)); } - final Function findUniqueAnnotation(Class type) { - return (this.defaults != null) ? AuthorizationAnnotationUtils.withDefaults(type, this.defaults) - : AuthorizationAnnotationUtils.withDefaults(type); - } - /** * Returns the {@link MethodSecurityExpressionHandler}. * @return the {@link MethodSecurityExpressionHandler} to use @@ -86,10 +76,6 @@ abstract class AbstractExpressionAttributeRegistry - * There are numerous ways that two annotations of the same type may be attached to the - * same method. For example, a class may implement a method defined in two separate - * interfaces. If both of those interfaces have a {@code @PreAuthorize} annotation, then - * it's unclear which {@code @PreAuthorize} expression Spring Security should use. - * - *

- * Another way is when one of Spring Security's annotations is used as a meta-annotation. - * In that case, two custom annotations can be declared, each with their own - * {@code @PreAuthorize} declaration. If both custom annotations are used on the same - * method, then it's unclear which {@code @PreAuthorize} expression Spring Security should - * use. - * - * @author Josh Cummings - * @author Sam Brannen - */ -final class AuthorizationAnnotationUtils { - - static Function withDefaults(Class type, - PrePostTemplateDefaults defaults) { - Function, A> map = (mergedAnnotation) -> { - if (mergedAnnotation.getMetaSource() == null) { - return mergedAnnotation.synthesize(); - } - PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("{", "}", null, - defaults.isIgnoreUnknown()); - String expression = (String) mergedAnnotation.asMap().get("value"); - Map annotationProperties = mergedAnnotation.getMetaSource().asMap(); - Map stringProperties = new HashMap<>(); - for (Map.Entry property : annotationProperties.entrySet()) { - String key = property.getKey(); - Object value = property.getValue(); - String asString = (value instanceof String) ? (String) value - : DefaultConversionService.getSharedInstance().convert(value, String.class); - stringProperties.put(key, asString); - } - AnnotatedElement annotatedElement = (AnnotatedElement) mergedAnnotation.getSource(); - String value = helper.replacePlaceholders(expression, stringProperties::get); - Map properties = new HashMap<>(mergedAnnotation.asMap()); - properties.put("value", value); - return MergedAnnotation.of(annotatedElement, type, properties).synthesize(); - }; - return (annotatedElement) -> findDistinctAnnotation(annotatedElement, type, map); - } - - static Function withDefaults(Class type) { - return (annotatedElement) -> findDistinctAnnotation(annotatedElement, type, MergedAnnotation::synthesize); - } - - static A findUniqueAnnotation(Method method, Class annotationType) { - return findDistinctAnnotation(method, annotationType, MergedAnnotation::synthesize); - } - - static A findUniqueAnnotation(Class type, Class annotationType) { - return findDistinctAnnotation(type, annotationType, MergedAnnotation::synthesize); - } - - /** - * Perform an exhaustive search on the type hierarchy of the given {@link Method} for - * the annotation of type {@code annotationType}, including any annotations using - * {@code annotationType} as a meta-annotation. - * - *

- * If more than one unique annotation is found, then throw an error. - * @param method the method declaration to search from - * @param annotationType the annotation type to search for - * @return a unique instance of the annotation attributed to the method, {@code null} - * otherwise - * @throws AnnotationConfigurationException if more than one unique instance of the - * annotation is found - */ - static A findUniqueAnnotation(Method method, Class annotationType, - Function, A> map) { - return findDistinctAnnotation(method, annotationType, map); - } - - /** - * Perform an exhaustive search on the type hierarchy of the given {@link Class} for - * the annotation of type {@code annotationType}, including any annotations using - * {@code annotationType} as a meta-annotation. - * - *

- * If more than one unique annotation is found, then throw an error. - * @param type the type to search from - * @param annotationType the annotation type to search for - * @return a unique instance of the annotation attributed to the class, {@code null} - * otherwise - * @throws AnnotationConfigurationException if more than one unique instance of the - * annotation is found - */ - static A findUniqueAnnotation(Class type, Class annotationType, - Function, A> map) { - return findDistinctAnnotation(type, annotationType, map); - } - - private static A findDistinctAnnotation(AnnotatedElement annotatedElement, - Class annotationType, Function, A> map) { - MergedAnnotations mergedAnnotations = MergedAnnotations.from(annotatedElement, SearchStrategy.TYPE_HIERARCHY, - RepeatableContainers.none()); - List annotations = mergedAnnotations.stream(annotationType).map(map).distinct().toList(); - - return switch (annotations.size()) { - case 0 -> null; - case 1 -> annotations.get(0); - default -> throw new AnnotationConfigurationException(""" - Please ensure there is one unique annotation of type @%s attributed to %s. \ - Found %d competing annotations: %s""".formatted(annotationType.getName(), annotatedElement, - annotations.size(), annotations)); - }; - } - - private AuthorizationAnnotationUtils() { - - } - -} 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 f913db85f6..34f6b20ca9 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 @@ -20,6 +20,7 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.function.Supplier; @@ -29,12 +30,13 @@ import jakarta.annotation.security.RolesAllowed; import org.aopalliance.intercept.MethodInvocation; import org.springframework.aop.support.AopUtils; -import org.springframework.core.annotation.AnnotationConfigurationException; import org.springframework.lang.NonNull; import org.springframework.security.authorization.AuthoritiesAuthorizationManager; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; +import org.springframework.security.core.annotation.AnnotationSynthesizer; +import org.springframework.security.core.annotation.AnnotationSynthesizers; import org.springframework.util.Assert; /** @@ -49,14 +51,6 @@ import org.springframework.util.Assert; */ public final class Jsr250AuthorizationManager implements AuthorizationManager { - private static final Set> JSR250_ANNOTATIONS = new HashSet<>(); - - static { - JSR250_ANNOTATIONS.add(DenyAll.class); - JSR250_ANNOTATIONS.add(PermitAll.class); - JSR250_ANNOTATIONS.add(RolesAllowed.class); - } - private final Jsr250AuthorizationManagerRegistry registry = new Jsr250AuthorizationManagerRegistry(); private AuthorizationManager> authoritiesAuthorizationManager = new AuthoritiesAuthorizationManager(); @@ -102,6 +96,9 @@ public final class Jsr250AuthorizationManager implements AuthorizationManager synthesizer = AnnotationSynthesizers + .requireUnique(List.of(DenyAll.class, PermitAll.class, RolesAllowed.class)); + @NonNull @Override AuthorizationManager resolveManager(Method method, Class targetClass) { @@ -121,45 +118,8 @@ public final class Jsr250AuthorizationManager implements AuthorizationManager targetClass) { Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass); - Annotation annotation = findAnnotation(specificMethod); - return (annotation != null) ? annotation - : findAnnotation((targetClass != null) ? targetClass : specificMethod.getDeclaringClass()); - } - - private Annotation findAnnotation(Method method) { - Set annotations = new HashSet<>(); - for (Class annotationClass : JSR250_ANNOTATIONS) { - Annotation annotation = AuthorizationAnnotationUtils.findUniqueAnnotation(method, annotationClass); - if (annotation != null) { - annotations.add(annotation); - } - } - if (annotations.isEmpty()) { - return null; - } - if (annotations.size() > 1) { - throw new AnnotationConfigurationException( - "The JSR-250 specification disallows DenyAll, PermitAll, and RolesAllowed from appearing on the same method."); - } - return annotations.iterator().next(); - } - - private Annotation findAnnotation(Class clazz) { - Set annotations = new HashSet<>(); - for (Class annotationClass : JSR250_ANNOTATIONS) { - Annotation annotation = AuthorizationAnnotationUtils.findUniqueAnnotation(clazz, annotationClass); - if (annotation != null) { - annotations.add(annotation); - } - } - if (annotations.isEmpty()) { - return null; - } - if (annotations.size() > 1) { - throw new AnnotationConfigurationException( - "The JSR-250 specification disallows DenyAll, PermitAll, and RolesAllowed from appearing on the same class definition."); - } - return annotations.iterator().next(); + Class targetClassToUse = (targetClass != null) ? targetClass : specificMethod.getDeclaringClass(); + return this.synthesizer.synthesize(specificMethod, targetClassToUse); } private Set getAllowedRolesWithPrefix(RolesAllowed rolesAllowed) { diff --git a/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java index 7dc43f649a..0152581f1b 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PostAuthorizeExpressionAttributeRegistry.java @@ -16,7 +16,6 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; import java.util.Arrays; import java.util.function.Function; @@ -27,6 +26,8 @@ import org.springframework.aop.support.AopUtils; import org.springframework.context.ApplicationContext; import org.springframework.expression.Expression; import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.core.annotation.AnnotationSynthesizer; +import org.springframework.security.core.annotation.AnnotationSynthesizers; import org.springframework.util.Assert; /** @@ -40,8 +41,14 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA private final MethodAuthorizationDeniedHandler defaultHandler = new ThrowingMethodAuthorizationDeniedHandler(); + private final AnnotationSynthesizer handleAuthorizationDeniedSynthesizer = AnnotationSynthesizers + .requireUnique(HandleAuthorizationDenied.class); + private Function, MethodAuthorizationDeniedHandler> handlerResolver; + private AnnotationSynthesizer postAuthorizeSynthesizer = AnnotationSynthesizers + .requireUnique(PostAuthorize.class); + PostAuthorizeExpressionAttributeRegistry() { this.handlerResolver = (clazz) -> this.defaultHandler; } @@ -60,13 +67,9 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA } private MethodAuthorizationDeniedHandler resolveHandler(Method method, Class targetClass) { - Function lookup = AuthorizationAnnotationUtils - .withDefaults(HandleAuthorizationDenied.class); - HandleAuthorizationDenied deniedHandler = lookup.apply(method); - if (deniedHandler != null) { - return this.handlerResolver.apply(deniedHandler.handlerClass()); - } - deniedHandler = lookup.apply(targetClass(method, targetClass)); + Class targetClassToUse = targetClass(method, targetClass); + HandleAuthorizationDenied deniedHandler = this.handleAuthorizationDeniedSynthesizer.synthesize(method, + targetClassToUse); if (deniedHandler != null) { return this.handlerResolver.apply(deniedHandler.handlerClass()); } @@ -74,9 +77,8 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA } private PostAuthorize findPostAuthorizeAnnotation(Method method, Class targetClass) { - Function lookup = findUniqueAnnotation(PostAuthorize.class); - PostAuthorize postAuthorize = lookup.apply(method); - return (postAuthorize != null) ? postAuthorize : lookup.apply(targetClass(method, targetClass)); + Class targetClassToUse = targetClass(method, targetClass); + return this.postAuthorizeSynthesizer.synthesize(method, targetClass); } /** @@ -89,6 +91,10 @@ final class PostAuthorizeExpressionAttributeRegistry extends AbstractExpressionA this.handlerResolver = (clazz) -> resolveHandler(context, clazz); } + void setTemplateDefaults(PrePostTemplateDefaults templateDefaults) { + this.postAuthorizeSynthesizer = AnnotationSynthesizers.requireUnique(PostAuthorize.class, templateDefaults); + } + private MethodAuthorizationDeniedHandler resolveHandler(ApplicationContext context, Class handlerClass) { if (handlerClass == this.defaultHandler.getClass()) { diff --git a/core/src/main/java/org/springframework/security/authorization/method/PostFilterExpressionAttributeRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/PostFilterExpressionAttributeRegistry.java index 8b80c5101c..6859e90b27 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PostFilterExpressionAttributeRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PostFilterExpressionAttributeRegistry.java @@ -16,14 +16,14 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; -import java.util.function.Function; import org.springframework.aop.support.AopUtils; import org.springframework.expression.Expression; import org.springframework.lang.NonNull; import org.springframework.security.access.prepost.PostFilter; +import org.springframework.security.core.annotation.AnnotationSynthesizer; +import org.springframework.security.core.annotation.AnnotationSynthesizers; /** * For internal use only, as this contract is likely to change. @@ -34,6 +34,8 @@ import org.springframework.security.access.prepost.PostFilter; */ final class PostFilterExpressionAttributeRegistry extends AbstractExpressionAttributeRegistry { + private AnnotationSynthesizer synthesizer = AnnotationSynthesizers.requireUnique(PostFilter.class); + @NonNull @Override ExpressionAttribute resolveAttribute(Method method, Class targetClass) { @@ -47,10 +49,13 @@ final class PostFilterExpressionAttributeRegistry extends AbstractExpressionAttr return new ExpressionAttribute(postFilterExpression); } + void setTemplateDefaults(PrePostTemplateDefaults defaults) { + this.synthesizer = AnnotationSynthesizers.requireUnique(PostFilter.class, defaults); + } + private PostFilter findPostFilterAnnotation(Method method, Class targetClass) { - Function lookup = findUniqueAnnotation(PostFilter.class); - PostFilter postFilter = lookup.apply(method); - return (postFilter != null) ? postFilter : lookup.apply(targetClass(method, targetClass)); + Class targetClassToUse = targetClass(method, targetClass); + return this.synthesizer.synthesize(method, targetClassToUse); } } diff --git a/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java index 2bfe20a932..c93334a620 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PreAuthorizeExpressionAttributeRegistry.java @@ -16,7 +16,6 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; import java.util.Arrays; import java.util.function.Function; @@ -27,6 +26,8 @@ import org.springframework.aop.support.AopUtils; import org.springframework.context.ApplicationContext; import org.springframework.expression.Expression; import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.annotation.AnnotationSynthesizer; +import org.springframework.security.core.annotation.AnnotationSynthesizers; import org.springframework.util.Assert; /** @@ -40,8 +41,14 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt private final MethodAuthorizationDeniedHandler defaultHandler = new ThrowingMethodAuthorizationDeniedHandler(); + private final AnnotationSynthesizer handleAuthorizationDeniedSynthesizer = AnnotationSynthesizers + .requireUnique(HandleAuthorizationDenied.class); + private Function, MethodAuthorizationDeniedHandler> handlerResolver; + private AnnotationSynthesizer preAuthorizeSynthesizer = AnnotationSynthesizers + .requireUnique(PreAuthorize.class); + PreAuthorizeExpressionAttributeRegistry() { this.handlerResolver = (clazz) -> this.defaultHandler; } @@ -60,13 +67,9 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt } private MethodAuthorizationDeniedHandler resolveHandler(Method method, Class targetClass) { - Function lookup = AuthorizationAnnotationUtils - .withDefaults(HandleAuthorizationDenied.class); - HandleAuthorizationDenied deniedHandler = lookup.apply(method); - if (deniedHandler != null) { - return this.handlerResolver.apply(deniedHandler.handlerClass()); - } - deniedHandler = lookup.apply(targetClass(method, targetClass)); + Class targetClassToUse = targetClass(method, targetClass); + HandleAuthorizationDenied deniedHandler = this.handleAuthorizationDeniedSynthesizer.synthesize(method, + targetClassToUse); if (deniedHandler != null) { return this.handlerResolver.apply(deniedHandler.handlerClass()); } @@ -74,9 +77,8 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt } private PreAuthorize findPreAuthorizeAnnotation(Method method, Class targetClass) { - Function lookup = findUniqueAnnotation(PreAuthorize.class); - PreAuthorize preAuthorize = lookup.apply(method); - return (preAuthorize != null) ? preAuthorize : lookup.apply(targetClass(method, targetClass)); + Class targetClassToUse = targetClass(method, targetClass); + return this.preAuthorizeSynthesizer.synthesize(method, targetClassToUse); } /** @@ -89,6 +91,10 @@ final class PreAuthorizeExpressionAttributeRegistry extends AbstractExpressionAt this.handlerResolver = (clazz) -> resolveHandler(context, clazz); } + void setTemplateDefaults(PrePostTemplateDefaults defaults) { + this.preAuthorizeSynthesizer = AnnotationSynthesizers.requireUnique(PreAuthorize.class, defaults); + } + private MethodAuthorizationDeniedHandler resolveHandler(ApplicationContext context, Class handlerClass) { if (handlerClass == this.defaultHandler.getClass()) { diff --git a/core/src/main/java/org/springframework/security/authorization/method/PreFilterExpressionAttributeRegistry.java b/core/src/main/java/org/springframework/security/authorization/method/PreFilterExpressionAttributeRegistry.java index 95425c438d..06b8b32a14 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PreFilterExpressionAttributeRegistry.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PreFilterExpressionAttributeRegistry.java @@ -16,14 +16,14 @@ package org.springframework.security.authorization.method; -import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; -import java.util.function.Function; import org.springframework.aop.support.AopUtils; import org.springframework.expression.Expression; import org.springframework.lang.NonNull; import org.springframework.security.access.prepost.PreFilter; +import org.springframework.security.core.annotation.AnnotationSynthesizer; +import org.springframework.security.core.annotation.AnnotationSynthesizers; /** * For internal use only, as this contract is likely to change. @@ -35,6 +35,8 @@ import org.springframework.security.access.prepost.PreFilter; final class PreFilterExpressionAttributeRegistry extends AbstractExpressionAttributeRegistry { + private AnnotationSynthesizer synthesizer = AnnotationSynthesizers.requireUnique(PreFilter.class); + @NonNull @Override PreFilterExpressionAttribute resolveAttribute(Method method, Class targetClass) { @@ -48,10 +50,13 @@ final class PreFilterExpressionAttributeRegistry return new PreFilterExpressionAttribute(preFilterExpression, preFilter.filterTarget()); } + void setTemplateDefaults(PrePostTemplateDefaults defaults) { + this.synthesizer = AnnotationSynthesizers.requireUnique(PreFilter.class, defaults); + } + private PreFilter findPreFilterAnnotation(Method method, Class targetClass) { - Function lookup = findUniqueAnnotation(PreFilter.class); - PreFilter preFilter = lookup.apply(method); - return (preFilter != null) ? preFilter : lookup.apply(targetClass(method, targetClass)); + Class targetClassToUse = targetClass(method, targetClass); + return this.synthesizer.synthesize(method, targetClassToUse); } static final class PreFilterExpressionAttribute extends ExpressionAttribute { diff --git a/core/src/main/java/org/springframework/security/authorization/method/PrePostTemplateDefaults.java b/core/src/main/java/org/springframework/security/authorization/method/PrePostTemplateDefaults.java index 7c309582c0..e31ac4aca9 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/PrePostTemplateDefaults.java +++ b/core/src/main/java/org/springframework/security/authorization/method/PrePostTemplateDefaults.java @@ -16,6 +16,8 @@ package org.springframework.security.authorization.method; +import org.springframework.security.core.annotation.AnnotationTemplateExpressionDefaults; + /** * A component for configuring various cross-cutting aspects of pre/post method security * @@ -26,31 +28,6 @@ package org.springframework.security.authorization.method; * @see org.springframework.security.access.prepost.PreFilter * @see org.springframework.security.access.prepost.PostFilter */ -public final class PrePostTemplateDefaults { - - private boolean ignoreUnknown = true; - - /** - * Whether template resolution should ignore placeholders it doesn't recognize. - *

- * By default, this value is true. - * @since 6.3 - */ - public boolean isIgnoreUnknown() { - return this.ignoreUnknown; - } - - /** - * Configure template resolution to ignore unknown placeholders. When set to - * false, template resolution will throw an exception for unknown - * placeholders. - *

- * By default, this value is true. - * @param ignoreUnknown - whether to ignore unknown placeholders parameters - * @since 6.3 - */ - public void setIgnoreUnknown(boolean ignoreUnknown) { - this.ignoreUnknown = ignoreUnknown; - } +public final class PrePostTemplateDefaults extends AnnotationTemplateExpressionDefaults { } 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 63553503d2..e064234617 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 @@ -33,6 +33,8 @@ import org.springframework.security.authorization.AuthoritiesAuthorizationManage import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; +import org.springframework.security.core.annotation.AnnotationSynthesizer; +import org.springframework.security.core.annotation.AnnotationSynthesizers; import org.springframework.util.Assert; /** @@ -50,6 +52,8 @@ public final class SecuredAuthorizationManager implements AuthorizationManager> cachedAuthorities = new ConcurrentHashMap<>(); + private final AnnotationSynthesizer synthesizer = AnnotationSynthesizers.requireUnique(Secured.class); + /** * Sets an {@link AuthorizationManager} that accepts a collection of authority * strings. @@ -92,9 +96,8 @@ public final class SecuredAuthorizationManager implements AuthorizationManager targetClass) { - Secured secured = AuthorizationAnnotationUtils.findUniqueAnnotation(method, Secured.class); - return (secured != null) ? secured : AuthorizationAnnotationUtils - .findUniqueAnnotation((targetClass != null) ? targetClass : method.getDeclaringClass(), Secured.class); + Class targetClassToUse = (targetClass != null) ? targetClass : method.getDeclaringClass(); + return this.synthesizer.synthesize(method, targetClassToUse); } } diff --git a/core/src/main/java/org/springframework/security/core/annotation/AnnotationSynthesizer.java b/core/src/main/java/org/springframework/security/core/annotation/AnnotationSynthesizer.java new file mode 100644 index 0000000000..34c9ac67f8 --- /dev/null +++ b/core/src/main/java/org/springframework/security/core/annotation/AnnotationSynthesizer.java @@ -0,0 +1,88 @@ +/* + * Copyright 2002-2024 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.core.annotation; + +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; + +import org.springframework.core.annotation.MergedAnnotation; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +/** + * A strategy for synthesizing an annotation from an {@link AnnotatedElement}. + * + *

+ * Synthesis generally refers to the process of taking an annotation's meta-annotations + * and placeholders, resolving them, and then combining these elements into a facade of + * the raw annotation instance. + *

+ * + *

+ * Since the process of synthesizing an annotation can be expensive, it is recommended to + * cache the synthesized annotation to prevent multiple computations. + *

+ * + * @param the annotation type + * @author Josh Cummings + * @since 6.4 + * @see UniqueMergedAnnotationSynthesizer + * @see ExpressionTemplateAnnotationSynthesizer + */ +public interface AnnotationSynthesizer { + + /** + * Synthesize an annotation of type {@code A} from the given {@link AnnotatedElement}. + * + *

+ * Implementations should fail if they encounter more than one annotation of that type + * on the element. + *

+ * + *

+ * Implementations should describe their strategy for searching the element and any + * surrounding class, interfaces, or super-class. + *

+ * @param element the element to search + * @return the synthesized annotation or {@code null} if not found + */ + @Nullable + default A synthesize(AnnotatedElement element, Class targetClass) { + Assert.notNull(targetClass, "targetClass cannot be null"); + MergedAnnotation annotation = merge(element, targetClass); + if (annotation == null) { + return null; + } + return annotation.synthesize(); + } + + @Nullable + default A synthesize(AnnotatedElement element) { + if (element instanceof Method method) { + return synthesize(element, method.getDeclaringClass()); + } + if (element instanceof Parameter parameter) { + return synthesize(parameter, parameter.getDeclaringExecutable().getDeclaringClass()); + } + throw new UnsupportedOperationException("Unsupported element of type " + element.getClass()); + } + + MergedAnnotation merge(AnnotatedElement element, Class targetClass); + +} diff --git a/core/src/main/java/org/springframework/security/core/annotation/AnnotationSynthesizers.java b/core/src/main/java/org/springframework/security/core/annotation/AnnotationSynthesizers.java new file mode 100644 index 0000000000..33085588b9 --- /dev/null +++ b/core/src/main/java/org/springframework/security/core/annotation/AnnotationSynthesizers.java @@ -0,0 +1,81 @@ +/* + * Copyright 2002-2024 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.core.annotation; + +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; +import java.util.ArrayList; +import java.util.List; + +/** + * Factory for creating {@link AnnotationSynthesizer} instances. + * + * @author Josh Cummings + * @since 6.4 + */ +public final class AnnotationSynthesizers { + + private AnnotationSynthesizers() { + } + + /** + * Create a {@link AnnotationSynthesizer} that requires synthesized annotations to be + * unique on the given {@link AnnotatedElement}. + * @param type the annotation type + * @param the annotation type + * @return the default {@link AnnotationSynthesizer} + */ + public static AnnotationSynthesizer requireUnique(Class type) { + return new UniqueMergedAnnotationSynthesizer<>(type); + } + + /** + * Create a {@link AnnotationSynthesizer} that requires synthesized annotations to be + * unique on the given {@link AnnotatedElement}. + * + *

+ * When a {@link AnnotationTemplateExpressionDefaults} is provided, it will return a + * synthesizer that supports placeholders in the annotation's attributes in addition + * to the meta-annotation synthesizing provided by {@link #requireUnique(Class)}. + * @param type the annotation type + * @param templateDefaults the defaults for resolving placeholders in the annotation's + * attributes + * @param the annotation type + * @return the default {@link AnnotationSynthesizer} + */ + public static AnnotationSynthesizer requireUnique(Class type, + AnnotationTemplateExpressionDefaults templateDefaults) { + if (templateDefaults == null) { + return new UniqueMergedAnnotationSynthesizer<>(type); + } + return new ExpressionTemplateAnnotationSynthesizer<>(type, templateDefaults); + } + + /** + * Create a {@link AnnotationSynthesizer} that requires synthesized annotations to be + * unique on the given {@link AnnotatedElement}. Supplying multiple types implies that + * the synthesized annotation must be unique across all specified types. + * @param types the annotation types + * @return the default {@link AnnotationSynthesizer} + */ + public static AnnotationSynthesizer requireUnique(List> types) { + List> casted = new ArrayList<>(); + types.forEach((type) -> casted.add((Class) type)); + return new UniqueMergedAnnotationSynthesizer<>(casted); + } + +} diff --git a/core/src/main/java/org/springframework/security/core/annotation/AnnotationTemplateExpressionDefaults.java b/core/src/main/java/org/springframework/security/core/annotation/AnnotationTemplateExpressionDefaults.java new file mode 100644 index 0000000000..8ab2158dbc --- /dev/null +++ b/core/src/main/java/org/springframework/security/core/annotation/AnnotationTemplateExpressionDefaults.java @@ -0,0 +1,53 @@ +/* + * Copyright 2002-2024 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.core.annotation; + +/** + * A component for configuring the expression attribute template of the parsed Spring + * Security annotation + * + * @author DingHao + * @since 6.4 + * @see AuthenticationPrincipal + * @see CurrentSecurityContext + */ +public class AnnotationTemplateExpressionDefaults { + + private boolean ignoreUnknown = true; + + /** + * Whether template resolution should ignore placeholders it doesn't recognize. + *

+ * By default, this value is true. + */ + public boolean isIgnoreUnknown() { + return this.ignoreUnknown; + } + + /** + * Configure template resolution to ignore unknown placeholders. When set to + * false, template resolution will throw an exception for unknown + * placeholders. + *

+ * By default, this value is true. + * @param ignoreUnknown - whether to ignore unknown placeholders parameters + */ + public void setIgnoreUnknown(boolean ignoreUnknown) { + this.ignoreUnknown = ignoreUnknown; + } + +} diff --git a/core/src/main/java/org/springframework/security/core/annotation/ExpressionTemplateAnnotationSynthesizer.java b/core/src/main/java/org/springframework/security/core/annotation/ExpressionTemplateAnnotationSynthesizer.java new file mode 100644 index 0000000000..c5da99a800 --- /dev/null +++ b/core/src/main/java/org/springframework/security/core/annotation/ExpressionTemplateAnnotationSynthesizer.java @@ -0,0 +1,135 @@ +/* + * Copyright 2002-2024 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.core.annotation; + +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import java.util.HashMap; +import java.util.Map; + +import org.springframework.core.MethodClassKey; +import org.springframework.core.annotation.MergedAnnotation; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.util.Assert; +import org.springframework.util.PropertyPlaceholderHelper; + +/** + * A strategy for synthesizing an annotation from an {@link AnnotatedElement} that + * supports meta-annotations with placeholders, like the following: + * + * 

+ *	@PreAuthorize("hasRole({role})")
+ *	public @annotation HasRole {
+ *		String role();
+ *	}
+ *
+ * + *

+ * In that case, you could use an {@link ExpressionTemplateAnnotationSynthesizer} of type + * {@link org.springframework.security.access.prepost.PreAuthorize} to synthesize any + * {@code @HasRole} annotation found on a given {@link AnnotatedElement}. + * + *

+ * Note that in all cases, Spring Security does not allow for repeatable annotations. So + * this class delegates to {@link UniqueMergedAnnotationSynthesizer} in order to error if + * a repeat is discovered. + * + *

+ * Since the process of synthesis is expensive, it is recommended to cache the synthesized + * result to prevent multiple computations. + * + * @param the annotation type + * @author Josh Cummings + * @since 6.4 + */ +final class ExpressionTemplateAnnotationSynthesizer implements AnnotationSynthesizer { + + private final Class type; + + private final UniqueMergedAnnotationSynthesizer unique; + + private final AnnotationTemplateExpressionDefaults templateDefaults; + + private final Map> uniqueParameterAnnotationCache = new HashMap<>(); + + private final Map> uniqueMethodAnnotationCache = new HashMap<>(); + + ExpressionTemplateAnnotationSynthesizer(Class type, AnnotationTemplateExpressionDefaults templateDefaults) { + Assert.notNull(type, "type cannot be null"); + Assert.notNull(templateDefaults, "templateDefaults cannot be null"); + this.type = type; + this.unique = new UniqueMergedAnnotationSynthesizer<>(type); + this.templateDefaults = templateDefaults; + } + + @Override + public MergedAnnotation merge(AnnotatedElement element, Class targetClass) { + if (element instanceof Parameter parameter) { + MergedAnnotation annotation = this.uniqueParameterAnnotationCache.computeIfAbsent(parameter, + (p) -> this.unique.merge(p, targetClass)); + if (annotation == null) { + return null; + } + return resolvePlaceholders(annotation); + } + if (element instanceof Method method) { + MethodClassKey key = new MethodClassKey(method, targetClass); + MergedAnnotation annotation = this.uniqueMethodAnnotationCache.computeIfAbsent(key, + (k) -> this.unique.merge(method, targetClass)); + if (annotation == null) { + return null; + } + return resolvePlaceholders(annotation); + } + throw new IllegalArgumentException("Unsupported element of type " + element.getClass()); + } + + private MergedAnnotation resolvePlaceholders(MergedAnnotation mergedAnnotation) { + if (this.templateDefaults == null) { + return mergedAnnotation; + } + if (mergedAnnotation.getMetaSource() == null) { + return mergedAnnotation; + } + PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("{", "}", null, null, + this.templateDefaults.isIgnoreUnknown()); + Map properties = new HashMap<>(mergedAnnotation.asMap()); + Map metaAnnotationProperties = mergedAnnotation.getMetaSource().asMap(); + Map stringProperties = new HashMap<>(); + for (Map.Entry property : metaAnnotationProperties.entrySet()) { + String key = property.getKey(); + Object value = property.getValue(); + String asString = (value instanceof String) ? (String) value + : DefaultConversionService.getSharedInstance().convert(value, String.class); + stringProperties.put(key, asString); + } + Map annotationProperties = mergedAnnotation.asMap(); + for (Map.Entry annotationProperty : annotationProperties.entrySet()) { + if (!(annotationProperty.getValue() instanceof String)) { + continue; + } + String expression = (String) annotationProperty.getValue(); + String value = helper.replacePlaceholders(expression, stringProperties::get); + properties.put(annotationProperty.getKey(), value); + } + AnnotatedElement annotatedElement = (AnnotatedElement) mergedAnnotation.getSource(); + return MergedAnnotation.of(annotatedElement, this.type, properties); + } + +} diff --git a/core/src/main/java/org/springframework/security/core/annotation/UniqueMergedAnnotationSynthesizer.java b/core/src/main/java/org/springframework/security/core/annotation/UniqueMergedAnnotationSynthesizer.java new file mode 100644 index 0000000000..340fec9556 --- /dev/null +++ b/core/src/main/java/org/springframework/security/core/annotation/UniqueMergedAnnotationSynthesizer.java @@ -0,0 +1,183 @@ +/* + * Copyright 2002-2024 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.core.annotation; + +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.springframework.core.annotation.AnnotationConfigurationException; +import org.springframework.core.annotation.MergedAnnotation; +import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.core.annotation.RepeatableContainers; +import org.springframework.util.Assert; + +/** + * A strategy for synthesizing an annotation from an {@link AnnotatedElement} that + * supports meta-annotations, like the following: + * + * 

+ *	@PreAuthorize("hasRole('ROLE_ADMIN')")
+ *	public @annotation HasRole {
+ *	}
+ *
+ * + *

+ * In that case, you can use an {@link UniqueMergedAnnotationSynthesizer} of type + * {@link org.springframework.security.access.prepost.PreAuthorize} to synthesize any + * {@code @HasRole} annotation found on a given {@link AnnotatedElement}. + * + *

+ * Note that in all cases, Spring Security does not allow for repeatable annotations. As + * such, this class errors if a repeat is discovered. + * + *

+ * If the given annotation can be applied to types, this class will search for annotations + * across the entire {@link MergedAnnotations.SearchStrategy type hierarchy}; otherwise, + * it will only look for annotations {@link MergedAnnotations.SearchStrategy directly} + * attributed to the element. + * + *

+ * When traversing the type hierarchy, this class will first look for annotations on the + * given method, then on any methods that method overrides. If no annotations are found, + * it will then search for annotations on the given class, then on any classes that class + * extends and on any interfaces that class implements. + * + *

+ * Since the process of synthesis is expensive, it is recommended to cache the synthesized + * result to prevent multiple computations. + * + * @param the annotation type + * @author Josh Cummings + * @since 6.4 + */ +final class UniqueMergedAnnotationSynthesizer implements AnnotationSynthesizer { + + private final List> types; + + UniqueMergedAnnotationSynthesizer(Class type) { + Assert.notNull(type, "type cannot be null"); + this.types = List.of(type); + } + + UniqueMergedAnnotationSynthesizer(List> types) { + Assert.notNull(types, "types cannot be null"); + this.types = types; + } + + @Override + public MergedAnnotation merge(AnnotatedElement element, Class targetClass) { + if (element instanceof Parameter parameter) { + return handleParameterElement(parameter); + } + if (element instanceof Method method) { + return handleMethodElement(method, targetClass); + } + throw new AnnotationConfigurationException("Unsupported element of type " + element.getClass()); + } + + private MergedAnnotation handleParameterElement(Parameter parameter) { + List> annotations = findDirectAnnotations(parameter); + return requireUnique(parameter, annotations); + } + + private MergedAnnotation handleMethodElement(Method method, Class targetClass) { + List> annotations = findMethodAnnotations(method, targetClass); + return requireUnique(method, annotations); + } + + private MergedAnnotation requireUnique(AnnotatedElement element, List> annotations) { + return switch (annotations.size()) { + case 0 -> null; + case 1 -> annotations.get(0); + default -> { + List synthesized = new ArrayList<>(); + for (MergedAnnotation annotation : annotations) { + synthesized.add(annotation.synthesize()); + } + throw new AnnotationConfigurationException(""" + Please ensure there is one unique annotation of type %s attributed to %s. \ + Found %d competing annotations: %s""".formatted(this.types, element, annotations.size(), + synthesized)); + } + }; + } + + private List> findMethodAnnotations(Method method, Class targetClass) { + List> annotations = findClosestMethodAnnotations(method, targetClass, new HashSet<>()); + if (!annotations.isEmpty()) { + return annotations; + } + return findClosestClassAnnotations(targetClass, new HashSet<>()); + } + + private List> findClosestMethodAnnotations(Method method, Class targetClass, + Set> classesToSkip) { + if (targetClass == null || classesToSkip.contains(targetClass) || targetClass == Object.class) { + return Collections.emptyList(); + } + classesToSkip.add(targetClass); + try { + Method methodToUse = targetClass.getDeclaredMethod(method.getName(), method.getParameterTypes()); + List> annotations = findDirectAnnotations(methodToUse); + if (!annotations.isEmpty()) { + return annotations; + } + } + catch (NoSuchMethodException ex) { + // move on + } + List> annotations = new ArrayList<>(); + annotations.addAll(findClosestMethodAnnotations(method, targetClass.getSuperclass(), classesToSkip)); + for (Class inter : targetClass.getInterfaces()) { + annotations.addAll(findClosestMethodAnnotations(method, inter, classesToSkip)); + } + return annotations; + } + + private List> findClosestClassAnnotations(Class targetClass, Set> classesToSkip) { + if (targetClass == null || classesToSkip.contains(targetClass) || targetClass == Object.class) { + return Collections.emptyList(); + } + classesToSkip.add(targetClass); + List> annotations = new ArrayList<>(findDirectAnnotations(targetClass)); + if (!annotations.isEmpty()) { + return annotations; + } + annotations.addAll(findClosestClassAnnotations(targetClass.getSuperclass(), classesToSkip)); + for (Class inter : targetClass.getInterfaces()) { + annotations.addAll(findClosestClassAnnotations(inter, classesToSkip)); + } + return annotations; + } + + private List> findDirectAnnotations(AnnotatedElement element) { + MergedAnnotations mergedAnnotations = MergedAnnotations.from(element, MergedAnnotations.SearchStrategy.DIRECT, + RepeatableContainers.none()); + return mergedAnnotations.stream() + .filter((annotation) -> this.types.contains(annotation.getType())) + .map((annotation) -> (MergedAnnotation) annotation) + .toList(); + } + +} diff --git a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationAnnotationUtilsTests.java b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationAnnotationUtilsTests.java deleted file mode 100644 index f6bf450afd..0000000000 --- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationAnnotationUtilsTests.java +++ /dev/null @@ -1,186 +0,0 @@ -/* - * Copyright 2002-2024 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.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.reflect.Method; -import java.lang.reflect.Proxy; -import java.util.List; - -import org.junit.jupiter.api.Test; - -import org.springframework.core.annotation.AliasFor; -import org.springframework.core.annotation.AnnotationConfigurationException; -import org.springframework.security.access.prepost.PreAuthorize; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; - -/** - * Tests for {@link AuthorizationAnnotationUtils}. - * - * @author Josh Cummings - * @author Sam Brannen - */ -class AuthorizationAnnotationUtilsTests { - - @Test // gh-13132 - void annotationsOnSyntheticMethodsShouldNotTriggerAnnotationConfigurationException() throws NoSuchMethodException { - StringRepository proxy = (StringRepository) Proxy.newProxyInstance( - Thread.currentThread().getContextClassLoader(), new Class[] { StringRepository.class }, - (p, m, args) -> null); - Method method = proxy.getClass().getDeclaredMethod("findAll"); - PreAuthorize preAuthorize = AuthorizationAnnotationUtils.findUniqueAnnotation(method, PreAuthorize.class); - assertThat(preAuthorize.value()).isEqualTo("hasRole('someRole')"); - } - - @Test // gh-13625 - void annotationsFromSuperSuperInterfaceShouldNotTriggerAnnotationConfigurationException() throws Exception { - Method method = HelloImpl.class.getDeclaredMethod("sayHello"); - PreAuthorize preAuthorize = AuthorizationAnnotationUtils.findUniqueAnnotation(method, PreAuthorize.class); - assertThat(preAuthorize.value()).isEqualTo("hasRole('someRole')"); - } - - @Test - void multipleIdenticalAnnotationsOnClassShouldNotTriggerAnnotationConfigurationException() { - Class clazz = MultipleIdenticalPreAuthorizeAnnotationsOnClass.class; - PreAuthorize preAuthorize = AuthorizationAnnotationUtils.findUniqueAnnotation(clazz, PreAuthorize.class); - assertThat(preAuthorize.value()).isEqualTo("hasRole('someRole')"); - } - - @Test - void multipleIdenticalAnnotationsOnMethodShouldNotTriggerAnnotationConfigurationException() throws Exception { - Method method = MultipleIdenticalPreAuthorizeAnnotationsOnMethod.class.getDeclaredMethod("method"); - PreAuthorize preAuthorize = AuthorizationAnnotationUtils.findUniqueAnnotation(method, PreAuthorize.class); - assertThat(preAuthorize.value()).isEqualTo("hasRole('someRole')"); - } - - @Test - void competingAnnotationsOnClassShouldTriggerAnnotationConfigurationException() { - Class clazz = CompetingPreAuthorizeAnnotationsOnClass.class; - assertThatExceptionOfType(AnnotationConfigurationException.class) - .isThrownBy(() -> AuthorizationAnnotationUtils.findUniqueAnnotation(clazz, PreAuthorize.class)) - .withMessageContainingAll("Found 2 competing annotations:", "someRole", "otherRole"); - } - - @Test - void competingAnnotationsOnMethodShouldTriggerAnnotationConfigurationException() throws Exception { - Method method = CompetingPreAuthorizeAnnotationsOnMethod.class.getDeclaredMethod("method"); - assertThatExceptionOfType(AnnotationConfigurationException.class) - .isThrownBy(() -> AuthorizationAnnotationUtils.findUniqueAnnotation(method, PreAuthorize.class)) - .withMessageContainingAll("Found 2 competing annotations:", "someRole", "otherRole"); - } - - @Test - void composedMergedAnnotationsAreNotSupported() { - Class clazz = ComposedPreAuthAnnotationOnClass.class; - PreAuthorize preAuthorize = AuthorizationAnnotationUtils.findUniqueAnnotation(clazz, PreAuthorize.class); - assertThat(preAuthorize.value()).isEqualTo("hasRole('composedRole')"); - } - - private interface BaseRepository { - - Iterable findAll(); - - } - - private interface StringRepository extends BaseRepository { - - @Override - @PreAuthorize("hasRole('someRole')") - List findAll(); - - } - - private interface Hello { - - @PreAuthorize("hasRole('someRole')") - String sayHello(); - - } - - private interface SayHello extends Hello { - - } - - private static class HelloImpl implements SayHello { - - @Override - public String sayHello() { - return "hello"; - } - - } - - @Retention(RetentionPolicy.RUNTIME) - @PreAuthorize("hasRole('someRole')") - private @interface RequireSomeRole { - - } - - @Retention(RetentionPolicy.RUNTIME) - @PreAuthorize("hasRole('otherRole')") - private @interface RequireOtherRole { - - } - - @RequireSomeRole - @PreAuthorize("hasRole('someRole')") - private static class MultipleIdenticalPreAuthorizeAnnotationsOnClass { - - } - - private static class MultipleIdenticalPreAuthorizeAnnotationsOnMethod { - - @RequireSomeRole - @PreAuthorize("hasRole('someRole')") - void method() { - } - - } - - @RequireOtherRole - @PreAuthorize("hasRole('someRole')") - private static class CompetingPreAuthorizeAnnotationsOnClass { - - } - - private static class CompetingPreAuthorizeAnnotationsOnMethod { - - @RequireOtherRole - @PreAuthorize("hasRole('someRole')") - void method() { - } - - } - - @Retention(RetentionPolicy.RUNTIME) - @PreAuthorize("hasRole('metaRole')") - private @interface ComposedPreAuth { - - @AliasFor(annotation = PreAuthorize.class) - String value(); - - } - - @ComposedPreAuth("hasRole('composedRole')") - private static class ComposedPreAuthAnnotationOnClass { - - } - -} diff --git a/core/src/test/java/org/springframework/security/core/annotation/UniqueMergedAnnotationSynthesizerTests.java b/core/src/test/java/org/springframework/security/core/annotation/UniqueMergedAnnotationSynthesizerTests.java new file mode 100644 index 0000000000..2df5d7362a --- /dev/null +++ b/core/src/test/java/org/springframework/security/core/annotation/UniqueMergedAnnotationSynthesizerTests.java @@ -0,0 +1,549 @@ +/* + * Copyright 2002-2024 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.core.annotation; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.core.annotation.AnnotationConfigurationException; +import org.springframework.security.access.prepost.PreAuthorize; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * Tests for {@link UniqueMergedAnnotationSynthesizer} + */ +public class UniqueMergedAnnotationSynthesizerTests { + + private UniqueMergedAnnotationSynthesizer synthesizer = new UniqueMergedAnnotationSynthesizer<>( + PreAuthorize.class); + + @Test + void synthesizeWhenAnnotationOnInterfaceThenResolves() throws Exception { + Method method = AnnotationOnInterface.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("one"); + } + + @Test + void synthesizeWhenAnnotationOnMethodThenResolves() throws Exception { + Method method = AnnotationOnInterfaceMethod.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("three"); + } + + @Test + void synthesizeWhenAnnotationOnClassThenResolves() throws Exception { + Method method = AnnotationOnClass.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("five"); + } + + @Test + void synthesizeWhenAnnotationOnClassMethodThenResolves() throws Exception { + Method method = AnnotationOnClassMethod.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("six"); + } + + @Test + void synthesizeWhenInterfaceOverridingAnnotationOnInterfaceThenResolves() throws Exception { + Method method = InterfaceMethodOverridingAnnotationOnInterface.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("eight"); + } + + @Test + void synthesizeWhenInterfaceOverridingMultipleInterfaceInheritanceThenResolves() throws Exception { + Method method = InterfaceOverridingMultipleInterfaceInheritance.class.getMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method, + InterfaceOverridingMultipleInterfaceInheritance.class); + assertThat(preAuthorize.value()).isEqualTo("ten"); + } + + @Test + void synthesizeWhenInterfaceMethodOverridingAnnotationOnInterfaceThenResolves() throws Exception { + Method method = InterfaceMethodOverridingMultipleInterfaceInheritance.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("eleven"); + } + + @Test + void synthesizeWhenClassMultipleInheritanceThenException() throws Exception { + Method method = ClassAttemptingMultipleInterfaceInheritance.class.getDeclaredMethod("method"); + assertThatExceptionOfType(AnnotationConfigurationException.class) + .isThrownBy(() -> this.synthesizer.synthesize(method)); + } + + // gh-15097 + @Test + void synthesizeWhenClassOverridingMultipleInterfaceInheritanceThenResolves() throws Exception { + Method method = ClassOverridingMultipleInterfaceInheritance.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("thirteen"); + } + + @Test + void synthesizeWhenClassMethodOverridingMultipleInterfaceInheritanceThenResolves() throws Exception { + Method method = ClassMethodOverridingMultipleInterfaceInheritance.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("fourteen"); + } + + @Test + void synthesizeWhenClassInheritingInterfaceOverridingInterfaceAnnotationThenResolves() throws Exception { + Method method = ClassInheritingInterfaceOverridingInterfaceAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("seven"); + } + + @Test + void synthesizeWhenClassOverridingGrandparentInterfaceAnnotationThenResolves() throws Exception { + Method method = ClassOverridingGrandparentInterfaceAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("sixteen"); + } + + @Test + void synthesizeWhenMethodOverridingGrandparentInterfaceAnnotationThenResolves() throws Exception { + Method method = MethodOverridingGrandparentInterfaceAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("seventeen"); + } + + @Test + void synthesizeWhenClassInheritingMethodOverriddenAnnotationThenResolves() throws Exception { + Method method = ClassInheritingMethodOverriddenAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("eight"); + } + + @Test + void synthesizeWhenClassOverridingMethodOverriddenAnnotationThenResolves() throws Exception { + Method method = ClassOverridingMethodOverriddenAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("eight"); + } + + @Test + void synthesizeWhenMethodOverridingMethodOverriddenAnnotationThenResolves() throws Exception { + Method method = MethodOverridingMethodOverriddenAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("twenty"); + } + + @Test + void synthesizeWhenClassInheritingMultipleInheritanceThenException() throws Exception { + Method method = ClassInheritingMultipleInheritance.class.getDeclaredMethod("method"); + assertThatExceptionOfType(AnnotationConfigurationException.class) + .isThrownBy(() -> this.synthesizer.synthesize(method)); + } + + @Test + void synthesizeWhenClassOverridingMultipleInheritanceThenResolves() throws Exception { + Method method = ClassOverridingMultipleInheritance.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("twentytwo"); + } + + @Test + void synthesizeWhenMethodOverridingMultipleInheritanceThenResolves() throws Exception { + Method method = MethodOverridingMultipleInheritance.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("twentythree"); + } + + @Test + void synthesizeWhenInheritingInterfaceAndMethodAnnotationsThenResolves() throws Exception { + Method method = InheritingInterfaceAndMethodAnnotations.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("three"); + } + + @Test + void synthesizeWhenClassOverridingInterfaceAndMethodInheritanceThenResolves() throws Exception { + Method method = ClassOverridingInterfaceAndMethodInheritance.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("three"); + } + + @Test + void synthesizeWhenMethodOverridingInterfaceAndMethodInheritanceThenResolves() throws Exception { + Method method = MethodOverridingInterfaceAndMethodInheritance.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("twentysix"); + } + + @Test + void synthesizeWhenMultipleMethodInheritanceThenException() throws Exception { + Method method = MultipleMethodInheritance.class.getDeclaredMethod("method"); + assertThatExceptionOfType(AnnotationConfigurationException.class) + .isThrownBy(() -> this.synthesizer.synthesize(method)); + } + + // gh-13234 + @Test + void synthesizeWhenClassInheritingGrandparentInterfaceAnnotationThenResolves() throws Exception { + Method method = ClassInheritingGrandparentInterfaceAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("one"); + } + + @Test + void synthesizeWhenMethodInheritingMethodOverridingInterfaceAndMethodInheritanceThenResolves() throws Exception { + Method method = MethodInheritingMethodOverridingInterfaceAndMethodInheritance.class.getMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("twentysix"); + } + + @Test + void synthesizeWhenClassOverridingMethodOverridingInterfaceAndMethodInheritanceThenResolves() throws Exception { + Method method = ClassOverridingMethodOverridingInterfaceAndMethodInheritance.class.getMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method, + ClassOverridingMethodOverridingInterfaceAndMethodInheritance.class); + assertThat(preAuthorize.value()).isEqualTo("twentysix"); + } + + @Test + void synthesizeWhenInterfaceInheritingAnnotationsAtDifferentLevelsThenException() throws Exception { + Method method = InterfaceInheritingAnnotationsAtDifferentLevels.class.getMethod("method"); + assertThatExceptionOfType(AnnotationConfigurationException.class) + .isThrownBy(() -> this.synthesizer.synthesize(method)); + } + + @Test + void synthesizeWhenClassMethodOverridingAnnotationOnMethodThenResolves() throws Exception { + Method method = ClassMethodOverridingAnnotationOnMethod.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("twentyeight"); + } + + // gh-13490 + @Test + void synthesizeWhenClassInheritingInterfaceInheritingInterfaceMethodAnnotationThenResolves() throws Exception { + Method method = ClassInheritingInterfaceInheritingInterfaceMethodAnnotation.class.getDeclaredMethod("method"); + PreAuthorize preAuthorize = this.synthesizer.synthesize(method); + assertThat(preAuthorize.value()).isEqualTo("three"); + } + + @PreAuthorize("one") + private interface AnnotationOnInterface { + + String method(); + + } + + @PreAuthorize("two") + private interface AlsoAnnotationOnInterface { + + String method(); + + } + + private interface AnnotationOnInterfaceMethod { + + @PreAuthorize("three") + String method(); + + } + + private interface AlsoAnnotationOnInterfaceMethod { + + @PreAuthorize("four") + String method(); + + } + + @PreAuthorize("five") + private static class AnnotationOnClass { + + String method() { + return "ok"; + } + + } + + private static class AnnotationOnClassMethod { + + @PreAuthorize("six") + String method() { + return "ok"; + } + + } + + @PreAuthorize("seven") + private interface InterfaceOverridingAnnotationOnInterface extends AnnotationOnInterface { + + } + + private interface InterfaceMethodOverridingAnnotationOnInterface extends AnnotationOnInterface { + + @PreAuthorize("eight") + String method(); + + } + + private interface InterfaceAttemptingMultipleInterfaceInheritance + extends AnnotationOnInterface, AlsoAnnotationOnInterface { + + } + + @PreAuthorize("ten") + private interface InterfaceOverridingMultipleInterfaceInheritance + extends AnnotationOnInterface, AlsoAnnotationOnInterface { + + } + + private interface InterfaceMethodOverridingMultipleInterfaceInheritance + extends AnnotationOnInterface, AlsoAnnotationOnInterface { + + @PreAuthorize("eleven") + String method(); + + } + + private static class ClassAttemptingMultipleInterfaceInheritance + implements AnnotationOnInterface, AlsoAnnotationOnInterface { + + @Override + public String method() { + return "ok"; + } + + } + + @PreAuthorize("thirteen") + private static class ClassOverridingMultipleInterfaceInheritance + implements AnnotationOnInterface, AlsoAnnotationOnInterface { + + @Override + public String method() { + return "ok"; + } + + } + + private static class ClassMethodOverridingMultipleInterfaceInheritance + implements AnnotationOnInterface, AlsoAnnotationOnInterface { + + @Override + @PreAuthorize("fourteen") + public String method() { + return "ok"; + } + + } + + private static class ClassInheritingInterfaceOverridingInterfaceAnnotation + implements InterfaceOverridingAnnotationOnInterface { + + @Override + public String method() { + return "ok"; + } + + } + + @PreAuthorize("sixteen") + private static class ClassOverridingGrandparentInterfaceAnnotation + implements InterfaceOverridingAnnotationOnInterface { + + @Override + public String method() { + return "ok"; + } + + } + + private static class MethodOverridingGrandparentInterfaceAnnotation + implements InterfaceOverridingAnnotationOnInterface { + + @Override + @PreAuthorize("seventeen") + public String method() { + return "ok"; + } // unambiguously seventeen + + } + + private static class ClassInheritingMethodOverriddenAnnotation + implements InterfaceMethodOverridingAnnotationOnInterface { + + @Override + public String method() { + return "ok"; + } + + } + + @PreAuthorize("nineteen") + private static class ClassOverridingMethodOverriddenAnnotation + implements InterfaceMethodOverridingAnnotationOnInterface { + + @Override + public String method() { + return "ok"; + } + + } + + private static class MethodOverridingMethodOverriddenAnnotation + implements InterfaceMethodOverridingAnnotationOnInterface { + + @Override + @PreAuthorize("twenty") + public String method() { + return "ok"; + } + + } + + private static class ClassInheritingMultipleInheritance implements InterfaceAttemptingMultipleInterfaceInheritance { + + @Override + public String method() { + return "ok"; + } + + } + + @PreAuthorize("twentytwo") + private static class ClassOverridingMultipleInheritance implements InterfaceAttemptingMultipleInterfaceInheritance { + + @Override + public String method() { + return "ok"; + } + + } + + private static class MethodOverridingMultipleInheritance + implements InterfaceAttemptingMultipleInterfaceInheritance { + + @Override + @PreAuthorize("twentythree") + public String method() { + return "ok"; + } + + } + + private static class InheritingInterfaceAndMethodAnnotations + implements AnnotationOnInterface, AnnotationOnInterfaceMethod { + + @Override + public String method() { + return "ok"; + } + + } + + @PreAuthorize("twentyfive") + private static class ClassOverridingInterfaceAndMethodInheritance + implements AnnotationOnInterface, AnnotationOnInterfaceMethod { + + @Override + public String method() { + return "ok"; + } + + } + + private static class MethodOverridingInterfaceAndMethodInheritance + implements AnnotationOnInterface, AnnotationOnInterfaceMethod { + + @Override + @PreAuthorize("twentysix") + public String method() { + return "ok"; + } + + } + + private static class MultipleMethodInheritance + implements AnnotationOnInterfaceMethod, AlsoAnnotationOnInterfaceMethod { + + @Override + public String method() { + return "ok"; + } + + } + + private interface InterfaceInheritingInterfaceAnnotation extends AnnotationOnInterface { + + } + + private static class ClassInheritingGrandparentInterfaceAnnotation + implements InterfaceInheritingInterfaceAnnotation { + + @Override + public String method() { + return "ok"; + } + + } + + private static class MethodInheritingMethodOverridingInterfaceAndMethodInheritance + extends MethodOverridingInterfaceAndMethodInheritance { + + } + + @PreAuthorize("twentyseven") + private static class ClassOverridingMethodOverridingInterfaceAndMethodInheritance + extends MethodOverridingInterfaceAndMethodInheritance { + + } + + private static class InterfaceInheritingAnnotationsAtDifferentLevels + implements InterfaceInheritingInterfaceAnnotation, AlsoAnnotationOnInterface { + + @Override + public String method() { + return "ok"; + } + + } + + private static class ClassMethodOverridingAnnotationOnMethod implements AnnotationOnInterfaceMethod { + + @Override + @PreAuthorize("twentyeight") + public String method() { + return "ok"; + } + + } + + private interface InterfaceInheritingInterfaceMethodAnnotation extends AnnotationOnInterfaceMethod { + + } + + private static class ClassInheritingInterfaceInheritingInterfaceMethodAnnotation + implements InterfaceInheritingInterfaceMethodAnnotation { + + @Override + public String method() { + return "ok"; + } + + } + +}