ann
* the annotation of type {@code annotationType}, including any annotations using
* {@code annotationType} as a meta-annotation.
*
- * If more than one is found, then throw an error.
+ *
+ * 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 the unique instance of the annotation attributed to the method,
- * {@code null} otherwise
- * @throws AnnotationConfigurationException if more than one instance of the
+ * @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) {
- MergedAnnotations mergedAnnotations = MergedAnnotations.from(type,
- MergedAnnotations.SearchStrategy.TYPE_HIERARCHY, RepeatableContainers.none());
- if (hasDuplicate(mergedAnnotations, annotationType)) {
- throw new AnnotationConfigurationException("Found more than one annotation of type " + annotationType
- + " attributed to " + type
- + " Please remove the duplicate annotations and publish a bean to handle your authorization logic.");
- }
- return AnnotationUtils.findAnnotation(type, annotationType);
+ return findDistinctAnnotation(type, annotationType);
}
- private static boolean hasDuplicate(MergedAnnotations mergedAnnotations,
+ private static A findDistinctAnnotation(AnnotatedElement annotatedElement,
Class annotationType) {
- MergedAnnotation alreadyFound = null;
- for (MergedAnnotation mergedAnnotation : mergedAnnotations) {
- if (isSynthetic(mergedAnnotation.getSource())) {
- continue;
- }
-
- if (mergedAnnotation.getType() != annotationType) {
- continue;
- }
-
- if (alreadyFound == null) {
- alreadyFound = mergedAnnotation;
- continue;
- }
-
- // https://github.com/spring-projects/spring-framework/issues/31803
- if (!mergedAnnotation.getSource().equals(alreadyFound.getSource())) {
- return true;
- }
-
- if (mergedAnnotation.getRoot().getType() != alreadyFound.getRoot().getType()) {
- return true;
- }
- }
- return false;
- }
-
- private static boolean isSynthetic(Object object) {
- if (object instanceof Executable) {
- return ((Executable) object).isSynthetic();
- }
-
- return false;
+ MergedAnnotations mergedAnnotations = MergedAnnotations.from(annotatedElement, SearchStrategy.TYPE_HIERARCHY,
+ RepeatableContainers.none());
+
+ List annotations = mergedAnnotations.stream(annotationType)
+ .map(MergedAnnotation::withNonMergedAttributes)
+ .map(MergedAnnotation::synthesize)
+ .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/test/java/org/springframework/security/access/annotation/RequireUserRole.java b/core/src/test/java/org/springframework/security/access/annotation/RequireUserRole.java
index 575a326f01b..2e504b06831 100644
--- a/core/src/test/java/org/springframework/security/access/annotation/RequireUserRole.java
+++ b/core/src/test/java/org/springframework/security/access/annotation/RequireUserRole.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2021 the original author or authors.
+ * 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.
@@ -25,7 +25,7 @@
@Retention(RetentionPolicy.RUNTIME)
@PreAuthorize("hasRole('USER')")
-@RolesAllowed("ADMIN")
+@RolesAllowed("USER")
@Secured("USER")
public @interface RequireUserRole {
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
index 8c7cfc3ec8f..3dfc24a9e3d 100644
--- a/core/src/test/java/org/springframework/security/authorization/method/AuthorizationAnnotationUtilsTests.java
+++ b/core/src/test/java/org/springframework/security/authorization/method/AuthorizationAnnotationUtilsTests.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2023 the original author or authors.
+ * 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.
@@ -16,18 +16,26 @@
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.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
/**
- * Tests for {@link AuthorizationAnnotationUtils}
+ * Tests for {@link AuthorizationAnnotationUtils}.
+ *
+ * @author Josh Cummings
+ * @author Sam Brannen
*/
class AuthorizationAnnotationUtilsTests {
@@ -37,15 +45,56 @@ void annotationsOnSyntheticMethodsShouldNotTriggerAnnotationConfigurationExcepti
Thread.currentThread().getContextClassLoader(), new Class[] { StringRepository.class },
(p, m, args) -> null);
Method method = proxy.getClass().getDeclaredMethod("findAll");
- assertThatNoException()
- .isThrownBy(() -> AuthorizationAnnotationUtils.findUniqueAnnotation(method, PreAuthorize.class));
+ PreAuthorize preAuthorize = AuthorizationAnnotationUtils.findUniqueAnnotation(method, PreAuthorize.class);
+ assertThat(preAuthorize.value()).isEqualTo("hasRole('someRole')");
}
@Test // gh-13625
void annotationsFromSuperSuperInterfaceShouldNotTriggerAnnotationConfigurationException() throws Exception {
- Method method = HelloImpl.class.getMethod("sayHello");
- assertThatNoException()
- .isThrownBy(() -> AuthorizationAnnotationUtils.findUniqueAnnotation(method, PreAuthorize.class));
+ 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);
+
+ // If you comment out .map(MergedAnnotation::withNonMergedAttributes) in
+ // AuthorizationAnnotationUtils.findDistinctAnnotation(), the value of
+ // the merged annotation would be "hasRole('composedRole')".
+ assertThat(preAuthorize.value()).isEqualTo("hasRole('metaRole')");
}
private interface BaseRepository {
@@ -82,4 +131,60 @@ public String sayHello() {
}
+ @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 {
+
+ }
+
}