Support @AuthenticationPrincipal on interfaces

Closes spring-projectsgh-16177
kse-music · Dec 5, 2024 · 760ec9c · 760ec9c
1 parent dc82a6e
commit 760ec9c
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 7 deletions.
diff --git a/...in/java/org/springframework/security/core/annotation/UniqueSecurityAnnotationScanner.java b/...in/java/org/springframework/security/core/annotation/UniqueSecurityAnnotationScanner.java
@@ -18,9 +18,11 @@
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Executable;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -29,6 +31,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.springframework.core.MethodClassKey;
+import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.AnnotationConfigurationException;
 import org.springframework.core.annotation.MergedAnnotation;
 import org.springframework.core.annotation.MergedAnnotations;
@@ -107,7 +110,7 @@ final class UniqueSecurityAnnotationScanner<A extends Annotation> extends Abstra
 	MergedAnnotation<A> merge(AnnotatedElement element, Class<?> targetClass) {
 		if (element instanceof Parameter parameter) {
 			return this.uniqueParameterAnnotationCache.computeIfAbsent(parameter, (p) -> {
-				List<MergedAnnotation<A>> annotations = findDirectAnnotations(p);
+				List<MergedAnnotation<A>> annotations = findParameterAnnotations(p);
 				return requireUnique(p, annotations);
 			});
 		}
@@ -137,6 +140,64 @@ private MergedAnnotation<A> requireUnique(AnnotatedElement element, List<MergedA
 		};
 	}
 
+	private List<MergedAnnotation<A>> findParameterAnnotations(Method method, Class<?> superOrIfc, Parameter current) {
+		List<MergedAnnotation<A>> directAnnotations = Collections.emptyList();
+		for (Method candidate : superOrIfc.getMethods()) {
+			if (isOverrideFor(method, candidate)) {
+				for (Parameter parameter : candidate.getParameters()) {
+					if (parameter.getName().equals(current.getName())) {
+						directAnnotations = findDirectAnnotations(parameter);
+						if (!directAnnotations.isEmpty()) {
+							return directAnnotations;
+						}
+					}
+				}
+			}
+		}
+		return directAnnotations;
+	}
+
+	private List<MergedAnnotation<A>> findParameterAnnotations(Parameter current) {
+		List<MergedAnnotation<A>> directAnnotations = new ArrayList<>(findDirectAnnotations(current));
+		if (directAnnotations.isEmpty()) {
+			Executable executable = current.getDeclaringExecutable();
+			if (executable instanceof Method method) {
+				Class<?> clazz = method.getDeclaringClass();
+				while (clazz != null) {
+					for (Class<?> ifc : clazz.getInterfaces()) {
+						directAnnotations.addAll(findParameterAnnotations(method, ifc, current));
+					}
+					clazz = clazz.getSuperclass();
+					if (clazz == Object.class) {
+						clazz = null;
+					}
+					if (clazz != null) {
+						directAnnotations.addAll(findParameterAnnotations(method, clazz, current));
+					}
+				}
+			}
+		}
+		return directAnnotations;
+	}
+
+	private boolean isOverrideFor(Method method, Method candidate) {
+		if (!candidate.getName().equals(method.getName())
+				|| candidate.getParameterCount() != method.getParameterCount()) {
+			return false;
+		}
+		Class<?>[] paramTypes = method.getParameterTypes();
+		if (Arrays.equals(candidate.getParameterTypes(), paramTypes)) {
+			return true;
+		}
+		for (int i = 0; i < paramTypes.length; i++) {
+			if (paramTypes[i] != ResolvableType.forMethodParameter(candidate, i, method.getDeclaringClass())
+				.resolve()) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	private List<MergedAnnotation<A>> findMethodAnnotations(Method method, Class<?> targetClass) {
 		// The method may be on an interface, but we need attributes from the target
 		// class.

diff --git a/...ringframework/security/web/method/annotation/AuthenticationPrincipalArgumentResolver.java b/...ringframework/security/web/method/annotation/AuthenticationPrincipalArgumentResolver.java
@@ -19,6 +19,7 @@
 import java.lang.annotation.Annotation;
 
 import org.springframework.core.MethodParameter;
+import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.expression.BeanResolver;
 import org.springframework.expression.Expression;
 import org.springframework.expression.ExpressionParser;
@@ -98,8 +99,12 @@ public final class AuthenticationPrincipalArgumentResolver implements HandlerMet
 
 	private ExpressionParser parser = new SpelExpressionParser();
 
+	private final Class<AuthenticationPrincipal> annotationType = AuthenticationPrincipal.class;
+
 	private SecurityAnnotationScanner<AuthenticationPrincipal> scanner = SecurityAnnotationScanners
-		.requireUnique(AuthenticationPrincipal.class);
+		.requireUnique(this.annotationType);
+
+	private boolean useAnnotationTemplate = false;
 
 	private BeanResolver beanResolver;
 
@@ -164,7 +169,8 @@ public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy secur
 	 * @since 6.4
 	 */
 	public void setTemplateDefaults(AnnotationTemplateExpressionDefaults templateDefaults) {
-		this.scanner = SecurityAnnotationScanners.requireUnique(AuthenticationPrincipal.class, templateDefaults);
+		this.useAnnotationTemplate = templateDefaults != null;
+		this.scanner = SecurityAnnotationScanners.requireUnique(this.annotationType, templateDefaults);
 	}
 
 	/**
@@ -173,9 +179,22 @@ public void setTemplateDefaults(AnnotationTemplateExpressionDefaults templateDef
 	 * @param parameter the {@link MethodParameter} to search for an {@link Annotation}
 	 * @return the {@link Annotation} that was found or null.
 	 */
-	@SuppressWarnings("unchecked")
-	private <T extends Annotation> T findMethodAnnotation(MethodParameter parameter) {
-		return (T) this.scanner.scan(parameter.getParameter());
+	private AuthenticationPrincipal findMethodAnnotation(MethodParameter parameter) {
+		if (this.useAnnotationTemplate) {
+			return this.scanner.scan(parameter.getParameter());
+		}
+		AuthenticationPrincipal annotation = parameter.getParameterAnnotation(this.annotationType);
+		if (annotation != null) {
+			return annotation;
+		}
+		Annotation[] annotationsToSearch = parameter.getParameterAnnotations();
+		for (Annotation toSearch : annotationsToSearch) {
+			annotation = AnnotationUtils.findAnnotation(toSearch.annotationType(), this.annotationType);
+			if (annotation != null) {
+				return annotation;
+			}
+		}
+		return null;
 	}
 
 }
diff --git a/...ramework/security/web/method/annotation/AuthenticationPrincipalArgumentResolverTests.java b/...ramework/security/web/method/annotation/AuthenticationPrincipalArgumentResolverTests.java
@@ -28,6 +28,7 @@
 
 import org.springframework.core.MethodParameter;
 import org.springframework.core.annotation.AliasFor;
+import org.springframework.core.annotation.AnnotationConfigurationException;
 import org.springframework.expression.BeanResolver;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.annotation.AnnotationTemplateExpressionDefaults;
@@ -214,6 +215,17 @@ public void resolveArgumentCustomMetaAnnotationTpl() throws Exception {
 			.isEqualTo(this.expectedPrincipal);
 	}
 
+	@Test
+	public void resolveArgumentAnnotationFromInterface() {
+		CustomUserPrincipal principal = new CustomUserPrincipal();
+		setAuthenticationPrincipal(principal);
+		this.resolver.setTemplateDefaults(new AnnotationTemplateExpressionDefaults());
+		assertThat(this.resolver.supportsParameter(getMethodParameter("getUserByInterface", CustomUserPrincipal.class)))
+			.isTrue();
+		assertThatExceptionOfType(AnnotationConfigurationException.class).isThrownBy(() -> this.resolver
+			.resolveArgument(getMethodParameter("username", CustomUserPrincipal.class), null, null, null));
+	}
+
 	private MethodParameter showUserNoAnnotation() {
 		return getMethodParameter("showUserNoAnnotation", String.class);
 	}
@@ -312,7 +324,31 @@ private void setAuthenticationPrincipal(Object principal) {
 
 	}
 
-	public static class TestController {
+	interface UserApi {
+
+		String getUserByInterface(@AuthenticationPrincipal CustomUserPrincipal user);
+
+		Object username(@AuthenticationPrincipal CustomUserPrincipal user);
+
+	}
+
+	interface UserPublicApi {
+
+		Object username(@AuthenticationPrincipal CustomUserPrincipal user);
+
+	}
+
+	public static class TestController implements UserApi, UserPublicApi {
+
+		@Override
+		public String getUserByInterface(CustomUserPrincipal user) {
+			return "";
+		}
+
+		@Override
+		public Object username(CustomUserPrincipal user) {
+			return user.getPrincipal();
+		}
 
 		public void showUserNoAnnotation(String user) {
 		}